From eb472ef156529787b4e076e1f0898910f73f4729 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Thu, 8 Oct 2026 14:51:44 -0700 Subject: [PATCH] Dispose stray validation scenes after every test #1920 moved stray-scene disposal behind the leak-cleanup workaround. When a test fails before currentScene is assigned (for example, wait-for-import rejects for the OpenPBR test), the scene it created was left on the reused engine for later tests. Dispose engine.scenes unconditionally again and keep only the texture-cache release and Draco reset behind leak-cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 97f1ddbf-b571-4a6c-8cb3-411b37e0d15a --- Apps/Playground/Scripts/validation_native.js | 18 +++++++++--------- Documentation/AddingNewValidationTests.md | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Apps/Playground/Scripts/validation_native.js b/Apps/Playground/Scripts/validation_native.js index c928876a8b..9596cce209 100644 --- a/Apps/Playground/Scripts/validation_native.js +++ b/Apps/Playground/Scripts/validation_native.js @@ -58,8 +58,8 @@ "undeclared-name": "declare `name` for the example code", // Example captures effect-layer output before its render targets were submitted once. "prime-effect-layers": "render one extra frame before capture when effect layers exist", - // Example leaves scenes, textures, or Draco state behind for later tests. - "leak-cleanup": "dispose leaked scenes and textures and reset Draco after the test", + // Example leaves textures or Draco state behind for later tests. + "leak-cleanup": "release leaked textures and reset Draco after the test", // Example starts an async model load in createScene without awaiting it. "wait-for-import": "wait for loader promises started by createScene before rendering", // Example clears opaque but writes alpha < 1 that the browser would composite. @@ -245,13 +245,6 @@ function cleanupLeaks(test) { let leaked = false; - // Async loads may leave additional scenes registered on the engine. - const strayScenes = engine.scenes.slice(); - leaked = leaked || strayScenes.length > 0; - for (let i = 0; i < strayScenes.length; ++i) { - try { strayScenes[i].dispose(); } catch (e) { console.error(e); } - } - // Cache keys omit load-time options; leaked textures can change later tests. const leakedTextures = engine.getLoadedTexturesCache(); leaked = leaked || leakedTextures.length > 0; @@ -287,6 +280,13 @@ return; } + // Async loads may leave additional scenes registered on the engine, including when a + // failed test never assigned currentScene, so dispose them for every test. + const strayScenes = engine.scenes.slice(); + for (let i = 0; i < strayScenes.length; ++i) { + try { strayScenes[i].dispose(); } catch (e) { console.error(e); } + } + if (hasWorkaround(test, "leak-cleanup")) { cleanupLeaks(test); } diff --git a/Documentation/AddingNewValidationTests.md b/Documentation/AddingNewValidationTests.md index 74e08d40ae..35b89c606a 100644 --- a/Documentation/AddingNewValidationTests.md +++ b/Documentation/AddingNewValidationTests.md @@ -53,7 +53,7 @@ TSV column. When a workaround changes what the runner does for a test, the runne | `wait-for-readiness` | Renders before utility-layer scenes, GUI images, or material effects are ready | Waits for those as described below | | `undeclared-name` | Reads the browser-only global `name` (`window.name`) | Declares `var name = ""` for the example code | | `prime-effect-layers` | Captures effect-layer (glow/highlight) output on the first frame | Renders one extra frame before counting frames | -| `leak-cleanup` | Leaves scenes, textures, or Draco state behind after disposal | Disposes stray scenes, releases leaked textures, and resets Draco after the test | +| `leak-cleanup` | Leaves textures or Draco state behind after disposal | Releases leaked textures and resets Draco after the test (stray scenes are disposed after every test) | | `wait-for-import` | Starts `ImportMeshAsync`/`AppendSceneAsync`/`LoadAssetContainerAsync` in `createScene` without awaiting it | Waits for loader promises started before the scene is returned | | `opaque-clear-alpha` | Clears opaque but leaves alpha < 1 in the final image | Skips canvas-background compositing unless the test sets `canvasBackgroundColor`, the scene uses a frame graph, or `clearColor.a < 1` |