Support standalone sampleable native depth textures - #1886
Merged
bkaradzic-microsoft merged 2 commits intoSep 18, 2026
Merged
bkaradzic-microsoft merged 2 commits into
bkaradzic-microsoft merged 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds support for Babylon.js standalone depth-texture requests by creating readable native depth attachments and aliasing them into the requested texture while retaining framebuffer ownership.
Changes:
- Detects uninitialized textures passed to
createFrameBufferand rejects requests without depth or stencil. - Creates readable depth textures for standalone requests, preserves write-only behavior for generated attachments, and reports depth for stencil-only framebuffers.
- Adds native regression coverage for attachment aliasing, disposal, flags, invalid requests, and legacy depth framebuffers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
Plugins/NativeEngine/Source/NativeEngine.h |
Extends the framebuffer implementation signature to accept a standalone depth/stencil texture. |
Plugins/NativeEngine/Source/NativeEngine.cpp |
Implements standalone depth-texture detection, readable attachment creation, aliasing, and depth bookkeeping. |
Apps/UnitTests/Source/Tests.NativeEngine.DepthTextures.cpp |
Adds regression tests for standalone and legacy depth framebuffer behavior. |
Apps/UnitTests/CMakeLists.txt |
Includes the new depth-texture test source in the unit-test target. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bkaradzic-microsoft
requested review from
CedricGuillemet,
bghgary and
ryantrem
and
a balanced review from Copilot
September 18, 2026 00:54
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new multisample depth-texture flag path is not exercised by the regression test.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
CedricGuillemet
approved these changes
Sep 18, 2026
Recognize the existing uninitialized-texture request, omit the invalid color attachment, and expose a non-owning alias of the readable depth attachment. Keep ordinary generated depth storage write-only. Report depth for stencil-only requests backed by combined depth/stencil. Cover attachment metadata, alias disposal, clears, and legacy null-texture depth framebuffers without changing protocol or dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
Exercise one, two, and four samples for depth-only and depth/stencil attachments. Assert the MSAA sampling bit and exact exclusive RT-MSAA mask, while retaining the alias lifetime and attachment metadata checks. Check backend format capabilities before optional multisample allocations and report unsupported combinations explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae
bkaradzic-microsoft
force-pushed
the
pr/native-sampleable-depth
branch
from
September 18, 2026 14:30
a7bc093 to
52273ca
Compare
bkaradzic-microsoft
enabled auto-merge (squash)
September 18, 2026 15:47
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
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.
Summary
createFrameBufferreceives a freshly created, uninitialized native texture.HasDepth()for stencil-only requests, since the allocated D24S8 attachment really contains depth.Independence
Based on current
master(bb01e283). Published Babylon.js 9.21.2 already issues this request. No JavaScript engine changes, protocol changes, dependency updates, reference changes, or tolerance changes. This does not include newer FrameGraph shared-depth APIs.Validation
Windows x64 / D3D11 / Chakra / RelWithDebInfo, using stock dependencies and Babylon.js 9.21.2:
Multisample review follow-up
a7bc0938extends the native ownership regression to 1x/2x/4x depth-only and depth/stencil allocations, asserting the exact RT-MSAA mask and the MSAA sampling bit. Unsupported multisample format combinations are reported explicitly before allocation. All combinations ran without skips and passed on Windows D3D11/Chakra and Linux Vulkan/JavaScriptCore/lavapipe; both existing D3D11 clear controls also pass. No production changes, renderer exclusions, or dependency on the separate MSAA PR were added.