From 1c47865a5b75d037da153361239c58178211874d Mon Sep 17 00:00:00 2001
From: Chris Joel <cdata@google.com>
Date: Fri, 15 Mar 2019 15:06:08 -0700
Subject: [PATCH] Change API to be more like <video> element

---
 examples/animation.html             | 50 +++++++++++++---
 index.html                          | 81 ++++++++++++++++++++------
 src/features/animation.ts           | 88 +++++++++++++++++++++--------
 src/test/features/animation-spec.ts | 45 +++++++++++++--
 src/three-components/Model.js       | 18 ++++++
 5 files changed, 229 insertions(+), 53 deletions(-)

diff --git a/examples/animation.html b/examples/animation.html
index 024b621e1d..5d356de3d6 100644
--- a/examples/animation.html
+++ b/examples/animation.html
@@ -60,13 +60,13 @@ <h4>This page showcases how you can control models with animations</h4>
     <div class="content">
       <div class="wrapper">
         <div class="index">1</div>
-        <h2>Activate animations with the
-          <span class="attribute">animated</span> attribute</h2>
+        <h2>Automatically play animations with the
+          <span class="attribute">autoplay</span> attribute</h2>
         <example-snippet stamp-to="demo-container-1" highlight-as="html">
           <template>
             <model-viewer
                 controls
-                animated
+                autoplay
                 src="assets/RobotExpressive.glb"
                 background-color="#622dcf"
                 exposure="2"
@@ -88,7 +88,7 @@ <h2>Select a specific animation with <span class="attribute">animation-name</spa
           <template>
             <model-viewer
                 controls
-                animated
+                autoplay
                 animation-name="Running"
                 src="assets/RobotExpressive.glb"
                 background-color="#25997c"
@@ -108,27 +108,61 @@ <h2>Select a specific animation with <span class="attribute">animation-name</spa
         <div class="index">3</div>
         <h2>Animations crossfade when you change them</h2>
         <example-snippet stamp-to="demo-container-3" highlight-as="html">
+          <template>
+            <model-viewer
+                id="paused-change-demo"
+                controls
+                autoplay
+                animation-name="Running"
+                src="assets/RobotExpressive.glb"
+                background-color="#622dcf"
+                exposure="2"
+                environment-intensity="0"
+                alt="An animated 3D model of a robot"></model-viewer>
+<script>
+(() => {
+  const modelViewer = document.querySelector('#paused-change-demo');
+
+  self.setInterval(() => {
+    modelViewer.animationName = modelViewer.animationName === 'Running' ?
+      'Wave' : 'Running';
+  }, 1500.0);
+})();
+</script>
+          </template>
+        </example-snippet>
+      </div>
+    </div>
+  </div>
+
+
+  <div class="sample">
+    <div class="demo" id="demo-container-4"></div>
+    <div class="content">
+      <div class="wrapper">
+        <div class="index">4</div>
+        <h2>A paused model shows the first frame of the configured animation</h2>
+        <example-snippet stamp-to="demo-container-4" highlight-as="html">
           <template>
             <model-viewer
                 id="xfade-demo"
                 controls
-                animated
                 animation-name="Running"
                 src="assets/RobotExpressive.glb"
                 background-color="#622dcf"
                 exposure="2"
                 environment-intensity="0"
                 alt="An animated 3D model of a robot"></model-viewer>
-            <script>
+<script>
 (() => {
   const modelViewer = document.querySelector('#xfade-demo');
 
   self.setInterval(() => {
     modelViewer.animationName = modelViewer.animationName === 'Running' ?
-      'Wave' : 'Running';
+      'Idle' : 'Running';
   }, 1500.0);
 })();
-            </script>
+</script>
           </template>
         </example-snippet>
       </div>
diff --git a/index.html b/index.html
index af4571864c..d210663103 100644
--- a/index.html
+++ b/index.html
@@ -153,19 +153,13 @@ <h4>src<a href="#required-for-display">*</a></h4>
             <h4>alt</h4>
             <p>Configures the model with custom text that will be used to describe the model to viewers who use a screen reader or otherwise depend on additional semantic context to understand what they are viewing.</p>
           </li>
-          <li>
-            <h4>animated</h4>
-            <p>Enables animations. If a model has animations, they will
-            automatically begin to play when this attribute is set (or when the
-            property is set to true). If no animation-name is specified, plays
-            the first animation.</p>
-          </li>
           <li>
             <h4>animation-name</h4>
-            <p>Selects an animation to play by name when the
-            &lt;model-viewer&gt; is configured to be animated. If no
-            animation-name is specified, &lt;model-viewer&gt; always picks the
-            first animation it finds in the model.</p>
+            <p>Selects an animation to play by name. This animation will play
+            when the .play() method is invoked, or when the &lt;model-viewer&gt;
+            is configured to autoplay. If no animation-name is specified,
+            &lt;model-viewer&gt; always picks the first animation it finds in
+            the model.</p>
           </li>
           <li>
             <h4>animation-crossfade-duration</h4>
@@ -178,6 +172,13 @@ <h4>animation-crossfade-duration</h4>
             <h4>auto-rotate</h4>
             <p>Enables the auto rotation of the model.</p>
           </li>
+          <li>
+            <h4>autoplay</h4>
+            <p>If this is true and a model has animations, an animation will
+            automatically begin to play when this attribute is set (or when the
+            property is set to true). If no animation-name is specified, plays
+            the first animation.</p>
+          </li>
           <li>
             <h4>background-color</h4>
             <p>Sets the background color of the scene when viewed inline. Takes any valid CSS color string.</p>
@@ -220,10 +221,6 @@ <h4>ios-src</h4>
             <h4>magic-leap</h4>
             <P>Enables the ability to view models in AR when viewing content on <a href="https://magicleaphelio.com/">Magic Leap's Helio</a> browser, requires that src is a GLB model, and requires the inclusion of the <a href="https://www.npmjs.com/package/@magicleap/prismatic">@magicleap/prismatic</a> library.</P>
           </li>
-          <li>
-            <h4>pause-animation</h4>
-            <p>When set to true, pauses the current animation. Default is false.</p>
-          </li>
           <li>
             <h4>poster</h4>
             <p>Displays an image instead of the model. See <a href="https://github.com/GoogleWebComponents/model-viewer#on-loading">On Loading</a> for more information.</p>
@@ -257,6 +254,43 @@ <h4>unstable-webxr</h4>
         <p style="color:rgba(0,0,0,.54);" id="required-for-display">* Parameters that are required for display.</p>
         <p style="color:rgba(0,0,0,.54);">Note: All attributes have a corresponding property in camel-case format. For example, the background-color attribute can also be configured using the backgroundColor property.</p>
 
+        <h3 class="grouping-title" style="margin-top: 40px;">Properties</h3>
+
+        <ul class="list-attribute">
+          <li>
+            <h4>currentTime</h4>
+            <p>This property reports the current track time of the currently
+            selected animation. If no animations are available, the value is
+            always 0. This property can be set in order to seek along the
+            timeline of the currently playing animation. For example, if you
+            set it to 0, it will reset an animation to the beginning.</p>
+          </li>
+          <li>
+            <h4>paused</h4>
+            <p>This property is read-only. It returns true if animations are
+            paused. It returns false if animations are playing. Animations
+            always start paused, and remain so unless the autoplay attribute
+            is set or the .play() method is invoked.</p>
+          </li>
+        </ul>
+
+        <h3 class="grouping-title" style="margin-top: 40px;">Methods</h3>
+
+        <ul class="list-attribute">
+          <li>
+            <h4>play()</h4>
+            <p>Causes animations to be played. Use the autoplay attribute if
+            you want animations to be played automatically. If there are no
+            animations, nothing will happen, so make sure that the model is
+            loaded before invoking this method.</p>
+          </li>
+          <li>
+            <h4>pause()</h4>
+            <p>Causes animations to be paused. If you want to reset the
+            current animation to the beginning, you should also set the
+            currentTime property to 0.</p>
+          </li>
+        </ul>
 
         <h3 class="grouping-title" style="margin-top: 40px;">Events</h3>
 
@@ -277,6 +311,17 @@ <h4>poster-visibility</h4>
             changes. The current visibility state can be read from the
             <span class="attribute">event.detail.visible</span> property</p>
           </li>
+          <li>
+            <h4>play</h4>
+            <p>Dispatched when animations begin to play.</p>
+          </li>
+          <li>
+            <h4>pause</h4>
+            <p>Dispatched when animations are paused. A model always begins in
+            the paused state, so it is worth mentioning that this event will not
+            be dispatched until the the pause() method is invoked after
+            animations have begun playing.</p>
+          </li>
           <li>
             <h4>preload</h4>
             <p>When preload is enabled this event is fired when preloading is done.</p>
@@ -299,7 +344,7 @@ <h3 class="grouping-title" style="margin-top: 40px;">Browser Support</h3>
             <th><img class="logo-chrome" title="Chrome" alt="Chrome"></th>
             <th><img class="logo-canary" title="Canary" alt="Canary"></th>
             <th><img class="logo-safari" title="Safari 12" alt="Safari 12"></th>
-            <th><img class="logo-firefox" title="Firefox 65" alt="Firefox 65" src="logo-firefox"></th>
+            <th><img class="logo-firefox" title="Firefox 65" alt="Firefox 65"></th>
             <th><img class="logo-edge" title="Edge" alt="Edge"></th>
             <th><img class="logo-ie" title="IE 11" alt="IE 11"></th>
           </tr>
@@ -330,7 +375,7 @@ <h3 class="grouping-title" style="margin-top: 40px;">Browser Support</h3>
             <th><img class="logo-chrome" title="Chrome" alt="Chrome"></th>
             <th><img class="logo-canary" title="Canary" alt="Canary"></th>
             <th><img class="logo-safari" title="Safari 12" alt="Safari 12"></th>
-            <th><img class="logo-firefox" title="Firefox 65" alt="Firefox 65" src="logo-firefox"></th>
+            <th><img class="logo-firefox" title="Firefox 65" alt="Firefox 65"></th>
             <th><img class="logo-edge" title="Edge" alt="Edge"></th>
             <th><img class="logo-ie" title="IE 11" alt="IE 11"></th>
           </tr>
@@ -361,7 +406,7 @@ <h3 class="grouping-title" style="margin-top: 40px;">Browser Support</h3>
             <th><img class="logo-chrome" title="Chrome" alt="Chrome"></th>
             <th><img class="logo-canary" title="Canary" alt="Canary"></th>
             <th><img class="logo-safari" title="Safari 12" alt="Safari 12"></th>
-            <th><img class="logo-firefox" title="Firefox 65" alt="Firefox 65" src="logo-firefox"></th>
+            <th><img class="logo-firefox" title="Firefox 65" alt="Firefox 65"></th>
             <th><img class="logo-edge" title="Edge" alt="Edge"></th>
             <th><img class="logo-ie" title="IE 11" alt="IE 11"></th>
           </tr>
diff --git a/src/features/animation.ts b/src/features/animation.ts
index 9b98efd32c..9a1c827a15 100644
--- a/src/features/animation.ts
+++ b/src/features/animation.ts
@@ -20,20 +20,21 @@ import {Constructor} from '../utils.js';
 
 const MILLISECONDS_PER_SECOND = 1000.0
 
-const $updateAnimation = Symbol('updateAnimation');
+const $changeAnimation = Symbol('changeAnimation');
+const $paused = Symbol('paused');
 
 export const AnimationMixin =
     (ModelViewerElement: Constructor<ModelViewerElementBase>):
         Constructor<ModelViewerElementBase> => {
           class AnimationModelViewerElement extends ModelViewerElement {
-            @property({type: Boolean}) animated: boolean = false;
-            @property({type: Boolean, attribute: 'pause-animation'})
-            pauseAnimation: boolean = false;
+            @property({type: Boolean}) autoplay: boolean = false;
             @property({type: String, attribute: 'animation-name'})
-            animationName: string|null = null;
+            animationName: string|void = undefined;
             @property({type: Number, attribute: 'animation-crossfade-duration'})
             animationCrossfadeDuration: number = 300;
 
+            protected[$paused]: boolean = true;
+
             /**
              * Returns an array
              */
@@ -45,12 +46,50 @@ export const AnimationMixin =
               return [];
             }
 
+            get paused(): boolean {
+              return this[$paused];
+            }
+
+            get currentTime(): number {
+              return (this as any)[$scene].model.animationTime;
+            }
+
+            set currentTime(value: number) {
+              (this as any)[$scene].model.animationTime = value;
+            }
+
+            pause() {
+              if (this[$paused]) {
+                return;
+              }
+
+              this[$paused] = true;
+              this.dispatchEvent(new CustomEvent('pause'));
+            }
+
+            play() {
+              if (this[$paused] && this.availableAnimations.length > 0) {
+                this[$paused] = false;
+
+                if (!(this as any)[$scene].model.hasActiveAnimation) {
+                  this[$changeAnimation]();
+                }
+
+                this.dispatchEvent(new CustomEvent('play'));
+              }
+            }
+
             [$onModelLoad]() {
-              this[$updateAnimation]();
+              this[$paused] = true;
+
+              if (this.autoplay) {
+                this[$changeAnimation]();
+                this.play();
+              }
             }
 
             [$tick](_time: number, delta: number) {
-              if (this.pauseAnimation || !this.animated) {
+              if (this[$paused]) {
                 return;
               }
 
@@ -63,33 +102,36 @@ export const AnimationMixin =
             updated(changedProperties: Map<string, any>) {
               super.updated(changedProperties);
 
-              if (changedProperties.has('animated') ||
-                  changedProperties.has('animationName')) {
-                this[$updateAnimation]();
+              if (changedProperties.has('autoplay') && this.autoplay) {
+                this.play();
+              }
+
+              if (changedProperties.has('animationName')) {
+                this[$changeAnimation]();
               }
             }
 
             async[$updateSource]() {
               super[$updateSource]();
 
-              // Reject a pending model load promise, effectively cancelling
-              // any pending work to set the animation for a model that has
-              // not fully loaded:
+              // If we are loading a new model, we need to stop the animation of
+              // the current one (if any is playing). Otherwise, we might lose
+              // the reference to the scene root and running actions start to
+              // throw exceptions and/or behave in unexpected ways:
               (this as any)[$scene].model.stopAnimation();
             }
 
-            async[$updateAnimation]() {
+            [$changeAnimation]() {
               const {model} = (this as any)[$scene];
 
-              if (this.animated === true) {
-                model.playAnimation(
-                    this.animationName,
-                    this.animationCrossfadeDuration / MILLISECONDS_PER_SECOND);
-              } else {
-                model.stopAnimation();
-                // Tick steps are no longer invoked if animated is false, so we
-                // need to invoke one last render or else the model will appear
-                // to freeze on the last renderered animation frame:
+              model.playAnimation(
+                  this.animationName,
+                  this.animationCrossfadeDuration / MILLISECONDS_PER_SECOND);
+
+              // If we are currently paused, we need to force a render so that
+              // the model updates to the first frame of the new animation
+              if (this[$paused]) {
+                model.updateAnimation(0);
                 this[$needsRender]();
               }
             }
diff --git a/src/test/features/animation-spec.ts b/src/test/features/animation-spec.ts
index 90a3c1cf1c..c3263c4dbc 100644
--- a/src/test/features/animation-spec.ts
+++ b/src/test/features/animation-spec.ts
@@ -28,7 +28,8 @@ const animationIsPlaying = (element: any, animationName = null): boolean => {
   if (currentAnimationAction != null &&
       (animationName == null ||
        currentAnimationAction.getClip().name === animationName)) {
-    return currentAnimationAction.enabled === true &&
+    return element.paused === false &&
+        currentAnimationAction.enabled === true &&
         !currentAnimationAction.paused;
   }
 
@@ -39,6 +40,7 @@ suite('ModelViewerElementBase with AnimationMixin', () => {
   let nextId = 0;
   let tagName: string;
   let ModelViewerElement: any;
+  let element: any;
 
   setup(() => {
     tagName = `model-viewer-animation-${nextId++}`;
@@ -53,12 +55,47 @@ suite('ModelViewerElementBase with AnimationMixin', () => {
 
   BasicSpecTemplate(() => ModelViewerElement, () => tagName);
 
-  suite('when configured to animate', () => {
-    let element: any;
+  suite('a model with animations', () => {
+    setup(async () => {
+      element = new ModelViewerElement();
+      element.src = ANIMATED_GLB_PATH;
+      document.body.appendChild(element);
+
+      await waitForEvent(element, 'load');
+    });
+
+    teardown(() => {
+      document.body.removeChild(element);
+    });
+
+    test('remains in a paused state', () => {
+      expect(element.paused).to.be.true;
+    });
+
+    suite('when play is invoked', () => {
+      setup(async () => {
+        const animationsPlay = waitForEvent(element, 'play');
+        element.play();
+        await animationsPlay;
+      });
+
+      test('animations play', async () => {
+        expect(animationIsPlaying(element)).to.be.true;
+      });
+
+      test('animations can be paused', async () => {
+        const animationsPause = waitForEvent(element, 'pause');
+        element.pause();
+        await animationsPause;
+        expect(animationIsPlaying(element)).to.be.false;
+      });
+    });
+  });
 
+  suite('when configured to autoplay', () => {
     setup(() => {
       element = new ModelViewerElement();
-      element.animated = true;
+      element.autoplay = true;
       document.body.appendChild(element);
     });
 
diff --git a/src/three-components/Model.js b/src/three-components/Model.js
index 1b5df68063..375a5ccdd0 100644
--- a/src/three-components/Model.js
+++ b/src/three-components/Model.js
@@ -186,6 +186,24 @@ export default class Model extends Object3D {
     this.dispatchEvent({type: 'model-load', url});
   }
 
+  set animationTime(value = 0) {
+    if (this.currentAnimationAction != null) {
+      this.currentAnimationAction.time = value;
+    }
+  }
+
+  get animationTime() {
+    if (this.currentAnimationAction != null) {
+      return this.currentAnimationAction.time;
+    }
+
+    return 0;
+  }
+
+  get hasActiveAnimation() {
+    return this.currentAnimationAction != null;
+  }
+
   /**
    * Plays an animation if there are any associated with the current model.
    * Accepts an optional string name of an animation to play. If no name is