[Native] MultiRenderTarget, reverse-Z clear, applyStates, OIT alpha modes#18568
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/18568/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/18568/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/18568/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. |
🟢 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 |
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
5e7e85e to
c5da3b6
Compare
|
@bkaradzic, any update on this one ? |
c5da3b6 to
f4fdea7
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: |
|
Visualization tests for WebGPU |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
bghgary
left a comment
bghgary
left a comment
There was a problem hiding this comment.
[Reviewed by Copilot on behalf of @bghgary]
LGTM. The description is now stale, though: with the feature-detection this no longer requires #1754 (it warns and falls back on older native), so the "Paired native PR: Requires…" and "co-dependent / landing order" sections no longer apply — the two can land separately, and this can come out of draft.
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
🟢 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: |
🟢 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 |
…odes Foundational native-engine surface so MultiRenderTarget, the reverse depth buffer, and the order-independent-transparency renderer stop dereferencing the null _gl context. Removes several crash classes; does not by itself land the dependent validation tests. - createMultipleRenderTarget + the MRT helper overrides (bindAttachments, buildTextureLayout, restoreSingleAttachment[ForRenderTarget], generateMipMapsMultiFramebuffer, resolveMultiFramebuffer, unBindMultiColorAttachmentFramebuffer, updateMultipleRenderTargetTextureSampleCount). bgfx writes every color attachment of the bound framebuffer, so the WebGL drawBuffers / MSAA-resolve plumbing becomes no-ops on Native. Reports drawBuffersExtension = true. - clear(): implement the reverse depth buffer (clear depth to 0 + GEQUAL) instead of throwing. - applyStates(): override so it flushes the depth-culling state through the native command path (the base implementation drives the null _gl directly), fixing callers that mutate engine.depthCullingState then call applyStates() (e.g. the OIT depth-peeling renderer). - Map alpha modes ALPHA_ONEONE_ONEONE and ALPHA_LAYER_ACCUMULATE to the native engine. Pairs with the BabylonNative change (createMultiFrameBuffer + native alpha modes). Known follow-ups: OIT depth-peeling still faults in the D3D11 driver on submit; the blend equation (MAX) is not yet applied natively. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ative Address review feedback: don't hard-break against Babylon Native binaries that predate multi render target support (BabylonNative#1754). - thinNativeEngine: guard createMultiFrameBuffer; when the native binary does not expose it, warn and fall back to a single-attachment framebuffer bound to the first color target so the scene keeps rendering. - nativeHelpers.getNativeAlphaMode: ALPHA_ONEONE_ONEONE / ALPHA_LAYER_ACCUMULATE now warn once and fall back to ALPHA_ONEONE when the native constant is undefined, instead of silently returning undefined. - nativeInterfaces: mark createMultiFrameBuffer and the two OIT alpha constants optional to reflect that older native does not provide them. Keeps the PR mergeable against the currently published native binary and unblocks the Native tests CI without requiring a coordinated protocol bump. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
clear() set depthCullingState.depthFunc = GEQUAL to mirror the WebGL engine, but the native draw path never calls applyStates() before a draw (only depth-test enable/disable is reconciled in _flushDepthTestState), so the GEQUAL comparison never reached the backend and reverse-Z had no effect. Also call setDepthFunction(GEQUAL) to encode the compare via the native command path, mirroring the WebGPU engine's clear path (setDepthFunctionToGreaterOrEqual). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bgfx binds a fixed attachment set at framebuffer creation and cannot re-point an individual attachment the way GL's framebufferTexture2D can. When a multi render target attachment is replaced after creation (e.g. the OIT depth-peeling renderer swaps every attachment via MultiRenderTarget.setInternalTexture), the native framebuffer stayed bound to the original creation-time textures. Extract the framebuffer build from createMultipleRenderTarget into _createMultiRenderTargetFramebuffer and call it from a NativeRenderTargetWrapper.setTexture override so the framebuffer is rebuilt from the current attachment set on every swap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…esent bgfx framebuffers use a dense, ordered attachment list. Building _createMultiRenderTargetFramebuffer from a partial attachment set (attachments deferred via dontCreateTextures / targetTypes[i] === -1, or filled out of order) would compress the attachment indices and produce a framebuffer that does not match the MRT layout. Guard the build so it only runs when the collected color handles match the expected attachment count (rtWrapper._attachments.length), which also guarantees contiguous, in-order handles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
updateMultipleRenderTargetTextureSampleCount was a no-op that only recorded _samples, so RenderTargetWrapper.setSamples() silently did nothing for MRTs on Native (bgfx couples MSAA to texture creation flags). Mirror updateRenderTargetTextureSampleCount: reissue each non-external color attachment's bgfx handle with the new sample count via initializeTexture, update texture.samples, then recreate the framebuffer so it points at the rotated handles. Also pass limit=1 to the createMultiFrameBuffer-fallback Logger.Warn so it warns once instead of spamming when the framebuffer is rebuilt during attachment setup/swaps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
33cdf3b to
ddc7578
Compare
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
⚡ Performance Test ResultsBaseline: Latest · Candidate: Dev
🔘 Not Significant — p ≥ 0.05 (1)
|
Paired native PR: BabylonJS/BabylonNative#1754
What
Foundational native-engine surface so
MultiRenderTarget, the reverse depth buffer, and theorder-independent-transparency (depth-peeling) renderer stop dereferencing the null
_glcontext.This removes several distinct crash classes. It does not by itself turn the MRT/OIT/FrameGraph
validation tests green — those need further work (see Follow-ups).
Changes (
packages/dev/core/src/Engines)thinNativeEngine.pure.tscreateMultipleRenderTarget+ the MRT helper overrides:bindAttachments,buildTextureLayout,restoreSingleAttachment/restoreSingleAttachmentForRenderTarget,generateMipMapsMultiFramebuffer,resolveMultiFramebuffer,unBindMultiColorAttachmentFramebuffer,updateMultipleRenderTargetTextureSampleCount. bgfx writes every color attachment of the boundframebuffer, so the WebGL
drawBuffers/ MSAA-resolve plumbing becomes no-ops on Native. ReportsdrawBuffersExtension = true.clear(): implement the reverse depth buffer (clear depth to 0 +GEQUAL) instead of throwing.applyStates(): override so it flushes the depth-culling state through the native command path —the base implementation drives the null
_gldirectly, which crashed callers that mutateengine.depthCullingStatethen callapplyStates()(e.g. the OIT depth-peeling renderer).Native/nativeHelpers.ts/nativeInterfaces.ts: map alpha modesALPHA_ONEONE_ONEONEandALPHA_LAYER_ACCUMULATEto the native engine, and declarecreateMultiFrameBuffer.Backward compatibility (feature detection)
createMultiFrameBufferand the two OIT alpha constants only exist on a Babylon Native binary thatcarries the paired #1754. They are declared optional on the native interface and feature-detected
at runtime, so this PR runs against the currently-published native without a protocol/version bump:
createMultiFrameBufferabsent →Logger.Warn+ fall back to a single-attachmentcreateFrameBufferbound to the first color target.
ALPHA_ONEONE_ONEONE/ALPHA_LAYER_ACCUMULATEabsent → warn once + fall back toALPHA_ONEONE.This is also what fixes the previously-red "Native tests (experimental)" CI job, which pulls the
BabylonNative master Playground (no #1754): before the guards it hit
createMultiFrameBuffer is not a function/ anundefinedalpha mode, the harness exited on the first failure, and the step's retriesblew the job timeout. With the guards the MRT/OIT scenes degrade gracefully instead of crashing.
Paired native PR
Pairs with the BabylonNative C++ change (the shared
CreateFrameBufferImplframebuffer helper + the twoalpha blend modes). Thanks to the feature detection above, this JS PR can land before #1754.
Follow-ups (not in this PR)
thinDepthPeelingRenderer) still faults inside the D3D11 driver onsubmit(a multi-output / SRV↔RTV ping-pong state the driver rejects); needs interactive GPU debugging.
MAX) is not yet applied on Native (setAlphaEquationis a no-op),so OIT blending would still be incorrect once the crash is resolved.
Testing
nx run babylonjs:build(fulltsc -b+ rollup + declaration) and ESLint pass.babylon.max.jsand ran the BabylonNative Playground suite(D3D11). MRT, reverse-Z, and the depth-peeling setup no longer crash with
COLOR_ATTACHMENT0/reverse depth buffer/depthMask of undefined/Unsupported alpha mode. No regressions inexisting 2D render-target tests.
they warn and degrade to a single-attachment framebuffer +
ALPHA_ONEONE.Related PRs & landing order
These two are co-dependent. With the feature-detection guards, #18568 no longer hard-requires the
native side and can merge independently:
no validation tests on its own, and degrades gracefully on native that predates check animations all stop bug. #1754.
babylonjsnpm release ships that TS change.babylonjsto that release. (Thisfoundational pair does not yet turn the MRT/OIT/FrameGraph tests green — that is separate follow-up
work.)