Remove retired swapchain images immediately

framework/encode/custom_vulkan_encoder_commands.h

@@ -236,7 +236,17 @@ struct CustomEncoderPreCall<format::ApiCallId::ApiCall_vkCreateSwapchainKHR>
     template <typename... Args>
     static void Dispatch(VulkanCaptureManager* manager, Args... args)
     {
-        manager->PreProcess_vkCreateSwapchain(args...);
+        manager->PreProcess_vkCreateSwapchainKHR(args...);
+    }
+};
+
+template <>
+struct CustomEncoderPostCall<format::ApiCallId::ApiCall_vkCreateSwapchainKHR>
+{
+    template <typename... Args>
+    static void Dispatch(VulkanCaptureManager* manager, Args... args)
+    {
+        manager->PostProcess_vkCreateSwapchainKHR(args...);
     }
 };
 

framework/encode/vulkan_capture_manager.cpp

@@ -1869,23 +1869,70 @@ void VulkanCaptureManager::PreProcess_vkCreateWaylandSurfaceKHR(VkInstance
     }
 }
 
-void VulkanCaptureManager::PreProcess_vkCreateSwapchain(VkDevice                        device,
-                                                        const VkSwapchainCreateInfoKHR* pCreateInfo,
-                                                        const VkAllocationCallbacks*    pAllocator,
-                                                        VkSwapchainKHR*                 pSwapchain)
+void VulkanCaptureManager::PreProcess_vkCreateSwapchainKHR(VkDevice                        device,
+                                                           const VkSwapchainCreateInfoKHR* pCreateInfo,
+                                                           const VkAllocationCallbacks*    pAllocator,
+                                                           VkSwapchainKHR*                 pSwapchain)
 {
     GFXRECON_UNREFERENCED_PARAMETER(device);
     GFXRECON_UNREFERENCED_PARAMETER(pAllocator);
     GFXRECON_UNREFERENCED_PARAMETER(pSwapchain);
 
-    assert(pCreateInfo != nullptr);
+    GFXRECON_ASSERT(pCreateInfo != nullptr);
+
+    WriteResizeWindowCmd2(vulkan_wrappers::GetWrappedId<vulkan_wrappers::SurfaceKHRWrapper>(pCreateInfo->surface),
+                          pCreateInfo->imageExtent.width,
+                          pCreateInfo->imageExtent.height,
+                          pCreateInfo->preTransform);
+}
+
+void VulkanCaptureManager::PostProcess_vkCreateSwapchainKHR(VkResult                        result,
+                                                            VkDevice                        device,
+                                                            const VkSwapchainCreateInfoKHR* pCreateInfo,
+                                                            const VkAllocationCallbacks*    pAllocator,
+                                                            VkSwapchainKHR*                 pSwapchain)
+{
+    GFXRECON_UNREFERENCED_PARAMETER(result);
+    GFXRECON_UNREFERENCED_PARAMETER(device);
+    GFXRECON_UNREFERENCED_PARAMETER(pAllocator);
+    GFXRECON_UNREFERENCED_PARAMETER(pSwapchain);
+
+    GFXRECON_ASSERT(pCreateInfo != nullptr);
+
+    // Vulkan Spec: Upon calling vkCreateSwapchainKHR with an oldSwapchain that is not VK_NULL_HANDLE, any images
+    // from oldSwapchain that are not acquired by the application may be freed by the implementation, which may
+    // occur even if creation of the new swapchain fails.
 
-    if (pCreateInfo)
+    // The capture layer needs to be conservative and treat these images as destroyed now because the implementation
+    // is free to destroy and reuse the image handles before the retired swapchain is destroyed.
+    if (pCreateInfo->oldSwapchain != VK_NULL_HANDLE)
     {
-        WriteResizeWindowCmd2(vulkan_wrappers::GetWrappedId<vulkan_wrappers::SurfaceKHRWrapper>(pCreateInfo->surface),
-                              pCreateInfo->imageExtent.width,
-                              pCreateInfo->imageExtent.height,
-                              pCreateInfo->preTransform);
+        auto old_wrapper = vulkan_wrappers::GetWrapper<vulkan_wrappers::SwapchainKHRWrapper>(pCreateInfo->oldSwapchain);
+        old_wrapper->retired = true;
+
+        for (int i = old_wrapper->child_images.size() - 1; i >= 0; --i)
+        {
+            bool is_acquired = false;
+            if (i < old_wrapper->image_acquired_info.size())
+                is_acquired = old_wrapper->image_acquired_info[i].is_acquired;
+
+            if (!is_acquired)
+            {
+                const auto image_handle = old_wrapper->child_images[i]->handle;
+
+                // Remove from swapchain info struct
+                old_wrapper->child_images.erase(old_wrapper->child_images.begin() + i);
+                if (i < old_wrapper->image_acquired_info.size())
+                    old_wrapper->image_acquired_info.erase(old_wrapper->image_acquired_info.begin() + i);
+
+                // Destroy handle wrapper
+                if (IsCaptureModeTrack())
+                {
+                    state_tracker_->RemoveEntry<vulkan_wrappers::ImageWrapper>(image_handle);
+                }
+                vulkan_wrappers::DestroyWrappedHandle<vulkan_wrappers::ImageWrapper>(image_handle);
+            }
+        }
     }
 }
 

framework/encode/vulkan_capture_manager.h

@@ -479,10 +479,16 @@ class VulkanCaptureManager : public ApiCaptureManager
                                               const VkAllocationCallbacks*         pAllocator,
                                               VkSurfaceKHR*                        pSurface);
 
-    void PreProcess_vkCreateSwapchain(VkDevice                        device,
-                                      const VkSwapchainCreateInfoKHR* pCreateInfo,
-                                      const VkAllocationCallbacks*    pAllocator,
-                                      VkSwapchainKHR*                 pSwapchain);
+    void PreProcess_vkCreateSwapchainKHR(VkDevice                        device,
+                                         const VkSwapchainCreateInfoKHR* pCreateInfo,
+                                         const VkAllocationCallbacks*    pAllocator,
+                                         VkSwapchainKHR*                 pSwapchain);
+
+    void PostProcess_vkCreateSwapchainKHR(VkResult                        result,
+                                          VkDevice                        device,
+                                          const VkSwapchainCreateInfoKHR* pCreateInfo,
+                                          const VkAllocationCallbacks*    pAllocator,
+                                          VkSwapchainKHR*                 pSwapchain);
 
     void PostProcess_vkAcquireNextImageKHR(VkResult result,
                                            VkDevice,

framework/encode/vulkan_handle_wrapper_util.h

@@ -200,9 +200,12 @@ void CreateWrappedDispatchHandle(typename ParentWrapper::HandleType parent,
         }
         if (!state_handle_table_.InsertWrapper(wrapper))
         {
-            GFXRECON_LOG_WARNING("Create a duplicated Handle: %" PRIu64
-                                 ". This wrapper can't be written into VulkanStateHandleTable.",
-                                 *handle);
+            auto existing_wrapper = state_handle_table_.GetWrapper<Wrapper>(wrapper->handle);
+            GFXRECON_LOG_WARNING("Cannot add duplicate entry to VulkanStateHandleTable for handle 0x%" PRIx64
+                                 " with ID %" PRIu64 ". This handle is already wrapped with ID %" PRIu64 ".",
+                                 wrapper->handle,
+                                 wrapper->handle_id,
+                                 existing_wrapper->handle_id);
         }
     }
 }
@@ -219,9 +222,12 @@ void CreateWrappedNonDispatchHandle(typename Wrapper::HandleType* handle, PFN_Ge
         wrapper->handle_id = get_id();
         if (!state_handle_table_.InsertWrapper(wrapper))
         {
-            GFXRECON_LOG_WARNING("Create a duplicated Handle: %" PRIu64
-                                 ". This wrapper can't be written into VulkanStateHandleTable.",
-                                 *handle);
+            auto existing_wrapper = state_handle_table_.GetWrapper<Wrapper>(wrapper->handle);
+            GFXRECON_LOG_WARNING("Cannot add duplicate entry to VulkanStateHandleTable for handle 0x%" PRIx64
+                                 " with ID %" PRIu64 ". This handle is already wrapped with ID %" PRIu64 ".",
+                                 wrapper->handle,
+                                 wrapper->handle_id,
+                                 existing_wrapper->handle_id);
         }
     }
 }
@@ -441,7 +447,9 @@ inline void CreateWrappedHandle<DeviceWrapper,
 
     auto parent_wrapper = GetWrapper<SwapchainKHRWrapper>(co_parent);
 
-    // Filter duplicate display retrieval.
+    GFXRECON_ASSERT(!parent_wrapper->retired);
+
+    // Filter duplicate image retrieval.
     ImageWrapper* wrapper = nullptr;
     for (auto entry : parent_wrapper->child_images)
     {
@@ -457,7 +465,6 @@ inline void CreateWrappedHandle<DeviceWrapper,
         CreateWrappedNonDispatchHandle<ImageWrapper>(handle, get_id);
         wrapper                     = GetWrapper<ImageWrapper>(*handle);
         wrapper->is_swapchain_image = true;
-        wrapper->parent_swapchains.insert(co_parent);
         parent_wrapper->child_images.push_back(wrapper);
     }
 }
@@ -649,12 +656,8 @@ inline void DestroyWrappedHandle<SwapchainKHRWrapper>(VkSwapchainKHR handle)
 
         for (auto image_wrapper : wrapper->child_images)
         {
-            image_wrapper->parent_swapchains.erase(handle);
-            if (image_wrapper->parent_swapchains.empty())
-            {
-                RemoveWrapper<ImageWrapper>(image_wrapper);
-                delete image_wrapper;
-            }
+            RemoveWrapper<ImageWrapper>(image_wrapper);
+            delete image_wrapper;
         }
 
         RemoveWrapper<SwapchainKHRWrapper>(wrapper);

framework/encode/vulkan_handle_wrappers.h

@@ -227,7 +227,6 @@ struct ImageWrapper : public HandleWrapper<VkImage>, AssetWrapperBase
     VkImageTiling            tiling{};
     VkImageLayout            current_layout{ VK_IMAGE_LAYOUT_UNDEFINED };
     bool                     is_swapchain_image{ false };
-    std::set<VkSwapchainKHR> parent_swapchains;
 
     std::set<ImageViewWrapper*> image_views;
 };
@@ -530,6 +529,7 @@ struct SwapchainKHRWrapper : public HandleWrapper<VkSwapchainKHR>
 {
     // Members for general wrapper support.
     std::vector<ImageWrapper*> child_images;
+    bool                       retired{ false };
 
     // Members for trimming state tracking.
     DeviceWrapper*                                    device{ nullptr };

framework/encode/vulkan_state_writer.cpp

@@ -1285,7 +1285,12 @@ void VulkanStateWriter::WriteSwapchainKhrState(const VulkanStateTable& state_tab
             encoder_.EncodeHandleIdValue(device_wrapper->handle_id);
             encoder_.EncodeHandleIdValue(wrapper->handle_id);
             encoder_.EncodeUInt32Ptr(&image_count, false);
-            encoder_.EncodeHandleIdArray(nullptr, 0, false);
+            auto handle_array = std::vector<format::HandleId>(wrapper->child_images.size());
+            for (int i = 0; i < wrapper->child_images.size(); ++i)
+            {
+                handle_array[i] = wrapper->child_images[i]->handle_id;
+            }
+            encoder_.EncodeHandleIdArray(handle_array.data(), handle_array.size(), false);
             encoder_.EncodeEnumValue(result);
 
             WriteFunctionCall(format::ApiCallId::ApiCall_vkGetSwapchainImagesKHR, &parameter_stream_);

framework/encode/vulkan_state_writer.h

@@ -326,6 +326,16 @@ class VulkanStateWriter
     {
         std::set<util::MemoryOutputStream*> processed;
         state_table.VisitWrappers([&](const Wrapper* wrapper) {
+            // Skip create call for swapchain images, i.e. vkGetSwapchainImagesKHR
+            // This call is already emitted by the state setup for the parent swapchain
+            if constexpr (std::is_same<Wrapper, vulkan_wrappers::ImageWrapper>::value)
+            {
+                auto image_wrapper = reinterpret_cast<const vulkan_wrappers::ImageWrapper*>(wrapper);
+                if (image_wrapper->is_swapchain_image)
+                {
+                    return;
+                }
+            }
             // Filter duplicate entries for calls that create multiple objects, where objects created by the same call
             // all reference the same parameter buffer.
             if (processed.find(wrapper->create_parameters.get()) == processed.end())