From a3f719e469d4e277d3bc8ce074432de421fdbbe3 Mon Sep 17 00:00:00 2001 From: David Catuhe Date: Thu, 28 Nov 2024 20:03:42 -0800 Subject: [PATCH] Fix disposing of effects for submeshes (#15910) --- .build/config.json | 8 ++++---- packages/dev/core/src/Engines/thinEngine.ts | 2 +- .../dev/core/src/Materials/drawWrapper.ts | 19 +++++++++++++++++-- packages/dev/core/src/Materials/effect.ts | 16 ++++++++++++++-- packages/dev/core/src/Meshes/abstractMesh.ts | 16 +++++++++------- packages/dev/core/src/Meshes/subMesh.ts | 16 +++++++++------- 6 files changed, 54 insertions(+), 23 deletions(-) diff --git a/.build/config.json b/.build/config.json index 8f2a6aceb0d..99d444fc11c 100644 --- a/.build/config.json +++ b/.build/config.json @@ -1,5 +1,5 @@ { - "versionDefinition": "minor", - "preid": "rc", - "nonce": 298 -} \ No newline at end of file + "versionDefinition": "minor", + "preid": "rc", + "nonce": 299 +} diff --git a/packages/dev/core/src/Engines/thinEngine.ts b/packages/dev/core/src/Engines/thinEngine.ts index 69f5a333e68..13d711e9d16 100644 --- a/packages/dev/core/src/Engines/thinEngine.ts +++ b/packages/dev/core/src/Engines/thinEngine.ts @@ -4037,7 +4037,7 @@ export class ThinEngine extends AbstractEngine { const keys = Object.keys(this._compiledEffects); for (const name of keys) { const effect = this._compiledEffects[name]; - effect.dispose(); + effect.dispose(true); } this._compiledEffects = {}; diff --git a/packages/dev/core/src/Materials/drawWrapper.ts b/packages/dev/core/src/Materials/drawWrapper.ts index cbaff3d7ec4..86178f7f627 100644 --- a/packages/dev/core/src/Materials/drawWrapper.ts +++ b/packages/dev/core/src/Materials/drawWrapper.ts @@ -5,6 +5,7 @@ import type { Nullable } from "../types"; import type { AbstractEngine } from "../Engines/abstractEngine"; import type { Effect } from "./effect"; import type { MaterialDefines } from "./materialDefines"; +import { TimingTools } from "core/Misc/timingTools"; /** @internal */ export class DrawWrapper { @@ -54,9 +55,23 @@ export class DrawWrapper { } } - public dispose(): void { + /** + * Dispose the effect wrapper and its resources + * @param immediate if the effect should be disposed immediately or on the next frame. + * If dispose() is not called during a scene or engine dispose, we want to delay the dispose of the underlying effect. Mostly to give a chance to user code to reuse the effect in some way. + */ + public dispose(immediate = false): void { if (this.effect) { - this.effect.dispose(); + const effect = this.effect; + if (immediate) { + effect.dispose(); + } else { + TimingTools.SetImmediate(() => { + effect.getEngine().onEndFrameObservable.addOnce(() => { + effect.dispose(); + }); + }); + } this.effect = null; } this.drawContext?.dispose(); diff --git a/packages/dev/core/src/Materials/effect.ts b/packages/dev/core/src/Materials/effect.ts index 147f60ea961..9737dce1521 100644 --- a/packages/dev/core/src/Materials/effect.ts +++ b/packages/dev/core/src/Materials/effect.ts @@ -195,6 +195,13 @@ export class Effect implements IDisposable { private _isDisposed = false; + /** + * Gets a boolean indicating that the effect was already disposed + */ + public get isDisposed(): boolean { + return this._isDisposed; + } + /** @internal */ public _refCount = 1; @@ -1466,9 +1473,14 @@ export class Effect implements IDisposable { /** * Release all associated resources. + * @param force specifies if the effect must be released no matter what **/ - public dispose() { - this._refCount--; + public dispose(force = false) { + if (force) { + this._refCount = 0; + } else { + this._refCount--; + } if (this._refCount > 0 || this._isDisposed) { // Others are still using the effect or the effect was already disposed diff --git a/packages/dev/core/src/Meshes/abstractMesh.ts b/packages/dev/core/src/Meshes/abstractMesh.ts index 385ae18e413..32d56f36cf8 100644 --- a/packages/dev/core/src/Meshes/abstractMesh.ts +++ b/packages/dev/core/src/Meshes/abstractMesh.ts @@ -670,7 +670,7 @@ export abstract class AbstractMesh extends TransformNode implements IDisposable, return; } - this.resetDrawCache(); + this.resetDrawCache(undefined, value == null); this._unBindEffect(); } @@ -1257,14 +1257,15 @@ export abstract class AbstractMesh extends TransformNode implements IDisposable, /** * Resets the draw wrappers cache for all submeshes of this abstract mesh * @param passId If provided, releases only the draw wrapper corresponding to this render pass id + * @param immediate If true, the effect will be released immediately, otherwise it will be released at the next frame */ - public resetDrawCache(passId?: number): void { + public resetDrawCache(passId?: number, immediate = false): void { if (!this.subMeshes) { return; } for (const subMesh of this.subMeshes) { - subMesh.resetDrawCache(passId); + subMesh.resetDrawCache(passId, immediate); } } @@ -2180,13 +2181,14 @@ export abstract class AbstractMesh extends TransformNode implements IDisposable, } /** - * Disposes all the submeshes of the current meshnp + * Disposes all the submeshes of the current mesh + * @param immediate should dispose the effects immediately or not * @returns the current mesh */ - public releaseSubMeshes(): AbstractMesh { + public releaseSubMeshes(immediate = false): AbstractMesh { if (this.subMeshes) { while (this.subMeshes.length) { - this.subMeshes[0].dispose(); + this.subMeshes[0].dispose(immediate); } } else { this.subMeshes = [] as SubMesh[]; @@ -2283,7 +2285,7 @@ export abstract class AbstractMesh extends TransformNode implements IDisposable, // SubMeshes if (this.getClassName() !== "InstancedMesh" || this.getClassName() !== "InstancedLinesMesh") { - this.releaseSubMeshes(); + this.releaseSubMeshes(true); } // Query diff --git a/packages/dev/core/src/Meshes/subMesh.ts b/packages/dev/core/src/Meshes/subMesh.ts index a8348461b38..951327caf2a 100644 --- a/packages/dev/core/src/Meshes/subMesh.ts +++ b/packages/dev/core/src/Meshes/subMesh.ts @@ -61,9 +61,9 @@ export class SubMesh implements ICullable { /** * @internal */ - public _removeDrawWrapper(passId: number, disposeWrapper = true) { + public _removeDrawWrapper(passId: number, disposeWrapper = true, immediate = false): void { if (disposeWrapper) { - this._drawWrappers[passId]?.dispose(); + this._drawWrappers[passId]?.dispose(immediate); } this._drawWrappers[passId] = undefined as any; } @@ -114,15 +114,16 @@ export class SubMesh implements ICullable { /** * Resets the draw wrappers cache * @param passId If provided, releases only the draw wrapper corresponding to this render pass id + * @param immediate If true, the draw wrapper will dispose the effect immediately (false by default) */ - public resetDrawCache(passId?: number): void { + public resetDrawCache(passId?: number, immediate = false): void { if (this._drawWrappers) { if (passId !== undefined) { - this._removeDrawWrapper(passId); + this._removeDrawWrapper(passId, true, immediate); return; } else { for (const drawWrapper of this._drawWrappers) { - drawWrapper?.dispose(); + drawWrapper?.dispose(immediate); } } } @@ -697,8 +698,9 @@ export class SubMesh implements ICullable { /** * Release associated resources + * @param immediate If true, the effect will be disposed immediately (false by default) */ - public dispose(): void { + public dispose(immediate = false): void { if (this._linesIndexBuffer) { this._mesh.getScene().getEngine()._releaseBuffer(this._linesIndexBuffer); this._linesIndexBuffer = null; @@ -708,7 +710,7 @@ export class SubMesh implements ICullable { const index = this._mesh.subMeshes.indexOf(this); this._mesh.subMeshes.splice(index, 1); - this.resetDrawCache(); + this.resetDrawCache(undefined, immediate); } /**