From 80942d520d77357eb650ea03a31a0e10227bf7d8 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 14 Jan 2025 12:58:07 -0800 Subject: [PATCH] [Impeller] fixes for AHB swapchains. (#161562) Fixes for a variety of speculative fixes for AHB swapchain issues. Details TBD while some tests run... 1. Even If the SurfaceControl API is supported, there is no guarantee that we can import the AHB as there may be a mismatch in the required memory properties and what is available. Add a validity check to the texture pool and bail from the swapchain if any are invalid. 2. Rather than submitting a dummy cmd buffer that does a layout transition, use the final command buffer signal semaphore. 3. Import the render ready semaphore and use it to block the onscreen command buffer. --- .../swapchain/ahb/ahb_swapchain_impl_vk.cc | 75 +++++++------------ .../swapchain/ahb/ahb_swapchain_impl_vk.h | 14 +++- .../swapchain/ahb/ahb_texture_pool_vk.cc | 6 +- 3 files changed, 43 insertions(+), 52 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.cc index fcc4707890..0fd2c7f636 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.cc @@ -6,15 +6,11 @@ #include "flutter/fml/trace_event.h" #include "impeller/base/validation.h" -#include "impeller/renderer/backend/vulkan/barrier_vk.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" -#include "impeller/renderer/backend/vulkan/fence_waiter_vk.h" -#include "impeller/renderer/backend/vulkan/gpu_tracer_vk.h" #include "impeller/renderer/backend/vulkan/swapchain/ahb/ahb_formats.h" #include "impeller/renderer/backend/vulkan/swapchain/surface_vk.h" #include "impeller/toolkit/android/surface_transaction.h" #include "impeller/toolkit/android/surface_transaction_stats.h" -#include "vulkan/vulkan_to_string.hpp" namespace impeller { @@ -96,6 +92,7 @@ std::unique_ptr AHBSwapchainImplVK::AcquireNextDrawable() { } } + frame_index_ = (frame_index_ + 1) % kMaxPendingPresents; AutoSemaSignaler auto_sema_signaler = std::make_shared( [sema = pending_presents_]() { sema->Signal(); }); @@ -111,8 +108,8 @@ std::unique_ptr AHBSwapchainImplVK::AcquireNextDrawable() { return nullptr; } - // Ask the GPU to wait for the render ready semaphore to be signaled before - // performing rendering operations. + // Import the render ready semaphore that will block onscreen rendering until + // it is ready. if (!SubmitWaitForRenderReady(pool_entry.render_ready_fence, pool_entry.texture)) { VALIDATION_LOG << "Could wait on render ready fence."; @@ -194,11 +191,7 @@ bool AHBSwapchainImplVK::Present( void AHBSwapchainImplVK::AddFinalCommandBuffer( std::shared_ptr cmd_buffer) { - auto context = transients_->GetContext().lock(); - if (!context) { - return; - } - context->GetCommandQueue()->Submit({std::move(cmd_buffer)}); + frame_data_[frame_index_].command_buffer = std::move(cmd_buffer); } std::shared_ptr @@ -213,27 +206,12 @@ AHBSwapchainImplVK::SubmitSignalForPresentReady( return nullptr; } - auto command_buffer = context->CreateCommandBuffer(); + auto command_buffer = frame_data_[frame_index_].command_buffer; if (!command_buffer) { return nullptr; } - command_buffer->SetLabel("AHBSubmitSignalForPresentReadyCB"); CommandBufferVK& command_buffer_vk = CommandBufferVK::Cast(*command_buffer); - const auto command_encoder_vk = command_buffer_vk.GetCommandBuffer(); - - BarrierVK barrier; - barrier.cmd_buffer = command_encoder_vk; - barrier.new_layout = vk::ImageLayout::eGeneral; - barrier.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput; - barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite; - barrier.dst_stage = vk::PipelineStageFlagBits::eBottomOfPipe; - barrier.dst_access = {}; - - if (!texture->SetLayout(barrier).ok()) { - return nullptr; - } - command_buffer_vk.Track(fence->GetSharedHandle()); if (!command_buffer_vk.EndCommandBuffer()) { @@ -241,6 +219,13 @@ AHBSwapchainImplVK::SubmitSignalForPresentReady( } vk::SubmitInfo submit_info; + vk::PipelineStageFlags wait_stage = + vk::PipelineStageFlagBits::eColorAttachmentOutput; + if (frame_data_[frame_index_].semaphore) { + submit_info.setPWaitSemaphores(&frame_data_[frame_index_].semaphore.get()); + submit_info.setWaitSemaphoreCount(1); + submit_info.setWaitDstStageMask(wait_stage); + } submit_info.setCommandBuffers(command_encoder_vk); auto result = ContextVK::Cast(*context).GetGraphicsQueue()->Submit( @@ -251,7 +236,7 @@ AHBSwapchainImplVK::SubmitSignalForPresentReady( return fence; } -vk::UniqueFence AHBSwapchainImplVK::CreateRenderReadyFence( +vk::UniqueSemaphore AHBSwapchainImplVK::CreateRenderReadySemaphore( const std::shared_ptr& fd) const { if (!fd->is_valid()) { return {}; @@ -265,22 +250,22 @@ vk::UniqueFence AHBSwapchainImplVK::CreateRenderReadyFence( const auto& context_vk = ContextVK::Cast(*context); const auto& device = context_vk.GetDevice(); - auto signal_wait = device.createFenceUnique({}); + auto signal_wait = device.createSemaphoreUnique({}); if (signal_wait.result != vk::Result::eSuccess) { return {}; } - context_vk.SetDebugName(*signal_wait.value, "AHBRenderReadyFence"); + context_vk.SetDebugName(*signal_wait.value, "AHBRenderReadySemaphore"); - vk::ImportFenceFdInfoKHR import_info; - import_info.fence = *signal_wait.value; + vk::ImportSemaphoreFdInfoKHR import_info; + import_info.semaphore = *signal_wait.value; import_info.fd = fd->get(); - import_info.handleType = vk::ExternalFenceHandleTypeFlagBits::eSyncFd; + import_info.handleType = vk::ExternalSemaphoreHandleTypeFlagBits::eSyncFd; // From the spec: Sync FDs can only be imported temporarily. - import_info.flags = vk::FenceImportFlagBitsKHR::eTemporary; + import_info.flags = vk::SemaphoreImportFlagBitsKHR::eTemporary; - const auto import_result = device.importFenceFdKHR(import_info); + const auto import_result = device.importSemaphoreFdKHR(import_info); if (import_result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not import semaphore FD: " @@ -299,10 +284,11 @@ vk::UniqueFence AHBSwapchainImplVK::CreateRenderReadyFence( bool AHBSwapchainImplVK::SubmitWaitForRenderReady( const std::shared_ptr& render_ready_fence, - const std::shared_ptr& texture) const { + const std::shared_ptr& texture) { // If there is no render ready fence, we are already ready to render into // the texture. There is nothing more to do. if (!render_ready_fence || !render_ready_fence->is_valid()) { + frame_data_[frame_index_].semaphore = {}; return true; } @@ -311,20 +297,13 @@ bool AHBSwapchainImplVK::SubmitWaitForRenderReady( return false; } - auto fence = CreateRenderReadyFence(render_ready_fence); - - auto result = ContextVK::Cast(*context).GetDevice().waitForFences( - *fence, // fence - true, // wait all - std::numeric_limits::max() // timeout (ns) - ); - - if (!(result == vk::Result::eSuccess || result == vk::Result::eTimeout)) { - VALIDATION_LOG << "Encountered error while waiting on swapchain image: " - << vk::to_string(result); + auto semaphore = CreateRenderReadySemaphore(render_ready_fence); + if (!semaphore) { return false; } - + // This semaphore will be later used to block the onscreen render pass + // from starting until the system is done reading the onscreen. + frame_data_[frame_index_].semaphore = std::move(semaphore); return true; } diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.h b/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.h index f44e8b15f8..b733fe300c 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.h +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_swapchain_impl_vk.h @@ -17,6 +17,7 @@ #include "impeller/renderer/surface.h" #include "impeller/toolkit/android/hardware_buffer.h" #include "impeller/toolkit/android/surface_control.h" +#include "vulkan/vulkan_handles.hpp" namespace impeller { @@ -104,7 +105,14 @@ class AHBSwapchainImplVK final std::shared_ptr currently_displayed_texture_ IPLR_GUARDED_BY(currently_displayed_texture_mutex_); std::shared_ptr pending_presents_; - std::shared_ptr pending_cmd_buffer_; + + struct FrameData { + std::shared_ptr command_buffer; + vk::UniqueSemaphore semaphore; + }; + + std::array frame_data_; + size_t frame_index_ = 0; bool is_valid_ = false; explicit AHBSwapchainImplVK( @@ -117,12 +125,12 @@ class AHBSwapchainImplVK final bool Present(const AutoSemaSignaler& signaler, const std::shared_ptr& texture); - vk::UniqueFence CreateRenderReadyFence( + vk::UniqueSemaphore CreateRenderReadySemaphore( const std::shared_ptr& fd) const; bool SubmitWaitForRenderReady( const std::shared_ptr& render_ready_fence, - const std::shared_ptr& texture) const; + const std::shared_ptr& texture); std::shared_ptr SubmitSignalForPresentReady( const std::shared_ptr& texture) const; diff --git a/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_texture_pool_vk.cc b/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_texture_pool_vk.cc index 712af01b98..dfeb182eeb 100644 --- a/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_texture_pool_vk.cc +++ b/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/ahb/ahb_texture_pool_vk.cc @@ -17,7 +17,11 @@ AHBTexturePoolVK::AHBTexturePoolVK(std::weak_ptr context, return; } for (auto i = 0u; i < max_entries_; i++) { - pool_.emplace_back(CreateTexture()); + auto texture = CreateTexture(); + if (!texture->IsValid()) { + return; + } + pool_.emplace_back(std::move(texture)); } is_valid_ = true; }