Client: Fix 100ms freeze when applications do not swap after deliverUpdateRequest

qt5-qtwayland.spec

@@ -3,7 +3,7 @@
 Summary: Qt5 - Wayland platform support and QtCompositor module
 Name:    qt5-%{qt_module}
 Version: 5.12.5
-Release: 1%{?dist}
+Release: 2%{?dist}
 
 License: LGPLv3
 Url:     http://www.qt.io
@@ -12,6 +12,7 @@ Source0: https://download.qt.io/official_releases/qt/%{majmin}/%{version}/submod
 
 # Upstream patches
 Patch0:  qtwayland-do-not-redraw-decorations-everytime.patch
+Patch1:  qtwayland-fix-100ms-freeze-when-apps-dont-swap-after-deliverupdaterequest.patch
 
 # Upstreamable patches
 # https://fedoraproject.org/wiki/Changes/Qt_Wayland_By_Default_On_Gnome
@@ -120,6 +121,9 @@ popd
 
 
 %changelog
+* Fri Oct 18 2019 Jan Grulich <jgrulich@redhat.com> - 5.12.5-2
+- Client: Fix 100ms freeze when applications do not swap after deliverUpdateRequest
+
 * Tue Sep 24 2019 Jan Grulich <jgrulich@redhat.com> - 5.12.5-1
 - 5.12.5
 

qtwayland-fix-100ms-freeze-when-apps-dont-swap-after-deliverupdaterequest.patch

@@ -0,0 +1,127 @@
+From 9f5b96225885f927727a57b6123d8550d6c373bb Mon Sep 17 00:00:00 2001
+From: Johan Klokkhammer Helsing <johan.helsing@qt.io>
+Date: Tue, 15 Oct 2019 09:51:43 +0200
+Subject: [PATCH] Client: Fix 100ms freeze when applications do not swap after deliverUpdateRequest
+
+[ChangeLog][QPA plugin] Fixed a 100 ms freeze that would occur if applications
+did not draw after receiving a deliverUpdateRequest().
+
+QtQuick does this at the start of animations. This should get rid of those
+backingstore warnings (and also remove a 100ms freeze before animations start
+in those instances).
+
+Fixes: QTBUG-76813
+Change-Id: Id366bf4a14f402fa44530ae46e7b66d9988c14f6
+Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>
+Reviewed-by: John Brooks <john.brooks@qt.io>
+---
+
+diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp
+index ae26ba0..8d34afd 100644
+--- a/src/client/qwaylandwindow.cpp
++++ b/src/client/qwaylandwindow.cpp
+@@ -1105,25 +1105,6 @@
+ 
+ void QWaylandWindow::timerEvent(QTimerEvent *event)
+ {
+-    if (event->timerId() == mFallbackUpdateTimerId) {
+-        killTimer(mFallbackUpdateTimerId);
+-        mFallbackUpdateTimerId = -1;
+-        qCDebug(lcWaylandBackingstore) << "mFallbackUpdateTimer timed out";
+-
+-        if (!isExposed()) {
+-            qCDebug(lcWaylandBackingstore) << "Fallback update timer: Window not exposed,"
+-                                           << "not delivering update request.";
+-            return;
+-        }
+-
+-        if (mWaitingForUpdate && hasPendingUpdateRequest() && !mWaitingForFrameCallback) {
+-            qCWarning(lcWaylandBackingstore) << "Delivering update request through fallback timer,"
+-                                             << "may not be in sync with display";
+-            deliverUpdateRequest();
+-        }
+-    }
+-
+-
+     if (mFrameCallbackTimerId.testAndSetOrdered(event->timerId(), -1)) {
+         killTimer(event->timerId());
+         qCDebug(lcWaylandBackingstore) << "Didn't receive frame callback in time, window should now be inexposed";
+@@ -1135,6 +1116,7 @@
+ 
+ void QWaylandWindow::requestUpdate()
+ {
++    qCDebug(lcWaylandBackingstore) << "requestUpdate";
+     Q_ASSERT(hasPendingUpdateRequest()); // should be set by QPA
+ 
+     // If we have a frame callback all is good and will be taken care of there
+@@ -1142,20 +1124,17 @@
+         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;
+-    }
++    // This is a somewhat redundant behavior and might indicate a bug in the calling code, so log
++    // here so we can get this information when debugging update/frame callback issues.
++    // Continue as nothing happened, though.
++    if (mWaitingForUpdate)
++        qCDebug(lcWaylandBackingstore) << "requestUpdate called twice without committing anything";
+ 
+     // 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 (hasPendingUpdateRequest() && !mWaitingForUpdate && !mWaitingForFrameCallback)
++        if (hasPendingUpdateRequest() && !mWaitingForFrameCallback)
+             deliverUpdateRequest();
+     }, Qt::QueuedConnection);
+ }
+@@ -1165,6 +1144,7 @@
+ // Can be called from the render thread (without locking anything) so make sure to not make races in this method.
+ void QWaylandWindow::handleUpdate()
+ {
++    qCDebug(lcWaylandBackingstore) << "handleUpdate" << QThread::currentThread();
+     // TODO: Should sync subsurfaces avoid requesting frame callbacks?
+     QReadLocker lock(&mSurfaceLock);
+     if (!isInitialized())
+@@ -1175,15 +1155,6 @@
+         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, [this, id] { killTimer(id); }, Qt::QueuedConnection);
+-    }
+-
+     mFrameCallback = frame();
+     wl_callback_add_listener(mFrameCallback, &QWaylandWindow::callbackListener, this);
+     mWaitingForFrameCallback = true;
+@@ -1203,6 +1174,7 @@
+ 
+ void QWaylandWindow::deliverUpdateRequest()
+ {
++    qCDebug(lcWaylandBackingstore) << "deliverUpdateRequest";
+     mWaitingForUpdate = true;
+     QPlatformWindow::deliverUpdateRequest();
+ }
+diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h
+index b03d92e..e4a1124 100644
+--- a/src/client/qwaylandwindow_p.h
++++ b/src/client/qwaylandwindow_p.h
+@@ -232,7 +232,6 @@
+ 
+     // True when we have called deliverRequestUpdate, but the client has not yet attached a new buffer
+     bool mWaitingForUpdate = false;
+-    int mFallbackUpdateTimerId = -1; // Started when waiting for app to commit
+ 
+     QMutex mResizeLock;
+     bool mWaitingToApplyConfigure = false;