From 71844cbde124a70249ab7f583feaa2115122b435 Mon Sep 17 00:00:00 2001 From: Maximilian Seidler Date: Tue, 7 Jan 2025 19:02:46 +0100 Subject: [PATCH] animation: let callbacks have SP's to the self variable and dispatch them internally To be absolutely sure that adding/removing callbacks can't cause problems, dispatch them in tickDone outside of the loop. I think looping over all the vars twice is still better than having disconnectFromActive handle the removal, because that needs to search for each variable in the active vector. --- .../hyprutils/animation/AnimatedVariable.hpp | 2 +- .../hyprutils/animation/AnimationManager.hpp | 1 + src/animation/AnimatedVariable.cpp | 24 +++++++++++-------- src/animation/AnimationManager.cpp | 21 ++++++++++++++++ tests/animation.cpp | 21 +++++++++------- 5 files changed, 49 insertions(+), 20 deletions(-) diff --git a/include/hyprutils/animation/AnimatedVariable.hpp b/include/hyprutils/animation/AnimatedVariable.hpp index 346a733..1578321 100644 --- a/include/hyprutils/animation/AnimatedVariable.hpp +++ b/include/hyprutils/animation/AnimatedVariable.hpp @@ -14,7 +14,7 @@ namespace Hyprutils { /* A base class for animated variables. */ class CBaseAnimatedVariable { public: - using CallbackFun = std::function thisptr)>; + using CallbackFun = std::function thisptr)>; CBaseAnimatedVariable() { ; // m_bDummy = true; diff --git a/include/hyprutils/animation/AnimationManager.hpp b/include/hyprutils/animation/AnimationManager.hpp index 9cb7e65..60cafb8 100644 --- a/include/hyprutils/animation/AnimationManager.hpp +++ b/include/hyprutils/animation/AnimationManager.hpp @@ -14,6 +14,7 @@ namespace Hyprutils { class CAnimationManager { public: CAnimationManager(); + virtual ~CAnimationManager() = default; void tickDone(); bool shouldTickForNext(); diff --git a/src/animation/AnimatedVariable.cpp b/src/animation/AnimatedVariable.cpp index 05c181b..501c960 100644 --- a/src/animation/AnimatedVariable.cpp +++ b/src/animation/AnimatedVariable.cpp @@ -103,8 +103,11 @@ bool CBaseAnimatedVariable::ok() const { } void CBaseAnimatedVariable::onUpdate() { - if (m_bIsBeingAnimated && m_fUpdateCallback) - m_fUpdateCallback(m_pSelf); + if (!m_bIsBeingAnimated || !m_fUpdateCallback) + return; + + if (const auto SELF = m_pSelf.lock(); SELF) + m_fUpdateCallback(SELF); } void CBaseAnimatedVariable::setCallbackOnEnd(CallbackFun func, bool remove) { @@ -133,14 +136,15 @@ void CBaseAnimatedVariable::resetAllCallbacks() { } void CBaseAnimatedVariable::onAnimationEnd() { - m_bIsBeingAnimated = false; /* We do not call disconnectFromActive here. The animation manager will remove it on a call to tickDone. */ + m_bIsBeingAnimated = false; + + if (m_bIsConnectedToActive) // callback will we handled by tickDone + return; - if (m_fEndCallback) { - /* loading m_bRemoveEndAfterRan before calling the callback allows the callback to delete this animation safely if it is false. */ - auto removeEndCallback = m_bRemoveEndAfterRan; - m_fEndCallback(m_pSelf); - if (removeEndCallback) + if (const auto SELF = m_pSelf.lock(); SELF && m_fEndCallback) { + m_fEndCallback(SELF); + if (m_bRemoveEndAfterRan) m_fEndCallback = nullptr; // reset } } @@ -150,8 +154,8 @@ void CBaseAnimatedVariable::onAnimationBegin() { animationBegin = std::chrono::steady_clock::now(); connectToActive(); - if (m_fBeginCallback) { - m_fBeginCallback(m_pSelf); + if (const auto SELF = m_pSelf.lock(); SELF && m_fBeginCallback) { + m_fBeginCallback(SELF); if (m_bRemoveBeginAfterRan) m_fBeginCallback = nullptr; // reset } diff --git a/src/animation/AnimationManager.cpp b/src/animation/AnimationManager.cpp index b020dda..c710c91 100644 --- a/src/animation/AnimationManager.cpp +++ b/src/animation/AnimationManager.cpp @@ -37,6 +37,27 @@ bool CAnimationManager::shouldTickForNext() { } void CAnimationManager::tickDone() { + std::vector> callbacks; + 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->m_fUpdateCallback) + callbacks.emplace_back(std::bind(PAV->m_fUpdateCallback, PAV)); + } else { + if (PAV->m_fEndCallback) + callbacks.emplace_back(std::bind(PAV->m_fEndCallback, PAV)); + if (PAV->m_bRemoveEndAfterRan) + PAV->m_fEndCallback = nullptr; + } + } + + for (const auto& cb : callbacks) + cb(); + std::vector> active; active.reserve(m_vActiveAnimatedVariables.size()); // avoid reallocations for (auto const& av : m_vActiveAnimatedVariables) { diff --git a/tests/animation.cpp b/tests/animation.cpp index bedbadc..a6be39f 100644 --- a/tests/animation.cpp +++ b/tests/animation.cpp @@ -43,8 +43,8 @@ CAnimationConfigTree animationTree; class CMyAnimationManager : public CAnimationManager { public: void tick() { - for (size_t i = 0; i < m_vActiveAnimatedVariables.size(); i++) { - const auto PAV = m_vActiveAnimatedVariables[i].lock(); + for (const auto& av : m_vActiveAnimatedVariables) { + const auto PAV = av.lock(); if (!PAV || !PAV->ok()) continue; @@ -79,8 +79,6 @@ class CMyAnimationManager : public CAnimationManager { std::cout << Colors::RED << "What are we even doing?" << Colors::RESET; } break; } - - PAV->onUpdate(); } tickDone(); @@ -263,12 +261,16 @@ int main(int argc, char** argv, char** envp) { int beginCallbackRan = 0; int updateCallbackRan = 0; int endCallbackRan = 0; - s.m_iA->setCallbackOnBegin([&beginCallbackRan](WP pav) { beginCallbackRan++; }); - s.m_iA->setUpdateCallback([&updateCallbackRan](WP pav) { updateCallbackRan++; }); - s.m_iA->setCallbackOnEnd([&endCallbackRan](WP pav) { endCallbackRan++; }, false); + s.m_iA->setCallbackOnBegin([&beginCallbackRan](SP pav) { beginCallbackRan++; }, false); + s.m_iA->setUpdateCallback([&updateCallbackRan](SP pav) { updateCallbackRan++; }); + s.m_iA->setCallbackOnEnd([&endCallbackRan](SP pav) { endCallbackRan++; }, false); + + EXPECT(gAnimationManager.shouldTickForNext(), false); s.m_iA->setValueAndWarp(42); + EXPECT(gAnimationManager.shouldTickForNext(), false); + EXPECT(beginCallbackRan, 0); EXPECT(updateCallbackRan, 1); EXPECT(endCallbackRan, 2); // first called when setting the callback, then when warping. @@ -291,8 +293,8 @@ int main(int argc, char** argv, char** envp) { // test adding / removing vars during a tick s.m_iA->resetAllCallbacks(); - s.m_iA->setUpdateCallback([&vars](WP v) { - if (v.lock() != vars.back()) + s.m_iA->setUpdateCallback([&vars](SP v) { + if (v != vars.back()) vars.back()->warp(); }); s.m_iA->setCallbackOnEnd([&s, &vars](auto) { @@ -310,6 +312,7 @@ int main(int argc, char** argv, char** envp) { EXPECT(s.m_iA->value(), 1000000); // all vars should be set to 1337 EXPECT(std::find_if(vars.begin(), vars.end(), [](const auto& v) { return v->value() != 1337; }) == vars.end(), true); + EXPECT(endCallbackRan, 3); // test one-time callbacks s.m_iA->resetAllCallbacks();