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

@@ -419,47 +419,32 @@ void TestVirtualDesktops::updateGrid_data()
 {
     QTest::addColumn<uint>("initCount");
     QTest::addColumn<QSize>("size");
-    QTest::addColumn<Qt::Orientation>("orientation");
     QTest::addColumn<QPoint>("coords");
     QTest::addColumn<uint>("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());

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<int>{XCB_PROPERTY_NOTIFY, XCB_CLIENT_MESSAGE})
+    : X11EventFilter(QList<int>{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;
 }
 

src/virtualdesktops.cpp

@@ -141,7 +141,7 @@ VirtualDesktopGrid::VirtualDesktopGrid()
 
 VirtualDesktopGrid::~VirtualDesktopGrid() = default;
 
-void VirtualDesktopGrid::update(const QSize &size, Qt::Orientation orientation, const QList<VirtualDesktop *> &desktops)
+void VirtualDesktopGrid::update(const QSize &size, const QList<VirtualDesktop *> &desktops)
 {
     // Set private variables
     m_size = size;
@@ -151,7 +151,6 @@ 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<VirtualDesktop *> row;
         for (uint x = 0; x < width && it != end; ++x) {
@@ -160,18 +159,6 @@ void VirtualDesktopGrid::update(const QSize &size, Qt::Orientation orientation,
         }
         m_grid << row;
     }
-    } else {
-        for (uint y = 0; y < height; ++y) {
-            m_grid << QList<VirtualDesktop *>();
-        }
-        for (uint x = 0; x < width; ++x) {
-            for (uint y = 0; y < height && it != end; ++y) {
-                auto &row = m_grid[y];
-                row << *it;
-                it++;
-            }
-        }
-    }
 }
 
 QPoint VirtualDesktopGrid::gridCoords(uint id) const
@@ -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();

src/virtualdesktops.h

@@ -86,7 +86,7 @@ class VirtualDesktopGrid
 public:
     VirtualDesktopGrid();
     ~VirtualDesktopGrid();
-    void update(const QSize &size, Qt::Orientation orientation, const QList<VirtualDesktop *> &desktops);
+    void update(const QSize &size, const QList<VirtualDesktop *> &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
      */