From 77f712d3a7e60db1933e8583942c3e1b7cdc970f Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Wed, 2 Feb 2022 22:34:01 +0200 Subject: [PATCH] Fix tracking of geometry restore with electric maximize If the user wants to move a tiled window, but changes their mind and tiles the window back to the previous position, the geometryRestore() will be corrupted because initialMoveResizeGeometry() is the same as the geometry of the window in the tiled mode. This change fixes tracking of the geometry restore by precomputing the geometry restore when starting interactive move. That way, if the window is untiled and tiled again without release left pointer button, the geometry restore will be set to the correct value in setQuickTileMode(). This change also adjusts the test suite so such a subtle case won't be broken again without noticing it. --- autotests/integration/quick_tiling_test.cpp | 72 ++++++++++++--------- src/abstract_client.cpp | 48 ++++++++++---- src/abstract_client.h | 3 + 3 files changed, 79 insertions(+), 44 deletions(-) diff --git a/autotests/integration/quick_tiling_test.cpp b/autotests/integration/quick_tiling_test.cpp index abeb0a9ea5..c15a239d80 100644 --- a/autotests/integration/quick_tiling_test.cpp +++ b/autotests/integration/quick_tiling_test.cpp @@ -386,15 +386,16 @@ void QuickTilingTest::testQuickTilingKeyboardMove() void QuickTilingTest::testQuickTilingPointerMove_data() { - QTest::addColumn("targetPos"); + QTest::addColumn("pointerPos"); + QTest::addColumn("tileSize"); QTest::addColumn("expectedMode"); - QTest::newRow("topRight") << QPoint(2559, 24) << QuickTileMode(QuickTileFlag::Top | QuickTileFlag::Right); - QTest::newRow("right") << QPoint(2559, 512) << QuickTileMode(QuickTileFlag::Right); - QTest::newRow("bottomRight") << QPoint(2559, 1023) << QuickTileMode(QuickTileFlag::Bottom | QuickTileFlag::Right); - QTest::newRow("bottomLeft") << QPoint(0, 1023) << QuickTileMode(QuickTileFlag::Bottom | QuickTileFlag::Left); - QTest::newRow("Left") << QPoint(0, 512) << QuickTileMode(QuickTileFlag::Left); - QTest::newRow("topLeft") << QPoint(0, 24) << QuickTileMode(QuickTileFlag::Top | QuickTileFlag::Left); + QTest::newRow("topRight") << QPoint(2559, 24) << QSize(640, 512) << QuickTileMode(QuickTileFlag::Top | QuickTileFlag::Right); + QTest::newRow("right") << QPoint(2559, 512) << QSize(640, 1024) << QuickTileMode(QuickTileFlag::Right); + QTest::newRow("bottomRight") << QPoint(2559, 1023) << QSize(640, 512) << QuickTileMode(QuickTileFlag::Bottom | QuickTileFlag::Right); + QTest::newRow("bottomLeft") << QPoint(0, 1023) << QSize(640, 512) << QuickTileMode(QuickTileFlag::Bottom | QuickTileFlag::Left); + QTest::newRow("Left") << QPoint(0, 512) << QSize(640, 1024) << QuickTileMode(QuickTileFlag::Left); + QTest::newRow("topLeft") << QPoint(0, 24) << QSize(640, 512) << QuickTileMode(QuickTileFlag::Top | QuickTileFlag::Left); } void QuickTilingTest::testQuickTilingPointerMove() @@ -402,24 +403,10 @@ void QuickTilingTest::testQuickTilingPointerMove() using namespace KWayland::Client; QScopedPointer surface(Test::createSurface()); - QVERIFY(!surface.isNull()); - - QScopedPointer shellSurface(Test::createXdgToplevelSurface(surface.data(), Test::CreationSetup::CreateOnly)); - QVERIFY(!shellSurface.isNull()); - - // wait for the initial configure event - QSignalSpy toplevelConfigureRequestedSpy(shellSurface.data(), &Test::XdgToplevel::configureRequested); - QVERIFY(toplevelConfigureRequestedSpy.isValid()); - QSignalSpy surfaceConfigureRequestedSpy(shellSurface->xdgSurface(), &Test::XdgSurface::configureRequested); - QVERIFY(surfaceConfigureRequestedSpy.isValid()); - surface->commit(KWayland::Client::Surface::CommitFlag::None); - QVERIFY(surfaceConfigureRequestedSpy.wait()); - QCOMPARE(surfaceConfigureRequestedSpy.count(), 1); + QScopedPointer shellSurface(Test::createXdgToplevelSurface(surface.data())); // let's render - shellSurface->xdgSurface()->ack_configure(surfaceConfigureRequestedSpy.last().at(0).value()); auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue); - QVERIFY(c); QCOMPARE(workspace()->activeClient(), c); QCOMPARE(c->frameGeometry(), QRect(0, 0, 100, 50)); @@ -427,29 +414,52 @@ void QuickTilingTest::testQuickTilingPointerMove() QCOMPARE(c->maximizeMode(), MaximizeRestore); // we have to receive a configure event when the client becomes active + QSignalSpy toplevelConfigureRequestedSpy(shellSurface.data(), &Test::XdgToplevel::configureRequested); + QSignalSpy surfaceConfigureRequestedSpy(shellSurface->xdgSurface(), &Test::XdgSurface::configureRequested); QVERIFY(surfaceConfigureRequestedSpy.wait()); - QTRY_COMPARE(surfaceConfigureRequestedSpy.count(), 2); + QCOMPARE(surfaceConfigureRequestedSpy.count(), 1); + // verify that basic quick tile mode works as expected, i.e. the window is going to be + // tiled if the user drags it to a screen edge or a corner QSignalSpy quickTileChangedSpy(c, &AbstractClient::quickTileModeChanged); - QVERIFY(quickTileChangedSpy.isValid()); - workspace()->performWindowOperation(c, Options::UnrestrictedMoveOp); QCOMPARE(c, workspace()->moveResizeClient()); QCOMPARE(Cursors::self()->mouse()->pos(), QPoint(49, 24)); - QFETCH(QPoint, targetPos); + QFETCH(QPoint, pointerPos); + QFETCH(QSize, tileSize); quint32 timestamp = 1; - kwinApp()->platform()->pointerMotion(targetPos, timestamp++); kwinApp()->platform()->pointerButtonPressed(BTN_LEFT, timestamp++); + kwinApp()->platform()->pointerMotion(pointerPos, timestamp++); kwinApp()->platform()->pointerButtonReleased(BTN_LEFT, timestamp++); - QCOMPARE(Cursors::self()->mouse()->pos(), targetPos); - QVERIFY(!workspace()->moveResizeClient()); - QCOMPARE(quickTileChangedSpy.count(), 1); QTEST(c->quickTileMode(), "expectedMode"); + QCOMPARE(c->geometryRestore(), QRect(0, 0, 100, 50)); + QVERIFY(surfaceConfigureRequestedSpy.wait()); + QCOMPARE(surfaceConfigureRequestedSpy.count(), 2); + QCOMPARE(toplevelConfigureRequestedSpy.last().at(0).toSize(), tileSize); + + // verify that geometry restore is correct after user untiles the window, but changes + // their mind and tiles the window again while still holding left button + workspace()->performWindowOperation(c, Options::UnrestrictedMoveOp); + QCOMPARE(c, workspace()->moveResizeClient()); + + kwinApp()->platform()->pointerButtonPressed(BTN_LEFT, timestamp++); // untile the window + kwinApp()->platform()->pointerMotion(QPoint(1280, 1024) / 2, timestamp++); + QCOMPARE(quickTileChangedSpy.count(), 2); + QCOMPARE(c->quickTileMode(), QuickTileMode(QuickTileFlag::None)); QVERIFY(surfaceConfigureRequestedSpy.wait()); QCOMPARE(surfaceConfigureRequestedSpy.count(), 3); - QCOMPARE(false, toplevelConfigureRequestedSpy.last().first().toSize().isEmpty()); + QCOMPARE(toplevelConfigureRequestedSpy.last().at(0).toSize(), QSize(100, 50)); + + kwinApp()->platform()->pointerMotion(pointerPos, timestamp++); // tile the window again + kwinApp()->platform()->pointerButtonReleased(BTN_LEFT, timestamp++); + QCOMPARE(quickTileChangedSpy.count(), 3); + QTEST(c->quickTileMode(), "expectedMode"); + QCOMPARE(c->geometryRestore(), QRect(0, 0, 100, 50)); + QVERIFY(surfaceConfigureRequestedSpy.wait()); + QCOMPARE(surfaceConfigureRequestedSpy.count(), 4); + QCOMPARE(toplevelConfigureRequestedSpy.last().at(0).toSize(), tileSize); } void QuickTilingTest::testQuickTilingTouchMove_data() diff --git a/src/abstract_client.cpp b/src/abstract_client.cpp index fbb0b038b8..998431df11 100644 --- a/src/abstract_client.cpp +++ b/src/abstract_client.cpp @@ -991,6 +991,7 @@ bool AbstractClient::startInteractiveMoveResize() } updateInitialMoveResizeGeometry(); + updateElectricGeometryRestore(); checkUnrestrictedInteractiveMoveResize(); Q_EMIT clientStartUserMovedResized(this); if (ScreenEdges::self()->isDesktopSwitchingMovingClients()) @@ -3053,6 +3054,35 @@ QRect AbstractClient::electricBorderMaximizeGeometry(const QPoint &pos) const return ret; } +void AbstractClient::updateElectricGeometryRestore() +{ + m_electricGeometryRestore = geometryRestore(); + if (quickTileMode() == QuickTileMode(QuickTileFlag::None)) { + if (!(maximizeMode() & MaximizeHorizontal)) { + m_electricGeometryRestore.setX(x()); + m_electricGeometryRestore.setWidth(width()); + } + if (!(maximizeMode() & MaximizeVertical)) { + m_electricGeometryRestore.setY(y()); + m_electricGeometryRestore.setHeight(height()); + } + } +} + +QRect AbstractClient::quickTileGeometryRestore() const +{ + if (quickTileMode() != QuickTileMode(QuickTileFlag::None)) { + // If the window is tiled, geometryRestore() already has a good value. + return geometryRestore(); + } + + if (isElectricBorderMaximizing()) { + return m_electricGeometryRestore; + } else { + return moveResizeGeometry(); + } +} + void AbstractClient::setQuickTileMode(QuickTileMode mode, bool keyboard) { // Only allow quick tile on a regular window. @@ -3069,15 +3099,7 @@ void AbstractClient::setQuickTileMode(QuickTileMode mode, bool keyboard) m_quickTileMode = int(QuickTileFlag::None); setMaximize(false, false); } else { - // If the window is tiled, geometryRestore() already has a good value. - QRect effectiveGeometryRestore = geometryRestore(); - if (m_quickTileMode == QuickTileMode(QuickTileFlag::None)) { - if (isElectricBorderMaximizing()) { - effectiveGeometryRestore = initialInteractiveMoveResizeGeometry(); - } else { - effectiveGeometryRestore = moveResizeGeometry(); - } - } + QRect effectiveGeometryRestore = quickTileGeometryRestore(); m_quickTileMode = int(QuickTileFlag::Maximize); setMaximize(true, true); setGeometryRestore(effectiveGeometryRestore); @@ -3169,7 +3191,7 @@ void AbstractClient::setQuickTileMode(QuickTileMode mode, bool keyboard) } else if (quickTileMode() == QuickTileMode(QuickTileFlag::None)) { // Not coming out of an existing tile, not shifting monitors, we're setting a brand new tile. // Store geometry first, so we can go out of this tile later. - setGeometryRestore(moveResizeGeometry()); + setGeometryRestore(quickTileGeometryRestore()); } if (mode != QuickTileMode(QuickTileFlag::None)) { @@ -3186,9 +3208,9 @@ void AbstractClient::setQuickTileMode(QuickTileMode mode, bool keyboard) if (mode == QuickTileMode(QuickTileFlag::None)) { m_quickTileMode = int(QuickTileFlag::None); // Untiling, so just restore geometry, and we're done. - if (!geometryRestore().isValid()) // invalid if we started maximized and wait for placement - setGeometryRestore(moveResizeGeometry()); - moveResize(geometryRestore()); + if (geometryRestore().isValid()) { // invalid if we started maximized and wait for placement + moveResize(geometryRestore()); + } checkWorkspacePosition(); // Just in case it's a different screen } doSetQuickTileMode(); diff --git a/src/abstract_client.h b/src/abstract_client.h index 1dfd64b1d8..c9d0bd799c 100644 --- a/src/abstract_client.h +++ b/src/abstract_client.h @@ -1030,6 +1030,8 @@ protected: return m_electricMaximizing; } QRect electricBorderMaximizeGeometry(const QPoint &pos) const; + void updateElectricGeometryRestore(); + QRect quickTileGeometryRestore() const; void updateQuickTileMode(QuickTileMode newMode) { m_quickTileMode = newMode; } @@ -1259,6 +1261,7 @@ private: // electric border/quick tiling QuickTileMode m_electricMode = QuickTileFlag::None; + QRect m_electricGeometryRestore; bool m_electricMaximizing = false; // The quick tile mode of this window. int m_quickTileMode = int(QuickTileFlag::None);