Removed all D3D12 exclusions from tests.#1665
Conversation
There was a problem hiding this comment.
Pull request overview
Re-enables the previously D3D12-excluded scissor visual tests by updating bgfx integration and adjusting framebuffer attachment resolve behavior to avoid newer bgfx validation asserts around auto-mip generation.
Changes:
- Update
bgfx.cmakeFetchContent tag to a newer commit. - Adjust framebuffer attachment initialization to conditionally enable
BGFX_RESOLVE_AUTO_GEN_MIPSonly when the texture format supports mip autogen, and explicitly disable resolve for depth attachments. - Remove the D3D12 exclusions (and related comments) for scissor tests in the playground validation config.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Plugins/NativeEngine/Source/NativeEngine.cpp | Guards auto-mip resolve flag based on caps to prevent bgfx validation asserts; explicitly disables resolve for depth attachment. |
| CMakeLists.txt | Bumps bgfx.cmake dependency commit used by the build. |
| Apps/Playground/Scripts/config.json | Re-enables scissor tests on D3D12 by removing exclusions. |
bghgary
left a comment
There was a problem hiding this comment.
[Review by Copilot on behalf of @bghgary]
Nice fix, and the comment on the color attachment in CreateFrameBuffer captures the subtlety well. One concern: there are other Attachment::init(...) call sites using the short overload that rely on the same default-ignores-unsupported-formats behavior and will hit the new bgfx assert after this bump. They aren't covered by the scissor tests, so CI going green won't prove they're safe:
Polyfills/Canvas/Source/FrameBufferPool.cpp:65— loops over a color (RGBA8) and a depth (D24S8) attachment, both with the default overload. The depth one is the concern: depth formats don't support auto-mipgen, so this should reliably trip the new assert once Canvas is exercised.Plugins/NativeXr/Source/NativeXrImpl.cpp:285-286— same pattern, color + depth, both relying on defaults.
Could we fix these in the same PR? The change is mechanical (query caps->formats[...] & BGFX_CAPS_FORMAT_TEXTURE_MIP_AUTOGEN for color, explicit BGFX_RESOLVE_NONE for depth), and leaving them for later means a known assert waiting to fire on the first Canvas/WebXR workload after the merge.
Re-enables the scissor test cases that were previously excluded on D3D12 due to an automatic mipmap generation issue in bgfx.