Add Device.UpdateDevice unit test for D3D11/D3D12#1687
Open
bghgary wants to merge 10 commits intoBabylonJS:masterfrom
Open
Add Device.UpdateDevice unit test for D3D11/D3D12#1687bghgary wants to merge 10 commits intoBabylonJS:masterfrom
bghgary wants to merge 10 commits intoBabylonJS:masterfrom
Conversation
Adds a TEST(Device, UpdateDevice) to Apps/UnitTests that demonstrates the
proper sequence for swapping the underlying graphics device at runtime
(canonical D3D11/D3D12 device-removed recovery; also voluntary GPU swap
on Metal multi-GPU machines and host-driven device swap in embedding).
The contract this test pins down: UpdateDevice alone only stores the new
device pointer in Bgfx.InitState.platformData -- while bgfx is already
initialized, EnableRendering (from StartRenderingCurrentFrame) early-outs
because Bgfx.Initialized == true, so the new pointer never reaches
bgfx::init. DisableRendering must be called first to shut bgfx down so
the next frame re-runs bgfx::init with the new device. The render-reset
callback (wired by NativeEngine to JS-side setDeviceLostCallback) fires
automatically on the second EnableRendering.
Per-platform device-creation helpers added to Utils.${API}.${EXT}:
CreateGraphicsDeviceForTest, DestroyGraphicsDeviceForTest, plus D3D11-only
CreateBackBufferForTest / DestroyBackBufferForTest.
Per-platform behaviour:
- D3D11: WARP via D3D11CreateDevice; UpdateBackBuffer accompanies the swap.
- D3D12: SKIPPED at runtime. bgfx D3D12 shutdown asserts a zero device
refcount which the caller-provided ID3D12Device pattern can't satisfy
(caller still holds a reference). Same class of issue the existing
App.Win32.cpp HWND workaround comment hints at.
- Metal: MTL::Device is per-GPU singleton; no API exists to create a
second device for the same GPU, so on single-GPU hardware deviceA ==
deviceB and the post-swap EXPECT_EQ is satisfied trivially. CI macOS
runners use the bgfx noop renderer (USE_NOOP_METAL_DEVICE) so the test
skips there.
- OpenGL: GLX context construction against the App layer's X Display.
Skips if helper returns null.
Local verification: D3D11 full UnitTests suite 8 PASSED including the new
Device.UpdateDevice. D3D12 Device.UpdateDevice SKIPPED with the
documented message; other tests unaffected.
[Created by Copilot on behalf of @bghgary]
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
gtest-port.h aggressively #undef's None (and similar X11 collision macros) to defend itself against X11 macro contamination. By the time our code in Utils.OpenGL.cpp references None / True (after #include <X11/Xlib.h>), gcc has already processed gtest's #undef and the macros are no longer visible. Use the literal 0 / 1 values that the X11 macros expand to. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Like D3D12, the bgfx OpenGL backend does not handle a caller-provided GLXContext cleanly during init -- bgfx asserts in the maxTextureSize check (GL extension/limit queries see no current context). The production Linux path uses pd.context = nullptr (bgfx owns the GL context). Documented in file header alongside the D3D12 skip rationale. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Buffer
Two cleanups based on review feedback:
1. Decouple from BackBuffer.
The earlier version paired UpdateDevice with UpdateBackBuffer because on
D3D11 a caller-owned ID3D11RenderTargetView is bound to a specific
ID3D11Device, so swapping the device requires also providing a fresh
RTV. That's a real concern, but it's a property of caller-provided
BackBuffers, not of UpdateDevice itself. The test now lets bgfx manage
its own swap chain on the App-layer HWND so it focuses purely on the
UpdateDevice contract. The caller-owned BackBuffer flow is already
covered by TEST(Device, BackBuffer) in Tests.Device.D3D11.cpp.
2. Restrict to D3D11 / D3D12 backends.
- Metal: MTL::Device is a per-GPU singleton (no API to create distinct
devices for the same GPU), so the test cannot meaningfully exercise
the swap on typical CI / dev hardware. CI macOS runners use the bgfx
noop renderer anyway.
- OpenGL: bgfx's GL backend does not handle a caller-provided GLXContext
through bgfx::init -- shipping code uses pd.context = nullptr.
The Tests.Device.cpp file is now only included in WIN32 D3D11 / D3D12
builds via the CMakeLists gate.
Reverts that fall out of (2):
- Utils.Metal.mm and Utils.OpenGL.cpp lose their CreateGraphicsDeviceForTest
definitions (unused on those platforms now).
- App.X11.cpp loses the g_display global that was added for OpenGL test
consumption.
- GRAPHICS_API_${GRAPHICS_API} compile def is no longer needed in
Apps/UnitTests/CMakeLists.txt.
Note on D3D12 + WARP:
D3D12CreateDevice(WARP) returns the same singleton pointer on successive
calls in the same process, so deviceA == deviceB after CreateGraphicsDeviceForTest
is called twice. The test no longer asserts EXPECT_NE -- the bgfx
teardown / re-init lifecycle is still exercised even when the underlying
pointer is the same. D3D11CreateDevice(WARP) does return distinct
pointers; the test is identical on both backends.
Local verification: D3D11 and D3D12 UnitTests both PASS Device.UpdateDevice.
[Created by Copilot on behalf of @bghgary]
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bghgary
changed the title
Updates the Tests.Device.cpp file-header to reflect that the DisableRendering requirement is enforced (UpdateDevice throws std::runtime_error if called while rendering is enabled, per BabylonJS#1689) rather than just documented. Cross-references the contract test in Tests.Device.D3D11.cpp. [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bghgary
added a commit
to bghgary/BabylonNative
that referenced
this pull request
May 6, 2026
The post-Disable frame + GetPlatformInfo assertion was demonstrating the canonical UpdateDevice usage sequence, which is what TEST(Device, UpdateDevice) in BabylonJS#1687 does. The throw-test should focus on the throw contract only: - permitted pre-init - throws after EnableRendering - permitted again after DisableRendering [Created by Copilot on behalf of @bghgary] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkaradzic-microsoft
approved these changes
May 6, 2026
Drop the file-header explanation of the contract (now documented on the API itself in Device.h) and the cross-reference to TEST(Device, UpdateDeviceThrowsWhenRenderingEnabled). Also fix the Utils.h doc-comment that incorrectly claimed CreateGraphicsDeviceForTest returns distinct handles per call -- D3D12CreateDevice(WARP) returns the same singleton on successive calls in one process. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Quiescence -> Idle. Same meaning, plainer word. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new UnitTests case validating the runtime graphics-device swap sequence (DisableRendering → UpdateDevice → render again) for the D3D11 and D3D12 backends, using WARP-created devices.
Changes:
- Add
TEST(Device, UpdateDevice)that drives frames before/after swapping the underlying graphics device pointer. - Introduce
CreateGraphicsDeviceForTest/DestroyGraphicsDeviceForTesthelpers for D3D11/D3D12 WARP device creation/teardown. - Gate the new test source to D3D11/D3D12 builds in UnitTests CMake.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Apps/UnitTests/Source/Utils.h | Declares new per-backend helpers for creating/destroying test graphics devices. |
| Apps/UnitTests/Source/Utils.D3D11.cpp | Implements WARP ID3D11Device creation + teardown helper. |
| Apps/UnitTests/Source/Utils.D3D12.cpp | Implements WARP adapter enumeration + ID3D12Device creation + teardown helper. |
| Apps/UnitTests/Source/Tests.Device.cpp | Adds Device.UpdateDevice test that swaps Configuration::Device at runtime and verifies the active device pointer. |
| Apps/UnitTests/CMakeLists.txt | Ensures Tests.Device.cpp is only compiled for D3D11/D3D12. |
CreateGraphicsDeviceForTest -> CreateTestGraphicsDevice (and Destroy* analogously) so the new helpers follow the existing <Verb>Test<Noun> shape used by CreateTestTexture / DestroyTestTexture in the same file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…azards Cascading null-deref hazards in CreateTestGraphicsDevice on D3D12 if CreateDXGIFactory1 / EnumWarpAdapter / D3D12CreateDevice fail mid-way: EXPECT_HRESULT_SUCCEEDED logs the failure but does not abort the test, so the next call still runs against a null pointer, and the unconditional Release calls at the end will null-deref too. Switch to winrt::com_ptr for the transient COM objects (factory, warpAdapter) and use winrt::check_hresult which throws on failure -- gtest catches and marks the test failed, and RAII handles cleanup automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The multi-paragraph header duplicated the inline comments next to the EXPECT_EQ / scope-block lines they were explaining (WARP singleton caveat, cleanup-order rationale). The remaining "D3D11/D3D12 only" justification belongs at the gate in Apps/UnitTests/CMakeLists.txt, not at the test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Change
Adds
TEST(Device, UpdateDevice)toApps/UnitTestsexercising the canonical sequence for swapping the underlying graphics device at runtime (D3D11/D3D12 device-removed recovery, host-driven device swaps in embedding scenarios):Babylon::Graphics::Devicewith caller-provideddeviceA.device.DisableRendering().device.UpdateDevice(deviceB).device.GetPlatformInfo().Device == deviceB.The test lets the swap chain be managed on the App layer's HWND — the caller-owned BackBuffer flow is a separate concern already covered by
TEST(Device, BackBuffer)inTests.Device.D3D11.cpp.Per-platform device creation lives in
Utils.${API}.${EXT}helpers (CreateGraphicsDeviceForTest,DestroyGraphicsDeviceForTest).Per-platform scope
The test runs on D3D11 and D3D12 only. It is not built on Metal or OpenGL because:
MTL::Deviceis a per-GPU singleton — there is no API to create distinct devices for the same physical GPU. CI macOS runners additionally use the noop renderer (no real Metal device available).pd.context = nullptr(the backend owns the context).D3D12 + WARP caveat
D3D12CreateDevice(WARP)returns the same singleton pointer on successive calls in the same process, sodeviceA == deviceBafterCreateGraphicsDeviceForTestis called twice. The test does not assert distinctness for that reason — the teardown / re-init lifecycle is still exercised even when the underlying pointer happens to be the same.D3D11CreateDevice(WARP)does return distinct pointers; the test is otherwise identical on both backends.Local verification
Device.UpdateDevice.Device.UpdateDevicePASSED in 221ms.Files
Apps/UnitTests/Source/Tests.Device.cpp(new).Apps/UnitTests/Source/Utils.h— declarations forCreateGraphicsDeviceForTest/DestroyGraphicsDeviceForTest.Apps/UnitTests/Source/Utils.{D3D11,D3D12}.cpp— WARP device creation.Apps/UnitTests/CMakeLists.txt— gatesTests.Device.cppto D3D11 / D3D12 builds only.[Created by Copilot on behalf of @bghgary]