From fe57e817f3bcbcb860e2a9ca751ce0b4c99fce04 Mon Sep 17 00:00:00 2001
From: mjtalbot <hello@rive.app>
Date: Tue, 28 Feb 2023 11:53:47 +0000
Subject: [PATCH] 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)
040a27c1c Generate Android builds directly from premake (#4871)
12285f625 Put SIMD perf warnings behind a flag (#4861)
157a399d2 update id in code (#4855)
---
.rive_head | 2 +-
.../animation/advanceable_state_base.dart | 46 +++++++++++++++++++
.../animation/animation_state_base.dart | 6 ++-
lib/src/generated/rive_core_context.dart | 14 ++++++
.../animation/advanceable_state.dart | 7 +++
.../animation/animation_state_instance.dart | 13 ++++--
.../rive_core/animation/linear_animation.dart | 17 ++++---
.../animation/linear_animation_instance.dart | 34 +++++++++-----
.../rive_core/animation/state_instance.dart | 5 +-
lib/src/rive_core/artboard.dart | 5 ++
lib/src/rive_core/shapes/image.dart | 1 -
.../rive_core/state_machine_controller.dart | 13 +++++-
12 files changed, 135 insertions(+), 28 deletions(-)
create mode 100644 lib/src/generated/animation/advanceable_state_base.dart
create mode 100644 lib/src/rive_core/animation/advanceable_state.dart
diff --git a/.rive_head b/.rive_head
index 7a3ff79..3727ac5 100644
--- a/.rive_head
+++ b/.rive_head
@@ -1 +1 @@
-ffeb9afafb69c3bf34e2df8c4c0c205083f2aabf
+bc6c6f467b8f1e0f794be093a514768bd068088a
diff --git a/lib/src/generated/animation/advanceable_state_base.dart b/lib/src/generated/animation/advanceable_state_base.dart
new file mode 100644
index 0000000..736535d
--- /dev/null
+++ b/lib/src/generated/animation/advanceable_state_base.dart
@@ -0,0 +1,46 @@
+/// Core automatically generated
+/// lib/src/generated/animation/advanceable_state_base.dart.
+/// Do not modify manually.
+
+import 'package:rive/src/generated/animation/state_machine_layer_component_base.dart';
+import 'package:rive/src/rive_core/animation/layer_state.dart';
+
+abstract class AdvanceableStateBase extends LayerState {
+ static const int typeKey = 145;
+ @override
+ int get coreType => AdvanceableStateBase.typeKey;
+ @override
+ Set<int> get coreTypes => {
+ AdvanceableStateBase.typeKey,
+ LayerStateBase.typeKey,
+ StateMachineLayerComponentBase.typeKey
+ };
+
+ /// --------------------------------------------------------------------------
+ /// Speed field with key 292.
+ static const double speedInitialValue = 1;
+ double _speed = speedInitialValue;
+ static const int speedPropertyKey = 292;
+ double get speed => _speed;
+
+ /// Change the [_speed] field value.
+ /// [speedChanged] will be invoked only if the field's value has changed.
+ set speed(double value) {
+ if (_speed == value) {
+ return;
+ }
+ double from = _speed;
+ _speed = value;
+ if (hasValidated) {
+ speedChanged(from, value);
+ }
+ }
+
+ void speedChanged(double from, double to);
+
+ @override
+ void copy(covariant AdvanceableStateBase source) {
+ super.copy(source);
+ _speed = source._speed;
+ }
+}
diff --git a/lib/src/generated/animation/animation_state_base.dart b/lib/src/generated/animation/animation_state_base.dart
index b571096..4e83f92 100644
--- a/lib/src/generated/animation/animation_state_base.dart
+++ b/lib/src/generated/animation/animation_state_base.dart
@@ -2,16 +2,18 @@
/// lib/src/generated/animation/animation_state_base.dart.
/// Do not modify manually.
+import 'package:rive/src/generated/animation/layer_state_base.dart';
import 'package:rive/src/generated/animation/state_machine_layer_component_base.dart';
-import 'package:rive/src/rive_core/animation/layer_state.dart';
+import 'package:rive/src/rive_core/animation/advanceable_state.dart';
-abstract class AnimationStateBase extends LayerState {
+abstract class AnimationStateBase extends AdvanceableState {
static const int typeKey = 61;
@override
int get coreType => AnimationStateBase.typeKey;
@override
Set<int> get coreTypes => {
AnimationStateBase.typeKey,
+ AdvanceableStateBase.typeKey,
LayerStateBase.typeKey,
StateMachineLayerComponentBase.typeKey
};
diff --git a/lib/src/generated/rive_core_context.dart b/lib/src/generated/rive_core_context.dart
index ab14603..b304fd4 100644
--- a/lib/src/generated/rive_core_context.dart
+++ b/lib/src/generated/rive_core_context.dart
@@ -6,6 +6,7 @@ import 'package:rive/src/core/field_types/core_double_type.dart';
import 'package:rive/src/core/field_types/core_field_type.dart';
import 'package:rive/src/core/field_types/core_string_type.dart';
import 'package:rive/src/core/field_types/core_uint_type.dart';
+import 'package:rive/src/generated/animation/advanceable_state_base.dart';
import 'package:rive/src/generated/animation/blend_animation_base.dart';
import 'package:rive/src/generated/animation/cubic_ease_interpolator_base.dart';
import 'package:rive/src/generated/animation/cubic_interpolator_base.dart';
@@ -567,6 +568,11 @@ class RiveCoreContext {
object.inputId = value;
}
break;
+ case AdvanceableStateBase.speedPropertyKey:
+ if (object is AdvanceableStateBase && value is double) {
+ object.speed = value;
+ }
+ break;
case AnimationStateBase.animationIdPropertyKey:
if (object is AnimationStateBase && value is int) {
object.animationId = value;
@@ -1366,6 +1372,7 @@ class RiveCoreContext {
case LinearAnimationBase.speedPropertyKey:
case NestedLinearAnimationBase.mixPropertyKey:
case NestedSimpleAnimationBase.speedPropertyKey:
+ case AdvanceableStateBase.speedPropertyKey:
case StateMachineNumberBase.valuePropertyKey:
case CubicInterpolatorBase.x1PropertyKey:
case CubicInterpolatorBase.y1PropertyKey:
@@ -1668,6 +1675,8 @@ class RiveCoreContext {
return (object as NestedLinearAnimationBase).mix;
case NestedSimpleAnimationBase.speedPropertyKey:
return (object as NestedSimpleAnimationBase).speed;
+ case AdvanceableStateBase.speedPropertyKey:
+ return (object as AdvanceableStateBase).speed;
case StateMachineNumberBase.valuePropertyKey:
return (object as StateMachineNumberBase).value;
case CubicInterpolatorBase.x1PropertyKey:
@@ -2351,6 +2360,11 @@ class RiveCoreContext {
object.speed = value;
}
break;
+ case AdvanceableStateBase.speedPropertyKey:
+ if (object is AdvanceableStateBase) {
+ object.speed = value;
+ }
+ break;
case StateMachineNumberBase.valuePropertyKey:
if (object is StateMachineNumberBase) {
object.value = value;
diff --git a/lib/src/rive_core/animation/advanceable_state.dart b/lib/src/rive_core/animation/advanceable_state.dart
new file mode 100644
index 0000000..4936773
--- /dev/null
+++ b/lib/src/rive_core/animation/advanceable_state.dart
@@ -0,0 +1,7 @@
+import 'package:rive/src/generated/animation/advanceable_state_base.dart';
+export 'package:rive/src/generated/animation/advanceable_state_base.dart';
+
+abstract class AdvanceableState extends AdvanceableStateBase {
+ @override
+ void speedChanged(double from, double to) {}
+}
diff --git a/lib/src/rive_core/animation/animation_state_instance.dart b/lib/src/rive_core/animation/animation_state_instance.dart
index 906fe0b..2de118b 100644
--- a/lib/src/rive_core/animation/animation_state_instance.dart
+++ b/lib/src/rive_core/animation/animation_state_instance.dart
@@ -5,16 +5,20 @@ import 'package:rive/src/rive_core/animation/state_instance.dart';
/// Simple wrapper around [LinearAnimationInstance] making it compatible with
/// the [StateMachine]'s [StateInstance] interface.
-class AnimationStateInstance extends StateInstance {
+class AnimationStateInstance extends StateInstance<AnimationState> {
final LinearAnimationInstance animationInstance;
AnimationStateInstance(AnimationState state)
: assert(state.animation != null),
- animationInstance = LinearAnimationInstance(state.animation!),
+ animationInstance = LinearAnimationInstance(
+ state.animation!,
+ speedMultiplier: state.speed,
+ ),
super(state);
@override
- void advance(double seconds, _) => animationInstance.advance(seconds);
+ void advance(double seconds, _) =>
+ animationInstance.advance(seconds * state.speed);
@override
void apply(CoreContext core, double mix) => animationInstance.animation
@@ -22,4 +26,7 @@ class AnimationStateInstance extends StateInstance {
@override
bool get keepGoing => animationInstance.keepGoing;
+
+ @override
+ void clearSpilledTime() => animationInstance.clearSpilledTime();
}
diff --git a/lib/src/rive_core/animation/linear_animation.dart b/lib/src/rive_core/animation/linear_animation.dart
index 772b9fa..5f40f4c 100644
--- a/lib/src/rive_core/animation/linear_animation.dart
+++ b/lib/src/rive_core/animation/linear_animation.dart
@@ -38,11 +38,22 @@ class LinearAnimation extends LinearAnimationBase {
return true;
}
+ /// Returns the seconds where the animiation work area starts
double get startSeconds => (enableWorkArea ? workStart : 0).toDouble() / fps;
+
+ /// Returns the seconds where the animiation work area ends
double get endSeconds =>
(enableWorkArea ? workEnd : duration).toDouble() / fps;
+
+ /// Returns the length of the animation
double get durationSeconds => endSeconds - startSeconds;
+ /// Returns the end time of the animation in seconds, considering speed
+ double get endTime => (speed >= 0) ? endSeconds : startSeconds;
+
+ /// Returns the start time of the animation in seconds, considering speed
+ double get startTime => (speed >= 0) ? startSeconds : endSeconds;
+
/// Pass in a different [core] context if you want to apply the animation to a
/// different instance. This isn't meant to be used yet but left as mostly a
/// note to remember that at runtime we have to support applying animations to
@@ -79,12 +90,6 @@ class LinearAnimation extends LinearAnimationBase {
@override
void workStartChanged(int from, int to) {}
- /// Returns the end time of the animation in seconds
- double get endTime => (enableWorkArea ? workEnd : duration).toDouble() / fps;
-
- /// Returns the start time of the animation in seconds
- double get startTime => (enableWorkArea ? workStart : 0).toDouble() / fps;
-
/// Convert a global clock to local seconds (takes into consideration work
/// area start/end, speed, looping).
double globalToLocalTime(double seconds) {
diff --git a/lib/src/rive_core/animation/linear_animation_instance.dart b/lib/src/rive_core/animation/linear_animation_instance.dart
index 31a4db2..dc28497 100644
--- a/lib/src/rive_core/animation/linear_animation_instance.dart
+++ b/lib/src/rive_core/animation/linear_animation_instance.dart
@@ -8,7 +8,7 @@ class LinearAnimationInstance {
double _time = 0;
double _totalTime = 0;
double _lastTotalTime = 0;
- int _direction = 1;
+ double _direction = 1;
bool _didLoop = false;
bool get didLoop => _didLoop;
double _spilledTime = 0;
@@ -17,21 +17,23 @@ class LinearAnimationInstance {
double get totalTime => _totalTime;
double get lastTotalTime => _lastTotalTime;
- LinearAnimationInstance(this.animation)
+ LinearAnimationInstance(this.animation, {double speedMultiplier = 1.0})
: _time =
- (animation.enableWorkArea ? animation.workStart : 0).toDouble() /
- animation.fps;
+ (speedMultiplier >= 0) ? animation.startTime : animation.endTime;
- /// Note that when time is set, the direction will be changed to 1
+ /// NOTE: that when time is set, the direction will be changed to 1
set time(double value) {
if (_time == value) {
return;
}
+
// Make sure to keep last and total in relative lockstep so state machines
// can track change even when setting time.
var diff = _totalTime - _lastTotalTime;
_time = _totalTime = value;
_lastTotalTime = _totalTime - diff;
+
+ // NOTE: will cause ping-pongs to get reset if "seeking"
_direction = 1;
}
@@ -39,16 +41,18 @@ class LinearAnimationInstance {
double get time => _time;
/// Direction should only be +1 or -1
- set direction(int value) => _direction = value == -1 ? -1 : 1;
+ set direction(double value) => _direction = value == -1 ? -1 : 1;
/// Returns the animation's play direction: 1 for forwards, -1 for backwards
- int get direction => _direction;
+ double get direction => _direction;
double get progress =>
- (_time - animation.startTime) / (animation.endTime - animation.startTime);
+ (_time - animation.startTime).abs() /
+ (animation.endTime - animation.startTime).abs();
/// Resets the animation to the starting frame
- void reset() => _time = animation.startTime;
+ void reset({double speedMultiplier = 1}) =>
+ _time = (speedMultiplier >= 0) ? animation.startTime : animation.endTime;
/// Whether the controller driving this animation should keep requesting
/// frames be drawn.
@@ -60,14 +64,20 @@ class LinearAnimationInstance {
animation.apply(time, coreContext: core, mix: mix);
}
+ void clearSpilledTime() {
+ _spilledTime = 0;
+ }
+
bool advance(double elapsedSeconds) {
var deltaSeconds = elapsedSeconds * animation.speed * _direction;
if (deltaSeconds == 0) {
// we say keep going, if you advance by 0.
- // could argue that any further advances by 0 result in nothing so you should not keep going
- // could argue its saying, we are not at the end of the animation yet, so keep going
- // our runtimes currently expect the latter, so we say keep going!
+ // could argue that any further advances by 0 result in nothing so you
+ // should not keep going
+ // could argue its saying, we are not at the end of the animation yet,
+ // so keep going our runtimes currently expect the latter, so we say keep
+ // going!
_didLoop = false;
return true;
}
diff --git a/lib/src/rive_core/animation/state_instance.dart b/lib/src/rive_core/animation/state_instance.dart
index 7fb032d..b48bdf2 100644
--- a/lib/src/rive_core/animation/state_instance.dart
+++ b/lib/src/rive_core/animation/state_instance.dart
@@ -6,8 +6,8 @@ import 'package:rive/src/rive_core/state_machine_controller.dart';
/// [LayerController] of a [StateMachineController]. Abstract representation of
/// an Animation (for [AnimationState]) or set of Animations in the case of a
/// [BlendState].
-abstract class StateInstance {
- final LayerState state;
+abstract class StateInstance<T extends LayerState> {
+ final T state;
StateInstance(this.state);
@@ -17,6 +17,7 @@ abstract class StateInstance {
bool get keepGoing;
void dispose() {}
+ void clearSpilledTime() {}
}
/// A single one of these is created per Layer which just represents/wraps the
diff --git a/lib/src/rive_core/artboard.dart b/lib/src/rive_core/artboard.dart
index 9b730b3..ecc9c9e 100644
--- a/lib/src/rive_core/artboard.dart
+++ b/lib/src/rive_core/artboard.dart
@@ -578,4 +578,9 @@ class Artboard extends ArtboardBase with ShapePaintContainer {
void defaultStateMachineIdChanged(int from, int to) {
defaultStateMachine = to == Core.missingId ? null : context.resolve(to);
}
+
+ @override
+ void selectedAnimationsChanged(List<int> from, List<int> to) {
+ // If the selection is different, we might have a different Editing Animation
+ }
}
diff --git a/lib/src/rive_core/shapes/image.dart b/lib/src/rive_core/shapes/image.dart
index 3eabebb..212406a 100644
--- a/lib/src/rive_core/shapes/image.dart
+++ b/lib/src/rive_core/shapes/image.dart
@@ -1,6 +1,5 @@
import 'dart:ui' as ui;
-import 'package:rive/src/core/core.dart';
import 'package:rive/src/generated/shapes/image_base.dart';
import 'package:rive/src/rive_core/assets/file_asset.dart';
import 'package:rive/src/rive_core/assets/image_asset.dart';
diff --git a/lib/src/rive_core/state_machine_controller.dart b/lib/src/rive_core/state_machine_controller.dart
index 8abd9b3..f870799 100644
--- a/lib/src/rive_core/state_machine_controller.dart
+++ b/lib/src/rive_core/state_machine_controller.dart
@@ -109,7 +109,11 @@ class LayerController {
bool apply(CoreContext core, double elapsedSeconds) {
if (_currentState != null) {
- _currentState!.advance(elapsedSeconds, controller);
+ if (_currentState?.keepGoing == true) {
+ // NOTE: if we advance even after we have been told not to keep going
+ // we are not just inappropriate we are also re adding spilled time
+ _currentState!.advance(elapsedSeconds, controller);
+ }
}
_updateMix(elapsedSeconds);
@@ -136,6 +140,13 @@ class LayerController {
_apply(core);
+ // give the current state the oportunity to clear spilled time, so that we
+ // do not carry this over into another iteration.
+ _currentState?.clearSpilledTime();
+
+ // We still need to mix in the current state if mix value is less than one
+ // as it still contributes to the end result.
+ // It may not need to advance but it does need to apply.
return _mix != 1 || _waitingForExit || (_currentState?.keepGoing ?? false);
}