gotoFrame not working with additive animation

Bugs
,
1

Hello,

AnimationGroup.gotoFrame() will not work correctly for additive animations: this will also apply the animation (as it were NOT additive) to all targets.

I think this should not be done for additive animations, but I’ve not tested.

I figured out by trying to optimize my previous method, which was consuming A LOT of CPU (I need to apply an additive animation at a calculated animation frame each render call):

group.stop();
group.start(true, 1, frame, frame);

As gotoFrame is not working as expected, my current working workaround is this

for (const animatable of group.animatables) {
    animatable.fromFrame = frame;
    animatable.toFrame = frame;
}
2

cc @Evgeni_Popov

4

I tried to reproduce the problem, but couldn’t:

https://playground.babylonjs.com/#EJBTLO

The sphere is at the right location after we call goToFrame(300) on the base and additive animations (comment lines 75-76 and 79-82 to see the full animation).

Are you able to setup a repro in the Playground, to let us investigate?

5

Ok, I’ll try with your base playground, as my case is more complex (that’s a full animated human armature and the additive animation is for turning the head any chosen direction, with an animation from 90° left to 90° right)

I’ve set it up so that the additive animation is controlling the ball height, from Y=0 (first frame) to Y=5 (last frame)

Here is the case with gotoFrame() : the base animation is not applied anymore

https://playground.babylonjs.com/#EJBTLO#3

And here is it with my workaround

https://playground.babylonjs.com/#EJBTLO#4

Edit: I also tried to pause the offset animation and change the frame but it did not work, the additive animation will not be applied when paused.
Your playground is working because you are letting another frame run before pausing. If you pause right after gotoFrame(), you will see only the additive animation is applied https://playground.babylonjs.com/#EJBTLO#5

6

It doesn’t work because of the way in handle the goToFrame function, which calculates a delta time so that the next time we run the animate function, we automatically process the right frame. I think the problem is that, when you goToFrame each frame, the calculation error adds up in our offset, and the frame is no longer exactly the one you jumped to.

In your case, you manage the frame number yourself each frame, so you don’t need all our (somewhat complex) code for that. In this PR, I added an option to AnimationGroup.goToFrame() to just set fromFrame and toFrame as you did in your workaround:

7

Thanks for looking at this!

I’m not sure the fix is right, as setting fromFrame/toFrame is a very specific behavior on my side where I only play the same frame in a loop. The additional parameter is confusing as to what it does exactly and why, and with little value compared to calling fromFrame/toFrame directly instead.

I think this playground illustrate the issue more clearly: https://playground.babylonjs.com/#EJBTLO#5

The expected general behavior would be that the call to offsetGrp.goToFrame() does not replace the values but adds them (for additive animations). The main problem here, that caused visible glitches for me, is that additive values are set as they were non-additive (as RuntimeAnimation.goToFrame is doing a non-additive setValue).

I wouldn’t personally expect goToFrame() to work after a mix of additive and non-additive animations is immediately paused, and without running a full frame like it’s done in animate():

  • it doesn’t take weights into account, for different animations
  • it doesn’t take multiple potential additive animations

So the theoretical fix would be instead NOT to apply any value within goToFrame(), whether additive or not, and expect them to be applied next frame (or manually). And optionally provide a clear way to trigger manually the frame for the entire animation group.

As this would be probably a big breaking change, my proposal would be instead that no value is applied for additive animations only, and document clearly in the function description for goToFrame() that no value is applied for additive animation, and that this can only be done by waiting next frame, or with of manual call to animate() (or similar), as we need to compute base+additive animations to get a consistent result.

8

Unfortunately, it won’t work, because if goToFrame doesn’t set a value, it won’t have any effect.

The frame that is set by goToFrame (this._currentFrame) is only used if the currentFrame getter is called before animate is called, to get the right value. But animate itself doesn’t read from this._currentFrame, it generates a frame value based on the input parameters (elapsedTimeSinceAnimationStart, from, to, etc).

That’s why I think the PR would still have some value. We could update the documentation to explain what this parameter is for and in which case you should use it?

Regarding your PG, you can either do:

    animGrp.play(true);
    offsetGrp.play(true);

    animGrp.goToFrame(300);
    offsetGrp.goToFrame(300);

    scene.onBeforeRenderObservable.add(() => {
        animGrp.pause();
        offsetGrp.pause();
    });

or:

    animGrp.play(true);
    offsetGrp.play(true);

    animGrp.goToFrame(300);
    offsetGrp.goToFrame(300);

    scene.animate();

    animGrp.pause();
    offsetGrp.pause();

to make it work.

I think it corresponds to your comment:

and that this can only be done by waiting next frame, or with of manual call to animate() (or similar), as we need to compute base+additive animations to get a consistent result.

9

From reading the code, I think this is only partially true, and why I suggested it.

goToFrame is also changing this._manualJumpDelay which is later used to compute elapsedTimeSinceAnimationStart.

I tried my fix and this is the result: it’s kind of working but with weird random movements to the ball.

I’m not sure how the delay is computed.

Let’s try something more basic, with the animations playing normally unchanged, and trying to reset the additive frame (ball height) with a button click: https://playground.babylonjs.com/#EJBTLO#9.

It does not work for me, the position is reset erratically. I couldn’t make a reset to the base animation work reliably either (animGrp.goToFrame(0))

Edit: it looks like it won’t work with ANIMATIONLOOPMODE_YOYO but both will work as expected with ANIMATIONLOOPMODE_CYCLE. So maybe the calculation of manualJumpDelay is wrong with yoyo mode.

10

I think the problem is that we don’t store the frame you pass to goToFrame for later reuse, but instead calculate an offset delay so that, when we recalculate the frame later, we get the frame value you passed to goToFrame. The thing is that it’s not an exact calculation, we may not exactly get the frame you passed, and sometimes we may get the previous or the next frame, depending on the floating point rounding. It’s not really a problem if you call goToFrame once, but if you call it every frame, it will produce the artifacts you see.

The problem here is that goToFrame recalculate the new value of the property(ies) which is (are) animated, and replaces the previous value(s), even for additive animations. That’s not something we can change, because additive animations have a special handling, the animation system must re-run for the additive + normal animations to be processed correctly together.

YOYO mode is a bit special, because it is non linear, it uses the sin function to calculate the proper frame. I think that’s why the movement is not linear anymore when you goToFrame during the animation.