From dbe589453083037a6e4c0a23e6f342cfd9172877 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Tue, 17 Sep 2024 22:49:34 +0100 Subject: [PATCH] Client: Use synchronous delivery for geometry events This simplifies the data flow and reduces the paths for some races with threaded rendering. Making all events synchronous does not work due to application logic having nested event loops. The design is all normal wayland events are flushed at the end of each dispatch so that everything remains ordered. When a configure event is received applying that state is queued. When it is applied, all events emitted from this are flushed. Application driven geometry changes are also synchronous. The old code to manage frame synchronization is removed, this attempted to not apply configure events whilst a frame is in flight. It doesn't work properly as the render thread could start on the next frame whilst the main thread processed the resize. It is problematic in this port as the mutexes will cause deadlocks with sync delivery. Task-number: QTBUG-121731 Fixes: QTBUG-117762 Fixes: QTBUG-119112 Change-Id: I5802b4006d307a45468767d68c3ad40078df5703 Reviewed-by: Vlad Zahorodnii --- .../xdg-shell/qwaylandxdgshell.cpp | 9 +- .../platforms/wayland/qwaylanddisplay.cpp | 1 + .../platforms/wayland/qwaylandwindow.cpp | 86 +++++-------------- .../platforms/wayland/qwaylandwindow_p.h | 8 -- tests/auto/wayland/xdgshell/tst_xdgshell.cpp | 1 - 5 files changed, 28 insertions(+), 77 deletions(-) diff --git a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp index 11add2809bc..ce0ab795732 100644 --- a/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp +++ b/src/plugins/platforms/wayland/plugins/shellintegration/xdg-shell/qwaylandxdgshell.cpp @@ -413,15 +413,16 @@ void QWaylandXdgSurface::applyConfigure() if (m_pendingConfigureSerial == m_appliedConfigureSerial) return; - if (m_toplevel) - m_toplevel->applyConfigure(); - if (m_popup) - m_popup->applyConfigure(); m_appliedConfigureSerial = m_pendingConfigureSerial; m_configured = true; ack_configure(m_appliedConfigureSerial); + if (m_toplevel) + m_toplevel->applyConfigure(); + if (m_popup) + m_popup->applyConfigure(); + window()->updateExposure(); } diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp index edd769b35a8..b680c810f99 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp @@ -503,6 +503,7 @@ void QWaylandDisplay::reconnect() void QWaylandDisplay::flushRequests() { m_eventThread->readAndDispatchEvents(); + QWindowSystemInterface::flushWindowSystemEvents(); } // We have to wait until we have an eventDispatcher before creating the eventThread, diff --git a/src/plugins/platforms/wayland/qwaylandwindow.cpp b/src/plugins/platforms/wayland/qwaylandwindow.cpp index 232b6503d9b..81969d16a5d 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow.cpp +++ b/src/plugins/platforms/wayland/qwaylandwindow.cpp @@ -335,7 +335,6 @@ void QWaylandWindow::resetSurfaceRole() } mFrameCallbackTimedOut = false; mWaitingToApplyConfigure = false; - mCanResize = true; mResizeDirty = false; mExposed = false; } @@ -452,27 +451,27 @@ void QWaylandWindow::setGeometry(const QRect &r) } } - if (window()->isVisible() && rect.isValid()) { - if (mWindowDecorationEnabled) - mWindowDecoration->update(); - - if (mResizeAfterSwap && windowType() == Egl && mSentInitialResize) { - QMutexLocker lock(&mResizeLock); - mResizeDirty = true; - } else { - QWindowSystemInterface::handleGeometryChange(window(), geometry()); - } - mSentInitialResize = true; - } - QRect exposeGeometry(QPoint(), geometry().size()); - if (isExposed() && !mInResizeFromApplyConfigure && exposeGeometry != mLastExposeGeometry) - sendExposeEvent(exposeGeometry); - if (mShellSurface) mShellSurface->setContentGeometry(windowContentGeometry()); if (isOpaque() && mMask.isEmpty()) setOpaqueArea(QRect(QPoint(0, 0), rect.size())); + + + if (window()->isVisible() && rect.isValid()) { + ensureSize(); + if (mWindowDecorationEnabled) + mWindowDecoration->update(); + + if (mResizeAfterSwap && windowType() == Egl && mSentInitialResize) { + mResizeDirty = true; + } + QWindowSystemInterface::handleGeometryChange(window(), geometry()); + mSentInitialResize = true; + } + QRect exposeGeometry(QPoint(), geometry().size()); + if (isExposed() && !mInResizeFromApplyConfigure && exposeGeometry != mLastExposeGeometry) + sendExposeEvent(exposeGeometry); } void QWaylandWindow::updateInputRegion() @@ -549,8 +548,8 @@ void QWaylandWindow::resizeFromApplyConfigure(const QSize &sizeWithMargins, cons void QWaylandWindow::sendExposeEvent(const QRect &rect) { if (!(mShellSurface && mShellSurface->handleExpose(rect))) { - QWindowSystemInterface::handleExposeEvent(window(), rect); mLastExposeGeometry = rect; + QWindowSystemInterface::handleExposeEvent(window(), rect); } else qCDebug(lcQpaWayland) << "sendExposeEvent: intercepted by shell extension, not sending"; @@ -645,67 +644,26 @@ bool QWaylandWindow::isAlertState() const void QWaylandWindow::applyConfigureWhenPossible() { - QMutexLocker resizeLocker(&mResizeLock); if (!mWaitingToApplyConfigure) { mWaitingToApplyConfigure = true; QMetaObject::invokeMethod(this, "applyConfigure", Qt::QueuedConnection); } } -void QWaylandWindow::doApplyConfigure() +void QWaylandWindow::applyConfigure() { if (!mWaitingToApplyConfigure) return; Q_ASSERT_X(QThread::currentThreadId() == QThreadData::get2(thread())->threadId.loadRelaxed(), - "QWaylandWindow::doApplyConfigure", "not called from main thread"); + "QWaylandWindow::applyConfigure", "not called from main thread"); if (mShellSurface) mShellSurface->applyConfigure(); mWaitingToApplyConfigure = false; - - sendExposeEvent(QRect(QPoint(), geometry().size())); -} - -void QWaylandWindow::doApplyConfigureFromOtherThread() -{ - QMutexLocker lock(&mResizeLock); - if (!mCanResize || !mWaitingToApplyConfigure) - return; - doApplyConfigure(); -} - -void QWaylandWindow::setCanResize(bool canResize) -{ - QMutexLocker lock(&mResizeLock); - mCanResize = canResize; - - if (canResize) { - if (mResizeDirty) { - QWindowSystemInterface::handleGeometryChange(window(), geometry()); - } - if (mWaitingToApplyConfigure) { - bool inGuiThread = QThread::currentThreadId() == QThreadData::get2(thread())->threadId.loadRelaxed(); - if (inGuiThread) { - doApplyConfigure(); - } else { - QMetaObject::invokeMethod(this, &QWaylandWindow::doApplyConfigureFromOtherThread, Qt::QueuedConnection); - } - } else if (mResizeDirty) { - mResizeDirty = false; - } - } -} - -void QWaylandWindow::applyConfigure() -{ - QMutexLocker lock(&mResizeLock); - - if (mCanResize || !mSentInitialResize) - doApplyConfigure(); - - lock.unlock(); + QRect exposeGeometry(QPoint(), geometry().size()); + sendExposeEvent(exposeGeometry); QWindowSystemInterface::flushWindowSystemEvents(); } @@ -1488,7 +1446,6 @@ void QWaylandWindow::setScale(qreal newScale) return; mScale = newScale; - QWindowSystemInterface::handleWindowDevicePixelRatioChanged(window()); if (mSurface) { if (mViewport) updateViewport(); @@ -1497,6 +1454,7 @@ void QWaylandWindow::setScale(qreal newScale) } ensureSize(); + QWindowSystemInterface::handleWindowDevicePixelRatioChanged(window()); if (isExposed()) { // redraw at the new DPR window()->requestUpdate(); diff --git a/src/plugins/platforms/wayland/qwaylandwindow_p.h b/src/plugins/platforms/wayland/qwaylandwindow_p.h index bb8c5b18826..5bb7e2c6e02 100644 --- a/src/plugins/platforms/wayland/qwaylandwindow_p.h +++ b/src/plugins/platforms/wayland/qwaylandwindow_p.h @@ -193,9 +193,6 @@ public: QWaylandWindow *transientParent() const; - void doApplyConfigure(); - void setCanResize(bool canResize); - bool setMouseGrabEnabled(bool grab) override; static QWaylandWindow *mouseGrab() { return mMouseGrab; } @@ -309,9 +306,7 @@ protected: bool mWaitingForUpdate = false; bool mExposed = false; - QRecursiveMutex mResizeLock; bool mWaitingToApplyConfigure = false; - bool mCanResize = true; bool mResizeDirty = false; bool mResizeAfterSwap; int mFrameCallbackTimeout = 100; @@ -344,9 +339,6 @@ protected: Qt::ScreenOrientation mLastReportedContentOrientation = Qt::PrimaryOrientation; -private Q_SLOTS: - void doApplyConfigureFromOtherThread(); - private: void setGeometry_helper(const QRect &rect); void initWindow(); diff --git a/tests/auto/wayland/xdgshell/tst_xdgshell.cpp b/tests/auto/wayland/xdgshell/tst_xdgshell.cpp index a14f103219c..0bb3b3ecf9d 100644 --- a/tests/auto/wayland/xdgshell/tst_xdgshell.cpp +++ b/tests/auto/wayland/xdgshell/tst_xdgshell.cpp @@ -49,7 +49,6 @@ void tst_xdgshell::showMinimized() // between a window preview and an unminimized window. QWindow window; window.showMinimized(); - QCOMPARE(window.windowStates(), Qt::WindowMinimized); // should return minimized until QTRY_COMPARE(window.windowStates(), Qt::WindowNoState); // rejected by handleWindowStateChanged // Make sure the window on the compositor side is/was created here, and not after the test