Fix negative speeds in state machines look commit by commit potentially, last couple of commits are running core generator & its quite a small change code wise, so it'd be good to get feedback on the actual change sooner rather than later there are at least a couple of changes in there that should make you question if this is a good idea so i expect some feedback. add to cpp: (i just got it working in the viewer, probably broke some things) - [x] negative time fix - [x] combine state speed to determine time for state - [x] no longer carry spilled time into new advances add test - [x] negative time fix to state machines - [x] negative combined time state fix to state machines - [ ] no longer carry spilled time into new advances @luigi-rosso i tried adding tests for this, but i was not really able to construct a state machine instance (with an animations, an animation state and the right transition) in tests I had to add a bunch of public methods all over the place, can you show me the way? probably not a blocker for merging this) apply changes to rive_flutter - [x] run script (ran both core generator and core generator runtime, the runtime one wanted to remove a load of comments so i didnt let those bits be committed. but still.. annoying) Diffs= bc6c6f467 Fix negative speeds in state machines (#4887)
diff --git a/.rive_head b/.rive_head index c2ac205..3727ac5 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +1 @@ -040a27c1c53dfb53ffce3a8b2503124fe1a2f068 +bc6c6f467b8f1e0f794be093a514768bd068088a
diff --git a/include/rive/animation/animation_state_instance.hpp b/include/rive/animation/animation_state_instance.hpp index 379f5d3..689b8ce 100644 --- a/include/rive/animation/animation_state_instance.hpp +++ b/include/rive/animation/animation_state_instance.hpp
@@ -23,6 +23,7 @@ void apply(float mix) override; bool keepGoing() const override; + void clearSpilledTime() override; const LinearAnimationInstance* animationInstance() const { return &m_AnimationInstance; }
diff --git a/include/rive/animation/linear_animation.hpp b/include/rive/animation/linear_animation.hpp index 309c6d7..6d8aeb5 100644 --- a/include/rive/animation/linear_animation.hpp +++ b/include/rive/animation/linear_animation.hpp
@@ -27,9 +27,14 @@ StatusCode import(ImportStack& importStack) override; + float durationSeconds() const; + /// Returns the start time/ end time of the animation in seconds float startSeconds() const; float endSeconds() const; - float durationSeconds() const; + + /// Returns the start time/ end time of the animation in seconds, considering speed + float startTime() const; + float endTime() const; /// Convert a global clock to local seconds (takes into consideration /// work area start/end, speed, looping).
diff --git a/include/rive/animation/linear_animation_instance.hpp b/include/rive/animation/linear_animation_instance.hpp index e430995..3e645ab 100644 --- a/include/rive/animation/linear_animation_instance.hpp +++ b/include/rive/animation/linear_animation_instance.hpp
@@ -23,7 +23,7 @@ int m_LoopValue = -1; public: - LinearAnimationInstance(const LinearAnimation*, ArtboardInstance*); + LinearAnimationInstance(const LinearAnimation*, ArtboardInstance*, float speedMultiplier = 1.0); LinearAnimationInstance(LinearAnimationInstance const&); ~LinearAnimationInstance() override; @@ -31,6 +31,8 @@ // animation will continue to animate after this advance. bool advance(float seconds); + void clearSpilledTime() { m_SpilledTime = 0; } + // Returns a pointer to the instance's animation const LinearAnimation* animation() const { return m_Animation; } @@ -76,7 +78,7 @@ uint32_t fps() const; uint32_t duration() const; float speed() const; - float startSeconds() const; + float startTime() const; // Returns either the animation's default or overridden loop values Loop loop() const override { return (Loop)loopValue(); } @@ -87,6 +89,7 @@ bool isTranslucent() const override; bool advanceAndApply(float seconds) override; std::string name() const override; + void reset(float speedMultiplier); }; } // namespace rive #endif
diff --git a/include/rive/animation/state_instance.hpp b/include/rive/animation/state_instance.hpp index 3465a43..549555d 100644 --- a/include/rive/animation/state_instance.hpp +++ b/include/rive/animation/state_instance.hpp
@@ -27,6 +27,7 @@ /// Returns true when the State Machine needs to keep advancing this /// state. virtual bool keepGoing() const = 0; + virtual void clearSpilledTime() {} const LayerState* state() const; };
diff --git a/src/animation/animation_state_instance.cpp b/src/animation/animation_state_instance.cpp index 1c4063e..a2ed1ed 100644 --- a/src/animation/animation_state_instance.cpp +++ b/src/animation/animation_state_instance.cpp
@@ -15,7 +15,9 @@ // SystemStateInstance (basically a no-op StateMachine state) which would // cause bad casts in parts of the code where we assumed AnimationStates // would have create AnimationStateInstances. - m_AnimationInstance(state->animation() ? state->animation() : &emptyAnimation, instance), + m_AnimationInstance(state->animation() ? state->animation() : &emptyAnimation, + instance, + state->speed()), m_KeepGoing(true) {} @@ -28,4 +30,5 @@ void AnimationStateInstance::apply(float mix) { m_AnimationInstance.apply(mix); } -bool AnimationStateInstance::keepGoing() const { return m_KeepGoing; } \ No newline at end of file +bool AnimationStateInstance::keepGoing() const { return m_KeepGoing; } +void AnimationStateInstance::clearSpilledTime() { m_AnimationInstance.clearSpilledTime(); } \ No newline at end of file
diff --git a/src/animation/linear_animation.cpp b/src/animation/linear_animation.cpp index 3bbdc4a..f9ecb39 100644 --- a/src/animation/linear_animation.cpp +++ b/src/animation/linear_animation.cpp
@@ -3,6 +3,7 @@ #include "rive/artboard.hpp" #include "rive/importers/artboard_importer.hpp" #include "rive/importers/import_stack.hpp" +#include <cmath> using namespace rive; @@ -77,7 +78,10 @@ { return (enableWorkArea() ? workEnd() : duration()) / (float)fps(); } -float LinearAnimation::durationSeconds() const { return endSeconds() - startSeconds(); } + +float LinearAnimation::startTime() const { return (speed() >= 0) ? startSeconds() : endSeconds(); } +float LinearAnimation::endTime() const { return (speed() >= 0) ? endSeconds() : startSeconds(); } +float LinearAnimation::durationSeconds() const { return std::abs(endSeconds() - startSeconds()); } // Matches Dart modulus: https://api.dart.dev/stable/2.19.0/dart-core/double/operator_modulo.html static float positiveMod(float value, float range) @@ -96,13 +100,13 @@ switch (loop()) { case Loop::oneShot: - return seconds + startSeconds(); + return seconds + startTime(); case Loop::loop: - return positiveMod(seconds, (endSeconds() - startSeconds())) + startSeconds(); + return positiveMod(seconds, (durationSeconds())) + startTime(); case Loop::pingPong: - float localTime = positiveMod(seconds, (endSeconds() - startSeconds())); - int direction = ((int)(seconds / (endSeconds() - startSeconds()))) % 2; - return direction == 0 ? localTime + startSeconds() : endSeconds() - localTime; + float localTime = positiveMod(seconds, (durationSeconds())); + int direction = ((int)(seconds / (durationSeconds()))) % 2; + return direction == 0 ? localTime + startTime() : endTime() - localTime; } RIVE_UNREACHABLE(); } \ No newline at end of file
diff --git a/src/animation/linear_animation_instance.cpp b/src/animation/linear_animation_instance.cpp index 2e4b0f1..0e9ea17 100644 --- a/src/animation/linear_animation_instance.cpp +++ b/src/animation/linear_animation_instance.cpp
@@ -8,10 +8,11 @@ using namespace rive; LinearAnimationInstance::LinearAnimationInstance(const LinearAnimation* animation, - ArtboardInstance* instance) : + ArtboardInstance* instance, + float speedMultiplier) : Scene(instance), m_Animation((assert(animation != nullptr), animation)), - m_Time(animation->enableWorkArea() ? (float)animation->workStart() / animation->fps() : 0), + m_Time((speedMultiplier >= 0) ? animation->startTime() : animation->endTime()), m_TotalTime(0.0f), m_LastTotalTime(0.0f), m_SpilledTime(0.0f), @@ -173,13 +174,18 @@ m_Direction = 1; } +void LinearAnimationInstance::reset(float speedMultiplier = 1.0) +{ + m_Time = (speedMultiplier >= 0) ? m_Animation->startTime() : m_Animation->endTime(); +} + uint32_t LinearAnimationInstance::fps() const { return m_Animation->fps(); } uint32_t LinearAnimationInstance::duration() const { return m_Animation->duration(); } float LinearAnimationInstance::speed() const { return m_Animation->speed(); } -float LinearAnimationInstance::startSeconds() const { return m_Animation->startSeconds(); } +float LinearAnimationInstance::startTime() const { return m_Animation->startTime(); } std::string LinearAnimationInstance::name() const { return m_Animation->name(); }
diff --git a/src/animation/state_machine_instance.cpp b/src/animation/state_machine_instance.cpp index ef70627..544aa9a 100644 --- a/src/animation/state_machine_instance.cpp +++ b/src/animation/state_machine_instance.cpp
@@ -88,7 +88,7 @@ { m_StateChangedOnAdvance = false; - if (m_CurrentState != nullptr) + if (m_CurrentState != nullptr && m_CurrentState->keepGoing()) { m_CurrentState->advance(seconds, inputs); } @@ -115,6 +115,8 @@ apply(); + m_CurrentState->clearSpilledTime(); + return m_Mix != 1.0f || m_WaitingForExit || (m_CurrentState != nullptr && m_CurrentState->keepGoing()); }
diff --git a/src/animation/state_transition.cpp b/src/animation/state_transition.cpp index caffa41..5efa228 100644 --- a/src/animation/state_transition.cpp +++ b/src/animation/state_transition.cpp
@@ -99,6 +99,7 @@ auto exitAnimation = exitTimeAnimation(stateFrom); if (exitAnimation != nullptr) { + // TODO: needs a looking for speed start = absolute ? exitAnimation->startSeconds() : 0.0f; animationDuration = exitAnimation->durationSeconds(); } @@ -176,6 +177,7 @@ exitTime += std::floor(lastTime / duration) * duration; } + // TODO: needs a looking for speed if (time < exitTime) { return AllowTransition::waitingForExit;
diff --git a/test/animation_state_instance_test.cpp b/test/animation_state_instance_test.cpp index cdced64..af648d4 100644 --- a/test/animation_state_instance_test.cpp +++ b/test/animation_state_instance_test.cpp
@@ -140,3 +140,117 @@ delete animationState; delete linearAnimation; } + +TEST_CASE("AnimationStateInstance starts a positive animation at the beginning", "[animation]") +{ + rive::NoOpFactory emptyFactory; + // For each of these tests, we cons up a dummy artboard/instance + // just to make the animations happy. + rive::Artboard ab(&emptyFactory); + auto abi = ab.instance(); + + rive::LinearAnimation* linearAnimation = new rive::LinearAnimation(); + // duration in seconds is 5 + linearAnimation->duration(10); + linearAnimation->fps(2); + + rive::AnimationState* animationState = new rive::AnimationState(); + animationState->animation(linearAnimation); + + rive::AnimationStateInstance* animationStateInstance = + new rive::AnimationStateInstance(animationState, abi.get()); + + // backwards 2 seconds from 5. + REQUIRE(animationStateInstance->animationInstance()->time() == 0.0); + + delete animationStateInstance; + delete animationState; + delete linearAnimation; +} + +TEST_CASE("AnimationStateInstance starts a negative animation at the end", "[animation]") +{ + rive::NoOpFactory emptyFactory; + // For each of these tests, we cons up a dummy artboard/instance + // just to make the animations happy. + rive::Artboard ab(&emptyFactory); + auto abi = ab.instance(); + + rive::LinearAnimation* linearAnimation = new rive::LinearAnimation(); + // duration in seconds is 5 + linearAnimation->duration(10); + linearAnimation->fps(2); + linearAnimation->speed(-1); + + rive::AnimationState* animationState = new rive::AnimationState(); + animationState->animation(linearAnimation); + + rive::AnimationStateInstance* animationStateInstance = + new rive::AnimationStateInstance(animationState, abi.get()); + + // backwards 2 seconds from 5. + REQUIRE(animationStateInstance->animationInstance()->time() == 5.0); + + delete animationStateInstance; + delete animationState; + delete linearAnimation; +} + +TEST_CASE("AnimationStateInstance with negative speed starts a positive animation at the end", + "[animation]") +{ + rive::NoOpFactory emptyFactory; + // For each of these tests, we cons up a dummy artboard/instance + // just to make the animations happy. + rive::Artboard ab(&emptyFactory); + auto abi = ab.instance(); + + rive::LinearAnimation* linearAnimation = new rive::LinearAnimation(); + // duration in seconds is 5 + linearAnimation->duration(10); + linearAnimation->fps(2); + + rive::AnimationState* animationState = new rive::AnimationState(); + animationState->speed(-1); + animationState->animation(linearAnimation); + + rive::AnimationStateInstance* animationStateInstance = + new rive::AnimationStateInstance(animationState, abi.get()); + + // backwards 2 seconds from 5. + REQUIRE(animationStateInstance->animationInstance()->time() == 5.0); + + delete animationStateInstance; + delete animationState; + delete linearAnimation; +} + +TEST_CASE("AnimationStateInstance with negative speed starts a negative animation at the beginning", + "[animation]") +{ + rive::NoOpFactory emptyFactory; + // For each of these tests, we cons up a dummy artboard/instance + // just to make the animations happy. + rive::Artboard ab(&emptyFactory); + auto abi = ab.instance(); + + rive::LinearAnimation* linearAnimation = new rive::LinearAnimation(); + // duration in seconds is 5 + linearAnimation->duration(10); + linearAnimation->fps(2); + linearAnimation->speed(-1); + + rive::AnimationState* animationState = new rive::AnimationState(); + animationState->speed(-1); + animationState->animation(linearAnimation); + + rive::AnimationStateInstance* animationStateInstance = + new rive::AnimationStateInstance(animationState, abi.get()); + + // backwards 2 seconds from 5. + REQUIRE(animationStateInstance->animationInstance()->time() == 0.0); + + delete animationStateInstance; + delete animationState; + delete linearAnimation; +} \ No newline at end of file
diff --git a/test/linear_animation_test.cpp b/test/linear_animation_test.cpp new file mode 100644 index 0000000..5d61c8f --- /dev/null +++ b/test/linear_animation_test.cpp
@@ -0,0 +1,53 @@ +#include <rive/artboard.hpp> +#include <rive/animation/linear_animation.hpp> +#include "utils/no_op_factory.hpp" +#include <catch.hpp> +#include <cstdio> + +TEST_CASE("LinearAnimation with positive speed have normal start and end seconds", "[animation]") +{ + rive::NoOpFactory emptyFactory; + // For each of these tests, we cons up a dummy artboard/instance + // just to make the animations happy. + rive::Artboard ab(&emptyFactory); + auto abi = ab.instance(); + + rive::LinearAnimation* linearAnimation = new rive::LinearAnimation(); + // duration in seconds is 5 + linearAnimation->duration(10); + linearAnimation->fps(2); + linearAnimation->speed(1); + + REQUIRE(linearAnimation->startSeconds() == 0.0); + REQUIRE(linearAnimation->endSeconds() == 5.0); + + REQUIRE(linearAnimation->startTime() == 0.0); + REQUIRE(linearAnimation->endTime() == 5.0); + REQUIRE(linearAnimation->durationSeconds() == 5.0); + + delete linearAnimation; +} + +TEST_CASE("LinearAnimation with negative speed have reversed start and end seconds", "[animation]") +{ + rive::NoOpFactory emptyFactory; + // For each of these tests, we cons up a dummy artboard/instance + // just to make the animations happy. + rive::Artboard ab(&emptyFactory); + auto abi = ab.instance(); + + rive::LinearAnimation* linearAnimation = new rive::LinearAnimation(); + // duration in seconds is 5 + linearAnimation->duration(10); + linearAnimation->fps(2); + linearAnimation->speed(-1); + + REQUIRE(linearAnimation->startSeconds() == 0.0); + REQUIRE(linearAnimation->endSeconds() == 5.0); + REQUIRE(linearAnimation->startTime() == 5.0); + REQUIRE(linearAnimation->endTime() == 0.0); + + REQUIRE(linearAnimation->durationSeconds() == 5.0); + + delete linearAnimation; +}