From 6258c4994200348220be6f35667a2c0f4b705539 Mon Sep 17 00:00:00 2001 From: Johan Klokkhammer Helsing Date: Tue, 8 Jan 2019 12:15:55 +0100 Subject: Revert "Client: Full implementation for frame callbacks" This caused regressions because QtQuick depends on swapBuffers for throttling animations. We probably need to emulate a blocking swapBuffers and continue after a timeout, but until we have a patch for this, revert this to avoid releasing a regression. This brings back the bug with a frozen event loop when a surface is waiting for a frame callback, but this is preferable to a regression. This reverts commit 1dc85b95ab0adc1e805d059e2c35c671ef790011. Fixes: QTBUG-72578 Change-Id: If6435a947aae5e9fd775404649a392bfafe9130a Reviewed-by: Paul Olav Tvete --- src/client/qwaylandwindow.cpp | 107 ++++----------------- src/client/qwaylandwindow_p.h | 9 +- .../client/wayland-egl/qwaylandglcontext.cpp | 22 +++-- 3 files changed, 35 insertions(+), 103 deletions(-) diff --git a/src/client/qwaylandwindow.cpp b/src/client/qwaylandwindow.cpp index 470ae0091..282179efb 100644 --- a/src/client/qwaylandwindow.cpp +++ b/src/client/qwaylandwindow.cpp @@ -358,8 +358,6 @@ 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; } @@ -544,11 +542,18 @@ 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) { - handleUpdate(); + mFrameCallback = frame(); + wl_callback_add_listener(mFrameCallback, &QWaylandWindow::callbackListener, this); + mWaitingForFrameSync = true; buffer->setBusy(); - QtWayland::wl_surface::attach(buffer->buffer(), x, y); + attach(buffer->buffer(), x, y); } else { QtWayland::wl_surface::attach(nullptr, 0, 0); } @@ -613,9 +618,11 @@ void QWaylandWindow::frameCallback(void *data, struct wl_callback *callback, uin Q_UNUSED(callback); QWaylandWindow *self = static_cast(data); - self->mWaitingForFrameCallback = false; - if (self->mUpdateRequested) + self->mWaitingForFrameSync = false; + if (self->mUpdateRequested) { + self->mUpdateRequested = false; self->deliverUpdateRequest(); + } } QMutex QWaylandWindow::mFrameSyncMutex; @@ -623,10 +630,10 @@ QMutex QWaylandWindow::mFrameSyncMutex; void QWaylandWindow::waitForFrameSync() { QMutexLocker locker(&mFrameSyncMutex); - if (!mWaitingForFrameCallback) + if (!mWaitingForFrameSync) return; mDisplay->flushRequests(); - while (mWaitingForFrameCallback) + while (mWaitingForFrameSync) mDisplay->blockingReadEvents(); } @@ -1027,88 +1034,12 @@ 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 (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(); + if (!mWaitingForFrameSync) + QPlatformWindow::requestUpdate(); + else + mUpdateRequested = true; } void QWaylandWindow::addAttachOffset(const QPoint point) diff --git a/src/client/qwaylandwindow_p.h b/src/client/qwaylandwindow_p.h index e5838d231..56ebd3cc6 100644 --- a/src/client/qwaylandwindow_p.h +++ b/src/client/qwaylandwindow_p.h @@ -191,10 +191,7 @@ public: bool startSystemMove(const QPoint &pos) override; - void timerEvent(QTimerEvent *event) override; void requestUpdate() override; - void handleUpdate(); - void deliverUpdateRequest() override; public slots: void applyConfigure(); @@ -214,14 +211,10 @@ protected: Qt::MouseButtons mMousePressedInContentArea = Qt::NoButton; WId mWindowId; - bool mWaitingForFrameCallback = false; + bool mWaitingForFrameSync = 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 0cbbe5389..e58403ad0 100644 --- a/src/hardwareintegration/client/wayland-egl/qwaylandglcontext.cpp +++ b/src/hardwareintegration/client/wayland-egl/qwaylandglcontext.cpp @@ -315,9 +315,7 @@ QWaylandGLContext::QWaylandGLContext(EGLDisplay eglDisplay, QWaylandDisplay *dis mSupportNonBlockingSwap = false; } if (!mSupportNonBlockingSwap) { - 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"; + qWarning() << "Non-blocking swap buffers not supported. Subsurface rendering can be affected."; } updateGLFormat(); @@ -552,10 +550,20 @@ void QWaylandGLContext::swapBuffers(QPlatformSurface *surface) m_blitter->blit(window); } - window->handleUpdate(); - int swapInterval = mSupportNonBlockingSwap ? 0 : m_format.swapInterval(); - eglSwapInterval(m_eglDisplay, swapInterval); - eglSwapBuffers(m_eglDisplay, eglSurface); + + 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->setCanResize(true); } -- cgit v1.2.3