From 14c8440f11f9af48e63ca0fcf05ee7988f445b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Sat, 8 Jul 2017 08:41:16 +0200 Subject: [PATCH] Restrict move resize area only on the screen the strut window is on Summary: By allowing panels between screens in 5.8 to have a strut we created a "regression" in KWin. KWin always was wrong, just we didn't notice as neither Plasma nor previously Kicker set a strut on panels between shared screen edges. The strut is created from the edge of the overall screen setup. This means a panel on the left edge of a screen on the right has the strut starting from the left screen. KWin uses the strut to restrict the move resize area: a window decoration is not allowed to go below a strut. Thus it becomes impossible to move the window from the right to the left screen. This change tries to solve this problem by only restricting the move area on the screen the window with the strut is on. E.g. if the window is on the right screen, the left screen is not affected. Thus it's possible again to move a window from one screen to the other as the added test case shows. Unfortunately there are still corner cases where this won't work correctly. If the window is on both screens this won't work. It is also a rather heavy change for KWin and thus it's targeted for master and not for the 5.10 or the 5.8 branch. If we notice that the patch works well and doesn't create further issues, it should be considered for backporting. BUG: 371199 CCBUG: 370510 FIXED-IN: 5.11 Test Plan: Added test case Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D6562 --- autotests/integration/struts_test.cpp | 170 +++++++++++++++++++++++--- geometry.cpp | 4 + 2 files changed, 155 insertions(+), 19 deletions(-) diff --git a/autotests/integration/struts_test.cpp b/autotests/integration/struts_test.cpp index 92e0672bef..8a208b347c 100644 --- a/autotests/integration/struts_test.cpp +++ b/autotests/integration/struts_test.cpp @@ -59,6 +59,7 @@ private Q_SLOTS: void testX11Struts(); void test363804(); void testLeftScreenSmallerBottomAligned(); + void testWindowMoveWithPanelBetweenScreens(); private: KWayland::Client::Compositor *m_compositor = nullptr; @@ -321,6 +322,7 @@ void StrutsTest::testX11Struts_data() QTest::addColumn("screen0Maximized"); QTest::addColumn("screen1Maximized"); QTest::addColumn("workArea"); + QTest::addColumn("restrictedMoveArea"); QTest::newRow("bottom panel/no strut") << QRect(0, 980, 1280, 44) << 0 << 0 << 0 << 0 @@ -330,7 +332,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(); QTest::newRow("bottom panel/strut") << QRect(0, 980, 1280, 44) << 0 << 0 << 0 << 44 << 0 << 0 @@ -339,7 +342,8 @@ void StrutsTest::testX11Struts_data() << 0 << 1279 << QRect(0, 0, 1280, 980) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 980); + << QRect(0, 0, 2560, 980) + << QRegion(0, 980, 1279, 44); QTest::newRow("top panel/no strut") << QRect(0, 0, 1280, 44) << 0 << 0 << 0 << 0 << 0 << 0 @@ -348,7 +352,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(); QTest::newRow("top panel/strut") << QRect(0, 0, 1280, 44) << 0 << 0 << 44 << 0 << 0 << 0 @@ -357,7 +362,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 44, 1280, 980) << QRect(1280, 0, 1280, 1024) - << QRect(0, 44, 2560, 980); + << QRect(0, 44, 2560, 980) + << QRegion(0, 0, 1279, 44); QTest::newRow("left panel/no strut") << QRect(0, 0, 60, 1024) << 0 << 0 << 0 << 0 << 0 << 0 @@ -366,7 +372,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(); QTest::newRow("left panel/strut") << QRect(0, 0, 60, 1024) << 60 << 0 << 0 << 0 << 0 << 1023 @@ -375,8 +382,9 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(60, 0, 1220, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(60, 0, 2500, 1024); - QTest::newRow("right panel/no strut") << QRect(0, 1220, 60, 1024) + << QRect(60, 0, 2500, 1024) + << QRegion(0, 0, 60, 1023); + QTest::newRow("right panel/no strut") << QRect(1220, 0, 60, 1024) << 0 << 0 << 0 << 0 << 0 << 0 << 0 << 0 @@ -384,8 +392,9 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); - QTest::newRow("right panel/strut") << QRect(0, 1220, 60, 1024) + << QRect(0, 0, 2560, 1024) + << QRegion(); + QTest::newRow("right panel/strut") << QRect(1220, 0, 60, 1024) << 0 << 1340 << 0 << 0 << 0 << 0 << 0 << 1023 @@ -393,7 +402,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1220, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(1220, 0, 60, 1023); // second screen QTest::newRow("bottom panel 1/no strut") << QRect(1280, 980, 1280, 44) << 0 << 0 << 0 << 0 @@ -403,7 +413,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(); QTest::newRow("bottom panel 1/strut") << QRect(1280, 980, 1280, 44) << 0 << 0 << 0 << 44 << 0 << 0 @@ -412,7 +423,8 @@ void StrutsTest::testX11Struts_data() << 1280 << 2559 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 980) - << QRect(0, 0, 2560, 980); + << QRect(0, 0, 2560, 980) + << QRegion(1280, 980, 1279, 44); QTest::newRow("top panel 1/no strut") << QRect(1280, 0, 1280, 44) << 0 << 0 << 0 << 0 << 0 << 0 @@ -421,7 +433,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(); QTest::newRow("top panel 1 /strut") << QRect(1280, 0, 1280, 44) << 0 << 0 << 44 << 0 << 0 << 0 @@ -430,7 +443,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 44, 1280, 980) - << QRect(0, 44, 2560, 980); + << QRect(0, 44, 2560, 980) + << QRegion(1280, 0, 1279, 44); QTest::newRow("left panel 1/no strut") << QRect(1280, 0, 60, 1024) << 0 << 0 << 0 << 0 << 0 << 0 @@ -439,7 +453,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(); QTest::newRow("left panel 1/strut") << QRect(1280, 0, 60, 1024) << 1340 << 0 << 0 << 0 << 0 << 1023 @@ -448,7 +463,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1340, 0, 1220, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(1280, 0, 60, 1023); // invalid struts QTest::newRow("bottom panel/ invalid strut") << QRect(0, 980, 1280, 44) << 1280 << 0 << 0 << 44 @@ -458,7 +474,8 @@ void StrutsTest::testX11Struts_data() << 0 << 1279 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(0, 980, 1280, 44); QTest::newRow("top panel/ invalid strut") << QRect(0, 0, 1280, 44) << 1280 << 0 << 44 << 0 << 0 << 44 @@ -467,7 +484,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(0, 0, 1280, 44); QTest::newRow("top panel/invalid strut 2") << QRect(0, 0, 1280, 44) << 0 << 0 << 1024 << 0 << 0 << 0 @@ -476,7 +494,8 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << QRect(0, 0, 1280, 1024) << QRect(1280, 0, 1280, 1024) - << QRect(0, 0, 2560, 1024); + << QRect(0, 0, 2560, 1024) + << QRegion(0, 0, 1279, 1024); } struct XcbConnectionDeleter @@ -509,6 +528,7 @@ void StrutsTest::testX11Struts() // combined QCOMPARE(workspace()->clientArea(WorkArea, 0, 1), QRect(0, 0, 2560, 1024)); QCOMPARE(workspace()->clientArea(FullArea, 0, 1), QRect(0, 0, 2560, 1024)); + QCOMPARE(workspace()->restrictedMoveArea(-1), QRegion()); // create an xcb window QScopedPointer c(xcb_connect(nullptr, nullptr)); @@ -591,6 +611,7 @@ void StrutsTest::testX11Struts() QTEST(workspace()->clientArea(PlacementArea, 1, 1), "screen1Maximized"); QTEST(workspace()->clientArea(MaximizeArea, 1, 1), "screen1Maximized"); QTEST(workspace()->clientArea(WorkArea, 0, 1), "workArea"); + QTEST(workspace()->restrictedMoveArea(-1), "restrictedMoveArea"); // and destroy the window again xcb_unmap_window(c.data(), w); @@ -619,6 +640,7 @@ void StrutsTest::testX11Struts() // combined QCOMPARE(workspace()->clientArea(WorkArea, 0, 1), QRect(0, 0, 2560, 1024)); QCOMPARE(workspace()->clientArea(FullArea, 0, 1), QRect(0, 0, 2560, 1024)); + QCOMPARE(workspace()->restrictedMoveArea(-1), QRegion()); } void StrutsTest::test363804() @@ -808,6 +830,116 @@ void StrutsTest::testLeftScreenSmallerBottomAligned() QVERIFY(windowClosedSpy.wait()); } +void StrutsTest::testWindowMoveWithPanelBetweenScreens() +{ + // this test verifies the condition of BUG + // when moving a window with decorations in a restricted way it should pass from one screen + // to the other even if there is a panel in between. + + // left screen must be smaller than right screen + const QVector geometries{QRect(0, 282, 1366, 768), QRect(1366, 0, 1680, 1050)}; + QMetaObject::invokeMethod(kwinApp()->platform(), "outputGeometriesChanged", + Qt::DirectConnection, + Q_ARG(QVector, geometries)); + QCOMPARE(screens()->geometry(0), geometries.at(0)); + QCOMPARE(screens()->geometry(1), geometries.at(1)); + QCOMPARE(screens()->geometry(), QRect(0, 0, 3046, 1050)); + + // create the panel on the right screen, left edge + QScopedPointer c(xcb_connect(nullptr, nullptr)); + QVERIFY(!xcb_connection_has_error(c.data())); + + xcb_window_t w = xcb_generate_id(c.data()); + const QRect windowGeometry(1366, 0, 24, 1050); + xcb_create_window(c.data(), XCB_COPY_FROM_PARENT, w, rootWindow(), + windowGeometry.x(), + windowGeometry.y(), + windowGeometry.width(), + windowGeometry.height(), + 0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT, 0, nullptr); + xcb_size_hints_t hints; + memset(&hints, 0, sizeof(hints)); + xcb_icccm_size_hints_set_position(&hints, 1, windowGeometry.x(), windowGeometry.y()); + xcb_icccm_size_hints_set_size(&hints, 1, windowGeometry.width(), windowGeometry.height()); + xcb_icccm_set_wm_normal_hints(c.data(), w, &hints); + NETWinInfo info(c.data(), w, rootWindow(), NET::WMAllProperties, NET::WM2AllProperties); + info.setWindowType(NET::Dock); + NETExtendedStrut strut; + strut.left_start = 0; + strut.left_end = 1050; + strut.left_width = 1366+24; + strut.right_start = 0; + strut.right_end = 0; + strut.right_width = 0; + strut.top_start = 0; + strut.top_end = 0; + strut.top_width = 0; + strut.bottom_start = 0; + strut.bottom_end = 0; + strut.bottom_width = 0; + info.setExtendedStrut(strut); + xcb_map_window(c.data(), w); + xcb_flush(c.data()); + + // we should get a client for it + QSignalSpy windowCreatedSpy(workspace(), &Workspace::clientAdded); + QVERIFY(windowCreatedSpy.isValid()); + QVERIFY(windowCreatedSpy.wait()); + Client *client = windowCreatedSpy.first().first().value(); + QVERIFY(client); + QCOMPARE(client->window(), w); + QVERIFY(!client->isDecorated()); + QCOMPARE(client->windowType(), NET::Dock); + QCOMPARE(client->geometry(), windowGeometry); + + // now verify the actual updated client areas + QCOMPARE(workspace()->clientArea(PlacementArea, 0, 1), QRect(0, 282, 1366, 768)); + QCOMPARE(workspace()->clientArea(MaximizeArea, 0, 1), QRect(0, 282, 1366, 768)); + QCOMPARE(workspace()->clientArea(PlacementArea, 1, 1), QRect(1390, 0, 1656, 1050)); + QCOMPARE(workspace()->clientArea(MaximizeArea, 1, 1), QRect(1390, 0, 1656, 1050)); + QCOMPARE(workspace()->clientArea(WorkArea, 0, 1), QRect(0, 0, 3046, 1050)); + QCOMPARE(workspace()->restrictedMoveArea(-1), QRegion(1366, 0, 24, 1050)); + + // create another window and try to move it + + xcb_window_t w2 = xcb_generate_id(c.data()); + const QRect windowGeometry2(1500, 400, 200, 300); + xcb_create_window(c.data(), XCB_COPY_FROM_PARENT, w2, rootWindow(), + windowGeometry2.x(), + windowGeometry2.y(), + windowGeometry2.width(), + windowGeometry2.height(), + 0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT, 0, nullptr); + xcb_size_hints_t hints2; + memset(&hints2, 0, sizeof(hints2)); + xcb_icccm_size_hints_set_position(&hints2, 1, windowGeometry2.x(), windowGeometry2.y()); + xcb_icccm_size_hints_set_min_size(&hints2, 200, 300); + xcb_icccm_set_wm_normal_hints(c.data(), w2, &hints2); + xcb_map_window(c.data(), w2); + xcb_flush(c.data()); + QVERIFY(windowCreatedSpy.wait()); + Client *client2 = windowCreatedSpy.last().first().value(); + QVERIFY(client2); + QVERIFY(client2 != client); + QVERIFY(client2->isDecorated()); + QCOMPARE(client2->clientSize(), QSize(200, 300)); + QCOMPARE(client2->pos(), QPoint(1500, 400)); + + const QRect origGeo = client2->geometry(); + Cursor::setPos(origGeo.center()); + workspace()->performWindowOperation(client2, Options::MoveOp); + QTRY_COMPARE(workspace()->getMovingClient(), client2); + QVERIFY(client2->isMove()); + // move to next screen - step is 8 pixel, so 800 pixel + for (int i = 0; i < 100; i++) { + client2->keyPressEvent(Qt::Key_Left); + } + client2->keyPressEvent(Qt::Key_Enter); + QCOMPARE(client2->isMove(), false); + QVERIFY(workspace()->getMovingClient() == nullptr); + QCOMPARE(client2->geometry(), QRect(origGeo.translated(-800, 0))); +} + } WAYLANDTEST_MAIN(KWin::StrutsTest) diff --git a/geometry.cpp b/geometry.cpp index 89de7b968a..ccd7239f3b 100644 --- a/geometry.cpp +++ b/geometry.cpp @@ -150,6 +150,10 @@ void Workspace::updateClientArea(bool force) } } StrutRects strutRegion = (*it)->strutRects(); + const QRect clientsScreenRect = KWin::screens()->geometry((*it)->screen()); + for (auto strut = strutRegion.begin(); strut != strutRegion.end(); strut++) { + *strut = StrutRect((*strut).intersected(clientsScreenRect), (*strut).area()); + } // Ignore offscreen xinerama struts. These interfere with the larger monitors on the setup // and should be ignored so that applications that use the work area to work out where