animation: fix adding/removign vars during ::tick

hyprwm · Jan 11, 2025 · a572036 · a572036
1 parent 1548e8b
commit a572036
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 23 deletions.
diff --git a/include/hyprutils/animation/AnimatedVariable.hpp b/include/hyprutils/animation/AnimatedVariable.hpp
@@ -62,9 +62,6 @@ namespace Hyprutils {
             /* checks m_bDummy and m_pAnimationManager */
             bool ok() const;
 
-            /* calls the update callback */
-            void onUpdate();
-
             /* sets a function to be ran when an animation ended.
                if "remove" is set to true, it will remove the callback when ran. */
             void setCallbackOnEnd(CallbackFun func, bool remove = true);
@@ -80,6 +77,8 @@ namespace Hyprutils {
             /* resets all callbacks. Does not call any. */
             void resetAllCallbacks();
 
+            /* both of those may invoke callbacks and thus should never be called while looping over active animations,
+               since it is allowed to add/remove animations in callbacks. */
             void onAnimationEnd();
             void onAnimationBegin();
 
@@ -150,8 +149,6 @@ namespace Hyprutils {
 
                 m_bIsBeingAnimated = false;
 
-                onUpdate();
-
                 if (endCallback)
                     onAnimationEnd();
             }

diff --git a/src/animation/AnimatedVariable.cpp b/src/animation/AnimatedVariable.cpp
@@ -102,11 +102,6 @@ bool CBaseAnimatedVariable::ok() const {
     return m_pConfig && m_pAnimationManager;
 }
 
-void CBaseAnimatedVariable::onUpdate() {
-    if (m_bIsBeingAnimated && m_fUpdateCallback)
-        m_fUpdateCallback(m_pSelf);
-}
-
 void CBaseAnimatedVariable::setCallbackOnEnd(CallbackFun func, bool remove) {
     m_fEndCallback       = std::move(func);
     m_bRemoveEndAfterRan = remove;

diff --git a/src/animation/AnimationManager.cpp b/src/animation/AnimationManager.cpp
@@ -36,18 +36,39 @@ bool CAnimationManager::shouldTickForNext() {
     return !m_vActiveAnimatedVariables.empty();
 }
 
+struct SCallbackData {
+    CBaseAnimatedVariable::CallbackFun    callback;
+    CSharedPointer<CBaseAnimatedVariable> pav;
+};
+
 void CAnimationManager::tickDone() {
     std::vector<CWeakPointer<CBaseAnimatedVariable>> active;
+    std::vector<SCallbackData>                       callbacks;
     active.reserve(m_vActiveAnimatedVariables.size()); // avoid reallocations
+    callbacks.reserve(m_vActiveAnimatedVariables.size());
+
     for (auto const& av : m_vActiveAnimatedVariables) {
         const auto PAV = av.lock();
         if (!PAV)
             continue;
 
-        if (PAV->ok() && PAV->isBeingAnimated())
+        if (PAV->ok() && PAV->isBeingAnimated()) {
             active.emplace_back(av);
-        else
+            if (PAV->m_fUpdateCallback)
+                callbacks.emplace_back(SCallbackData{PAV->m_fUpdateCallback, PAV});
+        } else {
             PAV->m_bIsConnectedToActive = false;
+            if (PAV->m_fEndCallback)
+                callbacks.emplace_back(SCallbackData{PAV->m_fEndCallback, PAV});
+
+            if (PAV->m_bRemoveEndAfterRan)
+                PAV->m_fUpdateCallback = nullptr;
+        }
+    }
+
+    for (const auto& CBD : callbacks) {
+        // Here it is ok for callbacks to remove from or append to m_vActiveAnimatedVariables
+        CBD.callback(CBD.pav);
     }
 
     m_vActiveAnimatedVariables = std::move(active);

diff --git a/tests/animation.cpp b/tests/animation.cpp
@@ -52,7 +52,7 @@ class CMyAnimationManager : public CAnimationManager {
             const auto PBEZIER = getBezier(PAV->getBezierName());
 
             if (SPENT >= 1.f || !PAV->enabled()) {
-                PAV->warp();
+                PAV->warp(false);
                 continue;
             }
 
@@ -79,8 +79,6 @@ class CMyAnimationManager : public CAnimationManager {
                     std::cout << Colors::RED << "What are we even doing?" << Colors::RESET;
                 } break;
             }
-
-            av->onUpdate();
         }
 
         tickDone();
@@ -295,15 +293,13 @@ int main(int argc, char** argv, char** envp) {
 
     // test adding / removing vars during a tick
     s.m_iA->resetAllCallbacks();
-    s.m_iA->setUpdateCallback([&s, &vars](auto) {
-        std::erase_if(gAnimationManager.m_vActiveAnimatedVariables, [&](const auto& other) { return other.get() == vars.back().get(); });
+    s.m_iA->setUpdateCallback([&vars](auto) {
+        if (vars.empty())
+            return;
+        std::erase_if(gAnimationManager.m_vActiveAnimatedVariables, [&](const auto& other) { return !other || other.get() == vars.back().get(); });
         vars.pop_back();
     });
-    s.m_iA->setCallbackOnEnd([&s, &vars](auto) {
-        vars.resize(vars.size() + 1);
-        gAnimationManager.createAnimation(1, vars.back(), "default");
-        *vars.back() = 1337;
-    });
+    s.m_iA->setCallbackOnEnd([&s](auto) { *s.m_iB = 1337; });
 
     *s.m_iA = 1000000;
 
@@ -312,7 +308,7 @@ int main(int argc, char** argv, char** envp) {
     }
 
     EXPECT(s.m_iA->value(), 1000000);
-    // all vars should be set to 1337
+    EXPECT(s.m_iB->value(), 1337);
     EXPECT(std::find_if(vars.begin(), vars.end(), [](const auto& v) { return v->value() != 1337; }) == vars.end(), true);
 
     // test one-time callbacks