[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

@@ -405,6 +405,24 @@ void TestShellClient::testFullscreen()
     QFETCH(Test::ShellSurfaceType, type);
     QScopedPointer<QObject> 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*>(shellSurface.data());
+        break;
+    case Test::ShellSurfaceType::XdgShellV5:
+    case Test::ShellSurfaceType::XdgShellV6:
+        xdgShellSurface = qobject_cast<XdgShellSurface*>(shellSurface.data());
+        break;
+    default:
+        Q_UNREACHABLE();
+        break;
+    }
+    QVERIFY(wlShellSurface || xdgShellSurface);
+    QVERIFY(!(wlShellSurface && xdgShellSurface));
+
     // create deco
     QScopedPointer<ServerSideDecoration> 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());
-
+    QSignalSpy configureRequestedSpy(shellSurface.data(), SIGNAL(configureRequested(QSize, KWayland::Client::XdgShellSurface::States, quint32)));
-    // fullscreen the window
+    if (xdgShellSurface) {
-    switch (type) {
+        QVERIFY(configureRequestedSpy.isValid());
-    case Test::ShellSurfaceType::WlShell:
-        qobject_cast<ShellSurface*>(shellSurface.data())->setFullscreen();
-        break;
-    case Test::ShellSurfaceType::XdgShellV5:
-    case Test::ShellSurfaceType::XdgShellV6:
-        qobject_cast<XdgShellSurface*>(shellSurface.data())->setFullscreen(true);
-        break;
-    default:
-        Q_UNREACHABLE();
-        break;
     }
+
+    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) {
+    if (wlShellSurface) {
-    case Test::ShellSurfaceType::WlShell:
+        wlShellSurface->setToplevel();
-        qobject_cast<ShellSurface*>(shellSurface.data())->setToplevel();
+    }
-        break;
+    if (xdgShellSurface) {
-    case Test::ShellSurfaceType::XdgShellV5:
+        xdgShellSurface->setFullscreen(false);
-    case Test::ShellSurfaceType::XdgShellV6:
-        qobject_cast<XdgShellSurface*>(shellSurface.data())->setFullscreen(false);
-        break;
-    default:
-        Q_UNREACHABLE();
-        break;
     }
     QVERIFY(fullscreenChangedSpy.wait());
     QVERIFY(sizeChangeRequestedSpy.wait());
@@ -602,6 +617,24 @@ void TestShellClient::testMaximizedToFullscreen()
     QFETCH(Test::ShellSurfaceType, type);
     QScopedPointer<QObject> 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*>(shellSurface.data());
+        break;
+    case Test::ShellSurfaceType::XdgShellV5:
+    case Test::ShellSurfaceType::XdgShellV6:
+        xdgShellSurface = qobject_cast<XdgShellSurface*>(shellSurface.data());
+        break;
+    default:
+        Q_UNREACHABLE();
+        break;
+    }
+    QVERIFY(wlShellSurface || xdgShellSurface);
+    QVERIFY(!(wlShellSurface && xdgShellSurface));
+
     // create deco
     QScopedPointer<ServerSideDecoration> 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) {
+    if (wlShellSurface) {
-    case Test::ShellSurfaceType::WlShell:
+        wlShellSurface->setMaximized();
-        qobject_cast<ShellSurface*>(shellSurface.data())->setMaximized();
+    }
-        break;
+    if (xdgShellSurface) {
-    case Test::ShellSurfaceType::XdgShellV5:
+        xdgShellSurface->setMaximized(true);
-    case Test::ShellSurfaceType::XdgShellV6:
-        qobject_cast<XdgShellSurface*>(shellSurface.data())->setMaximized(true);
-        break;
-    default:
-        Q_UNREACHABLE();
-        break;
     }
     QVERIFY(sizeChangeRequestedSpy.wait());
     QCOMPARE(sizeChangeRequestedSpy.count(), 1);
@@ -645,17 +676,11 @@ void TestShellClient::testMaximizedToFullscreen()
     geometryChangedSpy.clear();
 
     // fullscreen the window
-    switch (type) {
+    if (wlShellSurface) {
-    case Test::ShellSurfaceType::WlShell:
+        wlShellSurface->setFullscreen();
-        qobject_cast<ShellSurface*>(shellSurface.data())->setFullscreen();
+    }
-        break;
+    if (xdgShellSurface) {
-    case Test::ShellSurfaceType::XdgShellV5:
+        xdgShellSurface->setFullscreen(true);
-    case Test::ShellSurfaceType::XdgShellV6:
-        qobject_cast<XdgShellSurface*>(shellSurface.data())->setFullscreen(true);
-        break;
-    default:
-        Q_UNREACHABLE();
-        break;
     }
     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());

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();
+        updatePendingGeometry();
-        if (m_positionAfterResize.isValid()) {
-            addLayerRepaint(geometry());
-            position = m_positionAfterResize.point();
-            m_positionAfterResize.clear();
-        }
-        doSetGeometry(QRect(position, m_clientSize + QSize(borderLeft() + borderRight(), borderTop() + borderBottom())));
     }
     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()

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<PendingConfigureRequest> 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;