From 0a44f73b3e0976d1e06c6084f8e5ec13bde5674d Mon Sep 17 00:00:00 2001 From: David Edmundson Date: Thu, 15 Nov 2018 11:53:50 +0000 Subject: [PATCH] Add VirtualDesktops to PlasmaWindowModel Summary: VirtualDesktops with the new plurality was added PlasmaWindowManagement, but PlasmaWindowModel was left unchanged. In behavioural changes, setting a window to be on all running desktops should not mark it as being set on all desktops, they are 2 distinct pieces of information. Test Plan: Relevant unit test Reviewers: #kwin, mart Reviewed By: #kwin, mart Subscribers: kde-frameworks-devel Tags: #frameworks Differential Revision: https://phabricator.kde.org/D16883 --- .../client/test_plasma_virtual_desktop.cpp | 18 +------ .../client/test_plasma_window_model.cpp | 50 +++++++++++++------ .../plasmawindowmanagement_interface.cpp | 6 --- 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/src/wayland/autotests/client/test_plasma_virtual_desktop.cpp b/src/wayland/autotests/client/test_plasma_virtual_desktop.cpp index 5ef40936e7..8ebc6a16e9 100644 --- a/src/wayland/autotests/client/test_plasma_virtual_desktop.cpp +++ b/src/wayland/autotests/client/test_plasma_virtual_desktop.cpp @@ -466,6 +466,8 @@ void TestVirtualDesktop::testAllDesktops() virtualDesktopLeftSpy.wait(); QCOMPARE(virtualDesktopLeftSpy.count(), 2); + QCOMPARE(m_window->plasmaVirtualDesktops().length(), 0); + QVERIFY(m_window->isOnAllDesktops()); QCOMPARE(m_window->plasmaVirtualDesktops().length(), 0); QVERIFY(m_window->isOnAllDesktops()); @@ -476,22 +478,6 @@ void TestVirtualDesktop::testAllDesktops() QCOMPARE(m_window->plasmaVirtualDesktops().length(), 1); QCOMPARE(m_windowInterface->plasmaVirtualDesktops().first(), QStringLiteral("0-1")); QVERIFY(!m_window->isOnAllDesktops()); - - //try setting on virtual desktops again but by setting every desktop by hand - m_windowInterface->addPlasmaVirtualDesktop(QStringLiteral("0-3")); - virtualDesktopEnteredSpy.wait(); - - virtualDesktopEnteredSpy.clear(); - virtualDesktopLeftSpy.clear(); - - m_windowInterface->addPlasmaVirtualDesktop(QStringLiteral("0-2")); - virtualDesktopLeftSpy.wait(); - QCOMPARE(virtualDesktopLeftSpy.count(), 2); - //note that virtualDesktopEntered should *not* have been emitted for 0-2 - QCOMPARE(virtualDesktopEnteredSpy.count(), 0); - - QCOMPARE(m_window->plasmaVirtualDesktops().length(), 0); - QVERIFY(m_window->isOnAllDesktops()); } void TestVirtualDesktop::testCreateRequested() diff --git a/src/wayland/autotests/client/test_plasma_window_model.cpp b/src/wayland/autotests/client/test_plasma_window_model.cpp index cafaa171fb..1167d466f2 100644 --- a/src/wayland/autotests/client/test_plasma_window_model.cpp +++ b/src/wayland/autotests/client/test_plasma_window_model.cpp @@ -28,6 +28,7 @@ License along with this library. If not, see . // server #include "../../src/server/display.h" #include "../../src/server/plasmawindowmanagement_interface.h" +#include "../../src/server/plasmavirtualdesktop_interface.h" #include @@ -67,7 +68,6 @@ private Q_SLOTS: void testIsMinimized(); void testIsKeepAbove(); void testIsKeepBelow(); - void testIsOnAllDesktops(); void testIsDemandingAttention(); void testSkipTaskbar(); void testSkipSwitcher(); @@ -81,7 +81,7 @@ private Q_SLOTS: void testTitle(); void testAppId(); void testPid(); - void testVirtualDesktop(); + void testVirtualDesktops(); // TODO icon: can we ensure a theme is installed on CI? void testRequests(); // TODO: minimized geometry @@ -96,6 +96,7 @@ private: Display *m_display = nullptr; PlasmaWindowManagementInterface *m_pwInterface = nullptr; PlasmaWindowManagement *m_pw = nullptr; + KWayland::Server::PlasmaVirtualDesktopManagementInterface *m_plasmaVirtualDesktopManagementInterface = nullptr; ConnectionThread *m_connection = nullptr; QThread *m_thread = nullptr; EventQueue *m_queue = nullptr; @@ -113,6 +114,12 @@ void PlasmaWindowModelTest::init() m_display->createShm(); m_pwInterface = m_display->createPlasmaWindowManagement(); m_pwInterface->create(); + m_plasmaVirtualDesktopManagementInterface = m_display->createPlasmaVirtualDesktopManagement(m_display); + m_plasmaVirtualDesktopManagementInterface->create(); + QVERIFY(m_plasmaVirtualDesktopManagementInterface->isValid()); + m_plasmaVirtualDesktopManagementInterface->createDesktop("desktop1"); + m_plasmaVirtualDesktopManagementInterface->createDesktop("desktop2"); + m_pwInterface->setPlasmaVirtualDesktopManagementInterface(m_plasmaVirtualDesktopManagementInterface); // setup connection m_connection = new KWayland::Client::ConnectionThread; @@ -153,6 +160,7 @@ void PlasmaWindowModelTest::cleanup() variable = nullptr; \ } CLEANUP(m_pw) + CLEANUP(m_plasmaVirtualDesktopManagementInterface) CLEANUP(m_queue) if (m_connection) { m_connection->deleteLater(); @@ -401,11 +409,6 @@ void PlasmaWindowModelTest::testIsKeepBelow() QVERIFY(testBooleanData(PlasmaWindowModel::IsKeepBelow, &PlasmaWindowInterface::setKeepBelow)); } -void PlasmaWindowModelTest::testIsOnAllDesktops() -{ - QVERIFY(testBooleanData(PlasmaWindowModel::IsOnAllDesktops, &PlasmaWindowInterface::setOnAllDesktops)); -} - void PlasmaWindowModelTest::testIsDemandingAttention() { QVERIFY(testBooleanData(PlasmaWindowModel::IsDemandingAttention, &PlasmaWindowInterface::setDemandsAttention)); @@ -549,7 +552,7 @@ void PlasmaWindowModelTest::testPid() QCOMPARE(model->data(index, PlasmaWindowModel::Pid).toInt(), 1337); } -void PlasmaWindowModelTest::testVirtualDesktop() +void PlasmaWindowModelTest::testVirtualDesktops() { auto model = m_pw->createWindowModel(); QVERIFY(model); @@ -564,17 +567,36 @@ void PlasmaWindowModelTest::testVirtualDesktop() QVERIFY(dataChangedSpy.isValid()); const QModelIndex index = model->index(0); - QCOMPARE(model->data(index, PlasmaWindowModel::VirtualDesktop).toInt(), 0); + QCOMPARE(model->data(index, PlasmaWindowModel::VirtualDesktops).toStringList(), QStringList()); - w->setVirtualDesktop(1); + w->addPlasmaVirtualDesktop("desktop1"); + QVERIFY(dataChangedSpy.wait()); + QCOMPARE(dataChangedSpy.count(), 2); + QCOMPARE(dataChangedSpy.first().first().toModelIndex(), index); + QCOMPARE(dataChangedSpy.last().first().toModelIndex(), index); + + QCOMPARE(dataChangedSpy.first().last().value>(), QVector{int(PlasmaWindowModel::VirtualDesktops)}); + QCOMPARE(dataChangedSpy.last().last().value>(), QVector{int(PlasmaWindowModel::IsOnAllDesktops)}); + + QCOMPARE(model->data(index, PlasmaWindowModel::VirtualDesktops).toStringList(), QStringList({"desktop1"})); + QCOMPARE(model->data(index, PlasmaWindowModel::IsOnAllDesktops).toBool(), false); + + dataChangedSpy.clear(); + w->addPlasmaVirtualDesktop("desktop2"); QVERIFY(dataChangedSpy.wait()); QCOMPARE(dataChangedSpy.count(), 1); QCOMPARE(dataChangedSpy.last().first().toModelIndex(), index); - QCOMPARE(dataChangedSpy.last().last().value>(), QVector{int(PlasmaWindowModel::VirtualDesktop)}); - QCOMPARE(model->data(index, PlasmaWindowModel::VirtualDesktop).toInt(), 1); + QCOMPARE(dataChangedSpy.last().last().value>(), QVector{int(PlasmaWindowModel::VirtualDesktops)}); + QCOMPARE(model->data(index, PlasmaWindowModel::VirtualDesktops).toStringList(), QStringList({"desktop1", "desktop2"})); + QCOMPARE(model->data(index, PlasmaWindowModel::IsOnAllDesktops).toBool(), false); + + w->removePlasmaVirtualDesktop("desktop2"); + w->removePlasmaVirtualDesktop("desktop1"); + QVERIFY(dataChangedSpy.wait()); + QCOMPARE(dataChangedSpy.last().last().value>(), QVector{int(PlasmaWindowModel::IsOnAllDesktops)}); + QCOMPARE(model->data(index, PlasmaWindowModel::VirtualDesktops).toStringList(), QStringList({})); + QCOMPARE(model->data(index, PlasmaWindowModel::IsOnAllDesktops).toBool(), true); - // setting to same should not trigger - w->setVirtualDesktop(1); QVERIFY(!dataChangedSpy.wait(100)); } diff --git a/src/wayland/plasmawindowmanagement_interface.cpp b/src/wayland/plasmawindowmanagement_interface.cpp index b1e583bdfd..d7f665d20d 100644 --- a/src/wayland/plasmawindowmanagement_interface.cpp +++ b/src/wayland/plasmawindowmanagement_interface.cpp @@ -882,12 +882,6 @@ void PlasmaWindowInterface::addPlasmaVirtualDesktop(const QString &id) return; } - //full? lets set it on all desktops, the plasmaVirtualDesktops list will get empty, which means it's on all desktops - if (d->wm->plasmaVirtualDesktopManagementInterface()->desktops().count() == d->plasmaVirtualDesktops.count() + 1) { - setOnAllDesktops(true); - return; - } - d->plasmaVirtualDesktops << id; //if the desktop dies, remove it from or list