[Native] Combined native-engine GL fixes (depth, cube load, cube RTT, updateTextureData)#18567
Draft
bkaradzic-microsoft wants to merge 6 commits into
Draft
[Native] Combined native-engine GL fixes (depth, cube load, cube RTT, updateTextureData)#18567bkaradzic-microsoft wants to merge 6 commits into
bkaradzic-microsoft wants to merge 6 commits into
Conversation
Collaborator
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Collaborator
|
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. |
Collaborator
Collaborator
|
WebGL2 visualization test reporter: |
Collaborator
🟢 Memory Leak Test Results13 passed, 0 leaked out of 13 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (13)
|
Collaborator
|
Visualization tests for WebGPU |
Collaborator
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
EffectRenderer (and ThinDepthPeelingRenderer) disable depth testing by setting engine.depthCullingState.depthTest directly. On WebGL/WebGPU this is flushed by applyStates() at draw time, but the native engine only honored depth state set through the explicit setDepthBuffer() command and never flushed depthCullingState before a draw, so the toggle was silently dropped. A fullscreen EffectWrapper pass then kept the previous draw's depth test and every fragment was discarded, producing an all-black target. Reconcile depthCullingState.depthTest before each native draw to match the cross-engine contract: setDepthBuffer() keeps the state object authoritative and _flushDepthTestState() emits the command when the direct toggle differs from what was last encoded. No public API change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
NativeEngine had no createRenderTargetCubeTexture / per-face bindFramebuffer override, so ReflectionProbe and point-light cube shadows fell through to the WebGL path and dereferenced the null _gl context (TEXTURE_CUBE_MAP undefined). - createRenderTargetCubeTexture: native cube color texture + one framebuffer per face (the native side binds the matching cube layer). - bindFramebuffer: bind the per-face framebuffer for cube render targets. - generateMipMapsForCubemap: no-op on Native (bgfx auto-generates the mip chain on render-target resolve, like 2D RTTs). - NativeRenderTargetWrapper: track per-face framebuffers, release on dispose. - nativeInterfaces: thread cube/layer params through initializeTexture and createFrameBuffer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The native engine inherited engine.name === "WebGL" from ThinEngine, so code
that guards WebGL-only _gl access with �ngine.name === "WebGL" ran on the
native engine and dereferenced the null _gl context (e.g. TEXTURE_2D undefined).
updateTextureData also threw "not implemented".
- Report a distinct engine name ("Native"), mirroring how the WebGPU engine
reports "WebGPU", so name-gated WebGL-only code paths skip the native engine.
- Implement updateTextureData: forward the sub-rectangle (plus invertY, so the
native side can match the base texture's vertical orientation) to the native
engine. generateMipMaps is ignored (mip regeneration after a partial update is
not supported on Native).
Pairs with the BabylonNative change adding NativeEngine::UpdateTextureData.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkaradzic-microsoft
force-pushed
the
native-combined-gl-fixes
branch
from
73c6f7b to
bbac444
Compare
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
3 participants
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.
Paired native PR: BabylonJS/BabylonNative#1752
What
Integration branch combining the four native-engine fixes below (each also exists as its own focused
PR — those remain the review units). Opened so combined CI / testing can validate the cumulative
effect and the full set of BabylonNative validation tests they re-enable.
depthCullingState.depthTeston the native engine.dds/.ktx/.ktx2cubemapsengine.name = "Native"+ implementupdateTextureDataValidation tests re-enabled (10)
Built
babylon.max.jsfrom this branch and ran the BabylonNative Playground validation suite(Win32 / D3D11). All 10 previously-excluded tests now pass pixel comparison:
Honor depthCullingState.depthTest (#18558)
Single-file
.dds/.ktx/.ktx2cubemap loading (#18560)Cube render targets (#18561)
updateTextureData+engine.name = "Native"(#18566)Note
Draft integration branch — the four individual PRs are the units to review/merge.
Related PRs & landing order
Integration branches — combined CI/validation only; the four constituent paired PRs are the review/merge units:
Each constituent pair lands in the order Babylon.js PR →
babylonjsnpm release → BabylonNative PR (the native validation tests only go green once the paired JS is in the bundled engine):The integration branches themselves merge (if at all) only after their constituent pairs.