XdgPopupWindow: Reposition for non-reactive positioners

Ensures that e.g. context menus move about with their parents when they
get moved around.

However, as per spec don't re-constrain the window when its positioner
is non-reactive. This change calculates the offset from its parent window
once initially and places the window relative to that whenever the parent
moves.

Only when the positioner is reactive, will it recalculate the placement fully.

BUG: 461994

autotests/integration/xdgshellwindow_test.cpp

@@ -93,8 +93,9 @@ private Q_SLOTS:
     void testXdgWindowGeometryInteractiveResize();
     void testXdgWindowGeometryFullScreen();
     void testXdgWindowGeometryMaximize();
-    void testXdgWindowReactive();
-    void testXdgWindowRepositioning();
+    void testXdgPopupReactive_data();
+    void testXdgPopupReactive();
+    void testXdgPopupReposition();
     void testPointerInputTransform();
     void testReentrantSetFrameGeometry();
     void testDoubleMaximize();
@@ -104,12 +105,34 @@ private Q_SLOTS:
     void testChangeDecorationModeAfterInitialCommit();
 };
 
-void TestXdgShellWindow::testXdgWindowReactive()
+void TestXdgShellWindow::testXdgPopupReactive_data()
 {
+    QTest::addColumn<bool>("reactive");
+    QTest::addColumn<QPointF>("parentPos");
+    QTest::addColumn<QPointF>("popupPos");
+
+    QTest::addRow("reactive") << true << QPointF(0, 1024) << QPointF(50, 1024 - 10);
+    QTest::addRow("not reactive") << false << QPointF(0, 1024) << QPointF(50, 1024 + 40);
+}
+
+void TestXdgShellWindow::testXdgPopupReactive()
+{
+    // This test verifies the behavior of reactive popups. If a popup is not reactive,
+    // it only has to move together with its parent. If a popup is reactive, it moves
+    // with its parent and it's reconstrained as needed.
+
     std::unique_ptr<Test::XdgPositioner> positioner(Test::createXdgPositioner());
     positioner->set_size(10, 10);
     positioner->set_anchor_rect(10, 10, 10, 10);
-    positioner->set_reactive();
+    positioner->set_anchor_rect(0, 0, 50, 40);
+    positioner->set_anchor(Test::XdgPositioner::anchor_bottom_right);
+    positioner->set_gravity(Test::XdgPositioner::gravity_bottom_right);
+    positioner->set_constraint_adjustment(Test::XdgPositioner::constraint_adjustment_slide_y);
+
+    QFETCH(bool, reactive);
+    if (reactive) {
+        positioner->set_reactive();
+    }
 
     std::unique_ptr<KWayland::Client::Surface> rootSurface(Test::createSurface());
     std::unique_ptr<Test::XdgToplevel> root(Test::createXdgToplevelSurface(rootSurface.get()));
@@ -121,14 +144,18 @@ void TestXdgShellWindow::testXdgWindowReactive()
     auto childWindow = Test::renderAndWaitForShown(childSurface.get(), QSize(10, 10), Qt::cyan);
     QVERIFY(childWindow);
 
-    QSignalSpy popupConfigureRequested(popup.get(), &Test::XdgPopup::configureRequested);
-    rootWindow->move(rootWindow->pos() + QPoint(20, 20));
+    QFETCH(QPointF, parentPos);
+    QFETCH(QPointF, popupPos);
 
+    QSignalSpy popupConfigureRequested(popup.get(), &Test::XdgPopup::configureRequested);
+    rootWindow->move(parentPos);
     QVERIFY(popupConfigureRequested.wait());
     QCOMPARE(popupConfigureRequested.count(), 1);
+
+    QCOMPARE(childWindow->pos(), popupPos);
 }
 
-void TestXdgShellWindow::testXdgWindowRepositioning()
+void TestXdgShellWindow::testXdgPopupReposition()
 {
     std::unique_ptr<Test::XdgPositioner> positioner(Test::createXdgPositioner());
     positioner->set_size(10, 10);

src/xdgshellwindow.cpp

@@ -1613,27 +1613,25 @@ void XdgPopupWindow::handleRoleDestroyed()
     XdgSurfaceWindow::handleRoleDestroyed();
 }
 
-void XdgPopupWindow::updateReactive()
-{
-    if (m_shellSurface->positioner().isReactive()) {
-        connect(transientFor(), &Window::frameGeometryChanged,
-                this, &XdgPopupWindow::relayout, Qt::UniqueConnection);
-    } else {
-        disconnect(transientFor(), &Window::frameGeometryChanged,
-                   this, &XdgPopupWindow::relayout);
-    }
-}
-
 void XdgPopupWindow::handleRepositionRequested(quint32 token)
 {
-    updateReactive();
+    updateRelativePlacement();
     m_shellSurface->sendRepositioned(token);
     relayout();
 }
 
+void XdgPopupWindow::updateRelativePlacement()
+{
+    const QPointF parentPosition = transientFor()->framePosToClientPos(transientFor()->pos());
+    m_relativePlacement = placement().translated(-parentPosition);
+}
+
 void XdgPopupWindow::relayout()
 {
-    workspace()->placement()->place(this, QRect());
+    if (m_shellSurface->positioner().isReactive()) {
+        updateRelativePlacement();
+    }
+    workspace()->placement()->place(this, QRectF());
     scheduleConfigure();
 }
 
@@ -1681,8 +1679,16 @@ bool XdgPopupWindow::hasTransientPlacementHint() const
     return true;
 }
 
-QRectF XdgPopupWindow::transientPlacement(const QRectF &bounds) const
+QRectF XdgPopupWindow::transientPlacement() const
 {
+    const QPointF parentPosition = transientFor()->framePosToClientPos(transientFor()->pos());
+    return m_relativePlacement.translated(parentPosition);
+}
+
+QRectF XdgPopupWindow::placement() const
+{
+    const QRectF bounds = Workspace::self()->clientArea(transientFor()->isFullScreen() ? FullScreenArea : PlacementArea, transientFor());
+
     const XdgPositioner positioner = m_shellSurface->positioner();
     const QSize desiredSize = positioner.size();
 
@@ -1854,10 +1860,10 @@ void XdgPopupWindow::initialize()
     parent->addTransient(this);
     setTransientFor(parent);
 
-    updateReactive();
+    updateRelativePlacement();
+    connect(parent, &Window::frameGeometryChanged, this, &XdgPopupWindow::relayout);
 
-    const QRectF area = workspace()->clientArea(PlacementArea, this, workspace()->activeOutput());
-    workspace()->placement()->place(this, area);
+    workspace()->placement()->place(this, QRectF());
     scheduleConfigure();
 }
 

src/xdgshellwindow.h

@@ -250,7 +250,7 @@ public:
     bool isMovable() const override;
     bool isMovableAcrossScreens() const override;
     bool hasTransientPlacementHint() const override;
-    QRectF transientPlacement(const QRectF &bounds) const override;
+    QRectF transientPlacement() const override;
     bool isCloseable() const override;
     void closeWindow() override;
     bool wantsInput() const override;
@@ -262,14 +262,17 @@ protected:
     void handleRoleDestroyed() override;
 
 private:
+    QRectF placement() const;
+
     void handleGrabRequested(KWaylandServer::SeatInterface *seat, quint32 serial);
     void handleRepositionRequested(quint32 token);
     void initialize();
+    void updateRelativePlacement();
     void relayout();
-    void updateReactive();
 
     KWaylandServer::XdgPopupInterface *m_shellSurface;
     bool m_haveExplicitGrab = false;
+    QRectF m_relativePlacement;
 };
 
 } // namespace KWin