Client: Full implementation for frame callbacks

The Wayland plugin now takes full control over delivering update request and
implement frame callbacks for both egl and shm.

Fixes two bugs:

[ChangeLog][Client] The non-blocking version of eglSwapBuffers is now used.
This fixed a bug where minimized windows would block the event loop.

Also, when we relied on the QPA version of requestUpdate, we would sometimes
deliver one update request while we were waiting for a frame callback. When we
implement the fallback timer ourselves we can make sure we only deliver the
fallback if there are no pending frame callbacks.

Fixes: QTBUG-69077
Change-Id: I2d3a6896c32e63d8520b57448a3601a817816a91
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>

3 files changed, 103 insertions, 35 deletions

diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
index 4ac2ca51e..f596438b6 100644
--- a/src/client/qwaylandwindow.cpp
+++ b/src/client/qwaylandwindow.cpp
@@ -358,6 +358,8 @@ void QWaylandWindow::sendExposeEvent(const QRect &rect)
 {
     if (!(mShellSurface && mShellSurface->handleExpose(rect)))
         QWindowSystemInterface::handleExposeEvent(window(), rect);
+    else
+        qCDebug(lcQpaWayland) << "sendExposeEvent: intercepted by shell extension, not sending";
     mLastExposeGeometry = rect;
 }
 
@@ -542,18 +544,11 @@ 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;
-    }
-
     if (buffer) {
-        mFrameCallback = frame();
-        wl_callback_add_listener(mFrameCallback, &QWaylandWindow::callbackListener, this);
-        mWaitingForFrameSync = true;
+        handleUpdate();
         buffer->setBusy();
 
-        attach(buffer->buffer(), x, y);
+        QtWayland::wl_surface::attach(buffer->buffer(), x, y);
     } else {
         QtWayland::wl_surface::attach(nullptr, 0, 0);
     }
@@ -618,11 +613,9 @@ void QWaylandWindow::frameCallback(void *data, struct wl_callback *callback, uin
     Q_UNUSED(callback);
     QWaylandWindow *self = static_cast<QWaylandWindow*>(data);
 
-    self->mWaitingForFrameSync = false;
-    if (self->mUpdateRequested) {
-        self->mUpdateRequested = false;
+    self->mWaitingForFrameCallback = false;
+    if (self->mUpdateRequested)
         self->deliverUpdateRequest();
-    }
 }
 
 QMutex QWaylandWindow::mFrameSyncMutex;
@@ -630,10 +623,10 @@ QMutex QWaylandWindow::mFrameSyncMutex;
 void QWaylandWindow::waitForFrameSync()
 {
     QMutexLocker locker(&mFrameSyncMutex);
-    if (!mWaitingForFrameSync)
+    if (!mWaitingForFrameCallback)
         return;
     mDisplay->flushRequests();
-    while (mWaitingForFrameSync)
+    while (mWaitingForFrameCallback)
         mDisplay->blockingReadEvents();
 }
 
@@ -1034,12 +1027,88 @@ QVariant QWaylandWindow::property(const QString &name, const QVariant &defaultVa
     return m_properties.value(name, defaultValue);
 }
 
+void QWaylandWindow::timerEvent(QTimerEvent *event)
+{
+    if (event->timerId() == mFallbackUpdateTimerId) {
+        killTimer(mFallbackUpdateTimerId);
+        mFallbackUpdateTimerId = -1;
+
+        if (!isExposed()) {
+            qCDebug(lcWaylandBackingstore) << "Fallback update timer: Window not exposed,"
+                                           << "not delivering update request.";
+            return;
+        }
+
+        if (mWaitingForUpdate && mUpdateRequested && !mWaitingForFrameCallback) {
+            qCWarning(lcWaylandBackingstore) << "Delivering update request through fallback timer,"
+                                             << "may not be in sync with display";
+            deliverUpdateRequest();
+        }
+    }
+}
+
 void QWaylandWindow::requestUpdate()
 {
-    if (!mWaitingForFrameSync)
-        QPlatformWindow::requestUpdate();
-    else
-        mUpdateRequested = true;
+    if (mUpdateRequested)
+        return;
+
+    mUpdateRequested = true;
+
+    // If we have a frame callback all is good and will be taken care of there
+    if (mWaitingForFrameCallback)
+        return;
+
+    // If we've already called deliverUpdateRequest(), but haven't seen any attach+commit/swap yet
+    if (mWaitingForUpdate) {
+        // Ideally, we should just have returned here, but we're not guaranteed that the client
+        // will actually update, so start this timer to deliver another request update after a while
+        // *IF* the client doesn't update.
+        int fallbackTimeout = 100;
+        mFallbackUpdateTimerId = startTimer(fallbackTimeout);
+        return;
+    }
+
+    // Some applications (such as Qt Quick) depend on updates being delivered asynchronously,
+    // so use invokeMethod to delay the delivery a bit.
+    QMetaObject::invokeMethod(this, [this] {
+        // Things might have changed in the meantime
+        if (mUpdateRequested && !mWaitingForUpdate && !mWaitingForFrameCallback)
+            deliverUpdateRequest();
+    }, Qt::QueuedConnection);
+}
+
+// Should be called whenever we commit a buffer (directly through wl_surface.commit or indirectly
+// with eglSwapBuffers) to know when it's time to commit the next one.
+// Can be called from the render thread (without locking anything) so make sure to not make races in this method.
+void QWaylandWindow::handleUpdate()
+{
+    // TODO: Should sync subsurfaces avoid requesting frame callbacks?
+
+    if (mFrameCallback) {
+        wl_callback_destroy(mFrameCallback);
+        mFrameCallback = nullptr;
+    }
+
+    if (mFallbackUpdateTimerId != -1) {
+        // Ideally, we would stop the fallback timer here, but since we're on another thread,
+        // it's not allowed. Instead we set mFallbackUpdateTimer to -1 here, so we'll just
+        // ignore it if it times out before it's cleaned up by the invokeMethod call.
+        int id = mFallbackUpdateTimerId;
+        mFallbackUpdateTimerId = -1;
+        QMetaObject::invokeMethod(this, [=] { killTimer(id); }, Qt::QueuedConnection);
+    }
+
+    mFrameCallback = frame();
+    wl_callback_add_listener(mFrameCallback, &QWaylandWindow::callbackListener, this);
+    mWaitingForFrameCallback = true;
+    mWaitingForUpdate = false;
+}
+
+void QWaylandWindow::deliverUpdateRequest()
+{
+    mUpdateRequested = false;
+    mWaitingForUpdate = true;
+    QPlatformWindow::deliverUpdateRequest();
 }
 
 void QWaylandWindow::addAttachOffset(const QPoint point)
diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h
index 56ebd3cc6..e5838d231 100644
--- a/src/client/qwaylandwindow_p.h
+++ b/src/client/qwaylandwindow_p.h
@@ -191,7 +191,10 @@ public:
 
     bool startSystemMove(const QPoint &pos) override;
 
+    void timerEvent(QTimerEvent *event) override;
     void requestUpdate() override;
+    void handleUpdate();
+    void deliverUpdateRequest() override;
 
 public slots:
     void applyConfigure();
@@ -211,10 +214,14 @@ protected:
     Qt::MouseButtons mMousePressedInContentArea = Qt::NoButton;
 
     WId mWindowId;
-    bool mWaitingForFrameSync = false;
+    bool mWaitingForFrameCallback = false;
     struct ::wl_callback *mFrameCallback = nullptr;
     QWaitCondition mFrameSyncWait;
 
+    // True when we have called deliverRequestUpdate, but the client has not yet attached a new buffer
+    bool mWaitingForUpdate = false;
+    int mFallbackUpdateTimerId = -1;
+
     QMutex mResizeLock;
     bool mWaitingToApplyConfigure = false;
     bool mCanResize = true;
diff --git a/src/hardwareintegration/client/wayland-egl/qwaylandglcontext.cpp b/src/hardwareintegration/client/wayland-egl/qwaylandglcontext.cpp
index e58403ad0..0cbbe5389 100644
--- a/src/hardwareintegration/client/wayland-egl/qwaylandglcontext.cpp
+++ b/src/hardwareintegration/client/wayland-egl/qwaylandglcontext.cpp
@@ -315,7 +315,9 @@ QWaylandGLContext::QWaylandGLContext(EGLDisplay eglDisplay, QWaylandDisplay *dis
        mSupportNonBlockingSwap = false;
     }
     if (!mSupportNonBlockingSwap) {
-        qWarning() << "Non-blocking swap buffers not supported. Subsurface rendering can be affected.";
+        qWarning(lcQpaWayland) << "Non-blocking swap buffers not supported."
+                               << "Subsurface rendering can be affected."
+                               << "It may also cause the event loop to freeze in some situations";
     }
 
     updateGLFormat();
@@ -550,20 +552,10 @@ void QWaylandGLContext::swapBuffers(QPlatformSurface *surface)
         m_blitter->blit(window);
     }
 
-
-    QWaylandSubSurface *sub = window->subSurfaceWindow();
-    if (sub) {
-        QMutexLocker l(sub->syncMutex());
-
-        int si = (sub->isSync() && mSupportNonBlockingSwap) ? 0 : m_format.swapInterval();
-
-        eglSwapInterval(m_eglDisplay, si);
-        eglSwapBuffers(m_eglDisplay, eglSurface);
-    } else {
-        eglSwapInterval(m_eglDisplay, m_format.swapInterval());
-        eglSwapBuffers(m_eglDisplay, eglSurface);
-    }
-
+    window->handleUpdate();
+    int swapInterval = mSupportNonBlockingSwap ? 0 : m_format.swapInterval();
+    eglSwapInterval(m_eglDisplay, swapInterval);
+    eglSwapBuffers(m_eglDisplay, eglSurface);
 
     window->setCanResize(true);
 }