[runtimes] AnimationState improvements

[NathanSweet]

From @NathanSweet on May 7, 2016 16:14

• 1. Bug: spine-csharp fires onComplete in update.
• 2. Mix unkeyed properties to/from setup pose:
• track 0, animation 1, keys a
• track 0, animation 2, keys b
  Result: a is left in the state it was when the animation changed, b snaps into position.
  Expected: a mixes back to the setup pose, b mixes from the setup pose.
  Solution: Need a TrackEntry setting for animations to mix to/from the setup pose during mixing.
• 3. Attachment keys on different tracks:
• track 0, animation 1, keys attachment a
• track 1, animation 2, keys attachment b for the same slot
  Result: animation 1's attachment is shown if a key is after the b key.
  Expected: highest track attachment is shown.
  Solution: Don't use lastTime in AttachmentTimeline. Store attachment name for each slot to avoid excessive skin lookups, then get the attachment from the skeleton/skin lazily. If name is the same, we can't avoid the skin lookup entirely (eg the skin may have changed).
• 4. Firing events during mixing:
• track 0, animation 1, keys events
• track 0, animation 2
  Result: Animation 1 never fires events during mixing.
  Expected: Events for previous animation can be enabled or disabled as needed.
  Solution: TrackEntry "event threshold" setting for what percentage of the mix is required for events to be fired. Could a single threshold setting for events and attachment keys (12 below).
• 5. Mix any number of animations:
• track 0, animation 1
• track 0, animation 2
• track 0, animation 3
  Result: With a long mix time, animation 1 disappears when animation 3 starts.
  Expected: All animations are mixed.
  Solution: Support mixing any number of animations. Mixing is order dependent.
• 6. Mix to/from setup pose on empty track:
• track 0, run animation
• track 1, aim animation
  Result: Aim animation mix starts at 1 because there is no previous animation to mix from.
  Expected: Aim animation should be mixed in.
  Solution: Currently have TrackEntry.mix but no way for it to change over time.
• 7. Time is lost for each animation change: [All runtimes] Should remaining delta time carry over to queued animations delta? #154
• track 0, animation 1
• track 0, animation 2
• track 0, animation 3
  Result: A small amount of time is lost at each animation change, as it takes a frame to change animations.
  Expected: Sum of elapsed time == sum of animation durations.
  Solution: Carry any leftover time from last animation to next.
• 8. Calling setAnimation from events crashes:
• track 0, animation 1
• end: track 0, animation 2
  Result: Calling setAnimation from end event causes stackoverflow.
  Expected: No crash.
  Solution: Defer events so changes in callbacks can't corrupt internal state.
• 9. Negative scale for flip makes for unwanted transition across zero:
• track 0, animation 1, scaleX 1
• track 0, animation 2, scaleX -1.2
  Result: scaleX mixes from 1 to -1.2.
  Expected: Depending on the animation, scaleX should either transition from 1 to -1.2 or instantly flip to negative scale.
  Solution: TrackEntry setting that causes scaleX to be either -1.2 (not mix at all) or mix from -1 to -1.2 (mix but use sign from animation 2). Could also have a threshold similar to events to define at what mix percentage is the scale sign is taken from animation 2.
• 10. Rotation during mix may take the wrong direction:
• track 0, animation 1, rotation 0
• track 0, animation 2, rotation 170
  Result: Rotation mixes from 0 to 170.
  Expected: Depending on the skeleton, rotation may need to take the other direction, eg to avoid a shoulder rotating unrealistically.
  Solution: Additive mixing would preserve the rotation directions of the animations being mixed, but has other problems such as higher tracks mixing with lower ones. Possibly we could provide an angle that rotation mixing should not cross, but this seems like a complex solution.
• 11. Events are not fired in order:
  Expected: When animations are interrupted, events are not always fired in a sensible order. Eg, all events are fired after complete, even if some events were from the last few frames of the animation and some from the first few frames.
  Solution: Add interrupt event. Define and enforce the order events are fired using an event queue.
• 12. Attachment keys during mixing:
• track 0, animation 1, keys attachment visibility
• track 0, animation 2
  Result: Animation 1 changes attachment visibility during mixing.
  Expected: Attachment visibility changes for previous animation can be enabled or disabled as needed.
  Solution: TrackEntry "attachment visibility threshold" setting for what percentage of the mix is required for attachment visibility to be changed. Could be a single threshold setting for events (4 above) and attachment keys.
• 13. TrackEntry mix is a confusing name:
  Result: TrackEntry mix (how much of this animation is applied) has nothing to do with mixTime or mixDuration (crossfading from previous animation to this animation).
  Solution: Rename to alpha.
• 14. Draw order keys during mixing:
  Solution: Similar to 4 and 12 above, draw order timelines for animations being mixed out should use a threshold setting so changes after the new animation are set can be disallowed.
• 15. Rotation jumps around during mixing:
  See this crude image.
  1. The animation being mixed out is rotating the bone.
  2. The new animation has the bone keyed pointing down.
  3. The result is a mix of 1 and 2.
  4. The animation being mixed continues rotating the bone.
  5. The new animation still has the bone keyed pointing down.
  6. The result is a mix of 4 and 5, but the shortest rotation now results in the bone jumping to the other side.
  Solution: One "fix" is to not update the trackTime of the mixingFrom track. This usually looks OK for short mix times. Any other ideas?

Copied from original issue: badlogic/spine-internal#49

[NathanSweet]

This is partially done for spine-libgdx, including unit tests. Tabled until #48 is done.

[NathanSweet]

Testing #github Slack channel!

[NathanSweet]

From @pharan on May 7, 2016 20:26

(I want to see how discussions work/look here.)

Case 3 (or a related case) should include:
track 0, animation 1 keys attachment for slot a
track 0, animation 2 keys different attachment for slot a
result: when there's mixing/crossfading, animation 1's attachment may be shown
expected: attachment in animation 2 is shown

Probably the same fix as case 3. But this might be also be important distinction.

[NathanSweet]

From @pharan on May 18, 2016 3:45

2 forum users recently looked for case 8 and this case:

track 0, animation 1 playing in a loop
track 1, animation 2 play once.
result: animation 2 starts abruptly and ends abruptly.
expected: if mix is set, animation 2 should crossfade in and out.

I think this is case 2?

In light of all this "mix" stuff, I also propose disambiguating some of the terminology.

At least TrackEntry.mix would be nicer as TrackEntry.alpha so it's not confused (to be related) with the process of "mixing" between next and previous. (ie, .mix has nothing to do with trackEntry.mixTime and trackEntry.mixDuration.)

Just for its clarity, I'm partial to the term "crossfade" to refer to the process of fading between previous and next animations. But I also think it looks a bit weird in the API, so I could go for something else.

I agree that "Mix" is a good term for the process that would otherwise be known as "Apply, but with Alpha against the current values".

[NathanSweet]

I believe those cases are described above.

I've rewritten the cases in a better format and checked the ones that are complete in the dev branch.

TrackEntry.alpha is OK, though it breaks existing code for maybe little gain.

[Xelnath]

This stuff is great. Really looking forward to #2!!

[pharan]

Linking this topic since we talked about threshold values and AttachmentTimelines during mixing, (but only events during mixing is listed).

http://esotericsoftware.com/forum/Libgdx-Attachment-Bug-Fix-Proposal-6614
(topic might move. will change this reply when the topic is moved)

[pharan]

Clarification on addAnimation behavior when used on an empty track.

Current implementation code:
https://github.com/EsotericSoftware/spine-runtimes/blob/master/spine-libgdx/spine-libgdx/src/com/esotericsoftware/spine/AnimationState.java#L254

case:
Subscribe to AnimationState Start event.
Call addAnimation for an empty track (0 delay)
result: doesn't fire the Start event.
expected: it should fire the Start event.

related case:
Call addAnimation for an empty track with a delay > 0. (try passing 5)
result: animation plays immediately
expected: should wait the passed number of seconds before starting?

[badlogic]

See also:

#634
#626
#593
#535

[pharan]

BinaryCats reported this weird issue. Makes sense to me, though it might be super specific.
http://esotericsoftware.com/forum/Annoying-SetAnimation-Quirk-6740

case:
track 0: empty
Call setAnimation twice (two different animations) on the same track on the same frame.
result: Two animations get mixed.
expected: First animation should be ignored/discarded.

This is also a consistency thing: The expected behavior is how it would act if the track was not empty, because of this code in AnimationState.setCurrent:

    // If a mix is in progress, mix from the closest animation.
    if (previous != null && current.mixTime / current.mixDuration < 0.5f) {
        entry.previous = previous;
        previous = current;
    } else
        entry.previous = current;

https://github.com/EsotericSoftware/spine-runtimes/blob/master/spine-libgdx/spine-libgdx/src/com/esotericsoftware/spine/AnimationState.java#L194-L199
I think it's interesting because it gives extra context to this behavior too.

bcats fix: add current.time == 0 in the if's condition.

[NathanSweet]

I believe all of the comments and linked issues above have been handled.

[pharan]

I think 6 may be three separate issues/mislabeled.

It would be:

• 6.1 Mix to/from empty track (multi-track):
  track 0, run animation, keys aim timelines.
  track 1, aim animation
• 6.2 Mix to/from empty track (multi-track, from setup pose):
  track 0, run animation, doesn't key aim timelines.
  track 1, aim animation

Result: Aim animation is immediately applied with alpha: 1. There is no previous TrackEntry to mix from.
Expected: Aim animation should be mixed in.

I think:

• 6.1 requires implementing fading-in a TrackEntry where TrackEntry.mixingFrom can be empty and applyMixingFrom does nothing.
• I guess nothing about AnimationState currently defines fading-out a TrackEntry to nothing.
• 6.2 requires solving 6.1 and case 2 (Mix unkeyed properties to/from setup pose).

---

• 6.3 Mix to/from setup pose for an empty track. (single track):
  track 0, empty.
  
  call setAnimation(0, "run", true);
  

Result: "run" animation immediately starts because there is no previous animation to mix from.
Expected: "run" animation should transition from and to setup pose.

I think this case is mostly undefinable and less important (users haven't had problems with this/nobody's looking for it), but may also be solved if 6.2 is solved.

[Xelnath]

Pharan's understanding matches my own. I've had scenarios where an upper body animation is playing on another track to override the full body idle or run animation on the arms and you want to blend back into it.

[pharan]

Idea for case 2:

• Have a special "reset track", conceptually below track 0; The reset track is "applied" before any of the actual tracks.
• To apply the reset track: iterate through all of getCurrent(x) and check if:
  • animation mix argument < 1. (if in the middle of a crossfade or TrackEntry.alpha < 1) and
  • a new user-settable bool like mixWithSetupPose.
• When the correct conditions are true for that TrackEntry (mixWithSetupPose && animationMixArgument < 1), use the TrackEntry.animation to reset.
• "Use the TrackEntry.animation to reset" means set the target properties of all the timelines in that Animation to their data value:
  eg. RotateTimelines should cause its target bone.rotation to become bone.data.rotation.

pros:

• doing all the resetting before track 0 makes it work with multi-track setups.
• bool mixWithSetupPose makes it optional.
• checking the resolved animation mix argument means there won't be unnecessary resetting when the animations are applied with alpha: 1.
• using the Animations to determine what properties to reset means it only resets the things that need to be reset, but also ensures the correct ones are reset, rather than using skeleton.setToSetupPose which resets almost everything but also not really.
• helps solve multi-track case 6.

cons:

• have to iterate through all tracks. maybe not a big deal.
• some/many target properties may be reset more than once for timelines from different animations that target the same things. Still better than skeleton.setToSetupPose.
• have to dereference timelines and timeline targets extra times. more memory fetching? maybe a big deal?

[pharan]

The above idea for case 2 is different from my current implementation of the modified AnimationState. That just did the reset as tracks were applied and had no bool per TrackEntry.

I think users should have the convenience of just setting a bool on the AnimationState so this resetting behavior is applied with all setAnimation/addAnimation calls. From what I can tell, this is the most common case. And the hands-off case is for users who plan complex animations.

[NathanSweet]

Lots of work has been done in the dev-setupPose branch, for the above and other niceties:
https://github.com/EsotericSoftware/spine-runtimes/tree/dev-setupPose

Assuming what has been done so far properly solves all the uses cases, what we have left is 5, 6 (including @pharan's expansion), and 10.

5 is probably pretty straightforward, we'll keep a linked list of animations being mixed out. We already have a next field that can be repurposed for this, since animations being mixed out don't otherwise have a next animation.

I have a feeling 6 may be best implemented using an empty animation. Eg:

setAnimation(0, null, false);
addAnimation(0, "run", false, 0).setMixDuration(0.1f);
setAnimation(0, null, false).setMixDuration(0.2f);

This would result in the run animation mixing in from the setup pose for 0.1 seconds, playing, then mixing out for 0.2 seconds. This gives explicit control in the usual way over how mixing happens or doesn't happen, which are both valid use cases. Internally an empty animation is used, so we don't litter the code with null checks. This is already being used for the new resetTrack methods.

10 is maybe the hardest, as there is no way to know which direction is the correct rotation. Additive mixing doesn't appear to be a solution, since it doesn't handle layering and mixing animations. The only real solution is likely for bones to describe some limits to their rotations that can be respected when mixing. This is beyond the scope of this AnimationState fix up effort, so 10 is likely to be pushed to a later date.

[NathanSweet]

The empty animation approach is committed to the dev-setupPose branch. Added setEmptyAnimation and addEmptyAnimation so we don't pass null as a special value. Works like this:

state.setEmptyAnimation(0, 0.1f);
state.addAnimation(0, "run", true, 0).setMixDuration(0.2f);
state.addEmptyAnimation(0, 0.3f, 4);

This sets an empty animation, which mixes to the setup pose over 0.1f (of course, that mix would only be useful if an animation was already playing). After that, it mixes from the setup pose to the run animation over 0.2 seconds. After 4 seconds of run looping, the run animation mixes to the setup pose over 0.3 seconds.

[NathanSweet]

Added 15 above, and man is it painful. We've prototyped a solution that should work, just need to get it in there now. Behold:

[badlogic]

Sniff, it's beautiful.

On Aug 28, 2016 6:08 PM, "Nathan Sweet" notifications@github.com wrote:

Added 15 above, and man is it painful. We've prototyped a solution that
should work, just need to get it in there now. Behold:

https://camo.githubusercontent.com/a7116df878cec5f5ec072ada40cbe3933c5d5ea3/687474703a2f2f6e3474652e636f6d2f782f3531322d726f746174696f6e2d68656c6c2e676966

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#621 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAfYBGfHsQ4KW8Ezq_wKjuF48PMaiI6Sks5qkbKKgaJpZM4I-1GD
.

[NathanSweet]

Solution for 15 is in the dev branch. That involved some serious pain, but look what it can do!

The green bone is mixing from red to blue while red and blue rotate crazily.

Once we solve 5 we can close this monster of an issue

[NathanSweet]

Everything we wanted to do here is now done in the dev branch, though there may still be a few bug fixes.