Fix SHM drawing logic

The old logic didn't care to listen for wl_buffer.release events
so it always drew in the same buffer, potentially resulting in
tearing if the compositor was scanning out the buffer at the same time.
Instead properly cycle between a few buffers and don't reuse the same
one until the release event was received.

The old code also used to throttle the redraws, unless the buffer was
changing, that is unless the window was getting resized. This is now
lost, and no throttling is ever done. Doing it properly, by waiting
for the frame callback before committing the new buffer shows very
noticeable lags with many applications when resizing, because they
paint many times per resize event, so they fall behind the cursor.
A proper fix will be to implement the support for requestUpdate(),
and using it in the applications.

Change-Id: I02732c34769a5c75a6ad68c095bae916e4b274d3
Reviewed-by: Robin Burchell <robin.burchell@viroteck.net>

12 files changed, 163 insertions, 112 deletions

diff --git a/.gitignore b/.gitignore
index b33ca907..5bc0f373 100644
--- a/.gitignore
+++ b/.gitignore
@@ -40,7 +40,7 @@ examples/wayland/server-buffer/compositor/qwayland-server-*.cpp
 examples/wayland/server-buffer/compositor/qwayland-server-*.h
 examples/wayland/server-buffer/compositor/*-server-protocol.h
 examples/wayland/server-buffer/compositor/*-protocol.c
-src/client/qwayland*.cpp
+src/client/qwayland-*.cpp
 src/client/*-protocol.c
 src/compositor/*-server-protocol.h
 src/compositor/*-protocol.c
diff --git a/src/client/client.pro b/src/client/client.pro
index 2895e19d..59234b14 100644
--- a/src/client/client.pro
+++ b/src/client/client.pro
@@ -73,6 +73,7 @@ SOURCES +=  qwaylandintegration.cpp \
             qwaylandwindowmanagerintegration.cpp \
             qwaylandinputcontext.cpp \
             qwaylanddatadevice.cpp \
+            qwaylandbuffer.cpp \
 
 HEADERS +=  qwaylandintegration_p.h \
             qwaylandnativeinterface_p.h \
diff --git a/src/client/qwaylandbuffer.cpp b/src/client/qwaylandbuffer.cpp
new file mode 100644
index 00000000..c6aace2c
--- /dev/null
+++ b/src/client/qwaylandbuffer.cpp
@@ -0,0 +1,72 @@
+/****************************************************************************
+**
+** Copyright (C) 2015 The Qt Company Ltd.
+** Copyright (C) 2015 Giulio Camuffo.
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the plugins of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL21$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file. Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** As a special exception, The Qt Company gives you certain additional
+** rights. These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "qwaylandbuffer_p.h"
+
+#include <QDebug>
+
+QT_BEGIN_NAMESPACE
+
+namespace QtWaylandClient {
+
+QWaylandBuffer::QWaylandBuffer()
+              : mBuffer(0)
+              , mBusy(false)
+{
+}
+
+QWaylandBuffer::~QWaylandBuffer()
+{
+    if (mBuffer)
+        wl_buffer_destroy(mBuffer);
+}
+
+void QWaylandBuffer::init(wl_buffer *buf)
+{
+    mBuffer = buf;
+    wl_buffer_add_listener(buf, &listener, this);
+}
+
+void QWaylandBuffer::release(void *data, wl_buffer *)
+{
+    static_cast<QWaylandBuffer *>(data)->mBusy = false;
+}
+
+const wl_buffer_listener QWaylandBuffer::listener = {
+    QWaylandBuffer::release
+};
+
+}
+
+QT_END_NAMESPACE
diff --git a/src/client/qwaylandbuffer_p.h b/src/client/qwaylandbuffer_p.h
index 6f8f7b26..7db6cfef 100644
--- a/src/client/qwaylandbuffer_p.h
+++ b/src/client/qwaylandbuffer_p.h
@@ -59,17 +59,25 @@ namespace QtWaylandClient {
 
 class Q_WAYLAND_CLIENT_EXPORT QWaylandBuffer {
 public:
-    QWaylandBuffer()
-        : mBuffer(0)
-    {
-    }
-    virtual ~QWaylandBuffer() { }
+    QWaylandBuffer();
+    virtual ~QWaylandBuffer();
+    void init(wl_buffer *buf);
+
     wl_buffer *buffer() {return mBuffer;}
     virtual QSize size() const = 0;
     virtual int scale() const { return 1; }
 
+    void setBusy() { mBusy = true; }
+    bool busy() const { return mBusy; }
+
 protected:
     struct wl_buffer *mBuffer;
+
+private:
+    bool mBusy;
+
+    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 f009e081..3bbe24c3 100644
--- a/src/client/qwaylandshmbackingstore.cpp
+++ b/src/client/qwaylandshmbackingstore.cpp
@@ -39,6 +39,7 @@
 #include <QtCore/qdebug.h>
 #include <QtGui/QPainter>
 #include <QMutexLocker>
+#include <QLoggingCategory>
 
 #include <wayland-client.h>
 #include <wayland-client-protocol.h>
@@ -53,9 +54,14 @@ QT_BEGIN_NAMESPACE
 
 namespace QtWaylandClient {
 
+Q_DECLARE_LOGGING_CATEGORY(logCategory)
+
+Q_LOGGING_CATEGORY(logCategory, "qt.qpa.wayland.backingstore")
+
 QWaylandShmBuffer::QWaylandShmBuffer(QWaylandDisplay *display,
                      const QSize &size, QImage::Format format, int scale)
-    : mShmPool(0)
+    : QWaylandBuffer()
+    , mShmPool(0)
     , mMarginsImage(0)
 {
     int stride = size.width() * 4;
@@ -90,8 +96,8 @@ QWaylandShmBuffer::QWaylandShmBuffer(QWaylandDisplay *display,
     mImage.setDevicePixelRatio(qreal(scale));
 
     mShmPool = wl_shm_create_pool(display->shm(), fd, alloc);
-    mBuffer = wl_shm_pool_create_buffer(mShmPool,0, size.width(), size.height(),
-                                       stride, wl_format);
+    init(wl_shm_pool_create_buffer(mShmPool,0, size.width(), size.height(),
+                                       stride, wl_format));
     close(fd);
 }
 
@@ -100,8 +106,6 @@ QWaylandShmBuffer::~QWaylandShmBuffer(void)
     delete mMarginsImage;
     if (mImage.constBits())
         munmap((void *) mImage.constBits(), mImage.byteCount());
-    if (mBuffer)
-        wl_buffer_destroy(mBuffer);
     if (mShmPool)
         wl_shm_pool_destroy(mShmPool);
 }
@@ -139,9 +143,7 @@ QWaylandShmBackingStore::QWaylandShmBackingStore(QWindow *window)
     , mDisplay(QWaylandScreen::waylandScreenFromWindow(window)->display())
     , mFrontBuffer(0)
     , mBackBuffer(0)
-    , mFrontBufferIsDirty(false)
     , mPainting(false)
-    , mFrameCallback(0)
 {
 
 }
@@ -151,16 +153,10 @@ QWaylandShmBackingStore::~QWaylandShmBackingStore()
     if (QWaylandWindow *w = waylandWindow())
         w->setBackingStore(Q_NULLPTR);
 
-    if (mFrameCallback)
-        wl_callback_destroy(mFrameCallback);
-
 //    if (mFrontBuffer == waylandWindow()->attached())
 //        waylandWindow()->attach(0);
 
-    if (mFrontBuffer != mBackBuffer)
-        delete mFrontBuffer;
-
-    delete mBackBuffer;
+    qDeleteAll(mBuffers);
 }
 
 QPaintDevice *QWaylandShmBackingStore::paintDevice()
@@ -173,11 +169,7 @@ void QWaylandShmBackingStore::beginPaint(const QRegion &)
     mPainting = true;
     ensureSize();
 
-    QWaylandWindow *window = waylandWindow();
-    if (window->attached() && mBackBuffer == window->attached() && mFrameCallback)
-        window->waitForFrameSync();
-
-    window->setCanResize(false);
+    waylandWindow()->setCanResize(false);
 }
 
 void QWaylandShmBackingStore::endPaint()
@@ -188,11 +180,6 @@ void QWaylandShmBackingStore::endPaint()
 
 void QWaylandShmBackingStore::hidden()
 {
-    QMutexLocker lock(&mMutex);
-    if (mFrameCallback) {
-        wl_callback_destroy(mFrameCallback);
-        mFrameCallback = Q_NULLPTR;
-    }
 }
 
 void QWaylandShmBackingStore::ensureSize()
@@ -219,35 +206,15 @@ void QWaylandShmBackingStore::flush(QWindow *window, const QRegion &region, cons
 
     mFrontBuffer = mBackBuffer;
 
-    if (mFrameCallback) {
-        mFrontBufferIsDirty = true;
-        return;
-    }
-
-    mFrameCallback = waylandWindow()->frame();
-    wl_callback_add_listener(mFrameCallback,&frameCallbackListener,this);
     QMargins margins = windowDecorationMargins();
 
-    bool damageAll = false;
-    if (waylandWindow()->attached() != mFrontBuffer) {
-        delete waylandWindow()->attached();
-        damageAll = true;
-    }
     waylandWindow()->attachOffset(mFrontBuffer);
+    mFrontBuffer->setBusy();
 
-    if (damageAll) {
-        //need to damage it all, otherwise the attach offset may screw up
-        waylandWindow()->damage(QRect(QPoint(0,0), window->size()));
-    } else {
-        QVector<QRect> rects = region.rects();
-        for (int i = 0; i < rects.size(); i++) {
-            QRect rect = rects.at(i);
-            rect.translate(margins.left(),margins.top());
-            waylandWindow()->damage(rect);
-        }
-    }
+    QVector<QRect> rects = region.rects();
+    foreach (const QRect &rect, rects)
+        waylandWindow()->damage(rect.translated(margins.left(), margins.top()));
     waylandWindow()->commit();
-    mFrontBufferIsDirty = false;
 }
 
 void QWaylandShmBackingStore::resize(const QSize &size, const QRegion &)
@@ -255,22 +222,65 @@ void QWaylandShmBackingStore::resize(const QSize &size, const QRegion &)
     mRequestedSize = size;
 }
 
+QWaylandShmBuffer *QWaylandShmBackingStore::getBuffer(const QSize &size)
+{
+    foreach (QWaylandShmBuffer *b, mBuffers) {
+        if (!b->busy()) {
+            if (b->size() == size) {
+                return b;
+            } else {
+                mBuffers.removeOne(b);
+                if (mBackBuffer == b)
+                    mBackBuffer = 0;
+                delete b;
+            }
+        }
+    }
+
+    static const int MAX_BUFFERS = 5;
+    if (mBuffers.count() < MAX_BUFFERS) {
+        QImage::Format format = QPlatformScreen::platformScreenForWindow(window())->format();
+        QWaylandShmBuffer *b = new QWaylandShmBuffer(mDisplay, size, format, waylandWindow()->scale());
+        mBuffers.prepend(b);
+        return b;
+    }
+    return 0;
+}
+
 void QWaylandShmBackingStore::resize(const QSize &size)
 {
     QMargins margins = windowDecorationMargins();
     int scale = waylandWindow()->scale();
     QSize sizeWithMargins = (size + QSize(margins.left()+margins.right(),margins.top()+margins.bottom())) * scale;
 
-    QImage::Format format = QPlatformScreen::platformScreenForWindow(window())->format();
-
-    if (mBackBuffer != NULL && mBackBuffer->size() == sizeWithMargins)
-        return;
-
-    if (mBackBuffer != mFrontBuffer) {
-        delete mBackBuffer; //we delete the attached buffer when we flush
+    // We look for a free buffer to draw into. If the buffer is not the last buffer we used,
+    // that is mBackBuffer, and the size is the same we memcpy the old content into the new
+    // buffer so that QPainter is happy to find the stuff it had drawn before. If the new
+    // buffer has a different size it needs to be redrawn completely anyway, and if the buffer
+    // is the same the stuff is there already.
+    // You can exercise the different codepaths with weston, switching between the gl and the
+    // pixman renderer. With the gl renderer release events are sent early so we can effectively
+    // run single buffered, while with the pixman renderer we have to use two.
+    QWaylandShmBuffer *buffer = getBuffer(sizeWithMargins);
+    while (!buffer) {
+        qCDebug(logCategory, "QWaylandShmBackingStore: stalling waiting for a buffer to be released from the compositor...");
+
+        mDisplay->blockingReadEvents();
+        buffer = getBuffer(sizeWithMargins);
     }
 
-    mBackBuffer = new QWaylandShmBuffer(mDisplay, sizeWithMargins, format, scale);
+    int oldSize = mBackBuffer ? mBackBuffer->image()->byteCount() : 0;
+    // mBackBuffer may have been deleted here but if so it means its size was different so we wouldn't copy it anyway
+    if (mBackBuffer != buffer && oldSize == buffer->image()->byteCount()) {
+        memcpy(buffer->image()->bits(), mBackBuffer->image()->constBits(), buffer->image()->byteCount());
+    }
+    mBackBuffer = buffer;
+    // ensure the new buffer is at the beginning of the list so next time getBuffer() will pick
+    // it if possible
+    if (mBuffers.first() != buffer) {
+        mBuffers.removeOne(buffer);
+        mBuffers.prepend(buffer);
+    }
 
     if (windowDecoration() && window()->isVisible())
         windowDecoration()->update();
@@ -344,36 +354,6 @@ QImage QWaylandShmBackingStore::toImage() const
 }
 #endif // QT_NO_OPENGL
 
-void QWaylandShmBackingStore::done(void *data, wl_callback *callback, uint32_t time)
-{
-    Q_UNUSED(time);
-    QWaylandShmBackingStore *self =
-            static_cast<QWaylandShmBackingStore *>(data);
-    if (callback != self->mFrameCallback) // others, like QWaylandWindow, may trigger callbacks too
-        return;
-    QMutexLocker lock(&self->mMutex);
-    QWaylandWindow *window = self->waylandWindow();
-    wl_callback_destroy(self->mFrameCallback);
-    self->mFrameCallback = 0;
-
-
-    if (self->mFrontBufferIsDirty && !self->mPainting) {
-        self->mFrontBufferIsDirty = false;
-        self->mFrameCallback = wl_surface_frame(window->object());
-        wl_callback_add_listener(self->mFrameCallback,&self->frameCallbackListener,self);
-        if (self->mFrontBuffer != window->attached()) {
-            delete window->attached();
-        }
-        window->attachOffset(self->mFrontBuffer);
-        window->damage(QRect(QPoint(0,0),window->geometry().size()));
-        window->commit();
-    }
-}
-
-const struct wl_callback_listener QWaylandShmBackingStore::frameCallbackListener = {
-    QWaylandShmBackingStore::done
-};
-
 }
 
 QT_END_NAMESPACE
diff --git a/src/client/qwaylandshmbackingstore_p.h b/src/client/qwaylandshmbackingstore_p.h
index a1a6e19c..8b58c0b5 100644
--- a/src/client/qwaylandshmbackingstore_p.h
+++ b/src/client/qwaylandshmbackingstore_p.h
@@ -51,6 +51,7 @@
 #include <QtGui/QImage>
 #include <qpa/qplatformwindow.h>
 #include <QMutex>
+#include <QLinkedList>
 
 QT_BEGIN_NAMESPACE
 
@@ -107,22 +108,17 @@ public:
 
 private:
     void updateDecorations();
+    QWaylandShmBuffer *getBuffer(const QSize &size);
 
     QWaylandDisplay *mDisplay;
+    QLinkedList<QWaylandShmBuffer *> mBuffers;
     QWaylandShmBuffer *mFrontBuffer;
     QWaylandShmBuffer *mBackBuffer;
-    bool mFrontBufferIsDirty;
     bool mPainting;
     QMutex mMutex;
 
     QSize mRequestedSize;
     Qt::WindowFlags mCurrentWindowFlags;
-
-    static const struct wl_callback_listener frameCallbackListener;
-    static void done(void *data,
-             struct wl_callback *callback,
-             uint32_t time);
-    struct wl_callback *mFrameCallback;
 };
 
 }
diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
index fa4e20f9..caf24c0a 100644
--- a/src/client/qwaylandwindow.cpp
+++ b/src/client/qwaylandwindow.cpp
@@ -74,7 +74,6 @@ QWaylandWindow::QWaylandWindow(QWindow *window)
     , mMouseEventsInContentArea(false)
     , mMousePressedInContentArea(Qt::NoButton)
     , m_cursorShape(Qt::ArrowCursor)
-    , mBuffer(0)
     , mWaitingForFrameSync(false)
     , mFrameCallback(0)
     , mRequestResizeSent(false)
@@ -412,9 +411,8 @@ void QWaylandWindow::requestResize()
 
 void QWaylandWindow::attach(QWaylandBuffer *buffer, int x, int y)
 {
-    mBuffer = buffer;
-    if (mBuffer)
-        attach(mBuffer->buffer(), x, y);
+    if (buffer)
+        attach(buffer->buffer(), x, y);
     else
         QtWayland::wl_surface::attach(0, 0, 0);
 }
@@ -425,11 +423,6 @@ void QWaylandWindow::attachOffset(QWaylandBuffer *buffer)
     mOffset = QPoint();
 }
 
-QWaylandBuffer *QWaylandWindow::attached() const
-{
-    return mBuffer;
-}
-
 void QWaylandWindow::damage(const QRect &rect)
 {
     //We have to do sync stuff before calling damage, or we might
@@ -439,9 +432,7 @@ void QWaylandWindow::damage(const QRect &rect)
         wl_callback_add_listener(mFrameCallback,&QWaylandWindow::callbackListener,this);
         mWaitingForFrameSync = true;
     }
-    if (mBuffer) {
-        damage(rect.x(), rect.y(), rect.width(), rect.height());
-    }
+    damage(rect.x(), rect.y(), rect.width(), rect.height());
 }
 
 const wl_callback_listener QWaylandWindow::callbackListener = {
diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h
index be8f5308..701a9cdf 100644
--- a/src/client/qwaylandwindow_p.h
+++ b/src/client/qwaylandwindow_p.h
@@ -208,7 +208,6 @@ protected:
     Qt::MouseButtons mMousePressedInContentArea;
     Qt::CursorShape m_cursorShape;
 
-    QWaylandBuffer *mBuffer;
     WId mWindowId;
     bool mWaitingForFrameSync;
     struct wl_callback *mFrameCallback;
diff --git a/src/hardwareintegration/client/xcomposite-egl/qwaylandxcompositeeglwindow.cpp b/src/hardwareintegration/client/xcomposite-egl/qwaylandxcompositeeglwindow.cpp
index dc9b1fb7..329cc762 100644
--- a/src/hardwareintegration/client/xcomposite-egl/qwaylandxcompositeeglwindow.cpp
+++ b/src/hardwareintegration/client/xcomposite-egl/qwaylandxcompositeeglwindow.cpp
@@ -56,6 +56,7 @@ QWaylandXCompositeEGLWindow::QWaylandXCompositeEGLWindow(QWindow *window, QWayla
     , m_xWindow(0)
     , m_config(q_configFromGLFormat(glxIntegration->eglDisplay(), window->format(), true, EGL_WINDOW_BIT | EGL_PIXMAP_BIT))
     , m_surface(0)
+    , mBuffer(0)
 {
 }
 
diff --git a/src/hardwareintegration/client/xcomposite-egl/qwaylandxcompositeeglwindow.h b/src/hardwareintegration/client/xcomposite-egl/qwaylandxcompositeeglwindow.h
index c1e8242c..489097f8 100644
--- a/src/hardwareintegration/client/xcomposite-egl/qwaylandxcompositeeglwindow.h
+++ b/src/hardwareintegration/client/xcomposite-egl/qwaylandxcompositeeglwindow.h
@@ -66,6 +66,7 @@ private:
     Window m_xWindow;
     EGLConfig m_config;
     EGLSurface m_surface;
+    QWaylandBuffer *mBuffer;
 };
 
 }
diff --git a/src/hardwareintegration/client/xcomposite-glx/qwaylandxcompositeglxwindow.cpp b/src/hardwareintegration/client/xcomposite-glx/qwaylandxcompositeglxwindow.cpp
index 1675b8a5..3fa4b471 100644
--- a/src/hardwareintegration/client/xcomposite-glx/qwaylandxcompositeglxwindow.cpp
+++ b/src/hardwareintegration/client/xcomposite-glx/qwaylandxcompositeglxwindow.cpp
@@ -50,6 +50,7 @@ QWaylandXCompositeGLXWindow::QWaylandXCompositeGLXWindow(QWindow *window, QWayla
     , m_glxIntegration(glxIntegration)
     , m_xWindow(0)
     , m_config(qglx_findConfig(glxIntegration->xDisplay(), glxIntegration->screen(), window->format(), GLX_WINDOW_BIT | GLX_PIXMAP_BIT))
+    , mBuffer(0)
 {
 }
 
diff --git a/src/hardwareintegration/client/xcomposite-glx/qwaylandxcompositeglxwindow.h b/src/hardwareintegration/client/xcomposite-glx/qwaylandxcompositeglxwindow.h
index b0966d0f..e6fee1cf 100644
--- a/src/hardwareintegration/client/xcomposite-glx/qwaylandxcompositeglxwindow.h
+++ b/src/hardwareintegration/client/xcomposite-glx/qwaylandxcompositeglxwindow.h
@@ -65,6 +65,7 @@ private:
 
     Window m_xWindow;
     GLXFBConfig m_config;
+    QWaylandBuffer *mBuffer;
 };
 
 }