From a45046358e99982b25957d5845d69539ff841305 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 24 Aug 2023 16:30:25 -0700 Subject: [PATCH 1/4] split out setting the layouts from creating the render pass --- .../renderer/backend/vulkan/render_pass_vk.cc | 47 ++++++++++++++----- .../renderer/backend/vulkan/render_pass_vk.h | 3 +- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 6729a4ab09d0b..b6b15790bcf4b 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -97,9 +97,41 @@ static void SetTextureLayout( texture_vk.SetLayoutWithoutEncoding(attachment_desc.finalLayout); } +namespace { +void SetTextureLayouts(const RenderTarget& render_target, + const std::shared_ptr& command_buffer) { + for (const auto& [bind_point, color] : render_target.GetColorAttachments()) { + SetTextureLayout(color, + CreateAttachmentDescription(color, &Attachment::texture), + command_buffer, &Attachment::texture); + if (color.resolve_texture) { + SetTextureLayout( + color, + CreateAttachmentDescription(color, &Attachment::resolve_texture), + command_buffer, &Attachment::resolve_texture); + } + } + + if (auto depth = render_target.GetDepthAttachment(); depth.has_value()) { + SetTextureLayout( + depth.value(), + CreateAttachmentDescription(depth.value(), &Attachment::texture), + command_buffer, &Attachment::texture); + } + + if (auto stencil = render_target.GetStencilAttachment(); + stencil.has_value()) { + SetTextureLayout( + stencil.value(), + CreateAttachmentDescription(stencil.value(), &Attachment::texture), + command_buffer, &Attachment::texture); + } +} +} // namespace + SharedHandleVK RenderPassVK::CreateVKRenderPass( - const ContextVK& context, - const std::shared_ptr& command_buffer) const { + const ContextVK& context) const { + TRACE_EVENT0("impeller", "RenderPassVK::CreateVKRenderPass"); std::vector attachments; std::vector color_refs; @@ -125,15 +157,11 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( vk::ImageLayout::eColorAttachmentOptimal}; attachments.emplace_back( CreateAttachmentDescription(color, &Attachment::texture)); - SetTextureLayout(color, attachments.back(), command_buffer, - &Attachment::texture); if (color.resolve_texture) { resolve_refs[bind_point] = vk::AttachmentReference{ static_cast(attachments.size()), vk::ImageLayout::eGeneral}; attachments.emplace_back( CreateAttachmentDescription(color, &Attachment::resolve_texture)); - SetTextureLayout(color, attachments.back(), command_buffer, - &Attachment::resolve_texture); } } @@ -143,8 +171,6 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( vk::ImageLayout::eDepthStencilAttachmentOptimal}; attachments.emplace_back( CreateAttachmentDescription(depth.value(), &Attachment::texture)); - SetTextureLayout(depth.value(), attachments.back(), command_buffer, - &Attachment::texture); } if (auto stencil = render_target_.GetStencilAttachment(); @@ -154,8 +180,6 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( vk::ImageLayout::eDepthStencilAttachmentOptimal}; attachments.emplace_back( CreateAttachmentDescription(stencil.value(), &Attachment::texture)); - SetTextureLayout(stencil.value(), attachments.back(), command_buffer, - &Attachment::texture); } vk::SubpassDescription subpass_desc; @@ -628,11 +652,12 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { const auto& target_size = render_target_.GetRenderTargetSize(); - auto render_pass = CreateVKRenderPass(vk_context, command_buffer); + auto render_pass = CreateVKRenderPass(vk_context); if (!render_pass) { VALIDATION_LOG << "Could not create renderpass."; return false; } + SetTextureLayouts(render_target_, command_buffer); auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass); if (!framebuffer) { diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 818bb4c6003af..273c9089aad3f 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -45,8 +45,7 @@ class RenderPassVK final : public RenderPass { bool OnEncodeCommands(const Context& context) const override; SharedHandleVK CreateVKRenderPass( - const ContextVK& context, - const std::shared_ptr& command_buffer) const; + const ContextVK& context) const; SharedHandleVK CreateVKFramebuffer( const ContextVK& context, From 7c540162bc1c80df9d2292ffb2c11d419bd59fde Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 18 Aug 2023 13:51:33 -0700 Subject: [PATCH 2/4] [Impeller] started recycling vkRenderPass and vkFramebuffer --- .../backend/vulkan/background_context_vk.h | 60 +++++++++++++++ impeller/renderer/backend/vulkan/context_vk.h | 74 +++++++++++++++++++ .../renderer/backend/vulkan/render_pass_vk.cc | 40 ++++++---- .../backend/vulkan/thread_context_vk.h | 14 ++++ 4 files changed, 175 insertions(+), 13 deletions(-) create mode 100644 impeller/renderer/backend/vulkan/background_context_vk.h create mode 100644 impeller/renderer/backend/vulkan/thread_context_vk.h diff --git a/impeller/renderer/backend/vulkan/background_context_vk.h b/impeller/renderer/backend/vulkan/background_context_vk.h new file mode 100644 index 0000000000000..a7d675e9e3f81 --- /dev/null +++ b/impeller/renderer/backend/vulkan/background_context_vk.h @@ -0,0 +1,60 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include "impeller/renderer/backend/vulkan/context_vk.h" + +namespace impeller { + +class BackgroundContextVK : public ContextVK { +public: + virtual BackendType GetBackendType() const { + + } + + virtual std::string DescribeGpuModel() const = 0; + + virtual bool IsValid() const = 0; + + virtual const std::shared_ptr& GetCapabilities() + const = 0; + + virtual bool UpdateOffscreenLayerPixelFormat(PixelFormat format); + + virtual std::shared_ptr GetResourceAllocator() const = 0; + + virtual std::shared_ptr GetShaderLibrary() const = 0; + + virtual std::shared_ptr GetSamplerLibrary() const = 0; + + virtual std::shared_ptr GetPipelineLibrary() const = 0; + + virtual std::shared_ptr CreateCommandBuffer() const = 0; + + virtual void Shutdown() = 0; + + virtual std::shared_ptr GetDeviceHolder() const = 0; + + virtual vk::Instance GetInstance() const = 0; + + virtual const vk::Device& GetDevice() const = 0; + + virtual const std::shared_ptr + GetConcurrentWorkerTaskRunner() const = 0; + + virtual std::shared_ptr CreateSurfaceContext() = 0; + + virtual const std::shared_ptr& GetGraphicsQueue() const = 0; + + virtual vk::PhysicalDevice GetPhysicalDevice() const = 0; + + virtual std::shared_ptr GetFenceWaiter() const = 0; + + virtual std::shared_ptr GetResourceManager() const = 0; + +private: + std::weak_ptr root_context_; +}; +} diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 43d4531ff5fcd..0ccfc393cc5bf 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -17,6 +17,7 @@ #include "impeller/renderer/backend/vulkan/queue_vk.h" #include "impeller/renderer/backend/vulkan/sampler_library_vk.h" #include "impeller/renderer/backend/vulkan/shader_library_vk.h" +#include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/capabilities.h" #include "impeller/renderer/context.h" @@ -31,6 +32,71 @@ class FenceWaiterVK; class ResourceManagerVK; class SurfaceContextVK; +/// @brief Ring buffer for one writer thread and one reader thread. +template +class RingBuffer { + public: + RingBuffer(int32_t size) : read_(0), write_(0), buffer_(size) {} + + std::optional Read() { + int32_t current_read = read_.load(); + if (current_read == write_.load()) { + return {}; + } else { + int32_t next_read = (current_read + 1) % buffer_.size(); + read_.store(next_read); + return buffer_[current_read]; + } + } + + bool Write(const T& value) { + int32_t current_write = write_.load(); + int32_t next_write = (current_write + 1) % buffer_.size(); + if (next_write == read_.load()) { // Buffer full + return false; + } + buffer_[current_write] = value; + write_.store(next_write); + return true; + } + + private: + std::atomic read_; + std::atomic write_; + std::vector buffer_; +}; + +template +class RingBufferItem : public SharedObjectVK { + public: + RingBufferItem(std::weak_ptr context, + RingBuffer* ring_buffer, + std::optional value) + : context_(context), ring_buffer_(ring_buffer), value_(value) {} + + virtual ~RingBufferItem() { + auto context = context_.lock(); + if (value_ && context) { + bool did_write = ring_buffer_->Write(value_.value()); + FML_CHECK(did_write); + } + } + + std::optional& GetValue() { return value_; } + + private: + FML_DISALLOW_COPY_AND_ASSIGN(RingBufferItem); + + std::weak_ptr context_; + RingBuffer* ring_buffer_; + std::optional value_; +}; + +struct RenderPassFrameBufferPair { + SharedHandleVK render_pass; + SharedHandleVK framebuffer; +}; + class ContextVK final : public Context, public BackendCast, public std::enable_shared_from_this { @@ -135,6 +201,12 @@ class ContextVK final : public Context, std::shared_ptr GetResourceManager() const; + std::shared_ptr> + GetRenderPassFrameBufferPair() const { + return std::make_shared>( + shared_from_this(), &render_pass_pool_, render_pass_pool_.Read()); + } + private: struct DeviceHolderImpl : public DeviceHolder { // |DeviceHolder| @@ -162,6 +234,8 @@ class ContextVK final : public Context, std::string device_name_; std::shared_ptr raster_message_loop_; const uint64_t hash_; + mutable RingBuffer render_pass_pool_ = + RingBuffer(10); bool is_valid_ = false; diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index b6b15790bcf4b..f0761dd16c6f0 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -652,28 +652,42 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { const auto& target_size = render_target_.GetRenderTargetSize(); - auto render_pass = CreateVKRenderPass(vk_context); - if (!render_pass) { - VALIDATION_LOG << "Could not create renderpass."; - return false; - } - SetTextureLayouts(render_target_, command_buffer); + std::shared_ptr> + render_pass_frame_buffer_pair = vk_context.GetRenderPassFrameBufferPair(); - auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass); - if (!framebuffer) { - VALIDATION_LOG << "Could not create framebuffer."; - return false; + if (!render_pass_frame_buffer_pair->GetValue()) { + auto render_pass = CreateVKRenderPass(vk_context); + if (!render_pass) { + VALIDATION_LOG << "Could not create renderpass."; + return false; + } + + auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass); + if (!framebuffer) { + VALIDATION_LOG << "Could not create framebuffer."; + return false; + } + + RenderPassFrameBufferPair pair = RenderPassFrameBufferPair{ + .render_pass = render_pass, + .framebuffer = framebuffer, + }; + + render_pass_frame_buffer_pair->GetValue() = std::move(pair); } + SetTextureLayouts(render_target_, command_buffer); - if (!encoder->Track(framebuffer) || !encoder->Track(render_pass)) { + if (!encoder->Track(render_pass_frame_buffer_pair)) { return false; } auto clear_values = GetVKClearValues(render_target_); vk::RenderPassBeginInfo pass_info; - pass_info.renderPass = *render_pass; - pass_info.framebuffer = *framebuffer; + pass_info.renderPass = + *render_pass_frame_buffer_pair->GetValue()->render_pass; + pass_info.framebuffer = + *render_pass_frame_buffer_pair->GetValue()->framebuffer; pass_info.renderArea.extent.width = static_cast(target_size.width); pass_info.renderArea.extent.height = static_cast(target_size.height); diff --git a/impeller/renderer/backend/vulkan/thread_context_vk.h b/impeller/renderer/backend/vulkan/thread_context_vk.h new file mode 100644 index 0000000000000..5666e6950a6e1 --- /dev/null +++ b/impeller/renderer/backend/vulkan/thread_context_vk.h @@ -0,0 +1,14 @@ +#pragma once + +#include + +namespace impeller { +class ThreadContextVK { + public: + ThreadContextVK(std::weak_ptr context) : context_(context) {} + + private: + std::weak_ptr context_; + std::shared_ptr command_pool_; +}; +} // namespace impeller \ No newline at end of file From c81691e123b11f1030ba6ae3fc8fd3a900891c35 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 25 Aug 2023 14:23:40 -0700 Subject: [PATCH 3/4] removed stray files --- .../backend/vulkan/background_context_vk.h | 60 ------------------- .../backend/vulkan/thread_context_vk.h | 14 ----- 2 files changed, 74 deletions(-) delete mode 100644 impeller/renderer/backend/vulkan/background_context_vk.h delete mode 100644 impeller/renderer/backend/vulkan/thread_context_vk.h diff --git a/impeller/renderer/backend/vulkan/background_context_vk.h b/impeller/renderer/backend/vulkan/background_context_vk.h deleted file mode 100644 index a7d675e9e3f81..0000000000000 --- a/impeller/renderer/backend/vulkan/background_context_vk.h +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#pragma once - -#include "impeller/renderer/backend/vulkan/context_vk.h" - -namespace impeller { - -class BackgroundContextVK : public ContextVK { -public: - virtual BackendType GetBackendType() const { - - } - - virtual std::string DescribeGpuModel() const = 0; - - virtual bool IsValid() const = 0; - - virtual const std::shared_ptr& GetCapabilities() - const = 0; - - virtual bool UpdateOffscreenLayerPixelFormat(PixelFormat format); - - virtual std::shared_ptr GetResourceAllocator() const = 0; - - virtual std::shared_ptr GetShaderLibrary() const = 0; - - virtual std::shared_ptr GetSamplerLibrary() const = 0; - - virtual std::shared_ptr GetPipelineLibrary() const = 0; - - virtual std::shared_ptr CreateCommandBuffer() const = 0; - - virtual void Shutdown() = 0; - - virtual std::shared_ptr GetDeviceHolder() const = 0; - - virtual vk::Instance GetInstance() const = 0; - - virtual const vk::Device& GetDevice() const = 0; - - virtual const std::shared_ptr - GetConcurrentWorkerTaskRunner() const = 0; - - virtual std::shared_ptr CreateSurfaceContext() = 0; - - virtual const std::shared_ptr& GetGraphicsQueue() const = 0; - - virtual vk::PhysicalDevice GetPhysicalDevice() const = 0; - - virtual std::shared_ptr GetFenceWaiter() const = 0; - - virtual std::shared_ptr GetResourceManager() const = 0; - -private: - std::weak_ptr root_context_; -}; -} diff --git a/impeller/renderer/backend/vulkan/thread_context_vk.h b/impeller/renderer/backend/vulkan/thread_context_vk.h deleted file mode 100644 index 5666e6950a6e1..0000000000000 --- a/impeller/renderer/backend/vulkan/thread_context_vk.h +++ /dev/null @@ -1,14 +0,0 @@ -#pragma once - -#include - -namespace impeller { -class ThreadContextVK { - public: - ThreadContextVK(std::weak_ptr context) : context_(context) {} - - private: - std::weak_ptr context_; - std::shared_ptr command_pool_; -}; -} // namespace impeller \ No newline at end of file From 90d7f264273e28ab9ae8c5752f7276949e59b063 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 25 Aug 2023 15:22:44 -0700 Subject: [PATCH 4/4] added traces and tried to fix layout problems --- .../renderer/backend/vulkan/render_pass_vk.cc | 28 ++++++++++++++++++- .../backend/vulkan/swapchain_impl_vk.cc | 2 ++ .../backend/vulkan/texture_source_vk.cc | 7 +++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index f0761dd16c6f0..3fe270003e806 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -77,8 +77,10 @@ static void SetTextureLayout( return; } const auto& texture_vk = TextureVK::Cast(*texture); + auto current_layout = texture_vk.GetLayout(); - if (attachment_desc.initialLayout == vk::ImageLayout::eGeneral) { + if (current_layout != vk::ImageLayout::ePresentSrcKHR && + current_layout != vk::ImageLayout::eUndefined) { BarrierVK barrier; barrier.new_layout = vk::ImageLayout::eGeneral; barrier.cmd_buffer = command_buffer->GetEncoder()->GetCommandBuffer(); @@ -200,6 +202,27 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( return {}; } context.SetDebugName(pass.get(), debug_label_.c_str()); + + std::stringstream ss; + ss << debug_label_ << " " << pass.get() << " " << std::endl; + for (const vk::AttachmentDescription& desc : attachments) { + ss << " attachment" + << ", initial: " << vk::to_string(desc.initialLayout) + << ", final layout: " << vk::to_string(desc.finalLayout) << std::endl; + } + + for (const vk::AttachmentReference& ref : color_refs) { + ss << " color ref: " << vk::to_string(ref.layout); + } + + for (const vk::AttachmentReference& ref : resolve_refs) { + ss << ", resolve ref: " << vk::to_string(ref.layout); + } + + ss << ", depth stencil ref: " << vk::to_string(depth_stencil_ref.layout); + + FML_LOG(ERROR) << ss.str(); + return MakeSharedVK(std::move(pass)); } @@ -675,7 +698,10 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { render_pass_frame_buffer_pair->GetValue() = std::move(pair); } + FML_LOG(ERROR) << render_target_.ToString() << " " + << render_pass_frame_buffer_pair->GetValue()->render_pass; SetTextureLayouts(render_target_, command_buffer); + FML_LOG(ERROR) << "end SetTextureLayouts"; if (!encoder->Track(render_pass_frame_buffer_pair)) { return false; diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index c5058591d1c86..12a98f895d442 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -327,6 +327,7 @@ std::shared_ptr SwapchainImplVK::GetContext() const { SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { auto context_strong = context_.lock(); + FML_LOG(ERROR) << "SwapchainImplVK::AcquireNextDrawable"; if (!context_strong) { return {}; } @@ -506,6 +507,7 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, } FML_UNREACHABLE(); }); + FML_LOG(ERROR) << "SwapchainImplVK::Present"; return true; } diff --git a/impeller/renderer/backend/vulkan/texture_source_vk.cc b/impeller/renderer/backend/vulkan/texture_source_vk.cc index 4c8f892441be9..02d169780fa20 100644 --- a/impeller/renderer/backend/vulkan/texture_source_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_source_vk.cc @@ -21,6 +21,8 @@ vk::ImageLayout TextureSourceVK::GetLayout() const { vk::ImageLayout TextureSourceVK::SetLayoutWithoutEncoding( vk::ImageLayout layout) const { + FML_LOG(ERROR) << "SetLayoutWithoutEncoding " << GetImage() << " " + << vk::to_string(layout); WriterLock lock(layout_mutex_); const auto old_layout = layout_; layout_ = layout; @@ -32,6 +34,11 @@ bool TextureSourceVK::SetLayout(const BarrierVK& barrier) const { if (barrier.new_layout == old_layout) { return true; } + FML_LOG(ERROR) << "SetLayout " << GetImage() << " " + << "src:" << vk::to_string(barrier.src_access) << " " + << "dst:" << vk::to_string(barrier.dst_access) << " " + << vk::to_string(old_layout) << " " + << vk::to_string(barrier.new_layout); vk::ImageMemoryBarrier image_barrier; image_barrier.srcAccessMask = barrier.src_access;