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)
This commit is contained in:
mjtalbot
2023-02-28 11:53:47 +00:00
parent 236ae26640
commit fe57e817f3
12 changed files with 135 additions and 28 deletions

View File

@ -1 +1 @@
ffeb9afafb69c3bf34e2df8c4c0c205083f2aabf
bc6c6f467b8f1e0f794be093a514768bd068088a

View File

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

View File

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

View File

@ -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;

View File

@ -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) {}
}

View File

@ -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();
}

View File

@ -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) {

View File

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

View File

@ -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

View File

@ -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
}
}

View File

@ -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';

View File

@ -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);
}