From 9a776f13f354ffeea90b852b4338eb9cfa990085 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Wed, 6 May 2026 10:21:02 -0700 Subject: [PATCH 01/10] test: cross-platform Device.UpdateDevice unit test 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> --- Apps/UnitTests/CMakeLists.txt | 5 +- Apps/UnitTests/Source/App.X11.cpp | 5 + Apps/UnitTests/Source/Tests.Device.cpp | 147 +++++++++++++++++++++++++ Apps/UnitTests/Source/Utils.D3D11.cpp | 66 +++++++++++ Apps/UnitTests/Source/Utils.D3D12.cpp | 27 +++++ Apps/UnitTests/Source/Utils.Metal.mm | 24 ++++ Apps/UnitTests/Source/Utils.OpenGL.cpp | 51 +++++++++ Apps/UnitTests/Source/Utils.h | 26 +++++ 8 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 Apps/UnitTests/Source/Tests.Device.cpp diff --git a/Apps/UnitTests/CMakeLists.txt b/Apps/UnitTests/CMakeLists.txt index ae1fb713e..05cdc4fec 100644 --- a/Apps/UnitTests/CMakeLists.txt +++ b/Apps/UnitTests/CMakeLists.txt @@ -17,6 +17,7 @@ set(TEST_ASSETS set(SOURCES "Source/App.h" "Source/App.cpp" + "Source/Tests.Device.cpp" "Source/Tests.ExternalTexture.cpp" "Source/Tests.JavaScript.cpp" "Source/Tests.ShaderCache.cpp" @@ -42,6 +43,8 @@ if(APPLE) elseif(UNIX AND NOT ANDROID) set(SOURCES ${SOURCES} "Source/App.X11.cpp") set(ADDITIONAL_COMPILE_DEFINITIONS PRIVATE SKIP_EXTERNAL_TEXTURE_TESTS) + find_package(OpenGL REQUIRED) + set(ADDITIONAL_LIBRARIES PRIVATE OpenGL::GLX PRIVATE OpenGL::GL) elseif(WIN32) set(SOURCES ${SOURCES} "Source/App.Win32.cpp") endif() @@ -63,7 +66,7 @@ target_link_libraries(UnitTests PRIVATE gtest_main ${ADDITIONAL_LIBRARIES}) -target_compile_definitions(UnitTests PRIVATE ${ADDITIONAL_COMPILE_DEFINITIONS}) +target_compile_definitions(UnitTests PRIVATE ${ADDITIONAL_COMPILE_DEFINITIONS} PRIVATE GRAPHICS_API_${GRAPHICS_API}) add_test(NAME UnitTests COMMAND UnitTests) diff --git a/Apps/UnitTests/Source/App.X11.cpp b/Apps/UnitTests/Source/App.X11.cpp index efee592c9..27d256ab5 100644 --- a/Apps/UnitTests/Source/App.X11.cpp +++ b/Apps/UnitTests/Source/App.X11.cpp @@ -16,6 +16,10 @@ namespace constexpr const char* wmDeleteWindowName = "WM_DELETE_WINDOW"; } +// Exposed so that helpers in Utils.OpenGL.cpp can construct GLX contexts against the same X Display +// the App layer is already using. +Display* g_display = nullptr; + std::filesystem::path GetExecutableDirectory() { return std::filesystem::canonical("/proc/self/exe").parent_path(); @@ -25,6 +29,7 @@ int main(int argc, char* argv[]) { XInitThreads(); Display* display = XOpenDisplay(NULL); + g_display = display; int32_t screen = DefaultScreen(display); int32_t depth = DefaultDepth(display, screen); Visual* visual = DefaultVisual(display, screen); diff --git a/Apps/UnitTests/Source/Tests.Device.cpp b/Apps/UnitTests/Source/Tests.Device.cpp new file mode 100644 index 000000000..9afb8b0a7 --- /dev/null +++ b/Apps/UnitTests/Source/Tests.Device.cpp @@ -0,0 +1,147 @@ +#include + +#include + +#include "Utils.h" + +extern Babylon::Graphics::Configuration g_deviceConfig; + +// Demonstrates the proper sequence for swapping the underlying graphics device at runtime. +// +// Motivating use cases: +// - D3D11/D3D12 device-removed recovery (DXGI_ERROR_DEVICE_REMOVED on Present, GPU TDR, hot +// unplug). The app detects the removal, creates a fresh device, and swaps it in. +// - Voluntary GPU swap on Metal multi-GPU machines (integrated <-> discrete, eGPU connect / +// disconnect, multi-display compositing). +// - Host-driven device swap in embedding scenarios (e.g. WPF/Win32 interop where the host's +// ID3D11Device changes). +// +// Mechanics: +// UpdateDevice on its own only stores the new device pointer in Bgfx.InitState.platformData. +// While bgfx is already initialized, EnableRendering (called 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 the JS-side +// setDeviceLostCallback) fires automatically as part of that second EnableRendering, allowing +// JS to rebuild GPU resources on the new device. +// +// Quiescence requirement (not exercised here, but required in production): +// DisableRendering must be called between frames and with no outstanding JS / FrameCompletionScope +// work. This test runs purely on the render thread with no JS engine attached, so the requirement +// is trivially satisfied. +// +// Per-platform notes: +// D3D11 -> back-buffer textures are owned by a specific ID3D11Device, so UpdateBackBuffer must +// accompany UpdateDevice. UpdateDevice and UpdateBackBuffer form a single logical swap +// and must NOT be separated by rendering. +// D3D12 -> SKIPPED at runtime (see GRAPHICS_API_D3D12 guard below). The bgfx D3D12 backend +// hits a fatal lifecycle issue when the caller provides their own ID3D12Device: +// bgfx::shutdown unconditionally enforces that the device's COM refcount drops to 0, +// but with a caller-provided device the caller still holds a reference, so shutdown +// terminates the process. The same ref-count assumption is also flagged in +// App.Win32.cpp (the App-layer workaround creates an HWND so bgfx can manage the +// device internally). Until that bgfx limitation is addressed, exercising +// DisableRendering -> UpdateDevice -> re-init on D3D12 with a caller-provided device +// is not feasible from a unit test. +// Metal -> MTL::Device is a per-GPU singleton; MTL::CreateSystemDefaultDevice returns the same +// handle on successive calls for the same physical GPU. There is no Metal API that +// creates a second device for the same GPU. On single-GPU machines (typical Apple +// Silicon dev hardware and any macOS CI runner where Metal is exposed at all) +// deviceA == deviceB; the test still exercises the bgfx teardown/re-init path but the +// EXPECT_EQ on the post-swap PlatformInfo.Device passes for the same reason as before +// the swap, not because a different device was actually adopted. Distinct-device +// coverage requires running on multi-GPU hardware. Additionally, GitHub Actions macOS +// runners are headless VMs without a real Metal device; the BabylonNative CI +// workflow forces the bgfx noop renderer via BABYLON_NATIVE_TESTS_USE_NOOP_METAL_DEVICE, +// which defines USE_NOOP_METAL_DEVICE here -- the test is skipped in that environment. +// OpenGL -> the bgfx GL backend production path on Linux passes pd.context = nullptr and lets +// bgfx own the GLX context. This test exercises the alternate caller-provided-context +// path. If the test environment can't construct a GLX context (e.g. Mesa software +// rasterizer without the requested visual), the helper returns nullptr and the test +// skips. +// +// Cleanup order: +// The Babylon::Graphics::Device destructor calls DisableRendering internally, which still expects +// the active back-buffer and graphics device to be valid. The Device is therefore scoped so it +// destructs before any caller-owned device / back-buffer handles are released. +TEST(Device, UpdateDevice) +{ +#ifdef GRAPHICS_API_D3D12 + GTEST_SKIP() << "Skipping: bgfx D3D12 backend asserts a zero device refcount on shutdown, which " + "the caller-provided ID3D12Device pattern cannot satisfy. See file header."; +#elif defined(USE_NOOP_METAL_DEVICE) + GTEST_SKIP() << "Skipping: BABYLON_NATIVE_TESTS_USE_NOOP_METAL_DEVICE is enabled, so the bgfx " + "noop renderer is in use and there is no real Metal device to exercise."; +#else + Babylon::Graphics::DeviceT deviceA = CreateGraphicsDeviceForTest(); + if (deviceA == nullptr) + { + GTEST_SKIP() << "Skipping: graphics device creation failed in this test environment."; + } + +#ifdef GRAPHICS_BACK_BUFFER_SUPPORT + const uint32_t backBufferWidth = g_deviceConfig.Width != 0 ? static_cast(g_deviceConfig.Width) : 1280u; + const uint32_t backBufferHeight = g_deviceConfig.Height != 0 ? static_cast(g_deviceConfig.Height) : 720u; + Babylon::Graphics::BackBufferColorT backBufferA = CreateBackBufferForTest(deviceA, backBufferWidth, backBufferHeight); +#endif + + Babylon::Graphics::DeviceT deviceB = nullptr; +#ifdef GRAPHICS_BACK_BUFFER_SUPPORT + Babylon::Graphics::BackBufferColorT backBufferB = nullptr; +#endif + + { + // Inherit Window / Width / Height (and on Apple any platform-default Device) from the App + // layer's config -- bgfx GL / D3D12 backends need a real window handle for their swap chain. + Babylon::Graphics::Configuration config = g_deviceConfig; + config.Device = deviceA; +#ifdef GRAPHICS_BACK_BUFFER_SUPPORT + config.BackBufferColor = backBufferA; + config.Width = backBufferWidth; + config.Height = backBufferHeight; +#endif + + Babylon::Graphics::Device device{config}; + + // Drive a frame to force EnableRendering -> bgfx::init with deviceA. + device.StartRenderingCurrentFrame(); + device.FinishRenderingCurrentFrame(); + + EXPECT_EQ(device.GetPlatformInfo().Device, deviceA); + + deviceB = CreateGraphicsDeviceForTest(); + ASSERT_NE(deviceB, nullptr); + +#ifdef GRAPHICS_BACK_BUFFER_SUPPORT + backBufferB = CreateBackBufferForTest(deviceB, backBufferWidth, backBufferHeight); +#endif + + // Tear bgfx down before pointing the device at the new graphics device. + device.DisableRendering(); + + // UpdateDevice + UpdateBackBuffer (on D3D11) form a single logical swap: do not start a + // frame between these calls or the temporary mismatched back-buffer/device state would be + // visible to bgfx::init. + device.UpdateDevice(deviceB); +#ifdef GRAPHICS_BACK_BUFFER_SUPPORT + device.UpdateBackBuffer(backBufferB); +#endif + + // Drive another frame to force EnableRendering -> bgfx::init with deviceB. + device.StartRenderingCurrentFrame(); + device.FinishRenderingCurrentFrame(); + + // On Metal single-GPU hardware deviceA == deviceB, so this assertion is also satisfied for + // the trivial reason that nothing changed. On all other platforms (and on multi-GPU Metal) + // it proves the new device was adopted. + EXPECT_EQ(device.GetPlatformInfo().Device, deviceB); + } // Babylon::Graphics::Device destructs here, calling DisableRendering on deviceB / backBufferB. + +#ifdef GRAPHICS_BACK_BUFFER_SUPPORT + DestroyBackBufferForTest(backBufferB); + DestroyBackBufferForTest(backBufferA); +#endif + DestroyGraphicsDeviceForTest(deviceB); + DestroyGraphicsDeviceForTest(deviceA); +#endif +} diff --git a/Apps/UnitTests/Source/Utils.D3D11.cpp b/Apps/UnitTests/Source/Utils.D3D11.cpp index e5bfd2c26..4aafe6186 100644 --- a/Apps/UnitTests/Source/Utils.D3D11.cpp +++ b/Apps/UnitTests/Source/Utils.D3D11.cpp @@ -26,3 +26,69 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) { texture->Release(); } + +Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() +{ + ID3D11Device* device = nullptr; + EXPECT_HRESULT_SUCCEEDED(D3D11CreateDevice( + nullptr, + D3D_DRIVER_TYPE_WARP, + nullptr, + 0, + nullptr, + 0, + D3D11_SDK_VERSION, + &device, + nullptr, + nullptr)); + return device; +} + +void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) +{ + if (device != nullptr) + { + device->Release(); + } +} + +Babylon::Graphics::BackBufferColorT CreateBackBufferForTest(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height) +{ + D3D11_TEXTURE2D_DESC desc{}; + desc.Width = width; + desc.Height = height; + desc.MipLevels = 1; + desc.ArraySize = 1; + desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM; + desc.SampleDesc.Count = 1; + desc.SampleDesc.Quality = 0; + desc.Usage = D3D11_USAGE_DEFAULT; + desc.BindFlags = D3D11_BIND_RENDER_TARGET; + desc.CPUAccessFlags = 0; + desc.MiscFlags = 0; + + ID3D11Texture2D* texture = nullptr; + EXPECT_HRESULT_SUCCEEDED(device->CreateTexture2D(&desc, nullptr, &texture)); + + D3D11_RENDER_TARGET_VIEW_DESC rtvDesc{}; + rtvDesc.Format = desc.Format; + rtvDesc.ViewDimension = D3D11_RTV_DIMENSION_TEXTURE2D; + rtvDesc.Texture2D.MipSlice = 0; + + ID3D11RenderTargetView* view = nullptr; + EXPECT_HRESULT_SUCCEEDED(device->CreateRenderTargetView(texture, &rtvDesc, &view)); + + // The view holds the only outstanding reference to the texture from now on; releasing the view + // (in DestroyBackBufferForTest) also releases the texture. + texture->Release(); + + return view; +} + +void DestroyBackBufferForTest(Babylon::Graphics::BackBufferColorT backBuffer) +{ + if (backBuffer != nullptr) + { + backBuffer->Release(); + } +} diff --git a/Apps/UnitTests/Source/Utils.D3D12.cpp b/Apps/UnitTests/Source/Utils.D3D12.cpp index 65b4f6d23..dc35ed454 100644 --- a/Apps/UnitTests/Source/Utils.D3D12.cpp +++ b/Apps/UnitTests/Source/Utils.D3D12.cpp @@ -1,6 +1,8 @@ #include #include "Utils.h" +#include + Babylon::Graphics::TextureT CreateTestTexture(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height, uint32_t arraySize) { D3D12_RESOURCE_DESC desc{}; @@ -39,3 +41,28 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) { texture->Release(); } + +Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() +{ + IDXGIFactory4* factory = nullptr; + EXPECT_HRESULT_SUCCEEDED(CreateDXGIFactory1(IID_PPV_ARGS(&factory))); + + IDXGIAdapter* warpAdapter = nullptr; + EXPECT_HRESULT_SUCCEEDED(factory->EnumWarpAdapter(IID_PPV_ARGS(&warpAdapter))); + + ID3D12Device* device = nullptr; + EXPECT_HRESULT_SUCCEEDED(D3D12CreateDevice(warpAdapter, D3D_FEATURE_LEVEL_11_0, IID_PPV_ARGS(&device))); + + warpAdapter->Release(); + factory->Release(); + + return device; +} + +void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) +{ + if (device != nullptr) + { + device->Release(); + } +} diff --git a/Apps/UnitTests/Source/Utils.Metal.mm b/Apps/UnitTests/Source/Utils.Metal.mm index b2ec3603e..129682a02 100644 --- a/Apps/UnitTests/Source/Utils.Metal.mm +++ b/Apps/UnitTests/Source/Utils.Metal.mm @@ -22,3 +22,27 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) { texture->release(); } + +Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() +{ + // Metal does not expose a "create a fresh device for the same GPU" API. MTL::Device is a + // per-GPU singleton: every call to MTL::CreateSystemDefaultDevice (or any entry of + // MTL::CopyAllDevices) for the same physical GPU returns a reference to the same logical + // device. On single-GPU machines (typical Apple Silicon Macs and most macOS CI runners that + // expose Metal at all) two successive calls therefore return the same MTL::Device handle. + // + // For the UpdateDevice test this means the "swap from device A to device B" assertion only + // proves a different device was actually adopted on multi-GPU hardware (Intel Mac with discrete + // GPU, eGPU attached, etc.). On single-GPU hardware the test still exercises the bgfx + // teardown/re-init path but the post-swap PlatformInfo.Device equals the pre-swap one because + // there is only one Metal device to swap between. + return MTL::CreateSystemDefaultDevice(); +} + +void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) +{ + if (device != nullptr) + { + device->release(); + } +} diff --git a/Apps/UnitTests/Source/Utils.OpenGL.cpp b/Apps/UnitTests/Source/Utils.OpenGL.cpp index 7f1ab668a..98cd2b682 100644 --- a/Apps/UnitTests/Source/Utils.OpenGL.cpp +++ b/Apps/UnitTests/Source/Utils.OpenGL.cpp @@ -1,6 +1,11 @@ #include #include "Utils.h" +#include +#include + +extern Display* g_display; + Babylon::Graphics::TextureT CreateTestTexture(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height, uint32_t arraySize) { throw std::runtime_error{"not implemented"}; @@ -10,3 +15,49 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) { throw std::runtime_error{"not implemented"}; } + +Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() +{ + // The bgfx OpenGL backend production path on Linux passes pd.context = nullptr and lets bgfx + // create its own GLX context bound to the X window. The UpdateDevice cross-platform test + // exercises the alternate path where the caller owns the GL context, so we construct a + // GLXContext here against the App layer's X Display. + if (g_display == nullptr) + { + return nullptr; + } + + static int fbAttribs[] = { + GLX_X_RENDERABLE, True, + GLX_DRAWABLE_TYPE, GLX_WINDOW_BIT, + GLX_RENDER_TYPE, GLX_RGBA_BIT, + GLX_X_VISUAL_TYPE, GLX_TRUE_COLOR, + GLX_RED_SIZE, 8, + GLX_GREEN_SIZE, 8, + GLX_BLUE_SIZE, 8, + GLX_ALPHA_SIZE, 8, + GLX_DEPTH_SIZE, 24, + GLX_STENCIL_SIZE, 8, + GLX_DOUBLEBUFFER, True, + None}; + + int fbCount = 0; + GLXFBConfig* fbConfigs = glXChooseFBConfig(g_display, DefaultScreen(g_display), fbAttribs, &fbCount); + if (fbConfigs == nullptr || fbCount == 0) + { + return nullptr; + } + + GLXContext context = glXCreateNewContext(g_display, fbConfigs[0], GLX_RGBA_TYPE, nullptr, True); + XFree(fbConfigs); + + return reinterpret_cast(context); +} + +void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) +{ + if (device != nullptr && g_display != nullptr) + { + glXDestroyContext(g_display, reinterpret_cast(device)); + } +} diff --git a/Apps/UnitTests/Source/Utils.h b/Apps/UnitTests/Source/Utils.h index a3ee6a982..db7945071 100644 --- a/Apps/UnitTests/Source/Utils.h +++ b/Apps/UnitTests/Source/Utils.h @@ -4,3 +4,29 @@ Babylon::Graphics::TextureT CreateTestTexture(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height, uint32_t arraySize = 1); void DestroyTestTexture(Babylon::Graphics::TextureT texture); + +// Returns a graphics device suitable for use as Babylon::Graphics::Configuration::Device. +// +// The returned handle is owned by the caller and must be released with DestroyGraphicsDeviceForTest. +// +// Per-platform behaviour: +// D3D11 -> a fresh ID3D11Device created via D3D11CreateDevice(WARP). Each call returns a distinct handle. +// D3D12 -> a fresh ID3D12Device created via D3D12CreateDevice on the WARP DXGI adapter. Each call returns a distinct handle. +// Metal -> the system-default MTL::Device (MTLCreateSystemDefaultDevice). Metal devices are singleton-per-GPU, +// so on single-GPU machines successive calls return the same handle. There is no Metal API that +// creates a second device for the same GPU. +// OpenGL -> a GLXContext created via glXCreateNewContext on the test app's X Display. Each call returns a +// distinct handle. Returns nullptr if the test environment cannot construct a GLX context. +Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest(); + +void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device); + +#ifdef GRAPHICS_BACK_BUFFER_SUPPORT +// Creates a back-buffer color render target on the given D3D11 device. +// +// The returned RTV holds the only reference to the underlying texture, so releasing it with +// DestroyBackBufferForTest also frees the texture. +Babylon::Graphics::BackBufferColorT CreateBackBufferForTest(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height); + +void DestroyBackBufferForTest(Babylon::Graphics::BackBufferColorT backBuffer); +#endif From 1eff5c6715a7bb72c83ee4b91e04fa28d6b62033 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Wed, 6 May 2026 10:36:19 -0700 Subject: [PATCH 02/10] fix(test): use X11 None/True literal values in Utils.OpenGL.cpp 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 ), 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> --- Apps/UnitTests/Source/Utils.OpenGL.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Apps/UnitTests/Source/Utils.OpenGL.cpp b/Apps/UnitTests/Source/Utils.OpenGL.cpp index 98cd2b682..f3829f76c 100644 --- a/Apps/UnitTests/Source/Utils.OpenGL.cpp +++ b/Apps/UnitTests/Source/Utils.OpenGL.cpp @@ -27,8 +27,11 @@ Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() return nullptr; } + // GLX attribute list. X11's True / None macros expand to 1 / 0L respectively; we use the + // literals here because gtest-port.h may have already #undef'd them (it defends itself + // against X11 macro contamination). static int fbAttribs[] = { - GLX_X_RENDERABLE, True, + GLX_X_RENDERABLE, 1, GLX_DRAWABLE_TYPE, GLX_WINDOW_BIT, GLX_RENDER_TYPE, GLX_RGBA_BIT, GLX_X_VISUAL_TYPE, GLX_TRUE_COLOR, @@ -38,8 +41,11 @@ Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() GLX_ALPHA_SIZE, 8, GLX_DEPTH_SIZE, 24, GLX_STENCIL_SIZE, 8, - GLX_DOUBLEBUFFER, True, - None}; + GLX_DOUBLEBUFFER, 1, + // X11 None (0L) is the conventional list terminator. Spelled as a literal here because + // gtest-port.h aggressively #undef's None to defend against X11 contamination, so the + // macro isn't visible at this point even though X11/Xlib.h was included above. + 0}; int fbCount = 0; GLXFBConfig* fbConfigs = glXChooseFBConfig(g_display, DefaultScreen(g_display), fbAttribs, &fbCount); @@ -48,7 +54,7 @@ Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() return nullptr; } - GLXContext context = glXCreateNewContext(g_display, fbConfigs[0], GLX_RGBA_TYPE, nullptr, True); + GLXContext context = glXCreateNewContext(g_display, fbConfigs[0], GLX_RGBA_TYPE, nullptr, 1); XFree(fbConfigs); return reinterpret_cast(context); From 9e30c6915ea129d6c5ba2ab149f2292cbfc03688 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Wed, 6 May 2026 10:46:37 -0700 Subject: [PATCH 03/10] fix(test): skip Device.UpdateDevice on OpenGL 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> --- Apps/UnitTests/Source/Tests.Device.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.Device.cpp b/Apps/UnitTests/Source/Tests.Device.cpp index 9afb8b0a7..274369596 100644 --- a/Apps/UnitTests/Source/Tests.Device.cpp +++ b/Apps/UnitTests/Source/Tests.Device.cpp @@ -54,11 +54,15 @@ extern Babylon::Graphics::Configuration g_deviceConfig; // runners are headless VMs without a real Metal device; the BabylonNative CI // workflow forces the bgfx noop renderer via BABYLON_NATIVE_TESTS_USE_NOOP_METAL_DEVICE, // which defines USE_NOOP_METAL_DEVICE here -- the test is skipped in that environment. -// OpenGL -> the bgfx GL backend production path on Linux passes pd.context = nullptr and lets -// bgfx own the GLX context. This test exercises the alternate caller-provided-context -// path. If the test environment can't construct a GLX context (e.g. Mesa software -// rasterizer without the requested visual), the helper returns nullptr and the test -// skips. +// OpenGL -> SKIPPED at runtime (see GRAPHICS_API_OpenGL guard below). The bgfx OpenGL backend +// does not handle a caller-provided GLXContext cleanly during init -- it asserts in +// the maxTextureSize check, which suggests the GL extension/limit queries are not +// seeing the provided context (likely because the context isn't current on bgfx's +// render thread when bgfx::init runs). The production Linux path leaves +// Configuration.Device = nullptr so bgfx owns the GLX context internally, which is +// why this path isn't exercised in shipping code. Like the D3D12 case, exercising +// caller-provided-context UpdateDevice on OpenGL is not feasible from a unit test +// until the bgfx GL backend's caller-provided-context init path is fixed. // // Cleanup order: // The Babylon::Graphics::Device destructor calls DisableRendering internally, which still expects @@ -69,6 +73,12 @@ TEST(Device, UpdateDevice) #ifdef GRAPHICS_API_D3D12 GTEST_SKIP() << "Skipping: bgfx D3D12 backend asserts a zero device refcount on shutdown, which " "the caller-provided ID3D12Device pattern cannot satisfy. See file header."; +#elif defined(GRAPHICS_API_OpenGL) + GTEST_SKIP() << "Skipping: bgfx OpenGL backend does not handle a caller-provided GLXContext " + "cleanly during init -- bgfx asserts in the maxTextureSize check, suggesting " + "the GL extension/limit queries are not seeing the provided context. The " + "production Linux path uses pd.context = nullptr (bgfx owns the context). " + "See file header."; #elif defined(USE_NOOP_METAL_DEVICE) GTEST_SKIP() << "Skipping: BABYLON_NATIVE_TESTS_USE_NOOP_METAL_DEVICE is enabled, so the bgfx " "noop renderer is in use and there is no real Metal device to exercise."; From 952d031df4f4eb3e18c640580f9b319961e930f0 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Wed, 6 May 2026 13:43:34 -0700 Subject: [PATCH 04/10] test: scope Device.UpdateDevice to D3D11/D3D12 and decouple from BackBuffer 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> --- Apps/UnitTests/CMakeLists.txt | 10 +- Apps/UnitTests/Source/App.X11.cpp | 5 - Apps/UnitTests/Source/Tests.Device.cpp | 124 +++++++------------------ Apps/UnitTests/Source/Utils.D3D11.cpp | 41 -------- Apps/UnitTests/Source/Utils.Metal.mm | 24 ----- Apps/UnitTests/Source/Utils.OpenGL.cpp | 57 ------------ Apps/UnitTests/Source/Utils.h | 22 +---- 7 files changed, 45 insertions(+), 238 deletions(-) diff --git a/Apps/UnitTests/CMakeLists.txt b/Apps/UnitTests/CMakeLists.txt index 05cdc4fec..e99b63e43 100644 --- a/Apps/UnitTests/CMakeLists.txt +++ b/Apps/UnitTests/CMakeLists.txt @@ -17,7 +17,6 @@ set(TEST_ASSETS set(SOURCES "Source/App.h" "Source/App.cpp" - "Source/Tests.Device.cpp" "Source/Tests.ExternalTexture.cpp" "Source/Tests.JavaScript.cpp" "Source/Tests.ShaderCache.cpp" @@ -25,6 +24,11 @@ set(SOURCES "Source/Utils.h" "Source/Utils.${GRAPHICS_API}.${BABYLON_NATIVE_PLATFORM_IMPL_EXT}") +if(GRAPHICS_API STREQUAL "D3D11" OR GRAPHICS_API STREQUAL "D3D12") + set(SOURCES ${SOURCES} + "Source/Tests.Device.cpp") +endif() + if(GRAPHICS_API STREQUAL "D3D11") set(SOURCES ${SOURCES} "Source/Tests.Device.${GRAPHICS_API}.cpp" @@ -43,8 +47,6 @@ if(APPLE) elseif(UNIX AND NOT ANDROID) set(SOURCES ${SOURCES} "Source/App.X11.cpp") set(ADDITIONAL_COMPILE_DEFINITIONS PRIVATE SKIP_EXTERNAL_TEXTURE_TESTS) - find_package(OpenGL REQUIRED) - set(ADDITIONAL_LIBRARIES PRIVATE OpenGL::GLX PRIVATE OpenGL::GL) elseif(WIN32) set(SOURCES ${SOURCES} "Source/App.Win32.cpp") endif() @@ -66,7 +68,7 @@ target_link_libraries(UnitTests PRIVATE gtest_main ${ADDITIONAL_LIBRARIES}) -target_compile_definitions(UnitTests PRIVATE ${ADDITIONAL_COMPILE_DEFINITIONS} PRIVATE GRAPHICS_API_${GRAPHICS_API}) +target_compile_definitions(UnitTests PRIVATE ${ADDITIONAL_COMPILE_DEFINITIONS}) add_test(NAME UnitTests COMMAND UnitTests) diff --git a/Apps/UnitTests/Source/App.X11.cpp b/Apps/UnitTests/Source/App.X11.cpp index 27d256ab5..efee592c9 100644 --- a/Apps/UnitTests/Source/App.X11.cpp +++ b/Apps/UnitTests/Source/App.X11.cpp @@ -16,10 +16,6 @@ namespace constexpr const char* wmDeleteWindowName = "WM_DELETE_WINDOW"; } -// Exposed so that helpers in Utils.OpenGL.cpp can construct GLX contexts against the same X Display -// the App layer is already using. -Display* g_display = nullptr; - std::filesystem::path GetExecutableDirectory() { return std::filesystem::canonical("/proc/self/exe").parent_path(); @@ -29,7 +25,6 @@ int main(int argc, char* argv[]) { XInitThreads(); Display* display = XOpenDisplay(NULL); - g_display = display; int32_t screen = DefaultScreen(display); int32_t depth = DefaultDepth(display, screen); Visual* visual = DefaultVisual(display, screen); diff --git a/Apps/UnitTests/Source/Tests.Device.cpp b/Apps/UnitTests/Source/Tests.Device.cpp index 274369596..e2238b6ae 100644 --- a/Apps/UnitTests/Source/Tests.Device.cpp +++ b/Apps/UnitTests/Source/Tests.Device.cpp @@ -11,8 +11,6 @@ extern Babylon::Graphics::Configuration g_deviceConfig; // Motivating use cases: // - D3D11/D3D12 device-removed recovery (DXGI_ERROR_DEVICE_REMOVED on Present, GPU TDR, hot // unplug). The app detects the removal, creates a fresh device, and swaps it in. -// - Voluntary GPU swap on Metal multi-GPU machines (integrated <-> discrete, eGPU connect / -// disconnect, multi-display compositing). // - Host-driven device swap in embedding scenarios (e.g. WPF/Win32 interop where the host's // ID3D11Device changes). // @@ -30,86 +28,49 @@ extern Babylon::Graphics::Configuration g_deviceConfig; // work. This test runs purely on the render thread with no JS engine attached, so the requirement // is trivially satisfied. // -// Per-platform notes: -// D3D11 -> back-buffer textures are owned by a specific ID3D11Device, so UpdateBackBuffer must -// accompany UpdateDevice. UpdateDevice and UpdateBackBuffer form a single logical swap -// and must NOT be separated by rendering. -// D3D12 -> SKIPPED at runtime (see GRAPHICS_API_D3D12 guard below). The bgfx D3D12 backend -// hits a fatal lifecycle issue when the caller provides their own ID3D12Device: -// bgfx::shutdown unconditionally enforces that the device's COM refcount drops to 0, -// but with a caller-provided device the caller still holds a reference, so shutdown -// terminates the process. The same ref-count assumption is also flagged in -// App.Win32.cpp (the App-layer workaround creates an HWND so bgfx can manage the -// device internally). Until that bgfx limitation is addressed, exercising -// DisableRendering -> UpdateDevice -> re-init on D3D12 with a caller-provided device -// is not feasible from a unit test. -// Metal -> MTL::Device is a per-GPU singleton; MTL::CreateSystemDefaultDevice returns the same -// handle on successive calls for the same physical GPU. There is no Metal API that -// creates a second device for the same GPU. On single-GPU machines (typical Apple -// Silicon dev hardware and any macOS CI runner where Metal is exposed at all) -// deviceA == deviceB; the test still exercises the bgfx teardown/re-init path but the -// EXPECT_EQ on the post-swap PlatformInfo.Device passes for the same reason as before -// the swap, not because a different device was actually adopted. Distinct-device -// coverage requires running on multi-GPU hardware. Additionally, GitHub Actions macOS -// runners are headless VMs without a real Metal device; the BabylonNative CI -// workflow forces the bgfx noop renderer via BABYLON_NATIVE_TESTS_USE_NOOP_METAL_DEVICE, -// which defines USE_NOOP_METAL_DEVICE here -- the test is skipped in that environment. -// OpenGL -> SKIPPED at runtime (see GRAPHICS_API_OpenGL guard below). The bgfx OpenGL backend -// does not handle a caller-provided GLXContext cleanly during init -- it asserts in -// the maxTextureSize check, which suggests the GL extension/limit queries are not -// seeing the provided context (likely because the context isn't current on bgfx's -// render thread when bgfx::init runs). The production Linux path leaves -// Configuration.Device = nullptr so bgfx owns the GLX context internally, which is -// why this path isn't exercised in shipping code. Like the D3D12 case, exercising -// caller-provided-context UpdateDevice on OpenGL is not feasible from a unit test -// until the bgfx GL backend's caller-provided-context init path is fixed. +// Production note for D3D11 callers using a caller-owned BackBufferColor: +// When the caller provides Configuration::BackBufferColor (an ID3D11RenderTargetView) and then +// swaps to a new ID3D11Device, the existing RTV is bound to the old device and must be replaced +// with one created on the new device via UpdateBackBuffer. This test does not exercise that +// path -- it lets bgfx manage its own swap chain on the App layer's HWND so the test focuses +// purely on the UpdateDevice contract. The caller-owned BackBuffer flow is covered by +// TEST(Device, BackBuffer) in Tests.Device.D3D11.cpp. +// +// Per-platform scope: +// D3D11 / D3D12 -> the test runs using bgfx-managed swap chain bound to the App-layer HWND. +// On both backends bgfx::init / bgfx::shutdown handle a caller-provided +// ID3D11Device or ID3D12Device cleanly through the +// EnableRendering -> DisableRendering -> EnableRendering lifecycle. +// Note: D3D12CreateDevice(WARP) returns the same singleton pointer on +// successive calls (deviceA and deviceB compare equal), so post-swap distinctness +// cannot be asserted on D3D12 + WARP. The test still exercises the full bgfx +// teardown / re-init path -- bgfx::shutdown actually releases the command queue, +// descriptor heaps, root signatures, etc. and bgfx::init recreates them, even +// when the underlying ID3D12Device pointer happens to be the same. D3D11 +// (D3D11CreateDevice(WARP)) does return distinct pointers per call. +// Metal / OpenGL -> the test is not built on these platforms (see Apps/UnitTests/CMakeLists.txt). +// Metal does not expose an API to create distinct devices for the same GPU +// (MTL::Device is a per-GPU singleton), and the bgfx OpenGL backend does not +// support a caller-provided GLXContext through bgfx::init. The shipping code +// paths on those backends always use bgfx-owned contexts. // // Cleanup order: // The Babylon::Graphics::Device destructor calls DisableRendering internally, which still expects -// the active back-buffer and graphics device to be valid. The Device is therefore scoped so it -// destructs before any caller-owned device / back-buffer handles are released. +// the active graphics device to be valid. The Device is therefore scoped so it destructs before +// any caller-owned device handles are released. TEST(Device, UpdateDevice) { -#ifdef GRAPHICS_API_D3D12 - GTEST_SKIP() << "Skipping: bgfx D3D12 backend asserts a zero device refcount on shutdown, which " - "the caller-provided ID3D12Device pattern cannot satisfy. See file header."; -#elif defined(GRAPHICS_API_OpenGL) - GTEST_SKIP() << "Skipping: bgfx OpenGL backend does not handle a caller-provided GLXContext " - "cleanly during init -- bgfx asserts in the maxTextureSize check, suggesting " - "the GL extension/limit queries are not seeing the provided context. The " - "production Linux path uses pd.context = nullptr (bgfx owns the context). " - "See file header."; -#elif defined(USE_NOOP_METAL_DEVICE) - GTEST_SKIP() << "Skipping: BABYLON_NATIVE_TESTS_USE_NOOP_METAL_DEVICE is enabled, so the bgfx " - "noop renderer is in use and there is no real Metal device to exercise."; -#else Babylon::Graphics::DeviceT deviceA = CreateGraphicsDeviceForTest(); - if (deviceA == nullptr) - { - GTEST_SKIP() << "Skipping: graphics device creation failed in this test environment."; - } - -#ifdef GRAPHICS_BACK_BUFFER_SUPPORT - const uint32_t backBufferWidth = g_deviceConfig.Width != 0 ? static_cast(g_deviceConfig.Width) : 1280u; - const uint32_t backBufferHeight = g_deviceConfig.Height != 0 ? static_cast(g_deviceConfig.Height) : 720u; - Babylon::Graphics::BackBufferColorT backBufferA = CreateBackBufferForTest(deviceA, backBufferWidth, backBufferHeight); -#endif + ASSERT_NE(deviceA, nullptr); Babylon::Graphics::DeviceT deviceB = nullptr; -#ifdef GRAPHICS_BACK_BUFFER_SUPPORT - Babylon::Graphics::BackBufferColorT backBufferB = nullptr; -#endif { - // Inherit Window / Width / Height (and on Apple any platform-default Device) from the App - // layer's config -- bgfx GL / D3D12 backends need a real window handle for their swap chain. + // Inherit Window / Width / Height from the App layer's config so bgfx can manage its own + // swap chain on the HWND. We deliberately do NOT set BackBufferColor here -- the + // caller-provided-BackBuffer flow is a separate concern covered by TEST(Device, BackBuffer). Babylon::Graphics::Configuration config = g_deviceConfig; config.Device = deviceA; -#ifdef GRAPHICS_BACK_BUFFER_SUPPORT - config.BackBufferColor = backBufferA; - config.Width = backBufferWidth; - config.Height = backBufferHeight; -#endif Babylon::Graphics::Device device{config}; @@ -122,36 +83,21 @@ TEST(Device, UpdateDevice) deviceB = CreateGraphicsDeviceForTest(); ASSERT_NE(deviceB, nullptr); -#ifdef GRAPHICS_BACK_BUFFER_SUPPORT - backBufferB = CreateBackBufferForTest(deviceB, backBufferWidth, backBufferHeight); -#endif - // Tear bgfx down before pointing the device at the new graphics device. device.DisableRendering(); - // UpdateDevice + UpdateBackBuffer (on D3D11) form a single logical swap: do not start a - // frame between these calls or the temporary mismatched back-buffer/device state would be - // visible to bgfx::init. device.UpdateDevice(deviceB); -#ifdef GRAPHICS_BACK_BUFFER_SUPPORT - device.UpdateBackBuffer(backBufferB); -#endif // Drive another frame to force EnableRendering -> bgfx::init with deviceB. device.StartRenderingCurrentFrame(); device.FinishRenderingCurrentFrame(); - // On Metal single-GPU hardware deviceA == deviceB, so this assertion is also satisfied for - // the trivial reason that nothing changed. On all other platforms (and on multi-GPU Metal) - // it proves the new device was adopted. EXPECT_EQ(device.GetPlatformInfo().Device, deviceB); - } // Babylon::Graphics::Device destructs here, calling DisableRendering on deviceB / backBufferB. + // Note: no EXPECT_NE(deviceB, deviceA). On D3D12 with WARP, D3D12CreateDevice returns the + // same singleton pointer on successive calls so distinctness is not assertable. On D3D11 + // distinctness holds but exercising it does not add value over the EXPECT_EQ above. + } // Babylon::Graphics::Device destructs here, calling DisableRendering on deviceB. -#ifdef GRAPHICS_BACK_BUFFER_SUPPORT - DestroyBackBufferForTest(backBufferB); - DestroyBackBufferForTest(backBufferA); -#endif DestroyGraphicsDeviceForTest(deviceB); DestroyGraphicsDeviceForTest(deviceA); -#endif } diff --git a/Apps/UnitTests/Source/Utils.D3D11.cpp b/Apps/UnitTests/Source/Utils.D3D11.cpp index 4aafe6186..7dbec8ab3 100644 --- a/Apps/UnitTests/Source/Utils.D3D11.cpp +++ b/Apps/UnitTests/Source/Utils.D3D11.cpp @@ -51,44 +51,3 @@ void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) device->Release(); } } - -Babylon::Graphics::BackBufferColorT CreateBackBufferForTest(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height) -{ - D3D11_TEXTURE2D_DESC desc{}; - desc.Width = width; - desc.Height = height; - desc.MipLevels = 1; - desc.ArraySize = 1; - desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM; - desc.SampleDesc.Count = 1; - desc.SampleDesc.Quality = 0; - desc.Usage = D3D11_USAGE_DEFAULT; - desc.BindFlags = D3D11_BIND_RENDER_TARGET; - desc.CPUAccessFlags = 0; - desc.MiscFlags = 0; - - ID3D11Texture2D* texture = nullptr; - EXPECT_HRESULT_SUCCEEDED(device->CreateTexture2D(&desc, nullptr, &texture)); - - D3D11_RENDER_TARGET_VIEW_DESC rtvDesc{}; - rtvDesc.Format = desc.Format; - rtvDesc.ViewDimension = D3D11_RTV_DIMENSION_TEXTURE2D; - rtvDesc.Texture2D.MipSlice = 0; - - ID3D11RenderTargetView* view = nullptr; - EXPECT_HRESULT_SUCCEEDED(device->CreateRenderTargetView(texture, &rtvDesc, &view)); - - // The view holds the only outstanding reference to the texture from now on; releasing the view - // (in DestroyBackBufferForTest) also releases the texture. - texture->Release(); - - return view; -} - -void DestroyBackBufferForTest(Babylon::Graphics::BackBufferColorT backBuffer) -{ - if (backBuffer != nullptr) - { - backBuffer->Release(); - } -} diff --git a/Apps/UnitTests/Source/Utils.Metal.mm b/Apps/UnitTests/Source/Utils.Metal.mm index 129682a02..b2ec3603e 100644 --- a/Apps/UnitTests/Source/Utils.Metal.mm +++ b/Apps/UnitTests/Source/Utils.Metal.mm @@ -22,27 +22,3 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) { texture->release(); } - -Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() -{ - // Metal does not expose a "create a fresh device for the same GPU" API. MTL::Device is a - // per-GPU singleton: every call to MTL::CreateSystemDefaultDevice (or any entry of - // MTL::CopyAllDevices) for the same physical GPU returns a reference to the same logical - // device. On single-GPU machines (typical Apple Silicon Macs and most macOS CI runners that - // expose Metal at all) two successive calls therefore return the same MTL::Device handle. - // - // For the UpdateDevice test this means the "swap from device A to device B" assertion only - // proves a different device was actually adopted on multi-GPU hardware (Intel Mac with discrete - // GPU, eGPU attached, etc.). On single-GPU hardware the test still exercises the bgfx - // teardown/re-init path but the post-swap PlatformInfo.Device equals the pre-swap one because - // there is only one Metal device to swap between. - return MTL::CreateSystemDefaultDevice(); -} - -void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) -{ - if (device != nullptr) - { - device->release(); - } -} diff --git a/Apps/UnitTests/Source/Utils.OpenGL.cpp b/Apps/UnitTests/Source/Utils.OpenGL.cpp index f3829f76c..7f1ab668a 100644 --- a/Apps/UnitTests/Source/Utils.OpenGL.cpp +++ b/Apps/UnitTests/Source/Utils.OpenGL.cpp @@ -1,11 +1,6 @@ #include #include "Utils.h" -#include -#include - -extern Display* g_display; - Babylon::Graphics::TextureT CreateTestTexture(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height, uint32_t arraySize) { throw std::runtime_error{"not implemented"}; @@ -15,55 +10,3 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) { throw std::runtime_error{"not implemented"}; } - -Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() -{ - // The bgfx OpenGL backend production path on Linux passes pd.context = nullptr and lets bgfx - // create its own GLX context bound to the X window. The UpdateDevice cross-platform test - // exercises the alternate path where the caller owns the GL context, so we construct a - // GLXContext here against the App layer's X Display. - if (g_display == nullptr) - { - return nullptr; - } - - // GLX attribute list. X11's True / None macros expand to 1 / 0L respectively; we use the - // literals here because gtest-port.h may have already #undef'd them (it defends itself - // against X11 macro contamination). - static int fbAttribs[] = { - GLX_X_RENDERABLE, 1, - GLX_DRAWABLE_TYPE, GLX_WINDOW_BIT, - GLX_RENDER_TYPE, GLX_RGBA_BIT, - GLX_X_VISUAL_TYPE, GLX_TRUE_COLOR, - GLX_RED_SIZE, 8, - GLX_GREEN_SIZE, 8, - GLX_BLUE_SIZE, 8, - GLX_ALPHA_SIZE, 8, - GLX_DEPTH_SIZE, 24, - GLX_STENCIL_SIZE, 8, - GLX_DOUBLEBUFFER, 1, - // X11 None (0L) is the conventional list terminator. Spelled as a literal here because - // gtest-port.h aggressively #undef's None to defend against X11 contamination, so the - // macro isn't visible at this point even though X11/Xlib.h was included above. - 0}; - - int fbCount = 0; - GLXFBConfig* fbConfigs = glXChooseFBConfig(g_display, DefaultScreen(g_display), fbAttribs, &fbCount); - if (fbConfigs == nullptr || fbCount == 0) - { - return nullptr; - } - - GLXContext context = glXCreateNewContext(g_display, fbConfigs[0], GLX_RGBA_TYPE, nullptr, 1); - XFree(fbConfigs); - - return reinterpret_cast(context); -} - -void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) -{ - if (device != nullptr && g_display != nullptr) - { - glXDestroyContext(g_display, reinterpret_cast(device)); - } -} diff --git a/Apps/UnitTests/Source/Utils.h b/Apps/UnitTests/Source/Utils.h index db7945071..637777699 100644 --- a/Apps/UnitTests/Source/Utils.h +++ b/Apps/UnitTests/Source/Utils.h @@ -9,24 +9,10 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture); // // The returned handle is owned by the caller and must be released with DestroyGraphicsDeviceForTest. // -// Per-platform behaviour: -// D3D11 -> a fresh ID3D11Device created via D3D11CreateDevice(WARP). Each call returns a distinct handle. -// D3D12 -> a fresh ID3D12Device created via D3D12CreateDevice on the WARP DXGI adapter. Each call returns a distinct handle. -// Metal -> the system-default MTL::Device (MTLCreateSystemDefaultDevice). Metal devices are singleton-per-GPU, -// so on single-GPU machines successive calls return the same handle. There is no Metal API that -// creates a second device for the same GPU. -// OpenGL -> a GLXContext created via glXCreateNewContext on the test app's X Display. Each call returns a -// distinct handle. Returns nullptr if the test environment cannot construct a GLX context. +// Defined only on D3D11 and D3D12 -- the Tests.Device.cpp test that consumes these helpers is +// gated to those backends. On D3D11 returns a fresh ID3D11Device created via +// D3D11CreateDevice(WARP); on D3D12 returns a fresh ID3D12Device created via D3D12CreateDevice on +// the WARP DXGI adapter. Each call returns a distinct handle. Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest(); void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device); - -#ifdef GRAPHICS_BACK_BUFFER_SUPPORT -// Creates a back-buffer color render target on the given D3D11 device. -// -// The returned RTV holds the only reference to the underlying texture, so releasing it with -// DestroyBackBufferForTest also frees the texture. -Babylon::Graphics::BackBufferColorT CreateBackBufferForTest(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height); - -void DestroyBackBufferForTest(Babylon::Graphics::BackBufferColorT backBuffer); -#endif From ab3f409371d300a3e9021febf4c6f04fedc7f1ff Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Wed, 6 May 2026 14:53:46 -0700 Subject: [PATCH 05/10] test(doc): note that UpdateDevice contract is enforced via throw 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 #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> --- Apps/UnitTests/Source/Tests.Device.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.Device.cpp b/Apps/UnitTests/Source/Tests.Device.cpp index e2238b6ae..06cd069a0 100644 --- a/Apps/UnitTests/Source/Tests.Device.cpp +++ b/Apps/UnitTests/Source/Tests.Device.cpp @@ -16,12 +16,13 @@ extern Babylon::Graphics::Configuration g_deviceConfig; // // Mechanics: // UpdateDevice on its own only stores the new device pointer in Bgfx.InitState.platformData. -// While bgfx is already initialized, EnableRendering (called 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 the JS-side -// setDeviceLostCallback) fires automatically as part of that second EnableRendering, allowing -// JS to rebuild GPU resources on the new device. +// bgfx::init only runs again on the next EnableRendering, which early-outs while +// Bgfx.Initialized == true. The contract is therefore: DisableRendering must be called first to +// shut bgfx down so the next frame re-runs bgfx::init with the new device. This contract is +// enforced -- UpdateDevice throws std::runtime_error if called while rendering is enabled (see +// TEST(Device, UpdateDeviceThrowsWhenRenderingEnabled) in Tests.Device.D3D11.cpp). The render- +// reset callback (wired by NativeEngine to the JS-side setDeviceLostCallback) fires automatically +// as part of the second EnableRendering, allowing JS to rebuild GPU resources on the new device. // // Quiescence requirement (not exercised here, but required in production): // DisableRendering must be called between frames and with no outstanding JS / FrameCompletionScope From 536db57e80a378aa8421073b5af76b69f49b31bf Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Wed, 6 May 2026 16:03:42 -0700 Subject: [PATCH 06/10] Update Device.UpdateDevice test for landed throw contract 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> --- Apps/UnitTests/Source/Tests.Device.cpp | 55 ++++++++------------------ Apps/UnitTests/Source/Utils.h | 14 ++++--- 2 files changed, 25 insertions(+), 44 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.Device.cpp b/Apps/UnitTests/Source/Tests.Device.cpp index 06cd069a0..da922d49c 100644 --- a/Apps/UnitTests/Source/Tests.Device.cpp +++ b/Apps/UnitTests/Source/Tests.Device.cpp @@ -6,54 +6,33 @@ extern Babylon::Graphics::Configuration g_deviceConfig; -// Demonstrates the proper sequence for swapping the underlying graphics device at runtime. -// -// Motivating use cases: -// - D3D11/D3D12 device-removed recovery (DXGI_ERROR_DEVICE_REMOVED on Present, GPU TDR, hot -// unplug). The app detects the removal, creates a fresh device, and swaps it in. -// - Host-driven device swap in embedding scenarios (e.g. WPF/Win32 interop where the host's -// ID3D11Device changes). -// -// Mechanics: -// UpdateDevice on its own only stores the new device pointer in Bgfx.InitState.platformData. -// bgfx::init only runs again on the next EnableRendering, which early-outs while -// Bgfx.Initialized == true. The contract is therefore: DisableRendering must be called first to -// shut bgfx down so the next frame re-runs bgfx::init with the new device. This contract is -// enforced -- UpdateDevice throws std::runtime_error if called while rendering is enabled (see -// TEST(Device, UpdateDeviceThrowsWhenRenderingEnabled) in Tests.Device.D3D11.cpp). The render- -// reset callback (wired by NativeEngine to the JS-side setDeviceLostCallback) fires automatically -// as part of the second EnableRendering, allowing JS to rebuild GPU resources on the new device. +// Exercises the canonical UpdateDevice flow: drive a frame, DisableRendering, UpdateDevice to a +// new graphics device, drive another frame, and verify the active device pointer reflects the swap. +// The motivating use cases are D3D11/D3D12 device-removed recovery (DXGI_ERROR_DEVICE_REMOVED on +// Present, GPU TDR, hot unplug) and host-driven device swaps in embedding scenarios (e.g. WPF/Win32 +// interop where the host's ID3D11Device changes). // // Quiescence requirement (not exercised here, but required in production): // DisableRendering must be called between frames and with no outstanding JS / FrameCompletionScope // work. This test runs purely on the render thread with no JS engine attached, so the requirement // is trivially satisfied. // -// Production note for D3D11 callers using a caller-owned BackBufferColor: -// When the caller provides Configuration::BackBufferColor (an ID3D11RenderTargetView) and then -// swaps to a new ID3D11Device, the existing RTV is bound to the old device and must be replaced -// with one created on the new device via UpdateBackBuffer. This test does not exercise that -// path -- it lets bgfx manage its own swap chain on the App layer's HWND so the test focuses -// purely on the UpdateDevice contract. The caller-owned BackBuffer flow is covered by -// TEST(Device, BackBuffer) in Tests.Device.D3D11.cpp. -// // Per-platform scope: -// D3D11 / D3D12 -> the test runs using bgfx-managed swap chain bound to the App-layer HWND. -// On both backends bgfx::init / bgfx::shutdown handle a caller-provided -// ID3D11Device or ID3D12Device cleanly through the -// EnableRendering -> DisableRendering -> EnableRendering lifecycle. +// D3D11 / D3D12 -> the test runs using a bgfx-managed swap chain bound to the App-layer HWND. // Note: D3D12CreateDevice(WARP) returns the same singleton pointer on -// successive calls (deviceA and deviceB compare equal), so post-swap distinctness -// cannot be asserted on D3D12 + WARP. The test still exercises the full bgfx -// teardown / re-init path -- bgfx::shutdown actually releases the command queue, -// descriptor heaps, root signatures, etc. and bgfx::init recreates them, even -// when the underlying ID3D12Device pointer happens to be the same. D3D11 -// (D3D11CreateDevice(WARP)) does return distinct pointers per call. +// successive calls (deviceA and deviceB compare equal), so post-swap +// distinctness cannot be asserted on D3D12 + WARP. The test still exercises the +// full teardown / re-init path. D3D11 (D3D11CreateDevice(WARP)) does return +// distinct pointers per call. // Metal / OpenGL -> the test is not built on these platforms (see Apps/UnitTests/CMakeLists.txt). // Metal does not expose an API to create distinct devices for the same GPU -// (MTL::Device is a per-GPU singleton), and the bgfx OpenGL backend does not -// support a caller-provided GLXContext through bgfx::init. The shipping code -// paths on those backends always use bgfx-owned contexts. +// (MTL::Device is a per-GPU singleton), and the OpenGL backend does not +// support a caller-provided GLXContext at init. The shipping code paths on +// those backends always use backend-owned contexts. +// +// The test does not exercise the caller-owned BackBufferColor path; it lets the swap chain be +// managed on the App layer's HWND so the test focuses purely on UpdateDevice. The caller-owned +// BackBuffer flow is covered by TEST(Device, BackBuffer) in Tests.Device.D3D11.cpp. // // Cleanup order: // The Babylon::Graphics::Device destructor calls DisableRendering internally, which still expects diff --git a/Apps/UnitTests/Source/Utils.h b/Apps/UnitTests/Source/Utils.h index 637777699..0c389667b 100644 --- a/Apps/UnitTests/Source/Utils.h +++ b/Apps/UnitTests/Source/Utils.h @@ -5,14 +5,16 @@ Babylon::Graphics::TextureT CreateTestTexture(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height, uint32_t arraySize = 1); void DestroyTestTexture(Babylon::Graphics::TextureT texture); -// Returns a graphics device suitable for use as Babylon::Graphics::Configuration::Device. -// -// The returned handle is owned by the caller and must be released with DestroyGraphicsDeviceForTest. +// Returns a graphics device suitable for use as Babylon::Graphics::Configuration::Device. The +// returned handle is owned by the caller and must be released with DestroyGraphicsDeviceForTest. // // Defined only on D3D11 and D3D12 -- the Tests.Device.cpp test that consumes these helpers is -// gated to those backends. On D3D11 returns a fresh ID3D11Device created via -// D3D11CreateDevice(WARP); on D3D12 returns a fresh ID3D12Device created via D3D12CreateDevice on -// the WARP DXGI adapter. Each call returns a distinct handle. +// gated to those backends. On D3D11 returns an ID3D11Device created via D3D11CreateDevice(WARP); +// on D3D12 returns an ID3D12Device created via D3D12CreateDevice on the WARP DXGI adapter. +// +// Note: on D3D12, D3D12CreateDevice(WARP) returns the same singleton pointer on successive calls +// in the same process, so two calls in a row may return equal handles. D3D11CreateDevice(WARP) +// does return distinct handles per call. Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest(); void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device); From d3cdfeab02a99ceab901c2c681aafc3cadfaf1f7 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Thu, 7 May 2026 14:49:24 -0700 Subject: [PATCH 07/10] Drop jargon from Device.UpdateDevice test header Quiescence -> Idle. Same meaning, plainer word. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Apps/UnitTests/Source/Tests.Device.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Apps/UnitTests/Source/Tests.Device.cpp b/Apps/UnitTests/Source/Tests.Device.cpp index da922d49c..32fdfda7a 100644 --- a/Apps/UnitTests/Source/Tests.Device.cpp +++ b/Apps/UnitTests/Source/Tests.Device.cpp @@ -12,7 +12,7 @@ extern Babylon::Graphics::Configuration g_deviceConfig; // Present, GPU TDR, hot unplug) and host-driven device swaps in embedding scenarios (e.g. WPF/Win32 // interop where the host's ID3D11Device changes). // -// Quiescence requirement (not exercised here, but required in production): +// Idle requirement (not exercised here, but required in production): // DisableRendering must be called between frames and with no outstanding JS / FrameCompletionScope // work. This test runs purely on the render thread with no JS engine attached, so the requirement // is trivially satisfied. From 3ccb33cc34c8e523ee83d68caab01929175f925d Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Fri, 8 May 2026 08:44:14 -0700 Subject: [PATCH 08/10] Rename test device helpers to match CreateTestTexture style CreateGraphicsDeviceForTest -> CreateTestGraphicsDevice (and Destroy* analogously) so the new helpers follow the existing Test shape used by CreateTestTexture / DestroyTestTexture in the same file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Apps/UnitTests/Source/Tests.Device.cpp | 8 ++++---- Apps/UnitTests/Source/Utils.D3D11.cpp | 4 ++-- Apps/UnitTests/Source/Utils.D3D12.cpp | 4 ++-- Apps/UnitTests/Source/Utils.h | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.Device.cpp b/Apps/UnitTests/Source/Tests.Device.cpp index 32fdfda7a..6425bac53 100644 --- a/Apps/UnitTests/Source/Tests.Device.cpp +++ b/Apps/UnitTests/Source/Tests.Device.cpp @@ -40,7 +40,7 @@ extern Babylon::Graphics::Configuration g_deviceConfig; // any caller-owned device handles are released. TEST(Device, UpdateDevice) { - Babylon::Graphics::DeviceT deviceA = CreateGraphicsDeviceForTest(); + Babylon::Graphics::DeviceT deviceA = CreateTestGraphicsDevice(); ASSERT_NE(deviceA, nullptr); Babylon::Graphics::DeviceT deviceB = nullptr; @@ -60,7 +60,7 @@ TEST(Device, UpdateDevice) EXPECT_EQ(device.GetPlatformInfo().Device, deviceA); - deviceB = CreateGraphicsDeviceForTest(); + deviceB = CreateTestGraphicsDevice(); ASSERT_NE(deviceB, nullptr); // Tear bgfx down before pointing the device at the new graphics device. @@ -78,6 +78,6 @@ TEST(Device, UpdateDevice) // distinctness holds but exercising it does not add value over the EXPECT_EQ above. } // Babylon::Graphics::Device destructs here, calling DisableRendering on deviceB. - DestroyGraphicsDeviceForTest(deviceB); - DestroyGraphicsDeviceForTest(deviceA); + DestroyTestGraphicsDevice(deviceB); + DestroyTestGraphicsDevice(deviceA); } diff --git a/Apps/UnitTests/Source/Utils.D3D11.cpp b/Apps/UnitTests/Source/Utils.D3D11.cpp index 7dbec8ab3..8e57368c8 100644 --- a/Apps/UnitTests/Source/Utils.D3D11.cpp +++ b/Apps/UnitTests/Source/Utils.D3D11.cpp @@ -27,7 +27,7 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) texture->Release(); } -Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() +Babylon::Graphics::DeviceT CreateTestGraphicsDevice() { ID3D11Device* device = nullptr; EXPECT_HRESULT_SUCCEEDED(D3D11CreateDevice( @@ -44,7 +44,7 @@ Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() return device; } -void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) +void DestroyTestGraphicsDevice(Babylon::Graphics::DeviceT device) { if (device != nullptr) { diff --git a/Apps/UnitTests/Source/Utils.D3D12.cpp b/Apps/UnitTests/Source/Utils.D3D12.cpp index dc35ed454..2b9626847 100644 --- a/Apps/UnitTests/Source/Utils.D3D12.cpp +++ b/Apps/UnitTests/Source/Utils.D3D12.cpp @@ -42,7 +42,7 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) texture->Release(); } -Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() +Babylon::Graphics::DeviceT CreateTestGraphicsDevice() { IDXGIFactory4* factory = nullptr; EXPECT_HRESULT_SUCCEEDED(CreateDXGIFactory1(IID_PPV_ARGS(&factory))); @@ -59,7 +59,7 @@ Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest() return device; } -void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device) +void DestroyTestGraphicsDevice(Babylon::Graphics::DeviceT device) { if (device != nullptr) { diff --git a/Apps/UnitTests/Source/Utils.h b/Apps/UnitTests/Source/Utils.h index 0c389667b..60e7a76fd 100644 --- a/Apps/UnitTests/Source/Utils.h +++ b/Apps/UnitTests/Source/Utils.h @@ -6,7 +6,7 @@ Babylon::Graphics::TextureT CreateTestTexture(Babylon::Graphics::DeviceT device, void DestroyTestTexture(Babylon::Graphics::TextureT texture); // Returns a graphics device suitable for use as Babylon::Graphics::Configuration::Device. The -// returned handle is owned by the caller and must be released with DestroyGraphicsDeviceForTest. +// returned handle is owned by the caller and must be released with DestroyTestGraphicsDevice. // // Defined only on D3D11 and D3D12 -- the Tests.Device.cpp test that consumes these helpers is // gated to those backends. On D3D11 returns an ID3D11Device created via D3D11CreateDevice(WARP); @@ -15,6 +15,6 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture); // Note: on D3D12, D3D12CreateDevice(WARP) returns the same singleton pointer on successive calls // in the same process, so two calls in a row may return equal handles. D3D11CreateDevice(WARP) // does return distinct handles per call. -Babylon::Graphics::DeviceT CreateGraphicsDeviceForTest(); +Babylon::Graphics::DeviceT CreateTestGraphicsDevice(); -void DestroyGraphicsDeviceForTest(Babylon::Graphics::DeviceT device); +void DestroyTestGraphicsDevice(Babylon::Graphics::DeviceT device); From f34f29aca40b244efb3ea15f3b4a1261855d69f7 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Fri, 8 May 2026 08:54:19 -0700 Subject: [PATCH 09/10] Use winrt::com_ptr in CreateTestGraphicsDevice to remove null-deref hazards 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> --- Apps/UnitTests/Source/Utils.D3D12.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Apps/UnitTests/Source/Utils.D3D12.cpp b/Apps/UnitTests/Source/Utils.D3D12.cpp index 2b9626847..06197ad60 100644 --- a/Apps/UnitTests/Source/Utils.D3D12.cpp +++ b/Apps/UnitTests/Source/Utils.D3D12.cpp @@ -2,6 +2,7 @@ #include "Utils.h" #include +#include Babylon::Graphics::TextureT CreateTestTexture(Babylon::Graphics::DeviceT device, uint32_t width, uint32_t height, uint32_t arraySize) { @@ -44,17 +45,14 @@ void DestroyTestTexture(Babylon::Graphics::TextureT texture) Babylon::Graphics::DeviceT CreateTestGraphicsDevice() { - IDXGIFactory4* factory = nullptr; - EXPECT_HRESULT_SUCCEEDED(CreateDXGIFactory1(IID_PPV_ARGS(&factory))); + winrt::com_ptr factory; + winrt::check_hresult(CreateDXGIFactory1(IID_PPV_ARGS(factory.put()))); - IDXGIAdapter* warpAdapter = nullptr; - EXPECT_HRESULT_SUCCEEDED(factory->EnumWarpAdapter(IID_PPV_ARGS(&warpAdapter))); + winrt::com_ptr warpAdapter; + winrt::check_hresult(factory->EnumWarpAdapter(IID_PPV_ARGS(warpAdapter.put()))); ID3D12Device* device = nullptr; - EXPECT_HRESULT_SUCCEEDED(D3D12CreateDevice(warpAdapter, D3D_FEATURE_LEVEL_11_0, IID_PPV_ARGS(&device))); - - warpAdapter->Release(); - factory->Release(); + winrt::check_hresult(D3D12CreateDevice(warpAdapter.get(), D3D_FEATURE_LEVEL_11_0, IID_PPV_ARGS(&device))); return device; } From 583bd4022e44f80d14d202d6f8c1a7217d024405 Mon Sep 17 00:00:00 2001 From: Gary Hsu Date: Fri, 8 May 2026 16:30:16 -0700 Subject: [PATCH 10/10] Replace Tests.Device.cpp file header with a one-line summary 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> --- Apps/UnitTests/Source/Tests.Device.cpp | 33 +------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/Apps/UnitTests/Source/Tests.Device.cpp b/Apps/UnitTests/Source/Tests.Device.cpp index 6425bac53..ab24ef4af 100644 --- a/Apps/UnitTests/Source/Tests.Device.cpp +++ b/Apps/UnitTests/Source/Tests.Device.cpp @@ -6,38 +6,7 @@ extern Babylon::Graphics::Configuration g_deviceConfig; -// Exercises the canonical UpdateDevice flow: drive a frame, DisableRendering, UpdateDevice to a -// new graphics device, drive another frame, and verify the active device pointer reflects the swap. -// The motivating use cases are D3D11/D3D12 device-removed recovery (DXGI_ERROR_DEVICE_REMOVED on -// Present, GPU TDR, hot unplug) and host-driven device swaps in embedding scenarios (e.g. WPF/Win32 -// interop where the host's ID3D11Device changes). -// -// Idle requirement (not exercised here, but required in production): -// DisableRendering must be called between frames and with no outstanding JS / FrameCompletionScope -// work. This test runs purely on the render thread with no JS engine attached, so the requirement -// is trivially satisfied. -// -// Per-platform scope: -// D3D11 / D3D12 -> the test runs using a bgfx-managed swap chain bound to the App-layer HWND. -// Note: D3D12CreateDevice(WARP) returns the same singleton pointer on -// successive calls (deviceA and deviceB compare equal), so post-swap -// distinctness cannot be asserted on D3D12 + WARP. The test still exercises the -// full teardown / re-init path. D3D11 (D3D11CreateDevice(WARP)) does return -// distinct pointers per call. -// Metal / OpenGL -> the test is not built on these platforms (see Apps/UnitTests/CMakeLists.txt). -// Metal does not expose an API to create distinct devices for the same GPU -// (MTL::Device is a per-GPU singleton), and the OpenGL backend does not -// support a caller-provided GLXContext at init. The shipping code paths on -// those backends always use backend-owned contexts. -// -// The test does not exercise the caller-owned BackBufferColor path; it lets the swap chain be -// managed on the App layer's HWND so the test focuses purely on UpdateDevice. The caller-owned -// BackBuffer flow is covered by TEST(Device, BackBuffer) in Tests.Device.D3D11.cpp. -// -// Cleanup order: -// The Babylon::Graphics::Device destructor calls DisableRendering internally, which still expects -// the active graphics device to be valid. The Device is therefore scoped so it destructs before -// any caller-owned device handles are released. +// Verifies UpdateDevice replaces the active graphics device after a DisableRendering / EnableRendering cycle. TEST(Device, UpdateDevice) { Babylon::Graphics::DeviceT deviceA = CreateTestGraphicsDevice();