Add cubemap auto-expand polyfill for Playground (re-enables 7 PBR tests) by bkaradzic-microsoft · Pull Request #1710 · BabylonJS/BabylonNative

bkaradzic-microsoft · 2026-05-18T16:35:50Z

Adds a JS polyfill that patches BABYLON.NativeEngine.prototype.createCubeTexture so single-URL .dds/.ktx/.ktx2 cubemap loads (which BN does not currently support) transparently retry as .env. Babylon's CDN co-hosts both formats from the same source HDR, so the swap is invisible to the playground.

Changes:

New Apps/Playground/Scripts/cube_texture_polyfill.js - patches createCubeTexture; retries via self._loadFile; falls back to the original throw on 404.
Apps/Playground/CMakeLists.txt - adds cube_texture_polyfill.js to SCRIPTS list.
Apps/Playground/Shared/AppContext.cpp - loads cube_texture_polyfill.js after babylon.max.js (the polyfill needs BABYLON.NativeEngine to exist).
Apps/Playground/Scripts/config.json - re-enables 7 affected tests (idx 141, 172, 173, 246, 247, 248, 290 - PBR/cubemap heavy).

Note: Bridges the gap until `Plugins/NativeEngine` wires a proper loader registry path for cubemap formats. Remaining 35 of 42 affected tests either exceed pixel-diff threshold once they reach the renderer or hit unrelated secondary bugs (particles, PrePassRenderer caps, Chakra parse, etc.) - all already filed.

Landing context

This PR is one of 7 splits from the proven CI-green combined preview in draft PR #1702 (see #1702 for the full intended end-state and verified CI run 26044922430).

Note: the original split included an 8th PR (#1709, ES2020+ -> ES2019 syntax-repair polyfill for Chakra). It was closed in favour of investigating @babel/standalone properly (#1711).

Recommended landing order

Tier 1 - parallel-reviewable, no source conflicts:

Fix ExternalTexture_OpenGL throw-stubs to avoid MSVC C4702 under /WX #1703 - ExternalTexture C4702 build fix
Document accurate root cause for post-#1695 pixel-diff fallouts #1704 - config.json reason rewrites (5 entries)
Document accurate root cause for 17 subtle pixel-diff tests #1705 - config.json reason rewrites (17 entries)

Tier 2 - sequential, each touches Apps/Playground/CMakeLists.txt SCRIPTS list + Apps/Playground/Shared/AppContext.cpp LoadScript order; rebase the next branch after the previous merges:

Bump JsRuntimeHost for File/FileReader polyfill (re-enables 19 GLTF/OBJ tests) #1706 - File/Blob/FileReader polyfill (largest test impact: 19 re-enables)
Add fetch() polyfill over XMLHttpRequest for Playground #1707 - fetch polyfill
Add DOM globals polyfill + native AbortController for Playground #1708 - DOM globals + native AbortController + Android CMake link
Add cubemap auto-expand polyfill for Playground (re-enables 7 PBR tests) #1710 - Cubemap auto-expand polyfill (loaded after babylon.max.js)

Reference policy reminder

Reference PNGs across all 7 PRs come from Babylon.js; never re-baked by BN. Combined diff: 0 PNGs.

Babylon Native's NativeEngine.createCubeTexture override only handles .env single-file cubemaps and 6-face arrays; .dds / .ktx / .ktx2 URLs fall through to a throw ("Cannot load cubemap because 6 files were not defined"). The loader-aware path used by the WebGL engine (texture loader registry lookup) is bypassed entirely. Add a JS-side polyfill that detects .dds / .ktx / .ktx2 single-URL cubemap loads and retries with the .env extension. Babylon's CI generates both .dds and .env from the same source HDR and uploads them to the same CDN path, so the swap is transparent for the Babylon-hosted environments these tests reference. On 404 (no .env counterpart exists) the polyfill re-invokes the original code path, preserving the existing throw semantics. This unblocks 7 tests that were excluded because the throw aborted the scene before any rendering could happen: idx 141 NMEGLTF idx 172 Anisotropic idx 173 Clear Coat idx 246 PBRMetallicRoughnessMaterial idx 247 PBRSpecularGlossinessMaterial idx 248 PBR idx 290 Prepass SSAO + depth of field Strip excludeFromAutomaticTesting + reason from those 7 entries in config.json. All 7 validate sub-threshold on Win32 V8 D3D11 Release without --include-excluded after the strip (pixel diff range 308..2638, well under the 2.5% threshold).

Per-test PIL-composite triage of all 17 ``subtle pixel-diff`` tests. None of them are deterministic-cosmetic (no re-bakes possible - all show real visible regressions). Updates the `reason` field in `Apps/Playground/Scripts/config.json` for 17 tests with accurate symptom descriptions, classifying into recurring root-cause clusters: - 9 GUI green->red color regressions (idx 160, 174, 175, 196, 197, 370, 402, 566, 587) - 4 OpenPBR analytic-lights right-column red rendering (580, 584, 587, 592) - 1 instanced billboard foliage red (169) - 1 LineEdgesRenderer extra red lines (179) - 1 Background blur red splotches (602) - 1 Clip planes GUI sliders red (182) - 1 Instanced Bones edge-AA (256, borderline) **No source changes, no test re-enables, no PNGs.** Metadata-only correction so the issue tracker reflects actionable root causes. --- ## Landing context This PR is one of **7 splits** from the proven CI-green combined preview in **draft PR #1702** (see [#1702](#1702) for the full intended end-state and verified CI run [26044922430](https://github.com/BabylonJS/BabylonNative/actions/runs/26044922430)). > Note: the original split included an 8th PR (#1709, ES2020+ -> ES2019 syntax-repair polyfill for Chakra). It was closed in favour of investigating `@babel/standalone` properly (#1711). ### Recommended landing order **Tier 1 - parallel-reviewable, no source conflicts:** 1. #1703 - ExternalTexture C4702 build fix 2. #1704 - config.json `reason` rewrites (5 entries) 3. #1705 - config.json `reason` rewrites (17 entries) **Tier 2 - sequential, each touches `Apps/Playground/CMakeLists.txt` SCRIPTS list + `Apps/Playground/Shared/AppContext.cpp` LoadScript order; rebase the next branch after the previous merges:** 4. #1706 - File/Blob/FileReader polyfill (largest test impact: 19 re-enables) 5. #1707 - fetch polyfill 6. #1708 - DOM globals + native AbortController + Android CMake link 7. #1710 - Cubemap auto-expand polyfill (loaded after babylon.max.js) ### Reference policy reminder Reference PNGs across all 7 PRs come from Babylon.js; never re-baked by BN. Combined diff: **0 PNGs**.

Per-test triage of 5 post-#1695 pixel-diff fallouts. Updates the `reason` field in `Apps/Playground/Scripts/config.json` for 3 entries to name the real BN rendering regression instead of generic "Pixel comparison fails": - idx 363 SSR2 - SSR not rendering wet floor. - idx 369 Sprites Pixel Perfect - sprite alpha-blending broken. - idx 395 soft-transparent-shadows - soft-shadow filter precision degraded. **No source changes, no test re-enables, no PNGs.** Metadata-only correction so the issue tracker reflects actionable root causes for follow-up engineering work. --- ## Landing context This PR is one of **7 splits** from the proven CI-green combined preview in **draft PR #1702** (see [#1702](#1702) for the full intended end-state and verified CI run [26044922430](https://github.com/BabylonJS/BabylonNative/actions/runs/26044922430)). > Note: the original split included an 8th PR (#1709, ES2020+ -> ES2019 syntax-repair polyfill for Chakra). It was closed in favour of investigating `@babel/standalone` properly (#1711). ### Recommended landing order **Tier 1 - parallel-reviewable, no source conflicts:** 1. #1703 - ExternalTexture C4702 build fix 2. #1704 - config.json `reason` rewrites (5 entries) 3. #1705 - config.json `reason` rewrites (17 entries) **Tier 2 - sequential, each touches `Apps/Playground/CMakeLists.txt` SCRIPTS list + `Apps/Playground/Shared/AppContext.cpp` LoadScript order; rebase the next branch after the previous merges:** 4. #1706 - File/Blob/FileReader polyfill (largest test impact: 19 re-enables) 5. #1707 - fetch polyfill 6. #1708 - DOM globals + native AbortController + Android CMake link 7. #1710 - Cubemap auto-expand polyfill (loaded after babylon.max.js) ### Reference policy reminder Reference PNGs across all 7 PRs come from Babylon.js; never re-baked by BN. Combined diff: **0 PNGs**.

…exture polyfill Per review: this polyfill should be available to every Babylon Native consumer using Chakra, not just the Playground app. Move it from a Playground-only LoadScript() of cube_texture_polyfill.js into a proper C++ polyfill library under Polyfills/ that any embedder can link. The polyfill patches BABYLON.NativeEngine.prototype.createCubeTexture to transparently retry single-URL .dds / .ktx / .ktx2 cubemap loads (which BN does not currently handle) as their .env counterpart that Babylon's CDN co-hosts at the same path. On 404 it falls through to the original (which throws), preserving existing error semantics. Implementation pattern follows the existing ErrorCause / StringTrim native polyfills: - New Polyfills/CubeTexture/ library. - Public header Babylon/Polyfills/CubeTexture.h declares void BABYLON_API Initialize(Napi::Env env). - Source/CubeTexture.cpp embeds the polyfill IIFE verbatim from the prior cube_texture_polyfill.js (same semantics, same idempotency marker __cubeTexturePolyfillInstalled) and dispatches it via the free function Napi::Eval(env, source, url). Napi::Eval is declared by every engine-specific <napi/env.h> across both N-API trees (Chakra / V8 / JavaScriptCore in Core/Node-API/Include/Engine/<X>/ and JSI in Core/Node-API-JSI/Include/napi/) so the same source builds uniformly across all backends; no __has_include switch needed for this entry point. Wiring: - Top-level CMakeLists.txt: add BABYLON_NATIVE_POLYFILL_CUBETEXTURE option (default ON). - Polyfills/CMakeLists.txt: guarded add_subdirectory(CubeTexture). - Apps/Playground/CMakeLists.txt and Apps/Playground/Android/BabylonNative/CMakeLists.txt: PRIVATE link CubeTexture into the Playground / BabylonNativeJNI executables so AppContext.cpp can use the published header on every platform. - Apps/Playground/Shared/AppContext.cpp: include the new header and replace LoadScript("app:///Scripts/cube_texture_polyfill.js"); with Dispatch([](Napi::Env env) { Babylon::Polyfills::CubeTexture::Initialize(env); }); scheduled right after LoadScript("babylon.max.js"). ScriptLoader's Dispatch and LoadScript queue onto the same ordered task chain (see jsruntimehost ScriptLoader::Impl), so Initialize is guaranteed to run after babylon.max.js finishes evaluating - same timing the prior LoadScript provided. Delete Apps/Playground/Scripts/cube_texture_polyfill.js (no longer shipped). Verified locally on Chakra build (build/win32 RelWithDebInfo, headless) - all 7 cubemap-affected tests re-enabled by this PR pass: Clear Coat diff=519 px PBR diff=482 px (12 sub-tests skipped, expected) Anisotropic diff=308 px PBRMetallicRoughnessMaterial diff=482 px NMEGLTF diff=631 px PBRSpecularGlossinessMaterial diff=522 px Console log confirms the polyfill ran: [CubeTexture polyfill] NativeEngine.createCubeTexture patched with .env fallback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…1703) Fixes MSVC C4702 (unreachable code) under `/WX` in `Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp` so the OpenGL TU builds cleanly without the project-wide `/wd4702` workaround. ## What changed `Plugins/ExternalTexture/Source/ExternalTexture_OpenGL.cpp` keeps the unimplemented `Impl` stubs as `throw std::runtime_error{"not implemented"}` (so callers get an unambiguous diagnostic matching every other backend's unimplemented path) and wraps the `#include "ExternalTexture_Shared.h"` line in `#pragma warning(push)/(disable: 4702)/(pop)`. The shared dispatchers instantiated by that include unconditionally call the stubs, so MSVC's flow analysis flags the dispatcher's post-call code as unreachable; the pragma suppresses C4702 only for the dispatcher code instantiated in this translation unit. Any other code in this file is still subject to `/WX` C4702 enforcement. `Plugins/ExternalTexture/CMakeLists.txt` - drop the `/wd4702` target compile option, which previously silenced C4702 across the whole target. ## Alternatives considered - **`[[noreturn]]` on the stubs**: MSVC propagates "never returns" through the shared dispatcher, which flags *more* post-call statements as unreachable. Tried and rejected (it made the warning worse). - **TU-wide `/wd4702` via `target_compile_options`**: silences any future legitimate C4702 elsewhere in the same TU, defeating `/WX`. The current localized `#pragma warning(push/pop)` block keeps the rest of the TU under `/WX` enforcement. - **Replacing the throws with inert return-default stubs**: changes product behaviour (callers would silently receive a null texture instead of a clear "not implemented" error) to work around a compiler warning. Rejected on principle. ## Verified Clean under Debug + Release + RelWithDebInfo on OpenGL (`win32-gl`, `GRAPHICS_API=OpenGLWindowsDevOnly`) and D3D11 (`win32`). --- ## Landing context This PR is one of **7 splits** from the proven CI-green combined preview in **draft PR #1702** (see [#1702](#1702) for the full intended end-state and verified CI run [26044922430](https://github.com/BabylonJS/BabylonNative/actions/runs/26044922430)). > Note: the original split included an 8th PR (#1709, ES2020+ -> ES2019 syntax-repair polyfill for Chakra). It was closed in favour of investigating `@babel/standalone` properly (#1711). ### Recommended landing order **Tier 1 - parallel-reviewable, no source conflicts:** 1. #1703 - ExternalTexture C4702 build fix 2. #1704 - config.json `reason` rewrites (5 entries) 3. #1705 - config.json `reason` rewrites (17 entries) **Tier 2 - sequential, each touches `Apps/Playground/CMakeLists.txt` SCRIPTS list + `Apps/Playground/Shared/AppContext.cpp` LoadScript order; rebase the next branch after the previous merges:** 4. #1706 - File/Blob/FileReader polyfill (largest test impact: 19 re-enables) 5. #1707 - fetch polyfill 6. #1708 - DOM globals + native AbortController + Android CMake link 7. #1710 - Cubemap auto-expand polyfill (loaded after babylon.max.js) ### Reference policy reminder Reference PNGs across all 7 PRs come from Babylon.js; never re-baked by BN. Combined diff: **0 PNGs**. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bghgary

[Reviewed by Copilot on behalf of @bghgary]

Concerns inline. Short version: polyfill silently substitutes content and masks the real gap (no .dds/.ktx loader in BN). Let's discuss offline.

bghgary · 2026-05-29T22:32:35Z

@@ -0,0 +1,134 @@
+#include <Babylon/Polyfills/CubeTexture.h>


Three layered concerns:

Silent substitution. PG asks for .dds, we quietly load the .env co-located by Babylon's CDN. Tests "pass" but they're rendering a different file (different format, mip pyramid, precision). If BJS's .dds path regresses, we don't catch it.

CDN-specific. The fallback only works for assets where someone happens to co-host .env. Babylon's playground CDN does; arbitrary BN consumers don't. Shipping this in Polyfills/ advertises a benefit non-Babylon consumers won't get.

Wrong layer. This masks a real gap — BN's NativeEngine doesn't dispatch .dds/.ktx/.ktx2 cubemap loading even though bimg already supports the formats. The proper fix lives in Plugins/NativeEngine.

Let's discuss offline before merging.

Copilot AI review requested due to automatic review settings May 18, 2026 16:35

bkaradzic-microsoft requested review from CedricGuillemet, bghgary and ryantrem May 18, 2026 16:42

Copilot started reviewing on behalf of bkaradzic-microsoft May 18, 2026 17:56 View session

bkaradzic-microsoft enabled auto-merge (squash) May 29, 2026 00:35

bghgary requested changes May 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cubemap auto-expand polyfill for Playground (re-enables 7 PBR tests)#1710

Add cubemap auto-expand polyfill for Playground (re-enables 7 PBR tests)#1710
bkaradzic-microsoft wants to merge 2 commits into
BabylonJS:masterfrom
bkaradzic-microsoft:weekend/tpc-1577-cubemap-6-files

bkaradzic-microsoft commented May 18, 2026 •

edited

Loading

Uh oh!

bghgary left a comment

Uh oh!

bghgary May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bkaradzic-microsoft commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Landing context

Recommended landing order

Reference policy reminder

Uh oh!

bghgary left a comment

Choose a reason for hiding this comment

Uh oh!

bghgary May 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bkaradzic-microsoft commented May 18, 2026 •

edited

Loading