Remove preload event (#3799)

* refactored loading

* fixed extraneous load events

* removed preload and model-load events

Author: elalish

packages/model-viewer/src/features/environment.ts

@@ -14,10 +14,9 @@
  */
 
 import {property} from 'lit/decorators.js';
-import {Event as ThreeEvent, Texture} from 'three';
+import {Texture} from 'three';
 
-import ModelViewerElementBase, {$needsRender, $onModelLoad, $progressTracker, $renderer, $scene, $shouldAttemptPreload} from '../model-viewer-base.js';
-import {PreloadEvent} from '../three-components/CachingGLTFLoader.js';
+import ModelViewerElementBase, {$needsRender, $progressTracker, $renderer, $scene, $shouldAttemptPreload} from '../model-viewer-base.js';
 import {clamp, Constructor, deserializeUrl} from '../utilities.js';
 
 export const BASE_OPACITY = 0.5;
@@ -29,7 +28,6 @@ export const $currentEnvironmentMap = Symbol('currentEnvironmentMap');
 export const $currentBackground = Symbol('currentBackground');
 export const $updateEnvironment = Symbol('updateEnvironment');
 const $cancelEnvironmentUpdate = Symbol('cancelEnvironmentUpdate');
-const $onPreload = Symbol('onPreload');
 
 export declare interface EnvironmentInterface {
   environmentImage: string|null;
@@ -65,22 +63,6 @@ export const EnvironmentMixin = <T extends Constructor<ModelViewerElementBase>>(
 
     private[$cancelEnvironmentUpdate]: ((...args: any[]) => any)|null = null;
 
-    private[$onPreload] = (event: ThreeEvent) => {
-      if ((event as PreloadEvent).element === this) {
-        this[$updateEnvironment]();
-      }
-    };
-
-    connectedCallback() {
-      super.connectedCallback();
-      this[$renderer].loader.addEventListener('preload', this[$onPreload]);
-    }
-
-    disconnectedCallback() {
-      super.disconnectedCallback();
-      this[$renderer].loader.removeEventListener('preload', this[$onPreload]);
-    }
-
     updated(changedProperties: Map<string|number|symbol, unknown>) {
       super.updated(changedProperties);
 
@@ -110,13 +92,6 @@ export const EnvironmentMixin = <T extends Constructor<ModelViewerElementBase>>(
       return this[$scene].bakedShadows.size > 0;
     }
 
-    [$onModelLoad]() {
-      super[$onModelLoad]();
-
-      this[$scene].setEnvironmentAndSkybox(
-          this[$currentEnvironmentMap], this[$currentBackground]);
-    }
-
     async[$updateEnvironment]() {
       const {skyboxImage, environmentImage} = this;
 
@@ -138,8 +113,7 @@ export const EnvironmentMixin = <T extends Constructor<ModelViewerElementBase>>(
             await textureUtils.generateEnvironmentMapAndSkybox(
                 deserializeUrl(skyboxImage),
                 environmentImage,
-                (progress: number) =>
-                    updateEnvProgress(clamp(progress, 0, 1) * 0.99));
+                (progress: number) => updateEnvProgress(clamp(progress, 0, 1)));
 
         if (this[$currentEnvironmentMap] !== environmentMap) {
           this[$currentEnvironmentMap] = environmentMap;
@@ -163,11 +137,7 @@ export const EnvironmentMixin = <T extends Constructor<ModelViewerElementBase>>(
           throw errorOrPromise;
         }
       } finally {
-        requestAnimationFrame(() => {
-          requestAnimationFrame(() => {
-            updateEnvProgress(1.0);
-          });
-        });
+        updateEnvProgress(1.0);
       }
     }
   }

packages/model-viewer/src/model-viewer-base.ts

@@ -18,11 +18,12 @@ import {property} from 'lit/decorators.js';
 import {Event as ThreeEvent, Vector2, Vector3} from 'three';
 
 import {HAS_INTERSECTION_OBSERVER, HAS_RESIZE_OBSERVER} from './constants.js';
+import {$updateEnvironment} from './features/environment.js';
 import {makeTemplate} from './template.js';
 import {$evictionPolicy, CachingGLTFLoader} from './three-components/CachingGLTFLoader.js';
 import {ModelScene} from './three-components/ModelScene.js';
 import {ContextLostEvent, Renderer} from './three-components/Renderer.js';
-import {clamp, debounce, timePasses} from './utilities.js';
+import {clamp, debounce} from './utilities.js';
 import {dataUrlToBlob} from './utilities/data-conversion.js';
 import {ProgressTracker} from './utilities/progress-tracker.js';
 
@@ -257,17 +258,6 @@ export default class ModelViewerElementBase extends ReactiveElement {
     this[$scene] =
         new ModelScene({canvas: this[$canvas], element: this, width, height});
 
-    this[$scene].addEventListener('model-load', async (event) => {
-      this[$markLoaded]();
-      this[$onModelLoad]();
-
-      // Give loading async tasks a chance to complete.
-      await timePasses();
-
-      this.dispatchEvent(
-          new CustomEvent('load', {detail: {url: (event as any).url}}));
-    });
-
     // Update initial size on microtask timing so that subclasses have a
     // chance to initialize
     Promise.resolve().then(() => {
@@ -579,43 +569,55 @@ export default class ModelViewerElementBase extends ReactiveElement {
 
   /**
    * Parses the element for an appropriate source URL and
-   * sets the views to use the new model based off of the `preload`
-   * attribute.
+   * sets the views to use the new model based.
    */
   async[$updateSource]() {
-    if (this.loaded || !this[$shouldAttemptPreload]()) {
+    const scene = this[$scene];
+    if (this.loaded || !this[$shouldAttemptPreload]() ||
+        this.src === scene.url) {
       return;
     }
 
     if (this.generateSchema) {
-      this[$scene].updateSchema(this.src);
+      scene.updateSchema(this.src);
     }
     this[$updateStatus]('Loading');
     // 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[$scene].stopAnimation();
+    scene.stopAnimation();
 
     const updateSourceProgress = this[$progressTracker].beginActivity();
     const source = this.src;
     try {
-      await this[$scene].setSource(
+      const srcUpdated = scene.setSource(
           source,
           (progress: number) =>
               updateSourceProgress(clamp(progress, 0, 1) * 0.95));
 
-      const detail = {url: source};
-      this.dispatchEvent(new CustomEvent('preload', {detail}));
+      const envUpdated = (this as any)[$updateEnvironment]();
+
+      await Promise.all([srcUpdated, envUpdated]);
+
+      this[$markLoaded]();
+      this[$onModelLoad]();
+
+      // Wait for shaders to compile and pixels to be drawn.
+      await new Promise<void>(resolve => {
+        requestAnimationFrame(() => {
+          requestAnimationFrame(() => {
+            this.dispatchEvent(
+                new CustomEvent('load', {detail: {url: source}}));
+            resolve();
+          });
+        });
+      });
     } catch (error) {
       this.dispatchEvent(new CustomEvent(
           'error', {detail: {type: 'loadfailure', sourceError: error}}));
     } finally {
-      requestAnimationFrame(() => {
-        requestAnimationFrame(() => {
-          updateSourceProgress(1.0);
-        });
-      });
+      updateSourceProgress(1.0);
     }
   }
 }

packages/model-viewer/src/test/features/loading-spec.ts

@@ -53,16 +53,11 @@ suite('Loading', () => {
 
   test('does not load when hidden from render tree', async () => {
     let loadDispatched = false;
-    let preloadDispatched = false;
     const loadHandler = () => {
       loadDispatched = true;
     };
-    const preloadHandler = () => {
-      preloadDispatched = true;
-    };
 
     element.addEventListener('load', loadHandler);
-    element.addEventListener('preload', preloadHandler);
 
     element.style.display = 'none';
 
@@ -75,10 +70,8 @@ suite('Loading', () => {
     await timePasses(500);  // Arbitrary time to allow model to load
 
     element.removeEventListener('load', loadHandler);
-    element.removeEventListener('preload', preloadHandler);
 
     expect(loadDispatched).to.be.false;
-    expect(preloadDispatched).to.be.false;
   });
 
   suite('load', () => {
@@ -162,29 +155,22 @@ suite('Loading', () => {
 
   suite('loading', () => {
     suite('src changes quickly', () => {
-      test('eventually notifies that current src is preloaded', async () => {
+      test('eventually notifies that current src is loaded', async () => {
         element.loading = 'eager';
         element.src = CUBE_GLB_PATH;
 
-        await timePasses();
+        const loadCubeEvent =
+            waitForEvent(element, 'load') as Promise<CustomEvent>;
 
-        let preloadEvent = null;
-        const onPreload = (event: CustomEvent) => {
-          if (event.detail.url === HORSE_GLB_PATH) {
-            preloadEvent = event;
-          }
-        };
-        element.addEventListener<any>('preload', onPreload);
+        await timePasses();
 
         element.src = HORSE_GLB_PATH;
 
-        await until(() => element.loaded);
-
-        await timePasses();
-
-        element.removeEventListener<any>('preload', onPreload);
+        const loadCube = await loadCubeEvent;
+        const loadHorse = await waitForEvent(element, 'load') as CustomEvent;
 
-        expect(preloadEvent).to.be.ok;
+        expect(loadCube.detail.url).to.be.eq(CUBE_GLB_PATH);
+        expect(loadHorse.detail.url).to.be.eq(HORSE_GLB_PATH);
       });
     });
 

packages/model-viewer/src/test/three-components/ModelScene-spec.ts

@@ -44,15 +44,6 @@ suite('ModelScene', () => {
     document.body.removeChild(element);
   });
 
-  suite('setModelSource', () => {
-    test('fires a model-load event when loaded', async function() {
-      let fired = false;
-      scene.addEventListener('model-load', () => fired = true);
-      await scene.setSource(assetPath('models/Astronaut.glb'));
-      expect(fired).to.be.ok;
-    });
-  });
-
   suite('with a model', () => {
     setup(async () => {
       await scene.setSource(assetPath('models/Astronaut.glb'));

packages/model-viewer/src/three-components/ARRenderer.ts

@@ -231,7 +231,7 @@ export class ARRenderer extends EventDispatcher {
 
     this.oldTarget.copy(scene.getTarget());
 
-    scene.addEventListener('model-load', this.onUpdateScene);
+    scene.element.addEventListener('load', this.onUpdateScene);
 
     const radians = HIT_ANGLE_DEG * Math.PI / 180;
     const ray = this.placeOnWall === true ?
@@ -350,7 +350,7 @@ export class ARRenderer extends EventDispatcher {
       scene.setTarget(point.x, point.y, point.z);
       scene.xrCamera = null;
 
-      scene.removeEventListener('model-load', this.onUpdateScene);
+      scene.element.removeEventListener('load', this.onUpdateScene);
       scene.orientHotspots(0);
       element.requestUpdate('cameraTarget');
       element.requestUpdate('maxCameraOrbit');

packages/model-viewer/src/three-components/ModelScene.ts

@@ -204,8 +204,6 @@ export class ModelScene extends Scene {
 
       this.boundingSphere.radius = framingInfo.framedRadius;
       this.idealAspect = framingInfo.fieldOfViewAspect;
-
-      this.dispatchEvent({type: 'model-load', url: this.url});
       return;
     }
 
@@ -272,7 +270,6 @@ export class ModelScene extends Scene {
 
     this.updateShadow();
     this.setShadowIntensity(this.shadowIntensity);
-    this.dispatchEvent({type: 'model-load', url: this.url});
   }
 
   reset() {

packages/modelviewer.dev/data/docs.json

@@ -220,11 +220,6 @@
         "htmlName": "posterDismissed",
         "description": "Fires when the poster has fully transitioned away. Additional delay from the \"load\" event can be caused by the renderer compiling its shaders."
       },
-      {
-        "name": "preload",
-        "htmlName": "preload",
-        "description": "When <span class='attribute'>loading=\"eager\"</span> this event is fired when preloading is done."
-      },
       {
         "name": "model-visibility",
         "htmlName": "modelVisibility",