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>
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.)