aframevr / aframe

:a: Web framework for building virtual reality experiences.
https://aframe.io/
MIT License
16.42k stars 3.91k forks source link

Time leakage bug in anime.js when looping animations #5361

Open mrxz opened 8 months ago

mrxz commented 8 months ago

Description: Every time an animation loops the excess time is ignored. This leads to "time leakage" which can accumulate over time or cause issues when high delta times occur for a given tick. This can easily be triggered by giving focus to another tab, which causes browsers to reduce or halt the rAF until the user clicks back again.

Real-world case where this bug causes multiple entities with the same looping animations randomly offset from each other to end up synchronized. https://github.com/3DStreet/3dstreet/issues/364

This is a known bug in anime.js (https://github.com/juliangarnier/anime/issues/561) and is also present in A-Frame's fork. Filing this bug here instead for easier discovery in case other people face this issue.

Algorush commented 8 months ago

I looked at the code a little and discovered that when returning from another tab, the time is not counted, since the activeInstances array (elements with applied animation) is always empty. I still don't understand why this is so. Maybe thats the reason. Here: https://github.com/juliangarnier/anime/blob/3ebfd913a04f7dc59cc3d52e38275272a5a12ae6/src/index.js#L881

And also instance are added to the activeInstances array only inside the instance.play function, which is never called on test scenes: https://github.com/juliangarnier/anime/blob/3ebfd913a04f7dc59cc3d52e38275272a5a12ae6/src/index.js#L1125

mrxz commented 8 months ago

Note that A-frame uses this fork: https://github.com/supermedium/anime/tree/master, which is an older version and contains some custom modifications.

I believe the reason instance.play isn't called and activeInstances remains empty is that A-Frame manually invokes the tick function for animations instead of letting anime.js handle it. Part of the reason is that by default anime.js would use its own requestAnimationFrame loop, which wouldn't work during a WebXR session.

Effectively there are two "issues" at play:

  1. Whenever an animation loops, excess delta time is leaked
  2. The first tick after a tab regains focus will contain a (very) high delta time which isn't filtered

So it seems anime.js has measures to combat the latter, but due to the way A-Frame uses it, that doesn't apply. Ultimately fixing the former should make the second one a non-issue. And it would ensure other causes for high delta times don't case issues either.

Regardless, it might be a good idea to implement a similar workaround directly in A-Frame.It's difficult to make logic resilient against high delta times and it's not particularly uncommon for users to tab away. Of course such a change has both pros and cons :thinking:

dmarcos commented 8 months ago

Any PRs to address super welcome