Don't commit same buffer multiple times

In Qt we call flush() when we think the window might need to be
updated. It is also possible to trigger a flush while painting.

Two fixes:

1) If there are attempted flushes between beginPaint() and endPaint,
queue them up, and do them in endPaint().

2) Make sure we only commit the buffer once: after that the
compositor owns the buffer, and it can repaint on its own.

Change-Id: Ibf61068fa95760eb67dbc0b1d0534854114ea528
Reviewed-by: Johan Helsing <johan.helsing@qt.io>

6 files changed, 33 insertions, 6 deletions

diff --git a/src/client/qwaylandbuffer.cpp b/src/client/qwaylandbuffer.cpp
index 9792cdd61..12df9cc4f 100644
--- a/src/client/qwaylandbuffer.cpp
+++ b/src/client/qwaylandbuffer.cpp
@@ -64,7 +64,9 @@ void QWaylandBuffer::init(wl_buffer *buf)
 
 void QWaylandBuffer::release(void *data, wl_buffer *)
 {
-    static_cast<QWaylandBuffer *>(data)->mBusy = false;
+    QWaylandBuffer *self = static_cast<QWaylandBuffer *>(data);
+    self->mBusy = false;
+    self->mCommitted = false;
 }
 
 const wl_buffer_listener QWaylandBuffer::listener = {
diff --git a/src/client/qwaylandbuffer_p.h b/src/client/qwaylandbuffer_p.h
index 156ea9530..eea090f35 100644
--- a/src/client/qwaylandbuffer_p.h
+++ b/src/client/qwaylandbuffer_p.h
@@ -76,11 +76,15 @@ public:
     void setBusy() { mBusy = true; }
     bool busy() const { return mBusy; }
 
+    void setCommitted() { mCommitted = true; }
+    bool committed() const { return mCommitted; }
+
 protected:
     struct wl_buffer *mBuffer = nullptr;
 
 private:
     bool mBusy = false;
+    bool mCommitted = false;
 
     static void release(void *data, wl_buffer *);
     static const wl_buffer_listener listener;
diff --git a/src/client/qwaylandshmbackingstore.cpp b/src/client/qwaylandshmbackingstore.cpp
index dde01357b..ecb03c0d6 100644
--- a/src/client/qwaylandshmbackingstore.cpp
+++ b/src/client/qwaylandshmbackingstore.cpp
@@ -48,7 +48,6 @@
 #include <QtCore/qtemporaryfile.h>
 #include <QtGui/QPainter>
 #include <QMutexLocker>
-#include <QLoggingCategory>
 
 #include <wayland-client.h>
 #include <wayland-client-protocol.h>
@@ -68,10 +67,6 @@ QT_BEGIN_NAMESPACE
 
 namespace QtWaylandClient {
 
-Q_DECLARE_LOGGING_CATEGORY(lcWaylandBackingstore)
-
-Q_LOGGING_CATEGORY(lcWaylandBackingstore, "qt.qpa.wayland.backingstore")
-
 QWaylandShmBuffer::QWaylandShmBuffer(QWaylandDisplay *display,
                      const QSize &size, QImage::Format format, int scale)
 {
@@ -199,6 +194,8 @@ void QWaylandShmBackingStore::beginPaint(const QRegion &region)
 void QWaylandShmBackingStore::endPaint()
 {
     mPainting = false;
+    if (mPendingFlush)
+        flush(window(), mPendingRegion, QPoint());
     waylandWindow()->setCanResize(true);
 }
 
@@ -218,9 +215,19 @@ void QWaylandShmBackingStore::flush(QWindow *window, const QRegion &region, cons
     // called instead. The default implementation from QPlatformBackingStore is sufficient
     // however so no need to reimplement that.
 
+
     Q_UNUSED(window);
     Q_UNUSED(offset);
 
+    if (mPainting) {
+        mPendingRegion |= region;
+        mPendingFlush = true;
+        return;
+    }
+
+    mPendingFlush = false;
+    mPendingRegion = QRegion();
+
     if (windowDecoration() && windowDecoration()->isDirty())
         updateDecorations();
 
diff --git a/src/client/qwaylandshmbackingstore_p.h b/src/client/qwaylandshmbackingstore_p.h
index cb66288fc..88ecfc5ec 100644
--- a/src/client/qwaylandshmbackingstore_p.h
+++ b/src/client/qwaylandshmbackingstore_p.h
@@ -120,6 +120,8 @@ private:
     QWaylandShmBuffer *mFrontBuffer = nullptr;
     QWaylandShmBuffer *mBackBuffer = nullptr;
     bool mPainting = false;
+    bool mPendingFlush = false;
+    QRegion mPendingRegion;
     QMutex mMutex;
 
     QSize mRequestedSize;
diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
index 5d658f675..b88541ad8 100644
--- a/src/client/qwaylandwindow.cpp
+++ b/src/client/qwaylandwindow.cpp
@@ -75,6 +75,8 @@ QT_BEGIN_NAMESPACE
 
 namespace QtWaylandClient {
 
+Q_LOGGING_CATEGORY(lcWaylandBackingstore, "qt.qpa.wayland.backingstore")
+
 QWaylandWindow *QWaylandWindow::mMouseGrab = nullptr;
 
 QWaylandWindow::QWaylandWindow(QWindow *window)
@@ -556,6 +558,7 @@ void QWaylandWindow::handleScreenRemoved(QScreen *qScreen)
 
 void QWaylandWindow::attach(QWaylandBuffer *buffer, int x, int y)
 {
+    Q_ASSERT(!buffer->committed());
     if (mFrameCallback) {
         wl_callback_destroy(mFrameCallback);
         mFrameCallback = nullptr;
@@ -586,12 +589,18 @@ void QWaylandWindow::damage(const QRect &rect)
 
 void QWaylandWindow::commit(QWaylandBuffer *buffer, const QRegion &damage)
 {
+    if (buffer->committed()) {
+        qCDebug(lcWaylandBackingstore) << "Buffer already committed, ignoring.";
+        return;
+    }
     if (!isInitialized())
         return;
 
     attachOffset(buffer);
     for (const QRect &rect: damage)
         wl_surface::damage(rect.x(), rect.y(), rect.width(), rect.height());
+    Q_ASSERT(!buffer->committed());
+    buffer->setCommitted();
     wl_surface::commit();
 }
 
diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h
index 3324bf700..366548635 100644
--- a/src/client/qwaylandwindow_p.h
+++ b/src/client/qwaylandwindow_p.h
@@ -55,6 +55,7 @@
 #include <QtCore/QMutex>
 #include <QtGui/QIcon>
 #include <QtCore/QVariant>
+#include <QtCore/QLoggingCategory>
 
 #include <qpa/qplatformwindow.h>
 
@@ -67,6 +68,8 @@ QT_BEGIN_NAMESPACE
 
 namespace QtWaylandClient {
 
+Q_DECLARE_LOGGING_CATEGORY(lcWaylandBackingstore)
+
 class QWaylandDisplay;
 class QWaylandBuffer;
 class QWaylandShellSurface;