Bug 206280 – Do not detect the stopped animations in Nicosia::Animation to avoid flashback

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Tomoki Imai]

Created attachment 387759 [details]
The video shows what we see on morphing-cubes.html

How to reproduce the flashback:
- Build webkitgtk with MiniBrowser
- Run minibrowser and open https://webkit.org/blog-files/3d-transforms/morphing-cubes.html
- Push "Toggle Shape" button several times. We see flashback when the toggling shape animation stops.

Confirmed revision: r254528

The flashback was caused by using the old layer transform matrix saved when the animation has been started.

The root cause is an inconsistency of animation state in Nicosia::Animation and CoordinatedGraphicsLayer.
For Nicosia::Animation, ThreadedCompositor increases MonitonicTime for animation every frame, and calls Nicosia::Animation::apply.
For CoordinatedGraphicsLayer, CSSAnimationController updates animations list and if the animation has been finished it updates CSS value.
There is a chance to use old layer state while the Nicosia::Animation stopped, but CoordinatedGraphicsLayer still obtains old CSS value and animations.

[Tomoki Imai]

Created attachment 387761 [details]
patch

Patch to fix the issue

[Tomoki Imai]

Created attachment 387762 [details]
patch

Added bug URL

[Miguel Gomez]

I see what happens here.

On a proper execution, CSSAnimationController sets an animation to the CoordinatedGraphicsLayer, which is flushed to the appropriate TextureMapperLayer, and several compositions happen in the compositor until the animation is over. At that point the CSSAnimationController removes the animation from the CoordinatedGraphicsLayer, that's flushed to the TextureMapperLayer and the animation is gone.

The problem is that a new composition may happen after the animation is finished on the compositor side, but the CSSAnimationController didn't have time to propagate the removal of the animation to the TextureMapperLayer. In the morphing cubes example this extra composition is probably caused by the other animations running (as the finished one won't request more compositions).

This extra composition happens when the animation is over on the compositor side, so the animation transform is not applied and the TextureMapperLayer is painted where it was before the animation, and there's the flashback.

The idea of the patch looks good: basically always apply the animation until it's removed by the CSSAnimationController. This causes that even if the animation is over, the TextureMapperLayer will be painted in the last position of the animation until it gets removed, which makes sense. Also, this is not causing extra composition requests, because in order to decide whether there are animations ongoing to request more compositions we check whether there are animations in the playing state and not whether they are active.

Keeping the last animation transform until it's removed by CSSAnimationController doesn't seem to cause any visual regression (I tried several tests to check) and it doesn't cause regressions on the layout tests, so it looks good to me.

[WebKit Commit Bot]

Comment on attachment 387762 [details]
patch

Clearing flags on attachment: 387762

Committed r254680: <https://trac.webkit.org/changeset/254680>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Tomoki Imai]

Thanks for your kind review!