Fix disposing of effects for submeshes (#15910)

.build/config.json

@@ -1,5 +1,5 @@
 {
-  "versionDefinition": "minor",
-  "preid": "rc",
-  "nonce": 298
-}
+    "versionDefinition": "minor",
+    "preid": "rc",
+    "nonce": 299
+}

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 = {};

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();

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

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

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);
     }
 
     /**