An error in the way frames were submitted to XR resulted in intermittent
slowdown that fooled me for a while as being shader compilation stutter,
and had me down the wrong track of investigating that.
However this was actually the root cause. This should bring Baiji up to
performance parity with the current Dolphin release, as near as I can tell.
Resolves Issue #13.
Opening a game, closing it, and opening another crashed or device-lost on
the Vulkan backend with VR enabled. D3D11 and D3D12 both restart cleanly,
so this was Vulkan-specific rather than an OpenXR runtime limitation.
Root cause, proven with VK_EXT_device_fault plus
VK_EXT_device_address_binding_report: the fault is a write-invalid at a
page-aligned address that the binding tracer reports as not owned by the
second game device. The OpenXR compositor still references resources that
lived on the first game VkDevice; destroying that device frees the memory
and the compositor later writes into the freed page. The D3D backends are
immune because their shared compositor resources are COM ref-counted,
while vkDestroyDevice frees unconditionally.
Fix: persist and reuse the VulkanContext (VkInstance and VkDevice) across
VR games, so vkCreateDevice/vkDestroyDevice run once per process and those
pages are never unmapped. Per-game objects are still fully recreated,
including the surface, swapchain, object cache, command buffers, and the
OpenXR session and XrInstance. A device-lost context is never persisted. A
non-VR launch, or a runtime requiring a device extension the persisted
device lacks, discards it and initializes fresh.
Two problems that persisting exposed:
- InitBackendInfo() creates and destroys a temporary VkInstance on every
game boot and then calls UnloadVulkanLibrary(), which nulls every global
Vulkan function pointer. Reuse now re-resolves the instance and device
dispatch for the persisted handles.
- That same UnloadVulkanLibrary() unmaps vulkan-1.dll on runtimes that do
not otherwise pin it, invalidating the persisted handles. This is why
VDXR still failed after SteamVR worked. Added KeepVulkanLibraryLoaded()
so unloading is suppressed while a device is persisted.
Also removes the XR_KHR_vulkan_enable2 path. Creating a second device
against a retained XrInstance crashes inside the runtime in a way SEH
cannot catch. The backend now always uses legacy XR_KHR_vulkan_enable,
where Dolphin owns instance and device creation and the runtime binds at
xrCreateSession.
Supporting changes: ordered OpenXR session teardown (ShutdownSession and
DestroySession), a device-lost guard in CommandBufferManager that stops
emulation instead of resubmitting to a dead device, synchronous release of
Dolphin views and framebuffers before xrDestroySwapchain, and the two GPU
fault diagnostics. Note that vkGetDeviceFaultInfoEXT must be called with
vendorBinarySize zeroed on the second call, or the driver writes the
vendor blob to a null pointer.
Verified on SteamVR/PSVR2, VDXR/Quest 3, and Meta Link over repeated
open/close cycles.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Yellow squiggly lines begone!
Done automatically on .cpp files through `run-clang-tidy`, with manual corrections to the mistakes.
If an import is directly used, but is technically unnecessary since it's recursively imported by something else, it is *not* removed.
The tool doesn't touch .h files, so I did some of them by hand while fixing errors due to old recursive imports.
Not everything is removed, but the cleanup should be substantial enough.
Because this done on Linux, code that isn't used on it is mostly untouched.
(Hopefully no open PR is depending on these imports...)
SPDX standardizes how source code conveys its copyright and licensing
information. See https://spdx.github.io/spdx-spec/1-rationale/ . SPDX
tags are adopted in many large projects, including things like the Linux
kernel.
Since C++17, non-member std::size() is present in the standard library
which also operates on regular C arrays. Given that, we can just replace
usages of ArraySize with that where applicable.
In many cases, we can just change the actual C array ArraySize() was
called on into a std::array and just use its .size() member function
instead.
In some other cases, we can collapse the loops they were used in, into a
ranged-for loop, eliminating the need for en explicit bounds query.
Drivers can return VK_ERROR_OUT_OF_DATE_KHR from vkQueuePresentKHR, and
we should resize the image in this case, as well as when getting it back
from vkAcquireNextImageKHR.
Before this change, we simply fail if the device does not expose one
queue family that supports both graphics and present. Currently this is
fine, since devices tend to lay out their queues in this way. NV, for
instance, tends to have one queue family for all graphics operations and
one more for transfer only. However, it's not a hard requirement, and it
is cheap to use a separate queue, so we might as well.
Instead of resetting two command buffers, now we only have to call
vkResetCommandPool once at the start of a frame.
NV's recommends using one pool per frame/thread. May offer a very small
boost in performance on some systems.
Jeremy Bowers