From 933efc806157f7c8a37974ed004201dd0667726e Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Fri, 12 Jun 2026 21:20:33 +0200 Subject: [PATCH 1/2] fix: emit finished event for one-time animations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two separate bugs prevented the 'finished' event from being dispatched when an animation was played a single time (repetitions: 1): 1. Mixer event subscriptions were lost on model load (regression from 4.3.0). The multi-model refactor recreates the animation mixers on every setSource(), but the mixer event subscriptions are registered only once, when the animation feature is constructed — before any model is loaded, while there are no mixers yet. They were never re-applied to the mixers created later, so 'finished' (and 'loop') were never dispatched. ModelScene now remembers its subscriptions and attaches them to every mixer it creates. 2. Setting the animationName property and then calling play() looped forever (present in all versions). Assigning animationName schedules an asynchronous reactive update whose updated() re-triggers playback; it did so with the default options (repetitions: Infinity / LoopRepeat), overwriting a play({repetitions: 1}) issued in the same tick, so the clip looped and never finished. Using the declarative animation-name attribute avoided this only because the property never changed. play() now records its options and the re-trigger reuses them instead of resetting to an infinite loop. Both paths are covered by regression tests in animation-spec.ts. --- .../model-viewer/src/features/animation.ts | 24 ++++++++--- .../src/test/features/animation-spec.ts | 40 +++++++++++++++++++ .../src/three-components/ModelScene.ts | 24 ++++++++++- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/packages/model-viewer/src/features/animation.ts b/packages/model-viewer/src/features/animation.ts index 9ff94831f9..402ccbde9c 100644 --- a/packages/model-viewer/src/features/animation.ts +++ b/packages/model-viewer/src/features/animation.ts @@ -25,6 +25,7 @@ const $changeAnimation = Symbol('changeAnimation'); const $appendAnimation = Symbol('appendAnimation'); const $detachAnimation = Symbol('detachAnimation'); const $paused = Symbol('paused'); +const $currentAnimationOptions = Symbol('currentAnimationOptions'); interface PlayAnimationOptions { repetitions: number; @@ -97,6 +98,13 @@ export const AnimationMixin = >( animationCrossfadeDuration: number = 300; protected[$paused]: boolean = true; + // Remembers the options from the most recent play() call so re-applying the + // current animation (e.g. when animationName changes) preserves them rather + // than resetting to an infinite loop. Without this, `animationName = name` + // followed by `play({repetitions: 1})` loops forever and never emits + // 'finished', because the asynchronous animationName update re-triggers + // playback with the default (infinite) options. + private[$currentAnimationOptions]: PlayAnimationOptions = DEFAULT_PLAY_OPTIONS; constructor(...args: any[]) { super(args); @@ -189,7 +197,8 @@ export const AnimationMixin = >( const modelIndex = options?.modelIndex ?? null; this[$scene].unpauseAnimation(modelIndex); - this[$changeAnimation](options); + this[$currentAnimationOptions] = {...DEFAULT_PLAY_OPTIONS, ...options}; + this[$changeAnimation](this[$currentAnimationOptions]); this.dispatchEvent(new CustomEvent('play')); } @@ -256,9 +265,14 @@ export const AnimationMixin = >( } } - [$changeAnimation](options: PlayAnimationOptions = DEFAULT_PLAY_OPTIONS) { - const repetitions = options.repetitions ?? Infinity; - const mode = options.pingpong ? + [$changeAnimation](options?: PlayAnimationOptions) { + // When re-triggered without explicit options (e.g. from an animationName + // change or onModelLoad), reuse the options from the most recent play() + // so an in-flight `play({repetitions: 1})` is not overwritten with the + // infinite-loop default. + const opts = options ?? this[$currentAnimationOptions]; + const repetitions = opts.repetitions ?? Infinity; + const mode = opts.pingpong ? LoopPingPong : (repetitions === 1 ? LoopOnce : LoopRepeat); this[$scene].playAnimation( @@ -266,7 +280,7 @@ export const AnimationMixin = >( this.animationCrossfadeDuration / MILLISECONDS_PER_SECOND, mode, repetitions, - options.modelIndex); + opts.modelIndex); // If we are currently paused, we need to force a render so that // the scene updates to the first frame of the new animation diff --git a/packages/model-viewer/src/test/features/animation-spec.ts b/packages/model-viewer/src/test/features/animation-spec.ts index d4eb6502bf..4571d696e8 100644 --- a/packages/model-viewer/src/test/features/animation-spec.ts +++ b/packages/model-viewer/src/test/features/animation-spec.ts @@ -81,6 +81,46 @@ suite('Animation', () => { expect(element.paused).to.be.true; }); + test('dispatches "finished" when a repetition-limited animation completes', + async () => { + element.play({repetitions: 1, pingpong: false}); + await element.updateComplete; + + const finished = waitForEvent(element, 'finished'); + // Jump near the end so the LoopOnce action completes quickly (the + // default clip is longer than the test timeout). + element.currentTime = Math.max(element.duration - 0.05, 0); + await timePasses(500); + await finished; + }); + + // Regression test: setting `animationName` schedules an async reactive + // update whose updated() re-triggers playback. It used to do so with the + // default (infinite-loop) options, clobbering a `play({repetitions: 1})` + // issued in the same tick, so the animation looped forever and never + // emitted 'finished'. + test('dispatches "finished" when animationName is set then played once', + async () => { + const target = element.availableAnimations.find( + (name) => name !== element.animationName) ?? + element.availableAnimations[0]; + + element.animationName = target; + // Restart from the first frame, mirroring how a one-time animation is + // typically (re)played after a previous one finished and clamped. + element.currentTime = 0; + element.play({repetitions: 1, pingpong: false}); + + // Flush the animationName update, where the clobber used to happen. + await element.updateComplete; + + const finished = waitForEvent(element, 'finished'); + // Jump near the end so the LoopOnce action completes quickly. + element.currentTime = Math.max(element.duration - 0.05, 0); + await timePasses(500); + await finished; + }); + suite('when play is invoked with no options', () => { setup(async () => { const animationsPlay = waitForEvent(element, 'play'); diff --git a/packages/model-viewer/src/three-components/ModelScene.ts b/packages/model-viewer/src/three-components/ModelScene.ts index 9201d174eb..6fc539d3f6 100644 --- a/packages/model-viewer/src/three-components/ModelScene.ts +++ b/packages/model-viewer/src/three-components/ModelScene.ts @@ -120,6 +120,12 @@ export class ModelScene extends Scene { private boundsAndShadowDirty = false; private mixers: AnimationMixer[] = []; private mixerPausedStates: boolean[] = []; + // Mixer event subscriptions are kept so they can be re-applied to the mixers + // that are recreated on every setSource() (one per loaded glTF). + private mixerEventSubscriptions: Array<{ + event: keyof AnimationMixerEventMap, + callback: (...args: any[]) => void + }> = []; private cancelPendingSourceChange: (() => void)|null = null; private animationsByName: Map = new Map(); private currentAnimationActions: (AnimationAction|null)[] = []; @@ -281,11 +287,11 @@ export class ModelScene extends Scene { if (gltf != null) { this._models.push(gltf.scene); this.target.add(gltf.scene); - this.mixers.push(new AnimationMixer(gltf.scene)); + this.mixers.push(this.createMixer(gltf.scene)); this.mixerPausedStates.push(false); this.currentAnimationActions.push(null); } else { - this.mixers.push(new AnimationMixer(this.target)); + this.mixers.push(this.createMixer(this.target)); this.mixerPausedStates.push(false); this.currentAnimationActions.push(null); } @@ -1282,11 +1288,25 @@ export class ModelScene extends Scene { subscribeMixerEvent( event: keyof AnimationMixerEventMap, callback: (...args: any[]) => void) { + // Remember the subscription so it survives the mixers being recreated on + // the next setSource(), then apply it to the mixers that already exist. + this.mixerEventSubscriptions.push({event, callback}); for (const mixer of this.mixers) { mixer.addEventListener(event, callback); } } + // Creates an AnimationMixer with the currently subscribed events already + // attached, so events (e.g. 'finished', 'loop') keep firing after a new + // model is loaded. + private createMixer(root: Object3D): AnimationMixer { + const mixer = new AnimationMixer(root); + for (const {event, callback} of this.mixerEventSubscriptions) { + mixer.addEventListener(event, callback); + } + return mixer; + } + /** * Call if the object has been changed in such a way that the shadow's * shape has changed (not a rotation about the Y axis). From 8c72426c298f82316b6706d5b32140fc7027bfbf Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Mon, 15 Jun 2026 11:54:34 +0200 Subject: [PATCH 2/2] docs: add "Play Once" animation example Demonstrate playing an animation a single time on demand: set animationName, reset currentTime to 0, call play({repetitions: 1}), and re-enable the control on the 'finished' event. Doubles as a live test of the one-time-animation finished fix. --- packages/modelviewer.dev/data/examples.json | 4 ++ .../examples/animation/index.html | 58 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/packages/modelviewer.dev/data/examples.json b/packages/modelviewer.dev/data/examples.json index 28e733f1fd..6595e2b2c8 100644 --- a/packages/modelviewer.dev/data/examples.json +++ b/packages/modelviewer.dev/data/examples.json @@ -221,6 +221,10 @@ "htmlId": "controlSpeed", "name": "Control Speed" }, + { + "htmlId": "playOnce", + "name": "Play Once" + }, { "htmlId": "crossFade", "name": "Crossfade" diff --git a/packages/modelviewer.dev/examples/animation/index.html b/packages/modelviewer.dev/examples/animation/index.html index 420b88a616..bcc4cf9005 100644 --- a/packages/modelviewer.dev/examples/animation/index.html +++ b/packages/modelviewer.dev/examples/animation/index.html @@ -161,6 +161,64 @@

+
+
+
+
+
+

Play an animation a single time and react to the finished event

+

+
+ + + +
+
+
+