From c49d72e7d3be5e72189d64d4a5a1f8298f8273fe Mon Sep 17 00:00:00 2001
From: Powei Feng <powei@google.com>
Date: Thu, 16 Jan 2025 22:39:05 -0800
Subject: [PATCH] vk: properly destroy swapchain on recreate (#8365)

Fixes #8185, #6445
---
 .../platform/VulkanPlatformSwapChainImpl.cpp  | 27 ++++++++++++++++---
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp
index f8b900c6b17..cb80e489850 100644
--- a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp
+++ b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp
@@ -117,7 +117,7 @@ void VulkanPlatformSwapChainImpl::destroy() {
     mSwapChainBundle.colors.clear();
 }
 
-VkImage VulkanPlatformSwapChainImpl::createImage(VkExtent2D extent, VkFormat format, 
+VkImage VulkanPlatformSwapChainImpl::createImage(VkExtent2D extent, VkFormat format,
         bool isProtected) {
     auto [image, memory] = createImageAndMemory(mContext, mDevice, extent, format, isProtected);
     mMemory.insert({image, memory});
@@ -140,9 +140,8 @@ VulkanPlatformSurfaceSwapChain::VulkanPlatformSurfaceSwapChain(VulkanContext con
 }
 
 VulkanPlatformSurfaceSwapChain::~VulkanPlatformSurfaceSwapChain() {
-    vkDestroySwapchainKHR(mDevice, mSwapchain, VKALLOC);
-    vkDestroySurfaceKHR(mInstance, mSurface, VKALLOC);
     destroy();
+    vkDestroySurfaceKHR(mInstance, mSurface, VKALLOC);
 }
 
 VkResult VulkanPlatformSurfaceSwapChain::create() {
@@ -256,7 +255,7 @@ VkResult VulkanPlatformSurfaceSwapChain::create() {
     mSwapChainBundle.colorFormat = surfaceFormat.format;
     mSwapChainBundle.depthFormat =
             selectDepthFormat(mContext.getAttachmentDepthStencilFormats(), mHasStencil);
-    mSwapChainBundle.depth = createImage(mSwapChainBundle.extent, 
+    mSwapChainBundle.depth = createImage(mSwapChainBundle.extent,
             mSwapChainBundle.depthFormat, mIsProtected);
     mSwapChainBundle.isProtected = mIsProtected;
 
@@ -344,6 +343,22 @@ VkResult VulkanPlatformSurfaceSwapChain::recreate() {
 }
 
 void VulkanPlatformSurfaceSwapChain::destroy() {
+    // The next part is not ideal. We don't have a good signal on when it's ok to destroy
+    // a swapchain. This is a spec oversight and mentioned as much:
+    // https://github.com/KhronosGroup/Vulkan-Docs/issues/1678
+    //
+    // One workaround [1] is:
+    // https://docs.vulkan.org/samples/latest/samples/api/swapchain_recreation/README.html
+    //
+    // The proper fix is to use VK_EXT_swapchain_maintenance1, but availability of this extension is
+    // unknown (not yet ratified).
+    //
+    // Instead of adding too much mechanics, we're taking a hacksaw to the problem - just wait for
+    // the queue to be idle. The hope is that this only happens on resize, where performance
+    // degradation is less obvious (until, of course, people complain about lag when rotating their
+    // phone). If necessary, we can revisit and implement the workaround [1].
+    vkQueueWaitIdle(mQueue);
+
     VulkanPlatformSwapChainImpl::destroy();
 
     for (uint32_t i = 0; i < IMAGE_READY_SEMAPHORE_COUNT; ++i) {
@@ -352,6 +367,10 @@ void VulkanPlatformSurfaceSwapChain::destroy() {
             mImageReady[i] = VK_NULL_HANDLE;
         }
     }
+    if (mSwapchain) {
+        vkDestroySwapchainKHR(mDevice, mSwapchain, VKALLOC);
+        mSwapchain = VK_NULL_HANDLE;
+    }
 }
 
 VulkanPlatformHeadlessSwapChain::VulkanPlatformHeadlessSwapChain(VulkanContext const& context,