From 57f8c6d5f88cfb05945d8a2837ed0cec3218e2f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Fri, 28 Aug 2015 07:50:20 +0200 Subject: [PATCH 1/4] recreate presentwindows grids from desktopgrid Theory: ---------- because PresentWindowsEffect::screenCountChanged() is shortcut for "if (!isActive())", but the desktopgrid doesn't call PresentWindowsEffect::setActive (or at least PresentWindowsEffect::screenCountChanged), so the effect can "miss" the increasing screen count change (it sees the signal, but ignores it) and when desktopgrid calls it, it assumes the m_gridSizes array is big enough (but it isn't) Steps: ---------- 1. effects are loaded, 1 screen present 2. 2nd screen gets added, but inactive effects ignore that 3. desktop grid gets activated, updates according to screen count, calls presentwindows for screen #2 4. presentwindows data is only prepared for one screen from step 1 => BOOM BUG: 351724 CCBUG: 326032 FIXED-IN: 5.4.2 REVIEW: 124960 --- effects/desktopgrid/desktopgrid.cpp | 1 + effects/presentwindows/presentwindows.cpp | 13 ++++++++----- effects/presentwindows/presentwindows.h | 2 +- effects/presentwindows/presentwindows_proxy.cpp | 5 +++++ effects/presentwindows/presentwindows_proxy.h | 2 ++ 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/effects/desktopgrid/desktopgrid.cpp b/effects/desktopgrid/desktopgrid.cpp index 50b2cce7a1..396ce64857 100644 --- a/effects/desktopgrid/desktopgrid.cpp +++ b/effects/desktopgrid/desktopgrid.cpp @@ -1098,6 +1098,7 @@ void DesktopGridEffect::setup() if (m_usePresentWindows) m_proxy = static_cast(effects->getProxy(BuiltInEffects::nameForEffect(BuiltInEffect::PresentWindows))); if (isUsingPresentWindows()) { + m_proxy->reCreateGrids(); // revalidation on multiscreen, bug #351724 for (int i = 1; i <= effects->numberOfDesktops(); i++) { for (int j = 0; j < effects->numScreens(); j++) { WindowMotionManager manager; diff --git a/effects/presentwindows/presentwindows.cpp b/effects/presentwindows/presentwindows.cpp index be62b619f7..6fcc8c6477 100755 --- a/effects/presentwindows/presentwindows.cpp +++ b/effects/presentwindows/presentwindows.cpp @@ -96,7 +96,12 @@ PresentWindowsEffect::PresentWindowsEffect() connect(effects, SIGNAL(windowDeleted(KWin::EffectWindow*)), this, SLOT(slotWindowDeleted(KWin::EffectWindow*))); connect(effects, SIGNAL(windowGeometryShapeChanged(KWin::EffectWindow*,QRect)), this, SLOT(slotWindowGeometryShapeChanged(KWin::EffectWindow*,QRect))); connect(effects, SIGNAL(propertyNotify(KWin::EffectWindow*,long)), this, SLOT(slotPropertyNotify(KWin::EffectWindow*,long))); - connect(effects, &EffectsHandler::numberScreensChanged, this, &PresentWindowsEffect::screenCountChanged); + connect(effects, &EffectsHandler::numberScreensChanged, this, + [this] { + if (isActive()) + reCreateGrids(); + } + ); } PresentWindowsEffect::~PresentWindowsEffect() @@ -1489,7 +1494,7 @@ void PresentWindowsEffect::setActive(bool active) m_hasKeyboardGrab = effects->grabKeyboard(this); effects->setActiveFullScreenEffect(this); - screenCountChanged(); + reCreateGrids(); rearrangeWindows(); setHighlightedWindow(effects->activeWindow()); @@ -1866,10 +1871,8 @@ bool PresentWindowsEffect::isActive() const return m_activated || m_motionManager.managingWindows(); } -void PresentWindowsEffect::screenCountChanged() +void PresentWindowsEffect::reCreateGrids() { - if (!isActive()) - return; m_gridSizes.clear(); for (int i = 0; i < effects->numScreens(); ++i) { m_gridSizes.append(GridSize()); diff --git a/effects/presentwindows/presentwindows.h b/effects/presentwindows/presentwindows.h index f57c6a77dd..81154b51c4 100644 --- a/effects/presentwindows/presentwindows.h +++ b/effects/presentwindows/presentwindows.h @@ -225,11 +225,11 @@ public Q_SLOTS: private Q_SLOTS: void closeWindow(); void elevateCloseWindow(); - void screenCountChanged(); protected: // Window rearranging void rearrangeWindows(); + void reCreateGrids(); void calculateWindowTransformations(EffectWindowList windowlist, int screen, WindowMotionManager& motionManager, bool external = false); void calculateWindowTransformationsClosest(EffectWindowList windowlist, int screen, diff --git a/effects/presentwindows/presentwindows_proxy.cpp b/effects/presentwindows/presentwindows_proxy.cpp index 5c4937e53e..f6aca8737a 100644 --- a/effects/presentwindows/presentwindows_proxy.cpp +++ b/effects/presentwindows/presentwindows_proxy.cpp @@ -39,4 +39,9 @@ void PresentWindowsEffectProxy::calculateWindowTransformations(EffectWindowList return m_effect->calculateWindowTransformations(windows, screen, manager, true); } +void PresentWindowsEffectProxy::reCreateGrids() +{ + m_effect->reCreateGrids(); +} + } // namespace diff --git a/effects/presentwindows/presentwindows_proxy.h b/effects/presentwindows/presentwindows_proxy.h index 1381f30401..8c8bd294f9 100644 --- a/effects/presentwindows/presentwindows_proxy.h +++ b/effects/presentwindows/presentwindows_proxy.h @@ -35,6 +35,8 @@ public: void calculateWindowTransformations(EffectWindowList windows, int screen, WindowMotionManager& manager); + void reCreateGrids(); + private: PresentWindowsEffect* m_effect; }; From cc6886d7dd91ab7a4206ae6637886d7664127bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Mon, 31 Aug 2015 22:25:48 +0200 Subject: [PATCH 2/4] fetch motif hints when get them for managed client notably *after* storing the old values. Otherwise the old value is polluted because of m_hints being nullptr, thus a default value is returned (instead of the actual old value) BUG: 347818 FIXED-IN: 5.4.2 REVIEW: 125007 --- client.cpp | 2 ++ events.cpp | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/client.cpp b/client.cpp index d1725b7612..f325699c55 100644 --- a/client.cpp +++ b/client.cpp @@ -1710,6 +1710,8 @@ void Client::getMotifHints() { const bool wasClosable = m_motif.close(); const bool wasNoBorder = m_motif.noBorder(); + if (m_managed) // only on property change, initial read is prefetched + m_motif.fetch(); m_motif.read(); if (m_motif.hasDecoration() && m_motif.noBorder() != wasNoBorder) { // If we just got a hint telling us to hide decorations, we do so. diff --git a/events.cpp b/events.cpp index 5baab1fb1a..70dd572715 100644 --- a/events.cpp +++ b/events.cpp @@ -909,7 +909,6 @@ void Client::propertyNotifyEvent(xcb_property_notify_event_t *e) break; default: if (e->atom == atoms->motif_wm_hints) { - m_motif.fetch(); getMotifHints(); } else if (e->atom == atoms->net_wm_sync_request_counter) getSyncCounter(); From 584850f1604ceff0cd8e3cd5f6b16b2c5c4d0c6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Fri, 28 Aug 2015 22:52:03 +0200 Subject: [PATCH 3/4] ensure to "hide" desktop buttons before, hiding the buttons relied on the effect seeing a paint event for the (with a low timeline value) what's too wonky at least for instant animations (certain failure) or when effect exits immediatey (due to screen edge invocation issues or whatever) BUG: 351869 FIXED-IN: 5.4.2 REVIEW: 124970 --- effects/desktopgrid/desktopgrid.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/effects/desktopgrid/desktopgrid.cpp b/effects/desktopgrid/desktopgrid.cpp index 396ce64857..00aa846e16 100644 --- a/effects/desktopgrid/desktopgrid.cpp +++ b/effects/desktopgrid/desktopgrid.cpp @@ -36,6 +36,7 @@ along with this program. If not, see . #include #include #include +#include #include #include #include @@ -1052,6 +1053,15 @@ void DesktopGridEffect::setActive(bool active) } } } + QTimer::singleShot(zoomDuration + 1, this, + [this] { + if (activated) + return; + foreach (DesktopButtonsView *view, m_desktopButtonsViews.keys()) { + view->hide(); + } + } + ); setHighlightedDesktop(effects->currentDesktop()); // Ensure selected desktop is highlighted } effects->addRepaintFull(); From 295132deef776a9e7691b38c3db86f091227fcc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20L=C3=BCbking?= Date: Wed, 9 Sep 2015 21:45:55 +0200 Subject: [PATCH 4/4] reset the transientInfo id when cleaning group otherwise a pseudo-transient window with no group causes a nullptr deref in eg. mainClients BUG: 352483 FIXED-IN: 5.4.2 REVIEW: 125122 --- group.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/group.cpp b/group.cpp index b878f3dc5f..434f787492 100644 --- a/group.cpp +++ b/group.cpp @@ -672,6 +672,7 @@ void Client::cleanGrouping() // it != group_members.end(); // ++it ) // qDebug() << "CL4:" << *it; + m_transientForId = XCB_WINDOW_NONE; } // Make sure that no group transient is considered transient @@ -886,6 +887,7 @@ ClientList Client::mainClients() const if (transientFor() != NULL) return ClientList() << const_cast< Client* >(transientFor()); ClientList result; + Q_ASSERT(group()); for (ClientList::ConstIterator it = group()->members().constBegin(); it != group()->members().constEnd(); ++it)