From 65b8ba1770c21ed96e5faa4615623a4cddd79928 Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Fri, 5 Oct 2018 17:27:05 +0300 Subject: [PATCH] [wayland] Syncronise pending geometry with acked configure requests Summary: When we want to change a client's size and position together we have to request the client becomes a new size and only then move the window to the new location. Currently we process the new position the next time the buffer updates, but with no guarantee that it has actually tried to resize/whatever yet. The client could be providing a new buffer just because the contents have changed. XDGShell has an acked serial designed to keep everything precisely in sync. A surface represents the last configure that was acked. This patch tracks the pending position for each configure and applies it accordingly. WL_shell does not have this mechanism, so behaviour is kept the same as before. ---- This is a pre-requisite to syncing maximisedState/isFullScreen with the configure request. Potentially we could remove the isWaitingForResizeSync checks when resizing and it will still resize smoothly. Test Plan: Relevant unit test still passes with the client responding Resized a window from the left edge with WLShell and XDGShellV6 Reviewers: #kwin, romangg Reviewed By: #kwin, romangg Subscribers: romangg, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D15135 --- autotests/integration/shell_client_test.cpp | 125 ++++++++++++-------- shell_client.cpp | 52 ++++++-- shell_client.h | 13 +- 3 files changed, 130 insertions(+), 60 deletions(-) diff --git a/autotests/integration/shell_client_test.cpp b/autotests/integration/shell_client_test.cpp index 2e01da7617..b15a1f3119 100644 --- a/autotests/integration/shell_client_test.cpp +++ b/autotests/integration/shell_client_test.cpp @@ -405,6 +405,24 @@ void TestShellClient::testFullscreen() QFETCH(Test::ShellSurfaceType, type); QScopedPointer shellSurface(Test::createShellSurface(type, surface.data())); + ShellSurface* wlShellSurface = nullptr; + XdgShellSurface *xdgShellSurface = nullptr; + // fullscreen the window + switch (type) { + case Test::ShellSurfaceType::WlShell: + wlShellSurface = qobject_cast(shellSurface.data()); + break; + case Test::ShellSurfaceType::XdgShellV5: + case Test::ShellSurfaceType::XdgShellV6: + xdgShellSurface = qobject_cast(shellSurface.data()); + break; + default: + Q_UNREACHABLE(); + break; + } + QVERIFY(wlShellSurface || xdgShellSurface); + QVERIFY(!(wlShellSurface && xdgShellSurface)); + // create deco QScopedPointer deco(Test::waylandServerSideDecoration()->create(surface.data())); QSignalSpy decoSpy(deco.data(), &ServerSideDecoration::modeChanged); @@ -429,20 +447,18 @@ void TestShellClient::testFullscreen() QVERIFY(geometryChangedSpy.isValid()); QSignalSpy sizeChangeRequestedSpy(shellSurface.data(), SIGNAL(sizeChanged(QSize))); QVERIFY(sizeChangeRequestedSpy.isValid()); - - // fullscreen the window - switch (type) { - case Test::ShellSurfaceType::WlShell: - qobject_cast(shellSurface.data())->setFullscreen(); - break; - case Test::ShellSurfaceType::XdgShellV5: - case Test::ShellSurfaceType::XdgShellV6: - qobject_cast(shellSurface.data())->setFullscreen(true); - break; - default: - Q_UNREACHABLE(); - break; + QSignalSpy configureRequestedSpy(shellSurface.data(), SIGNAL(configureRequested(QSize, KWayland::Client::XdgShellSurface::States, quint32))); + if (xdgShellSurface) { + QVERIFY(configureRequestedSpy.isValid()); } + + if (wlShellSurface) { + wlShellSurface->setFullscreen(); + } + if (xdgShellSurface) { + xdgShellSurface->setFullscreen(true); + } + QVERIFY(fullscreenChangedSpy.wait()); QVERIFY(sizeChangeRequestedSpy.wait()); QCOMPARE(sizeChangeRequestedSpy.count(), 1); @@ -452,7 +468,12 @@ void TestShellClient::testFullscreen() QCOMPARE(c->clientSize(), QSize(100, 50)); QVERIFY(geometryChangedSpy.isEmpty()); - // render at the new size + if (xdgShellSurface) { + for (const auto &it: configureRequestedSpy) { + xdgShellSurface->ackConfigure(it[2].toInt()); + } + } + Test::render(surface.data(), sizeChangeRequestedSpy.first().first().toSize(), Qt::red); QVERIFY(geometryChangedSpy.wait()); QCOMPARE(geometryChangedSpy.count(), 1); @@ -462,17 +483,11 @@ void TestShellClient::testFullscreen() QCOMPARE(c->layer(), ActiveLayer); // swap back to normal - switch (type) { - case Test::ShellSurfaceType::WlShell: - qobject_cast(shellSurface.data())->setToplevel(); - break; - case Test::ShellSurfaceType::XdgShellV5: - case Test::ShellSurfaceType::XdgShellV6: - qobject_cast(shellSurface.data())->setFullscreen(false); - break; - default: - Q_UNREACHABLE(); - break; + if (wlShellSurface) { + wlShellSurface->setToplevel(); + } + if (xdgShellSurface) { + xdgShellSurface->setFullscreen(false); } QVERIFY(fullscreenChangedSpy.wait()); QVERIFY(sizeChangeRequestedSpy.wait()); @@ -602,6 +617,24 @@ void TestShellClient::testMaximizedToFullscreen() QFETCH(Test::ShellSurfaceType, type); QScopedPointer shellSurface(Test::createShellSurface(type, surface.data())); + ShellSurface* wlShellSurface = nullptr; + XdgShellSurface *xdgShellSurface = nullptr; + // fullscreen the window + switch (type) { + case Test::ShellSurfaceType::WlShell: + wlShellSurface = qobject_cast(shellSurface.data()); + break; + case Test::ShellSurfaceType::XdgShellV5: + case Test::ShellSurfaceType::XdgShellV6: + xdgShellSurface = qobject_cast(shellSurface.data()); + break; + default: + Q_UNREACHABLE(); + break; + } + QVERIFY(wlShellSurface || xdgShellSurface); + QVERIFY(!(wlShellSurface && xdgShellSurface)); + // create deco QScopedPointer deco(Test::waylandServerSideDecoration()->create(surface.data())); QSignalSpy decoSpy(deco.data(), &ServerSideDecoration::modeChanged); @@ -624,19 +657,17 @@ void TestShellClient::testMaximizedToFullscreen() QVERIFY(geometryChangedSpy.isValid()); QSignalSpy sizeChangeRequestedSpy(shellSurface.data(), SIGNAL(sizeChanged(QSize))); QVERIFY(sizeChangeRequestedSpy.isValid()); + QSignalSpy configureRequestedSpy(shellSurface.data(), SIGNAL(configureRequested(QSize, KWayland::Client::XdgShellSurface::States, quint32))); + if (xdgShellSurface) { + QVERIFY(configureRequestedSpy.isValid()); + } // change to maximize - switch (type) { - case Test::ShellSurfaceType::WlShell: - qobject_cast(shellSurface.data())->setMaximized(); - break; - case Test::ShellSurfaceType::XdgShellV5: - case Test::ShellSurfaceType::XdgShellV6: - qobject_cast(shellSurface.data())->setMaximized(true); - break; - default: - Q_UNREACHABLE(); - break; + if (wlShellSurface) { + wlShellSurface->setMaximized(); + } + if (xdgShellSurface) { + xdgShellSurface->setMaximized(true); } QVERIFY(sizeChangeRequestedSpy.wait()); QCOMPARE(sizeChangeRequestedSpy.count(), 1); @@ -645,17 +676,11 @@ void TestShellClient::testMaximizedToFullscreen() geometryChangedSpy.clear(); // fullscreen the window - switch (type) { - case Test::ShellSurfaceType::WlShell: - qobject_cast(shellSurface.data())->setFullscreen(); - break; - case Test::ShellSurfaceType::XdgShellV5: - case Test::ShellSurfaceType::XdgShellV6: - qobject_cast(shellSurface.data())->setFullscreen(true); - break; - default: - Q_UNREACHABLE(); - break; + if (wlShellSurface) { + wlShellSurface->setFullscreen(); + } + if (xdgShellSurface) { + xdgShellSurface->setFullscreen(true); } QVERIFY(fullscreenChangedSpy.wait()); if (decoMode == ServerSideDecoration::Mode::Server) { @@ -668,6 +693,12 @@ void TestShellClient::testMaximizedToFullscreen() QCOMPARE(c->clientSize(), QSize(100, 50)); QVERIFY(geometryChangedSpy.isEmpty()); + if (xdgShellSurface) { + for (const auto &it: configureRequestedSpy) { + xdgShellSurface->ackConfigure(it[2].toInt()); + } + } + // render at the new size Test::render(surface.data(), sizeChangeRequestedSpy.last().first().toSize(), Qt::red); QVERIFY(geometryChangedSpy.wait()); diff --git a/shell_client.cpp b/shell_client.cpp index 7a9401224d..fe8bf0f253 100644 --- a/shell_client.cpp +++ b/shell_client.cpp @@ -261,6 +261,10 @@ void ShellClient::init() } }); + connect(m_xdgShellSurface, &XdgShellSurfaceInterface::configureAcknowledged, this, [this](int serial) { + m_lastAckedConfigureRequest = serial; + }); + connect(global, &XdgShellInterface::pingTimeout, this, [this](qint32 serial) { auto it = m_pingSerials.find(serial); @@ -322,6 +326,10 @@ void ShellClient::init() m_hasPopupGrab = true; }); + connect(m_xdgShellSurface, &XdgShellSurfaceInterface::configureAcknowledged, this, [this](int serial) { + m_lastAckedConfigureRequest = serial; + }); + QRect position = QRect(m_xdgShellPopup->transientOffset(), m_xdgShellPopup->initialSize()); m_xdgShellPopup->configure(position); @@ -485,13 +493,7 @@ void ShellClient::addDamage(const QRegion &damage) auto s = surface(); if (s->size().isValid()) { m_clientSize = s->size(); - QPoint position = geom.topLeft(); - if (m_positionAfterResize.isValid()) { - addLayerRepaint(geometry()); - position = m_positionAfterResize.point(); - m_positionAfterResize.clear(); - } - doSetGeometry(QRect(position, m_clientSize + QSize(borderLeft() + borderRight(), borderTop() + borderBottom()))); + updatePendingGeometry(); } markAsMapped(); setDepth((s->buffer()->hasAlphaChannel() && !isDesktop()) ? 32 : 24); @@ -601,7 +603,7 @@ void ShellClient::setGeometry(int x, int y, int w, int h, ForceGeometry_t force) geom = geometryBeforeUpdateBlocking(); } // TODO: better merge with Client's implementation - if (QSize(w, h) == geom.size() && !m_positionAfterResize.isValid()) { + if (QSize(w, h) == geom.size() && !isWaitingForMoveResizeSync()) { // size didn't change, update directly doSetGeometry(QRect(x, y, w, h)); } else { @@ -1170,29 +1172,55 @@ void ShellClient::requestGeometry(const QRect &rect) m_blockedRequestGeometry = rect; return; } - m_positionAfterResize.setPoint(rect.topLeft()); + PendingConfigureRequest configureRequest; + configureRequest.positionAfterResize = rect.topLeft(); + const QSize size = rect.size() - QSize(borderLeft() + borderRight(), borderTop() + borderBottom()); if (m_shellSurface) { m_shellSurface->requestSize(size); } if (m_xdgShellSurface) { - m_xdgShellSurface->configure(xdgSurfaceStates(), size); + configureRequest.serialId = m_xdgShellSurface->configure(xdgSurfaceStates(), size); } if (m_xdgShellPopup) { auto parent = transientFor(); if (parent) { const QPoint globalClientContentPos = parent->geometry().topLeft() + parent->clientPos(); const QPoint relativeOffset = rect.topLeft() -globalClientContentPos; - m_xdgShellPopup->configure(QRect(relativeOffset, rect.size())); + configureRequest.serialId = m_xdgShellPopup->configure(QRect(relativeOffset, rect.size())); } } + m_pendingConfigureRequests.append(configureRequest); + m_blockedRequestGeometry = QRect(); if (m_internal) { m_internalWindow->setGeometry(QRect(rect.topLeft() + QPoint(borderLeft(), borderTop()), rect.size() - QSize(borderLeft() + borderRight(), borderTop() + borderBottom()))); } } +void ShellClient::updatePendingGeometry() +{ + QPoint position = geom.topLeft(); + for (auto it = m_pendingConfigureRequests.begin(); it != m_pendingConfigureRequests.end(); it++) { + if (it->serialId > m_lastAckedConfigureRequest) { + //this serial is not acked yet, therefore we know all future serials are not + break; + } + if (it->serialId == m_lastAckedConfigureRequest) { + if (position != it->positionAfterResize) { + addLayerRepaint(geometry()); + } + position = it->positionAfterResize; + + m_pendingConfigureRequests.erase(m_pendingConfigureRequests.begin(), ++it); + break; + } + //else serialId < m_lastAckedConfigureRequest and the state is now irrelevant and can be ignored + } + doSetGeometry(QRect(position, m_clientSize + QSize(borderLeft() + borderRight(), borderTop() + borderBottom()))); +} + void ShellClient::clientFullScreenChanged(bool fullScreen) { setFullScreen(fullScreen, false); @@ -1534,7 +1562,7 @@ QPoint ShellClient::transientPlacementHint() const bool ShellClient::isWaitingForMoveResizeSync() const { - return m_positionAfterResize.isValid(); + return !m_pendingConfigureRequests.isEmpty(); } void ShellClient::doResizeSync() diff --git a/shell_client.h b/shell_client.h index 205e9caec3..46145678cc 100644 --- a/shell_client.h +++ b/shell_client.h @@ -202,6 +202,8 @@ private: void updateClientOutputs(); KWayland::Server::XdgShellSurfaceInterface::States xdgSurfaceStates() const; void updateShowOnScreenEdge(); + // called on surface commit and processes all m_pendingConfigureRequests up to m_lastAckedConfigureReqest + void updatePendingGeometry(); static void deleteClient(ShellClient *c); KWayland::Server::ShellSurfaceInterface *m_shellSurface; @@ -209,7 +211,16 @@ private: KWayland::Server::XdgShellPopupInterface *m_xdgShellPopup; QSize m_clientSize; - ClearablePoint m_positionAfterResize; // co-ordinates saved from a requestGeometry call, real geometry will be updated after the next damage event when the client has resized + struct PendingConfigureRequest { + //note for wl_shell we have no serial, so serialId and m_lastAckedConfigureRequest will always be 0 + //meaning we treat a surface commit as having processed all requests + quint32 serialId = 0; + // position to apply after a resize operation has been completed + QPoint positionAfterResize; + }; + QVector m_pendingConfigureRequests; + quint32 m_lastAckedConfigureRequest = 0; + QRect m_geomFsRestore; //size and position of the window before it was set to fullscreen bool m_closing = false; quint32 m_windowId = 0;