vk: handle non-error cases for swapchain acquire

When acquiring an image in a swapchain, we could get
VK_ERROR_OUT_OF_DATE_KHR or VK_SUBOPTIMAL_KHR for acceptable states
like window resizing. For those cases, instead of failing to acquire,
we recreate the swapchain and try to acquire swapchain image again.

In case where acquire fails even after the recreation, then we
simply don't draw(). Because acquire happens on beginRenderPass
(we need backing images for the fbo), if acquire fails, then
we assume the renderpass is also invalid, and subsquent calls to
make use of it (e.g. bindPipeline) would just be no-op.

Fixes #9732

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) {
@@ -1947,9 +1946,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();
+            }
         }
     }
 
@@ -2098,6 +2103,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);
@@ -2115,6 +2123,10 @@ void VulkanDriver::endRenderPass(int) {
 }
 
 void VulkanDriver::nextSubpass(int) {
+    if (skipDueToEmptyRenderPass()) {
+        return;
+    }
+
     FILAMENT_CHECK_PRECONDITION(mCurrentRenderPass.currentSubpass == 0)
             << "Only two subpasses are currently supported.";
 
@@ -2201,6 +2213,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(),
@@ -2466,6 +2481,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);
@@ -2528,6 +2547,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();
@@ -2584,6 +2606,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;
 
@@ -2643,6 +2669,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)
@@ -2725,23 +2755,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 @@ private:
     // 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,76 @@ 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))) {
+    // Used for cases where the backing swapchain needs to be recreated.
+    auto recreate = [&]() {
+        // 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;
+    };
+
+    // 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) {
+            recreate();
+        }
+        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;