From e5fe3137b8a1b82360aa1507b6bcee65089d93b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Thu, 2 Jun 2016 10:53:05 +0200 Subject: [PATCH] Fix the ignore struts multi-screen handling Summary: The checks in Client::adjustedClientArea were a little bit too agressive, excluding also valid setups. This change addresses the regression by keeping the actual intended improvements in place. The check in Client::adjustedClientArea is now only done for the special case of desktopArea == area. This ensures that a strut excluding a complete screen won't affect the overall workarea. In addition new checks are introduced in Workspace::updateClientArea. When calculating the new sareas a check is performed whether the intersection with the adjustedClientArea would result in the sarea becoming empty (thus a screen being completely removed). If that's the case the geometry is ignored to not exclude a complete screen. Interestingly I should have noticed that something with the logic is odd. As the test case had two commented geometries which we now get. BUG: 363804 Reviewers: #plasma, apol, lbeltrame Subscribers: plasma-devel Tags: #plasma Differential Revision: https://phabricator.kde.org/D1744 --- autotests/wayland/struts_test.cpp | 6 ++---- geometry.cpp | 32 ++++++++++++++++++------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/autotests/wayland/struts_test.cpp b/autotests/wayland/struts_test.cpp index 3fd852f513..ba599452e6 100644 --- a/autotests/wayland/struts_test.cpp +++ b/autotests/wayland/struts_test.cpp @@ -437,7 +437,7 @@ void StrutsTest::testX11Struts_data() << 0 << 1023 << 0 << 0 << 0 << 0 - << QRect(0, 0, /*1220*/1280, 1024) + << QRect(0, 0, 1220, 1024) << QRect(1280, 0, 1280, 1024) << QRect(0, 0, 2560, 1024); // second screen @@ -493,7 +493,7 @@ void StrutsTest::testX11Struts_data() << 0 << 0 << 0 << 0 << QRect(0, 0, 1280, 1024) - << QRect(1280, 0, 1280, 1024)//QRect(1340, 0, 1220, 1024) + << QRect(1340, 0, 1220, 1024) << QRect(0, 0, 2560, 1024); // invalid struts QTest::newRow("bottom panel/ invalid strut") << QRect(0, 980, 1280, 44) @@ -729,9 +729,7 @@ void StrutsTest::test363804() // now verify the actual updated client areas QCOMPARE(workspace()->clientArea(PlacementArea, 0, 1), geometries.at(0)); QCOMPARE(workspace()->clientArea(MaximizeArea, 0, 1), geometries.at(0)); - QEXPECT_FAIL("", "The actual bug", Continue); QCOMPARE(workspace()->clientArea(PlacementArea, 1, 1), QRect(554, 1080, 1366, 732)); - QEXPECT_FAIL("", "The actual bug", Continue); QCOMPARE(workspace()->clientArea(MaximizeArea, 1, 1), QRect(554, 1080, 1366, 732)); QCOMPARE(workspace()->clientArea(WorkArea, 0, 1), QRect(0, 0, 1920, 1812)); diff --git a/geometry.cpp b/geometry.cpp index aa33a8ddce..cdd4a13e3c 100644 --- a/geometry.cpp +++ b/geometry.cpp @@ -144,6 +144,15 @@ void Workspace::updateClientArea(bool force) if (!(*it)->hasStrut()) continue; QRect r = (*it)->adjustedClientArea(desktopArea, desktopArea); + // sanity check that a strut doesn't exclude a complete screen geometry + // this is a violation to EWMH, as KWin just ignores the strut + for (int i = 0; i < Screens::self()->count(); i++) { + if (!r.intersects(Screens::self()->geometry(i))) { + qCDebug(KWIN_CORE) << "Adjusted client area would exclude a complete screen, ignore"; + r = desktopArea; + break; + } + } StrutRects strutRegion = (*it)->strutRects(); // Ignore offscreen xinerama struts. These interfere with the larger monitors on the setup @@ -164,8 +173,12 @@ void Workspace::updateClientArea(bool force) for (int iS = 0; iS < nscreens; iS ++) { - new_sareas[ i ][ iS ] = new_sareas[ i ][ iS ].intersected( + const auto geo = new_sareas[ i ][ iS ].intersected( (*it)->adjustedClientArea(desktopArea, screens[ iS ])); + // ignore the geometry if it results in the screen getting removed completly + if (!geo.isEmpty()) { + new_sareas[ i ][ iS ] = geo; + } } } } else { @@ -176,9 +189,12 @@ void Workspace::updateClientArea(bool force) iS < nscreens; iS ++) { // qDebug() << "adjusting new_sarea: " << screens[ iS ]; - new_sareas[(*it)->desktop()][ iS ] - = new_sareas[(*it)->desktop()][ iS ].intersected( + const auto geo = new_sareas[(*it)->desktop()][ iS ].intersected( (*it)->adjustedClientArea(desktopArea, screens[ iS ])); + // ignore the geometry if it results in the screen getting removed completly + if (!geo.isEmpty()) { + new_sareas[(*it)->desktop()][ iS ] = geo; + } } } } @@ -952,16 +968,6 @@ QRect Client::adjustedClientArea(const QRect &desktopArea, const QRect& area) co r . setBottom(stareaB . top() - 1); } - // sanity check that a strut doesn't exclude a complete screen geometry - // this is a violation to EWMH, as KWin just ignores the strut - for (int i = 0; i < screens()->count(); i++) { - const QRect screenGeo = screens()->geometry(i); - if (!r.intersects(screenGeo)) { - qCDebug(KWIN_CORE) << "Adjusted client area would exclude a complete screen, ignore"; - return area; - } - } - return r; }