From 61435b8b2aaf160b92317b8d27e00cf8e86eefb3 Mon Sep 17 00:00:00 2001 From: Emmett Lalish Date: Mon, 7 Nov 2022 17:26:04 +0100 Subject: [PATCH] Canvas leak (#3927) * fixed setTimeout leak * dispose shadow * make context failure throw again * go back to catching context error * fix GLTFParser leak --- .../model-viewer/src/features/scene-graph.ts | 4 ++-- packages/model-viewer/src/model-viewer-base.ts | 3 ++- .../src/three-components/ModelScene.ts | 18 +++++++++++++++--- .../src/three-components/Renderer.ts | 3 --- .../src/three-components/Shadow.ts | 18 +++++++++++++++++- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/packages/model-viewer/src/features/scene-graph.ts b/packages/model-viewer/src/features/scene-graph.ts index a092fa26d2..3e4d54de68 100644 --- a/packages/model-viewer/src/features/scene-graph.ts +++ b/packages/model-viewer/src/features/scene-graph.ts @@ -31,10 +31,10 @@ import {Texture as ModelViewerTexture} from './scene-graph/texture'; export const $currentGLTF = Symbol('currentGLTF'); -const $model = Symbol('model'); +export const $originalGltfJson = Symbol('originalGltfJson'); +export const $model = Symbol('model'); const $getOnUpdateMethod = Symbol('getOnUpdateMethod'); const $textureLoader = Symbol('textureLoader'); -const $originalGltfJson = Symbol('originalGltfJson'); interface SceneExportOptions { binary?: boolean, trs?: boolean, onlyVisible?: boolean, diff --git a/packages/model-viewer/src/model-viewer-base.ts b/packages/model-viewer/src/model-viewer-base.ts index 6b6cb2d9fa..49388598cf 100644 --- a/packages/model-viewer/src/model-viewer-base.ts +++ b/packages/model-viewer/src/model-viewer-base.ts @@ -368,7 +368,8 @@ export default class ModelViewerElementBase extends ReactiveElement { renderer.unregisterScene(this[$scene]); this[$clearModelTimeout] = self.setTimeout(() => { - this[$scene].reset(); + this[$scene].dispose(); + this[$clearModelTimeout] = null; }, CLEAR_MODEL_TIMEOUT_MS); } diff --git a/packages/model-viewer/src/three-components/ModelScene.ts b/packages/model-viewer/src/three-components/ModelScene.ts index 45ee6f936d..5d2a30ae1f 100644 --- a/packages/model-viewer/src/three-components/ModelScene.ts +++ b/packages/model-viewer/src/three-components/ModelScene.ts @@ -16,6 +16,7 @@ import {AnimationAction, AnimationClip, AnimationMixer, Box3, Camera, Euler, Event as ThreeEvent, LoopPingPong, LoopRepeat, Material, Matrix3, Mesh, Object3D, PerspectiveCamera, Raycaster, Scene, Sphere, Texture, Vector2, Vector3, WebGLRenderer} from 'three'; import {CSS2DRenderer} from 'three/examples/jsm/renderers/CSS2DRenderer.js'; +import {$currentGLTF, $model, $originalGltfJson} from '../features/scene-graph.js'; import ModelViewerElementBase, {$renderer, RendererInterface} from '../model-viewer-base.js'; import {ModelViewerElement} from '../model-viewer.js'; import {normalizeUnit} from '../styles/conversions.js'; @@ -64,7 +65,6 @@ const ndc = new Vector2(); export class ModelScene extends Scene { public element: ModelViewerElement; public canvas: HTMLCanvasElement; - public context: CanvasRenderingContext2D|null = null; public annotationRenderer = new CSS2DRenderer(); public schemaElement = document.createElement('script'); public width = 1; @@ -148,8 +148,8 @@ export class ModelScene extends Scene { * directly. This extra context is necessary to copy the renderings into when * there are more than one. */ - createContext() { - this.context = this.canvas.getContext('2d'); + get context() { + return this.canvas.getContext('2d'); } getCamera(): Camera { @@ -236,6 +236,7 @@ export class ModelScene extends Scene { throw error; } + this.cancelPendingSourceChange = null; this.reset(); this.url = url; this._currentGLTF = gltf; @@ -300,6 +301,17 @@ export class ModelScene extends Scene { this.mixer.uncacheRoot(this); } + dispose() { + this.reset(); + if (this.shadow != null) { + this.shadow.dispose(); + this.shadow = null; + } + (this.element as any)[$currentGLTF] = null; + (this.element as any)[$originalGltfJson] = null; + (this.element as any)[$model] = null; + } + get currentGLTF() { return this._currentGLTF; } diff --git a/packages/model-viewer/src/three-components/Renderer.ts b/packages/model-viewer/src/three-components/Renderer.ts index 2f46fa9830..1b9d70e764 100644 --- a/packages/model-viewer/src/three-components/Renderer.ts +++ b/packages/model-viewer/src/three-components/Renderer.ts @@ -374,9 +374,6 @@ export class Renderer extends EventDispatcher { } private copyPixels(scene: ModelScene, width: number, height: number) { - if (scene.context == null) { - scene.createContext(); - } const context2D = scene.context; if (context2D == null) { console.log('could not acquire 2d context'); diff --git a/packages/model-viewer/src/three-components/Shadow.ts b/packages/model-viewer/src/three-components/Shadow.ts index 487c9f87ef..ee2cc9a42d 100644 --- a/packages/model-viewer/src/three-components/Shadow.ts +++ b/packages/model-viewer/src/three-components/Shadow.ts @@ -13,7 +13,7 @@ * limitations under the License. */ -import {BackSide, Box3, Mesh, MeshBasicMaterial, MeshDepthMaterial, Object3D, OrthographicCamera, PlaneGeometry, RGBAFormat, Scene, ShaderMaterial, Vector3, WebGLRenderer, WebGLRenderTarget, WebGLRenderTargetOptions} from 'three'; +import {BackSide, Box3, Material, Mesh, MeshBasicMaterial, MeshDepthMaterial, Object3D, OrthographicCamera, PlaneGeometry, RGBAFormat, Scene, ShaderMaterial, Vector3, WebGLRenderer, WebGLRenderTarget, WebGLRenderTargetOptions} from 'three'; import {HorizontalBlurShader} from 'three/examples/jsm/shaders/HorizontalBlurShader.js'; import {VerticalBlurShader} from 'three/examples/jsm/shaders/VerticalBlurShader.js'; import {lerp} from 'three/src/math/MathUtils.js'; @@ -330,4 +330,20 @@ export class Shadow extends Object3D { blurPlane.visible = false; } + + dispose() { + if (this.renderTarget != null) { + this.renderTarget.dispose(); + } + if (this.renderTargetBlur != null) { + this.renderTargetBlur.dispose(); + } + this.depthMaterial.dispose(); + this.horizontalBlurMaterial.dispose(); + this.verticalBlurMaterial.dispose(); + (this.floor.material as Material).dispose(); + this.floor.geometry.dispose(); + this.blurPlane.geometry.dispose(); + this.removeFromParent(); + } } \ No newline at end of file