From 24252702f264fe0b05a25139537177bb7ae04fd0 Mon Sep 17 00:00:00 2001 From: Brad Grantham Date: Tue, 8 Oct 2024 11:23:42 -0700 Subject: [PATCH 1/7] Fix invalid usage by waiting on cb fence before resetting cb * Add fences to per-image copy resource * Save device in swapchain state * Give fence to QueueSubmit to signal * Wait on fence before resetting commandbuffer --- .../decode/vulkan_captured_swapchain.cpp | 1 + .../decode/vulkan_offscreen_swapchain.cpp | 4 +- framework/decode/vulkan_swapchain.h | 2 + framework/decode/vulkan_virtual_swapchain.cpp | 56 ++++++++++++++++++- framework/decode/vulkan_virtual_swapchain.h | 1 + 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/framework/decode/vulkan_captured_swapchain.cpp b/framework/decode/vulkan_captured_swapchain.cpp index c14d28d762..dad5001172 100644 --- a/framework/decode/vulkan_captured_swapchain.cpp +++ b/framework/decode/vulkan_captured_swapchain.cpp @@ -43,6 +43,7 @@ VkResult VulkanCapturedSwapchain::CreateSwapchainKHR(VkResult device = device_info->handle; } device_table_ = device_table; + device_ = device; auto replay_swapchain = swapchain->GetHandlePointer(); return func(device, create_info, allocator, replay_swapchain); } diff --git a/framework/decode/vulkan_offscreen_swapchain.cpp b/framework/decode/vulkan_offscreen_swapchain.cpp index ba351155a2..1b49bc7ee4 100644 --- a/framework/decode/vulkan_offscreen_swapchain.cpp +++ b/framework/decode/vulkan_offscreen_swapchain.cpp @@ -88,12 +88,14 @@ VkResult VulkanOffscreenSwapchain::CreateSwapchainKHR(VkResult const encode::VulkanDeviceTable* device_table) { GFXRECON_ASSERT(device_info); - device_table_ = device_table; + device_table_ = device_table; const format::HandleId* id = swapchain->GetPointer(); VkSwapchainKHR* replay_swapchain = swapchain->GetHandlePointer(); VkDevice device = device_info->handle; + device_ = device; + // Give swapchain a fake handle. It's handle id. *replay_swapchain = UINT64_TO_VK_HANDLE(VkSwapchainKHR, *id); if (!AddSwapchainResourceData(*replay_swapchain)) diff --git a/framework/decode/vulkan_swapchain.h b/framework/decode/vulkan_swapchain.h index 0b31cd06ba..954b0715c5 100644 --- a/framework/decode/vulkan_swapchain.h +++ b/framework/decode/vulkan_swapchain.h @@ -179,6 +179,8 @@ class VulkanSwapchain const encode::VulkanInstanceTable* instance_table_{ nullptr }; const encode::VulkanDeviceTable* device_table_{ nullptr }; + VkDevice device_{ VK_NULL_HANDLE }; + application::Application* application_{ nullptr }; ActiveWindows active_windows_; int32_t create_surface_count_{ 0 }; diff --git a/framework/decode/vulkan_virtual_swapchain.cpp b/framework/decode/vulkan_virtual_swapchain.cpp index 99ce11e0e5..2ddbf25f83 100644 --- a/framework/decode/vulkan_virtual_swapchain.cpp +++ b/framework/decode/vulkan_virtual_swapchain.cpp @@ -58,6 +58,7 @@ VkResult VulkanVirtualSwapchain::CreateSwapchainKHR(VkResult device = device_info->handle; } device_table_ = device_table; + device_ = device; VkPhysicalDevice physical_device = device_info->parent; VkSwapchainCreateInfoKHR modified_create_info = *create_info; @@ -134,6 +135,10 @@ void VulkanVirtualSwapchain::CleanSwapchainResourceData(const DeviceInfo* { device_table_->DestroySemaphore(device, semaphore, nullptr); } + for (auto& fence : copy_cmd_data.second.fences) + { + device_table_->DestroyFence(device, fence, nullptr); + } } swapchain_resources_.erase(swapchain); @@ -362,6 +367,31 @@ VkResult VulkanVirtualSwapchain::CreateSwapchainResourceData(const DeviceInfo* copy_cmd_data.semaphores[ii] = semaphore; } } + uint32_t fence_count = static_cast(copy_cmd_data.fences.size()); + if (fence_count < capture_image_count) + { + copy_cmd_data.fences.resize(capture_image_count); + + for (uint32_t ii = fence_count; ii < capture_image_count; ++ii) + { + VkFenceCreateInfo fence_create_info = { + VK_STRUCTURE_TYPE_FENCE_CREATE_INFO, // sType + nullptr, // pNext + VK_FENCE_CREATE_SIGNALED_BIT // flags + }; + + VkFence fence = VK_NULL_HANDLE; + result = device_table_->CreateFence(device, &fence_create_info, nullptr, &fence); + if (result != VK_SUCCESS) + { + GFXRECON_LOG_ERROR("Virtual swapchain failed creating internal copy fence for " + "swapchain (ID = %" PRIu64 ")", + swapchain_info->capture_id); + return result; + } + copy_cmd_data.fences[ii] = fence; + } + } } } } @@ -431,7 +461,18 @@ VkResult VulkanVirtualSwapchain::CreateSwapchainResourceData(const DeviceInfo* begin_info.pInheritanceInfo = nullptr; auto command_buffer = swapchain_resources->copy_cmd_data[copy_queue_family_index].command_buffers[0]; + auto copy_fence = swapchain_resources->copy_cmd_data[copy_queue_family_index].fences[0]; + result = device_table_->WaitForFences(device_, 1, ©_fence, VK_TRUE, ~0UL); + if (result != VK_SUCCESS) + { + return result; + } + result = device_table_->ResetFences(device_, 1, ©_fence); + if (result != VK_SUCCESS) + { + return result; + } result = device_table_->ResetCommandBuffer(command_buffer, 0); if (result != VK_SUCCESS) { @@ -500,7 +541,7 @@ VkResult VulkanVirtualSwapchain::CreateSwapchainResourceData(const DeviceInfo* submit_info.commandBufferCount = 1; submit_info.pCommandBuffers = &command_buffer; - result = device_table_->QueueSubmit(initial_copy_queue, 1, &submit_info, VK_NULL_HANDLE); + result = device_table_->QueueSubmit(initial_copy_queue, 1, &submit_info, copy_fence); if (result != VK_SUCCESS) { GFXRECON_LOG_ERROR( @@ -769,6 +810,7 @@ VkResult VulkanVirtualSwapchain::QueuePresentKHR(VkResult auto& copy_cmd_data = swapchain_resources->copy_cmd_data[queue_family_index]; auto command_buffer = copy_cmd_data.command_buffers[capture_image_index]; auto copy_semaphore = copy_cmd_data.semaphores[capture_image_index]; + auto copy_fence = copy_cmd_data.fences[capture_image_index]; std::vector wait_semaphores; std::vector signal_semaphores; @@ -789,6 +831,16 @@ VkResult VulkanVirtualSwapchain::QueuePresentKHR(VkResult present_wait_semaphores.emplace_back(copy_semaphore); } + result = device_table_->WaitForFences(device_, 1, ©_fence, VK_TRUE, ~0UL); + if (result != VK_SUCCESS) + { + return result; + } + result = device_table_->ResetFences(device_, 1, ©_fence); + if (result != VK_SUCCESS) + { + return result; + } result = device_table_->ResetCommandBuffer(command_buffer, 0); if (result != VK_SUCCESS) { @@ -892,7 +944,7 @@ VkResult VulkanVirtualSwapchain::QueuePresentKHR(VkResult submit_info.commandBufferCount = 1; submit_info.pCommandBuffers = &command_buffer; - result = device_table_->QueueSubmit(queue, 1, &submit_info, VK_NULL_HANDLE); + result = device_table_->QueueSubmit(queue, 1, &submit_info, copy_fence); if (result != VK_SUCCESS) { diff --git a/framework/decode/vulkan_virtual_swapchain.h b/framework/decode/vulkan_virtual_swapchain.h index 0b62529981..cdd0c7a495 100644 --- a/framework/decode/vulkan_virtual_swapchain.h +++ b/framework/decode/vulkan_virtual_swapchain.h @@ -133,6 +133,7 @@ class VulkanVirtualSwapchain : public VulkanSwapchain { VkCommandPool command_pool; std::vector command_buffers; + std::vector fences; std::vector semaphores; }; From dcd6a4d962420b78b71b50f90e8cf41845f08f9b Mon Sep 17 00:00:00 2001 From: Brad Grantham Date: Tue, 8 Oct 2024 11:42:40 -0700 Subject: [PATCH 2/7] fix clang-format --- framework/decode/vulkan_offscreen_swapchain.cpp | 4 ++-- framework/decode/vulkan_virtual_swapchain.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/framework/decode/vulkan_offscreen_swapchain.cpp b/framework/decode/vulkan_offscreen_swapchain.cpp index 1b49bc7ee4..4a8cd1910f 100644 --- a/framework/decode/vulkan_offscreen_swapchain.cpp +++ b/framework/decode/vulkan_offscreen_swapchain.cpp @@ -88,13 +88,13 @@ VkResult VulkanOffscreenSwapchain::CreateSwapchainKHR(VkResult const encode::VulkanDeviceTable* device_table) { GFXRECON_ASSERT(device_info); - device_table_ = device_table; + device_table_ = device_table; const format::HandleId* id = swapchain->GetPointer(); VkSwapchainKHR* replay_swapchain = swapchain->GetHandlePointer(); VkDevice device = device_info->handle; - device_ = device; + device_ = device; // Give swapchain a fake handle. It's handle id. *replay_swapchain = UINT64_TO_VK_HANDLE(VkSwapchainKHR, *id); diff --git a/framework/decode/vulkan_virtual_swapchain.cpp b/framework/decode/vulkan_virtual_swapchain.cpp index 2ddbf25f83..ecd564a04e 100644 --- a/framework/decode/vulkan_virtual_swapchain.cpp +++ b/framework/decode/vulkan_virtual_swapchain.cpp @@ -381,7 +381,7 @@ VkResult VulkanVirtualSwapchain::CreateSwapchainResourceData(const DeviceInfo* }; VkFence fence = VK_NULL_HANDLE; - result = device_table_->CreateFence(device, &fence_create_info, nullptr, &fence); + result = device_table_->CreateFence(device, &fence_create_info, nullptr, &fence); if (result != VK_SUCCESS) { GFXRECON_LOG_ERROR("Virtual swapchain failed creating internal copy fence for " From 28ed423a86db85824e397bedf38fbf6cfffe6ea8 Mon Sep 17 00:00:00 2001 From: Brad Grantham Date: Tue, 22 Oct 2024 16:29:05 -0700 Subject: [PATCH 3/7] use device from QueueInfo or as passed in --- framework/decode/vulkan_captured_swapchain.cpp | 1 - framework/decode/vulkan_object_info.h | 1 + framework/decode/vulkan_offscreen_swapchain.cpp | 2 -- framework/decode/vulkan_replay_consumer_base.cpp | 2 ++ framework/decode/vulkan_swapchain.h | 2 -- framework/decode/vulkan_virtual_swapchain.cpp | 10 +++++----- 6 files changed, 8 insertions(+), 10 deletions(-) diff --git a/framework/decode/vulkan_captured_swapchain.cpp b/framework/decode/vulkan_captured_swapchain.cpp index dad5001172..c14d28d762 100644 --- a/framework/decode/vulkan_captured_swapchain.cpp +++ b/framework/decode/vulkan_captured_swapchain.cpp @@ -43,7 +43,6 @@ VkResult VulkanCapturedSwapchain::CreateSwapchainKHR(VkResult device = device_info->handle; } device_table_ = device_table; - device_ = device; auto replay_swapchain = swapchain->GetHandlePointer(); return func(device, create_info, allocator, replay_swapchain); } diff --git a/framework/decode/vulkan_object_info.h b/framework/decode/vulkan_object_info.h index 217a256c37..a4fb8085ea 100644 --- a/framework/decode/vulkan_object_info.h +++ b/framework/decode/vulkan_object_info.h @@ -314,6 +314,7 @@ struct DeviceInfo : public VulkanObjectInfo struct QueueInfo : public VulkanObjectInfo { + VkDevice device; std::unordered_map array_counts; uint32_t family_index; uint32_t queue_index; diff --git a/framework/decode/vulkan_offscreen_swapchain.cpp b/framework/decode/vulkan_offscreen_swapchain.cpp index 4a8cd1910f..ba351155a2 100644 --- a/framework/decode/vulkan_offscreen_swapchain.cpp +++ b/framework/decode/vulkan_offscreen_swapchain.cpp @@ -94,8 +94,6 @@ VkResult VulkanOffscreenSwapchain::CreateSwapchainKHR(VkResult VkSwapchainKHR* replay_swapchain = swapchain->GetHandlePointer(); VkDevice device = device_info->handle; - device_ = device; - // Give swapchain a fake handle. It's handle id. *replay_swapchain = UINT64_TO_VK_HANDLE(VkSwapchainKHR, *id); if (!AddSwapchainResourceData(*replay_swapchain)) diff --git a/framework/decode/vulkan_replay_consumer_base.cpp b/framework/decode/vulkan_replay_consumer_base.cpp index 26e14b3152..644a261409 100644 --- a/framework/decode/vulkan_replay_consumer_base.cpp +++ b/framework/decode/vulkan_replay_consumer_base.cpp @@ -3015,6 +3015,7 @@ void VulkanReplayConsumerBase::OverrideGetDeviceQueue(PFN_vkGetDeviceQueue // This is necessary for the virtual swapchain to determine which command buffer to use when // Bliting the images on the Presenting Queue. auto queue_info = reinterpret_cast(pQueue->GetConsumerData(0)); + queue_info->device = device; queue_info->family_index = queueFamilyIndex; queue_info->queue_index = queueIndex; } @@ -3038,6 +3039,7 @@ void VulkanReplayConsumerBase::OverrideGetDeviceQueue2(PFN_vkGetDeviceQueue2 // This is necessary for the virtual swapchain to determine which command buffer to use when // Bliting the images on the Presenting Queue. auto queue_info = reinterpret_cast(pQueue->GetConsumerData(0)); + queue_info->device = device; queue_info->family_index = in_pQueueInfo->queueFamilyIndex; queue_info->queue_index = in_pQueueInfo->queueIndex; } diff --git a/framework/decode/vulkan_swapchain.h b/framework/decode/vulkan_swapchain.h index 954b0715c5..0b31cd06ba 100644 --- a/framework/decode/vulkan_swapchain.h +++ b/framework/decode/vulkan_swapchain.h @@ -179,8 +179,6 @@ class VulkanSwapchain const encode::VulkanInstanceTable* instance_table_{ nullptr }; const encode::VulkanDeviceTable* device_table_{ nullptr }; - VkDevice device_{ VK_NULL_HANDLE }; - application::Application* application_{ nullptr }; ActiveWindows active_windows_; int32_t create_surface_count_{ 0 }; diff --git a/framework/decode/vulkan_virtual_swapchain.cpp b/framework/decode/vulkan_virtual_swapchain.cpp index ecd564a04e..7a55cd5511 100644 --- a/framework/decode/vulkan_virtual_swapchain.cpp +++ b/framework/decode/vulkan_virtual_swapchain.cpp @@ -58,7 +58,6 @@ VkResult VulkanVirtualSwapchain::CreateSwapchainKHR(VkResult device = device_info->handle; } device_table_ = device_table; - device_ = device; VkPhysicalDevice physical_device = device_info->parent; VkSwapchainCreateInfoKHR modified_create_info = *create_info; @@ -463,12 +462,12 @@ VkResult VulkanVirtualSwapchain::CreateSwapchainResourceData(const DeviceInfo* auto command_buffer = swapchain_resources->copy_cmd_data[copy_queue_family_index].command_buffers[0]; auto copy_fence = swapchain_resources->copy_cmd_data[copy_queue_family_index].fences[0]; - result = device_table_->WaitForFences(device_, 1, ©_fence, VK_TRUE, ~0UL); + result = device_table_->WaitForFences(device, 1, ©_fence, VK_TRUE, ~0UL); if (result != VK_SUCCESS) { return result; } - result = device_table_->ResetFences(device_, 1, ©_fence); + result = device_table_->ResetFences(device, 1, ©_fence); if (result != VK_SUCCESS) { return result; @@ -714,6 +713,7 @@ VkResult VulkanVirtualSwapchain::QueuePresentKHR(VkResult return func(queue_info->handle, present_info); } + VkDevice device = queue_info->device; VkQueue queue = queue_info->handle; uint32_t queue_family_index = queue_info->family_index; @@ -831,12 +831,12 @@ VkResult VulkanVirtualSwapchain::QueuePresentKHR(VkResult present_wait_semaphores.emplace_back(copy_semaphore); } - result = device_table_->WaitForFences(device_, 1, ©_fence, VK_TRUE, ~0UL); + result = device_table_->WaitForFences(device, 1, ©_fence, VK_TRUE, ~0UL); if (result != VK_SUCCESS) { return result; } - result = device_table_->ResetFences(device_, 1, ©_fence); + result = device_table_->ResetFences(device, 1, ©_fence); if (result != VK_SUCCESS) { return result; From 97ede5a7aa6c7f5663efdabd4c994c2e91a5122c Mon Sep 17 00:00:00 2001 From: Brad Grantham Date: Tue, 22 Oct 2024 16:44:51 -0700 Subject: [PATCH 4/7] fix clang-format --- framework/decode/vulkan_replay_consumer_base.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework/decode/vulkan_replay_consumer_base.cpp b/framework/decode/vulkan_replay_consumer_base.cpp index 644a261409..6ff37b8257 100644 --- a/framework/decode/vulkan_replay_consumer_base.cpp +++ b/framework/decode/vulkan_replay_consumer_base.cpp @@ -3015,7 +3015,7 @@ void VulkanReplayConsumerBase::OverrideGetDeviceQueue(PFN_vkGetDeviceQueue // This is necessary for the virtual swapchain to determine which command buffer to use when // Bliting the images on the Presenting Queue. auto queue_info = reinterpret_cast(pQueue->GetConsumerData(0)); - queue_info->device = device; + queue_info->device = device; queue_info->family_index = queueFamilyIndex; queue_info->queue_index = queueIndex; } @@ -3039,7 +3039,7 @@ void VulkanReplayConsumerBase::OverrideGetDeviceQueue2(PFN_vkGetDeviceQueue2 // This is necessary for the virtual swapchain to determine which command buffer to use when // Bliting the images on the Presenting Queue. auto queue_info = reinterpret_cast(pQueue->GetConsumerData(0)); - queue_info->device = device; + queue_info->device = device; queue_info->family_index = in_pQueueInfo->queueFamilyIndex; queue_info->queue_index = in_pQueueInfo->queueIndex; } From 995d3fcb01634d4c6e5f6761499a9504cf2babd3 Mon Sep 17 00:00:00 2001 From: Brad Grantham Date: Tue, 22 Oct 2024 16:51:23 -0700 Subject: [PATCH 5/7] name member parent, not device, and initialize it --- framework/decode/vulkan_object_info.h | 2 +- framework/decode/vulkan_replay_consumer_base.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/framework/decode/vulkan_object_info.h b/framework/decode/vulkan_object_info.h index a4fb8085ea..b663c7920e 100644 --- a/framework/decode/vulkan_object_info.h +++ b/framework/decode/vulkan_object_info.h @@ -314,7 +314,7 @@ struct DeviceInfo : public VulkanObjectInfo struct QueueInfo : public VulkanObjectInfo { - VkDevice device; + VkDevice parent { VK_NULL_HANDLE }; std::unordered_map array_counts; uint32_t family_index; uint32_t queue_index; diff --git a/framework/decode/vulkan_replay_consumer_base.cpp b/framework/decode/vulkan_replay_consumer_base.cpp index 6ff37b8257..77a8b09ca8 100644 --- a/framework/decode/vulkan_replay_consumer_base.cpp +++ b/framework/decode/vulkan_replay_consumer_base.cpp @@ -3015,7 +3015,7 @@ void VulkanReplayConsumerBase::OverrideGetDeviceQueue(PFN_vkGetDeviceQueue // This is necessary for the virtual swapchain to determine which command buffer to use when // Bliting the images on the Presenting Queue. auto queue_info = reinterpret_cast(pQueue->GetConsumerData(0)); - queue_info->device = device; + queue_info->parent = device; queue_info->family_index = queueFamilyIndex; queue_info->queue_index = queueIndex; } @@ -3039,7 +3039,7 @@ void VulkanReplayConsumerBase::OverrideGetDeviceQueue2(PFN_vkGetDeviceQueue2 // This is necessary for the virtual swapchain to determine which command buffer to use when // Bliting the images on the Presenting Queue. auto queue_info = reinterpret_cast(pQueue->GetConsumerData(0)); - queue_info->device = device; + queue_info->parent = device; queue_info->family_index = in_pQueueInfo->queueFamilyIndex; queue_info->queue_index = in_pQueueInfo->queueIndex; } From 237476f713dc02e71da23e29a533dba68cbcc3d3 Mon Sep 17 00:00:00 2001 From: Brad Grantham Date: Tue, 22 Oct 2024 16:53:48 -0700 Subject: [PATCH 6/7] fix clang-format --- framework/decode/vulkan_object_info.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/decode/vulkan_object_info.h b/framework/decode/vulkan_object_info.h index b663c7920e..5306798be7 100644 --- a/framework/decode/vulkan_object_info.h +++ b/framework/decode/vulkan_object_info.h @@ -314,7 +314,7 @@ struct DeviceInfo : public VulkanObjectInfo struct QueueInfo : public VulkanObjectInfo { - VkDevice parent { VK_NULL_HANDLE }; + VkDevice parent{ VK_NULL_HANDLE }; std::unordered_map array_counts; uint32_t family_index; uint32_t queue_index; From 0b256cf180248ae3b4f569e3d57164134ff69cdf Mon Sep 17 00:00:00 2001 From: Brad Grantham Date: Tue, 22 Oct 2024 16:57:36 -0700 Subject: [PATCH 7/7] fix forgotten rename --- framework/decode/vulkan_virtual_swapchain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/decode/vulkan_virtual_swapchain.cpp b/framework/decode/vulkan_virtual_swapchain.cpp index 7a55cd5511..fcbafb9c68 100644 --- a/framework/decode/vulkan_virtual_swapchain.cpp +++ b/framework/decode/vulkan_virtual_swapchain.cpp @@ -713,7 +713,7 @@ VkResult VulkanVirtualSwapchain::QueuePresentKHR(VkResult return func(queue_info->handle, present_info); } - VkDevice device = queue_info->device; + VkDevice device = queue_info->parent; VkQueue queue = queue_info->handle; uint32_t queue_family_index = queue_info->family_index;