Implement retired swapchain management for OpenXR backends: add logic to handle swapchain lifecycle and ensure proper resource cleanup

OPENXR.md

@@ -140,18 +140,26 @@ the left eye's size now and, while they differ, the size the slider's value give
 
 A new scale never interrupts the picture. Each backend keeps two swapchain pairs, one on display and
 one Aurora writes next, and rebuilds only the second, at the start of the frame that writes it; the
-other follows a frame later, once it is the one written. The new
-swapchains are created before the old ones go, and Aurora first forgets the old images: the D3D12
-bridge waits for any copy still writing them, the Windows Vulkan bridge drains Dawn's queue and drops
-its wraps of the old `VkImage`s (the runtime may hand the same handles to the new swapchains), and
-the Quest also replaces its shared eye buffers, whose Dawn imports are released
-(`aurora_vulkan_forget_stereo_buffers`). Those buffers are kept as large as both pairs, growing with
-the first pair rebuilt larger and shrinking once the second has followed it down, and each copy moves
-only what fits the image it writes. If the runtime cannot allocate a size, the log says so, the eyes
-keep the size they had (a pair already rebuilt goes back to it), and that size is not tried again
-until the scale changes; the saved value is still what the next launch asks for.
-`mkw_openxr_replay_tests` and `mkw_openxr_vulkan_replay_tests` cover the rebuild, the display pair
-left alone, the images forgotten before their swapchains are destroyed, and both kinds of refusal.
+other follows a frame later, once it is the one written. The new swapchains are created before the
+old ones go, and a replaced pair is not destroyed at once: the compositor may still be consuming the
+layer that last showed it (the spec lets a runtime use the images after `xrDestroySwapchain`, and
+destroying a pair straight after its last frame lost the Vulkan device on a PC runtime), so it lives
+on for `kOpenXRRetiredSwapchainCycles` (8) more pacing cycles, each of which ends another compositor
+frame, and goes with the session if it is still waiting then. Before a pair is destroyed Aurora
+forgets its images: the D3D12 bridge waits for any copy still writing them, and the Windows Vulkan
+bridge drains Dawn's queue and drops its wraps of the old `VkImage`s (the runtime may hand the same
+handles to the new swapchains); on the Quest the backend's own copy device is idled. The Windows
+Vulkan binding also takes Dawn's device guard around `xrCreateSwapchain` and `xrDestroySwapchain`,
+since mid-session Dawn's worker is submitting on the shared queue. The Quest also replaces its
+shared eye buffers, whose Dawn imports are released (`aurora_vulkan_forget_stereo_buffers`). Those
+buffers are kept as large as both pairs, growing with the first pair rebuilt larger and shrinking
+once the second has followed it down, and each copy moves only what fits the image it writes. If the
+runtime cannot allocate a size, the log says so, the eyes keep the size they had (a pair already
+rebuilt goes back to it), and that size is not tried again until the scale changes; the saved value
+is still what the next launch asks for. `mkw_openxr_replay_tests` and
+`mkw_openxr_vulkan_replay_tests` cover the rebuild, the display pair left alone, the replaced pairs
+outliving their last frame by exactly that many cycles and going with the session otherwise, the
+images forgotten before their swapchains are destroyed, and both kinds of refusal.
 `world_units_per_meter` controls the scale of headset translation in the game world.
 `hud_distance_meters` and `hud_width_meters` place and size the virtual screen. They are read at
 launch and govern both the menu screen and the in-race 2D screen, so 2D content keeps its place
@@ -336,7 +344,11 @@ of you.
 
 How it is drawn: `settings_overlay.cpp` builds the panel with a second Dear ImGui context of its own,
 a 1440 × 1080 canvas at twice the desktop menu's scale with its own font atlas, fed by the pointer
-that `openxr_input.cpp` publishes through `vr/openxr_settings_panel.h`. Aurora renders that draw data
+that `openxr_input.cpp` publishes through `vr/openxr_settings_panel.h`. That context shares the
+desktop context's renderer backend data: ImGui's current context is one process-wide pointer, and
+Aurora's frame worker renders draw data while the game thread may have switched to the panel
+context, so the WebGPU backend has to find its device objects through either (without this, a
+worker render during the switch crashed on a null backend, seen while dragging the panel's sliders). Aurora renders that draw data
 into a panel texture once per sealed frame (`aurora-main/lib/stereo_overlay.cpp`). The ImGui backend
 keeps a single projection uniform, so the panel's pass is submitted on its own command buffer before
 the desktop's ImGui pass of the same frame is recorded.

runtime/include/vr/openxr_backend.h

@@ -65,6 +65,16 @@ struct OpenXRPanelLayer {
 inline constexpr uint32_t kOpenXRPanelLayerWidth = static_cast<uint32_t>(kSettingsPanelWidthPixels);
 inline constexpr uint32_t kOpenXRPanelLayerHeight = static_cast<uint32_t>(kSettingsPanelHeightPixels);
 
+// How many more pacing cycles an eye swapchain pair replaced by a new render
+// resolution lives on before it is destroyed. The pair was last shown by the
+// xrEndFrame of the cycle before its replacement, and the compositor keeps
+// reading that layer's images until a later one lands and its own GPU work on
+// them is done; the spec allows a runtime to use the images after
+// xrDestroySwapchain, but destroying a pair the compositor was still consuming
+// lost the Vulkan device on a PC runtime. Each cycle ends a compositor frame,
+// so this many later frames have replaced the pair's picture by then.
+inline constexpr uint32_t kOpenXRRetiredSwapchainCycles = 8;
+
 // The panel's layer, submitted after (so over) the scene's.
 inline XrCompositionLayerQuad OpenXRPanelQuadLayer(const OpenXRPanelLayer& panel, XrSpace space,
                                                    XrSwapchain swapchain) noexcept {

runtime/src/settings_overlay.cpp

@@ -2261,11 +2261,20 @@ VrSettingsPanel g_vrSettingsPanel;
 
 ImGuiContext* CreateVrSettingsPanelContext() {
     ImGuiContext* const desktop = ImGui::GetCurrentContext();
+    const ImGuiIO& desktopIo = ImGui::GetIO();
     ImGuiContext* const context = ImGui::CreateContext();
     ImGui::SetCurrentContext(context);
     ImGuiIO& io = ImGui::GetIO();
     io.IniFilename = nullptr;
     io.LogFilename = nullptr;
+    // ImGui's current context is one process-wide pointer, and Aurora's frame
+    // worker renders draw data (the desktop overlay, this panel) while this
+    // thread may have switched to this context to draw the panel. The WebGPU
+    // backend finds its device objects through the current context's IO, so
+    // this context carries the desktop's: without them, a render on the worker
+    // during the switch reads through a null backend.
+    io.BackendRendererUserData = desktopIo.BackendRendererUserData;
+    io.BackendRendererName = desktopIo.BackendRendererName;
     // No platform backend draws a cursor for it, and nothing else shows where
     // the controller is aiming.
     io.MouseDrawCursor = true;

runtime/src/vr/openxr_d3d12.cpp

@@ -851,6 +851,7 @@ private:
     // before the old ones go, so a size the runtime cannot allocate leaves the pair as it was.
     // False only on a failure that ends the session.
     bool ResizeWritablePair() {
+        ReapRetiredPairs(false);
         std::array<OpenXREyeSize, kOpenXREyeCount> current{};
         for (uint32_t eye = 0; eye < kOpenXREyeCount; ++eye) {
             if (eye_swapchains_[eye].acquired) {
@@ -874,17 +875,8 @@ private:
             eye_size_ = current;
             return true;
         }
-        std::vector<void*> retired;
-        for (const EyeSwapchain& swapchain : eye_swapchains_) {
-            for (const auto& image : swapchain.images) {
-                retired.push_back(image.texture);
-            }
-        }
-        if (!aurora_d3d12_forget_stereo_targets(retired.data(), static_cast<uint32_t>(retired.size()))) {
-            DestroySwapchainPair(replacement);
-            return Fail("Aurora could not retire its copies into the old D3D12 eye swapchains");
-        }
-        DestroySwapchainPair(eye_swapchains_);
+        // The compositor may still be reading the old pair (kOpenXRRetiredSwapchainCycles).
+        retired_pairs_.push_back({std::move(eye_swapchains_), kOpenXRRetiredSwapchainCycles});
         eye_swapchains_ = std::move(replacement);
         std::ostringstream message;
         message << "OpenXR D3D12 eyes resized: " << eye_size_[0].width << 'x' << eye_size_[0].height << " / "
@@ -893,6 +885,32 @@ private:
         return true;
     }
 
+    // Destroys the retired pairs whose time is up (every one of them at shutdown), once Aurora's
+    // copies into their images are known complete. A pair Aurora cannot vouch for is left to
+    // xrDestroySession rather than destroyed under a possibly live copy.
+    void ReapRetiredPairs(bool all) {
+        for (auto it = retired_pairs_.begin(); it != retired_pairs_.end();) {
+            if (!all && --it->cycles_left != 0) {
+                ++it;
+                continue;
+            }
+            std::vector<void*> images;
+            for (const EyeSwapchain& swapchain : it->swapchains) {
+                for (const auto& image : swapchain.images) {
+                    images.push_back(image.texture);
+                }
+            }
+            if (aurora_d3d12_forget_stereo_targets(images.data(), static_cast<uint32_t>(images.size()))) {
+                DestroySwapchainPair(it->swapchains);
+            } else {
+                Log(OpenXRLogLevel::Warning,
+                    "Aurora could not retire its copies into a replaced D3D12 eye swapchain pair; "
+                    "deferring its destruction to xrDestroySession");
+            }
+            it = retired_pairs_.erase(it);
+        }
+    }
+
     bool CreateSwapchain(EyeSwapchain& swapchain, uint32_t width, uint32_t height, const char* what) {
         swapchain.width = width;
         swapchain.height = height;
@@ -1058,6 +1076,7 @@ private:
 
     void DestroySwapchains() {
         DestroyPanelSwapchains();
+        ReapRetiredPairs(true);
         DestroySwapchainPair(eye_swapchains_);
         DestroySwapchainPair(retained_swapchains_);
         have_retained_frame_ = false;
@@ -1137,6 +1156,12 @@ private:
     // for, which differ while a size the runtime refused is being kept.
     std::array<OpenXREyeSize, kOpenXREyeCount> eye_size_{};
     std::array<OpenXREyeSize, kOpenXREyeCount> requested_eye_size_{};
+    // Pairs replaced by a new size, destroyed once the compositor has moved on from them.
+    struct RetiredPair {
+        std::array<EyeSwapchain, kOpenXREyeCount> swapchains;
+        uint32_t cycles_left;
+    };
+    std::vector<RetiredPair> retired_pairs_;
     // The settings panel's layer: written like the eyes into panel_swapchain_,
     // shown from retained_panel_swapchain_ (see FinishFrame).
     EyeSwapchain panel_swapchain_{};

runtime/src/vr/openxr_vulkan.cpp

@@ -1848,6 +1848,7 @@ private:
     // writes). Everything new is allocated before anything old goes, so a size that cannot be had
     // leaves the eyes as they were.
     bool ResizeWritablePair() {
+        ReapRetiredPairs(false);
         std::array<OpenXREyeSize, kOpenXREyeCount> current{};
         for (uint32_t eye = 0; eye < kOpenXREyeCount; ++eye) {
             if (eye_swapchains_[eye].acquired) {
@@ -1945,7 +1946,8 @@ private:
                 }
             }
         }
-        DestroySwapchainPair(eye_swapchains_);
+        // The compositor may still be reading the old pair (kOpenXRRetiredSwapchainCycles).
+        retired_pairs_.push_back({std::move(eye_swapchains_), kOpenXRRetiredSwapchainCycles});
         eye_swapchains_ = std::move(replacement);
         std::ostringstream message;
         message << "OpenXR Vulkan eyes resized: " << eye_size_[0].width << 'x' << eye_size_[0].height << " / "
@@ -1957,6 +1959,31 @@ private:
         return true;
     }
 
+    // As the D3D12 backend's. Only this device's copies write the images, so its idle is the
+    // completion the spec asks for before xrDestroySwapchain.
+    void ReapRetiredPairs(bool all) {
+        bool waited = false;
+        for (auto it = retired_pairs_.begin(); it != retired_pairs_.end();) {
+            if (!all && --it->cycles_left != 0) {
+                ++it;
+                continue;
+            }
+            if (!waited && vk_device_ != VK_NULL_HANDLE) {
+                std::lock_guard lock(vk_mutex_);
+                waited = vkDeviceWaitIdle(vk_device_) == VK_SUCCESS;
+                if (!waited) {
+                    Log(OpenXRLogLevel::Warning,
+                        "vkDeviceWaitIdle failed before destroying replaced Vulkan eye swapchains; "
+                        "deferring their destruction to xrDestroySession");
+                }
+            }
+            if (waited || vk_device_ == VK_NULL_HANDLE) {
+                DestroySwapchainPair(it->swapchains);
+            }
+            it = retired_pairs_.erase(it);
+        }
+    }
+
     bool CreateSwapchain(EyeSwapchain& swapchain, uint32_t width, uint32_t height, const char* what) {
         swapchain.width = width;
         swapchain.height = height;
@@ -2115,6 +2142,7 @@ private:
 
     void DestroySwapchains() {
         DestroyPanelSwapchains();
+        ReapRetiredPairs(true);
         DestroySwapchainPair(eye_swapchains_);
         DestroySwapchainPair(retained_swapchains_);
         have_retained_frame_ = false;
@@ -2177,6 +2205,11 @@ private:
     // As the D3D12 backend's.
     std::array<OpenXREyeSize, kOpenXREyeCount> eye_size_{};
     std::array<OpenXREyeSize, kOpenXREyeCount> requested_eye_size_{};
+    struct RetiredPair {
+        std::array<EyeSwapchain, kOpenXREyeCount> swapchains;
+        uint32_t cycles_left;
+    };
+    std::vector<RetiredPair> retired_pairs_;
     // The settings panel's layer: written like the eyes into panel_swapchain_,
     // shown from retained_panel_swapchain_ (see FinishFrame).
     EyeSwapchain panel_swapchain_{};

runtime/src/vr/openxr_vulkan_win32.cpp

@@ -876,6 +876,7 @@ private:
     // As the D3D12 backend's. Aurora's bridge wraps each swapchain VkImage for Dawn, and the
     // runtime may hand the old handles out again, so the wraps go with the swapchains.
     bool ResizeWritablePair() {
+        ReapRetiredPairs(false);
         std::array<OpenXREyeSize, kOpenXREyeCount> current{};
         for (uint32_t eye = 0; eye < kOpenXREyeCount; ++eye) {
             if (eye_swapchains_[eye].acquired) {
@@ -899,17 +900,8 @@ private:
             eye_size_ = current;
             return true;
         }
-        std::vector<void*> retired;
-        for (const EyeSwapchain& swapchain : eye_swapchains_) {
-            for (const auto& image : swapchain.images) {
-                retired.push_back(reinterpret_cast<void*>(image.image));
-            }
-        }
-        if (!aurora_vulkan_win32_forget_targets(retired.data(), static_cast<uint32_t>(retired.size()))) {
-            DestroySwapchainPair(replacement);
-            return Fail("Aurora could not retire its copies into the old Vulkan eye swapchains");
-        }
-        DestroySwapchainPair(eye_swapchains_);
+        // The compositor may still be reading the old pair (kOpenXRRetiredSwapchainCycles).
+        retired_pairs_.push_back({std::move(eye_swapchains_), kOpenXRRetiredSwapchainCycles});
         eye_swapchains_ = std::move(replacement);
         std::ostringstream message;
         message << "OpenXR Vulkan eyes resized: " << eye_size_[0].width << 'x' << eye_size_[0].height << " / "
@@ -918,6 +910,31 @@ private:
         return true;
     }
 
+    // As the D3D12 backend's: Aurora drains Dawn's queue and drops its wraps of the images (which
+    // it does under Dawn's device guard, so before the guard is taken to destroy them).
+    void ReapRetiredPairs(bool all) {
+        for (auto it = retired_pairs_.begin(); it != retired_pairs_.end();) {
+            if (!all && --it->cycles_left != 0) {
+                ++it;
+                continue;
+            }
+            std::vector<void*> images;
+            for (const EyeSwapchain& swapchain : it->swapchains) {
+                for (const auto& image : swapchain.images) {
+                    images.push_back(reinterpret_cast<void*>(image.image));
+                }
+            }
+            if (aurora_vulkan_win32_forget_targets(images.data(), static_cast<uint32_t>(images.size()))) {
+                DestroySwapchainPair(it->swapchains);
+            } else {
+                Log(OpenXRLogLevel::Warning,
+                    "Aurora could not retire its copies into a replaced Vulkan eye swapchain pair; "
+                    "deferring its destruction to xrDestroySession");
+            }
+            it = retired_pairs_.erase(it);
+        }
+    }
+
     bool CreateSwapchain(EyeSwapchain& swapchain, uint32_t width, uint32_t height, const char* what) {
         swapchain.width = width;
         swapchain.height = height;
@@ -936,7 +953,14 @@ private:
         create.faceCount = 1;
         create.arraySize = 1;
         create.mipCount = 1;
-        XrResult result = xrCreateSwapchain(runtime_->Session(), &create, &swapchain.handle);
+        XrResult result;
+        {
+            // The spec keeps the runtime off the VkQueue here, but mid-session (the panel's
+            // swapchains, a new render resolution) Dawn's worker is submitting on it, and a
+            // runtime that transitions its new images would race that; the guard costs nothing.
+            const auto queue_guard = runtime_->LockGraphicsQueue();
+            result = xrCreateSwapchain(runtime_->Session(), &create, &swapchain.handle);
+        }
         ObserveResult(result);
         if (XR_FAILED(result)) {
             std::ostringstream message;
@@ -1075,6 +1099,7 @@ private:
     void DestroyPanelSwapchains() {
         for (auto* swapchain : {&panel_swapchain_, &retained_panel_swapchain_}) {
             if (swapchain->handle != XR_NULL_HANDLE && !swapchain->acquired) {
+                const auto queue_guard = runtime_ ? runtime_->LockGraphicsQueue() : OpenXRRuntime::GraphicsQueueGuard{nullptr, nullptr};
                 xrDestroySwapchain(swapchain->handle);
             } else if (swapchain->acquired) {
                 Log(OpenXRLogLevel::Warning,
@@ -1088,6 +1113,7 @@ private:
 
     void DestroySwapchains() {
         DestroyPanelSwapchains();
+        ReapRetiredPairs(true);
         DestroySwapchainPair(eye_swapchains_);
         DestroySwapchainPair(retained_swapchains_);
         have_retained_frame_ = false;
@@ -1098,6 +1124,7 @@ private:
     void DestroySwapchainPair(std::array<EyeSwapchain, kOpenXREyeCount>& pair) {
         for (auto& swapchain : pair) {
             if (swapchain.handle != XR_NULL_HANDLE && !swapchain.acquired) {
+                const auto queue_guard = runtime_ ? runtime_->LockGraphicsQueue() : OpenXRRuntime::GraphicsQueueGuard{nullptr, nullptr};
                 xrDestroySwapchain(swapchain.handle);
             } else if (swapchain.acquired) {
                 Log(OpenXRLogLevel::Warning,
@@ -1173,6 +1200,11 @@ private:
     // As the D3D12 backend's.
     std::array<OpenXREyeSize, kOpenXREyeCount> eye_size_{};
     std::array<OpenXREyeSize, kOpenXREyeCount> requested_eye_size_{};
+    struct RetiredPair {
+        std::array<EyeSwapchain, kOpenXREyeCount> swapchains;
+        uint32_t cycles_left;
+    };
+    std::vector<RetiredPair> retired_pairs_;
     // The settings panel's layer: written like the eyes into panel_swapchain_,
     // shown from retained_panel_swapchain_ (see FinishFrame).
     EyeSwapchain panel_swapchain_{};

runtime/tests/openxr_d3d12_replay_tests.cpp

@@ -635,7 +635,6 @@ void TestRenderScale() {
         Require(backend.BeginFrame(presentation, frame) == OpenXRBeginStatus::Ready);
         Require(frame.render_width[0] == width && frame.render_height[1] == height && targets.size() == 2);
         for (const auto& target : targets) Require(target.width == width && target.height == height);
-        Require(live_swapchains == 4);
     };
     const auto finish = [&] {
         Complete();
@@ -643,35 +642,49 @@ void TestRenderScale() {
         Require(backend.FinishFrame(frame, true));
         Require(layer_count == 1 && displayed_content == frame.xr_frame.serial);
     };
+    // Enough cycles at one size for every replaced pair to be destroyed.
+    const auto settle = [&](uint32_t width, uint32_t height) {
+        for (uint32_t i = 0; i <= kOpenXRRetiredSwapchainCycles; ++i) {
+            begin(width, height);
+            finish();
+        }
+        Require(live_swapchains == 4);
+    };
 
     backend.SetRenderScale(1.0f); // The session's own scale rebuilds nothing.
     begin(100, 80);
     finish();
-    Require(forgotten.empty() && shown_width == 100);
+    Require(forgotten.empty() && live_swapchains == 4 && shown_width == 100);
 
     backend.SetRenderScale(1.5f);
     begin(150, 120); // The pair written next, while the one on display still shows the last frame.
-    Require(forgotten.size() == 2);
+    Require(live_swapchains == 6 && forgotten.empty()); // The old pair lives on for the compositor.
     Require(backend.RepeatFrame(frame) && layer_count == 1 && shown_width == 100);
     finish();
     Require(shown_width == 150);
     begin(150, 120); // Then the other pair, once it is the one written.
-    Require(forgotten.size() == 4);
+    Require(live_swapchains == 8 && forgotten.empty());
     finish();
-    begin(150, 120); // Both are the new size: nothing more is rebuilt.
+    // Each replaced pair is forgotten by Aurora, then destroyed, kOpenXRRetiredSwapchainCycles
+    // cycles after the one that replaced it: the first pair one cycle before the second.
+    for (uint32_t i = 1; i < kOpenXRRetiredSwapchainCycles; ++i) {
+        begin(150, 120); // Both are the new size: nothing more is rebuilt.
+        Require(live_swapchains == (i < kOpenXRRetiredSwapchainCycles - 1 ? 8u : 6u));
+        finish();
+    }
+    Require(forgotten.size() == 2);
+    begin(150, 120);
+    Require(live_swapchains == 4 && forgotten.size() == 4);
     finish();
-    Require(forgotten.size() == 4);
 
     backend.SetRenderScale(5.0f); // Clamped to the runtime's maximum.
     begin(400, 320);
     finish();
-    begin(400, 320);
-    finish();
+    settle(400, 320);
     backend.SetRenderScale(1.5f);
     begin(150, 120);
     finish();
-    begin(150, 120);
-    finish();
+    settle(150, 120);
     const size_t forgotten_before_refusal = forgotten.size();
 
     // A size the runtime cannot make keeps the eyes, and is not retried while it is still asked for.
@@ -683,13 +696,14 @@ void TestRenderScale() {
     backend.SetRenderScale(3.0f);
     begin(150, 120);
     finish();
-    Require(create_attempts == attempts + 2 && forgotten.size() == forgotten_before_refusal);
+    Require(create_attempts == attempts + 2 && live_swapchains == 4);
+    settle(150, 120);
+    Require(forgotten.size() == forgotten_before_refusal);
     creates_before_failure = -1;
     backend.SetRenderScale(0.5f); // A new size is tried again.
     begin(50, 40);
     finish();
-    begin(50, 40);
-    finish();
+    settle(50, 40);
 
     // Refused for the second pair only: the first follows it back to the size it kept.
     creates_before_failure = 2;
@@ -703,6 +717,7 @@ void TestRenderScale() {
     creates_before_failure = -1;
     begin(50, 40);
     finish();
+    settle(50, 40);
     const size_t forgotten_settled = forgotten.size();
     begin(50, 40);
     finish();
@@ -721,6 +736,8 @@ void TestRenderScale() {
     Require(backend.FinishFrame(frame, true));
     Require(layer_count == 1 && shown_width == 100);
     expect_render_first = false;
+    // A pair still retired at shutdown goes with the rest, forgotten first.
+    Require(live_swapchains == 6);
     Require(backend.Shutdown() && live_swapchains == 0);
     display_time = 0;
 }