From a4945939de514d049baeed654efbbdd06bc5bdbf Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Thu, 6 Nov 2025 06:10:05 +0000 Subject: [PATCH] vk: fix leaking swapchains again (#9410) - Adjust order of destroy call in both headless and platform swapchains. We need to be careful of the order due to assumptions made by the base class's destroy(). - remove `=0` since VulkanPlatformSwapChainBase::destroy() has an implementation. Fixes #9403 --- .../src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp | 9 ++++++--- .../src/vulkan/platform/VulkanPlatformSwapChainImpl.h | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp index 27ac6f8685..c85307fdae 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp @@ -408,7 +408,6 @@ VkResult VulkanPlatformSurfaceSwapChain::recreate() { } void VulkanPlatformSurfaceSwapChain::destroy() { - VulkanPlatformSwapChainBase::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 @@ -425,6 +424,8 @@ void VulkanPlatformSurfaceSwapChain::destroy() { // phone). If necessary, we can revisit and implement the workaround [1]. vkQueueWaitIdle(mQueue); + VulkanPlatformSwapChainBase::destroy(); + for (uint32_t i = 0; i < IMAGE_READY_SEMAPHORE_COUNT; ++i) { if (mImageReady[i] != VK_NULL_HANDLE) { vkDestroySemaphore(mDevice, mImageReady[i], VKALLOC); @@ -475,7 +476,6 @@ VkResult VulkanPlatformHeadlessSwapChain::acquire(VulkanPlatform::ImageSyncData* } void VulkanPlatformHeadlessSwapChain::destroy() { - VulkanPlatformSwapChainBase::destroy(); // This is only ever called from the destructor since headless does not recreate. for (auto image: mSwapChainBundle.colors) { vkDestroyImage(mDevice, image, VKALLOC); @@ -485,7 +485,10 @@ void VulkanPlatformHeadlessSwapChain::destroy() { } } mSwapChainBundle.colors.clear(); - // No need to manually call through to the super because the super's destructor will be called + + // Still need to call through to free the depth image. But must do it after releasing the color + // images. + VulkanPlatformSwapChainBase::destroy(); } }// namespace filament::backend diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.h b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.h index fa3662a1ef..c1f97ba530 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.h +++ b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.h @@ -62,7 +62,7 @@ struct VulkanPlatformSwapChainBase : public Platform::SwapChain { virtual bool queryFrameTimestamps(uint64_t frameId, FrameTimestamps* outFrameTimestamps) const; protected: - virtual void destroy() = 0; + virtual void destroy(); VkImage createImage(VkExtent2D extent, VkFormat format, bool isProtected);