From b53621136ddc6fe596f3fd3bbaab445537e771a2 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 23 Feb 2023 21:01:12 +0200 Subject: [PATCH] Drop Window::desktopPresenceChanged() This signal exists as a convenience helper, but it's not always emitted as it's advertised to work. Instead of fixing it, let's drop the signal to simplify virtual desktop code. Its effects can be accomplished by monitoring Window::desktopChanged() and VirtualDesktopManager::currentChanged() signals in effects and scripts where needed. --- .../integration/virtual_desktop_test.cpp | 5 --- autotests/integration/xdgshellwindow_test.cpp | 31 ------------------- src/effects.cpp | 9 ++---- .../package/contents/code/main.js | 4 +-- .../frozenapp/package/contents/code/main.js | 4 +-- .../package/contents/code/main.js | 4 +-- src/libkwineffects/kwineffects.h | 14 +++------ src/scripting/workspace_wrapper.cpp | 1 - src/scripting/workspace_wrapper.h | 1 - src/window.cpp | 10 ++---- src/window.h | 1 - src/workspace.cpp | 1 - src/workspace.h | 1 - 13 files changed, 16 insertions(+), 70 deletions(-) diff --git a/autotests/integration/virtual_desktop_test.cpp b/autotests/integration/virtual_desktop_test.cpp index ca9497fe7d..fcdda3c958 100644 --- a/autotests/integration/virtual_desktop_test.cpp +++ b/autotests/integration/virtual_desktop_test.cpp @@ -131,8 +131,6 @@ void VirtualDesktopTest::testLastDesktopRemoved() QVERIFY(window); QCOMPARE(window->desktop(), 2); - QSignalSpy desktopPresenceChangedSpy(window, &Window::desktopPresenceChanged); - QCOMPARE(window->desktops().count(), 1u); QCOMPARE(VirtualDesktopManager::self()->currentDesktop(), window->desktops().first()); @@ -140,7 +138,6 @@ void VirtualDesktopTest::testLastDesktopRemoved() VirtualDesktopManager::self()->setCount(1); QCOMPARE(VirtualDesktopManager::self()->count(), 1u); // now the window should be moved as well - QTRY_COMPARE(desktopPresenceChangedSpy.count(), 1); QCOMPARE(window->desktop(), 1); QCOMPARE(window->desktops().count(), 1u); @@ -165,7 +162,6 @@ void VirtualDesktopTest::testWindowOnMultipleDesktops() QVERIFY(window); QCOMPARE(window->desktop(), 3u); - QSignalSpy desktopPresenceChangedSpy(window, &Window::desktopPresenceChanged); QCOMPARE(window->desktops().count(), 1u); QCOMPARE(VirtualDesktopManager::self()->currentDesktop(), window->desktops().first()); @@ -243,7 +239,6 @@ void VirtualDesktopTest::testRemoveDesktopWithWindow() QVERIFY(window); QCOMPARE(window->desktop(), 3u); - QSignalSpy desktopPresenceChangedSpy(window, &Window::desktopPresenceChanged); QCOMPARE(window->desktops().count(), 1u); QCOMPARE(VirtualDesktopManager::self()->currentDesktop(), window->desktops().first()); diff --git a/autotests/integration/xdgshellwindow_test.cpp b/autotests/integration/xdgshellwindow_test.cpp index acb8db2283..0c3295616f 100644 --- a/autotests/integration/xdgshellwindow_test.cpp +++ b/autotests/integration/xdgshellwindow_test.cpp @@ -59,7 +59,6 @@ private Q_SLOTS: void cleanup(); void testMapUnmap(); - void testDesktopPresenceChanged(); void testWindowOutputs(); void testMinimizeActiveWindow(); void testFullscreen_data(); @@ -253,36 +252,6 @@ void TestXdgShellWindow::testMapUnmap() QVERIFY(Test::waitForWindowDestroyed(window)); } -void TestXdgShellWindow::testDesktopPresenceChanged() -{ - // this test verifies that the desktop presence changed signals are properly emitted - std::unique_ptr surface(Test::createSurface()); - std::unique_ptr shellSurface(Test::createXdgToplevelSurface(surface.get())); - auto window = Test::renderAndWaitForShown(surface.get(), QSize(100, 50), Qt::blue); - QVERIFY(window); - QCOMPARE(window->desktop(), 1); - effects->setNumberOfDesktops(4); - QSignalSpy desktopPresenceChangedClientSpy(window, &Window::desktopPresenceChanged); - QSignalSpy desktopPresenceChangedWorkspaceSpy(workspace(), &Workspace::desktopPresenceChanged); - QSignalSpy desktopPresenceChangedEffectsSpy(effects, &EffectsHandler::desktopPresenceChanged); - - // let's change the desktop - workspace()->sendWindowToDesktop(window, 2, false); - QCOMPARE(window->desktop(), 2); - QCOMPARE(desktopPresenceChangedClientSpy.count(), 1); - QCOMPARE(desktopPresenceChangedWorkspaceSpy.count(), 1); - QCOMPARE(desktopPresenceChangedEffectsSpy.count(), 1); - - // verify the arguments - QCOMPARE(desktopPresenceChangedClientSpy.first().at(0).value(), window); - QCOMPARE(desktopPresenceChangedClientSpy.first().at(1).toInt(), 1); - QCOMPARE(desktopPresenceChangedWorkspaceSpy.first().at(0).value(), window); - QCOMPARE(desktopPresenceChangedWorkspaceSpy.first().at(1).toInt(), 1); - QCOMPARE(desktopPresenceChangedEffectsSpy.first().at(0).value(), window->effectWindow()); - QCOMPARE(desktopPresenceChangedEffectsSpy.first().at(1).toInt(), 1); - QCOMPARE(desktopPresenceChangedEffectsSpy.first().at(2).toInt(), 2); -} - void TestXdgShellWindow::testWindowOutputs() { std::unique_ptr surface(Test::createSurface()); diff --git a/src/effects.cpp b/src/effects.cpp index 40c44aea83..c686be2309 100644 --- a/src/effects.cpp +++ b/src/effects.cpp @@ -158,12 +158,6 @@ EffectsHandlerImpl::EffectsHandlerImpl(Compositor *compositor, WorkspaceScene *s connect(ws, &Workspace::currentDesktopChangingCancelled, this, [this]() { Q_EMIT desktopChangingCancelled(); }); - connect(ws, &Workspace::desktopPresenceChanged, this, [this](Window *window, int old) { - if (!window->effectWindow()) { - return; - } - Q_EMIT desktopPresenceChanged(window->effectWindow(), old, window->desktop()); - }); connect(ws, &Workspace::windowAdded, this, [this](Window *window) { if (window->readyForPainting()) { slotWindowShown(window); @@ -354,6 +348,9 @@ void EffectsHandlerImpl::setupWindowConnections(Window *window) connect(window, &Window::decorationChanged, this, [this, window]() { Q_EMIT windowDecorationChanged(window->effectWindow()); }); + connect(window, &Window::desktopChanged, this, [this, window]() { + Q_EMIT windowDesktopsChanged(window->effectWindow()); + }); } void EffectsHandlerImpl::setupUnmanagedConnections(Unmanaged *u) diff --git a/src/effects/dialogparent/package/contents/code/main.js b/src/effects/dialogparent/package/contents/code/main.js index 125d390f94..cffc0afc0b 100644 --- a/src/effects/dialogparent/package/contents/code/main.js +++ b/src/effects/dialogparent/package/contents/code/main.js @@ -138,8 +138,8 @@ var dialogParentEffect = { effects.windowUnminimized.connect(dialogParentEffect.restartAnimation); effects.windowModalityChanged.connect(dialogParentEffect.modalDialogChanged) effects.desktopChanged.connect(dialogParentEffect.desktopChanged); - effects.desktopPresenceChanged.connect(dialogParentEffect.cancelAnimationInstant); - effects.desktopPresenceChanged.connect(dialogParentEffect.restartAnimation); + effects.windowDesktopsChanged.connect(dialogParentEffect.cancelAnimationInstant); + effects.windowDesktopsChanged.connect(dialogParentEffect.restartAnimation); effects.activeFullScreenEffectChanged.connect( dialogParentEffect.activeFullScreenEffectChanged); diff --git a/src/effects/frozenapp/package/contents/code/main.js b/src/effects/frozenapp/package/contents/code/main.js index 102160d865..dc12e03431 100644 --- a/src/effects/frozenapp/package/contents/code/main.js +++ b/src/effects/frozenapp/package/contents/code/main.js @@ -106,8 +106,8 @@ var frozenAppEffect = { effects.windowUnminimized.connect(frozenAppEffect.restartAnimation); effects.windowUnresponsiveChanged.connect(frozenAppEffect.unresponsiveChanged); effects.desktopChanged.connect(frozenAppEffect.desktopChanged); - effects.desktopPresenceChanged.connect(frozenAppEffect.cancelAnimation); - effects.desktopPresenceChanged.connect(frozenAppEffect.restartAnimation); + effects.windowDesktopsChanged.connect(frozenAppEffect.cancelAnimation); + effects.windowDesktopsChanged.connect(frozenAppEffect.restartAnimation); var windows = effects.stackingOrder; for (var i = 0, length = windows.length; i < length; ++i) { diff --git a/src/effects/translucency/package/contents/code/main.js b/src/effects/translucency/package/contents/code/main.js index 8bfc0d9276..bc7007c020 100644 --- a/src/effects/translucency/package/contents/code/main.js +++ b/src/effects/translucency/package/contents/code/main.js @@ -206,8 +206,8 @@ var translucencyEffect = { }, init: function () { effect.configChanged.connect(translucencyEffect.loadConfig); - effects.desktopPresenceChanged.connect(translucencyEffect.cancelAnimations); - effects.desktopPresenceChanged.connect(translucencyEffect.startAnimation); + effects.windowDesktopsChanged.connect(translucencyEffect.cancelAnimations); + effects.windowDesktopsChanged.connect(translucencyEffect.startAnimation); effects.windowAdded.connect(translucencyEffect.startAnimation); effects.windowUnminimized.connect(translucencyEffect.startAnimation); effects.windowClosed.connect(translucencyEffect.cancelAnimations); diff --git a/src/libkwineffects/kwineffects.h b/src/libkwineffects/kwineffects.h index 76c673533c..c5b97cd89d 100644 --- a/src/libkwineffects/kwineffects.h +++ b/src/libkwineffects/kwineffects.h @@ -1459,15 +1459,6 @@ Q_SIGNALS: void desktopChanging(uint currentDesktop, QPointF offset, KWin::EffectWindow *with); void desktopChangingCancelled(); - /** - * Signal emitted when a window moved to another desktop - * NOTICE that this does NOT imply that the desktop has changed - * The @param window which is moved to the new desktop - * @param oldDesktop The previous desktop of the window - * @param newDesktop The new desktop of the window - * @since 4.11.4 - */ - void desktopPresenceChanged(KWin::EffectWindow *window, int oldDesktop, int newDesktop); /** * Emitted when the virtual desktop grid layout changes * @param size new size @@ -1934,6 +1925,11 @@ Q_SIGNALS: void inputPanelChanged(); + /** + * This signal is emitted when a window enters or leaves a virtual desktop. + */ + void windowDesktopsChanged(KWin::EffectWindow *window); + protected: QVector loaded_effects; // QHash< QString, EffectFactory* > effect_factories; diff --git a/src/scripting/workspace_wrapper.cpp b/src/scripting/workspace_wrapper.cpp index b892a86e5b..4362d831cf 100644 --- a/src/scripting/workspace_wrapper.cpp +++ b/src/scripting/workspace_wrapper.cpp @@ -31,7 +31,6 @@ WorkspaceWrapper::WorkspaceWrapper(QObject *parent) { KWin::Workspace *ws = KWin::Workspace::self(); KWin::VirtualDesktopManager *vds = KWin::VirtualDesktopManager::self(); - connect(ws, &Workspace::desktopPresenceChanged, this, &WorkspaceWrapper::desktopPresenceChanged); connect(ws, &Workspace::windowAdded, this, &WorkspaceWrapper::clientAdded); connect(ws, &Workspace::windowRemoved, this, &WorkspaceWrapper::clientRemoved); connect(ws, &Workspace::windowActivated, this, &WorkspaceWrapper::clientActivated); diff --git a/src/scripting/workspace_wrapper.h b/src/scripting/workspace_wrapper.h index a1f3334b76..20aa030686 100644 --- a/src/scripting/workspace_wrapper.h +++ b/src/scripting/workspace_wrapper.h @@ -63,7 +63,6 @@ private: Q_DISABLE_COPY(WorkspaceWrapper) Q_SIGNALS: - void desktopPresenceChanged(KWin::Window *client, int desktop); void clientAdded(KWin::Window *client); void clientRemoved(KWin::Window *client); void clientActivated(KWin::Window *client); diff --git a/src/window.cpp b/src/window.cpp index 7485f88a93..8a307763ab 100644 --- a/src/window.cpp +++ b/src/window.cpp @@ -1060,9 +1060,7 @@ void Window::setDesktops(QVector desktops) return; } - int was_desk = Window::desktop(); - const bool wasOnCurrentDesktop = isOnCurrentDesktop() && was_desk >= 0; - + const bool wasOnAllDesktops = isOnAllDesktops(); m_desktops = desktops; if (windowManagementInterface()) { @@ -1087,8 +1085,7 @@ void Window::setDesktops(QVector desktops) info->setDesktop(desktop()); } - if ((was_desk == NET::OnAllDesktops) != (desktop() == NET::OnAllDesktops)) { - // onAllDesktops changed + if (isOnAllDesktops() != wasOnAllDesktops) { workspace()->updateOnAllDesktopsOfTransients(this); } @@ -1113,9 +1110,6 @@ void Window::setDesktops(QVector desktops) updateWindowRules(Rules::Desktops); Q_EMIT desktopChanged(); - if (wasOnCurrentDesktop != isOnCurrentDesktop()) { - Q_EMIT desktopPresenceChanged(this, was_desk); - } } void Window::doSetDesktop() diff --git a/src/window.h b/src/window.h index f047b345e6..1f2e7c8aaf 100644 --- a/src/window.h +++ b/src/window.h @@ -1494,7 +1494,6 @@ Q_SIGNALS: * Emitted whenever the demands attention state changes. */ void demandsAttentionChanged(); - void desktopPresenceChanged(KWin::Window *, int); // to be forwarded by Workspace void desktopChanged(); void activitiesChanged(KWin::Window *window); void minimizedChanged(); diff --git a/src/workspace.cpp b/src/workspace.cpp index c317ac3732..523d65be89 100644 --- a/src/workspace.cpp +++ b/src/workspace.cpp @@ -770,7 +770,6 @@ void Workspace::updateOutputConfiguration() void Workspace::setupWindowConnections(Window *window) { - connect(window, &Window::desktopPresenceChanged, this, &Workspace::desktopPresenceChanged); connect(window, &Window::minimizedChanged, this, std::bind(&Workspace::windowMinimizedChanged, this, window)); connect(window, &Window::fullScreenChanged, m_screenEdges.get(), &ScreenEdges::checkBlocking); } diff --git a/src/workspace.h b/src/workspace.h index 5fb0cd651d..3f70c60fac 100644 --- a/src/workspace.h +++ b/src/workspace.h @@ -573,7 +573,6 @@ Q_SIGNALS: void geometryChanged(); // Signals required for the scripting interface - void desktopPresenceChanged(KWin::Window *, int); void currentActivityChanged(); void currentDesktopChanged(int, KWin::Window *); void currentDesktopChanging(uint currentDesktop, QPointF delta, KWin::Window *); // for realtime animations