From def3a5055899b1a9000a18060575fefcc7173c1a Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Tue, 30 Jan 2024 11:57:08 +0200 Subject: [PATCH] Ignore external updates of _NET_DESKTOP_LAYOUT and _NET_DESKTOP_NAMES At the moment, the desktop layout in _NET_DESKTOP_LAYOUT overwrites new desktop layout with outdated information. This happens because kwin tries to honor the desktop layout set by the pager. However, kwin itself already acts as the pager. The pager applet in plasma doesn't attempt to maintain _NET_DESKTOP_LAYOUT with proper values. On the other hand, kwin trying to both update and also sync its state to _NET_DESKTOP_LAYOUT and _NET_DESKTOP_NAMES has created a series of issues, like lockups or rendering glitches. Given that the window manager can ignore these properties, and the fact that kwin already does act like a pager, this patch makes kwin ignore external updates to _NET_DESKTOP_LAYOUT and _NET_DESKTOP_NAMES. In order to modify the desktop layout on X11, use the dbus api. On Wayland, either the dbus api or the virtual desktop wayland protocol. BUG: 422319 BUG: 480371 --- autotests/test_virtual_desktops.cpp | 56 +++++++------------ src/rootinfo_filter.cpp | 9 +--- src/virtualdesktops.cpp | 83 +++++------------------------ src/virtualdesktops.h | 6 +-- 4 files changed, 36 insertions(+), 118 deletions(-) diff --git a/autotests/test_virtual_desktops.cpp b/autotests/test_virtual_desktops.cpp index c931d926d6..b463985525 100644 --- a/autotests/test_virtual_desktops.cpp +++ b/autotests/test_virtual_desktops.cpp @@ -419,47 +419,32 @@ void TestVirtualDesktops::updateGrid_data() { QTest::addColumn("initCount"); QTest::addColumn("size"); - QTest::addColumn("orientation"); QTest::addColumn("coords"); QTest::addColumn("desktop"); - const Qt::Orientation h = Qt::Horizontal; - const Qt::Orientation v = Qt::Vertical; - QTest::newRow("one desktop, h") << (uint)1 << QSize(1, 1) << h << QPoint(0, 0) << (uint)1; - QTest::newRow("one desktop, v") << (uint)1 << QSize(1, 1) << v << QPoint(0, 0) << (uint)1; - QTest::newRow("one desktop, h, 0") << (uint)1 << QSize(1, 1) << h << QPoint(1, 0) << (uint)0; - QTest::newRow("one desktop, v, 0") << (uint)1 << QSize(1, 1) << v << QPoint(0, 1) << (uint)0; + QTest::newRow("one desktop, h") << (uint)1 << QSize(1, 1) << QPoint(0, 0) << (uint)1; + QTest::newRow("one desktop, h, 0") << (uint)1 << QSize(1, 1) << QPoint(1, 0) << (uint)0; - QTest::newRow("two desktops, h, 1") << (uint)2 << QSize(2, 1) << h << QPoint(0, 0) << (uint)1; - QTest::newRow("two desktops, h, 2") << (uint)2 << QSize(2, 1) << h << QPoint(1, 0) << (uint)2; - QTest::newRow("two desktops, h, 3") << (uint)2 << QSize(2, 1) << h << QPoint(0, 1) << (uint)0; - QTest::newRow("two desktops, h, 4") << (uint)2 << QSize(2, 1) << h << QPoint(2, 0) << (uint)0; + QTest::newRow("two desktops, h, 1") << (uint)2 << QSize(2, 1) << QPoint(0, 0) << (uint)1; + QTest::newRow("two desktops, h, 2") << (uint)2 << QSize(2, 1) << QPoint(1, 0) << (uint)2; + QTest::newRow("two desktops, h, 3") << (uint)2 << QSize(2, 1) << QPoint(0, 1) << (uint)0; + QTest::newRow("two desktops, h, 4") << (uint)2 << QSize(2, 1) << QPoint(2, 0) << (uint)0; - QTest::newRow("two desktops, v, 1") << (uint)2 << QSize(2, 1) << v << QPoint(0, 0) << (uint)1; - QTest::newRow("two desktops, v, 2") << (uint)2 << QSize(2, 1) << v << QPoint(1, 0) << (uint)2; - QTest::newRow("two desktops, v, 3") << (uint)2 << QSize(2, 1) << v << QPoint(0, 1) << (uint)0; - QTest::newRow("two desktops, v, 4") << (uint)2 << QSize(2, 1) << v << QPoint(2, 0) << (uint)0; + QTest::newRow("four desktops, h, one row, 1") << (uint)4 << QSize(4, 1) << QPoint(0, 0) << (uint)1; + QTest::newRow("four desktops, h, one row, 2") << (uint)4 << QSize(4, 1) << QPoint(1, 0) << (uint)2; + QTest::newRow("four desktops, h, one row, 3") << (uint)4 << QSize(4, 1) << QPoint(2, 0) << (uint)3; + QTest::newRow("four desktops, h, one row, 4") << (uint)4 << QSize(4, 1) << QPoint(3, 0) << (uint)4; - QTest::newRow("four desktops, h, one row, 1") << (uint)4 << QSize(4, 1) << h << QPoint(0, 0) << (uint)1; - QTest::newRow("four desktops, h, one row, 2") << (uint)4 << QSize(4, 1) << h << QPoint(1, 0) << (uint)2; - QTest::newRow("four desktops, h, one row, 3") << (uint)4 << QSize(4, 1) << h << QPoint(2, 0) << (uint)3; - QTest::newRow("four desktops, h, one row, 4") << (uint)4 << QSize(4, 1) << h << QPoint(3, 0) << (uint)4; + QTest::newRow("four desktops, h, grid, 1") << (uint)4 << QSize(2, 2) << QPoint(0, 0) << (uint)1; + QTest::newRow("four desktops, h, grid, 2") << (uint)4 << QSize(2, 2) << QPoint(1, 0) << (uint)2; + QTest::newRow("four desktops, h, grid, 3") << (uint)4 << QSize(2, 2) << QPoint(0, 1) << (uint)3; + QTest::newRow("four desktops, h, grid, 4") << (uint)4 << QSize(2, 2) << QPoint(1, 1) << (uint)4; + QTest::newRow("four desktops, h, grid, 0/3") << (uint)4 << QSize(2, 2) << QPoint(0, 3) << (uint)0; - QTest::newRow("four desktops, v, one column, 1") << (uint)4 << QSize(1, 4) << v << QPoint(0, 0) << (uint)1; - QTest::newRow("four desktops, v, one column, 2") << (uint)4 << QSize(1, 4) << v << QPoint(0, 1) << (uint)2; - QTest::newRow("four desktops, v, one column, 3") << (uint)4 << QSize(1, 4) << v << QPoint(0, 2) << (uint)3; - QTest::newRow("four desktops, v, one column, 4") << (uint)4 << QSize(1, 4) << v << QPoint(0, 3) << (uint)4; - - QTest::newRow("four desktops, h, grid, 1") << (uint)4 << QSize(2, 2) << h << QPoint(0, 0) << (uint)1; - QTest::newRow("four desktops, h, grid, 2") << (uint)4 << QSize(2, 2) << h << QPoint(1, 0) << (uint)2; - QTest::newRow("four desktops, h, grid, 3") << (uint)4 << QSize(2, 2) << h << QPoint(0, 1) << (uint)3; - QTest::newRow("four desktops, h, grid, 4") << (uint)4 << QSize(2, 2) << h << QPoint(1, 1) << (uint)4; - QTest::newRow("four desktops, h, grid, 0/3") << (uint)4 << QSize(2, 2) << h << QPoint(0, 3) << (uint)0; - - QTest::newRow("three desktops, h, grid, 1") << (uint)3 << QSize(2, 2) << h << QPoint(0, 0) << (uint)1; - QTest::newRow("three desktops, h, grid, 2") << (uint)3 << QSize(2, 2) << h << QPoint(1, 0) << (uint)2; - QTest::newRow("three desktops, h, grid, 3") << (uint)3 << QSize(2, 2) << h << QPoint(0, 1) << (uint)3; - QTest::newRow("three desktops, h, grid, 4") << (uint)3 << QSize(2, 2) << h << QPoint(1, 1) << (uint)0; + QTest::newRow("three desktops, h, grid, 1") << (uint)3 << QSize(2, 2) << QPoint(0, 0) << (uint)1; + QTest::newRow("three desktops, h, grid, 2") << (uint)3 << QSize(2, 2) << QPoint(1, 0) << (uint)2; + QTest::newRow("three desktops, h, grid, 3") << (uint)3 << QSize(2, 2) << QPoint(0, 1) << (uint)3; + QTest::newRow("three desktops, h, grid, 4") << (uint)3 << QSize(2, 2) << QPoint(1, 1) << (uint)0; } void TestVirtualDesktops::updateGrid() @@ -470,9 +455,8 @@ void TestVirtualDesktops::updateGrid() VirtualDesktopGrid grid; QFETCH(QSize, size); - QFETCH(Qt::Orientation, orientation); QCOMPARE(vds->desktops().count(), int(initCount)); - grid.update(size, orientation, vds->desktops()); + grid.update(size, vds->desktops()); QCOMPARE(grid.size(), size); QCOMPARE(grid.width(), size.width()); QCOMPARE(grid.height(), size.height()); diff --git a/src/rootinfo_filter.cpp b/src/rootinfo_filter.cpp index e269b5d828..42b6383e94 100644 --- a/src/rootinfo_filter.cpp +++ b/src/rootinfo_filter.cpp @@ -8,13 +8,12 @@ */ #include "rootinfo_filter.h" #include "netinfo.h" -#include "virtualdesktops.h" namespace KWin { RootInfoFilter::RootInfoFilter(RootInfo *parent) - : X11EventFilter(QList{XCB_PROPERTY_NOTIFY, XCB_CLIENT_MESSAGE}) + : X11EventFilter(QList{XCB_CLIENT_MESSAGE}) , m_rootInfo(parent) { } @@ -24,12 +23,6 @@ bool RootInfoFilter::event(xcb_generic_event_t *event) NET::Properties dirtyProtocols; NET::Properties2 dirtyProtocols2; m_rootInfo->event(event, &dirtyProtocols, &dirtyProtocols2); - if (dirtyProtocols & NET::DesktopNames) { - VirtualDesktopManager::self()->save(); - } - if (dirtyProtocols2 & NET::WM2DesktopLayout) { - VirtualDesktopManager::self()->updateLayout(); - } return false; } diff --git a/src/virtualdesktops.cpp b/src/virtualdesktops.cpp index 7b42f80eaf..540c3eddbb 100644 --- a/src/virtualdesktops.cpp +++ b/src/virtualdesktops.cpp @@ -141,7 +141,7 @@ VirtualDesktopGrid::VirtualDesktopGrid() VirtualDesktopGrid::~VirtualDesktopGrid() = default; -void VirtualDesktopGrid::update(const QSize &size, Qt::Orientation orientation, const QList &desktops) +void VirtualDesktopGrid::update(const QSize &size, const QList &desktops) { // Set private variables m_size = size; @@ -151,26 +151,13 @@ void VirtualDesktopGrid::update(const QSize &size, Qt::Orientation orientation, m_grid.clear(); auto it = desktops.begin(); auto end = desktops.end(); - if (orientation == Qt::Horizontal) { - for (uint y = 0; y < height; ++y) { - QList row; - for (uint x = 0; x < width && it != end; ++x) { - row << *it; - it++; - } - m_grid << row; - } - } else { - for (uint y = 0; y < height; ++y) { - m_grid << QList(); - } - for (uint x = 0; x < width; ++x) { - for (uint y = 0; y < height && it != end; ++y) { - auto &row = m_grid[y]; - row << *it; - it++; - } + for (uint y = 0; y < height; ++y) { + QList row; + for (uint x = 0; x < width && it != end; ++x) { + row << *it; + it++; } + m_grid << row; } } @@ -650,30 +637,17 @@ void VirtualDesktopManager::updateRootInfo() void VirtualDesktopManager::updateLayout() { m_rows = std::min(m_rows, count()); + int columns = count() / m_rows; - Qt::Orientation orientation = Qt::Horizontal; - if (m_rootInfo) { - // TODO: Is there a sane way to avoid overriding the existing grid? - columns = m_rootInfo->desktopLayoutColumnsRows().width(); - m_rows = std::max(1, m_rootInfo->desktopLayoutColumnsRows().height()); - orientation = m_rootInfo->desktopLayoutOrientation() == NET::OrientationHorizontal ? Qt::Horizontal : Qt::Vertical; + if (count() % m_rows > 0) { + columns++; } - if (columns == 0) { - // Not given, set default layout - m_rows = count() == 1u ? 1 : 2; - columns = count() / m_rows; - } + m_grid.update(QSize(columns, m_rows), m_desktops); + // TODO: why is there no call to m_rootInfo->setDesktopLayout? - // Patch to make desktop grid size equal 1 when 1 desktop for desktop switching animations - if (m_desktops.size() == 1) { - m_rows = 1; - columns = 1; - } - - setNETDesktopLayout(orientation, - columns, m_rows, 0 // rootInfo->desktopLayoutCorner() // Not really worth implementing right now. - ); + Q_EMIT layoutChanged(columns, m_rows); + Q_EMIT rowsChanged(m_rows); } void VirtualDesktopManager::load() @@ -762,35 +736,6 @@ QString VirtualDesktopManager::defaultName(int desktop) const return i18n("Desktop %1", desktop); } -void VirtualDesktopManager::setNETDesktopLayout(Qt::Orientation orientation, uint width, uint height, int startingCorner) -{ - // startingCorner is not really worth implementing right now. - - const uint count = m_desktops.count(); - - // Calculate valid grid size - Q_ASSERT(width > 0 || height > 0); - if ((width <= 0) && (height > 0)) { - width = (count + height - 1) / height; - } else if ((height <= 0) && (width > 0)) { - height = (count + width - 1) / width; - } - while (width * height < count) { - if (orientation == Qt::Horizontal) { - ++width; - } else { - ++height; - } - } - - m_rows = std::max(1u, height); - - m_grid.update(QSize(width, height), orientation, m_desktops); - // TODO: why is there no call to m_rootInfo->setDesktopLayout? - Q_EMIT layoutChanged(width, height); - Q_EMIT rowsChanged(height); -} - void VirtualDesktopManager::initShortcuts() { initSwitchToShortcuts(); diff --git a/src/virtualdesktops.h b/src/virtualdesktops.h index 69ac39f25c..c63d94fe78 100644 --- a/src/virtualdesktops.h +++ b/src/virtualdesktops.h @@ -86,7 +86,7 @@ class VirtualDesktopGrid public: VirtualDesktopGrid(); ~VirtualDesktopGrid(); - void update(const QSize &size, Qt::Orientation orientation, const QList &desktops); + void update(const QSize &size, const QList &desktops); /** * @returns The coords of desktop @a id in grid units. */ @@ -446,10 +446,6 @@ private Q_SLOTS: void gestureReleasedX(); private: - /** - * Generate a desktop layout from EWMH _NET_DESKTOP_LAYOUT property parameters. - */ - void setNETDesktopLayout(Qt::Orientation orientation, uint width, uint height, int startingCorner); /** * @returns A default name for the given @p desktop */