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;
+}
