Preserve animation frames during screenshot readiness renders#1762
Open
matthargett wants to merge 1 commit into
Open
Preserve animation frames during screenshot readiness renders#1762matthargett wants to merge 1 commit into
matthargett wants to merge 1 commit into
Conversation
Render screenshot readiness-pump frames with animations ignored so material and effect readiness can advance without consuming the configured validation frame count. The GLTF serializer skinning tests are already enabled on current upstream master, so this keeps them compatible with readiness-driven validation.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a “render readiness pump” to actively render scenes while waiting for executeWhenReady, aiming to compile materials/effects and prevent tests from silently hanging during readiness.
Changes:
- Introduces a configurable readiness render pump (
startSceneReadinessRenderPump) and gating logic (shouldUseRenderReadinessPump). - Stops the readiness pump on evaluation, readiness completion, timeout, and error paths.
- Adds logging around readiness pump rendering and failures.
Comment on lines
+296
to
+299
| if (readinessPump !== null) { | ||
| readinessPump.stop(); | ||
| readinessPump = null; | ||
| } |
Comment on lines
+315
to
+318
| if (readinessPump !== null) { | ||
| readinessPump.stop(); | ||
| readinessPump = null; | ||
| } |
Comment on lines
+341
to
+348
| if (readinessPump !== null) { | ||
| const readinessFrameCount = readinessPump.getFrameCount(); | ||
| readinessPump.stop(); | ||
| readinessPump = null; | ||
| if (readinessFrameCount > 0) { | ||
| console.log("Readiness render pump rendered " + readinessFrameCount + " frame(s) before validation frame counting for " + (test.title || "(unnamed)") + "."); | ||
| } | ||
| } |
Comment on lines
+388
to
+391
| if (readinessPump !== null) { | ||
| readinessPump.stop(); | ||
| readinessPump = null; | ||
| } |
Comment on lines
314
to
323
| currentScene.onReadyTimeoutObservable.addOnce(function () { | ||
| if (readinessPump !== null) { | ||
| readinessPump.stop(); | ||
| readinessPump = null; | ||
| } | ||
| evaluated = true; | ||
| console.error("Scene '" + (test.title || "?") + "' did not become ready within " + | ||
| (currentScene.onReadyTimeoutDuration / 1000) + "s."); | ||
| failTest(done); | ||
| }); |
Comment on lines
+89
to
+114
| const pump = function () { | ||
| if (stopped) { | ||
| return; | ||
| } | ||
| if (!scene || scene.isDisposed === true || (typeof scene.isDisposed === "function" && scene.isDisposed())) { | ||
| stopped = true; | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| if (scene.activeCamera && typeof scene.render === "function") { | ||
| // Readiness renders are only for material/effect compilation. | ||
| // Preserve the screenshot test's animation frame count. | ||
| scene.render(true, true); | ||
| frameCount++; | ||
| } | ||
| } catch (e) { | ||
| stopped = true; | ||
| if (typeof onFatalError === "function") { | ||
| onFatalError(e); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| setTimeout(pump, 16); | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This adds a small render-readiness pump to screenshot validation. While a scene is waiting for
executeWhenReady, the harness can render readiness frames so materials/effects get a chance to compile before validation frame counting starts.The pump calls
scene.render(true, true): update cameras, but ignore animations. That is the important part for animated screenshots. Readiness frames should prepare render state, not consume the test's configured animation frame budget.The pump can be disabled with
renderReadinessPump: falseon a test orglobalThis.__nativeValidationRenderReadinessPump = false.Current
masteralready has the GLTF serializer skinning tests enabled for the default path; this keeps readiness rendering compatible with those tests instead of shifting their animation state before frame 1.Validation
node --check Apps/Playground/Scripts/validation_native.js