NativeEngine: MultiRenderTarget framebuffers + OIT alpha blend modes#1754
NativeEngine: MultiRenderTarget framebuffers + OIT alpha blend modes#1754bkaradzic-microsoft wants to merge 3 commits into
Conversation
c773272 to
db75d2e
Compare
db75d2e to
1b4cbd0
Compare
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: since #18568 feature-detects the native surface, the "co-dependent / landing order" framing no longer applies — the two can land separately, and this can come out of draft.
There was a problem hiding this comment.
Pull request overview
This PR adds foundational NativeEngine support for MultiRenderTarget framebuffers by creating a single bgfx framebuffer with N color attachments (plus optional depth/stencil), and exposes two additional alpha blend modes needed by OIT depth-peeling workflows. It also updates the bundled Babylon.js npm dependencies used by the Apps workspace.
Changes:
- Add
createMultiFrameBufferand refactor framebuffer creation into a sharedCreateFrameBufferImplthat supports multiple color attachments. - Expose additional alpha blend mode constants (
ALPHA_ONEONE_ONEONE,ALPHA_LAYER_ACCUMULATE) for native-side blend state mapping. - Bump Babylon.js-related npm dependencies in
Apps/to^9.15.0(and update lockfile).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Plugins/NativeEngine/Source/NativeEngine.h | Declares new MRT framebuffer entrypoint and shared framebuffer helper. |
| Plugins/NativeEngine/Source/NativeEngine.cpp | Implements MRT framebuffer creation via a shared helper; adds OIT blend mode constants and JS binding for createMultiFrameBuffer. |
| Apps/package.json | Updates Babylon.js package dependency versions to ^9.15.0. |
| Apps/package-lock.json | Locks Babylon.js packages to 9.15.0 and updates related integrity metadata. |
Files not reviewed (1)
- Apps/package-lock.json: Generated file
| if (colorCount + 1 > caps->limits.maxFBAttachments) | ||
| { | ||
| throw Napi::Error::New(env, "Invalid number of color attachments for frame buffer"); | ||
| } |
There was a problem hiding this comment.
Fixed in 05862e2. The guard now reserves the depth/stencil slot only when one is generated: const uint32_t depthStencilCount = (generateStencilBuffer || generateDepth) ? 1u : 0u; then checks colorCount + depthStencilCount > caps->limits.maxFBAttachments. This matches the attachment-creation path below (which only appends the depth/stencil attachment inside the same generateStencilBuffer || generateDepth branch), so a max-color-count MRT with no depth/stencil is no longer wrongly rejected.
Foundational native-engine support for MultiRenderTarget (MRT) and the order- independent-transparency blend modes, removing several "engine._gl is null" crash classes. It does not by itself land the MRT/OIT/FrameGraph validation tests, which need further work (see follow-ups below). - Add NativeEngine::CreateMultiFrameBuffer: build one bgfx framebuffer with N color attachments (+ optional depth) so a MultiRenderTarget renders to all targets at once (bgfx writes every attachment of the bound framebuffer, so no drawBuffers is needed). JS-controlled attachment count is validated against caps->limits.maxFBAttachments. - Add alpha blend modes ALPHA_ONEONE_ONEONE (11) and ALPHA_LAYER_ACCUMULATE (17) used by the depth-peeling OIT renderer. Pairs with the Babylon.js change (createMultipleRenderTarget + MRT helper overrides, applyStates, reverse-Z clear). Known follow-ups: the OIT depth- peeling path still faults inside the D3D11 driver on submit (needs interactive GPU debugging), and the blend equation (MAX) is not yet applied natively. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ed helper Address review feedback (discussion_r3546780088): the single- and multi-RT framebuffer paths were ~95% identical. Extract the shared color/depth attachment setup, framebuffer creation and cleanup into a private CreateFrameBufferImpl helper. Both N-API methods are kept so the protocol is unchanged; they now only differ in how they parse info[0] (single nullable texture vs. array) before calling the shared helper. This restores the "Stencil without depth..." warning and the isTextureValid assert that the multi path had dropped, and lets the single-RT path be the 0-or-1-color case (the shared helper no longer rejects colorCount == 0). The maxFBAttachments upper-bound guard is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
04f709e to
779fc34
Compare
The attachment-count guard in CreateFrameBufferImpl unconditionally added 1 for a depth/stencil attachment (colorCount + 1), which incorrectly rejected a valid MRT that uses the maximum number of color attachments with no depth/stencil (generateDepth=false, generateStencilBuffer=false). Reserve the depth/stencil slot only when one is actually generated, matching the attachment-creation logic below. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…odes (#18568) Paired native PR: BabylonJS/BabylonNative#1754 ## What Foundational native-engine surface so `MultiRenderTarget`, the reverse depth buffer, and the order-independent-transparency (depth-peeling) renderer stop dereferencing the null `_gl` context. 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.ts`** - `createMultipleRenderTarget` + the MRT helper overrides: `bindAttachments`, `buildTextureLayout`, `restoreSingleAttachment`/`restoreSingleAttachmentForRenderTarget`, `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, which crashed callers that mutate `engine.depthCullingState` then call `applyStates()` (e.g. the OIT depth-peeling renderer). - **`Native/nativeHelpers.ts` / `nativeInterfaces.ts`**: map alpha modes `ALPHA_ONEONE_ONEONE` and `ALPHA_LAYER_ACCUMULATE` to the native engine, and declare `createMultiFrameBuffer`. ## Backward compatibility (feature detection) `createMultiFrameBuffer` and the two OIT alpha constants only exist on a Babylon Native binary that carries 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: - `createMultiFrameBuffer` absent → `Logger.Warn` + fall back to a single-attachment `createFrameBuffer` bound to the first color target. - `ALPHA_ONEONE_ONEONE` / `ALPHA_LAYER_ACCUMULATE` absent → warn once + fall back to `ALPHA_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` / an `undefined` alpha mode, the harness exited on the first failure, and the step's retries blew 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 `CreateFrameBufferImpl` framebuffer helper + the two alpha blend modes). Thanks to the feature detection above, this JS PR can land **before** #1754. ## Follow-ups (not in this PR) - OIT depth-peeling (`thinDepthPeelingRenderer`) still faults inside the D3D11 driver on `submit` (a multi-output / SRV↔RTV ping-pong state the driver rejects); needs interactive GPU debugging. - The blend **equation** (e.g. `MAX`) is not yet applied on Native (`setAlphaEquation` is a no-op), so OIT blending would still be incorrect once the crash is resolved. ## Testing - `nx run babylonjs:build` (full `tsc -b` + rollup + declaration) and ESLint pass. - Against native **with** #1754: built `babylon.max.js` and 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 in existing 2D render-target tests. - Against native **without** #1754: the feature-detection guards keep those same scenes from throwing — they warn and degrade to a single-attachment framebuffer + `ALPHA_ONEONE`. --- ## Related PRs & landing order - **Babylon.js (engine / TS):** #18568 - **BabylonNative (C++ engine):** BabylonJS/BabylonNative#1754 These two are co-dependent. With the feature-detection guards, **#18568 no longer hard-requires the native side** and can merge independently: 1. **Babylon.js #18568 first** — adds native-engine TS overrides, changes no WebGL behavior, re-enables no validation tests on its own, and degrades gracefully on native that predates #1754. 2. A **`babylonjs` npm release** ships that TS change. 3. **BabylonNative #1754 last** — after bumping the bundled `babylonjs` to that release. (This foundational pair does not yet turn the MRT/OIT/FrameGraph tests green — that is separate follow-up work.) --------- Co-authored-by: Branimir Karadzic <branimirkaradzic@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Paired engine PR: BabylonJS/Babylon.js#18568
What
Foundational native support for
MultiRenderTarget(MRT) and the order-independent-transparency blendmodes, removing several "
engine._glis null" crash classes. It does not by itself turn theMRT/OIT/FrameGraph validation tests green — those need further work (see Follow-ups).
Changes
Plugins/NativeEngine/Source/NativeEngine.cpp/.hCreateFrameBufferandCreateMultiFrameBuffernow share a privateCreateFrameBufferImpl(env, colorTextures, …)helper that builds one bgfx framebuffer with N colorattachments (+ an optional depth/stencil attachment), so a
MultiRenderTargetrenders to alltargets at once (bgfx writes every attachment of the bound framebuffer — no
drawBuffersneeded).The two N-API methods are kept for protocol compatibility and only differ in how they parse
info[0](single nullable texture vs. array); the single-RT path is just the 0-or-1-color case.The shared helper validates the attachment count against
caps->limits.maxFBAttachmentsand keepsthe
"Stencil without depth…"warning and theisTextureValidassert.ALPHA_ONEONE_ONEONE(11) andALPHA_LAYER_ACCUMULATE(17) used by thedepth-peeling OIT renderer.
Paired engine PR
Pairs with the Babylon.js change (
createMultipleRenderTarget+ MRT helper overrides,applyStatesoverride, reverse-Z
clear, alpha-mode mapping). That PR feature-detects bothcreateMultiFrameBufferand the two alpha constants, so it degrades gracefully on native binaries that predate this PR.
CI
The validation suite uses the published
babylonjsnpm, which doesn't yet contain the paired JS, sono new tests are enabled here and CI stays in the usual "pending dep bump" state. Verified locally
against a
babylon.max.jsbuilt from the paired branch, and the plugin compiles clean(
cmake --build build/win32 --target NativeEngine --config RelWithDebInfo).Follow-ups (separate work)
submit(multi-output / SRV↔RTVping-pong); needs interactive GPU debugging (PIX / VS Graphics Debugger).
MAX) —setAlphaEquationis a no-op — so OITblending would still be incorrect after the crash is fixed.
types").
Related PRs & landing order
These two are co-dependent and land in this order:
re-enables no validation tests on its own, and feature-detects this PR's additions, so it can merge
independently.
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.)