animation: fix crashes and cleanup of active vars (#42)

Minor stuff

---------

Co-authored-by: Vaxry <vaxry@vaxry.net>

include/hyprutils/animation/AnimatedVariable.hpp

@@ -2,14 +2,15 @@
 
 #include "AnimationConfig.hpp"
 #include "../memory/WeakPtr.hpp"
-#include "hyprutils/memory/SharedPtr.hpp"
+#include "../memory/SharedPtr.hpp"
+#include "../signal/Signal.hpp"
+#include "AnimationManager.hpp"
 
 #include <functional>
 #include <chrono>
 
 namespace Hyprutils {
     namespace Animation {
-        class CAnimationManager;
 
         /* A base class for animated variables. */
         class CBaseAnimatedVariable {
@@ -29,14 +30,15 @@ namespace Hyprutils {
                 disconnectFromActive();
             };
 
-            virtual void warp(bool endCallback = true) = 0;
+            virtual void warp(bool endCallback = true, bool forceDisconnect = true) = 0;
 
             CBaseAnimatedVariable(const CBaseAnimatedVariable&)            = delete;
             CBaseAnimatedVariable(CBaseAnimatedVariable&&)                 = delete;
             CBaseAnimatedVariable& operator=(const CBaseAnimatedVariable&) = delete;
             CBaseAnimatedVariable& operator=(CBaseAnimatedVariable&&)      = delete;
 
-            void                   setConfig(Memory::CSharedPointer<SAnimationPropertyConfig> pConfig) {
+            //
+            void setConfig(Memory::CSharedPointer<SAnimationPropertyConfig> pConfig) {
                 m_pConfig = pConfig;
             }
 
@@ -51,7 +53,7 @@ namespace Hyprutils {
             /* returns the spent (completion) % */
             float getPercent() const;
 
-            /* returns the current curve value */
+            /* returns the current curve value. */
             float getCurveValue() const;
 
             /* checks if an animation is in progress */
@@ -83,15 +85,22 @@ namespace Hyprutils {
             void onAnimationEnd();
             void onAnimationBegin();
 
+            /* returns whether the parent CAnimationManager is dead */
+            bool isAnimationManagerDead() const;
+
             int  m_Type = -1;
 
           protected:
             friend class CAnimationManager;
 
-            bool                                        m_bIsConnectedToActive = false;
+            CAnimationManager*                                                m_pAnimationManager = nullptr;
-            bool                                        m_bIsBeingAnimated     = false;
 
-            Memory::CWeakPointer<CBaseAnimatedVariable> m_pSelf;
+            bool                                                              m_bIsConnectedToActive = false;
+            bool                                                              m_bIsBeingAnimated     = false;
+
+            Memory::CWeakPointer<CBaseAnimatedVariable>                       m_pSelf;
+
+            Memory::CWeakPointer<CAnimationManager::SAnimationManagerSignals> m_pSignals;
 
           private:
             Memory::CWeakPointer<SAnimationPropertyConfig> m_pConfig;
@@ -100,7 +109,6 @@ namespace Hyprutils {
 
             bool                                           m_bDummy = true;
 
-            CAnimationManager*                             m_pAnimationManager    = nullptr;
             bool                                           m_bRemoveEndAfterRan   = true;
             bool                                           m_bRemoveBeginAfterRan = true;
 
@@ -142,7 +150,7 @@ namespace Hyprutils {
             CGenericAnimatedVariable& operator=(const CGenericAnimatedVariable&) = delete;
             CGenericAnimatedVariable& operator=(CGenericAnimatedVariable&&)      = delete;
 
-            virtual void              warp(bool endCallback = true) {
+            virtual void              warp(bool endCallback = true, bool forceDisconnect = true) {
                 if (!m_bIsBeingAnimated)
                     return;
 
@@ -154,6 +162,9 @@ namespace Hyprutils {
 
                 if (endCallback)
                     onAnimationEnd();
+
+                if (forceDisconnect)
+                    disconnectFromActive();
             }
 
             const VarType& value() const {

include/hyprutils/animation/AnimationManager.hpp

@@ -1,15 +1,18 @@
 #pragma once
 
 #include "./BezierCurve.hpp"
-#include "./AnimatedVariable.hpp"
 #include "../math/Vector2D.hpp"
 #include "../memory/WeakPtr.hpp"
+#include "../signal/Signal.hpp"
 
+#include <cstdint>
 #include <unordered_map>
 #include <vector>
 
 namespace Hyprutils {
     namespace Animation {
+        class CBaseAnimatedVariable;
+
         /* A class for managing bezier curves and variables that are being animated. */
         class CAnimationManager {
           public:
@@ -17,6 +20,7 @@ namespace Hyprutils {
             virtual ~CAnimationManager() = default;
 
             void                                                                         tickDone();
+            void                                                                         rotateActive();
             bool                                                                         shouldTickForNext();
 
             virtual void                                                                 scheduleTick() = 0;
@@ -30,12 +34,30 @@ namespace Hyprutils {
 
             const std::unordered_map<std::string, Memory::CSharedPointer<CBezierCurve>>& getAllBeziers();
 
-            std::vector<Memory::CWeakPointer<CBaseAnimatedVariable>>                     m_vActiveAnimatedVariables;
+            struct SAnimationManagerSignals {
+                Signal::CSignal connect;    // WP<CBaseAnimatedVariable>
+                Signal::CSignal disconnect; // WP<CBaseAnimatedVariable>
+            };
+
+            Memory::CWeakPointer<SAnimationManagerSignals>           getSignals() const;
+
+            std::vector<Memory::CWeakPointer<CBaseAnimatedVariable>> m_vActiveAnimatedVariables;
 
           private:
             std::unordered_map<std::string, Memory::CSharedPointer<CBezierCurve>> m_mBezierCurves;
 
             bool                                                                  m_bTickScheduled = false;
+
+            void                                                                  onConnect(std::any data);
+            void                                                                  onDisconnect(std::any data);
+
+            struct SAnimVarListeners {
+                Signal::CHyprSignalListener connect;
+                Signal::CHyprSignalListener disconnect;
+            };
+
+            Memory::CUniquePointer<SAnimVarListeners>        m_listeners;
+            Memory::CUniquePointer<SAnimationManagerSignals> m_events;
         };
     }
 }

src/animation/AnimatedVariable.cpp

@@ -8,29 +8,28 @@ using namespace Hyprutils::Memory;
 #define SP CSharedPointer
 #define WP CWeakPointer
 
-void CBaseAnimatedVariable::create(Hyprutils::Animation::CAnimationManager* pAnimationManager, int typeInfo, SP<CBaseAnimatedVariable> pSelf) {
+void CBaseAnimatedVariable::create(CAnimationManager* pManager, int typeInfo, SP<CBaseAnimatedVariable> pSelf) {
-    m_pAnimationManager = pAnimationManager;
+    m_Type  = typeInfo;
-    m_Type              = typeInfo;
+    m_pSelf = pSelf;
-    m_pSelf             = pSelf;
 
-    m_bDummy = false;
+    m_pAnimationManager = pManager;
+    m_pSignals = pManager->getSignals();
+    m_bDummy   = false;
 }
 
 void CBaseAnimatedVariable::connectToActive() {
-    if (!m_pAnimationManager || m_bDummy)
+    if (m_bDummy || m_bIsConnectedToActive || isAnimationManagerDead())
         return;
 
-    m_pAnimationManager->scheduleTick(); // otherwise the animation manager will never pick this up
+    m_pSignals->connect.emit(m_pSelf);
-    if (!m_bIsConnectedToActive)
-        m_pAnimationManager->m_vActiveAnimatedVariables.push_back(m_pSelf);
     m_bIsConnectedToActive = true;
 }
 
 void CBaseAnimatedVariable::disconnectFromActive() {
-    if (!m_pAnimationManager)
+    if (isAnimationManagerDead())
         return;
 
-    std::erase_if(m_pAnimationManager->m_vActiveAnimatedVariables, [&](const auto& other) { return other == m_pSelf; });
+    m_pSignals->disconnect.emit(m_pSelf);
     m_bIsConnectedToActive = false;
 }
 
@@ -77,7 +76,7 @@ float CBaseAnimatedVariable::getPercent() const {
 }
 
 float CBaseAnimatedVariable::getCurveValue() const {
-    if (!m_bIsBeingAnimated || !m_pAnimationManager)
+    if (!m_bIsBeingAnimated || isAnimationManagerDead())
         return 1.f;
 
     std::string bezierName = "";
@@ -99,7 +98,7 @@ float CBaseAnimatedVariable::getCurveValue() const {
 }
 
 bool CBaseAnimatedVariable::ok() const {
-    return m_pConfig && m_pAnimationManager;
+    return m_pConfig && !m_bDummy && !isAnimationManagerDead();
 }
 
 void CBaseAnimatedVariable::onUpdate() {
@@ -156,3 +155,7 @@ void CBaseAnimatedVariable::onAnimationBegin() {
             m_fBeginCallback = nullptr; // reset
     }
 }
+
+bool CBaseAnimatedVariable::isAnimationManagerDead() const {
+    return m_pSignals.expired();
+}

src/animation/AnimationManager.cpp

@@ -1,10 +1,13 @@
 #include <hyprutils/animation/AnimationManager.hpp>
+#include <hyprutils/animation/AnimatedVariable.hpp>
 
 using namespace Hyprutils::Animation;
 using namespace Hyprutils::Math;
 using namespace Hyprutils::Memory;
+using namespace Hyprutils::Signal;
 
 #define SP CSharedPointer
+#define WP CWeakPointer
 
 const std::array<Vector2D, 2> DEFAULTBEZIERPOINTS = {Vector2D(0.0, 0.75), Vector2D(0.15, 1.0)};
 
@@ -12,6 +15,35 @@ CAnimationManager::CAnimationManager() {
     const auto BEZIER = makeShared<CBezierCurve>();
     BEZIER->setup(DEFAULTBEZIERPOINTS);
     m_mBezierCurves["default"] = BEZIER;
+
+    m_events    = makeUnique<SAnimationManagerSignals>();
+    m_listeners = makeUnique<SAnimVarListeners>();
+
+    m_listeners->connect    = m_events->connect.registerListener([this](std::any data) { onConnect(data); });
+    m_listeners->disconnect = m_events->disconnect.registerListener([this](std::any data) { onDisconnect(data); });
+}
+
+void CAnimationManager::onConnect(std::any data) {
+    if (!m_bTickScheduled)
+        scheduleTick();
+
+    try {
+        const auto PAV = std::any_cast<WP<CBaseAnimatedVariable>>(data);
+        if (!PAV)
+            return;
+
+        m_vActiveAnimatedVariables.emplace_back(PAV);
+    } catch (const std::bad_any_cast&) { return; }
+}
+
+void CAnimationManager::onDisconnect(std::any data) {
+    try {
+        const auto PAV = std::any_cast<WP<CBaseAnimatedVariable>>(data);
+        if (!PAV)
+            return;
+
+        std::erase_if(m_vActiveAnimatedVariables, [&](const auto& other) { return !other || other == PAV; });
+    } catch (const std::bad_any_cast&) { return; }
 }
 
 void CAnimationManager::removeAllBeziers() {
@@ -37,6 +69,10 @@ bool CAnimationManager::shouldTickForNext() {
 }
 
 void CAnimationManager::tickDone() {
+    rotateActive();
+}
+
+void CAnimationManager::rotateActive() {
     std::vector<CWeakPointer<CBaseAnimatedVariable>> active;
     active.reserve(m_vActiveAnimatedVariables.size()); // avoid reallocations
     for (auto const& av : m_vActiveAnimatedVariables) {
@@ -71,3 +107,7 @@ SP<CBezierCurve> CAnimationManager::getBezier(const std::string& name) {
 const std::unordered_map<std::string, SP<CBezierCurve>>& CAnimationManager::getAllBeziers() {
     return m_mBezierCurves;
 }
+
+CWeakPointer<CAnimationManager::SAnimationManagerSignals> CAnimationManager::getSignals() const {
+    return m_events;
+}

tests/animation.cpp

@@ -2,10 +2,12 @@
 #include <hyprutils/animation/AnimationManager.hpp>
 #include <hyprutils/animation/AnimatedVariable.hpp>
 #include <hyprutils/memory/WeakPtr.hpp>
+#include <hyprutils/memory/UniquePtr.hpp>
 #include "shared.hpp"
 
 #define SP CSharedPointer
 #define WP CWeakPointer
+#define UP CUniquePointer
 
 using namespace Hyprutils::Animation;
 using namespace Hyprutils::Math;
@@ -45,14 +47,14 @@ class CMyAnimationManager : public CAnimationManager {
     void tick() {
         for (size_t i = 0; i < m_vActiveAnimatedVariables.size(); i++) {
             const auto PAV = m_vActiveAnimatedVariables[i].lock();
-            if (!PAV || !PAV->ok())
+            if (!PAV || !PAV->ok() || !PAV->isBeingAnimated())
                 continue;
 
             const auto SPENT   = PAV->getPercent();
             const auto PBEZIER = getBezier(PAV->getBezierName());
 
             if (SPENT >= 1.f || !PAV->enabled()) {
-                PAV->warp();
+                PAV->warp(true, false);
                 continue;
             }
 
@@ -105,14 +107,14 @@ class CMyAnimationManager : public CAnimationManager {
     }
 };
 
-CMyAnimationManager gAnimationManager;
+UP<CMyAnimationManager> pAnimationManager;
 
 class Subject {
   public:
     Subject(const int& a, const int& b) {
-        gAnimationManager.createAnimation(a, m_iA, "default");
+        pAnimationManager->createAnimation(a, m_iA, "default");
-        gAnimationManager.createAnimation(b, m_iB, "internal");
+        pAnimationManager->createAnimation(b, m_iB, "internal");
-        gAnimationManager.createAnimation({}, m_iC, "default");
+        pAnimationManager->createAnimation({}, m_iC, "default");
     }
     PANIMVAR<int>          m_iA;
     PANIMVAR<int>          m_iB;
@@ -120,6 +122,8 @@ class Subject {
 };
 
 int config() {
+    pAnimationManager = makeUnique<CMyAnimationManager>();
+
     int ret = 0;
 
     animationTree.createNode("global");
@@ -205,7 +209,7 @@ int main(int argc, char** argv, char** envp) {
         // We deliberately do not tick here, to make sure the destructor removes active animated variables
     }
 
-    EXPECT(gAnimationManager.shouldTickForNext(), false);
+    EXPECT(pAnimationManager->shouldTickForNext(), false);
     EXPECT(s.m_iC->value().done, false);
 
     *s.m_iA = 10;
@@ -214,8 +218,8 @@ int main(int argc, char** argv, char** envp) {
 
     EXPECT(s.m_iC->value().done, false);
 
-    while (gAnimationManager.shouldTickForNext()) {
+    while (pAnimationManager->shouldTickForNext()) {
-        gAnimationManager.tick();
+        pAnimationManager->tick();
     }
 
     EXPECT(s.m_iA->value(), 10);
@@ -225,8 +229,8 @@ int main(int argc, char** argv, char** envp) {
     s.m_iA->setValue(0);
     s.m_iB->setValue(0);
 
-    while (gAnimationManager.shouldTickForNext()) {
+    while (pAnimationManager->shouldTickForNext()) {
-        gAnimationManager.tick();
+        pAnimationManager->tick();
     }
 
     EXPECT(s.m_iA->value(), 10);
@@ -242,7 +246,7 @@ int main(int argc, char** argv, char** envp) {
     EXPECT(s.m_iA->enabled(), false);
 
     *s.m_iA = 50;
-    gAnimationManager.tick(); // Expecting a warp
+    pAnimationManager->tick(); // Expecting a warp
     EXPECT(s.m_iA->value(), 50);
 
     // Test missing pValues
@@ -274,8 +278,8 @@ int main(int argc, char** argv, char** envp) {
     EXPECT(endCallbackRan, 2); // first called when setting the callback, then when warping.
 
     *s.m_iA = 1337;
-    while (gAnimationManager.shouldTickForNext()) {
+    while (pAnimationManager->shouldTickForNext()) {
-        gAnimationManager.tick();
+        pAnimationManager->tick();
     }
 
     EXPECT(beginCallbackRan, 1);
@@ -285,7 +289,7 @@ int main(int argc, char** argv, char** envp) {
     std::vector<PANIMVAR<int>> vars;
     for (int i = 0; i < 10; i++) {
         vars.resize(vars.size() + 1);
-        gAnimationManager.createAnimation(1, vars.back(), "default");
+        pAnimationManager->createAnimation(1, vars.back(), "default");
         *vars.back() = 1337;
     }
 
@@ -297,14 +301,14 @@ int main(int argc, char** argv, char** envp) {
     });
     s.m_iA->setCallbackOnEnd([&s, &vars](auto) {
         vars.resize(vars.size() + 1);
-        gAnimationManager.createAnimation(1, vars.back(), "default");
+        pAnimationManager->createAnimation(1, vars.back(), "default");
         *vars.back() = 1337;
     });
 
     *s.m_iA = 1000000;
 
-    while (gAnimationManager.shouldTickForNext()) {
+    while (pAnimationManager->shouldTickForNext()) {
-        gAnimationManager.tick();
+        pAnimationManager->tick();
     }
 
     EXPECT(s.m_iA->value(), 1000000);
@@ -322,5 +326,55 @@ int main(int argc, char** argv, char** envp) {
     EXPECT(endCallbackRan, 4);
     EXPECT(s.m_iA->value(), 10);
 
+    // test warp
+    *s.m_iA = 3;
+    s.m_iA->setCallbackOnEnd([&endCallbackRan](auto) { endCallbackRan++; }, false);
+
+    s.m_iA->warp(false);
+    EXPECT(endCallbackRan, 4);
+
+    *s.m_iA = 4;
+    s.m_iA->warp(true);
+    EXPECT(endCallbackRan, 5);
+
+    // test getCurveValue
+    *s.m_iA = 0;
+    EXPECT(s.m_iA->getCurveValue(), 0.f);
+    s.m_iA->warp();
+    EXPECT(s.m_iA->getCurveValue(), 1.f);
+    EXPECT(endCallbackRan, 6);
+
+    // Test duplicate active anim vars are not allowed
+    {
+        EXPECT(pAnimationManager->m_vActiveAnimatedVariables.size(), 0);
+        PANIMVAR<int> a;
+        pAnimationManager->createAnimation(1, a, "default");
+        EXPECT(pAnimationManager->m_vActiveAnimatedVariables.size(), 0);
+        *a = 10;
+        EXPECT(pAnimationManager->m_vActiveAnimatedVariables.size(), 1);
+        *a = 20;
+        EXPECT(pAnimationManager->m_vActiveAnimatedVariables.size(), 1);
+        a->warp();
+        EXPECT(pAnimationManager->m_vActiveAnimatedVariables.size(), 0);
+        EXPECT(a->value(), 20);
+    }
+
+    // Test no crash when animation manager gets destroyed
+    {
+        PANIMVAR<int> a;
+        pAnimationManager->createAnimation(1, a, "default");
+        *a = 10;
+        pAnimationManager.reset();
+        EXPECT(a->isAnimationManagerDead(), true);
+        a->setValueAndWarp(11);
+        EXPECT(a->value(), 11);
+        *a = 12;
+        a->warp();
+        EXPECT(a->value(), 12);
+        *a = 13;
+    } // a gets destroyed
+
+    EXPECT(pAnimationManager.get(), nullptr);
+
     return ret;
 }