vk: handle non-error cases for swapchain acquire

filament/backend/src/vulkan/VulkanContext.h

@@ -61,11 +61,11 @@ struct VulkanAttachment {
 
 struct VulkanRenderPassContext {
     // Between the begin and end command render pass we cache the command buffer
-    VulkanCommandBuffer* commandBuffer;
-    fvkmemory::resource_ptr<VulkanRenderTarget> renderTarget;
-    fvkmemory::resource_ptr<VulkanRenderPass> renderPass;
-    RenderPassParams params;
-    int currentSubpass;
+    VulkanCommandBuffer* commandBuffer= nullptr;
+    fvkmemory::resource_ptr<VulkanRenderTarget> renderTarget {};
+    fvkmemory::resource_ptr<VulkanRenderPass> renderPass {};
+    RenderPassParams params = {};
+    int currentSubpass = 0;
 };
 
 // This is a collection of immutable data about the vulkan context. This actual handles to the

filament/backend/src/vulkan/VulkanDriver.cpp

@@ -814,9 +814,8 @@ void VulkanDriver::destroyTexture(Handle<HwTexture> th) {
         return;
     }
     auto texture = resource_ptr<VulkanTexture>::cast(&mResourceManager, th);
-    texture.dec();
-
     mExternalImageManager.removeExternallySampledTexture(texture);
+    texture.dec();
 }
 
 void VulkanDriver::createProgramR(Handle<HwProgram> ph, Program&& program, utils::ImmutableCString&& tag) {
@@ -1946,9 +1945,15 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
         fvkmemory::resource_ptr<VulkanSwapChain> sc = mCurrentSwapChain;
         assert_invariant(sc);
         if (sc->isFirstRenderPass()) {
-            discardStart |= TargetBufferFlags::COLOR;
-            sc->markFirstRenderPass();
-            acquireNextSwapchainImage();
+            if (!acquireNextSwapchainImage()) {
+                // We've failed to acquire the next image. Subsequent calls cannot assume the render
+                // pass exists.
+                mCurrentRenderPass = {};
+                return;
+            } else {
+                discardStart |= TargetBufferFlags::COLOR;
+                sc->markFirstRenderPass();
+            }
         }
     }
 
@@ -2101,6 +2106,9 @@ void VulkanDriver::beginRenderPass(Handle<HwRenderTarget> rth, const RenderPassP
 
 void VulkanDriver::endRenderPass(int) {
     FVK_SYSTRACE_SCOPE();
+    if (skipDueToEmptyRenderPass()) {
+        return;
+    }
 
     VkCommandBuffer cmdbuffer = mCurrentRenderPass.commandBuffer->buffer();
     vkCmdEndRenderPass(cmdbuffer);
@@ -2118,6 +2126,10 @@ void VulkanDriver::endRenderPass(int) {
 }
 
 void VulkanDriver::nextSubpass(int) {
+    if (skipDueToEmptyRenderPass()) {
+        return;
+    }
+
     FILAMENT_CHECK_PRECONDITION(mCurrentRenderPass.currentSubpass == 0)
             << "Only two subpasses are currently supported.";
 
@@ -2204,6 +2216,9 @@ void VulkanDriver::commit(Handle<HwSwapChain> sch) {
 
 void VulkanDriver::setPushConstant(backend::ShaderStage stage, uint8_t index,
         backend::PushConstantVariant value) {
+    if (skipDueToEmptyRenderPass()) {
+        return;
+    }
     assert_invariant(mPipelineState.program && "Expect a program when writing to push constants");
     assert_invariant(mCurrentRenderPass.commandBuffer && "Should be called within a renderpass");
     mPipelineState.program->writePushConstant(mCurrentRenderPass.commandBuffer->buffer(),
@@ -2469,6 +2484,10 @@ void VulkanDriver::bindPipeline(PipelineState const& pipelineState) {
 void VulkanDriver::bindPipelineImpl(PipelineState const& pipelineState,
         VkPipelineLayout pipelineLayout, fvkutils::DescriptorSetMask descriptorSetMask) {
     FVK_SYSTRACE_SCOPE();
+    if (skipDueToEmptyRenderPass()) {
+        return;
+    }
+
     auto commands = mCurrentRenderPass.commandBuffer;
     auto vbi = resource_ptr<VulkanVertexBufferInfo>::cast(&mResourceManager,
             pipelineState.vertexBufferInfo);
@@ -2531,6 +2550,9 @@ void VulkanDriver::bindPipelineImpl(PipelineState const& pipelineState,
 
 void VulkanDriver::bindRenderPrimitive(Handle<HwRenderPrimitive> rph) {
     FVK_SYSTRACE_SCOPE();
+    if (skipDueToEmptyRenderPass()) {
+        return;
+    }
 
     VulkanCommandBuffer* commands = mCurrentRenderPass.commandBuffer;
     VkCommandBuffer cmdbuffer = commands->buffer();
@@ -2587,6 +2609,10 @@ void VulkanDriver::bindDescriptorSet(
 
 void VulkanDriver::draw2(uint32_t indexOffset, uint32_t indexCount, uint32_t instanceCount) {
     FVK_SYSTRACE_SCOPE();
+    if (skipDueToEmptyRenderPass()) {
+        return;
+    }
+
     VkCommandBuffer cmdbuffer = mCurrentRenderPass.commandBuffer->buffer();
     auto const& [doBindInDraw, bundle] = mPipelineState.bindInDraw;
 
@@ -2646,6 +2672,10 @@ void VulkanDriver::dispatchCompute(Handle<HwProgram> program, math::uint3 workGr
 }
 
 void VulkanDriver::scissor(Viewport scissorBox) {
+    if (skipDueToEmptyRenderPass()) {
+        return;
+    }
+
     VkCommandBuffer cmdbuffer = mCurrentRenderPass.commandBuffer->buffer();
 
     // TODO: it's a common case that scissor() is called with (0, 0, maxint, maxint)
@@ -2728,23 +2758,31 @@ void VulkanDriver::endCommandRecording() {
     mDescriptorSetCache.resetCachedState();
 }
 
-void VulkanDriver::acquireNextSwapchainImage() {
+bool 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;
+        // true means that the rendertarget has the right images attached.
+        return true;
     }
 
-    bool resized = false;
-    mCurrentSwapChain->acquire(resized);
-    if (resized) {
+    auto const [acquired, backingChanged] = mCurrentSwapChain->acquire();
+    if (backingChanged) {
         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);
+
+    if (acquired) {
+        mDefaultRenderTarget->bindSwapChain(mCurrentSwapChain);
+        return true;
+    }
+    mDefaultRenderTarget->releaseSwapchain();
+    // We failed to acquire the next image in the swapchain.  The rendertarget is no longer valid
+    // for use.
+    return false;
 }
 
 // explicit instantiation of the Dispatcher

filament/backend/src/vulkan/VulkanDriver.h

@@ -132,7 +132,12 @@ class VulkanDriver final : public DriverBase {
     // Flush the current command buffer and reset the pipeline state.
     void endCommandRecording();
 
-    void acquireNextSwapchainImage();
+    // Returns whether the acquire was successful
+    bool acquireNextSwapchainImage();
+
+    bool skipDueToEmptyRenderPass() const {
+        return !bool(mCurrentRenderPass.renderTarget);
+    }
 
     VulkanPlatform* mPlatform = nullptr;
     fvkmemory::ResourceManager mResourceManager;

filament/backend/src/vulkan/VulkanSwapChain.cpp

@@ -149,41 +149,73 @@ void VulkanSwapChain::present(DriverBase& driver) {
     }
 }
 
-void VulkanSwapChain::acquire(bool& resized) {
-    // It's ok to call acquire multiple times due to it being linked to Driver::makeCurrent().
+std::pair<bool, bool> VulkanSwapChain::acquire() {
+    // Indicates whether the backing swapchain has changed (and might invalidate the associated
+    // images that are tracked in the FBO cache).
+    bool swapchainRecreated = false;
+
+    // Final result of the call to acquire a swapchain image.
+    VkResult result = VK_NOT_READY;
+
+    // It's ok to call acquire multiple times due to it being linked to Driver::makeCurrent(). If a
+    // valid swapchain has already been acquired, then this method is no-op.
     if (mAcquired) {
-        return;
+        return { mAcquired, swapchainRecreated };
     }
 
-    // Check if the swapchain should be resized.
-    if ((resized = mPlatform->hasResized(swapChain))) {
-        if (mFlushAndWaitOnResize) {
-            mCommands->flush();
-            mCommands->wait();
-        }
-        mPlatform->recreate(swapChain);
-        update();
+    // Check if the surface has resized; if so, we need to recreate a swapchain, which is done in
+    // the while loop.
+    if (mPlatform->hasResized(swapChain)) {
+        // This indicates a surface size change and a need to recreate swapchain.
+        result = VK_ERROR_OUT_OF_DATE_KHR;
     }
 
     VulkanPlatform::ImageSyncData imageSyncData;
-    VkResult const result = mPlatform->acquire(swapChain, &imageSyncData);
+
+    // Following is written as a loop to cover a few cases:
+    //   - If resize is true from hasResized() above, then result == VK_ERROR_OUT_OF_DATE_KHR.
+    //     And we will first recreate the swapchain before acquiring (on tryCount == 0).
+    //   - If resize is not true, then just try to acquire (on tryCount == 0)
+    //       - If the acquire succeeds, then result == VK_SUCCESS, break loop
+    //       - if acquire fails and result = VK_SUBOPTIMAL_KHR or VK_ERROR_OUT_OF_DATE_KHR
+    //         (on tryCount == 1), then
+    //           - recreate swapchain and try to acquire again (on tryCount == 1).
+
+    for (uint8_t tryCount = 0; result != VK_SUCCESS && tryCount < 2; tryCount++) {
+        if (result == VK_SUBOPTIMAL_KHR || result == VK_ERROR_OUT_OF_DATE_KHR) {
+            // Following recreates the swapchain
+
+            // Calling flush multiptle times is ok, since it's no-op if not recording.
+            if (mFlushAndWaitOnResize) {
+                mCommands->flush();
+                mCommands->wait();
+            }
+            mPlatform->recreate(swapChain);
+            update();
+            swapchainRecreated = true;
+        }
+        result = mPlatform->acquire(swapChain, &imageSyncData);
+    }
 
     if (result != VK_SUCCESS) {
         // We just don't set mAcquired here so the next present will just skip.
         FVK_LOGD << "Failed to acquire next image in the swapchain result=" << (int) result;
-        return;
+        return { false, swapchainRecreated };
     }
 
+    // At this point acquiring the next swapchain image has succeeded
+
     mCurrentSwapIndex = imageSyncData.imageIndex;
     assert_invariant(mCurrentSwapIndex < mFinishedDrawing.size());
     mFinishedDrawing[mCurrentSwapIndex] = {};
-    FILAMENT_CHECK_POSTCONDITION(result == VK_SUCCESS || result == VK_SUBOPTIMAL_KHR)
-            << "Cannot acquire in swapchain. error=" << static_cast<int32_t>(result);
+
     if (imageSyncData.imageReadySemaphore != VK_NULL_HANDLE) {
         mCommands->injectDependency(imageSyncData.imageReadySemaphore,
                 VK_PIPELINE_STAGE_ALL_COMMANDS_BIT);
     }
     mAcquired = true;
+
+    return { true, swapchainRecreated };
 }
 
 }// namespace filament::backend

filament/backend/src/vulkan/VulkanSwapChain.h

@@ -48,9 +48,10 @@ struct VulkanSwapChain : public HwSwapChain, fvkmemory::Resource {
 
     void present(DriverBase& driver);
 
-    // Acquire a new image from the swapchain. If the image is not available it would wait until it
-    // is.
-    void acquire(bool& resized);
+    // Acquire a new image from the swapchain. Returns a pair:
+    //   1. whether an acquire is successful.
+    //   2. whether the backing images have changed (due to resize or other factors).
+    std::pair<bool, bool> acquire();
 
     fvkmemory::resource_ptr<VulkanTexture> getCurrentColor() const noexcept {
         uint32_t const imageIndex = mCurrentSwapIndex;