Preserve animation frames during screenshot readiness renders#1762
Preserve animation frames during screenshot readiness renders#1762matthargett wants to merge 5 commits 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.
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.
bkaradzic-microsoft
left a comment
bkaradzic-microsoft
left a comment
There was a problem hiding this comment.
Thanks — the readiness-pump approach is a nice way to warm up compilation without spending the animation frame budget, and the additive/opt-out design is good. Two things I'd like addressed before merge (details inline):
1. Non-deterministic readiness state. scene.render(true, true) skips animation stepping, but it does not freeze particle systems, procedural textures, or onBefore/AfterRenderObservable-driven state — those advance on every render. Since the number of readiness frames depends on how long the scene takes to become ready (varies by machine/CI load), sensitive screenshot tests can see a variable amount of pre-frame-1 state drift. With the pump default-on for all tests, this risks reintroducing pixel-diff flakiness. Could we either (a) make the pump opt-in per test, or (b) also suspend particles/procedural updates during readiness (or reset render state after the pump stops) so frame 1 is deterministic?
2. Pre-ready render exceptions become hard failures. A throw from scene.render() in the pump routes to onFatalError → failTest(done). Rendering a not-yet-ready scene can transiently throw; treating that as a definitive failure can convert would-pass tests into failures. Suggest tolerating readiness-phase exceptions (log/count, keep polling) and failing only once the scene is actually ready.
…s-ignore-animations
Limit speculative pre-ready renders to validation cases that need shader and material compilation progress without advancing their configured animation frames. This avoids changing particles, procedural textures, and render-observer state in the rest of the screenshot catalog.\n\nKeep readiness-phase render exceptions visible while polling continues. Record the first and last failures with their stacks, report recovered exception counts, and include retained diagnostics when a scene times out; the normal post-ready validation render remains a hard failure.
Default top-level macOS builds to the selected Xcode SDK instead of allowing Command Line Tools headers to mix with Xcode frameworks. Mark the JavaScriptCore framework search path as a system framework through napi so JsRuntimeHost's pedantic warnings-as-errors policy ignores SDK implementation diagnostics without suppressing project warnings.\n\nThis fixes Xcode 26.5 nullability, invalid UTF-8, and related framework-header failures while leaving explicit cross-compilation toolchains and Apple mobile platforms unchanged.
Centralize readiness pump shutdown and summary collection so every completion and failure path stops the timer consistently. Mark timeout completion as stopped and avoid 60 Hz polling before an active camera is available while retaining render-rate polling once shader preparation can make progress.
|
the Android CI failure is an intermittent toolchain download problem that's unrelated to my changes |
|
Actually, we're considering different approach, not to diverge from B.js test harness... |
|
On the latest master, the readiness render pump is unnecessary and redundant. Can you explain exactly condition when you need this? |
let me retest with all my local enabled things and I'll report back. |
|
Retested against current BabylonNative master and Babylon.js master, including the merged BabylonJS/Babylon.js#18709 fix. I can no longer demonstrate a condition that requires the readiness render pump. Without it, Gaussian Splatting Loading, both serializer variants, the original GUI Slate playground, Near Menu, SpherePanel, and standard texture repetition all reached readiness and produced the expected comparisons. Enabling the pump added four pre-validation renders to Gaussian Splatting Loading but did not change its 9-pixel result. The remaining depth-prepass and VLS failures are independent shader-path issues and are not resolved by the pump. The instability that originally made the pump appear useful was a separate Native renderer screenshot-capture issue: the configured comparison frame could continue advancing while asynchronous screenshot readback was pending. Preserving the comparison frame directly stabilized the animated serializer tests without speculative readiness rendering. Closing this PR rather than adding redundant test-harness behavior. The unrelated Apple SDK build fix has been split into #1783. |
Summary
Add an opt-in render-readiness pump for screenshot tests whose material/effect preparation needs rendering before
executeWhenReadycompletes. Pump frames usescene.render(true, true)so they do not consume Babylon animation frames before the configured screenshot frame count begins.Enable the pump only for Gaussian Splatting Loading,
needDepthPrePass, and the two GLTF serializer skinning/animation variants. Other tests remain unchanged unless they setrenderReadinessPump: true;globalThis.__nativeValidationRenderReadinessPump = trueremains available as a diagnostic override.Pre-ready render exceptions are logged and counted while readiness polling continues. Every completion and failure path stops the pump consistently; before an active camera exists, polling is throttled to 100 ms.
Also make top-level macOS builds use one coherent Xcode SDK and classify JavaScriptCore framework headers as system headers. This avoids Xcode 26 SDK nullability diagnostics becoming JsRuntimeHost
-Werrorfailures without suppressing project warnings.Validation
node --check Apps/Playground/Scripts/validation_native.jsneedDepthPrePasspassed with 0 differing pixels