From 28d610cb72d40f5bb22b7b305644f8bc0df343da Mon Sep 17 00:00:00 2001 From: Daniel Lynch Date: Tue, 22 Sep 2026 22:44:34 -0400 Subject: [PATCH] fix(vk): spec-valid queue detection; null all lazy PFNs on teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Queue detection probed vkGetDeviceQueue over families 0-3 x indices 0-3, which is invalid for any queue the app did not create (VUID-00384/00385). ovrp_Initialize5 passes only the VkQueue, and physical-device queue counts bound what could exist, not what UE created. UE passes FVulkanDevice::GetGraphicsQueue(), which it fetches as index 0 of the first family with VK_QUEUE_GRAPHICS_BIT — so derive that (family,0) from vkGetPhysicalDeviceQueueFamilyProperties and confirm it with one valid vkGetDeviceQueue call. xrr_vk_teardown now nulls every device/instance-level PFN. The dump, barcode and copy-ring loaders only fetch a pointer when it is NULL, so a Shutdown2 -> Initialize5 with a new VkDevice kept calling through the old device's dispatch. Co-Authored-By: Claude Opus 5.5 (1M context) --- shim/src/core.c | 5 +-- shim/src/vk_session.c | 80 +++++++++++++++++++++++++++++++++---------- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/shim/src/core.c b/shim/src/core.c index 10a5353..0fd7e94 100644 --- a/shim/src/core.c +++ b/shim/src/core.c @@ -34,8 +34,9 @@ OVRP_EXPORT ovrpResult ovrp_Initialize5( apiType, activity, vkInstance, vkPhysicalDevice, vkDevice, queue, flags); xrr_set_android_activity(activity); xrr_vk_set_handles(vkDevice, queue, 0); /* for the end-of-frame flush barrier */ - /* OpenXR wants queueFamilyIndex+queueIndex; OVRPlugin gives a VkQueue we can't - * decompose -> default family 0/index 0 (UE Vulkan on Quest uses graphics fam 0). */ + /* OpenXR wants queueFamilyIndex+queueIndex; OVRPlugin gives only a VkQueue -> + * default family 0/index 0 here; vk_session.c derives UE's real graphics-queue + * (family,index) from the physical device and uses that when it can. */ ovrpResult r = xrr_init(vkInstance, vkPhysicalDevice, vkDevice, 0); XRRLOG("ovrp_Initialize5 -> %d", r); return r; diff --git a/shim/src/vk_session.c b/shim/src/vk_session.c index 29e4f89..3587e15 100644 --- a/shim/src/vk_session.c +++ b/shim/src/vk_session.c @@ -54,30 +54,57 @@ static int vk_loaders(PFN_vkGetDeviceProcAddr *gdpa, PFN_vkGetInstanceProcAddr * } /* The OpenXR runtime synchronizes its compositor against the queue named in the - * graphics binding. We only have UE's VkQueue handle, so scan (family,index) to - * find which one it is — a wrong queueIndex means the runtime waits on an idle - * queue and composites before UE finishes -> black unless we hard-wait ourselves. */ + * graphics binding. We only have UE's VkQueue handle (ovrp_Initialize5 passes no + * family/index), so work out which (family,index) it is — a wrong queueIndex means + * the runtime waits on an idle queue and composites before UE finishes -> black + * unless we hard-wait ourselves. + * + * We must NOT probe vkGetDeviceQueue over arbitrary (family,index) pairs: asking for + * a queue the app never created is invalid (VUID-vkGetDeviceQueue-queueFamilyIndex- + * 00384 / -queueIndex-00385), and the physical-device queue counts only bound what + * COULD exist, not what UE put in VkDeviceCreateInfo. Instead derive it the way UE + * does: the handle is FVulkanDevice::GetGraphicsQueue() (CustomPresent_Vulkan + * GetOvrpCommandQueue), which UE 4.25 fetches as index 0 of the FIRST family + * advertising VK_QUEUE_GRAPHICS_BIT. That (family,0) is guaranteed to have been + * created, so a single vkGetDeviceQueue on it is valid and confirms the match. */ static PFN_vkGetDeviceQueue load_get_device_queue(VkDevice dev) { PFN_vkGetDeviceProcAddr gdpa; vk_loaders(&gdpa, NULL); return gdpa ? (PFN_vkGetDeviceQueue)gdpa(dev, "vkGetDeviceQueue") : NULL; } /* On failure the caller's (family,index) are left untouched — the game-provided - * queueFamilyIndex is a better default than a hardcoded 0. */ -static void detect_ue_queue(VkDevice dev, uint32_t *family, uint32_t *index) { + * queueFamilyIndex is a better default than a guess. */ +static void detect_ue_queue(VkInstance inst, VkPhysicalDevice phys, VkDevice dev, + uint32_t *family, uint32_t *index) { if (!s_queue) { XRRLOG("queue detect: no UE queue handle yet, keeping fam=%u idx=%u", *family, *index); return; } + PFN_vkGetInstanceProcAddr gipa; vk_loaders(NULL, &gipa); + PFN_vkGetPhysicalDeviceQueueFamilyProperties qfp = (gipa && inst) + ? (PFN_vkGetPhysicalDeviceQueueFamilyProperties)gipa(inst, "vkGetPhysicalDeviceQueueFamilyProperties") + : NULL; PFN_vkGetDeviceQueue gdq = load_get_device_queue(dev); - if (!gdq) { XRRLOG("queue detect: no vkGetDeviceQueue, keeping fam=%u idx=%u", *family, *index); return; } - for (uint32_t f = 0; f < 4; f++) - for (uint32_t i = 0; i < 4; i++) { - VkQueue q = VK_NULL_HANDLE; - gdq(dev, f, i, &q); - if (q == s_queue) { - *family = f; *index = i; - XRRLOG("queue detect: UE queue is family=%u index=%u", f, i); - return; - } - } - XRRLOG("queue detect: UE queue not matched, keeping fam=%u idx=%u", *family, *index); + if (!qfp || !phys || !gdq) { + XRRLOG("queue detect: no QueueFamilyProperties/GetDeviceQueue, keeping fam=%u idx=%u", *family, *index); + return; + } + VkQueueFamilyProperties props[16]; + uint32_t n = 0; + qfp(phys, &n, NULL); + if (n > 16) n = 16; + qfp(phys, &n, props); + uint32_t gfx = UINT32_MAX; + for (uint32_t f = 0; f < n; f++) + if (props[f].queueCount > 0 && (props[f].queueFlags & VK_QUEUE_GRAPHICS_BIT)) { gfx = f; break; } + if (gfx == UINT32_MAX) { + XRRLOG("queue detect: no graphics family among %u, keeping fam=%u idx=%u", n, *family, *index); + return; + } + VkQueue q = VK_NULL_HANDLE; + gdq(dev, gfx, 0, &q); + *family = gfx; *index = 0; + if (q == s_queue) + XRRLOG("queue detect: UE queue is family=%u index=0 (first graphics family)", gfx); + else /* not UE's documented graphics queue — still the best spec-valid binding */ + XRRLOG("queue detect: WARN UE queue %p != graphics (fam=%u,idx=0) %p; binding fam=%u idx=0 anyway", + (void *)s_queue, gfx, (void *)q, gfx); } /* ----------------------------------------------- app-side extension queries -- */ @@ -182,7 +209,7 @@ int xrr_create_session_vulkan(void *vkInstance, void *vkPhysicalDevice, s_phys = (VkPhysicalDevice)vkPhysicalDevice; s_inst = (VkInstance)vkInstance; uint32_t qfam = queueFamilyIndex, qidx = 0; - detect_ue_queue((VkDevice)vkDevice, &qfam, &qidx); + detect_ue_queue(s_inst, s_phys, (VkDevice)vkDevice, &qfam, &qidx); s_qfam = qfam; /* the flush command pool must use the same family */ XrGraphicsBindingVulkanKHR binding = { XR_TYPE_GRAPHICS_BINDING_VULKAN_KHR }; @@ -956,9 +983,24 @@ uint32_t xrr_vk_enumerate_images(XrSwapchain sc, uint64_t *out, uint32_t max) { /* Reset shim Vulkan state so a post-Shutdown2 re-init (new VkDevice from a fresh ovrp_Initialize5) * rebuilds the command pool/fences/staging buffers instead of reusing handles from the dead device. * Called from xrr_shutdown under g_xrlock. Objects from the old device are left to the driver/process - * to reclaim — destroying them needs PFNs we don't load, and Shutdown2 is normally process-exit. */ + * to reclaim — destroying them needs PFNs we don't load, and Shutdown2 is normally process-exit. + * Every device/instance-level PFN is nulled too: the dump/barcode/copyring loaders only fetch a + * pointer when it is NULL, so without this they'd keep calling through the dead device's + * dispatch. (s_gdpa/s_gipa are libvulkan loader entry points, device-independent — kept.) */ void xrr_vk_teardown(void) { s_vkReady = 0; s_vkFailed = 0; + /* flush ring (vk_lazy_init) */ + p_CreatePool = NULL; p_AllocCmd = NULL; p_BeginCmd = NULL; p_Barrier = NULL; + p_EndCmd = NULL; p_Submit = NULL; p_ResetCmd = NULL; p_CreateFence = NULL; + p_WaitFences = NULL; p_ResetFences = NULL; p_FenceStatus = NULL; + p_QueueWaitIdle = NULL; p_DeviceWaitIdle = NULL; + /* dump / luma / barcode (dump_lazy, barcode_lazy) + instance-level MemProps */ + p_MemProps = NULL; p_CreateBuf = NULL; p_BufReq = NULL; p_AllocMem = NULL; + p_BindBuf = NULL; p_MapMem = NULL; p_Copy2Buf = NULL; p_DestroyBuf = NULL; + p_FreeMem = NULL; p_Copy2Img = NULL; + /* copy-ring (copyring_lazy) */ + p_CreateImage = NULL; p_ImgReq = NULL; p_BindImg = NULL; p_DestroyImage = NULL; + p_CopyImg = NULL; s_pool = VK_NULL_HANDLE; for (int i = 0; i < XRR_FLUSH_RING; i++) { s_cmd[i] = VK_NULL_HANDLE; s_fence[i] = VK_NULL_HANDLE; } s_bcBuf = VK_NULL_HANDLE; s_bcMem = VK_NULL_HANDLE; s_bcPx = NULL;