Skip to content

Commit 61435b8

Browse files
authored
Canvas leak (#3927)
* fixed setTimeout leak * dispose shadow * make context failure throw again * go back to catching context error * fix GLTFParser leak
1 parent 5974288 commit 61435b8

File tree

5 files changed

+36
-10
lines changed

5 files changed

+36
-10
lines changed

packages/model-viewer/src/features/scene-graph.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ import {Texture as ModelViewerTexture} from './scene-graph/texture';
3131

3232

3333
export const $currentGLTF = Symbol('currentGLTF');
34-
const $model = Symbol('model');
34+
export const $originalGltfJson = Symbol('originalGltfJson');
35+
export const $model = Symbol('model');
3536
const $getOnUpdateMethod = Symbol('getOnUpdateMethod');
3637
const $textureLoader = Symbol('textureLoader');
37-
const $originalGltfJson = Symbol('originalGltfJson');
3838

3939
interface SceneExportOptions {
4040
binary?: boolean, trs?: boolean, onlyVisible?: boolean,

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,8 @@ export default class ModelViewerElementBase extends ReactiveElement {
368368
renderer.unregisterScene(this[$scene]);
369369

370370
this[$clearModelTimeout] = self.setTimeout(() => {
371-
this[$scene].reset();
371+
this[$scene].dispose();
372+
this[$clearModelTimeout] = null;
372373
}, CLEAR_MODEL_TIMEOUT_MS);
373374
}
374375

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
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';
1717
import {CSS2DRenderer} from 'three/examples/jsm/renderers/CSS2DRenderer.js';
1818

19+
import {$currentGLTF, $model, $originalGltfJson} from '../features/scene-graph.js';
1920
import ModelViewerElementBase, {$renderer, RendererInterface} from '../model-viewer-base.js';
2021
import {ModelViewerElement} from '../model-viewer.js';
2122
import {normalizeUnit} from '../styles/conversions.js';
@@ -64,7 +65,6 @@ const ndc = new Vector2();
6465
export class ModelScene extends Scene {
6566
public element: ModelViewerElement;
6667
public canvas: HTMLCanvasElement;
67-
public context: CanvasRenderingContext2D|null = null;
6868
public annotationRenderer = new CSS2DRenderer();
6969
public schemaElement = document.createElement('script');
7070
public width = 1;
@@ -148,8 +148,8 @@ export class ModelScene extends Scene {
148148
* directly. This extra context is necessary to copy the renderings into when
149149
* there are more than one.
150150
*/
151-
createContext() {
152-
this.context = this.canvas.getContext('2d');
151+
get context() {
152+
return this.canvas.getContext('2d');
153153
}
154154

155155
getCamera(): Camera {
@@ -236,6 +236,7 @@ export class ModelScene extends Scene {
236236
throw error;
237237
}
238238

239+
this.cancelPendingSourceChange = null;
239240
this.reset();
240241
this.url = url;
241242
this._currentGLTF = gltf;
@@ -300,6 +301,17 @@ export class ModelScene extends Scene {
300301
this.mixer.uncacheRoot(this);
301302
}
302303

304+
dispose() {
305+
this.reset();
306+
if (this.shadow != null) {
307+
this.shadow.dispose();
308+
this.shadow = null;
309+
}
310+
(this.element as any)[$currentGLTF] = null;
311+
(this.element as any)[$originalGltfJson] = null;
312+
(this.element as any)[$model] = null;
313+
}
314+
303315
get currentGLTF() {
304316
return this._currentGLTF;
305317
}

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,6 @@ export class Renderer extends EventDispatcher {
374374
}
375375

376376
private copyPixels(scene: ModelScene, width: number, height: number) {
377-
if (scene.context == null) {
378-
scene.createContext();
379-
}
380377
const context2D = scene.context;
381378
if (context2D == null) {
382379
console.log('could not acquire 2d context');

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* limitations under the License.
1414
*/
1515

16-
import {BackSide, Box3, Mesh, MeshBasicMaterial, MeshDepthMaterial, Object3D, OrthographicCamera, PlaneGeometry, RGBAFormat, Scene, ShaderMaterial, Vector3, WebGLRenderer, WebGLRenderTarget, WebGLRenderTargetOptions} from 'three';
16+
import {BackSide, Box3, Material, Mesh, MeshBasicMaterial, MeshDepthMaterial, Object3D, OrthographicCamera, PlaneGeometry, RGBAFormat, Scene, ShaderMaterial, Vector3, WebGLRenderer, WebGLRenderTarget, WebGLRenderTargetOptions} from 'three';
1717
import {HorizontalBlurShader} from 'three/examples/jsm/shaders/HorizontalBlurShader.js';
1818
import {VerticalBlurShader} from 'three/examples/jsm/shaders/VerticalBlurShader.js';
1919
import {lerp} from 'three/src/math/MathUtils.js';
@@ -330,4 +330,20 @@ export class Shadow extends Object3D {
330330

331331
blurPlane.visible = false;
332332
}
333+
334+
dispose() {
335+
if (this.renderTarget != null) {
336+
this.renderTarget.dispose();
337+
}
338+
if (this.renderTargetBlur != null) {
339+
this.renderTargetBlur.dispose();
340+
}
341+
this.depthMaterial.dispose();
342+
this.horizontalBlurMaterial.dispose();
343+
this.verticalBlurMaterial.dispose();
344+
(this.floor.material as Material).dispose();
345+
this.floor.geometry.dispose();
346+
this.blurPlane.geometry.dispose();
347+
this.removeFromParent();
348+
}
333349
}

0 commit comments

Comments
 (0)