Fix render (google#3727)

* copy pixels earlier * fixed tests
e752938 · Aug 24, 2022 · c6107ae · c6107ae
1 parent 8541890
commit c6107ae
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 21 deletions.
diff --git a/packages/model-viewer/src/test/three-components/Renderer-spec.ts b/packages/model-viewer/src/test/three-components/Renderer-spec.ts
@@ -190,9 +190,7 @@ suite('Renderer with two scenes', () => {
     test('uses the proper canvas when unregistering scenes', () => {
       renderer.render(performance.now());
 
-      expect(renderer.canvas3D.classList.contains('show'))
-          .to.be.eq(
-              false, 'webgl canvas should not be shown with multiple scenes.');
+      expect(renderer.canvas3D.parentElement).to.be.not.ok;
       expect(scene.canvas.classList.contains('show'))
           .to.be.eq(true, 'scene canvas should be shown with multiple scenes.');
       expect(otherScene.canvas.classList.contains('show'))
@@ -204,9 +202,9 @@ suite('Renderer with two scenes', () => {
       renderer.render(performance.now());
 
       expect(renderer.canvas3D.parentElement)
-          .to.be.eq(otherScene.canvas.parentElement);
-      expect(renderer.canvas3D.classList.contains('show'))
-          .to.be.eq(true, 'webgl canvas should be shown with single scene.');
+          .to.be.eq(
+              otherScene.canvas.parentElement,
+              'webgl canvas should be shown with single scene.');
       expect(otherScene.canvas.classList.contains('show'))
           .to.be.eq(
               false,
@@ -222,8 +220,7 @@ suite('Renderer with two scenes', () => {
           .to.be.equal(1, 'scene should have rendered once');
       expect(otherScene.renderCount)
           .to.be.equal(1, 'otherScene should have rendered once');
-      expect(renderer.canvas3D.classList.contains('show'))
-          .to.be.eq(false, 'webgl canvas should still not be shown.');
+      expect(renderer.canvas3D.parentElement).to.be.not.ok;
       expect(otherScene.canvas.classList.contains('show'))
           .to.be.eq(true, 'otherScene canvas should still be shown.');
     });

diff --git a/packages/model-viewer/src/three-components/Renderer.ts b/packages/model-viewer/src/three-components/Renderer.ts
@@ -134,6 +134,7 @@ export class Renderer extends EventDispatcher {
 
     this.canvas3D = document.createElement('canvas');
     this.canvas3D.id = 'webgl-canvas';
+    this.canvas3D.classList.add('show');
 
     try {
       this.threeRenderer = new WebGLRenderer({
@@ -290,9 +291,6 @@ export class Renderer extends EventDispatcher {
     canvas.style.width = `${this.width / scale}px`;
     canvas.style.height = `${this.height / scale}px`;
 
-    if (this.multipleScenesVisible) {
-      canvas.classList.add('show');
-    }
     scene.queueRender();
 
     this.dispatchRenderScale(scene);
@@ -310,6 +308,10 @@ export class Renderer extends EventDispatcher {
   unregisterScene(scene: ModelScene) {
     this.scenes.delete(scene);
 
+    if (this.canvas3D.parentElement === scene.canvas.parentElement) {
+      scene.canvas.parentElement!.removeChild(this.canvas3D);
+    }
+
     if (this.canRender && this.scenes.size === 0) {
       this.threeRenderer.setAnimationLoop(null);
     }
@@ -330,23 +332,29 @@ export class Renderer extends EventDispatcher {
    * renderer's result into it.
    */
   private countVisibleScenes() {
+    const {canvas3D} = this;
     let visibleScenes = 0;
     let canvas3DScene = null;
     for (const scene of this.scenes) {
       const {element} = scene;
       if (element.modelIsVisible && scene.externalRenderer == null) {
         ++visibleScenes;
       }
-      if (this.canvas3D.parentElement === scene.canvas.parentElement) {
+      if (canvas3D.parentElement === scene.canvas.parentElement) {
         canvas3DScene = scene;
       }
     }
     const multipleScenesVisible = visibleScenes > 1;
 
-    if (canvas3DScene != null && multipleScenesVisible &&
-        !this.multipleScenesVisible) {
-      const {width, height} = this.sceneSize(canvas3DScene);
-      this.copyPixels(canvas3DScene, width, height);
+    if (canvas3DScene != null) {
+      const newlyMultiple =
+          multipleScenesVisible && !this.multipleScenesVisible;
+      const disappearing = !canvas3DScene.element.modelIsVisible;
+      if (newlyMultiple || disappearing) {
+        const {width, height} = this.sceneSize(canvas3DScene);
+        this.copyPixels(canvas3DScene, width, height);
+        canvas3D.parentElement!.removeChild(canvas3D);
+      }
     }
     this.multipleScenesVisible = multipleScenesVisible;
   }
@@ -487,17 +495,12 @@ export class Renderer extends EventDispatcher {
         this.copyPixels(scene, width, height);
       } else {
         scene.canvas.parentElement!.appendChild(canvas3D);
-        canvas3D.classList.add('show');
         scene.canvas.classList.remove('show');
       }
 
       scene.hasRendered();
       ++scene.renderCount;
     }
-
-    if (this.multipleScenesVisible) {
-      canvas3D.classList.remove('show');
-    }
   }
 
   dispose(): Array<ModelViewerElementBase> {