From d7866249c2bb20a73e02593c59db8445f430b42d39f41c6d1aee293a24b425b0 Mon Sep 17 00:00:00 2001 From: Luca Beltrame Date: Wed, 3 Jul 2019 10:31:22 +0000 Subject: [PATCH] Accepting request 713204 from home:Vogtinator:qt5.13 - Add patch from upstream 5.13 branch to fix crashes: * 0001-Don-t-crash-if-we-start-a-drag-without-dragFocus.patch - Add patch from upstream 5.12 branch to improve performance: * 0002-Client-Fix-stuttering-when-the-GUI-thread-is-busy.patch - Add patches from upstream code reviews to fix various issues: * 0003-Client-Don-t-send-fake-SurfaceCreated-Destroyed-even.patch * 0004-Client-Make-handleUpdate-aware-of-exposure-changes.patch * 0005-Client-Reset-frame-callback-timer-when-hiding-a-wind.patch OBS-URL: https://build.opensuse.org/request/show/713204 OBS-URL: https://build.opensuse.org/package/show/KDE:Qt:5.13/libqt5-qtwayland?expand=0&rev=10 --- ...if-we-start-a-drag-without-dragFocus.patch | 36 +++++ ...uttering-when-the-GUI-thread-is-busy.patch | 134 ++++++++++++++++++ ...d-fake-SurfaceCreated-Destroyed-even.patch | 93 ++++++++++++ ...ndleUpdate-aware-of-exposure-changes.patch | 80 +++++++++++ ...me-callback-timer-when-hiding-a-wind.patch | 40 ++++++ libqt5-qtwayland.changes | 12 ++ libqt5-qtwayland.spec | 11 ++ 7 files changed, 406 insertions(+) create mode 100644 0001-Don-t-crash-if-we-start-a-drag-without-dragFocus.patch create mode 100644 0002-Client-Fix-stuttering-when-the-GUI-thread-is-busy.patch create mode 100644 0003-Client-Don-t-send-fake-SurfaceCreated-Destroyed-even.patch create mode 100644 0004-Client-Make-handleUpdate-aware-of-exposure-changes.patch create mode 100644 0005-Client-Reset-frame-callback-timer-when-hiding-a-wind.patch diff --git a/0001-Don-t-crash-if-we-start-a-drag-without-dragFocus.patch b/0001-Don-t-crash-if-we-start-a-drag-without-dragFocus.patch new file mode 100644 index 0000000..eea3c08 --- /dev/null +++ b/0001-Don-t-crash-if-we-start-a-drag-without-dragFocus.patch @@ -0,0 +1,36 @@ +From 4eb9a5a70506cf76fbe954c56b39857f8a3be0eb Mon Sep 17 00:00:00 2001 +From: Aleix Pol +Date: Wed, 12 Jun 2019 16:03:13 +0200 +Subject: [PATCH 1/5] Don't crash if we start a drag without dragFocus + +Sometimes origin will be nullptr, triggering a crash. + +[ChangeLog][QPA plugin] Fixed a crash that sometimes happened when +starting a drag-and-drop operation. + +Fixes: QTBUG-76368 +Change-Id: I8f4e6b05f073644834c3c72a8307dac5b897f626 +Reviewed-by: Johan Helsing +--- + src/client/qwaylanddatadevice.cpp | 5 ++++- + 1 file changed, 4 insertions(+), 1 deletion(-) + +diff --git a/src/client/qwaylanddatadevice.cpp b/src/client/qwaylanddatadevice.cpp +index 300c9de0..11984f9d 100644 +--- a/src/client/qwaylanddatadevice.cpp ++++ b/src/client/qwaylanddatadevice.cpp +@@ -111,7 +111,10 @@ void QWaylandDataDevice::startDrag(QMimeData *mimeData, QWaylandWindow *icon) + if (!origin) + origin = m_display->currentInputDevice()->touchFocus(); + +- start_drag(m_dragSource->object(), origin->object(), icon->object(), m_display->currentInputDevice()->serial()); ++ if (origin) ++ start_drag(m_dragSource->object(), origin->object(), icon->object(), m_display->currentInputDevice()->serial()); ++ else ++ qCDebug(lcQpaWayland) << "Couldn't start a drag because the origin window could not be found."; + } + + void QWaylandDataDevice::cancelDrag() +-- +2.22.0 + diff --git a/0002-Client-Fix-stuttering-when-the-GUI-thread-is-busy.patch b/0002-Client-Fix-stuttering-when-the-GUI-thread-is-busy.patch new file mode 100644 index 0000000..ac2134a --- /dev/null +++ b/0002-Client-Fix-stuttering-when-the-GUI-thread-is-busy.patch @@ -0,0 +1,134 @@ +From 8e72a9b2f6b4a7af3f16ceb4216a8cc3c6768b8a Mon Sep 17 00:00:00 2001 +From: Johan Klokkhammer Helsing +Date: Wed, 19 Jun 2019 14:05:22 +0200 +Subject: [PATCH 2/5] Client: Fix stuttering when the GUI thread is busy + +When we did invokeMethod for handling the frame callbacks, we had to wait for +the GUI thread to finish whatever it's doing before we would stop blocking. + +Fix it by clearing the frame callback timer and stop blocking immediately, +while delaying the rest of the work until it can be run on the other thread. + +Fixes: QTBUG-76397 +Change-Id: I343e4feac4838926b4fa2ccac2948988bc6c3bb7 +Reviewed-by: Paul Olav Tvete +--- + src/client/qwaylandwindow.cpp | 59 ++++++++++++++++++----------------- + src/client/qwaylandwindow_p.h | 2 +- + 2 files changed, 32 insertions(+), 29 deletions(-) + +diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp +index 8b2c1227..7fca9783 100644 +--- a/src/client/qwaylandwindow.cpp ++++ b/src/client/qwaylandwindow.cpp +@@ -614,29 +614,34 @@ const wl_callback_listener QWaylandWindow::callbackListener = { + Q_UNUSED(callback); + Q_UNUSED(time); + auto *window = static_cast(data); +- if (window->thread() != QThread::currentThread()) +- QMetaObject::invokeMethod(window, [=] { window->handleFrameCallback(); }, Qt::QueuedConnection); +- else +- window->handleFrameCallback(); ++ window->handleFrameCallback(); + } + }; + + void QWaylandWindow::handleFrameCallback() + { +- bool wasExposed = isExposed(); ++ // Stop the timer and stop waiting immediately ++ int timerId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1); ++ mWaitingForFrameCallback = false; + +- if (mFrameCallbackTimerId != -1) { +- killTimer(mFrameCallbackTimerId); +- mFrameCallbackTimerId = -1; +- } ++ // The rest can wait until we can run it on the correct thread ++ auto doHandleExpose = [this, timerId]() { ++ if (timerId != -1) ++ killTimer(timerId); + +- mWaitingForFrameCallback = false; +- mFrameCallbackTimedOut = false; ++ bool wasExposed = isExposed(); ++ mFrameCallbackTimedOut = false; ++ if (!wasExposed && isExposed()) // Did setting mFrameCallbackTimedOut make the window exposed? ++ sendExposeEvent(QRect(QPoint(), geometry().size())); ++ if (wasExposed && hasPendingUpdateRequest()) ++ deliverUpdateRequest(); ++ }; + +- if (!wasExposed && isExposed()) +- sendExposeEvent(QRect(QPoint(), geometry().size())); +- if (wasExposed && hasPendingUpdateRequest()) +- deliverUpdateRequest(); ++ if (thread() != QThread::currentThread()) { ++ QMetaObject::invokeMethod(this, doHandleExpose); ++ } else { ++ doHandleExpose(); ++ } + } + + QMutex QWaylandWindow::mFrameSyncMutex; +@@ -658,11 +663,11 @@ bool QWaylandWindow::waitForFrameSync(int timeout) + } + + // Stop current frame timer if any, can't use killTimer directly, because we might be on a diffent thread +- if (mFrameCallbackTimerId != -1) { +- int id = mFrameCallbackTimerId; +- mFrameCallbackTimerId = -1; +- QMetaObject::invokeMethod(this, [=] { killTimer(id); }, Qt::QueuedConnection); +- } ++ // Ordered semantics is needed to avoid stopping the timer twice and not miss it when it's ++ // started by other writes ++ int fcbId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1); ++ if (fcbId != -1) ++ QMetaObject::invokeMethod(this, [=] { killTimer(fcbId); }, Qt::QueuedConnection); + + return !mWaitingForFrameCallback; + } +@@ -1100,9 +1105,9 @@ void QWaylandWindow::timerEvent(QTimerEvent *event) + } + } + +- if (event->timerId() == mFrameCallbackTimerId) { +- killTimer(mFrameCallbackTimerId); +- mFrameCallbackTimerId = -1; ++ ++ if (mFrameCallbackTimerId.testAndSetOrdered(event->timerId(), -1)) { ++ killTimer(event->timerId()); + qCDebug(lcWaylandBackingstore) << "Didn't receive frame callback in time, window should now be inexposed"; + mFrameCallbackTimedOut = true; + mWaitingForUpdate = false; +@@ -1164,11 +1169,9 @@ void QWaylandWindow::handleUpdate() + mWaitingForUpdate = false; + + // Stop current frame timer if any, can't use killTimer directly, see comment above. +- if (mFrameCallbackTimerId != -1) { +- int id = mFrameCallbackTimerId; +- mFrameCallbackTimerId = -1; +- QMetaObject::invokeMethod(this, [=] { killTimer(id); }, Qt::QueuedConnection); +- } ++ int fcbId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1); ++ if (fcbId != -1) ++ QMetaObject::invokeMethod(this, [=] { killTimer(fcbId); }, Qt::QueuedConnection); + + // Start a timer for handling the case when the compositor stops sending frame callbacks. + QMetaObject::invokeMethod(this, [=] { // Again; can't do it directly +diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h +index 8c1ebe16..86139243 100644 +--- a/src/client/qwaylandwindow_p.h ++++ b/src/client/qwaylandwindow_p.h +@@ -222,7 +222,7 @@ protected: + WId mWindowId; + bool mWaitingForFrameCallback = false; + bool mFrameCallbackTimedOut = false; // Whether the frame callback has timed out +- int mFrameCallbackTimerId = -1; // Started on commit, reset on frame callback ++ QAtomicInt mFrameCallbackTimerId = -1; // Started on commit, reset on frame callback + struct ::wl_callback *mFrameCallback = nullptr; + struct ::wl_event_queue *mFrameQueue = nullptr; + QWaitCondition mFrameSyncWait; +-- +2.22.0 + diff --git a/0003-Client-Don-t-send-fake-SurfaceCreated-Destroyed-even.patch b/0003-Client-Don-t-send-fake-SurfaceCreated-Destroyed-even.patch new file mode 100644 index 0000000..4c48a22 --- /dev/null +++ b/0003-Client-Don-t-send-fake-SurfaceCreated-Destroyed-even.patch @@ -0,0 +1,93 @@ +From 20ddfe8dffc3be253d799bc3f939a1b0e388ed73 Mon Sep 17 00:00:00 2001 +From: David Edmundson +Date: Sun, 23 Jun 2019 15:09:51 +0200 +Subject: [PATCH 3/5] Client: Don't send fake SurfaceCreated/Destroyed events + +QPlatformSurface relates to the backing store. Not the wl_surface. +They are emitted by QPlatformWindow. + +Due to a previously incorrect usage by KDE developers it was faked to +emit the events when the wl_surface is created/hidden to keep behavior. + +With QtBase a9246c7132a2c8864d3ae6cebd260bb9ee711fcb this now causes an +issue as now QWidgets react to this event in a breaking way. + +Change-Id: I2f003bc9da85f032a0053677fd281152099fc9eb +--- + .../custom-extension/client-common/customextension.cpp | 9 +++++++-- + src/client/qwaylandwindow.cpp | 10 ++-------- + src/client/qwaylandwindow_p.h | 2 +- + 3 files changed, 10 insertions(+), 11 deletions(-) + +diff --git a/examples/wayland/custom-extension/client-common/customextension.cpp b/examples/wayland/custom-extension/client-common/customextension.cpp +index aa0cb58a..16f18fd7 100644 +--- a/examples/wayland/custom-extension/client-common/customextension.cpp ++++ b/examples/wayland/custom-extension/client-common/customextension.cpp +@@ -81,8 +81,13 @@ QWindow *CustomExtension::windowForSurface(struct ::wl_surface *surface) + + bool CustomExtension::eventFilter(QObject *object, QEvent *event) + { +- if (event->type() == QEvent::PlatformSurface +- && static_cast(event)->surfaceEventType() == QPlatformSurfaceEvent::SurfaceCreated) { ++ if (event->type() == QEvent::Expose) { ++ auto ee = static_cast(event); ++ ++ if ((ee->region().isNull())) { ++ return false; ++ } ++ + QWindow *window = qobject_cast(object); + Q_ASSERT(window); + window->removeEventFilter(this); +diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp +index 7fca9783..76975b27 100644 +--- a/src/client/qwaylandwindow.cpp ++++ b/src/client/qwaylandwindow.cpp +@@ -91,7 +91,7 @@ QWaylandWindow::~QWaylandWindow() + delete mWindowDecoration; + + if (isInitialized()) +- reset(false); ++ reset(); + + const QWindow *parent = window(); + foreach (QWindow *w, QGuiApplication::topLevelWindows()) { +@@ -117,8 +117,6 @@ void QWaylandWindow::initWindow() + + if (!isInitialized()) { + initializeWlSurface(); +- QPlatformSurfaceEvent e(QPlatformSurfaceEvent::SurfaceCreated); +- QGuiApplication::sendEvent(window(), &e); + } + + if (shouldCreateSubSurface()) { +@@ -224,12 +222,8 @@ bool QWaylandWindow::shouldCreateSubSurface() const + return QPlatformWindow::parent() != nullptr; + } + +-void QWaylandWindow::reset(bool sendDestroyEvent) ++void QWaylandWindow::reset() + { +- if (isInitialized() && sendDestroyEvent) { +- QPlatformSurfaceEvent e(QPlatformSurfaceEvent::SurfaceAboutToBeDestroyed); +- QGuiApplication::sendEvent(window(), &e); +- } + delete mShellSurface; + mShellSurface = nullptr; + delete mSubSurfaceWindow; +diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h +index 86139243..fe24224f 100644 +--- a/src/client/qwaylandwindow_p.h ++++ b/src/client/qwaylandwindow_p.h +@@ -261,7 +261,7 @@ private: + void initializeWlSurface(); + bool shouldCreateShellSurface() const; + bool shouldCreateSubSurface() const; +- void reset(bool sendDestroyEvent = true); ++ void reset(); + void sendExposeEvent(const QRect &rect); + static void closePopups(QWaylandWindow *parent); + QWaylandScreen *calculateScreenFromSurfaceEvents() const; +-- +2.22.0 + diff --git a/0004-Client-Make-handleUpdate-aware-of-exposure-changes.patch b/0004-Client-Make-handleUpdate-aware-of-exposure-changes.patch new file mode 100644 index 0000000..beff9d8 --- /dev/null +++ b/0004-Client-Make-handleUpdate-aware-of-exposure-changes.patch @@ -0,0 +1,80 @@ +From 71a854f4eced28282df72e1b25888929799e8ee7 Mon Sep 17 00:00:00 2001 +From: David Edmundson +Date: Sun, 23 Jun 2019 14:48:30 +0200 +Subject: [PATCH 4/5] Client: Make handleUpdate aware of exposure changes + +The wl_surface can be destroyed whilst a render is happening. Calling +wl_surface::frame after the window is reset can crash as wl_surface is +null. + +Change-Id: I139a9b234cb6acba81d6c1d5fa58629904a25053 +--- + src/client/qwaylandwindow.cpp | 8 ++++++++ + src/client/qwaylandwindow_p.h | 4 ++++ + 2 files changed, 12 insertions(+) + +diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp +index 76975b27..c6723aa7 100644 +--- a/src/client/qwaylandwindow.cpp ++++ b/src/client/qwaylandwindow.cpp +@@ -72,6 +72,8 @@ Q_LOGGING_CATEGORY(lcWaylandBackingstore, "qt.qpa.wayland.backingstore") + + QWaylandWindow *QWaylandWindow::mMouseGrab = nullptr; + ++QReadWriteLock mSurfaceLock; ++ + QWaylandWindow::QWaylandWindow(QWindow *window) + : QPlatformWindow(window) + , mDisplay(waylandScreen()->display()) +@@ -197,6 +199,7 @@ void QWaylandWindow::initWindow() + + void QWaylandWindow::initializeWlSurface() + { ++ QWriteLocker lock(&mSurfaceLock); + init(mDisplay->createSurface(static_cast(this))); + } + +@@ -230,6 +233,7 @@ void QWaylandWindow::reset() + mSubSurfaceWindow = nullptr; + if (isInitialized()) { + emit wlSurfaceDestroyed(); ++ QWriteLocker lock(&mSurfaceLock); + destroy(); + } + mScreens.clear(); +@@ -1142,6 +1146,10 @@ void QWaylandWindow::requestUpdate() + void QWaylandWindow::handleUpdate() + { + // TODO: Should sync subsurfaces avoid requesting frame callbacks? ++ QReadLocker lock(&mSurfaceLock); ++ if (!isInitialized()) { ++ return; ++ } + + if (mFrameCallback) { + wl_callback_destroy(mFrameCallback); +diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h +index fe24224f..97884ba4 100644 +--- a/src/client/qwaylandwindow_p.h ++++ b/src/client/qwaylandwindow_p.h +@@ -53,6 +53,8 @@ + + #include + #include ++#include ++ + #include + #include + #include +@@ -277,6 +279,8 @@ private: + static QMutex mFrameSyncMutex; + static QWaylandWindow *mMouseGrab; + ++ QReadWriteLock mSurfaceLock; ++ + friend class QWaylandSubSurface; + }; + +-- +2.22.0 + diff --git a/0005-Client-Reset-frame-callback-timer-when-hiding-a-wind.patch b/0005-Client-Reset-frame-callback-timer-when-hiding-a-wind.patch new file mode 100644 index 0000000..2508971 --- /dev/null +++ b/0005-Client-Reset-frame-callback-timer-when-hiding-a-wind.patch @@ -0,0 +1,40 @@ +From 19e165aa58b4d6c2597e6732d6c7d1efe4c9b362 Mon Sep 17 00:00:00 2001 +From: David Edmundson +Date: Sun, 23 Jun 2019 13:25:16 +0200 +Subject: [PATCH 5/5] Client: Reset frame callback timer when hiding a window + +If we hide a window whilst a compositor has a pending frame to show, +it's possible the compositor will not render the frame and not return +the callback. + +If this happens on the next window expose we can be left with +mFrameCallbackTimedOut still true causing isExposed() to remain false +and us to not send the next buffer when we later show the window again. + +Change-Id: I507410415d1a930fd5fa736412055e68fdf6c1d3 +Reviewed-by: Johan Helsing +--- + src/client/qwaylandwindow.cpp | 7 +++++++ + 1 file changed, 7 insertions(+) + +diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp +index c6723aa7..f1be524b 100644 +--- a/src/client/qwaylandwindow.cpp ++++ b/src/client/qwaylandwindow.cpp +@@ -243,6 +243,13 @@ void QWaylandWindow::reset() + mFrameCallback = nullptr; + } + ++ int timerId = mFrameCallbackTimerId.fetchAndStoreOrdered(-1); ++ if (timerId != -1) { ++ killTimer(timerId); ++ } ++ mWaitingForFrameCallback = false; ++ mFrameCallbackTimedOut = false; ++ + mMask = QRegion(); + mQueuedBuffer = nullptr; + } +-- +2.22.0 + diff --git a/libqt5-qtwayland.changes b/libqt5-qtwayland.changes index 02e179f..7e12ac2 100644 --- a/libqt5-qtwayland.changes +++ b/libqt5-qtwayland.changes @@ -1,3 +1,15 @@ +------------------------------------------------------------------- +Wed Jul 3 08:50:36 UTC 2019 - Fabian Vogt + +- Add patch from upstream 5.13 branch to fix crashes: + * 0001-Don-t-crash-if-we-start-a-drag-without-dragFocus.patch +- Add patch from upstream 5.12 branch to improve performance: + * 0002-Client-Fix-stuttering-when-the-GUI-thread-is-busy.patch +- Add patches from upstream code reviews to fix various issues: + * 0003-Client-Don-t-send-fake-SurfaceCreated-Destroyed-even.patch + * 0004-Client-Make-handleUpdate-aware-of-exposure-changes.patch + * 0005-Client-Reset-frame-callback-timer-when-hiding-a-wind.patch + ------------------------------------------------------------------- Wed Jun 19 11:24:47 UTC 2019 - fabian@ritter-vogt.de diff --git a/libqt5-qtwayland.spec b/libqt5-qtwayland.spec index b7e29ec..e30151f 100644 --- a/libqt5-qtwayland.spec +++ b/libqt5-qtwayland.spec @@ -31,6 +31,17 @@ Group: Development/Libraries/X11 Url: https://www.qt.io Source: https://download.qt.io/official_releases/qt/5.13/%{real_version}/submodules/%{tar_version}.tar.xz Source1: baselibs.conf +# PATCH-FIX-UPSTREAM +Patch1: 0001-Don-t-crash-if-we-start-a-drag-without-dragFocus.patch +# In 5.12 branch only, not merged to 5.13 yet +Patch2: 0002-Client-Fix-stuttering-when-the-GUI-thread-is-busy.patch +# Those aren't merged upstream yet +# https://codereview.qt-project.org/c/qt/qtwayland/+/265999 +Patch3: 0003-Client-Don-t-send-fake-SurfaceCreated-Destroyed-even.patch +# https://codereview.qt-project.org/c/qt/qtwayland/+/265998 +Patch4: 0004-Client-Make-handleUpdate-aware-of-exposure-changes.patch +# https://codereview.qt-project.org/c/qt/qtwayland/+/265997 +Patch5: 0005-Client-Reset-frame-callback-timer-when-hiding-a-wind.patch # PATCH-FIX-OPENSUSE Patch100: workaround-null-object.patch BuildRequires: fdupes