[Native] Combined native-engine GL fixes (depth, cube load, cube RTT, updateTextureData)#18567
[Native] Combined native-engine GL fixes (depth, cube load, cube RTT, updateTextureData)#18567bkaradzic-microsoft wants to merge 3 commits into
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18567/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18567/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/18567/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
|
WebGL2 visualization test reporter: |
🟢 Memory Leak Test Results13 passed, 0 leaked out of 13 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (13)
|
|
Visualization tests for WebGPU |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
73c6f7b to
bbac444
Compare
|
@bkaradzic, any update on this one ? |
The native createCubeTexture override only handled a single .env file or six face files; a single self-contained cubemap container (.dds/.ktx/.ktx2, as produced by CubeTexture.CreateFromPrefilteredData) threw "Cannot load cubemap because 6 files were not defined". The generic WebGL loader route is unusable on native (its texture-upload and cube-readback entry points are unimplemented). Route a single-URL container cubemap to engine.loadCubeTexture with the raw buffer; the native engine decodes it (bimg) and, when polynomials are requested, returns spherical-harmonics coefficients computed from the top mip, which are set as the texture's spherical polynomial. loadCubeTexture's onSuccess now optionally carries those coefficients. The .env and six-file paths are unchanged. Pairs with a Babylon Native NativeEngine change implementing the single-buffer cube decode and spherical-harmonics computation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d onSuccess The six-file path passed the Promise resolve directly, which is not assignable to the widened onSuccess signature (sphericalPolynomial?: Float32Array). Wrap with () => resolve() / () => reject(new Error(...)). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bbac444 to
c746fdb
Compare
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
|
Visualization tests for WebGPU |
|
[Commented by Copilot on behalf of @bghgary] This is the JS side of the native-SH approach in BabylonJS/BabylonNative#1748 — it wires the spherical harmonics that the native engine computes in C++ into That approach is being replaced. Per the discussion on #1748, the plan is to implement the Suggest closing this, since it'll be made redundant by the #218 work — easy to reopen if that plan changes. |
bghgary
left a comment
bghgary
left a comment
There was a problem hiding this comment.
[Reviewed by Copilot on behalf of @bghgary]
Requesting changes to hold this from merging: it depends on the native-SH approach in BabylonJS/BabylonNative#1748, which is being replaced by the #218 route (native reuses the shared JS _DDSTextureLoader, SH computed in JS) — making these changes redundant. See the comment above. Shouldn't merge until the #218 direction is settled.
## What Re-enables the `blur-cube-with-the-effect-renderer` Playground validation test (`#4C900K#2`) by dropping its `excludeFromAutomaticTesting` entry in `config.json`, and bumps the bundled `babylonjs` to **9.15.0** (which contains the paired engine fix) so the test actually passes in CI. Two commits: 1. Re-enable `blur-cube-with-the-effect-renderer` in `Apps/Playground/Scripts/config.json`. 2. Bump `babylonjs` to 9.15.0 in `Apps/package.json` + `Apps/package-lock.json`. Broken out of #1748 so it can merge independently: #1748 re-enables this same test alongside six single-file-`.dds` cube tests, but those six are still gated on an unmerged cube-loading TS change (BabylonJS/Babylon.js#18567), whereas this test only needs the already-merged depth-test fix. (#1748 currently still carries a duplicate `blur-cube` re-enable; that entry should be dropped there once this lands.) ## Why it passes now The test previously failed pixel comparison because the native engine ignored `depthCullingState.depthTest`, so an `EffectRenderer` fullscreen pass produced an all-black frame. That is fixed by BabylonJS/Babylon.js#18558 ("[Native] Honor depthCullingState.depthTest on the native engine"), **merged 2026-06-16 and shipped in `babylonjs` 9.15.0** — the version this PR now pins. With that fix present in the bundled engine the pass renders correctly and the test matches its reference image. ## Status - **Pin bump included; the earlier gate is resolved.** The bump to 9.15.0 (which contains #18558) is part of this PR, so the re-enable no longer depends on a separate future `babylonjs` bump. - **CI is fully green**, including every Win32 D3D11 and Ubuntu desktop job that runs the validation suite — `blur-cube-with-the-effect-renderer` validates. Ready to merge (pending review). --------- Co-authored-by: Branimir Karadzic <branimirkaradzic@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Branimir Karadzic <bkaradzic@users.noreply.github.com>
|
Closing per @bghgary's suggestion. This is the JS half of the native-computed-SH cubemap approach (paired with BabylonJS/BabylonNative#1748), and that approach is being replaced. Per the discussion on #1748, the plan (BabylonJS/BabylonNative#218) is to implement the native engine's Closing as it'll be made redundant by the #218 work; easy to reopen if that plan changes. |
5 participants
Paired native PR: BabylonJS/BabylonNative#1748 (rolled up in the combined BN integration branch BabylonJS/BabylonNative#1752)
What
Loads self-contained single-file
.dds/.ktx/.ktx2cubemaps on the native engine — theformat produced by
CubeTexture.CreateFromPrefilteredData(...), i.e. all six faces plus theprefiltered mip chain in one container — and wires the spherical-harmonics coefficients returned by
the native loader into
_sphericalPolynomialfor diffuse IBL.Changes (2 files, +103 / −26 vs
master)packages/dev/core/src/Engines/Native/Extensions/nativeEngine.cubeTexture.pure.ts— dispatchessingle-URL
.dds/.ktx/.ktx2cube containers to the nativecreateCubeTexturepath and sets_sphericalPolynomialfrom the coefficients the native loader returns.packages/dev/core/src/Engines/Native/nativeInterfaces.ts—loadCubeTexture'sonSuccesscallback gains an optional
sphericalPolynomial?: Float32Arrayso the native loader can hand backthe parsed SH.
No WebGL behavior change.
Validation tests re-enabled
The paired native PR BabylonJS/BabylonNative#1748 re-enables six prefiltered-
.ddsPBRenvironment Playground validation tests (Win32 / D3D11). They pass pixel comparison once this TS
change is present in the bundled
babylonjs:Landing order
This PR →
babylonjsnpm release → BabylonNative#1748. The native validation tests only gogreen once this TS change ships in the bundled engine, so the JS side lands first. Verified locally
against a from-source
babylon.max.jsbuilt with this change (the six tests pass; SH integration iscorrect — total solid angle = 4π).