feat(effect): add deleted ref helper

This simplifies referencing windows by effects on deletion.
winft · Jun 17, 2023 · 0c0f75c · 0c0f75c
1 parent 0fddafe
commit 0c0f75c
Show file tree

Hide file tree

Showing 12 changed files with 91 additions and 66 deletions.
diff --git a/lib/effect/kwineffects/anidata.cpp b/lib/effect/kwineffects/anidata.cpp
@@ -28,17 +28,6 @@ FullScreenEffectLock::~FullScreenEffectLock()
     effects->setActiveFullScreenEffect(nullptr);
 }
 
-KeepAliveLock::KeepAliveLock(EffectWindow* w)
-    : m_window(w)
-{
-    m_window->refWindow();
-}
-
-KeepAliveLock::~KeepAliveLock()
-{
-    m_window->unrefWindow();
-}
-
 PreviousWindowPixmapLock::PreviousWindowPixmapLock(EffectWindow* w)
     : m_window(w)
 {

diff --git a/lib/effect/kwineffects/anidata_p.h b/lib/effect/kwineffects/anidata_p.h
@@ -10,6 +10,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
 
 #include "kwingl/utils.h"
 #include <kwineffects/animation_effect.h>
+#include <kwineffects/effect_window_deleted_ref.h>
 #include <kwineffects/time_line.h>
 
 #include <QEasingCurve>
@@ -32,21 +33,6 @@ class FullScreenEffectLock
 };
 typedef QSharedPointer<FullScreenEffectLock> FullScreenEffectLockPtr;
 
-/**
- * Keeps windows alive during animation after they got closed
- */
-class KeepAliveLock
-{
-public:
-    KeepAliveLock(EffectWindow* w);
-    ~KeepAliveLock();
-
-private:
-    EffectWindow* m_window;
-    Q_DISABLE_COPY(KeepAliveLock)
-};
-typedef QSharedPointer<KeepAliveLock> KeepAliveLockPtr;
-
 /**
  * References the previous window pixmap to prevent discarding.
  */
@@ -96,7 +82,7 @@ class KWINEFFECTS_EXPORT AniData
     QSharedPointer<FullScreenEffectLock> fullScreenEffectLock;
     bool waitAtSource;
     bool keepAlive;
-    KeepAliveLockPtr keepAliveLock;
+    EffectWindowDeletedRef deletedRef;
     PreviousWindowPixmapLockPtr previousWindowPixmapLock;
     AnimationEffect::TerminationFlags terminationFlags;
     GLShader* shader{nullptr};

diff --git a/lib/effect/kwineffects/animation_effect.cpp b/lib/effect/kwineffects/animation_effect.cpp
@@ -51,7 +51,6 @@ quint64 AnimationEffectPrivate::m_animCounter = 0;
 AnimationEffect::AnimationEffect()
     : d_ptr(new AnimationEffectPrivate())
 {
-    Q_D(AnimationEffect);
     if (!s_clock.isValid())
         s_clock.start();
     /* this is the same as the QTimer::singleShot(0, SLOT(init())) kludge
@@ -1036,19 +1035,11 @@ void AnimationEffect::_windowClosed(EffectWindow* w)
         return;
     }
 
-    KeepAliveLockPtr keepAliveLock;
-
     QList<AniData>& animations = (*it).first;
     for (auto animationIt = animations.begin(); animationIt != animations.end(); ++animationIt) {
-        if (!(*animationIt).keepAlive) {
-            continue;
+        if (animationIt->keepAlive) {
+            animationIt->deletedRef = EffectWindowDeletedRef(w);
         }
-
-        if (keepAliveLock.isNull()) {
-            keepAliveLock = KeepAliveLockPtr::create(w);
-        }
-
-        (*animationIt).keepAliveLock = keepAliveLock;
     }
 }
 

diff --git a/lib/effect/kwineffects/effect_window_deleted_ref.h b/lib/effect/kwineffects/effect_window_deleted_ref.h
@@ -0,0 +1,69 @@
+/*
+    SPDX-FileCopyrightText: 2023 Roman Gilg <[email protected]>
+
+    SPDX-License-Identifier: GPL-2.0-or-later
+*/
+#pragma once
+
+#include <kwineffects/effect_window.h>
+
+namespace KWin
+{
+
+class EffectWindow;
+
+/**
+ * The EffectWindowDeletedRef provides a convenient way to prevent deleting a closed
+ * window until an effect has finished animating it.
+ */
+class EffectWindowDeletedRef
+{
+public:
+    EffectWindowDeletedRef()
+        : m_window(nullptr)
+    {
+    }
+
+    explicit EffectWindowDeletedRef(EffectWindow* window)
+        : m_window(window)
+    {
+        m_window->refWindow();
+    }
+
+    EffectWindowDeletedRef(EffectWindowDeletedRef const& other)
+        : m_window(other.m_window)
+    {
+        if (m_window) {
+            m_window->refWindow();
+        }
+    }
+
+    ~EffectWindowDeletedRef()
+    {
+        if (m_window) {
+            m_window->unrefWindow();
+        }
+    }
+
+    EffectWindowDeletedRef& operator=(EffectWindowDeletedRef const& other)
+    {
+        if (other.m_window) {
+            other.m_window->refWindow();
+        }
+        if (m_window) {
+            m_window->unrefWindow();
+        }
+        m_window = other.m_window;
+        return *this;
+    }
+
+    bool isNull() const
+    {
+        return m_window == nullptr;
+    }
+
+private:
+    EffectWindow* m_window;
+};
+
+}
diff --git a/plugins/effects/fallapart/fallapart.cpp b/plugins/effects/fallapart/fallapart.cpp
@@ -126,7 +126,6 @@ void FallApartEffect::postPaintScreen()
             ++it;
         } else {
             unredirect(it.key());
-            it.key()->unrefWindow();
             it = windows.erase(it);
         }
     }
@@ -172,8 +171,11 @@ void FallApartEffect::slotWindowClosed(EffectWindow* c)
     if (e && e != this)
         return;
     c->setData(WindowClosedGrabRole, QVariant::fromValue(static_cast<void*>(this)));
-    windows[c].progress = 0;
-    c->refWindow();
+
+    auto& animation = windows[c];
+    animation.progress = 0;
+    animation.deletedRef = EffectWindowDeletedRef(c);
+
     redirect(c);
 }
 
@@ -198,7 +200,6 @@ void FallApartEffect::slotWindowDataChanged(EffectWindow* w, int role)
     }
 
     unredirect(it.key());
-    it.key()->unrefWindow();
     windows.erase(it);
 }
 

diff --git a/plugins/effects/fallapart/fallapart.h b/plugins/effects/fallapart/fallapart.h
@@ -7,12 +7,14 @@ SPDX-License-Identifier: GPL-2.0-or-later
 #ifndef KWIN_FALLAPART_H
 #define KWIN_FALLAPART_H
 
+#include <kwineffects/effect_window_deleted_ref.h>
 #include <kwineffects/offscreen_effect.h>
 
 namespace KWin
 {
 
 struct FallApartAnimation {
+    EffectWindowDeletedRef deletedRef;
     std::chrono::milliseconds lastPresentTime = std::chrono::milliseconds::zero();
     qreal progress = 0;
 };

diff --git a/plugins/effects/glide/glide.cpp b/plugins/effects/glide/glide.cpp
@@ -165,10 +165,6 @@ void GlideEffect::postPaintScreen()
     auto animationIt = m_animations.begin();
     while (animationIt != m_animations.end()) {
         if ((*animationIt).timeLine.done()) {
-            EffectWindow* w = animationIt.key();
-            if (w->isDeleted()) {
-                w->unrefWindow();
-            }
             animationIt = m_animations.erase(animationIt);
         } else {
             ++animationIt;
@@ -238,10 +234,10 @@ void GlideEffect::windowClosed(EffectWindow* w)
         return;
     }
 
-    w->refWindow();
     w->setData(WindowClosedGrabRole, QVariant::fromValue(static_cast<void*>(this)));
 
     GlideAnimation& animation = m_animations[w];
+    animation.deletedRef = EffectWindowDeletedRef(w);
     animation.timeLine.reset();
     animation.timeLine.setDirection(TimeLine::Forward);
     animation.timeLine.setDuration(m_duration);
@@ -270,10 +266,6 @@ void GlideEffect::windowDataChanged(EffectWindow* w, int role)
         return;
     }
 
-    if (w->isDeleted() && role == WindowClosedGrabRole) {
-        w->unrefWindow();
-    }
-
     m_animations.erase(animationIt);
 }
 

diff --git a/plugins/effects/glide/glide.h b/plugins/effects/glide/glide.h
@@ -10,12 +10,14 @@ SPDX-License-Identifier: GPL-2.0-or-later
 #define KWIN_GLIDE_H
 
 #include <kwineffects/effect.h>
+#include <kwineffects/effect_window_deleted_ref.h>
 #include <kwineffects/time_line.h>
 
 namespace KWin
 {
 
 struct GlideAnimation {
+    EffectWindowDeletedRef deletedRef;
     TimeLine timeLine;
 };
 

diff --git a/plugins/effects/sheet/sheet.cpp b/plugins/effects/sheet/sheet.cpp
@@ -111,9 +111,6 @@ void SheetEffect::postPaintWindow(EffectWindow* w)
         EffectWindow* w = animationIt.key();
         w->addRepaintFull();
         if ((*animationIt).timeLine.done()) {
-            if (w->isDeleted()) {
-                w->unrefWindow();
-            }
             animationIt = m_animations.erase(animationIt);
         } else {
             ++animationIt;
@@ -177,10 +174,9 @@ void SheetEffect::slotWindowClosed(EffectWindow* w)
         return;
     }
 
-    w->refWindow();
-
     Animation& animation = m_animations[w];
 
+    animation.deletedRef = EffectWindowDeletedRef(w);
     animation.timeLine.reset();
     animation.parentY = 0;
     animation.timeLine.setDuration(m_duration);

diff --git a/plugins/effects/sheet/sheet.h b/plugins/effects/sheet/sheet.h
@@ -9,6 +9,7 @@ SPDX-License-Identifier: GPL-2.0-or-later
 #define KWIN_SHEET_H
 
 #include <kwineffects/effect.h>
+#include <kwineffects/effect_window_deleted_ref.h>
 #include <kwineffects/time_line.h>
 
 namespace KWin
@@ -50,6 +51,7 @@ private Q_SLOTS:
     std::chrono::milliseconds m_duration;
 
     struct Animation {
+        EffectWindowDeletedRef deletedRef;
         TimeLine timeLine;
         int parentY;
     };

diff --git a/plugins/effects/slidingpopups/slidingpopups.cpp b/plugins/effects/slidingpopups/slidingpopups.cpp
@@ -235,9 +235,7 @@ void SlidingPopupsEffect::postPaintWindow(EffectWindow* win)
     auto animationIt = animations.find(win);
     if (animationIt != animations.end()) {
         if ((*animationIt).timeline.done()) {
-            if (win->isDeleted()) {
-                win->unrefWindow();
-            } else {
+            if (!win->isDeleted()) {
                 win->setData(WindowForceBackgroundContrastRole, QVariant());
                 win->setData(WindowForceBlurRole, QVariant());
             }
@@ -305,11 +303,8 @@ void SlidingPopupsEffect::slide_out(EffectWindow* win)
         return;
     }
 
-    if (win->isDeleted()) {
-        win->refWindow();
-    }
-
     auto& animation = animations[win];
+    animation.deletedRef = EffectWindowDeletedRef(win);
     animation.kind = AnimationKind::Out;
     animation.timeline.setDirection(TimeLine::Backward);
     animation.timeline.setDuration((*dataIt).out);
@@ -336,9 +331,7 @@ void SlidingPopupsEffect::stopAnimations()
     for (auto it = animations.constBegin(); it != animations.constEnd(); ++it) {
         auto win = it.key();
 
-        if (win->isDeleted()) {
-            win->unrefWindow();
-        } else {
+        if (!win->isDeleted()) {
             win->setData(WindowForceBackgroundContrastRole, QVariant());
             win->setData(WindowForceBlurRole, QVariant());
         }

diff --git a/plugins/effects/slidingpopups/slidingpopups.h b/plugins/effects/slidingpopups/slidingpopups.h
@@ -9,6 +9,7 @@
 
 #include <kwineffects/effect.h>
 #include <kwineffects/effect_integration.h>
+#include <kwineffects/effect_window_deleted_ref.h>
 #include <kwineffects/time_line.h>
 
 #include <memory>
@@ -47,6 +48,7 @@ class SlidingPopupsEffect : public Effect
     };
 
     struct Animation {
+        EffectWindowDeletedRef deletedRef;
         AnimationKind kind;
         TimeLine timeline;
     };