fix spilled time for animations with speed applied to them

when calculating the spilled time of an animation, we were not accounting for their speed, so it would calculate the remaining time incorrectly.
It could end up returning a value larger than the elapsed time causing an exponential time loop.

Diffs=
58a9574ce fix spilled time for animations with speed applied to them (#7630)

Co-authored-by: hernan <hernan@rive.app>

.rive_head

@@ -1 +1 @@
-49cabe3cbd9c9683bd704e0d72a455d698bfe061
+58a9574ce1a8fd45a85ba8118ec14fa8fb0b4a22

src/animation/linear_animation_instance.cpp

@@ -86,14 +86,23 @@ bool LinearAnimationInstance::advance(float elapsedSeconds, KeyedCallbackReporte
         case Loop::oneShot:
             if (direction == 1 && frames > end)
             {
-                m_spilledTime = (frames - end) / fps;
+                // Account for the time dilation or contraction applied in the
+                // animation local time by its speed to calculate spilled time.
+                // Calculate the ratio of the time excess by the total elapsed
+                // time in local time (deltaFrames) and multiply the elapsed time
+                // by it.
+                auto deltaFrames = deltaSeconds * fps;
+                auto spilledFramesRatio = (frames - end) / deltaFrames;
+                m_spilledTime = spilledFramesRatio * elapsedSeconds;
                 frames = (float)end;
                 m_time = frames / fps;
                 didLoop = true;
             }
             else if (direction == -1 && frames < start)
             {
-                m_spilledTime = (start - frames) / fps;
+                auto deltaFrames = std::abs(deltaSeconds * fps);
+                auto spilledFramesRatio = (start - frames) / deltaFrames;
+                m_spilledTime = spilledFramesRatio * elapsedSeconds;
                 frames = (float)start;
                 m_time = frames / fps;
                 didLoop = true;
@@ -102,9 +111,18 @@ bool LinearAnimationInstance::advance(float elapsedSeconds, KeyedCallbackReporte
         case Loop::loop:
             if (direction == 1 && frames >= end)
             {
-                m_spilledTime = (frames - end) / fps;
-                frames = m_time * fps;
-                frames = start + std::fmod(frames - start, (float)range);
+                // How spilled time has to be calculated, given that local time can be scaled
+                // to a factor of the regular time:
+                // - for convenience, calculate the local elapsed time in frames (deltaFrames)
+                // - get the remainder of current frame position (frames) by duration (range)
+                // - use that remainder as the ratio of the original time that was not consumed
+                // by the loop (spilledFramesRatio)
+                // - multiply the original elapsedTime by the ratio to set the spilled time
+                auto deltaFrames = deltaSeconds * fps;
+                auto remainder = std::fmod(frames - start, (float)range);
+                auto spilledFramesRatio = remainder / deltaFrames;
+                m_spilledTime = spilledFramesRatio * elapsedSeconds;
+                frames = start + remainder;
                 m_time = frames / fps;
                 didLoop = true;
                 if (reporter != nullptr)
@@ -114,9 +132,11 @@ bool LinearAnimationInstance::advance(float elapsedSeconds, KeyedCallbackReporte
             }
             else if (direction == -1 && frames <= start)
             {
-                m_spilledTime = (start - frames) / fps;
-                frames = m_time * fps;
-                frames = end - std::abs(std::fmod(start - frames, (float)range));
+                auto deltaFrames = deltaSeconds * fps;
+                auto remainder = std::abs(std::fmod(start - frames, (float)range));
+                auto spilledFramesRatio = std::abs(remainder / deltaFrames);
+                m_spilledTime = spilledFramesRatio * elapsedSeconds;
+                frames = end - remainder;
                 m_time = frames / fps;
                 didLoop = true;
                 if (reporter != nullptr)

test/animation_state_instance_test.cpp

@@ -256,6 +256,243 @@ TEST_CASE("AnimationStateInstance with negative speed starts a negative animatio
     // backwards 2 seconds from 5.
     REQUIRE(animationStateInstance->animationInstance()->time() == 0.0);
 
+    delete animationStateInstance;
+    delete animationState;
+    delete linearAnimation;
+}
+
+TEST_CASE("AnimationStateInstance spilledTime accounts for Nx speed with oneShot", "[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::StateMachine machine;
+    rive::StateMachineInstance stateMachineInstance(&machine, abi.get());
+
+    rive::LinearAnimation* linearAnimation = new rive::LinearAnimation();
+    // duration in seconds is 2
+    linearAnimation->duration(4);
+    linearAnimation->fps(2);
+    linearAnimation->speed(2);
+    linearAnimation->loopValue(static_cast<int>(rive::Loop::oneShot));
+
+    rive::AnimationState* animationState = new rive::AnimationState();
+    animationState->animation(linearAnimation);
+
+    rive::AnimationStateInstance* animationStateInstance =
+        new rive::AnimationStateInstance(animationState, abi.get());
+
+    // play from beginning.
+    animationStateInstance->advance(3.0, &stateMachineInstance);
+
+    REQUIRE(animationStateInstance->animationInstance()->time() == 2.0);
+    REQUIRE(animationStateInstance->animationInstance()->totalTime() == 6.0);
+    // Duration is 2s but at a 2x speed it takes 1s to end
+    // When advancing 3s, there are still 2s remaining (spilled)
+    REQUIRE(animationStateInstance->animationInstance()->spilledTime() == 2.0);
+
+    delete animationStateInstance;
+    delete animationState;
+    delete linearAnimation;
+}
+
+TEST_CASE("AnimationStateInstance spilledTime accounts for 1/Nx speed with oneShot", "[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::StateMachine machine;
+    rive::StateMachineInstance stateMachineInstance(&machine, abi.get());
+
+    rive::LinearAnimation* linearAnimation = new rive::LinearAnimation();
+    // duration in seconds is 2
+    linearAnimation->duration(4);
+    linearAnimation->fps(2);
+    linearAnimation->speed(0.5);
+    linearAnimation->loopValue(static_cast<int>(rive::Loop::oneShot));
+
+    rive::AnimationState* animationState = new rive::AnimationState();
+    animationState->animation(linearAnimation);
+
+    rive::AnimationStateInstance* animationStateInstance =
+        new rive::AnimationStateInstance(animationState, abi.get());
+
+    // play from beginning.
+    animationStateInstance->advance(5.0, &stateMachineInstance);
+
+    REQUIRE(animationStateInstance->animationInstance()->time() == 2.0);
+    REQUIRE(animationStateInstance->animationInstance()->totalTime() == 2.5);
+    // Duration is 2s but at a 0.5x speed it takes 4s to end
+    // When advancing 5.0s, there are still 1s remaining (spilled)
+    REQUIRE(animationStateInstance->animationInstance()->spilledTime() == 1.0);
+
+    delete animationStateInstance;
+    delete animationState;
+    delete linearAnimation;
+}
+
+TEST_CASE("AnimationStateInstance spilledTime accounts for Nx speed with loop", "[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::StateMachine machine;
+    rive::StateMachineInstance stateMachineInstance(&machine, abi.get());
+
+    rive::LinearAnimation* linearAnimation = new rive::LinearAnimation();
+    // duration in seconds is 2
+    linearAnimation->duration(4);
+    linearAnimation->fps(2);
+    linearAnimation->speed(2);
+    linearAnimation->loopValue(static_cast<int>(rive::Loop::loop));
+
+    rive::AnimationState* animationState = new rive::AnimationState();
+    animationState->animation(linearAnimation);
+
+    rive::AnimationStateInstance* animationStateInstance =
+        new rive::AnimationStateInstance(animationState, abi.get());
+
+    // play from beginning.
+    animationStateInstance->advance(5.5, &stateMachineInstance);
+
+    REQUIRE(animationStateInstance->animationInstance()->time() == 1.0);
+    REQUIRE(animationStateInstance->animationInstance()->totalTime() == 11.0);
+    // Duration is 2s but at a 2x speed it takes 1s to loop
+    // When advancing 5.5s, there is still 0.5s remaining (spilled)
+    REQUIRE(animationStateInstance->animationInstance()->spilledTime() == 0.5);
+
+    delete animationStateInstance;
+    delete animationState;
+    delete linearAnimation;
+}
+
+TEST_CASE("AnimationStateInstance spilledTime accounts for 1/Nx speed with loop", "[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::StateMachine machine;
+    rive::StateMachineInstance stateMachineInstance(&machine, abi.get());
+
+    rive::LinearAnimation* linearAnimation = new rive::LinearAnimation();
+    // duration in seconds is 2
+    linearAnimation->duration(4);
+    linearAnimation->fps(2);
+    linearAnimation->speed(0.5);
+    linearAnimation->loopValue(static_cast<int>(rive::Loop::loop));
+
+    rive::AnimationState* animationState = new rive::AnimationState();
+    animationState->animation(linearAnimation);
+
+    rive::AnimationStateInstance* animationStateInstance =
+        new rive::AnimationStateInstance(animationState, abi.get());
+
+    // play from beginning.
+    animationStateInstance->advance(10.0, &stateMachineInstance);
+
+    REQUIRE(animationStateInstance->animationInstance()->time() == 1.0);
+    REQUIRE(animationStateInstance->animationInstance()->totalTime() == 5.0);
+    // Duration is 2s but at a 2x speed it takes 1s to loop
+    // When advancing 5.5s, there is still 0.5s remaining (spilled)
+    REQUIRE(animationStateInstance->animationInstance()->spilledTime() == 2.0);
+
+    delete animationStateInstance;
+    delete animationState;
+    delete linearAnimation;
+}
+
+TEST_CASE("AnimationStateInstance spilledTime accounts for -Nx speed with oneShot", "[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::StateMachine machine;
+    rive::StateMachineInstance stateMachineInstance(&machine, abi.get());
+
+    rive::LinearAnimation* linearAnimation = new rive::LinearAnimation();
+    // duration in seconds is 2
+    linearAnimation->duration(4);
+    linearAnimation->fps(2);
+    linearAnimation->speed(-2);
+    linearAnimation->loopValue(static_cast<int>(rive::Loop::oneShot));
+
+    rive::AnimationState* animationState = new rive::AnimationState();
+    animationState->animation(linearAnimation);
+
+    rive::AnimationStateInstance* animationStateInstance =
+        new rive::AnimationStateInstance(animationState, abi.get());
+
+    // play from beginning.
+    animationStateInstance->advance(3.0, &stateMachineInstance);
+
+    REQUIRE(animationStateInstance->animationInstance()->time() == 0.0);
+    REQUIRE(animationStateInstance->animationInstance()->totalTime() == 6.0);
+    // Duration is 2s but at a -2x speed it takes 1s to end
+    // When advancing at negative speed, time starts at duration
+    // so starting at end and taking 1s to complete
+    // there are still 2s remaining (spilled)
+    REQUIRE(animationStateInstance->animationInstance()->spilledTime() == 2.0);
+
+    delete animationStateInstance;
+    delete animationState;
+    delete linearAnimation;
+}
+
+TEST_CASE("AnimationStateInstance spilledTime accounts for -Nx speed with loop", "[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::StateMachine machine;
+    rive::StateMachineInstance stateMachineInstance(&machine, abi.get());
+
+    rive::LinearAnimation* linearAnimation = new rive::LinearAnimation();
+    // duration in seconds is 2
+    linearAnimation->duration(4);
+    linearAnimation->fps(2);
+    linearAnimation->speed(-2);
+    linearAnimation->loopValue(static_cast<int>(rive::Loop::loop));
+
+    rive::AnimationState* animationState = new rive::AnimationState();
+    animationState->animation(linearAnimation);
+
+    rive::AnimationStateInstance* animationStateInstance =
+        new rive::AnimationStateInstance(animationState, abi.get());
+
+    // play from beginning.
+    animationStateInstance->advance(5.5, &stateMachineInstance);
+
+    REQUIRE(animationStateInstance->animationInstance()->time() == 1.0);
+    REQUIRE(animationStateInstance->animationInstance()->totalTime() == 11.0);
+    // Duration is 2s but at a -2x speed it takes 1s to end
+    // When advancing at negative speed, time starts at duration
+    // so starting at end and taking 1s to complete, it loops 5 times
+    // there is still 0.5s remaining (spilled)
+    REQUIRE(animationStateInstance->animationInstance()->spilledTime() == 0.5);
+
     delete animationStateInstance;
     delete animationState;
     delete linearAnimation;