Playground validation: once-only failure cleanup and pixel-count wording

Apps/Playground/Scripts/validation_native.js

@@ -37,6 +37,8 @@
     }
 
     function failTest(done) {
+        // done is the once-only completion wrapper from runTest; it stops the
+        // render loop, runs inter-test cleanup, then advances the suite.
         if (breakOnFail) {
             // Trigger the JS debugger if attached; on no-debugger runs the
             // host's bx exception filter prints a callstack on the next throw.
@@ -46,15 +48,88 @@
         done(false);
     }
 
+    // Dispose the current/stray scenes and reset engine state so the next test
+    // starts clean. Safe to call more than once (guards on currentScene).
+    function cleanupAfterTest() {
+        if (currentScene) {
+            try { currentScene.dispose(); } catch (e) { console.error(e); }
+            currentScene = null;
+        }
+
+        // A test can leave extra scenes behind (an async load that created its own scene, a scene
+        // whose creation promise resolved after validation, ...). They stay registered on the
+        // reused engine and keep their resources alive, so dispose them here.
+        if (engine && engine.scenes) {
+            const strayScenes = engine.scenes.slice();
+            for (let i = 0; i < strayScenes.length; ++i) {
+                try { strayScenes[i].dispose(); } catch (e) { console.error(e); }
+            }
+        }
+
+        if (!engine) {
+            return;
+        }
+
+        engine.setHardwareScalingLevel(1);
+
+        // Reset render state that persists on the reused engine so each test starts fresh.
+        // A test that leaves the stencil test enabled or a scissor rect set would otherwise
+        // corrupt later tests (e.g. the glow-layer test).
+        engine.setStencilBuffer(false);
+        engine.disableScissor();
+
+        // This is necessary because of https://github.com/BabylonJS/Babylon.js/pull/15217 so that each test starts fresh.
+        engine.releaseEffects();
+
+        // Textures are cached on the engine by URL (BaseTexture._getFromCache), and the cache key
+        // covers only url/noMipmap/isCube -- not the load-time options. A test that leaves a
+        // reference behind (e.g. assigning one texture to both scene.environmentTexture and a
+        // material's reflectionTexture) keeps its internal texture in that cache across
+        // scene.dispose(), so a later test loading the same URL silently reuses the *previous*
+        // test's texture along with its prefiltering/irradiance settings. Release whatever is
+        // left so every test loads its own textures and results do not depend on run order.
+        const leakedTextures = engine.getLoadedTexturesCache();
+        for (let i = leakedTextures.length - 1; i >= 0; --i) {
+            engine._releaseTexture(leakedTextures[i]);
+        }
+        engine.clearInternalTexturesCache();
+
+        // SceneLoader.OnPluginActivatedObservable is global and outlives the scene. Snippets use it
+        // to configure the glTF loader (animationStartMode, compileMaterials, ...) and never
+        // unregister, so without this every later glTF test would inherit those settings. The
+        // browser harness reloads the page per test and never sees this; here the engine is reused.
+        BABYLON.SceneLoader.OnPluginActivatedObservable.clear();
+    }
+
+    // Wrap the recursiveRunTest callback so every completion path (pixel pass/fail,
+    // render-loop throw, onReadyTimeout, load/eval errors) stops the loop, cleans once,
+    // and only then schedules the next test. Babylon runRenderLoop appends callbacks;
+    // without this, a failing test can leave a stale callback that renders the next
+    // scene or calls done(false) repeatedly under continue-on-failure.
+    function makeTestDone(outerDone) {
+        let finished = false;
+        return function (status) {
+            if (finished) {
+                return;
+            }
+            finished = true;
+            try {
+                engine.stopRenderLoop();
+            } catch (e) {
+                console.error(e);
+            }
+            cleanupAfterTest();
+            outerDone(status);
+        };
+    }
+
     // Emitted after a pixel-comparison failure to make triage faster. Prints the
     // rendered/diff PNG paths plus a re-run command. For scenes fetched from the
     // snippet server it also notes that assets/fonts arrive over the network, so
-    // async load timing is one possible cause -- but it is only one of several,
-    // and it is cheap to rule out: a timing flake varies run to run, while a real
-    // regression reproduces with an identical pixel-difference count. Say that
-    // explicitly. The previous wording called such diffs "often a transient flake",
-    // which led to a genuine, perfectly reproducible motion-blur regression being
-    // waved off for two weeks of nightlies.
+    // async load timing is one possible cause. A stable pixel-difference count on
+    // re-runs is a reason to open the saved result/diff images -- not proof that
+    // timing has been ruled out (the count discards which pixels changed and by
+    // how much, and a repeatable timing failure can produce the same fallback).
     function logFailureDiagnostics(test) {
         const outDir = TestUtils.getOutputDirectory();
         if (test.referenceImage) {
@@ -64,7 +139,7 @@
         if (test.playgroundId) {
             console.log(`  Note: this test loads playgroundId ${test.playgroundId} from the snippet server and pulls GUI/assets/fonts over the network, so async asset/font-load timing is one possible cause of a pixel diff.`);
         }
-        console.log("  Re-run in isolation; an identical pixel count on repeat runs means a real regression, not a timing flake:");
+        console.log("  Re-run in isolation; a stable pixel-difference count on repeat runs is a reason to compare the saved result/diff images (not proof timing is ruled out):");
         console.log(`    Playground --headless --once --test "${test.title || ""}" app:///Scripts/validation_native.js`);
     }
 
@@ -227,47 +302,7 @@
             }
         }
 
-        currentScene.dispose();
-        currentScene = null;
-
-        // A test can leave extra scenes behind (an async load that created its own scene, a scene
-        // whose creation promise resolved after validation, ...). They stay registered on the
-        // reused engine and keep their resources alive, so dispose them here.
-        const strayScenes = engine.scenes.slice();
-        for (let i = 0; i < strayScenes.length; ++i) {
-            strayScenes[i].dispose();
-        }
-
-        engine.setHardwareScalingLevel(1);
-
-        // Reset render state that persists on the reused engine so each test starts fresh.
-        // A test that leaves the stencil test enabled or a scissor rect set would otherwise
-        // corrupt later tests (e.g. the glow-layer test).
-        engine.setStencilBuffer(false);
-        engine.disableScissor();
-
-        // This is necessary because of https://github.com/BabylonJS/Babylon.js/pull/15217 so that each test starts fresh.
-        engine.releaseEffects();
-
-        // Textures are cached on the engine by URL (BaseTexture._getFromCache), and the cache key
-        // covers only url/noMipmap/isCube -- not the load-time options. A test that leaves a
-        // reference behind (e.g. assigning one texture to both scene.environmentTexture and a
-        // material's reflectionTexture) keeps its internal texture in that cache across
-        // scene.dispose(), so a later test loading the same URL silently reuses the *previous*
-        // test's texture along with its prefiltering/irradiance settings. Release whatever is
-        // left so every test loads its own textures and results do not depend on run order.
-        const leakedTextures = engine.getLoadedTexturesCache();
-        for (let i = leakedTextures.length - 1; i >= 0; --i) {
-            engine._releaseTexture(leakedTextures[i]);
-        }
-        engine.clearInternalTexturesCache();
-
-        // SceneLoader.OnPluginActivatedObservable is global and outlives the scene. Snippets use it
-        // to configure the glTF loader (animationStartMode, compileMaterials, ...) and never
-        // unregister, so without this every later glTF test would inherit those settings. The
-        // browser harness reloads the page per test and never sees this; here the engine is reused.
-        BABYLON.SceneLoader.OnPluginActivatedObservable.clear();
-
+        // Inter-test cleanup + once-only guard live in makeTestDone (see runTest).
         done(testRes);
     }
 
@@ -585,9 +620,11 @@
             request.send(null);
         }
     }
-    function runTest(index, done) {
+    function runTest(index, outerDone) {
+        const done = makeTestDone(outerDone);
         if (index >= config.tests.length) {
             done(false);
+            return;
         }
 
         const test = config.tests[index];