From 9cfef925bb8a3e5a0103170ae9d07b67a8b9401f Mon Sep 17 00:00:00 2001 From: Grant Commodore Date: Mon, 17 Jun 2024 15:29:28 -0700 Subject: [PATCH] Updates vulkan swapchain synchronization (#7915) Abstracts the synchronization of the vulkan swapchain so that it is easier to override during the acquisition and presentation of images. A new structure (ImageSyncData) is created to hold swapchain synchronization data. FIXES=[338303279] --- .../backend/platforms/VulkanPlatform.h | 27 ++++++++-- .../backend/src/vulkan/VulkanSwapChain.cpp | 53 +++++++------------ filament/backend/src/vulkan/VulkanSwapChain.h | 9 ++-- .../src/vulkan/platform/VulkanPlatform.cpp | 4 +- .../platform/VulkanPlatformSwapChainImpl.cpp | 36 ++++++++++--- .../platform/VulkanPlatformSwapChainImpl.h | 14 +++-- 6 files changed, 89 insertions(+), 54 deletions(-) diff --git a/filament/backend/include/backend/platforms/VulkanPlatform.h b/filament/backend/include/backend/platforms/VulkanPlatform.h index ddde79af90..2bbe1983f2 100644 --- a/filament/backend/include/backend/platforms/VulkanPlatform.h +++ b/filament/backend/include/backend/platforms/VulkanPlatform.h @@ -90,6 +90,20 @@ public: VkExtent2D extent = {0, 0}; }; + struct ImageSyncData { + static constexpr uint32_t INVALID_IMAGE_INDEX = UINT32_MAX; + + // The index of the next image as returned by vkAcquireNextImage or equivalent. + uint32_t imageIndex = INVALID_IMAGE_INDEX; + + // Semaphore to be signaled once the image is available. + VkSemaphore imageReadySemaphore = VK_NULL_HANDLE; + + // A function called right before vkQueueSubmit. After this call, the image must be + // available. This pointer can be null if imageReadySemaphore is not VK_NULL_HANDLE. + std::function explicitImageReadyWait = nullptr; + }; + VulkanPlatform(); ~VulkanPlatform() override; @@ -127,6 +141,12 @@ public: * before recreating the swapchain. Default is true. */ bool flushAndWaitOnWindowResize = true; + + /** + * Whether the swapchain image should be transitioned to a layout suitable for + * presentation. Default is true. + */ + bool transitionSwapChainImageLayoutForPresent = true; }; /** @@ -155,13 +175,10 @@ public: * corresponding VkImage will be used as the output color attachment. The client should signal * the `clientSignal` semaphore when the image is ready to be used by the backend. * @param handle The handle returned by createSwapChain() - * @param clientSignal The semaphore that the client will signal to indicate that the backend - * may render into the image. - * @param index Pointer to memory that will be filled with the index that corresponding - * to an image in the `SwapChainBundle.colors` array. + * @param outImageSyncData The synchronization data used for image readiness * @return Result of acquire */ - virtual VkResult acquire(SwapChainPtr handle, VkSemaphore clientSignal, uint32_t* index); + virtual VkResult acquire(SwapChainPtr handle, ImageSyncData* outImageSyncData); /** * Present the image corresponding to `index` to the display. The client should wait on diff --git a/filament/backend/src/vulkan/VulkanSwapChain.cpp b/filament/backend/src/vulkan/VulkanSwapChain.cpp index 9618763065..8c4a345cae 100644 --- a/filament/backend/src/vulkan/VulkanSwapChain.cpp +++ b/filament/backend/src/vulkan/VulkanSwapChain.cpp @@ -35,29 +35,13 @@ VulkanSwapChain::VulkanSwapChain(VulkanPlatform* platform, VulkanContext const& mStagePool(stagePool), mHeadless(extent.width != 0 && extent.height != 0 && !nativeWindow), mFlushAndWaitOnResize(platform->getCustomization().flushAndWaitOnWindowResize), - mCurrentImageReadyIndex(0), + mTransitionSwapChainImageLayoutForPresent( + platform->getCustomization().transitionSwapChainImageLayoutForPresent), mAcquired(false), mIsFirstRenderPass(true) { swapChain = mPlatform->createSwapChain(nativeWindow, flags, extent); FILAMENT_CHECK_POSTCONDITION(swapChain) << "Unable to create swapchain"; - VkSemaphoreCreateInfo const createInfo = { - .sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO, - }; - - // No need to wait on this semaphore before drawing when in Headless mode. - if (mHeadless) { - // Set all sempahores to VK_NULL_HANDLE - memset(mImageReady, 0, sizeof(mImageReady[0]) * IMAGE_READY_SEMAPHORE_COUNT); - } else { - for (uint32_t i = 0; i < IMAGE_READY_SEMAPHORE_COUNT; ++i) { - VkResult result = - vkCreateSemaphore(mPlatform->getDevice(), &createInfo, nullptr, mImageReady + i); - FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS) - << "Failed to create semaphore"; - } - } - update(); } @@ -68,11 +52,6 @@ VulkanSwapChain::~VulkanSwapChain() { mCommands->wait(); mPlatform->destroy(swapChain); - for (uint32_t i = 0; i < IMAGE_READY_SEMAPHORE_COUNT; ++i) { - if (mImageReady[i] != VK_NULL_HANDLE) { - vkDestroySemaphore(mPlatform->getDevice(), mImageReady[i], VKALLOC); - } - } } void VulkanSwapChain::update() { @@ -95,7 +74,7 @@ void VulkanSwapChain::update() { } void VulkanSwapChain::present() { - if (!mHeadless) { + if (!mHeadless && mTransitionSwapChainImageLayoutForPresent) { VkCommandBuffer const cmdbuf = mCommands->get().buffer(); VkImageSubresourceRange const subresources{ .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT, @@ -106,10 +85,15 @@ void VulkanSwapChain::present() { }; mColors[mCurrentSwapIndex]->transitionLayout(cmdbuf, subresources, VulkanLayout::PRESENT); } - mCommands->flush(); - // We only present if it is not headless. No-op for headless (but note that we still need the - // flush() in the above line). + mCommands->flush(); + + // call the image ready wait function + if (mExplicitImageReadyWait != nullptr) { + mExplicitImageReadyWait(swapChain); + } + + // We only present if it is not headless. No-op for headless. if (!mHeadless) { VkSemaphore const finishedDrawing = mCommands->acquireFinishedSignal(); VkResult const result = mPlatform->present(swapChain, mCurrentSwapIndex, finishedDrawing); @@ -139,13 +123,14 @@ void VulkanSwapChain::acquire(bool& resized) { update(); } - mCurrentImageReadyIndex = (mCurrentImageReadyIndex + 1) % IMAGE_READY_SEMAPHORE_COUNT; - const VkSemaphore imageReady = mImageReady[mCurrentImageReadyIndex]; - VkResult const result = mPlatform->acquire(swapChain, imageReady, &mCurrentSwapIndex); - FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS || result == VK_SUBOPTIMAL_KHR) - << "Cannot acquire in swapchain."; - if (imageReady != VK_NULL_HANDLE) { - mCommands->injectDependency(imageReady); + VulkanPlatform::ImageSyncData imageSyncData; + VkResult const result = mPlatform->acquire(swapChain, &imageSyncData); + mCurrentSwapIndex = imageSyncData.imageIndex; + mExplicitImageReadyWait = imageSyncData.explicitImageReadyWait; + FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS || result == VK_SUBOPTIMAL_KHR) + << "Cannot acquire in swapchain."; + if (imageSyncData.imageReadySemaphore != VK_NULL_HANDLE) { + mCommands->injectDependency(imageSyncData.imageReadySemaphore); } mAcquired = true; } diff --git a/filament/backend/src/vulkan/VulkanSwapChain.h b/filament/backend/src/vulkan/VulkanSwapChain.h index 55be65d403..80886f8b1b 100644 --- a/filament/backend/src/vulkan/VulkanSwapChain.h +++ b/filament/backend/src/vulkan/VulkanSwapChain.h @@ -50,7 +50,10 @@ struct VulkanSwapChain : public HwSwapChain, VulkanResource { void acquire(bool& reized); inline VulkanTexture* getCurrentColor() const noexcept { - return mColors[mCurrentSwapIndex].get(); + uint32_t const imageIndex = mCurrentSwapIndex; + FILAMENT_CHECK_PRECONDITION( + imageIndex != VulkanPlatform::ImageSyncData::INVALID_IMAGE_INDEX); + return mColors[imageIndex].get(); } inline VulkanTexture* getDepth() const noexcept { @@ -80,15 +83,15 @@ private: VulkanStagePool& mStagePool; bool const mHeadless; bool const mFlushAndWaitOnResize; + bool const mTransitionSwapChainImageLayoutForPresent; // We create VulkanTextures based on VkImages. VulkanTexture has facilities for doing layout // transitions, which are useful here. utils::FixedCapacityVector> mColors; std::unique_ptr mDepth; VkExtent2D mExtent; - VkSemaphore mImageReady[IMAGE_READY_SEMAPHORE_COUNT]; - uint32_t mCurrentImageReadyIndex; uint32_t mCurrentSwapIndex; + std::function mExplicitImageReadyWait = nullptr; bool mAcquired; bool mIsFirstRenderPass; }; diff --git a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp index 9f13893a93..b4152807cc 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatform.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatform.cpp @@ -772,8 +772,8 @@ VulkanPlatform::SwapChainBundle VulkanPlatform::getSwapChainBundle(SwapChainPtr SWAPCHAIN_RET_FUNC(getSwapChainBundle, handle, ) } -VkResult VulkanPlatform::acquire(SwapChainPtr handle, VkSemaphore clientSignal, uint32_t* index) { - SWAPCHAIN_RET_FUNC(acquire, handle, clientSignal, index) +VkResult VulkanPlatform::acquire(SwapChainPtr handle, ImageSyncData* outImageSyncData) { + SWAPCHAIN_RET_FUNC(acquire, handle, outImageSyncData) } VkResult VulkanPlatform::present(SwapChainPtr handle, uint32_t index, diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp index 6e8411b84d..6c643f754d 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp +++ b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.cpp @@ -133,6 +133,7 @@ VulkanPlatformSurfaceSwapChain::VulkanPlatformSurfaceSwapChain(VulkanContext con VulkanPlatformSurfaceSwapChain::~VulkanPlatformSurfaceSwapChain() { vkDestroySwapchainKHR(mDevice, mSwapchain, VKALLOC); vkDestroySurfaceKHR(mInstance, mSurface, VKALLOC); + destroy(); } VkResult VulkanPlatformSurfaceSwapChain::create() { @@ -254,14 +255,24 @@ VkResult VulkanPlatformSurfaceSwapChain::create() { << "depth=" << mSwapChainBundle.depthFormat << io::endl; + VkSemaphoreCreateInfo const semaphoreCreateInfo = { + .sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO, + }; + + for (uint32_t i = 0; i < IMAGE_READY_SEMAPHORE_COUNT; ++i) { + VkResult result = vkCreateSemaphore(mDevice, &semaphoreCreateInfo, nullptr, + mImageReady + i); + FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS) << "Failed to create semaphore"; + } + return result; } -VkResult VulkanPlatformSurfaceSwapChain::acquire(VkSemaphore clientSignal, uint32_t* index) { - // This immediately retrieves the index of the next available presentable image, and - // asynchronously requests the GPU to trigger the "imageAvailable" semaphore. - VkResult result = vkAcquireNextImageKHR(mDevice, mSwapchain, UINT64_MAX, clientSignal, - VK_NULL_HANDLE, index); +VkResult VulkanPlatformSurfaceSwapChain::acquire(VulkanPlatform::ImageSyncData* outImageSyncData) { + mCurrentImageReadyIndex = (mCurrentImageReadyIndex + 1) % IMAGE_READY_SEMAPHORE_COUNT; + outImageSyncData->imageReadySemaphore = mImageReady[mCurrentImageReadyIndex]; + VkResult result = vkAcquireNextImageKHR(mDevice, mSwapchain, UINT64_MAX, + outImageSyncData->imageReadySemaphore, VK_NULL_HANDLE, &outImageSyncData->imageIndex); // Users should be notified of a suboptimal surface, but it should not cause a cascade of // log messages or a loop of re-creations. @@ -312,6 +323,17 @@ VkResult VulkanPlatformSurfaceSwapChain::recreate() { return create(); } +void VulkanPlatformSurfaceSwapChain::destroy() { + VulkanPlatformSwapChainImpl::destroy(); + + for (uint32_t i = 0; i < IMAGE_READY_SEMAPHORE_COUNT; ++i) { + if (mImageReady[i] != VK_NULL_HANDLE) { + vkDestroySemaphore(mDevice, mImageReady[i], VKALLOC); + mImageReady[i] = VK_NULL_HANDLE; + } + } +} + VulkanPlatformHeadlessSwapChain::VulkanPlatformHeadlessSwapChain(VulkanContext const& context, VkDevice device, VkQueue queue, VkExtent2D extent, uint64_t flags) : VulkanPlatformSwapChainImpl(context, device, queue), @@ -343,8 +365,8 @@ VkResult VulkanPlatformHeadlessSwapChain::present(uint32_t index, VkSemaphore fi return VK_SUCCESS; } -VkResult VulkanPlatformHeadlessSwapChain::acquire(VkSemaphore clientSignal, uint32_t* index) { - *index = mCurrentIndex; +VkResult VulkanPlatformHeadlessSwapChain::acquire(VulkanPlatform::ImageSyncData* outImageSyncData) { + outImageSyncData->imageIndex = mCurrentIndex; mCurrentIndex = (mCurrentIndex + 1) % HEADLESS_SWAPCHAIN_SIZE; return VK_SUCCESS; } diff --git a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.h b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.h index ac4e26c105..bcad1ccb0e 100644 --- a/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.h +++ b/filament/backend/src/vulkan/platform/VulkanPlatformSwapChainImpl.h @@ -42,7 +42,7 @@ struct VulkanPlatformSwapChainImpl : public Platform::SwapChain { ~VulkanPlatformSwapChainImpl(); // Non-virtual override-able method - VkResult acquire(VkSemaphore clientSignal, uint32_t* index) { + VkResult acquire(VulkanPlatform::ImageSyncData* outImageSyncData) { PANIC_PRECONDITION("Should not be called"); return VK_ERROR_UNKNOWN; } @@ -86,7 +86,7 @@ struct VulkanPlatformSurfaceSwapChain : public VulkanPlatformSwapChainImpl { ~VulkanPlatformSurfaceSwapChain(); // Non-virtual override - VkResult acquire(VkSemaphore clientSignal, uint32_t* index); + VkResult acquire(VulkanPlatform::ImageSyncData* outImageSyncData); // Non-virtual override VkResult present(uint32_t index, VkSemaphore finished); @@ -97,7 +97,13 @@ struct VulkanPlatformSurfaceSwapChain : public VulkanPlatformSwapChainImpl { // Non-virtual override-able method bool hasResized(); +protected: + // Non-virtual override-able method + void destroy(); + private: + static constexpr int IMAGE_READY_SEMAPHORE_COUNT = FVK_MAX_COMMAND_BUFFERS; + VkResult create(); VkInstance mInstance; @@ -106,6 +112,8 @@ private: VkSurfaceKHR mSurface; VkSwapchainKHR mSwapchain = VK_NULL_HANDLE; VkExtent2D const mFallbackExtent; + VkSemaphore mImageReady[IMAGE_READY_SEMAPHORE_COUNT]; + uint32_t mCurrentImageReadyIndex; bool mUsesRGB = false; bool mHasStencil = false; @@ -121,7 +129,7 @@ struct VulkanPlatformHeadlessSwapChain : public VulkanPlatformSwapChainImpl { ~VulkanPlatformHeadlessSwapChain(); // Non-virtual override - VkResult acquire(VkSemaphore clientSignal, uint32_t* index); + VkResult acquire(VulkanPlatform::ImageSyncData* outImageSyncData); // Non-virtual override VkResult present(uint32_t index, VkSemaphore finished);