From b9b735708653ff6bf57278a1999679df31ee5775 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Mon, 24 Jan 2022 15:16:45 +0200
Subject: [PATCH] wayland: Fix getting the last configure event

If there's only one configure event that changes the position of the
window and it gets acknowledged but no buffer is attached yet, and a new
configure is sent, then the ConfigurePosition flag won't be inherited
by the new configure event and the window will be misplaced.

In order to fix that, this change makes XdgSurfaceClient pop the last
acknowledged configure event from the m_configureEvents list only when
it's about to be applied for sure.

BUG: 448856
---
 autotests/integration/xdgshellclient_test.cpp | 34 +++++++++++++++++++
 src/xdgshellclient.cpp                        | 18 ++++++----
 src/xdgshellclient.h                          |  3 ++
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/autotests/integration/xdgshellclient_test.cpp b/autotests/integration/xdgshellclient_test.cpp
index 7855e85287..d11c34c456 100644
--- a/autotests/integration/xdgshellclient_test.cpp
+++ b/autotests/integration/xdgshellclient_test.cpp
@@ -103,6 +103,7 @@ private Q_SLOTS:
     void testPointerInputTransform();
     void testReentrantSetFrameGeometry();
     void testDoubleMaximize();
+    void testDoubleFullscreenSeparatedByCommit();
     void testMaximizeAndChangeDecorationModeAfterInitialCommit();
     void testFullScreenAndChangeDecorationModeAfterInitialCommit();
     void testChangeDecorationModeAfterInitialCommit();
@@ -1565,6 +1566,39 @@ void TestXdgShellClient::testDoubleMaximize()
     QVERIFY(states.testFlag(Test::XdgToplevel::State::Maximized));
 }
 
+void TestXdgShellClient::testDoubleFullscreenSeparatedByCommit()
+{
+    // Some applications do weird things at startup and this is one of them. This test verifies
+    // that the window will have good frame geometry if the client has issued several
+    // xdg_toplevel.set_fullscreen requests and they are separated by a surface commit with
+    // no attached buffer.
+
+    QScopedPointer<KWayland::Client::Surface> surface(Test::createSurface());
+    QScopedPointer<Test::XdgToplevel> shellSurface(Test::createXdgToplevelSurface(surface.data()));
+    QSignalSpy toplevelConfigureRequestedSpy(shellSurface.data(), &Test::XdgToplevel::configureRequested);
+    QSignalSpy surfaceConfigureRequestedSpy(shellSurface->xdgSurface(), &Test::XdgSurface::configureRequested);
+
+    // Tell the compositor that we want the window to be shown in fullscreen mode.
+    shellSurface->set_fullscreen(nullptr);
+    QVERIFY(surfaceConfigureRequestedSpy.wait());
+    QCOMPARE(toplevelConfigureRequestedSpy.last().at(0).value<QSize>(), QSize(1280, 1024));
+    QVERIFY(toplevelConfigureRequestedSpy.last().at(1).value<Test::XdgToplevel::States>() & Test::XdgToplevel::State::Fullscreen);
+
+    // Ask again.
+    shellSurface->xdgSurface()->ack_configure(surfaceConfigureRequestedSpy.last().at(0).value<quint32>());
+    surface->commit(KWayland::Client::Surface::CommitFlag::None);
+    shellSurface->set_fullscreen(nullptr);
+    QVERIFY(surfaceConfigureRequestedSpy.wait());
+    QCOMPARE(toplevelConfigureRequestedSpy.last().at(0).value<QSize>(), QSize(1280, 1024));
+    QVERIFY(toplevelConfigureRequestedSpy.last().at(1).value<Test::XdgToplevel::States>() & Test::XdgToplevel::State::Fullscreen);
+
+    // Map the window.
+    shellSurface->xdgSurface()->ack_configure(surfaceConfigureRequestedSpy.last().at(0).value<quint32>());
+    auto client = Test::renderAndWaitForShown(surface.data(), QSize(1280, 1024), Qt::blue);
+    QVERIFY(client->isFullScreen());
+    QCOMPARE(client->frameGeometry(), QRect(0, 0, 1280, 1024));
+}
+
 void TestXdgShellClient::testMaximizeHorizontal()
 {
     // Create the test client.
diff --git a/src/xdgshellclient.cpp b/src/xdgshellclient.cpp
index b10d994687..b1934c4a88 100644
--- a/src/xdgshellclient.cpp
+++ b/src/xdgshellclient.cpp
@@ -146,12 +146,7 @@ void XdgSurfaceClient::sendConfigure()
 
 void XdgSurfaceClient::handleConfigureAcknowledged(quint32 serial)
 {
-    while (!m_configureEvents.isEmpty()) {
-        if (serial < m_configureEvents.first()->serial) {
-            break;
-        }
-        m_lastAcknowledgedConfigure.reset(m_configureEvents.takeFirst());
-    }
+    m_lastAcknowledgedConfigureSerial = serial;
 }
 
 void XdgSurfaceClient::handleCommit()
@@ -160,6 +155,16 @@ void XdgSurfaceClient::handleCommit()
         return;
     }
 
+    if (m_lastAcknowledgedConfigureSerial.has_value()) {
+        const quint32 serial = m_lastAcknowledgedConfigureSerial.value();
+        while (!m_configureEvents.isEmpty()) {
+            if (serial < m_configureEvents.constFirst()->serial) {
+                break;
+            }
+            m_lastAcknowledgedConfigure.reset(m_configureEvents.takeFirst());
+        }
+    }
+
     handleRolePrecommit();
     if (haveNextWindowGeometry()) {
         handleNextWindowGeometry();
@@ -168,6 +173,7 @@ void XdgSurfaceClient::handleCommit()
 
     handleRoleCommit();
     m_lastAcknowledgedConfigure.reset();
+    m_lastAcknowledgedConfigureSerial.reset();
 
     setReadyForPainting();
     updateDepth();
diff --git a/src/xdgshellclient.h b/src/xdgshellclient.h
index 93d2a490b9..e7810027ea 100644
--- a/src/xdgshellclient.h
+++ b/src/xdgshellclient.h
@@ -18,6 +18,8 @@
 #include <QQueue>
 #include <QTimer>
 
+#include <optional>
+
 namespace KWaylandServer
 {
 class AppMenuInterface;
@@ -96,6 +98,7 @@ private:
     XdgSurfaceConfigure::ConfigureFlags m_configureFlags;
     QQueue<XdgSurfaceConfigure *> m_configureEvents;
     QScopedPointer<XdgSurfaceConfigure> m_lastAcknowledgedConfigure;
+    std::optional<quint32> m_lastAcknowledgedConfigureSerial;
     QRect m_windowGeometry;
     bool m_haveNextWindowGeometry = false;
 };