From 1d13c81a5239300bb5bfa3138db6318834c1ed98 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Wed, 22 Apr 2026 14:38:48 -0700 Subject: [PATCH] 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 | 10 +-- filament/backend/src/vulkan/VulkanDriver.cpp | 60 ++++++++++++++---- filament/backend/src/vulkan/VulkanDriver.h | 7 ++- .../backend/src/vulkan/VulkanSwapChain.cpp | 62 ++++++++++++++----- filament/backend/src/vulkan/VulkanSwapChain.h | 7 ++- 5 files changed, 111 insertions(+), 35 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanContext.h b/filament/backend/src/vulkan/VulkanContext.h index ac487e1fe4cb..35fa551d85a3 100644 --- a/filament/backend/src/vulkan/VulkanContext.h +++ b/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 renderTarget; - fvkmemory::resource_ptr renderPass; - RenderPassParams params; - int currentSubpass; + VulkanCommandBuffer* commandBuffer= nullptr; + fvkmemory::resource_ptr renderTarget {}; + fvkmemory::resource_ptr renderPass {}; + RenderPassParams params = {}; + int currentSubpass = 0; }; // This is a collection of immutable data about the vulkan context. This actual handles to the diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 97f345c2b1f9..e69c94109ced 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -814,9 +814,8 @@ void VulkanDriver::destroyTexture(Handle th) { return; } auto texture = resource_ptr::cast(&mResourceManager, th); - texture.dec(); - mExternalImageManager.removeExternallySampledTexture(texture); + texture.dec(); } void VulkanDriver::createProgramR(Handle ph, Program&& program, utils::ImmutableCString&& tag) { @@ -1947,9 +1946,15 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP fvkmemory::resource_ptr 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 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 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::cast(&mResourceManager, pipelineState.vertexBufferInfo); @@ -2528,6 +2547,9 @@ void VulkanDriver::bindPipelineImpl(PipelineState const& pipelineState, void VulkanDriver::bindRenderPrimitive(Handle 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 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 diff --git a/filament/backend/src/vulkan/VulkanDriver.h b/filament/backend/src/vulkan/VulkanDriver.h index 785156f0fd30..8a329827c3bc 100644 --- a/filament/backend/src/vulkan/VulkanDriver.h +++ b/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; diff --git a/filament/backend/src/vulkan/VulkanSwapChain.cpp b/filament/backend/src/vulkan/VulkanSwapChain.cpp index 72e5ff425928..16cdc5de1cf1 100644 --- a/filament/backend/src/vulkan/VulkanSwapChain.cpp +++ b/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 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(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 diff --git a/filament/backend/src/vulkan/VulkanSwapChain.h b/filament/backend/src/vulkan/VulkanSwapChain.h index 11ae5385c25b..5c6c28c09886 100644 --- a/filament/backend/src/vulkan/VulkanSwapChain.h +++ b/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 acquire(); fvkmemory::resource_ptr getCurrentColor() const noexcept { uint32_t const imageIndex = mCurrentSwapIndex;