Skip to content

Fix Native shadow depth wrappers validation on engines without CSM - #1950

Open
CedricGuillemet wants to merge 1 commit into
BabylonJS:masterfrom
CedricGuillemet:cedricguillemet-native-shadow-fixture-ci
Open

CedricGuillemet wants to merge 1 commit into
BabylonJS:masterfrom
CedricGuillemet:cedricguillemet-native-shadow-fixture-ci

Conversation

@CedricGuillemet

@CedricGuillemet CedricGuillemet commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The Shadow depth wrappers Playground fixture chooses CascadedShadowGenerator whenever engine.webGLVersion >= 2. Babylon Native advertises WebGL 2 compatibility but does not support CSM, so the newly enabled fixture fails before rendering. Select the cascaded generator using Babylon.js's public CascadedShadowGenerator.IsSupported capability check instead; supported engines retain the cascaded path, and Native uses the snippet's existing ShadowGenerator fallback. The fixture, reference image, render count, and pixel thresholds remain unchanged.

This corrects the Native validation failure in BabylonJS/Babylon.js#18996 (Azure build 60940). CI recovery requires a refreshed master Native nightly artifact after this fix lands.

Verification

With nightly 60909 Playground.exe and the exact BabylonJS/Babylon.js#18996 CI merge snapshot (e89869471b95e5d9eca92745a465a453d2c151e4), running Playground.exe --headless --once --test "Shadow depth wrappers" app:///Scripts/validation_native.js:

  • Before: exit -1, CascadedShadowMap is not supported by the current engine, ran=1 passed=0 failed=1.
  • After: exit 0, ran=1 passed=1 failed=0; pixel difference 73 / 240000 (0.030%), under existing 2.5% allowance.

The updated fixture also passes with nightly 60909's original npm JavaScript payload: exit 0, ran=1 passed=1 failed=0, same pixel difference.

The full sequential Native visualization suite also passes against the exact failing Babylon.js CI snapshot plus this one-line fixture correction: Playground.exe --headless app:///Scripts/validation_native.js, exit 0, ran=471 passed=471 failed=0 missingRef=0 skipped=249, in 11m 37s. No exclusions, reference images, or thresholds were changed.

JSON parsing, whole-config semantic comparison, and git diff --check pass. Implemented with Sol and reviewed with Astra.

Use the public CSM support check instead of WebGL version so the Native validation fixture selects standard shadows when cascaded shadows are unavailable.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 54a3a23d-f251-49e7-a66c-3ad9e51b28e1

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused replacement matches the loader’s format and generator support check, with no unresolved findings.

0 open findings

What changed in this PR

Updates the “Shadow depth wrappers” validation fixture to select cascaded shadows by capability rather than WebGL version, allowing Babylon Native to use the existing fallback.

Changes:

  • Uses CascadedShadowGenerator.IsSupported for generator selection.
  • Leaves render count, reference image, and pixel thresholds unchanged.
File Description
Apps/​Playground/​Scripts/​config.json Adds the fixture’s capability-check replacement.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@bghgary bghgary left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Reviewed by Copilot on behalf of @bghgary]

Concerns inline.

{
"title": "Shadow depth wrappers",
"playgroundId": "#PNQRY1#73",
"replace": "engine.webGLVersion >= 2, BABYLON.CascadedShadowGenerator.IsSupported",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine as a temporary mitigation to unblock validation. Please add a note to this test entry explaining that the source Playground still needs its capability check fixed, then update playgroundId to the corrected revision and remove the replacement immediately afterward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants