vk: move swapchain acquire() to as late as possible (#9541)

makeCurrent is not meant to acquire the swapchain; we should
do so at the momemnt we need it, to shorten the amount of time
a swapchain image/buffer needs to be held.

Author: poweifeng

filament/backend/src/vulkan/VulkanDriver.cpp

@@ -810,7 +810,7 @@ void VulkanDriver::createDefaultRenderTargetR(Handle<HwRenderTarget> rth, utils:
     assert_invariant(mDefaultRenderTarget); // Default render target should already exist.
 
     auto renderTarget = resource_ptr<VulkanRenderTarget>::make(&mResourceManager, rth,
-            std::move(std::move(*mDefaultRenderTarget.get())));
+            std::move(*mDefaultRenderTarget.get()));
 
     mDefaultRenderTarget = renderTarget;
     mResourceManager.associateHandle(rth.getId(), std::move(tag));
@@ -1808,9 +1808,6 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
     FVK_SYSTRACE_SCOPE();
 
     auto rt = resource_ptr<VulkanRenderTarget>::cast(&mResourceManager, rth);
-    VkExtent2D const extent = rt->getExtent();
-
-    assert_invariant(rt == mDefaultRenderTarget || extent.width > 0 && extent.height > 0);
 
     VulkanCommandBuffer* commandBuffer = rt->isProtected() ?
            &mCommands.getProtected() : &mCommands.get();
@@ -1825,9 +1822,16 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
         if (sc->isFirstRenderPass()) {
             discardStart |= TargetBufferFlags::COLOR;
             sc->markFirstRenderPass();
+            acquireNextSwapchainImage();
         }
     }
 
+    // Note that retrieving the extent must come after the acquireNextSwapchainImage() above;
+    // otherwise it might be 0.
+    VkExtent2D const extent = rt->getExtent();
+
+    assert_invariant(rt == mDefaultRenderTarget || extent.width > 0 && extent.height > 0);
+
 #if FVK_ENABLED(FVK_DEBUG_TEXTURE)
     if (rt->hasDepth()) {
         auto depth = rt->getDepth();
@@ -2038,20 +2042,13 @@ void VulkanDriver::makeCurrent(Handle<HwSwapChain> drawSch, Handle<HwSwapChain>
     ASSERT_PRECONDITION_NON_FATAL(drawSch == readSch,
             "Vulkan driver does not support distinct draw/read swap chains.");
 
+    // Before we can render into the new swapchain, we need to make sure the default render target
+    // releases its old bound swapchain image.
+    mDefaultRenderTarget->releaseSwapchain();
+
     resource_ptr<VulkanSwapChain> swapChain =
             resource_ptr<VulkanSwapChain>::cast(&mResourceManager, drawSch);
     mCurrentSwapChain = swapChain;
-
-    bool resized = false;
-    swapChain->acquire(resized);
-
-    if (resized) {
-        mFramebufferCache.resetFramebuffers();
-    }
-
-    if (UTILS_LIKELY(mDefaultRenderTarget)) {
-        mDefaultRenderTarget->bindToSwapChain(swapChain);
-    }
 }
 
 void VulkanDriver::commit(Handle<HwSwapChain> sch) {
@@ -2222,6 +2219,14 @@ void VulkanDriver::blitDEPRECATED(TargetBufferFlags buffers,
     auto dstTarget = resource_ptr<VulkanRenderTarget>::cast(&mResourceManager, dst);
     auto srcTarget = resource_ptr<VulkanRenderTarget>::cast(&mResourceManager, src);
 
+    // This is a valid use of the api.  We need to make sure that the swapchain image has been
+    // acquired and associated with the default render target. It's okay to call
+    // acquireNextSwapchainImage() even if a swapchain image has already been bound to the default
+    // render target.
+    if (dstTarget == mDefaultRenderTarget) {
+        acquireNextSwapchainImage();
+    }
+
     VkFilter const vkfilter = (filter == SamplerMagFilter::NEAREST) ?
             VK_FILTER_NEAREST : VK_FILTER_LINEAR;
 
@@ -2553,6 +2558,25 @@ void VulkanDriver::endCommandRecording() {
     mDescriptorSetCache.resetCachedState();
 }
 
+void VulkanDriver::acquireNextSwapchainImage() {
+    assert_invariant(mCurrentSwapChain);
+    assert_invariant(mDefaultRenderTarget);
+
+    // Swapchain has already been bound to the default render target.  We just return.
+    if (mDefaultRenderTarget->isSwapchainBound()) {
+        return;
+    }
+
+    bool resized = false;
+    mCurrentSwapChain->acquire(resized);
+    if (resized) {
+        mFramebufferCache.resetFramebuffers();
+    }
+    // Note that ordering this after the above lines is necessary since we set the swapchain image
+    // to the render target in bindSwapChain().
+    mDefaultRenderTarget->bindSwapChain(mCurrentSwapChain);
+}
+
 // explicit instantiation of the Dispatcher
 template class ConcreteDispatcher<VulkanDriver>;
 

filament/backend/src/vulkan/VulkanDriver.h

@@ -132,6 +132,8 @@ class VulkanDriver final : public DriverBase {
     // Flush the current command buffer and reset the pipeline state.
     void endCommandRecording();
 
+    void acquireNextSwapchainImage();
+
     VulkanPlatform* mPlatform = nullptr;
     fvkmemory::ResourceManager mResourceManager;
 
@@ -160,7 +162,6 @@ class VulkanDriver final : public DriverBase {
     VulkanExternalImageManager mExternalImageManager;
     VulkanStreamedImageManager mStreamedImageManager;
 
-
     // This maps a VulkanSwapchain to a native swapchain. VulkanSwapchain should have a copy of the
     // Platform::Swapchain pointer, but queryFrameTimestamps() and queryCompositorTiming() are
     // synchronous calls, making access to VulkanSwapchain unsafe (this difference vs other backends

filament/backend/src/vulkan/VulkanHandles.cpp

@@ -387,16 +387,18 @@ VulkanRenderTarget::VulkanRenderTarget()
 
 VulkanRenderTarget::~VulkanRenderTarget() = default;
 
-void VulkanRenderTarget::bindToSwapChain(fvkmemory::resource_ptr<VulkanSwapChain> swapchain) {
+void VulkanRenderTarget::bindSwapChain(fvkmemory::resource_ptr<VulkanSwapChain> swapchain) {
     assert_invariant(!mOffscreen);
+    assert_invariant(!mInfo->colors[0]);
 
     VkExtent2D const extent = swapchain->getExtent();
     width = extent.width;
     height = extent.height;
     mProtected = swapchain->isProtected();
 
     VulkanAttachment color = createSwapchainAttachment(swapchain->getCurrentColor());
-    mInfo->attachments = {color};
+    assert_invariant(mInfo->attachments.size() == 0);
+    mInfo->attachments.push_back(color);
 
     auto& fbkey = mInfo->fbkey;
     auto& rpkey = mInfo->rpkey;
@@ -422,6 +424,11 @@ void VulkanRenderTarget::bindToSwapChain(fvkmemory::resource_ptr<VulkanSwapChain
     mInfo->colors.set(0);
 }
 
+void VulkanRenderTarget::releaseSwapchain() {
+    mInfo->colors = {};
+    mInfo->attachments.clear();
+}
+
 VulkanRenderTarget::VulkanRenderTarget(VkDevice device, VkPhysicalDevice physicalDevice,
         VulkanContext const& context, fvkmemory::ResourceManager* resourceManager,
         VmaAllocator allocator, VulkanCommands* commands, uint32_t width, uint32_t height,

filament/backend/src/vulkan/VulkanHandles.h

@@ -366,7 +366,13 @@ struct VulkanRenderTarget : private HwRenderTarget, fvkmemory::Resource {
     inline bool isSwapChain() const { return !mOffscreen; }
     inline bool isProtected() const { return mProtected; }
 
-    void bindToSwapChain(fvkmemory::resource_ptr<VulkanSwapChain> swapchain);
+    void bindSwapChain(fvkmemory::resource_ptr<VulkanSwapChain> swapchain);
+
+    void releaseSwapchain();
+
+    bool isSwapchainBound() const {
+        return isSwapChain() && mInfo->colors[0];
+    }
 
     void emitBarriersBeginRenderPass(VulkanCommandBuffer& commands);