search

google / model-viewer

Easily display interactive 3D models on the web and in AR!
https://modelviewer.dev
Apache License 2.0
6.88k stars 816 forks source link

Delay in starting and switching animations #3326

Closed LazZiya closed 2 years ago

LazZiya commented 2 years ago

I have model with multiple animations that plays on button click. The animations are not starting immediately, also it is not switching to another animation smoothly!

Buggy demo (with latest version)

https://demo.ziyad.info/webar-models/fs90/range-cooker.html

Bug-free demo (with v1.10.1)

https://demo.ziyad.info/webar-models/fs90/fs90-1.10.1.html

Version

Browser Affected

OS

AR

elalish commented 2 years ago

I see the bug, but your scripts are minified so it's hard to tell what's going on. Can you make a more basic repro with Glitch perhaps? I'd like to understand what's different about how you're switching animations vs our example: https://modelviewer.dev/examples/animation/index.html#crossFade

LazZiya commented 2 years ago

Sorry I missed that all my scripts are minified and encoded :(

Actually I am calling this function providing the new animation name as parameter to switch the animation on button click:

function animateModel(newAnim) {
        var currentAnim = modelViewer.animationName;

        // pause or play if it is the same animation
        if (currentAnim === newAnim) {
            if (modelViewer.paused)
                modelViewer.play();
            else
                modelViewer.pause();
        } else {
            // pause current animation
            modelViewer.pause();
            // switch to new animation
            modelViewer.animationName = newAnim;
            // play animation
            modelViewer.play();
        }
    }

I will try to post a basic repo in the weekend (due to official work).

elalish commented 2 years ago

Can you try simplifying it? Maybe something like:

function animateModel(newAnim) {
          // switch to new animation
          modelViewer.animationName = newAnim;
          // play animation
          if (modelViewer.paused)
              modelViewer.play();
    }

I think all the extra logic might be confusing things; we take care of most of this internally.

LazZiya commented 2 years ago

I simplified the model and the script, I am using just a cube with two simple animations (rotate and UpDown), this issue remain the same.

buggy sample (v1.11.0)

https://demo.ziyad.info/webar-models/cube/index.html

bug-free sample (v1.10.1)

https://demo.ziyad.info/webar-models/cube/index-v1.10.1.html

Current script is as simple as:

function animateModel(newAnim) {
    // switch to new animation
    modelViewer.animationName = newAnim;
    modelViewer.play();
}

I tried even with async methods and await modelViewer.updateComplete; but the issue kept the same.

What is more, I noticed the issue also exists in Chrome and Edge on Winodws!

elalish commented 2 years ago

Interesting; I think there is a bug of some kind here, but I believe you can avoid it simply by not calling play() when the animation is already playing (note the difference between my suggested function above and yours). In the debugger I can see that your play command is being called before the animation is switched, since attribute changes are processed asynchronously. But since you're playing on a loop anyway, there's no need to play over and over.

LazZiya commented 2 years ago

Yes, I can confirm that your code worked fine, but this works only with continues loop, in my scenario I need to pause and stop the animation on demand. I updated my script again and noticed that the bug appears only after calling pause() method!

And I noticed another unexpected behavior; in the buggy sample if I do pause() then play() the animation restarts from the first frame. But in the old version it continues from the frame it stopped at as expected.

You can see updated links with pause/play and reset buttons added:

LazZiya commented 2 years ago

Just reviewed the source code and noticed this code block, I think this is causing the animation to restart and not continue!

https://github.com/google/model-viewer/blob/18c855bf8c550a07b2123c6f892223ea52266eed/packages/model-viewer/src/features/animation.ts#L184-L189

elalish commented 2 years ago

Good catch! I'll check when that got added.

clochardM33 commented 2 years ago

I've just stumbled across the same: From paused .play starts from 0 not current time. Thankfully this thread has just stopped me loosing my mind!

Yeah, as LazZiya says, switching /model-viewer@1.10.1/ in reverts back to play continuing from current time.