From 4d763758d4cceba3589e192d207da85312b7cfa2 Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 20 Jun 2018 10:37:23 +0100 Subject: [PATCH] Fix some multi-queue bugs * If the capture requires an external queue but never uses it (e.g. it has some resource that's on another queue and either incorrectly accesses it on the wrong queue or doesn't access it at all but still somehow has it referenced) we need an external queue - if we didn't find one in the capture by the time we get to ApplyInitialContents() we fetch it ourselves. * We also fix up device creation info if we've remapped the queues into duplicates by having the replay with fewer queue families than capture. --- renderdoc/driver/vulkan/vk_core.cpp | 18 +++++ .../vulkan/wrappers/vk_device_funcs.cpp | 74 +++++++++++++++---- 2 files changed, 76 insertions(+), 16 deletions(-) diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index cedaf267e..f6c892226 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -2018,6 +2018,24 @@ ReplayStatus WrappedVulkan::ContextReplayLog(CaptureState readType, uint32_t sta void WrappedVulkan::ApplyInitialContents() { + // check that we have all external queues necessary + for(size_t i = 0; i < m_ExternalQueues.size(); i++) + { + // if we created a pool (so this is a queue family we're using) but + // didn't get a queue at all, fetch our own queue for this family + if(m_ExternalQueues[i].queue != VK_NULL_HANDLE || m_ExternalQueues[i].pool == VK_NULL_HANDLE) + continue; + + VkQueue queue; + + ObjDisp(m_Device)->GetDeviceQueue(Unwrap(m_Device), (uint32_t)i, 0, &queue); + + GetResourceManager()->WrapResource(Unwrap(m_Device), queue); + GetResourceManager()->AddLiveResource(ResourceIDGen::GetNewUniqueID(), queue); + + m_ExternalQueues[i].queue = queue; + } + // add a global memory barrier to ensure all writes have finished and are synchronised // add memory barrier to ensure this copy completes before any subsequent work // this is a very blunt instrument but it ensures we don't get random artifacts around diff --git a/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp index fb8e54900..b377d2a3f 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp @@ -1269,11 +1269,13 @@ bool WrappedVulkan::Serialise_vkCreateDevice(SerialiserType &ser, VkPhysicalDevi } } + VkDeviceQueueCreateInfo *queueCreateInfos = + (VkDeviceQueueCreateInfo *)createInfo.pQueueCreateInfos; + // now apply the remapping to the requested queues for(uint32_t i = 0; i < createInfo.queueCreateInfoCount; i++) { - VkDeviceQueueCreateInfo &queueCreate = - (VkDeviceQueueCreateInfo &)createInfo.pQueueCreateInfos[i]; + VkDeviceQueueCreateInfo &queueCreate = (VkDeviceQueueCreateInfo &)queueCreateInfos[i]; uint32_t queueFamily = queueCreate.queueFamilyIndex; queueFamily = m_QueueRemapping[queueFamily][0].family; @@ -1287,6 +1289,48 @@ bool WrappedVulkan::Serialise_vkCreateDevice(SerialiserType &ser, VkPhysicalDevi queueCreate.queueCount = queueCount; } + // remove any duplicates that have been created + std::vector queueInfos; + + for(uint32_t i = 0; i < createInfo.queueCreateInfoCount; i++) + { + VkDeviceQueueCreateInfo &queue1 = (VkDeviceQueueCreateInfo &)queueCreateInfos[i]; + + // if we already have this one in the list, continue + bool already = false; + for(const VkDeviceQueueCreateInfo &queue2 : queueInfos) + { + if(queue1.queueFamilyIndex == queue2.queueFamilyIndex) + { + already = true; + break; + } + } + + if(already) + continue; + + // get the 'biggest' queue allocation from all duplicates. That way we ensure we have enough + // queues in the queue family to satisfy any remap. + VkDeviceQueueCreateInfo biggest = queue1; + + for(uint32_t j = i + 1; j < createInfo.queueCreateInfoCount; j++) + { + VkDeviceQueueCreateInfo &queue2 = (VkDeviceQueueCreateInfo &)queueCreateInfos[j]; + + if(biggest.queueFamilyIndex == queue2.queueFamilyIndex) + { + if(queue2.queueCount > biggest.queueCount) + biggest = queue2; + } + } + + queueInfos.push_back(biggest); + } + + createInfo.queueCreateInfoCount = (uint32_t)queueInfos.size(); + createInfo.pQueueCreateInfos = queueInfos.data(); + bool found = false; uint32_t qFamilyIdx = 0; @@ -1296,9 +1340,6 @@ bool WrappedVulkan::Serialise_vkCreateDevice(SerialiserType &ser, VkPhysicalDevi // for queue priorities, if we need it float one = 1.0f; - // if we need to add a new requested queues, it will point to this - VkDeviceQueueCreateInfo *modQueues = NULL; - for(uint32_t i = 0; i < createInfo.queueCreateInfoCount; i++) { uint32_t idx = createInfo.pQueueCreateInfos[i].queueFamilyIndex; @@ -1336,16 +1377,17 @@ bool WrappedVulkan::Serialise_vkCreateDevice(SerialiserType &ser, VkPhysicalDevi else { // we found the queue family, add it - modQueues = new VkDeviceQueueCreateInfo[createInfo.queueCreateInfoCount + 1]; - for(uint32_t i = 0; i < createInfo.queueCreateInfoCount; i++) - modQueues[i] = createInfo.pQueueCreateInfos[i]; + VkDeviceQueueCreateInfo newQueue; - modQueues[createInfo.queueCreateInfoCount].queueFamilyIndex = qFamilyIdx; - modQueues[createInfo.queueCreateInfoCount].queueCount = 1; - modQueues[createInfo.queueCreateInfoCount].pQueuePriorities = &one; + newQueue.queueFamilyIndex = qFamilyIdx; + newQueue.queueCount = 1; + newQueue.pQueuePriorities = &one; - createInfo.pQueueCreateInfos = modQueues; - createInfo.queueCreateInfoCount++; + queueInfos.push_back(newQueue); + + // reset these in case the vector resized + createInfo.queueCreateInfoCount = (uint32_t)queueInfos.size(); + createInfo.pQueueCreateInfos = queueInfos.data(); } } @@ -1544,8 +1586,9 @@ bool WrappedVulkan::Serialise_vkCreateDevice(SerialiserType &ser, VkPhysicalDevi GetResourceManager()->WrapResource(Unwrap(device), m_InternalCmds.cmdpool); } - // for each queue family that isn't our own, create a command pool and command buffer on that - // queue + // for each queue family we've remapped to, ensure we have a command pool and command buffer on + // that queue, and we'll also use the first queue that the application creates (or fetch our + // own). for(uint32_t i = 0; i < createInfo.queueCreateInfoCount; i++) { uint32_t qidx = createInfo.pQueueCreateInfos[i].queueFamilyIndex; @@ -1621,7 +1664,6 @@ bool WrappedVulkan::Serialise_vkCreateDevice(SerialiserType &ser, VkPhysicalDevi m_Replay.CreateResources(); - SAFE_DELETE_ARRAY(modQueues); SAFE_DELETE_ARRAY(layerArray); SAFE_DELETE_ARRAY(extArray); }