From 7fcb5e5d7f3587c642d0432bebbd69bb277a5363 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 8 Aug 2023 20:25:41 -0700 Subject: [PATCH 01/20] [Impeller] less hacky but still hacky cache. --- impeller/core/allocator.cc | 25 ++++- impeller/core/allocator.h | 27 ++++- impeller/core/texture_descriptor.h | 1 + .../renderer/backend/vulkan/allocator_vk.cc | 1 + .../renderer/backend/vulkan/surface_vk.cc | 2 + .../backend/vulkan/swapchain_impl_vk.cc | 106 +++++++++--------- 6 files changed, 106 insertions(+), 56 deletions(-) diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index 075a738fe5b4f..a6ee8568eaab2 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -53,6 +53,29 @@ std::shared_ptr Allocator::CreateTexture( << " exceeds maximum supported size of " << max_size; return nullptr; } + if (desc.ignore_cache) { + return OnCreateTexture(desc); + } + if (desc.storage_mode != StorageMode::kHostVisible) { + for (auto& td : data_to_recycle_) { + const auto other_desc = td.texture->GetTextureDescriptor(); + if (!td.used_this_frame && + desc.size.width == other_desc.size.width && + desc.size.height == other_desc.size.height && + desc.storage_mode == other_desc.storage_mode && + desc.format == other_desc.format && + desc.usage == other_desc.usage && + desc.sample_count == other_desc.sample_count && + desc.type == other_desc.type) { + td.used_this_frame = true; + return td.texture; + } + } + auto result = OnCreateTexture(desc); + data_to_recycle_.push_back({.used_this_frame = true, + .texture = result}); + return result; + } return OnCreateTexture(desc); } @@ -61,6 +84,4 @@ uint16_t Allocator::MinimumBytesPerRow(PixelFormat format) const { return BytesPerPixelForPixelFormat(format); } -void Allocator::DidAcquireSurfaceFrame() {} - } // namespace impeller diff --git a/impeller/core/allocator.h b/impeller/core/allocator.h index 62b86b72b047f..0571ff4827549 100644 --- a/impeller/core/allocator.h +++ b/impeller/core/allocator.h @@ -10,6 +10,7 @@ #include "flutter/fml/mapping.h" #include "impeller/core/device_buffer_descriptor.h" #include "impeller/core/texture_descriptor.h" +#include "impeller/geometry/size.h" namespace impeller { @@ -47,7 +48,24 @@ class Allocator { /// @brief Increment an internal frame used to cycle through a ring buffer of /// allocation pools. - virtual void DidAcquireSurfaceFrame(); + virtual void DidAcquireSurfaceFrame() { + for (auto& td : data_to_recycle_) { + td.used_this_frame = false; + } + } + + virtual void DidFinishSurfaceFrame() { + std::vector retain; + + for (auto td : data_to_recycle_) { + if (td.used_this_frame) { + retain.push_back(td); + } + } + data_to_recycle_.clear(); + data_to_recycle_.insert(data_to_recycle_.end(), retain.begin(), + retain.end()); + } protected: Allocator(); @@ -59,6 +77,13 @@ class Allocator { const TextureDescriptor& desc) = 0; private: + struct TextureData { + bool used_this_frame; + std::shared_ptr texture; + }; + + std::vector data_to_recycle_; + FML_DISALLOW_COPY_AND_ASSIGN(Allocator); }; diff --git a/impeller/core/texture_descriptor.h b/impeller/core/texture_descriptor.h index 7d99d20408da5..4e7587014d2e1 100644 --- a/impeller/core/texture_descriptor.h +++ b/impeller/core/texture_descriptor.h @@ -46,6 +46,7 @@ struct TextureDescriptor { static_cast(TextureUsage::kShaderRead); SampleCount sample_count = SampleCount::kCount1; CompressionType compression_type = CompressionType::kLossless; + bool ignore_cache = false; constexpr size_t GetByteSizeOfBaseMipLevel() const { if (!IsValid()) { diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 2720a81587060..07767e9109260 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -434,6 +434,7 @@ std::shared_ptr AllocatorVK::OnCreateTexture( void AllocatorVK::DidAcquireSurfaceFrame() { frame_count_++; raster_thread_id_ = std::this_thread::get_id(); + Allocator::DidAcquireSurfaceFrame(); } // |Allocator| diff --git a/impeller/renderer/backend/vulkan/surface_vk.cc b/impeller/renderer/backend/vulkan/surface_vk.cc index 95846affdc1c3..d2f56d59f2465 100644 --- a/impeller/renderer/backend/vulkan/surface_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_vk.cc @@ -25,6 +25,7 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( msaa_tex_desc.format = swapchain_image->GetPixelFormat(); msaa_tex_desc.size = swapchain_image->GetSize(); msaa_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); + msaa_tex_desc.ignore_cache = true; std::shared_ptr msaa_tex; if (!swapchain_image->HasMSAATexture()) { @@ -47,6 +48,7 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( static_cast(TextureUsage::kRenderTarget); resolve_tex_desc.sample_count = SampleCount::kCount1; resolve_tex_desc.storage_mode = StorageMode::kDevicePrivate; + resolve_tex_desc.ignore_cache = true; std::shared_ptr resolve_tex = std::make_shared(context, // diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 504f2a538bce1..5e267ff09e802 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -390,66 +390,66 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, if (!context_strong) { return false; } - const auto& context = ContextVK::Cast(*context_strong); + const auto& sync = synchronizers_[current_frame_]; + + //---------------------------------------------------------------------------- + /// Transition the image to color-attachment-optimal. + /// + sync->final_cmd_buffer = context.CreateCommandBuffer(); + if (!sync->final_cmd_buffer) { + return false; + } + + auto vk_final_cmd_buffer = CommandBufferVK::Cast(*sync->final_cmd_buffer) + .GetEncoder() + ->GetCommandBuffer(); + { + BarrierVK barrier; + barrier.new_layout = vk::ImageLayout::ePresentSrcKHR; + barrier.cmd_buffer = vk_final_cmd_buffer; + barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite; + barrier.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput; + barrier.dst_access = {}; + barrier.dst_stage = vk::PipelineStageFlagBits::eBottomOfPipe; + + if (!image->SetLayout(barrier)) { + return false; + } + + if (vk_final_cmd_buffer.end() != vk::Result::eSuccess) { + return false; + } + } + + //---------------------------------------------------------------------------- + /// Signal that the presentation semaphore is ready. + /// + { + vk::SubmitInfo submit_info; + vk::PipelineStageFlags wait_stage = + vk::PipelineStageFlagBits::eColorAttachmentOutput; + submit_info.setWaitDstStageMask(wait_stage); + submit_info.setWaitSemaphores(*sync->render_ready); + submit_info.setSignalSemaphores(*sync->present_ready); + submit_info.setCommandBuffers(vk_final_cmd_buffer); + auto result = + context.GetGraphicsQueue()->Submit(submit_info, *sync->acquire); + if (result != vk::Result::eSuccess) { + VALIDATION_LOG << "Could not wait on render semaphore: " + << vk::to_string(result); + return false; + } + } + context_strong->GetResourceAllocator()->DidFinishSurfaceFrame(); + context.GetConcurrentWorkerTaskRunner()->PostTask( - [&, index, image, current_frame = current_frame_] { + [&, index, image] { auto context_strong = context_.lock(); if (!context_strong) { return; } const auto& context = ContextVK::Cast(*context_strong); - const auto& sync = synchronizers_[current_frame]; - - //---------------------------------------------------------------------------- - /// Transition the image to color-attachment-optimal. - /// - sync->final_cmd_buffer = context.CreateCommandBuffer(); - if (!sync->final_cmd_buffer) { - return; - } - - auto vk_final_cmd_buffer = - CommandBufferVK::Cast(*sync->final_cmd_buffer) - .GetEncoder() - ->GetCommandBuffer(); - { - BarrierVK barrier; - barrier.new_layout = vk::ImageLayout::ePresentSrcKHR; - barrier.cmd_buffer = vk_final_cmd_buffer; - barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite; - barrier.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput; - barrier.dst_access = {}; - barrier.dst_stage = vk::PipelineStageFlagBits::eBottomOfPipe; - - if (!image->SetLayout(barrier)) { - return; - } - - if (vk_final_cmd_buffer.end() != vk::Result::eSuccess) { - return; - } - } - - //---------------------------------------------------------------------------- - /// Signal that the presentation semaphore is ready. - /// - { - vk::SubmitInfo submit_info; - vk::PipelineStageFlags wait_stage = - vk::PipelineStageFlagBits::eColorAttachmentOutput; - submit_info.setWaitDstStageMask(wait_stage); - submit_info.setWaitSemaphores(*sync->render_ready); - submit_info.setSignalSemaphores(*sync->present_ready); - submit_info.setCommandBuffers(vk_final_cmd_buffer); - auto result = - context.GetGraphicsQueue()->Submit(submit_info, *sync->acquire); - if (result != vk::Result::eSuccess) { - VALIDATION_LOG << "Could not wait on render semaphore: " - << vk::to_string(result); - return; - } - } //---------------------------------------------------------------------------- /// Present the image. From d7353507537ed86577d847bc19ae716e7f1f6700 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 8 Aug 2023 20:26:23 -0700 Subject: [PATCH 02/20] ++ --- impeller/core/allocator.cc | 11 +-- .../backend/vulkan/swapchain_impl_vk.cc | 76 +++++++++---------- 2 files changed, 41 insertions(+), 46 deletions(-) diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index a6ee8568eaab2..f0a15996b6899 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -54,17 +54,15 @@ std::shared_ptr Allocator::CreateTexture( return nullptr; } if (desc.ignore_cache) { - return OnCreateTexture(desc); + return OnCreateTexture(desc); } if (desc.storage_mode != StorageMode::kHostVisible) { for (auto& td : data_to_recycle_) { const auto other_desc = td.texture->GetTextureDescriptor(); - if (!td.used_this_frame && - desc.size.width == other_desc.size.width && + if (!td.used_this_frame && desc.size.width == other_desc.size.width && desc.size.height == other_desc.size.height && desc.storage_mode == other_desc.storage_mode && - desc.format == other_desc.format && - desc.usage == other_desc.usage && + desc.format == other_desc.format && desc.usage == other_desc.usage && desc.sample_count == other_desc.sample_count && desc.type == other_desc.type) { td.used_this_frame = true; @@ -72,8 +70,7 @@ std::shared_ptr Allocator::CreateTexture( } } auto result = OnCreateTexture(desc); - data_to_recycle_.push_back({.used_this_frame = true, - .texture = result}); + data_to_recycle_.push_back({.used_this_frame = true, .texture = result}); return result; } diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 5e267ff09e802..81d531875b3d3 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -443,45 +443,43 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, } context_strong->GetResourceAllocator()->DidFinishSurfaceFrame(); - context.GetConcurrentWorkerTaskRunner()->PostTask( - [&, index, image] { - auto context_strong = context_.lock(); - if (!context_strong) { - return; - } - const auto& context = ContextVK::Cast(*context_strong); - - //---------------------------------------------------------------------------- - /// Present the image. - /// - uint32_t indices[] = {static_cast(index)}; - - vk::PresentInfoKHR present_info; - present_info.setSwapchains(*swapchain_); - present_info.setImageIndices(indices); - present_info.setWaitSemaphores(*sync->present_ready); - - switch (auto result = present_queue_.presentKHR(present_info)) { - case vk::Result::eErrorOutOfDateKHR: - // Caller will recreate the impl on acquisition, not submission. - [[fallthrough]]; - case vk::Result::eErrorSurfaceLostKHR: - // Vulkan guarantees that the set of queue operations will still - // complete successfully. - [[fallthrough]]; - case vk::Result::eSuccess: - is_rotated_ = false; - return; - case vk::Result::eSuboptimalKHR: - is_rotated_ = true; - return; - default: - VALIDATION_LOG << "Could not present queue: " - << vk::to_string(result); - return; - } - FML_UNREACHABLE(); - }); + context.GetConcurrentWorkerTaskRunner()->PostTask([&, index, image] { + auto context_strong = context_.lock(); + if (!context_strong) { + return; + } + const auto& context = ContextVK::Cast(*context_strong); + + //---------------------------------------------------------------------------- + /// Present the image. + /// + uint32_t indices[] = {static_cast(index)}; + + vk::PresentInfoKHR present_info; + present_info.setSwapchains(*swapchain_); + present_info.setImageIndices(indices); + present_info.setWaitSemaphores(*sync->present_ready); + + switch (auto result = present_queue_.presentKHR(present_info)) { + case vk::Result::eErrorOutOfDateKHR: + // Caller will recreate the impl on acquisition, not submission. + [[fallthrough]]; + case vk::Result::eErrorSurfaceLostKHR: + // Vulkan guarantees that the set of queue operations will still + // complete successfully. + [[fallthrough]]; + case vk::Result::eSuccess: + is_rotated_ = false; + return; + case vk::Result::eSuboptimalKHR: + is_rotated_ = true; + return; + default: + VALIDATION_LOG << "Could not present queue: " << vk::to_string(result); + return; + } + FML_UNREACHABLE(); + }); return true; } From 6c62257365d1cd371c57a7dd8eb1beb8f6c04148 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Aug 2023 17:28:46 -0700 Subject: [PATCH 03/20] add memory barriers for re-used textures --- impeller/core/allocator.cc | 6 ++- impeller/core/allocator.h | 5 +++ impeller/renderer/backend/vulkan/formats_vk.h | 2 +- .../renderer/backend/vulkan/render_pass_vk.cc | 38 +++++++++++++++---- .../renderer/backend/vulkan/render_pass_vk.h | 3 +- 5 files changed, 44 insertions(+), 10 deletions(-) diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index f0a15996b6899..0bde0865a5555 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -6,6 +6,7 @@ #include "impeller/base/validation.h" #include "impeller/core/device_buffer.h" +#include "impeller/core/formats.h" #include "impeller/core/range.h" namespace impeller { @@ -53,18 +54,21 @@ std::shared_ptr Allocator::CreateTexture( << " exceeds maximum supported size of " << max_size; return nullptr; } + total_count_++; if (desc.ignore_cache) { return OnCreateTexture(desc); } if (desc.storage_mode != StorageMode::kHostVisible) { for (auto& td : data_to_recycle_) { const auto other_desc = td.texture->GetTextureDescriptor(); - if (!td.used_this_frame && desc.size.width == other_desc.size.width && + if (!td.used_this_frame && + desc.size.width == other_desc.size.width && desc.size.height == other_desc.size.height && desc.storage_mode == other_desc.storage_mode && desc.format == other_desc.format && desc.usage == other_desc.usage && desc.sample_count == other_desc.sample_count && desc.type == other_desc.type) { + hit_count_++; td.used_this_frame = true; return td.texture; } diff --git a/impeller/core/allocator.h b/impeller/core/allocator.h index 0571ff4827549..9a8a9fb85bcdf 100644 --- a/impeller/core/allocator.h +++ b/impeller/core/allocator.h @@ -65,6 +65,9 @@ class Allocator { data_to_recycle_.clear(); data_to_recycle_.insert(data_to_recycle_.end(), retain.begin(), retain.end()); + FML_LOG(ERROR) << "" << hit_count_ << " / " << total_count_; + hit_count_ = 0; + total_count_ = 0; } protected: @@ -83,6 +86,8 @@ class Allocator { }; std::vector data_to_recycle_; + size_t hit_count_ = 0; + size_t total_count_ = 0; FML_DISALLOW_COPY_AND_ASSIGN(Allocator); }; diff --git a/impeller/renderer/backend/vulkan/formats_vk.h b/impeller/renderer/backend/vulkan/formats_vk.h index 634f647f0f533..724bb4996718f 100644 --- a/impeller/renderer/backend/vulkan/formats_vk.h +++ b/impeller/renderer/backend/vulkan/formats_vk.h @@ -465,7 +465,7 @@ constexpr vk::AttachmentDescription CreateAttachmentDescription( switch (kind) { case AttachmentKind::kColor: vk_attachment.initialLayout = current_layout; - vk_attachment.finalLayout = vk::ImageLayout::eGeneral; + vk_attachment.finalLayout = vk::ImageLayout::eColorAttachmentOptimal; break; case AttachmentKind::kDepth: case AttachmentKind::kStencil: diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index c7d84b4afea9c..491303b53693d 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -15,6 +15,7 @@ #include "impeller/core/formats.h" #include "impeller/core/sampler.h" #include "impeller/core/shader_types.h" +#include "impeller/renderer/backend/vulkan/barrier_vk.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" @@ -24,11 +25,13 @@ #include "impeller/renderer/backend/vulkan/sampler_vk.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" +#include "vulkan/vulkan_to_string.hpp" namespace impeller { static vk::AttachmentDescription CreateAttachmentDescription( const Attachment& attachment, + const std::shared_ptr& command_buffer, bool resolve_texture = false) { const auto& texture = resolve_texture ? attachment.resolve_texture : attachment.texture; @@ -37,7 +40,7 @@ static vk::AttachmentDescription CreateAttachmentDescription( } const auto& texture_vk = TextureVK::Cast(*texture); const auto& desc = texture->GetTextureDescriptor(); - const auto current_layout = texture_vk.GetLayout(); + auto current_layout = texture_vk.GetLayout(); auto load_action = attachment.load_action; auto store_action = attachment.store_action; @@ -52,6 +55,22 @@ static vk::AttachmentDescription CreateAttachmentDescription( store_action = StoreAction::kStore; } + 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(); + barrier.src_access = vk::AccessFlagBits::eShaderRead; + barrier.src_stage = vk::PipelineStageFlagBits::eFragmentShader; + barrier.dst_access = vk::AccessFlagBits::eColorAttachmentWrite | + vk::AccessFlagBits::eTransferWrite; + barrier.dst_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | + vk::PipelineStageFlagBits::eTransfer; + + texture_vk.SetLayout(barrier); + current_layout = vk::ImageLayout::eGeneral; + } + const auto attachment_desc = CreateAttachmentDescription(desc.format, // desc.sample_count, // @@ -68,7 +87,8 @@ static vk::AttachmentDescription CreateAttachmentDescription( } SharedHandleVK RenderPassVK::CreateVKRenderPass( - const ContextVK& context) const { + const ContextVK& context, + const std::shared_ptr& command_buffer) const { std::vector attachments; std::vector color_refs; @@ -92,11 +112,13 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( color_refs[bind_point] = vk::AttachmentReference{static_cast(attachments.size()), vk::ImageLayout::eColorAttachmentOptimal}; - attachments.emplace_back(CreateAttachmentDescription(color)); + attachments.emplace_back( + CreateAttachmentDescription(color, command_buffer)); if (color.resolve_texture) { resolve_refs[bind_point] = vk::AttachmentReference{ static_cast(attachments.size()), vk::ImageLayout::eGeneral}; - attachments.emplace_back(CreateAttachmentDescription(color, true)); + attachments.emplace_back( + CreateAttachmentDescription(color, command_buffer, true)); } } @@ -104,7 +126,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( depth_stencil_ref = vk::AttachmentReference{ static_cast(attachments.size()), vk::ImageLayout::eDepthStencilAttachmentOptimal}; - attachments.emplace_back(CreateAttachmentDescription(depth.value())); + attachments.emplace_back( + CreateAttachmentDescription(depth.value(), command_buffer)); } if (auto stencil = render_target_.GetStencilAttachment(); @@ -112,7 +135,8 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( depth_stencil_ref = vk::AttachmentReference{ static_cast(attachments.size()), vk::ImageLayout::eDepthStencilAttachmentOptimal}; - attachments.emplace_back(CreateAttachmentDescription(stencil.value())); + attachments.emplace_back( + CreateAttachmentDescription(stencil.value(), command_buffer)); } vk::SubpassDescription subpass_desc; @@ -592,7 +616,7 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { const auto& target_size = render_target_.GetRenderTargetSize(); - auto render_pass = CreateVKRenderPass(vk_context); + auto render_pass = CreateVKRenderPass(vk_context, command_buffer); if (!render_pass) { VALIDATION_LOG << "Could not create renderpass."; return false; diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 273c9089aad3f..818bb4c6003af 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -45,7 +45,8 @@ class RenderPassVK final : public RenderPass { bool OnEncodeCommands(const Context& context) const override; SharedHandleVK CreateVKRenderPass( - const ContextVK& context) const; + const ContextVK& context, + const std::shared_ptr& command_buffer) const; SharedHandleVK CreateVKFramebuffer( const ContextVK& context, From 9cbe8a2d80714cb32e8f1b498dac03bd1d75b668 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Aug 2023 17:29:48 -0700 Subject: [PATCH 04/20] ++ --- impeller/core/allocator.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index 0bde0865a5555..7c7fdbe79ffca 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -61,8 +61,7 @@ std::shared_ptr Allocator::CreateTexture( if (desc.storage_mode != StorageMode::kHostVisible) { for (auto& td : data_to_recycle_) { const auto other_desc = td.texture->GetTextureDescriptor(); - if (!td.used_this_frame && - desc.size.width == other_desc.size.width && + if (!td.used_this_frame && desc.size.width == other_desc.size.width && desc.size.height == other_desc.size.height && desc.storage_mode == other_desc.storage_mode && desc.format == other_desc.format && desc.usage == other_desc.usage && From 7e964604dd28d5fd13d201b8ad6110a1ba25be0c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 9 Aug 2023 18:08:57 -0700 Subject: [PATCH 05/20] fix metal --- impeller/renderer/backend/metal/surface_mtl.mm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm index 526e66f4c5ff5..179dedfa6e5b4 100644 --- a/impeller/renderer/backend/metal/surface_mtl.mm +++ b/impeller/renderer/backend/metal/surface_mtl.mm @@ -37,6 +37,7 @@ VALIDATION_LOG << "Could not acquire current drawable."; return nullptr; } + context->GetResourceAllocator()->DidAcquireSurfaceFrame(); return current_drawable; } @@ -224,6 +225,7 @@ if (!context) { return false; } + context->GetResourceAllocator()->DidFinishSurfaceFrame(); if (requires_blit_) { if (!(source_texture_ && destination_texture_)) { From 9bbfa676eabf972628101432bcbaf48985ab5af4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 10 Aug 2023 12:26:36 -0700 Subject: [PATCH 06/20] fix deallocation --- impeller/core/allocator.cc | 4 +++- impeller/core/allocator.h | 14 ++++++++------ impeller/renderer/backend/vulkan/allocator_vk.cc | 6 +++++- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index 7c7fdbe79ffca..ad79242535ac4 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -58,7 +58,9 @@ std::shared_ptr Allocator::CreateTexture( if (desc.ignore_cache) { return OnCreateTexture(desc); } - if (desc.storage_mode != StorageMode::kHostVisible) { + if (desc.storage_mode != StorageMode::kHostVisible && + (desc.usage & + static_cast(TextureUsage::kRenderTarget))) { for (auto& td : data_to_recycle_) { const auto other_desc = td.texture->GetTextureDescriptor(); if (!td.used_this_frame && desc.size.width == other_desc.size.width && diff --git a/impeller/core/allocator.h b/impeller/core/allocator.h index 9a8a9fb85bcdf..aa9c7184557be 100644 --- a/impeller/core/allocator.h +++ b/impeller/core/allocator.h @@ -11,6 +11,7 @@ #include "impeller/core/device_buffer_descriptor.h" #include "impeller/core/texture_descriptor.h" #include "impeller/geometry/size.h" +#include "impeller/core/texture.h" namespace impeller { @@ -73,6 +74,13 @@ class Allocator { protected: Allocator(); + struct TextureData { + bool used_this_frame; + std::shared_ptr texture; + }; + + std::vector data_to_recycle_; + virtual std::shared_ptr OnCreateBuffer( const DeviceBufferDescriptor& desc) = 0; @@ -80,12 +88,6 @@ class Allocator { const TextureDescriptor& desc) = 0; private: - struct TextureData { - bool used_this_frame; - std::shared_ptr texture; - }; - - std::vector data_to_recycle_; size_t hit_count_ = 0; size_t total_count_ = 0; diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 07767e9109260..2cc1f3bc9cd67 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -167,7 +167,11 @@ AllocatorVK::AllocatorVK(std::weak_ptr context, is_valid_ = true; } -AllocatorVK::~AllocatorVK() = default; +AllocatorVK::~AllocatorVK() { + // The set of cached textures must be cleared before the VMA allocator is + // destructed. + data_to_recycle_.clear(); +} // |Allocator| bool AllocatorVK::IsValid() const { From 66d9aa68bff3cc6229d7cf0d80e656e61fb50004 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 13 Aug 2023 12:11:34 -0700 Subject: [PATCH 07/20] ++ --- impeller/core/allocator.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/core/allocator.h b/impeller/core/allocator.h index aa9c7184557be..7a687e2077f37 100644 --- a/impeller/core/allocator.h +++ b/impeller/core/allocator.h @@ -9,9 +9,9 @@ #include "flutter/fml/macros.h" #include "flutter/fml/mapping.h" #include "impeller/core/device_buffer_descriptor.h" +#include "impeller/core/texture.h" #include "impeller/core/texture_descriptor.h" #include "impeller/geometry/size.h" -#include "impeller/core/texture.h" namespace impeller { From 14e21a049bc06cac952884f3a376d5dfc84de634 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 09:45:52 -0700 Subject: [PATCH 08/20] Add unittests and cleanup --- impeller/BUILD.gn | 1 + impeller/aiks/testing/context_mock.h | 4 + impeller/core/BUILD.gn | 11 ++ impeller/core/allocator.cc | 39 +++-- impeller/core/allocator.h | 29 +--- impeller/core/allocator_unittests.cc | 150 ++++++++++++++++++ impeller/core/texture_descriptor.h | 13 ++ .../renderer/backend/gles/allocator_gles.h | 2 +- .../renderer/backend/gles/context_gles.cc | 6 + impeller/renderer/backend/gles/context_gles.h | 6 + impeller/renderer/backend/metal/context_mtl.h | 6 + .../renderer/backend/metal/context_mtl.mm | 11 ++ .../renderer/backend/metal/surface_mtl.mm | 4 +- .../renderer/backend/vulkan/context_vk.cc | 11 ++ impeller/renderer/backend/vulkan/context_vk.h | 6 + .../backend/vulkan/surface_context_vk.cc | 11 +- .../backend/vulkan/surface_context_vk.h | 8 +- .../backend/vulkan/swapchain_impl_vk.cc | 2 +- impeller/renderer/context.h | 14 ++ impeller/renderer/testing/mocks.h | 4 + 20 files changed, 294 insertions(+), 44 deletions(-) create mode 100644 impeller/core/allocator_unittests.cc diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn index 056461e86863d..45b615e1bee53 100644 --- a/impeller/BUILD.gn +++ b/impeller/BUILD.gn @@ -82,6 +82,7 @@ impeller_component("impeller_unittests") { "runtime_stage:runtime_stage_unittests", "scene/importer:importer_unittests", "tessellator:tessellator_unittests", + "core:allocator_unittests", ] if (impeller_supports_rendering) { diff --git a/impeller/aiks/testing/context_mock.h b/impeller/aiks/testing/context_mock.h index 14c7c5a1b807b..2ee3f7b2efb1e 100644 --- a/impeller/aiks/testing/context_mock.h +++ b/impeller/aiks/testing/context_mock.h @@ -82,6 +82,10 @@ class ContextMock : public Context { MOCK_CONST_METHOD0(GetPipelineLibrary, std::shared_ptr()); + MOCK_CONST_METHOD0(DidAcquireSurfaceFrame, void()); + + MOCK_CONST_METHOD0(DidFinishSurfaceFrame, void()); + MOCK_CONST_METHOD0(CreateCommandBuffer, std::shared_ptr()); MOCK_METHOD0(Shutdown, void()); diff --git a/impeller/core/BUILD.gn b/impeller/core/BUILD.gn index 68891ed145198..55756bb2c06f5 100644 --- a/impeller/core/BUILD.gn +++ b/impeller/core/BUILD.gn @@ -49,3 +49,14 @@ impeller_component("core") { "//flutter/fml", ] } + +impeller_component("allocator_unittests") { + testonly = true + + sources = [ "allocator_unittests.cc" ] + + deps = [ + ":core", + "//flutter/testing:testing_lib", + ] +} diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index ad79242535ac4..1e073fd533464 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -36,6 +36,29 @@ std::shared_ptr Allocator::CreateBufferWithCopy( return new_buffer; } +void Allocator::DidAcquireSurfaceFrame() { + for (auto& td : data_to_recycle_) { + td.used_this_frame = false; + } +} + +void Allocator::DidFinishSurfaceFrame() { + std::vector retain; + + for (auto td : data_to_recycle_) { + if (td.used_this_frame) { + retain.push_back(td); + } + } + data_to_recycle_.clear(); + data_to_recycle_.insert(data_to_recycle_.end(), retain.begin(), + retain.end()); +} + +void Allocator::SetEnableRenderTargetTextureCache(bool value) { + enable_render_target_texture_cache_ = value; +} + std::shared_ptr Allocator::CreateBufferWithCopy( const fml::Mapping& mapping) { return CreateBufferWithCopy(mapping.GetMapping(), mapping.GetSize()); @@ -54,22 +77,14 @@ std::shared_ptr Allocator::CreateTexture( << " exceeds maximum supported size of " << max_size; return nullptr; } - total_count_++; - if (desc.ignore_cache) { - return OnCreateTexture(desc); - } - if (desc.storage_mode != StorageMode::kHostVisible && + + if (enable_render_target_texture_cache_ && !desc.ignore_cache && + desc.storage_mode != StorageMode::kHostVisible && (desc.usage & static_cast(TextureUsage::kRenderTarget))) { for (auto& td : data_to_recycle_) { const auto other_desc = td.texture->GetTextureDescriptor(); - if (!td.used_this_frame && desc.size.width == other_desc.size.width && - desc.size.height == other_desc.size.height && - desc.storage_mode == other_desc.storage_mode && - desc.format == other_desc.format && desc.usage == other_desc.usage && - desc.sample_count == other_desc.sample_count && - desc.type == other_desc.type) { - hit_count_++; + if (!td.used_this_frame && desc.IsCompatibleWith(other_desc)) { td.used_this_frame = true; return td.texture; } diff --git a/impeller/core/allocator.h b/impeller/core/allocator.h index 7a687e2077f37..efe4679ca4f5f 100644 --- a/impeller/core/allocator.h +++ b/impeller/core/allocator.h @@ -33,6 +33,8 @@ class Allocator { std::shared_ptr CreateTexture(const TextureDescriptor& desc); + void SetEnableRenderTargetTextureCache(bool value); + //------------------------------------------------------------------------------ /// @brief Minimum value for `row_bytes` on a Texture. The row /// bytes parameter of that method must be aligned to this value. @@ -49,27 +51,9 @@ class Allocator { /// @brief Increment an internal frame used to cycle through a ring buffer of /// allocation pools. - virtual void DidAcquireSurfaceFrame() { - for (auto& td : data_to_recycle_) { - td.used_this_frame = false; - } - } - - virtual void DidFinishSurfaceFrame() { - std::vector retain; - - for (auto td : data_to_recycle_) { - if (td.used_this_frame) { - retain.push_back(td); - } - } - data_to_recycle_.clear(); - data_to_recycle_.insert(data_to_recycle_.end(), retain.begin(), - retain.end()); - FML_LOG(ERROR) << "" << hit_count_ << " / " << total_count_; - hit_count_ = 0; - total_count_ = 0; - } + virtual void DidAcquireSurfaceFrame(); + + virtual void DidFinishSurfaceFrame(); protected: Allocator(); @@ -88,8 +72,7 @@ class Allocator { const TextureDescriptor& desc) = 0; private: - size_t hit_count_ = 0; - size_t total_count_ = 0; + bool enable_render_target_texture_cache_ = false; FML_DISALLOW_COPY_AND_ASSIGN(Allocator); }; diff --git a/impeller/core/allocator_unittests.cc b/impeller/core/allocator_unittests.cc new file mode 100644 index 0000000000000..8449fe7b96776 --- /dev/null +++ b/impeller/core/allocator_unittests.cc @@ -0,0 +1,150 @@ +// 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. + +#include +#include "flutter/testing/testing.h" +#include "impeller/core/allocator.h" +#include "impeller/core/formats.h" +#include "impeller/core/texture_descriptor.h" +#include "impeller/geometry/size.h" +#include "impeller/renderer/testing/mocks.h" + +namespace impeller { +namespace testing { + +class TestAllocator : public Allocator { + public: + TestAllocator() = default; + + ~TestAllocator() = default; + + std::vector GetCachedData() { + return data_to_recycle_; + }; + + ISize GetMaxTextureSizeSupported() const override { + return ISize(1024, 1024); + }; + + std::shared_ptr OnCreateBuffer( + const DeviceBufferDescriptor& desc) override { + return std::make_shared(desc); + }; + + virtual std::shared_ptr OnCreateTexture( + const TextureDescriptor& desc) override { + return std::make_shared(desc); + }; +}; + +TEST(AllocatorTest, TextureDescriptorCompatibility) { + // Size. + { + TextureDescriptor desc_a = {.size = ISize(100, 100)}; + TextureDescriptor desc_b = {.size = ISize(100, 100)}; + TextureDescriptor desc_c = {.size = ISize(101, 100)}; + + ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); + ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); + } + // Storage Mode. + { + TextureDescriptor desc_a = {.storage_mode = StorageMode::kDevicePrivate}; + TextureDescriptor desc_b = {.storage_mode = StorageMode::kDevicePrivate}; + TextureDescriptor desc_c = {.storage_mode = StorageMode::kHostVisible}; + + ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); + ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); + } + // Format. + { + TextureDescriptor desc_a = {.format = PixelFormat::kR8G8B8A8UNormInt}; + TextureDescriptor desc_b = {.format = PixelFormat::kR8G8B8A8UNormInt}; + TextureDescriptor desc_c = {.format = PixelFormat::kB10G10R10A10XR}; + + ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); + ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); + } + // Sample Count. + { + TextureDescriptor desc_a = {.sample_count = SampleCount::kCount4}; + TextureDescriptor desc_b = {.sample_count = SampleCount::kCount4}; + TextureDescriptor desc_c = {.sample_count = SampleCount::kCount1}; + + ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); + ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); + } + // Sample Count. + { + TextureDescriptor desc_a = {.type = TextureType::kTexture2DMultisample}; + TextureDescriptor desc_b = {.type = TextureType::kTexture2DMultisample}; + TextureDescriptor desc_c = {.type = TextureType::kTexture2D}; + + ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); + ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); + } +} + +TEST(AllocatorTest, CachesTexturesWhenEnabled) { + auto allocator = std::make_shared(); + auto desc = TextureDescriptor{ + .format = PixelFormat::kR8G8B8A8UNormInt, + .size = ISize(100, 100), + .usage = static_cast(TextureUsage::kRenderTarget)}; + + allocator->DidAcquireSurfaceFrame(); + allocator->CreateTexture(desc); + + ASSERT_TRUE(allocator->GetCachedData().size() == 0); + + allocator->SetEnableRenderTargetTextureCache(true); + allocator->CreateTexture(desc); + + ASSERT_TRUE(allocator->GetCachedData().size() == 1); +} + +TEST(AllocatorTest, DoesNotCacheNonRenderTargetTextures) { + auto allocator = std::make_shared(); + auto desc = TextureDescriptor{ + .format = PixelFormat::kR8G8B8A8UNormInt, + .size = ISize(100, 100), + .usage = static_cast(TextureUsage::kShaderRead)}; + + allocator->SetEnableRenderTargetTextureCache(true); + allocator->DidAcquireSurfaceFrame(); + allocator->CreateTexture(desc); + + ASSERT_TRUE(allocator->GetCachedData().size() == 0); +} + +TEST(AllocatorTest, CachesUsedTexturesAcrossFrames) { + auto allocator = std::make_shared(); + auto desc = TextureDescriptor{ + .format = PixelFormat::kR8G8B8A8UNormInt, + .size = ISize(100, 100), + .usage = static_cast(TextureUsage::kRenderTarget)}; + + allocator->DidAcquireSurfaceFrame(); + allocator->SetEnableRenderTargetTextureCache(true); + // Create two textures of the same exact size/shape. Both should be marked as + // used this frame, so the cached data set will contain two. + allocator->CreateTexture(desc); + allocator->CreateTexture(desc); + + ASSERT_TRUE(allocator->GetCachedData().size() == 2); + + allocator->DidFinishSurfaceFrame(); + allocator->DidAcquireSurfaceFrame(); + + // Next frame, only create one texture. The set will still contain two, + // but one will be removed at the end of the frame. + allocator->CreateTexture(desc); + ASSERT_TRUE(allocator->GetCachedData().size() == 2); + + allocator->DidFinishSurfaceFrame(); + ASSERT_TRUE(allocator->GetCachedData().size() == 1); +} + +} // namespace testing +} // namespace impeller diff --git a/impeller/core/texture_descriptor.h b/impeller/core/texture_descriptor.h index 4e7587014d2e1..41ffcdad61438 100644 --- a/impeller/core/texture_descriptor.h +++ b/impeller/core/texture_descriptor.h @@ -67,6 +67,19 @@ struct TextureDescriptor { return IsMultisampleCapable(type) ? count > 1 : count == 1; } + /// @brief Whether the [other] texture descriptor has the same properties. + /// + /// This is used to check compatibility of already allocated textures + /// in the render target cache.. + constexpr bool IsCompatibleWith(const TextureDescriptor& other) const { + return size == other.size && // + storage_mode == other.storage_mode && // + format == other.format && // + usage == other.usage && // + sample_count == other.sample_count && // + type == other.type; // + } + constexpr bool IsValid() const { return format != PixelFormat::kUnknown && // size.IsPositive() && // diff --git a/impeller/renderer/backend/gles/allocator_gles.h b/impeller/renderer/backend/gles/allocator_gles.h index 14f9dcf2dbe3a..4c3426d176cdc 100644 --- a/impeller/renderer/backend/gles/allocator_gles.h +++ b/impeller/renderer/backend/gles/allocator_gles.h @@ -21,7 +21,7 @@ class AllocatorGLES final : public Allocator { ReactorGLES::Ref reactor_; bool is_valid_ = false; - AllocatorGLES(ReactorGLES::Ref reactor); + explicit AllocatorGLES(ReactorGLES::Ref reactor); // |Allocator| bool IsValid() const; diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc index 4d0530127cf1a..cfc4b77852bbe 100644 --- a/impeller/renderer/backend/gles/context_gles.cc +++ b/impeller/renderer/backend/gles/context_gles.cc @@ -139,6 +139,12 @@ std::shared_ptr ContextGLES::GetPipelineLibrary() const { return pipeline_library_; } +// |Context| +void ContextGLES::DidAcquireSurfaceFrame() const {} + +// |Context| +void ContextGLES::DidFinishSurfaceFrame() const {} + // |Context| std::shared_ptr ContextGLES::CreateCommandBuffer() const { return std::shared_ptr( diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index 92054cb413bee..d6da058953b88 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -72,6 +72,12 @@ class ContextGLES final : public Context, // |Context| std::shared_ptr CreateCommandBuffer() const override; + // |Context| + void DidAcquireSurfaceFrame() const override; + + // |Context| + void DidFinishSurfaceFrame() const override; + // |Context| const std::shared_ptr& GetCapabilities() const override; diff --git a/impeller/renderer/backend/metal/context_mtl.h b/impeller/renderer/backend/metal/context_mtl.h index c272c68769fe9..ccd5dabbe5222 100644 --- a/impeller/renderer/backend/metal/context_mtl.h +++ b/impeller/renderer/backend/metal/context_mtl.h @@ -84,6 +84,12 @@ class ContextMTL final : public Context, // |Context| bool UpdateOffscreenLayerPixelFormat(PixelFormat format) override; + // |Context| + void DidAcquireSurfaceFrame() const override; + + // |Context| + void DidFinishSurfaceFrame() const override; + // |Context| void Shutdown() override; diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index e92a4bb384fc9..4918a93cbf3d8 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -138,6 +138,7 @@ static bool DeviceSupportsComputeSubgroups(id device) { VALIDATION_LOG << "Could not set up the resource allocator."; return; } + resource_allocator_->SetEnableRenderTargetTextureCache(true); } device_capabilities_ = @@ -320,6 +321,16 @@ new ContextMTL(device, command_queue, return CreateCommandBufferInQueue(command_queue_); } +// |Context| +void ContextMTL::DidAcquireSurfaceFrame() const { + resource_allocator_->DidAcquireSurfaceFrame(); +} + +// |Context| +void ContextMTL::DidFinishSurfaceFrame() const { + resource_allocator_->DidFinishSurfaceFrame(); +} + // |Context| void ContextMTL::Shutdown() { raster_message_loop_.reset(); diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm index 179dedfa6e5b4..2b9d184a03c1d 100644 --- a/impeller/renderer/backend/metal/surface_mtl.mm +++ b/impeller/renderer/backend/metal/surface_mtl.mm @@ -37,7 +37,7 @@ VALIDATION_LOG << "Could not acquire current drawable."; return nullptr; } - context->GetResourceAllocator()->DidAcquireSurfaceFrame(); + context->DidAcquireSurfaceFrame(); return current_drawable; } @@ -225,7 +225,7 @@ if (!context) { return false; } - context->GetResourceAllocator()->DidFinishSurfaceFrame(); + context->DidFinishSurfaceFrame(); if (requires_blit_) { if (!(source_texture_ && destination_texture_)) { diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 875f770637bea..626317a291d84 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -345,6 +345,7 @@ void ContextVK::Setup(Settings settings) { VALIDATION_LOG << "Could not create memory allocator."; return; } + allocator->SetEnableRenderTargetTextureCache(true); //---------------------------------------------------------------------------- /// Setup the pipeline library. @@ -510,6 +511,16 @@ std::shared_ptr ContextVK::GetResourceManager() const { return resource_manager_; } +// |Context| +void ContextVK::DidAcquireSurfaceFrame() const { + allocator_->DidAcquireSurfaceFrame(); +} + +// |Context| +void ContextVK::DidFinishSurfaceFrame() const { + allocator_->DidFinishSurfaceFrame(); +} + std::unique_ptr ContextVK::CreateGraphicsCommandEncoderFactory() const { return std::make_unique(weak_from_this()); diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 43d4531ff5fcd..08e30656261a1 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -80,6 +80,12 @@ class ContextVK final : public Context, // |Context| const std::shared_ptr& GetCapabilities() const override; + // |Context| + void DidAcquireSurfaceFrame() const override; + + // |Context| + void DidFinishSurfaceFrame() const override; + // |Context| void Shutdown() override; diff --git a/impeller/renderer/backend/vulkan/surface_context_vk.cc b/impeller/renderer/backend/vulkan/surface_context_vk.cc index 7f2fdf82003c6..fc2738e2eb36f 100644 --- a/impeller/renderer/backend/vulkan/surface_context_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_context_vk.cc @@ -74,13 +74,16 @@ std::unique_ptr SurfaceContextVK::AcquireNextSurface() { impeller::PipelineLibraryVK::Cast(*pipeline_library) .DidAcquireSurfaceFrame(); } - auto allocator = parent_->GetResourceAllocator(); - if (allocator) { - allocator->DidAcquireSurfaceFrame(); - } + parent_->DidAcquireSurfaceFrame(); return surface; } +// |Context| +void SurfaceContextVK::DidAcquireSurfaceFrame() const {} + +// |Context| +void SurfaceContextVK::DidFinishSurfaceFrame() const {} + #ifdef FML_OS_ANDROID vk::UniqueSurfaceKHR SurfaceContextVK::CreateAndroidSurface( diff --git a/impeller/renderer/backend/vulkan/surface_context_vk.h b/impeller/renderer/backend/vulkan/surface_context_vk.h index 5847d49aa0d47..2b8ef0befdb80 100644 --- a/impeller/renderer/backend/vulkan/surface_context_vk.h +++ b/impeller/renderer/backend/vulkan/surface_context_vk.h @@ -19,7 +19,7 @@ class SwapchainVK; class SurfaceContextVK : public Context, public BackendCast { public: - SurfaceContextVK(const std::shared_ptr& parent); + explicit SurfaceContextVK(const std::shared_ptr& parent); // |Context| ~SurfaceContextVK() override; @@ -51,6 +51,12 @@ class SurfaceContextVK : public Context, // |Context| const std::shared_ptr& GetCapabilities() const override; + // |Context| + void DidAcquireSurfaceFrame() const override; + + // |Context| + void DidFinishSurfaceFrame() const override; + // |Context| void Shutdown() override; diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index 81d531875b3d3..72b42c21ece47 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -441,7 +441,7 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, return false; } } - context_strong->GetResourceAllocator()->DidFinishSurfaceFrame(); + context_strong->DidFinishSurfaceFrame(); context.GetConcurrentWorkerTaskRunner()->PostTask([&, index, image] { auto context_strong = context_.lock(); diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index b597e4fee1a88..a2ba3f7e26f9f 100644 --- a/impeller/renderer/context.h +++ b/impeller/renderer/context.h @@ -154,6 +154,20 @@ class Context { /// virtual std::shared_ptr CreateCommandBuffer() const = 0; + //--------------------------------------------------------------------------- + /// @brief Indicate the beginning of a presentation cycle. + /// + /// This may or may not indicate a new frame, or there may be multiple + /// presents within a single frame due to platform view presence. + virtual void DidAcquireSurfaceFrame() const = 0; + + //--------------------------------------------------------------------------- + /// @brief Indicate the ending of a presentation cycle. + /// + /// This may or may not indicate a new frame, or there may be multiple + /// presents within a single frame due to platform view presence. + virtual void DidFinishSurfaceFrame() const = 0; + //---------------------------------------------------------------------------- /// @brief Force all pending asynchronous work to finish. This is /// achieved by deleting all owned concurrent message loops. diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index cd3910b1a9737..be4607b806430 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -102,6 +102,10 @@ class MockImpellerContext : public Context { MOCK_CONST_METHOD0(GetPipelineLibrary, std::shared_ptr()); + MOCK_CONST_METHOD0(DidAcquireSurfaceFrame, void()); + + MOCK_CONST_METHOD0(DidFinishSurfaceFrame, void()); + MOCK_CONST_METHOD0(CreateCommandBuffer, std::shared_ptr()); MOCK_CONST_METHOD0(GetCapabilities, From 88e9d876c38ba6b479ca4b6ecad5e455c76818ee Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 09:46:17 -0700 Subject: [PATCH 09/20] ++ --- impeller/BUILD.gn | 2 +- impeller/core/allocator.cc | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn index 45b615e1bee53..d85c97214df1f 100644 --- a/impeller/BUILD.gn +++ b/impeller/BUILD.gn @@ -77,12 +77,12 @@ impeller_component("impeller_unittests") { "base:base_unittests", "blobcat:blobcat_unittests", "compiler:compiler_unittests", + "core:allocator_unittests", "display_list:skia_conversions_unittests", "geometry:geometry_unittests", "runtime_stage:runtime_stage_unittests", "scene/importer:importer_unittests", "tessellator:tessellator_unittests", - "core:allocator_unittests", ] if (impeller_supports_rendering) { diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index 1e073fd533464..b012c86f7c228 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -51,8 +51,7 @@ void Allocator::DidFinishSurfaceFrame() { } } data_to_recycle_.clear(); - data_to_recycle_.insert(data_to_recycle_.end(), retain.begin(), - retain.end()); + data_to_recycle_.insert(data_to_recycle_.end(), retain.begin(), retain.end()); } void Allocator::SetEnableRenderTargetTextureCache(bool value) { From 4775a1eaa3ef6815f33b55a2a525ee64b94509c6 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 10:20:40 -0700 Subject: [PATCH 10/20] ++ --- impeller/core/BUILD.gn | 1 + .../backend/vulkan/swapchain_impl_vk.cc | 57 +++++++++++++------ .../backend/vulkan/swapchain_impl_vk.h | 4 +- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/impeller/core/BUILD.gn b/impeller/core/BUILD.gn index 55756bb2c06f5..cc66990c909a7 100644 --- a/impeller/core/BUILD.gn +++ b/impeller/core/BUILD.gn @@ -57,6 +57,7 @@ impeller_component("allocator_unittests") { deps = [ ":core", + "../geometry", "//flutter/testing:testing_lib", ] } diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index be090e38ac487..ddef275e68ce3 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -10,11 +10,17 @@ #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/surface_vk.h" #include "impeller/renderer/backend/vulkan/swapchain_image_vk.h" +#include "vulkan/vulkan_structs.hpp" namespace impeller { static constexpr size_t kMaxFramesInFlight = 3u; +// Number of frames to poll for orientation changes. For example `1u` means +// that the orientation will be polled every frame, while `2u` means that the +// orientation will be polled every other frame. +static constexpr size_t kPollFramesForOrientation = 1u; + struct FrameSynchronizer { vk::UniqueFence acquire; vk::UniqueSemaphore render_ready; @@ -124,16 +130,17 @@ static std::optional ChoosePresentQueue( std::shared_ptr SwapchainImplVK::Create( const std::shared_ptr& context, vk::UniqueSurfaceKHR surface, - bool was_rotated, - vk::SwapchainKHR old_swapchain) { + vk::SwapchainKHR old_swapchain, + vk::SurfaceTransformFlagBitsKHR last_transform) { return std::shared_ptr(new SwapchainImplVK( - context, std::move(surface), was_rotated, old_swapchain)); + context, std::move(surface), old_swapchain, last_transform)); } -SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, - vk::UniqueSurfaceKHR surface, - bool was_rotated, - vk::SwapchainKHR old_swapchain) { +SwapchainImplVK::SwapchainImplVK( + const std::shared_ptr& context, + vk::UniqueSurfaceKHR surface, + vk::SwapchainKHR old_swapchain, + vk::SurfaceTransformFlagBitsKHR last_transform) { if (!context) { return; } @@ -278,8 +285,7 @@ SwapchainImplVK::SwapchainImplVK(const std::shared_ptr& context, synchronizers_ = std::move(synchronizers); current_frame_ = synchronizers_.size() - 1u; is_valid_ = true; - was_rotated_ = was_rotated; - is_rotated_ = was_rotated; + transform_if_changed_discard_swapchain_ = last_transform; } SwapchainImplVK::~SwapchainImplVK() { @@ -311,6 +317,10 @@ vk::Format SwapchainImplVK::GetSurfaceFormat() const { return surface_format_; } +vk::SurfaceTransformFlagBitsKHR SwapchainImplVK::GetLastTransform() const { + return transform_if_changed_discard_swapchain_; +} + std::shared_ptr SwapchainImplVK::GetContext() const { return context_.lock(); } @@ -321,10 +331,6 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { return {}; } - if (was_rotated_ != is_rotated_) { - return AcquireResult{true /* out of date */}; - } - const auto& context = ContextVK::Cast(*context_strong); current_frame_ = (current_frame_ + 1u) % synchronizers_.size(); @@ -339,6 +345,26 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { return {}; } + //---------------------------------------------------------------------------- + /// Poll to see if the orientation has changed. + /// + /// https://developer.android.com/games/optimize/vulkan-prerotation#using_polling + current_transform_poll_count_++; + if (current_transform_poll_count_ >= kPollFramesForOrientation) { + current_transform_poll_count_ = 0u; + auto [caps_result, caps] = + context.GetPhysicalDevice().getSurfaceCapabilitiesKHR(*surface_); + if (caps_result != vk::Result::eSuccess) { + VALIDATION_LOG << "Could not get surface capabilities: " + << vk::to_string(caps_result); + return {}; + } + if (caps.currentTransform != transform_if_changed_discard_swapchain_) { + transform_if_changed_discard_swapchain_ = caps.currentTransform; + return AcquireResult{true /* out of date */}; + } + } + //---------------------------------------------------------------------------- /// Get the next image index. /// @@ -349,11 +375,6 @@ SwapchainImplVK::AcquireResult SwapchainImplVK::AcquireNextDrawable() { nullptr // fence ); - if (acq_result == vk::Result::eSuboptimalKHR) { - is_rotated_ = true; - return AcquireResult{true /* out of date */}; - } - if (acq_result == vk::Result::eErrorOutOfDateKHR) { return AcquireResult{true /* out of date */}; } diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.h b/impeller/renderer/backend/vulkan/swapchain_impl_vk.h index a762d7a7f454e..cacc6b2494b6b 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.h +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.h @@ -44,9 +44,9 @@ class SwapchainImplVK final std::unique_ptr surface; bool out_of_date = false; - explicit AcquireResult(bool p_out_of_date = false) : out_of_date(p_out_of_date) {} + AcquireResult(bool p_out_of_date = false) : out_of_date(p_out_of_date) {} - explicit AcquireResult(std::unique_ptr p_surface) + AcquireResult(std::unique_ptr p_surface) : surface(std::move(p_surface)) {} }; From 40c57e7b729c2ca4e7651ca7269bea97531e611c Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 10:31:59 -0700 Subject: [PATCH 11/20] ++ --- lib/ui/painting/image_decoder_unittests.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index b779a77ded17c..2732f979952c1 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -144,6 +144,10 @@ class TestImpellerContext : public impeller::Context { return nullptr; } + void DidAcquireSurfaceFrame() const override {} + + void DidFinishSurfaceFrame() const override {} + void Shutdown() override {} mutable size_t command_buffer_count_ = 0; From 386c4018eacd51693db33063e552929bb55abc9f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 10:49:21 -0700 Subject: [PATCH 12/20] excluded --- ci/licenses_golden/excluded_files | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 65bba4ad1aff2..b9c9db6b4b3f1 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -128,6 +128,7 @@ ../../../flutter/impeller/compiler/README.md ../../../flutter/impeller/compiler/compiler_unittests.cc ../../../flutter/impeller/compiler/switches_unittests.cc +../../../flutter/impeller/core/allocator_unittests.cc ../../../flutter/impeller/display_list/dl_unittests.cc ../../../flutter/impeller/display_list/skia_conversions_unittests.cc ../../../flutter/impeller/docs From 6acb7514979510bbea0c41efb88b6d7f66c6737d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 18:35:50 -0700 Subject: [PATCH 13/20] move caching to entity layer --- impeller/aiks/picture.cc | 6 + impeller/aiks/testing/context_mock.h | 4 - impeller/core/allocator.cc | 40 +-- impeller/core/allocator.h | 13 - impeller/core/allocator_unittests.cc | 264 +++++++++--------- impeller/entity/BUILD.gn | 2 + impeller/entity/contents/content_context.cc | 11 +- impeller/entity/contents/content_context.h | 7 + impeller/entity/contents/scene_contents.cc | 7 +- impeller/entity/entity_pass.cc | 24 +- impeller/entity/render_target_cache.cc | 49 ++++ impeller/entity/render_target_cache.h | 40 +++ .../renderer/backend/gles/context_gles.cc | 6 - impeller/renderer/backend/gles/context_gles.h | 6 - impeller/renderer/backend/metal/context_mtl.h | 6 - .../renderer/backend/metal/context_mtl.mm | 11 - .../renderer/backend/metal/surface_mtl.mm | 2 - .../renderer/backend/vulkan/allocator_vk.cc | 7 +- .../renderer/backend/vulkan/allocator_vk.h | 1 - .../renderer/backend/vulkan/context_vk.cc | 11 - impeller/renderer/backend/vulkan/context_vk.h | 6 - .../backend/vulkan/surface_context_vk.cc | 8 +- .../backend/vulkan/surface_context_vk.h | 6 - .../backend/vulkan/swapchain_impl_vk.cc | 1 - impeller/renderer/context.h | 14 - impeller/renderer/render_target.cc | 31 +- impeller/renderer/render_target.h | 32 +++ impeller/renderer/renderer_unittests.cc | 4 +- 28 files changed, 324 insertions(+), 295 deletions(-) create mode 100644 impeller/entity/render_target_cache.cc create mode 100644 impeller/entity/render_target_cache.h diff --git a/impeller/aiks/picture.cc b/impeller/aiks/picture.cc index 7ad91fb63ef0c..d7a6d4085811f 100644 --- a/impeller/aiks/picture.cc +++ b/impeller/aiks/picture.cc @@ -54,10 +54,15 @@ std::shared_ptr Picture::RenderToTexture( // This texture isn't host visible, but we might want to add host visible // features to Image someday. auto impeller_context = context.GetContext(); + // Do not use the render target cache as the lifecycle of this texture + // will outlive a particular frame. + RenderTargetAllocator render_target_allocator = + RenderTargetAllocator(impeller_context->GetResourceAllocator()); RenderTarget target; if (impeller_context->GetCapabilities()->SupportsOffscreenMSAA()) { target = RenderTarget::CreateOffscreenMSAA( *impeller_context, // context + render_target_allocator, // allocator size, // size "Picture Snapshot MSAA", // label RenderTarget:: @@ -70,6 +75,7 @@ std::shared_ptr Picture::RenderToTexture( } else { target = RenderTarget::CreateOffscreen( *impeller_context, // context + render_target_allocator, // allocator size, // size "Picture Snapshot", // label RenderTarget::kDefaultColorAttachmentConfig // color_attachment_config diff --git a/impeller/aiks/testing/context_mock.h b/impeller/aiks/testing/context_mock.h index 2ee3f7b2efb1e..14c7c5a1b807b 100644 --- a/impeller/aiks/testing/context_mock.h +++ b/impeller/aiks/testing/context_mock.h @@ -82,10 +82,6 @@ class ContextMock : public Context { MOCK_CONST_METHOD0(GetPipelineLibrary, std::shared_ptr()); - MOCK_CONST_METHOD0(DidAcquireSurfaceFrame, void()); - - MOCK_CONST_METHOD0(DidFinishSurfaceFrame, void()); - MOCK_CONST_METHOD0(CreateCommandBuffer, std::shared_ptr()); MOCK_METHOD0(Shutdown, void()); diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index b012c86f7c228..c58f012e37157 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -36,28 +36,6 @@ std::shared_ptr Allocator::CreateBufferWithCopy( return new_buffer; } -void Allocator::DidAcquireSurfaceFrame() { - for (auto& td : data_to_recycle_) { - td.used_this_frame = false; - } -} - -void Allocator::DidFinishSurfaceFrame() { - std::vector retain; - - for (auto td : data_to_recycle_) { - if (td.used_this_frame) { - retain.push_back(td); - } - } - data_to_recycle_.clear(); - data_to_recycle_.insert(data_to_recycle_.end(), retain.begin(), retain.end()); -} - -void Allocator::SetEnableRenderTargetTextureCache(bool value) { - enable_render_target_texture_cache_ = value; -} - std::shared_ptr Allocator::CreateBufferWithCopy( const fml::Mapping& mapping) { return CreateBufferWithCopy(mapping.GetMapping(), mapping.GetSize()); @@ -77,25 +55,11 @@ std::shared_ptr Allocator::CreateTexture( return nullptr; } - if (enable_render_target_texture_cache_ && !desc.ignore_cache && - desc.storage_mode != StorageMode::kHostVisible && - (desc.usage & - static_cast(TextureUsage::kRenderTarget))) { - for (auto& td : data_to_recycle_) { - const auto other_desc = td.texture->GetTextureDescriptor(); - if (!td.used_this_frame && desc.IsCompatibleWith(other_desc)) { - td.used_this_frame = true; - return td.texture; - } - } - auto result = OnCreateTexture(desc); - data_to_recycle_.push_back({.used_this_frame = true, .texture = result}); - return result; - } - return OnCreateTexture(desc); } +void Allocator::DidAcquireSurfaceFrame() { } + uint16_t Allocator::MinimumBytesPerRow(PixelFormat format) const { return BytesPerPixelForPixelFormat(format); } diff --git a/impeller/core/allocator.h b/impeller/core/allocator.h index efe4679ca4f5f..d5f8aee10512e 100644 --- a/impeller/core/allocator.h +++ b/impeller/core/allocator.h @@ -33,8 +33,6 @@ class Allocator { std::shared_ptr CreateTexture(const TextureDescriptor& desc); - void SetEnableRenderTargetTextureCache(bool value); - //------------------------------------------------------------------------------ /// @brief Minimum value for `row_bytes` on a Texture. The row /// bytes parameter of that method must be aligned to this value. @@ -53,18 +51,9 @@ class Allocator { /// allocation pools. virtual void DidAcquireSurfaceFrame(); - virtual void DidFinishSurfaceFrame(); - protected: Allocator(); - struct TextureData { - bool used_this_frame; - std::shared_ptr texture; - }; - - std::vector data_to_recycle_; - virtual std::shared_ptr OnCreateBuffer( const DeviceBufferDescriptor& desc) = 0; @@ -72,8 +61,6 @@ class Allocator { const TextureDescriptor& desc) = 0; private: - bool enable_render_target_texture_cache_ = false; - FML_DISALLOW_COPY_AND_ASSIGN(Allocator); }; diff --git a/impeller/core/allocator_unittests.cc b/impeller/core/allocator_unittests.cc index 8449fe7b96776..6d69b78698d18 100644 --- a/impeller/core/allocator_unittests.cc +++ b/impeller/core/allocator_unittests.cc @@ -13,138 +13,138 @@ namespace impeller { namespace testing { -class TestAllocator : public Allocator { - public: - TestAllocator() = default; - - ~TestAllocator() = default; - - std::vector GetCachedData() { - return data_to_recycle_; - }; - - ISize GetMaxTextureSizeSupported() const override { - return ISize(1024, 1024); - }; - - std::shared_ptr OnCreateBuffer( - const DeviceBufferDescriptor& desc) override { - return std::make_shared(desc); - }; - - virtual std::shared_ptr OnCreateTexture( - const TextureDescriptor& desc) override { - return std::make_shared(desc); - }; -}; - -TEST(AllocatorTest, TextureDescriptorCompatibility) { - // Size. - { - TextureDescriptor desc_a = {.size = ISize(100, 100)}; - TextureDescriptor desc_b = {.size = ISize(100, 100)}; - TextureDescriptor desc_c = {.size = ISize(101, 100)}; - - ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); - ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); - } - // Storage Mode. - { - TextureDescriptor desc_a = {.storage_mode = StorageMode::kDevicePrivate}; - TextureDescriptor desc_b = {.storage_mode = StorageMode::kDevicePrivate}; - TextureDescriptor desc_c = {.storage_mode = StorageMode::kHostVisible}; - - ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); - ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); - } - // Format. - { - TextureDescriptor desc_a = {.format = PixelFormat::kR8G8B8A8UNormInt}; - TextureDescriptor desc_b = {.format = PixelFormat::kR8G8B8A8UNormInt}; - TextureDescriptor desc_c = {.format = PixelFormat::kB10G10R10A10XR}; - - ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); - ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); - } - // Sample Count. - { - TextureDescriptor desc_a = {.sample_count = SampleCount::kCount4}; - TextureDescriptor desc_b = {.sample_count = SampleCount::kCount4}; - TextureDescriptor desc_c = {.sample_count = SampleCount::kCount1}; - - ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); - ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); - } - // Sample Count. - { - TextureDescriptor desc_a = {.type = TextureType::kTexture2DMultisample}; - TextureDescriptor desc_b = {.type = TextureType::kTexture2DMultisample}; - TextureDescriptor desc_c = {.type = TextureType::kTexture2D}; - - ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); - ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); - } -} - -TEST(AllocatorTest, CachesTexturesWhenEnabled) { - auto allocator = std::make_shared(); - auto desc = TextureDescriptor{ - .format = PixelFormat::kR8G8B8A8UNormInt, - .size = ISize(100, 100), - .usage = static_cast(TextureUsage::kRenderTarget)}; - - allocator->DidAcquireSurfaceFrame(); - allocator->CreateTexture(desc); - - ASSERT_TRUE(allocator->GetCachedData().size() == 0); - - allocator->SetEnableRenderTargetTextureCache(true); - allocator->CreateTexture(desc); - - ASSERT_TRUE(allocator->GetCachedData().size() == 1); -} - -TEST(AllocatorTest, DoesNotCacheNonRenderTargetTextures) { - auto allocator = std::make_shared(); - auto desc = TextureDescriptor{ - .format = PixelFormat::kR8G8B8A8UNormInt, - .size = ISize(100, 100), - .usage = static_cast(TextureUsage::kShaderRead)}; - - allocator->SetEnableRenderTargetTextureCache(true); - allocator->DidAcquireSurfaceFrame(); - allocator->CreateTexture(desc); - - ASSERT_TRUE(allocator->GetCachedData().size() == 0); -} - -TEST(AllocatorTest, CachesUsedTexturesAcrossFrames) { - auto allocator = std::make_shared(); - auto desc = TextureDescriptor{ - .format = PixelFormat::kR8G8B8A8UNormInt, - .size = ISize(100, 100), - .usage = static_cast(TextureUsage::kRenderTarget)}; - - allocator->DidAcquireSurfaceFrame(); - allocator->SetEnableRenderTargetTextureCache(true); - // Create two textures of the same exact size/shape. Both should be marked as - // used this frame, so the cached data set will contain two. - allocator->CreateTexture(desc); - allocator->CreateTexture(desc); - - ASSERT_TRUE(allocator->GetCachedData().size() == 2); - - allocator->DidFinishSurfaceFrame(); - allocator->DidAcquireSurfaceFrame(); - - // Next frame, only create one texture. The set will still contain two, - // but one will be removed at the end of the frame. - allocator->CreateTexture(desc); - ASSERT_TRUE(allocator->GetCachedData().size() == 2); - - allocator->DidFinishSurfaceFrame(); - ASSERT_TRUE(allocator->GetCachedData().size() == 1); -} +// class TestAllocator : public Allocator { +// public: +// TestAllocator() = default; + +// ~TestAllocator() = default; + +// std::vector GetCachedData() { +// return data_to_recycle_; +// }; + +// ISize GetMaxTextureSizeSupported() const override { +// return ISize(1024, 1024); +// }; + +// std::shared_ptr OnCreateBuffer( +// const DeviceBufferDescriptor& desc) override { +// return std::make_shared(desc); +// }; + +// virtual std::shared_ptr OnCreateTexture( +// const TextureDescriptor& desc) override { +// return std::make_shared(desc); +// }; +// }; + +// TEST(AllocatorTest, TextureDescriptorCompatibility) { +// // Size. +// { +// TextureDescriptor desc_a = {.size = ISize(100, 100)}; +// TextureDescriptor desc_b = {.size = ISize(100, 100)}; +// TextureDescriptor desc_c = {.size = ISize(101, 100)}; + +// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); +// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); +// } +// // Storage Mode. +// { +// TextureDescriptor desc_a = {.storage_mode = StorageMode::kDevicePrivate}; +// TextureDescriptor desc_b = {.storage_mode = StorageMode::kDevicePrivate}; +// TextureDescriptor desc_c = {.storage_mode = StorageMode::kHostVisible}; + +// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); +// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); +// } +// // Format. +// { +// TextureDescriptor desc_a = {.format = PixelFormat::kR8G8B8A8UNormInt}; +// TextureDescriptor desc_b = {.format = PixelFormat::kR8G8B8A8UNormInt}; +// TextureDescriptor desc_c = {.format = PixelFormat::kB10G10R10A10XR}; + +// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); +// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); +// } +// // Sample Count. +// { +// TextureDescriptor desc_a = {.sample_count = SampleCount::kCount4}; +// TextureDescriptor desc_b = {.sample_count = SampleCount::kCount4}; +// TextureDescriptor desc_c = {.sample_count = SampleCount::kCount1}; + +// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); +// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); +// } +// // Sample Count. +// { +// TextureDescriptor desc_a = {.type = TextureType::kTexture2DMultisample}; +// TextureDescriptor desc_b = {.type = TextureType::kTexture2DMultisample}; +// TextureDescriptor desc_c = {.type = TextureType::kTexture2D}; + +// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); +// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); +// } +// } + +// TEST(AllocatorTest, CachesTexturesWhenEnabled) { +// auto allocator = std::make_shared(); +// auto desc = TextureDescriptor{ +// .format = PixelFormat::kR8G8B8A8UNormInt, +// .size = ISize(100, 100), +// .usage = static_cast(TextureUsage::kRenderTarget)}; + +// allocator->DidAcquireSurfaceFrame(); +// allocator->CreateTexture(desc); + +// ASSERT_TRUE(allocator->GetCachedData().size() == 0); + +// allocator->SetEnableRenderTargetTextureCache(true); +// allocator->CreateTexture(desc); + +// ASSERT_TRUE(allocator->GetCachedData().size() == 1); +// } + +// TEST(AllocatorTest, DoesNotCacheNonRenderTargetTextures) { +// auto allocator = std::make_shared(); +// auto desc = TextureDescriptor{ +// .format = PixelFormat::kR8G8B8A8UNormInt, +// .size = ISize(100, 100), +// .usage = static_cast(TextureUsage::kShaderRead)}; + +// allocator->SetEnableRenderTargetTextureCache(true); +// allocator->DidAcquireSurfaceFrame(); +// allocator->CreateTexture(desc); + +// ASSERT_TRUE(allocator->GetCachedData().size() == 0); +// } + +// TEST(AllocatorTest, CachesUsedTexturesAcrossFrames) { +// auto allocator = std::make_shared(); +// auto desc = TextureDescriptor{ +// .format = PixelFormat::kR8G8B8A8UNormInt, +// .size = ISize(100, 100), +// .usage = static_cast(TextureUsage::kRenderTarget)}; + +// allocator->DidAcquireSurfaceFrame(); +// allocator->SetEnableRenderTargetTextureCache(true); +// // Create two textures of the same exact size/shape. Both should be marked as +// // used this frame, so the cached data set will contain two. +// allocator->CreateTexture(desc); +// allocator->CreateTexture(desc); + +// ASSERT_TRUE(allocator->GetCachedData().size() == 2); + +// allocator->DidFinishSurfaceFrame(); +// allocator->DidAcquireSurfaceFrame(); + +// // Next frame, only create one texture. The set will still contain two, +// // but one will be removed at the end of the frame. +// allocator->CreateTexture(desc); +// ASSERT_TRUE(allocator->GetCachedData().size() == 2); + +// allocator->DidFinishSurfaceFrame(); +// ASSERT_TRUE(allocator->GetCachedData().size() == 1); +// } } // namespace testing } // namespace impeller diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index 30da46ffc8b45..714bfd079495f 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -242,6 +242,8 @@ impeller_component("entity") { "geometry/vertices_geometry.h", "inline_pass_context.cc", "inline_pass_context.h", + "render_target_cache.h", + "render_target_cache.cc", ] if (impeller_debug) { diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index a727bf186e40d..809df7699b24c 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -10,6 +10,7 @@ #include "impeller/base/strings.h" #include "impeller/core/formats.h" #include "impeller/entity/entity.h" +#include "impeller/entity/render_target_cache.h" #include "impeller/renderer/command_buffer.h" #include "impeller/renderer/pipeline_library.h" #include "impeller/renderer/render_pass.h" @@ -162,7 +163,9 @@ ContentContext::ContentContext(std::shared_ptr context) : context_(std::move(context)), lazy_glyph_atlas_(std::make_shared()), tessellator_(std::make_shared()), - scene_context_(std::make_shared(context_)) { + scene_context_(std::make_shared(context_)), + render_target_cache_(std::make_shared( + context_->GetResourceAllocator())) { if (!context_ || !context_->IsValid()) { return; } @@ -359,7 +362,8 @@ std::shared_ptr ContentContext::MakeSubpass( RenderTarget subpass_target; if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) { subpass_target = RenderTarget::CreateOffscreenMSAA( - *context, texture_size, SPrintF("%s Offscreen", label.c_str()), + *context, *GetRenderTargetCache().get(), texture_size, + SPrintF("%s Offscreen", label.c_str()), RenderTarget::kDefaultColorAttachmentConfigMSAA // #ifndef FML_OS_ANDROID // Reduce PSO variants for Vulkan. , @@ -368,7 +372,8 @@ std::shared_ptr ContentContext::MakeSubpass( ); } else { subpass_target = RenderTarget::CreateOffscreen( - *context, texture_size, SPrintF("%s Offscreen", label.c_str()), + *context, *GetRenderTargetCache().get(), texture_size, + SPrintF("%s Offscreen", label.c_str()), RenderTarget::kDefaultColorAttachmentConfig // #ifndef FML_OS_ANDROID // Reduce PSO variants for Vulkan. , diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 423d9a1d87dd7..13b9a5e89ca83 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -17,6 +17,7 @@ #include "impeller/entity/entity.h" #include "impeller/renderer/capabilities.h" #include "impeller/renderer/pipeline.h" +#include "impeller/renderer/render_target.h" #include "impeller/scene/scene_context.h" #ifdef IMPELLER_DEBUG @@ -333,6 +334,7 @@ struct ContentContextOptions { }; class Tessellator; +class RenderTargetCache; class ContentContext { public: @@ -707,6 +709,10 @@ class ContentContext { return lazy_glyph_atlas_; } + std::shared_ptr GetRenderTargetCache() const { + return render_target_cache_; + } + private: std::shared_ptr context_; std::shared_ptr lazy_glyph_atlas_; @@ -864,6 +870,7 @@ class ContentContext { bool is_valid_ = false; std::shared_ptr tessellator_; std::shared_ptr scene_context_; + std::shared_ptr render_target_cache_; bool wireframe_ = false; FML_DISALLOW_COPY_AND_ASSIGN(ContentContext); diff --git a/impeller/entity/contents/scene_contents.cc b/impeller/entity/contents/scene_contents.cc index bb00ff923e26e..bb4f014a2ebdb 100644 --- a/impeller/entity/contents/scene_contents.cc +++ b/impeller/entity/contents/scene_contents.cc @@ -45,9 +45,10 @@ bool SceneContents::Render(const ContentContext& renderer, } RenderTarget subpass_target = RenderTarget::CreateOffscreenMSAA( - *renderer.GetContext(), // context - ISize(coverage.value().size), // size - "SceneContents", // label + *renderer.GetContext(), // context + *renderer.GetRenderTargetCache().get(), // allocator + ISize(coverage.value().size), // size + "SceneContents", // label RenderTarget::AttachmentConfigMSAA{ .storage_mode = StorageMode::kDeviceTransient, .resolve_storage_mode = StorageMode::kDevicePrivate, diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 3a9537daafb24..a1a6d085f184c 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -209,9 +209,10 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, RenderTarget target; if (context->GetCapabilities()->SupportsOffscreenMSAA()) { target = RenderTarget::CreateOffscreenMSAA( - *context, // context - size, // size - "EntityPass", // label + *context, // context + *renderer.GetRenderTargetCache().get(), // allocator + size, // size + "EntityPass", // label RenderTarget::AttachmentConfigMSAA{ .storage_mode = StorageMode::kDeviceTransient, .resolve_storage_mode = StorageMode::kDevicePrivate, @@ -222,9 +223,10 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, ); } else { target = RenderTarget::CreateOffscreen( - *context, // context - size, // size - "EntityPass", // label + *context, // context + *renderer.GetRenderTargetCache().get(), // allocator + size, // size + "EntityPass", // label RenderTarget::AttachmentConfig{ .storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kDontCare, @@ -248,6 +250,7 @@ uint32_t EntityPass::GetTotalPassReads(ContentContext& renderer) const { bool EntityPass::Render(ContentContext& renderer, const RenderTarget& render_target) const { + renderer.GetRenderTargetCache()->Start(); auto root_render_target = render_target; if (root_render_target.GetColorAttachments().find(0u) == @@ -256,8 +259,10 @@ bool EntityPass::Render(ContentContext& renderer, return false; } - fml::ScopedCleanupClosure reset_lazy_glyph_atlas( - [&renderer]() { renderer.GetLazyGlyphAtlas()->ResetTextFrames(); }); + fml::ScopedCleanupClosure reset_state([&renderer]() { + renderer.GetLazyGlyphAtlas()->ResetTextFrames(); + renderer.GetRenderTargetCache()->End(); + }); IterateAllEntities([lazy_glyph_atlas = renderer.GetLazyGlyphAtlas()](const Entity& entity) { @@ -381,7 +386,8 @@ bool EntityPass::Render(ContentContext& renderer, // provided by the caller. else { root_render_target.SetupStencilAttachment( - *renderer.GetContext(), color0.texture->GetSize(), + *renderer.GetContext(), *renderer.GetRenderTargetCache().get(), + color0.texture->GetSize(), renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA(), "ImpellerOnscreen", GetDefaultStencilConfig(reads_from_onscreen_backdrop)); diff --git a/impeller/entity/render_target_cache.cc b/impeller/entity/render_target_cache.cc new file mode 100644 index 0000000000000..e1c697eb79b7b --- /dev/null +++ b/impeller/entity/render_target_cache.cc @@ -0,0 +1,49 @@ +// 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. + +#include "impeller/entity/render_target_cache.h" +#include "impeller/renderer/render_target.h" + +namespace impeller { + +RenderTargetCache::RenderTargetCache(std::shared_ptr allocator) + : RenderTargetAllocator(std::move(allocator)) {} + +void RenderTargetCache::Start() { + for (auto& td : texture_data_) { + td.used_this_frame = false; + } +} + +void RenderTargetCache::End() { + std::vector retain; + + for (auto td : texture_data_) { + if (td.used_this_frame) { + retain.push_back(td); + } + } + texture_data_.swap(retain); +} + +std::shared_ptr RenderTargetCache::CreateTexture( + const TextureDescriptor& desc) { + FML_DCHECK(desc.storage_mode != StorageMode::kHostVisible); + FML_DCHECK(desc.usage & + static_cast(TextureUsage::kRenderTarget)); + + for (auto& td : texture_data_) { + const auto other_desc = td.texture->GetTextureDescriptor(); + if (!td.used_this_frame && desc.IsCompatibleWith(other_desc)) { + td.used_this_frame = true; + return td.texture; + } + } + auto result = RenderTargetAllocator::CreateTexture(desc); + texture_data_.push_back( + TextureData{.used_this_frame = true, .texture = result}); + return result; +} + +} // namespace impeller diff --git a/impeller/entity/render_target_cache.h b/impeller/entity/render_target_cache.h new file mode 100644 index 0000000000000..3ae6617f558eb --- /dev/null +++ b/impeller/entity/render_target_cache.h @@ -0,0 +1,40 @@ +// 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/render_target.h" + +namespace impeller { + +/// @brief An implementation of the [RenderTargetAllocator] that caches all +/// allocated texture data for one frame. +/// +/// Any textures unused after a frame are immediately discarded. +class RenderTargetCache : public RenderTargetAllocator { + public: + explicit RenderTargetCache(std::shared_ptr allocator); + + ~RenderTargetCache() = default; + + // |RenderTargetAllocator| + void Start() override; + + // |RenderTargetAllocator| + void End() override; + + // |RenderTargetAllocator| + std::shared_ptr CreateTexture( + const TextureDescriptor& desc) override; + + private: + struct TextureData { + bool used_this_frame; + std::shared_ptr texture; + }; + + std::vector texture_data_; +}; + +} // namespace impeller diff --git a/impeller/renderer/backend/gles/context_gles.cc b/impeller/renderer/backend/gles/context_gles.cc index cfc4b77852bbe..4d0530127cf1a 100644 --- a/impeller/renderer/backend/gles/context_gles.cc +++ b/impeller/renderer/backend/gles/context_gles.cc @@ -139,12 +139,6 @@ std::shared_ptr ContextGLES::GetPipelineLibrary() const { return pipeline_library_; } -// |Context| -void ContextGLES::DidAcquireSurfaceFrame() const {} - -// |Context| -void ContextGLES::DidFinishSurfaceFrame() const {} - // |Context| std::shared_ptr ContextGLES::CreateCommandBuffer() const { return std::shared_ptr( diff --git a/impeller/renderer/backend/gles/context_gles.h b/impeller/renderer/backend/gles/context_gles.h index d6da058953b88..92054cb413bee 100644 --- a/impeller/renderer/backend/gles/context_gles.h +++ b/impeller/renderer/backend/gles/context_gles.h @@ -72,12 +72,6 @@ class ContextGLES final : public Context, // |Context| std::shared_ptr CreateCommandBuffer() const override; - // |Context| - void DidAcquireSurfaceFrame() const override; - - // |Context| - void DidFinishSurfaceFrame() const override; - // |Context| const std::shared_ptr& GetCapabilities() const override; diff --git a/impeller/renderer/backend/metal/context_mtl.h b/impeller/renderer/backend/metal/context_mtl.h index ccd5dabbe5222..c272c68769fe9 100644 --- a/impeller/renderer/backend/metal/context_mtl.h +++ b/impeller/renderer/backend/metal/context_mtl.h @@ -84,12 +84,6 @@ class ContextMTL final : public Context, // |Context| bool UpdateOffscreenLayerPixelFormat(PixelFormat format) override; - // |Context| - void DidAcquireSurfaceFrame() const override; - - // |Context| - void DidFinishSurfaceFrame() const override; - // |Context| void Shutdown() override; diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 4918a93cbf3d8..e92a4bb384fc9 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -138,7 +138,6 @@ static bool DeviceSupportsComputeSubgroups(id device) { VALIDATION_LOG << "Could not set up the resource allocator."; return; } - resource_allocator_->SetEnableRenderTargetTextureCache(true); } device_capabilities_ = @@ -321,16 +320,6 @@ new ContextMTL(device, command_queue, return CreateCommandBufferInQueue(command_queue_); } -// |Context| -void ContextMTL::DidAcquireSurfaceFrame() const { - resource_allocator_->DidAcquireSurfaceFrame(); -} - -// |Context| -void ContextMTL::DidFinishSurfaceFrame() const { - resource_allocator_->DidFinishSurfaceFrame(); -} - // |Context| void ContextMTL::Shutdown() { raster_message_loop_.reset(); diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm index 2b9d184a03c1d..526e66f4c5ff5 100644 --- a/impeller/renderer/backend/metal/surface_mtl.mm +++ b/impeller/renderer/backend/metal/surface_mtl.mm @@ -37,7 +37,6 @@ VALIDATION_LOG << "Could not acquire current drawable."; return nullptr; } - context->DidAcquireSurfaceFrame(); return current_drawable; } @@ -225,7 +224,6 @@ if (!context) { return false; } - context->DidFinishSurfaceFrame(); if (requires_blit_) { if (!(source_texture_ && destination_texture_)) { diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 2cc1f3bc9cd67..4e35a416d99ba 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -167,11 +167,7 @@ AllocatorVK::AllocatorVK(std::weak_ptr context, is_valid_ = true; } -AllocatorVK::~AllocatorVK() { - // The set of cached textures must be cleared before the VMA allocator is - // destructed. - data_to_recycle_.clear(); -} +AllocatorVK::~AllocatorVK() {} // |Allocator| bool AllocatorVK::IsValid() const { @@ -438,7 +434,6 @@ std::shared_ptr AllocatorVK::OnCreateTexture( void AllocatorVK::DidAcquireSurfaceFrame() { frame_count_++; raster_thread_id_ = std::this_thread::get_id(); - Allocator::DidAcquireSurfaceFrame(); } // |Allocator| diff --git a/impeller/renderer/backend/vulkan/allocator_vk.h b/impeller/renderer/backend/vulkan/allocator_vk.h index d1753636d0b0b..05a5ab51e089f 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.h +++ b/impeller/renderer/backend/vulkan/allocator_vk.h @@ -51,7 +51,6 @@ class AllocatorVK final : public Allocator { // |Allocator| bool IsValid() const; - // |Allocator| void DidAcquireSurfaceFrame() override; // |Allocator| diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 626317a291d84..875f770637bea 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -345,7 +345,6 @@ void ContextVK::Setup(Settings settings) { VALIDATION_LOG << "Could not create memory allocator."; return; } - allocator->SetEnableRenderTargetTextureCache(true); //---------------------------------------------------------------------------- /// Setup the pipeline library. @@ -511,16 +510,6 @@ std::shared_ptr ContextVK::GetResourceManager() const { return resource_manager_; } -// |Context| -void ContextVK::DidAcquireSurfaceFrame() const { - allocator_->DidAcquireSurfaceFrame(); -} - -// |Context| -void ContextVK::DidFinishSurfaceFrame() const { - allocator_->DidFinishSurfaceFrame(); -} - std::unique_ptr ContextVK::CreateGraphicsCommandEncoderFactory() const { return std::make_unique(weak_from_this()); diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 08e30656261a1..43d4531ff5fcd 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -80,12 +80,6 @@ class ContextVK final : public Context, // |Context| const std::shared_ptr& GetCapabilities() const override; - // |Context| - void DidAcquireSurfaceFrame() const override; - - // |Context| - void DidFinishSurfaceFrame() const override; - // |Context| void Shutdown() override; diff --git a/impeller/renderer/backend/vulkan/surface_context_vk.cc b/impeller/renderer/backend/vulkan/surface_context_vk.cc index fc2738e2eb36f..814c406430b67 100644 --- a/impeller/renderer/backend/vulkan/surface_context_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_context_vk.cc @@ -74,16 +74,10 @@ std::unique_ptr SurfaceContextVK::AcquireNextSurface() { impeller::PipelineLibraryVK::Cast(*pipeline_library) .DidAcquireSurfaceFrame(); } - parent_->DidAcquireSurfaceFrame(); + parent_->GetResourceAllocator()->DidAcquireSurfaceFrame(); return surface; } -// |Context| -void SurfaceContextVK::DidAcquireSurfaceFrame() const {} - -// |Context| -void SurfaceContextVK::DidFinishSurfaceFrame() const {} - #ifdef FML_OS_ANDROID vk::UniqueSurfaceKHR SurfaceContextVK::CreateAndroidSurface( diff --git a/impeller/renderer/backend/vulkan/surface_context_vk.h b/impeller/renderer/backend/vulkan/surface_context_vk.h index 2b8ef0befdb80..37a38e4dc9c2a 100644 --- a/impeller/renderer/backend/vulkan/surface_context_vk.h +++ b/impeller/renderer/backend/vulkan/surface_context_vk.h @@ -51,12 +51,6 @@ class SurfaceContextVK : public Context, // |Context| const std::shared_ptr& GetCapabilities() const override; - // |Context| - void DidAcquireSurfaceFrame() const override; - - // |Context| - void DidFinishSurfaceFrame() const override; - // |Context| void Shutdown() override; diff --git a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc index ddef275e68ce3..c5058591d1c86 100644 --- a/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc +++ b/impeller/renderer/backend/vulkan/swapchain_impl_vk.cc @@ -463,7 +463,6 @@ bool SwapchainImplVK::Present(const std::shared_ptr& image, return false; } } - context.DidFinishSurfaceFrame(); context.GetConcurrentWorkerTaskRunner()->PostTask( [&, index, image, current_frame = current_frame_] { diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index a2ba3f7e26f9f..b597e4fee1a88 100644 --- a/impeller/renderer/context.h +++ b/impeller/renderer/context.h @@ -154,20 +154,6 @@ class Context { /// virtual std::shared_ptr CreateCommandBuffer() const = 0; - //--------------------------------------------------------------------------- - /// @brief Indicate the beginning of a presentation cycle. - /// - /// This may or may not indicate a new frame, or there may be multiple - /// presents within a single frame due to platform view presence. - virtual void DidAcquireSurfaceFrame() const = 0; - - //--------------------------------------------------------------------------- - /// @brief Indicate the ending of a presentation cycle. - /// - /// This may or may not indicate a new frame, or there may be multiple - /// presents within a single frame due to platform view presence. - virtual void DidFinishSurfaceFrame() const = 0; - //---------------------------------------------------------------------------- /// @brief Force all pending asynchronous work to finish. This is /// achieved by deleting all owned concurrent message loops. diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 46ca72b0a815b..636601ddd3874 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -14,6 +14,19 @@ namespace impeller { +RenderTargetAllocator::RenderTargetAllocator( + std::shared_ptr allocator) + : allocator_(std::move(allocator)) {} + +void RenderTargetAllocator::Start() {} + +void RenderTargetAllocator::End() {} + +std::shared_ptr RenderTargetAllocator::CreateTexture( + const TextureDescriptor& desc) { + return allocator_->CreateTexture(desc); +} + RenderTarget::RenderTarget() = default; RenderTarget::~RenderTarget() = default; @@ -209,6 +222,7 @@ const std::optional& RenderTarget::GetStencilAttachment() RenderTarget RenderTarget::CreateOffscreen( const Context& context, + RenderTargetAllocator& allocator, ISize size, const std::string& label, AttachmentConfig color_attachment_config, @@ -235,7 +249,7 @@ RenderTarget RenderTarget::CreateOffscreen( color0.clear_color = color_attachment_config.clear_color; color0.load_action = color_attachment_config.load_action; color0.store_action = color_attachment_config.store_action; - color0.texture = context.GetResourceAllocator()->CreateTexture(color_tex0); + color0.texture = allocator.CreateTexture(color_tex0); if (!color0.texture) { return {}; @@ -244,7 +258,7 @@ RenderTarget RenderTarget::CreateOffscreen( target.SetColorAttachment(color0, 0u); if (stencil_attachment_config.has_value()) { - target.SetupStencilAttachment(context, size, false, label, + target.SetupStencilAttachment(context, allocator, size, false, label, stencil_attachment_config.value()); } else { target.SetStencilAttachment(std::nullopt); @@ -255,6 +269,7 @@ RenderTarget RenderTarget::CreateOffscreen( RenderTarget RenderTarget::CreateOffscreenMSAA( const Context& context, + RenderTargetAllocator& allocator, ISize size, const std::string& label, AttachmentConfigMSAA color_attachment_config, @@ -281,8 +296,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( color0_tex_desc.size = size; color0_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); - auto color0_msaa_tex = - context.GetResourceAllocator()->CreateTexture(color0_tex_desc); + auto color0_msaa_tex = allocator.CreateTexture(color0_tex_desc); if (!color0_msaa_tex) { VALIDATION_LOG << "Could not create multisample color texture."; return {}; @@ -302,8 +316,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( static_cast(TextureUsage::kRenderTarget) | static_cast(TextureUsage::kShaderRead); - auto color0_resolve_tex = - context.GetResourceAllocator()->CreateTexture(color0_resolve_tex_desc); + auto color0_resolve_tex = allocator.CreateTexture(color0_resolve_tex_desc); if (!color0_resolve_tex) { VALIDATION_LOG << "Could not create color texture."; return {}; @@ -324,7 +337,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( // Create MSAA stencil texture. if (stencil_attachment_config.has_value()) { - target.SetupStencilAttachment(context, size, true, label, + target.SetupStencilAttachment(context, allocator, size, true, label, stencil_attachment_config.value()); } else { target.SetStencilAttachment(std::nullopt); @@ -335,6 +348,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( void RenderTarget::SetupStencilAttachment( const Context& context, + RenderTargetAllocator& allocator, ISize size, bool msaa, const std::string& label, @@ -354,8 +368,7 @@ void RenderTarget::SetupStencilAttachment( stencil0.load_action = stencil_attachment_config.load_action; stencil0.store_action = stencil_attachment_config.store_action; stencil0.clear_stencil = 0u; - stencil0.texture = - context.GetResourceAllocator()->CreateTexture(stencil_tex0); + stencil0.texture = allocator.CreateTexture(stencil_tex0); if (!stencil0.texture) { return; // Error messages are handled by `Allocator::CreateTexture`. diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index bc54976a030a8..93d89aa2c350f 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -17,6 +17,35 @@ namespace impeller { class Context; +/// @brief a wrapper around the impeller [Allocator] instance that can be used +/// to provide caching of allocated render target textures. +class RenderTargetAllocator { + public: + explicit RenderTargetAllocator(std::shared_ptr allocator); + + virtual ~RenderTargetAllocator() = default; + + /// @brief Create a new render target texture, or recycle a previously + /// allocated render + /// target texture. + virtual std::shared_ptr CreateTexture( + const TextureDescriptor& desc); + + /// @brief Mark the beginning of a frame workload. + /// + /// This may be used to reset any tracking state on whether or not a + /// particular texture instance is still in use. + virtual void Start(); + + /// @brief Mark the end of a frame workload. + /// + /// This may be used to deallocate any unused textures. + virtual void End(); + + private: + std::shared_ptr allocator_; +}; + class RenderTarget final { public: struct AttachmentConfig { @@ -55,6 +84,7 @@ class RenderTarget final { static RenderTarget CreateOffscreen( const Context& context, + RenderTargetAllocator& allocator, ISize size, const std::string& label = "Offscreen", AttachmentConfig color_attachment_config = kDefaultColorAttachmentConfig, @@ -63,6 +93,7 @@ class RenderTarget final { static RenderTarget CreateOffscreenMSAA( const Context& context, + RenderTargetAllocator& allocator, ISize size, const std::string& label = "Offscreen MSAA", AttachmentConfigMSAA color_attachment_config = @@ -77,6 +108,7 @@ class RenderTarget final { bool IsValid() const; void SetupStencilAttachment(const Context& context, + RenderTargetAllocator& allocator, ISize size, bool msaa, const std::string& label = "Offscreen", diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index ac649603c3687..a0ec1350ba578 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -1149,7 +1149,9 @@ TEST_P(RendererTest, StencilMask) { stencil_config.load_action = LoadAction::kLoad; stencil_config.store_action = StoreAction::kDontCare; stencil_config.storage_mode = StorageMode::kHostVisible; - render_target.SetupStencilAttachment(*context, + auto render_target_allocator = + RenderTargetAllocator(context->GetResourceAllocator()); + render_target.SetupStencilAttachment(*context, render_target_allocator, render_target.GetRenderTargetSize(), true, "stencil", stencil_config); // Fill the stencil buffer with an checkerboard pattern. From 682e87c354ef75ec59968c00fedce07b05b47fdc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 18:36:36 -0700 Subject: [PATCH 14/20] ++ --- impeller/core/allocator.cc | 2 +- impeller/core/allocator_unittests.cc | 3 ++- impeller/entity/BUILD.gn | 2 +- impeller/renderer/render_target.h | 3 +-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/impeller/core/allocator.cc b/impeller/core/allocator.cc index c58f012e37157..1785534a73d5b 100644 --- a/impeller/core/allocator.cc +++ b/impeller/core/allocator.cc @@ -58,7 +58,7 @@ std::shared_ptr Allocator::CreateTexture( return OnCreateTexture(desc); } -void Allocator::DidAcquireSurfaceFrame() { } +void Allocator::DidAcquireSurfaceFrame() {} uint16_t Allocator::MinimumBytesPerRow(PixelFormat format) const { return BytesPerPixelForPixelFormat(format); diff --git a/impeller/core/allocator_unittests.cc b/impeller/core/allocator_unittests.cc index 6d69b78698d18..0cbd6d776e073 100644 --- a/impeller/core/allocator_unittests.cc +++ b/impeller/core/allocator_unittests.cc @@ -127,7 +127,8 @@ namespace testing { // allocator->DidAcquireSurfaceFrame(); // allocator->SetEnableRenderTargetTextureCache(true); -// // Create two textures of the same exact size/shape. Both should be marked as +// // Create two textures of the same exact size/shape. Both should be marked +// as // // used this frame, so the cached data set will contain two. // allocator->CreateTexture(desc); // allocator->CreateTexture(desc); diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index 714bfd079495f..35d0a24061327 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -242,8 +242,8 @@ impeller_component("entity") { "geometry/vertices_geometry.h", "inline_pass_context.cc", "inline_pass_context.h", - "render_target_cache.h", "render_target_cache.cc", + "render_target_cache.h", ] if (impeller_debug) { diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index 93d89aa2c350f..d808f9feeff1d 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -28,8 +28,7 @@ class RenderTargetAllocator { /// @brief Create a new render target texture, or recycle a previously /// allocated render /// target texture. - virtual std::shared_ptr CreateTexture( - const TextureDescriptor& desc); + virtual std::shared_ptr CreateTexture(const TextureDescriptor& desc); /// @brief Mark the beginning of a frame workload. /// From ce5034824a33f88415c0ca3a6fc0001a96881583 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 19:15:47 -0700 Subject: [PATCH 15/20] ++ --- impeller/BUILD.gn | 1 + impeller/core/allocator_unittests.cc | 137 +++++++++--------- impeller/core/texture_descriptor.h | 32 ++-- impeller/entity/BUILD.gn | 13 ++ impeller/entity/render_target_cache.cc | 6 +- impeller/entity/render_target_cache.h | 5 + .../entity/render_target_cache_unittests.cc | 65 +++++++++ .../renderer/backend/vulkan/surface_vk.cc | 2 - 8 files changed, 174 insertions(+), 87 deletions(-) create mode 100644 impeller/entity/render_target_cache_unittests.cc diff --git a/impeller/BUILD.gn b/impeller/BUILD.gn index d85c97214df1f..c55631be197ff 100644 --- a/impeller/BUILD.gn +++ b/impeller/BUILD.gn @@ -90,6 +90,7 @@ impeller_component("impeller_unittests") { "aiks:aiks_unittests", "display_list:display_list_unittests", "entity:entity_unittests", + "entity:render_target_cache_unittests", "fixtures", "geometry:geometry_unittests", "image:image_unittests", diff --git a/impeller/core/allocator_unittests.cc b/impeller/core/allocator_unittests.cc index 0cbd6d776e073..c9a540bb40c95 100644 --- a/impeller/core/allocator_unittests.cc +++ b/impeller/core/allocator_unittests.cc @@ -13,78 +13,71 @@ namespace impeller { namespace testing { -// class TestAllocator : public Allocator { -// public: -// TestAllocator() = default; - -// ~TestAllocator() = default; - -// std::vector GetCachedData() { -// return data_to_recycle_; -// }; - -// ISize GetMaxTextureSizeSupported() const override { -// return ISize(1024, 1024); -// }; - -// std::shared_ptr OnCreateBuffer( -// const DeviceBufferDescriptor& desc) override { -// return std::make_shared(desc); -// }; - -// virtual std::shared_ptr OnCreateTexture( -// const TextureDescriptor& desc) override { -// return std::make_shared(desc); -// }; -// }; - -// TEST(AllocatorTest, TextureDescriptorCompatibility) { -// // Size. -// { -// TextureDescriptor desc_a = {.size = ISize(100, 100)}; -// TextureDescriptor desc_b = {.size = ISize(100, 100)}; -// TextureDescriptor desc_c = {.size = ISize(101, 100)}; - -// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); -// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); -// } -// // Storage Mode. -// { -// TextureDescriptor desc_a = {.storage_mode = StorageMode::kDevicePrivate}; -// TextureDescriptor desc_b = {.storage_mode = StorageMode::kDevicePrivate}; -// TextureDescriptor desc_c = {.storage_mode = StorageMode::kHostVisible}; - -// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); -// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); -// } -// // Format. -// { -// TextureDescriptor desc_a = {.format = PixelFormat::kR8G8B8A8UNormInt}; -// TextureDescriptor desc_b = {.format = PixelFormat::kR8G8B8A8UNormInt}; -// TextureDescriptor desc_c = {.format = PixelFormat::kB10G10R10A10XR}; - -// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); -// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); -// } -// // Sample Count. -// { -// TextureDescriptor desc_a = {.sample_count = SampleCount::kCount4}; -// TextureDescriptor desc_b = {.sample_count = SampleCount::kCount4}; -// TextureDescriptor desc_c = {.sample_count = SampleCount::kCount1}; - -// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); -// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); -// } -// // Sample Count. -// { -// TextureDescriptor desc_a = {.type = TextureType::kTexture2DMultisample}; -// TextureDescriptor desc_b = {.type = TextureType::kTexture2DMultisample}; -// TextureDescriptor desc_c = {.type = TextureType::kTexture2D}; - -// ASSERT_TRUE(desc_a.IsCompatibleWith(desc_b)); -// ASSERT_FALSE(desc_a.IsCompatibleWith(desc_c)); -// } -// } +TEST(AllocatorTest, TextureDescriptorCompatibility) { + // Size. + { + TextureDescriptor desc_a = {.size = ISize(100, 100)}; + TextureDescriptor desc_b = {.size = ISize(100, 100)}; + TextureDescriptor desc_c = {.size = ISize(101, 100)}; + + ASSERT_EQ(desc_a, desc_b); + ASSERT_NE(desc_a, desc_c); + } + // Storage Mode. + { + TextureDescriptor desc_a = {.storage_mode = StorageMode::kDevicePrivate}; + TextureDescriptor desc_b = {.storage_mode = StorageMode::kDevicePrivate}; + TextureDescriptor desc_c = {.storage_mode = StorageMode::kHostVisible}; + + ASSERT_EQ(desc_a, desc_b); + ASSERT_NE(desc_a, desc_c); + } + // Format. + { + TextureDescriptor desc_a = {.format = PixelFormat::kR8G8B8A8UNormInt}; + TextureDescriptor desc_b = {.format = PixelFormat::kR8G8B8A8UNormInt}; + TextureDescriptor desc_c = {.format = PixelFormat::kB10G10R10A10XR}; + + ASSERT_EQ(desc_a, desc_b); + ASSERT_NE(desc_a, desc_c); + } + // Sample Count. + { + TextureDescriptor desc_a = {.sample_count = SampleCount::kCount4}; + TextureDescriptor desc_b = {.sample_count = SampleCount::kCount4}; + TextureDescriptor desc_c = {.sample_count = SampleCount::kCount1}; + + ASSERT_EQ(desc_a, desc_b); + ASSERT_NE(desc_a, desc_c); + } + // Sample Count. + { + TextureDescriptor desc_a = {.type = TextureType::kTexture2DMultisample}; + TextureDescriptor desc_b = {.type = TextureType::kTexture2DMultisample}; + TextureDescriptor desc_c = {.type = TextureType::kTexture2D}; + + ASSERT_EQ(desc_a, desc_b); + ASSERT_NE(desc_a, desc_c); + } + // Compression. + { + TextureDescriptor desc_a = {.compression_type = CompressionType::kLossless}; + TextureDescriptor desc_b = {.compression_type = CompressionType::kLossless}; + TextureDescriptor desc_c = {.compression_type = CompressionType::kLossy}; + + ASSERT_EQ(desc_a, desc_b); + ASSERT_NE(desc_a, desc_c); + } + // Mip Count. + { + TextureDescriptor desc_a = {.mip_count = 1}; + TextureDescriptor desc_b = {.mip_count = 1}; + TextureDescriptor desc_c = {.mip_count = 4}; + + ASSERT_EQ(desc_a, desc_b); + ASSERT_NE(desc_a, desc_c); + } +} // TEST(AllocatorTest, CachesTexturesWhenEnabled) { // auto allocator = std::make_shared(); diff --git a/impeller/core/texture_descriptor.h b/impeller/core/texture_descriptor.h index 41ffcdad61438..458ebd08645c5 100644 --- a/impeller/core/texture_descriptor.h +++ b/impeller/core/texture_descriptor.h @@ -46,7 +46,6 @@ struct TextureDescriptor { static_cast(TextureUsage::kShaderRead); SampleCount sample_count = SampleCount::kCount1; CompressionType compression_type = CompressionType::kLossless; - bool ignore_cache = false; constexpr size_t GetByteSizeOfBaseMipLevel() const { if (!IsValid()) { @@ -67,17 +66,26 @@ struct TextureDescriptor { return IsMultisampleCapable(type) ? count > 1 : count == 1; } - /// @brief Whether the [other] texture descriptor has the same properties. - /// - /// This is used to check compatibility of already allocated textures - /// in the render target cache.. - constexpr bool IsCompatibleWith(const TextureDescriptor& other) const { - return size == other.size && // - storage_mode == other.storage_mode && // - format == other.format && // - usage == other.usage && // - sample_count == other.sample_count && // - type == other.type; // + constexpr bool operator==(const TextureDescriptor& other) const { + return size == other.size && // + storage_mode == other.storage_mode && // + format == other.format && // + usage == other.usage && // + sample_count == other.sample_count && // + type == other.type && // + compression_type == other.compression_type && // + mip_count == other.mip_count; + } + + constexpr bool operator!=(const TextureDescriptor& other) const { + return size != other.size || // + storage_mode != other.storage_mode || // + format != other.format || // + usage != other.usage || // + sample_count != other.sample_count || // + type != other.type || // + compression_type != other.compression_type || // + mip_count != other.mip_count; } constexpr bool IsValid() const { diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index 35d0a24061327..b79a51a51672d 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -284,3 +284,16 @@ impeller_component("entity_unittests") { "../playground:playground_test", ] } + +impeller_component("render_target_cache_unittests") { + testonly = true + + sources = [ + "render_target_cache_unittests.cc", + ] + + deps = [ + ":entity", + "//flutter/testing:testing_lib", + ] +} diff --git a/impeller/entity/render_target_cache.cc b/impeller/entity/render_target_cache.cc index e1c697eb79b7b..fc2ebf807205b 100644 --- a/impeller/entity/render_target_cache.cc +++ b/impeller/entity/render_target_cache.cc @@ -27,6 +27,10 @@ void RenderTargetCache::End() { texture_data_.swap(retain); } +size_t RenderTargetCache::CachedTextureCount() const { + return texture_data_.size(); +} + std::shared_ptr RenderTargetCache::CreateTexture( const TextureDescriptor& desc) { FML_DCHECK(desc.storage_mode != StorageMode::kHostVisible); @@ -35,7 +39,7 @@ std::shared_ptr RenderTargetCache::CreateTexture( for (auto& td : texture_data_) { const auto other_desc = td.texture->GetTextureDescriptor(); - if (!td.used_this_frame && desc.IsCompatibleWith(other_desc)) { + if (!td.used_this_frame && desc == other_desc) { td.used_this_frame = true; return td.texture; } diff --git a/impeller/entity/render_target_cache.h b/impeller/entity/render_target_cache.h index 3ae6617f558eb..512ad12ba9024 100644 --- a/impeller/entity/render_target_cache.h +++ b/impeller/entity/render_target_cache.h @@ -28,6 +28,9 @@ class RenderTargetCache : public RenderTargetAllocator { std::shared_ptr CreateTexture( const TextureDescriptor& desc) override; + // visible for testing. + size_t CachedTextureCount() const; + private: struct TextureData { bool used_this_frame; @@ -35,6 +38,8 @@ class RenderTargetCache : public RenderTargetAllocator { }; std::vector texture_data_; + + FML_DISALLOW_COPY_AND_ASSIGN(RenderTargetCache); }; } // namespace impeller diff --git a/impeller/entity/render_target_cache_unittests.cc b/impeller/entity/render_target_cache_unittests.cc new file mode 100644 index 0000000000000..184803db1ca7a --- /dev/null +++ b/impeller/entity/render_target_cache_unittests.cc @@ -0,0 +1,65 @@ +// 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. + +#include +#include "flutter/testing/testing.h" +#include "impeller/core/allocator.h" +#include "impeller/core/texture_descriptor.h" +#include "impeller/entity/render_target_cache.h" +#include "impeller/renderer/testing/mocks.h" + +namespace impeller { +namespace testing { + +class TestAllocator : public Allocator { + public: + TestAllocator() = default; + + ~TestAllocator() = default; + + ISize GetMaxTextureSizeSupported() const override { + return ISize(1024, 1024); + }; + + std::shared_ptr OnCreateBuffer( + const DeviceBufferDescriptor& desc) override { + return std::make_shared(desc); + }; + + virtual std::shared_ptr OnCreateTexture( + const TextureDescriptor& desc) override { + return std::make_shared(desc); + }; +}; + +TEST(RenderTargetCacheTest, CachesUsedTexturesAcrossFrames) { + auto allocator = std::make_shared(); + auto render_target_cache = RenderTargetCache(allocator); + auto desc = TextureDescriptor{ + .format = PixelFormat::kR8G8B8A8UNormInt, + .size = ISize(100, 100), + .usage = static_cast(TextureUsage::kRenderTarget)}; + + render_target_cache.Start(); + // Create two textures of the same exact size/shape. Both should be marked + // as used this frame, so the cached data set will contain two. + render_target_cache.CreateTexture(desc); + render_target_cache.CreateTexture(desc); + + ASSERT_EQ(render_target_cache.CachedTextureCount(), 2u); + + render_target_cache.End(); + render_target_cache.Start(); + + // Next frame, only create one texture. The set will still contain two, + // but one will be removed at the end of the frame. + render_target_cache.CreateTexture(desc); + ASSERT_EQ(render_target_cache.CachedTextureCount(), 2u); + + render_target_cache.End(); + ASSERT_EQ(render_target_cache.CachedTextureCount(), 1u); +} + +} // namespace testing +} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/surface_vk.cc b/impeller/renderer/backend/vulkan/surface_vk.cc index d2f56d59f2465..95846affdc1c3 100644 --- a/impeller/renderer/backend/vulkan/surface_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_vk.cc @@ -25,7 +25,6 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( msaa_tex_desc.format = swapchain_image->GetPixelFormat(); msaa_tex_desc.size = swapchain_image->GetSize(); msaa_tex_desc.usage = static_cast(TextureUsage::kRenderTarget); - msaa_tex_desc.ignore_cache = true; std::shared_ptr msaa_tex; if (!swapchain_image->HasMSAATexture()) { @@ -48,7 +47,6 @@ std::unique_ptr SurfaceVK::WrapSwapchainImage( static_cast(TextureUsage::kRenderTarget); resolve_tex_desc.sample_count = SampleCount::kCount1; resolve_tex_desc.storage_mode = StorageMode::kDevicePrivate; - resolve_tex_desc.ignore_cache = true; std::shared_ptr resolve_tex = std::make_shared(context, // From 06377e71129fb630509e8ca02bd8e7a7099075d9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 19:16:38 -0700 Subject: [PATCH 16/20] ++ --- impeller/entity/BUILD.gn | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index b79a51a51672d..6e1ddc886a25e 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -288,9 +288,7 @@ impeller_component("entity_unittests") { impeller_component("render_target_cache_unittests") { testonly = true - sources = [ - "render_target_cache_unittests.cc", - ] + sources = [ "render_target_cache_unittests.cc" ] deps = [ ":entity", From 7302822005cb18b1e55f608504252fe16caa527e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 19:24:27 -0700 Subject: [PATCH 17/20] ++ --- impeller/core/allocator_unittests.cc | 61 ------------------- .../renderer/backend/vulkan/allocator_vk.cc | 2 +- .../renderer/backend/vulkan/allocator_vk.h | 1 + .../backend/vulkan/surface_context_vk.cc | 5 +- impeller/renderer/testing/mocks.h | 4 -- lib/ui/painting/image_decoder_unittests.cc | 4 -- 6 files changed, 6 insertions(+), 71 deletions(-) diff --git a/impeller/core/allocator_unittests.cc b/impeller/core/allocator_unittests.cc index c9a540bb40c95..c0c2268dfb151 100644 --- a/impeller/core/allocator_unittests.cc +++ b/impeller/core/allocator_unittests.cc @@ -79,66 +79,5 @@ TEST(AllocatorTest, TextureDescriptorCompatibility) { } } -// TEST(AllocatorTest, CachesTexturesWhenEnabled) { -// auto allocator = std::make_shared(); -// auto desc = TextureDescriptor{ -// .format = PixelFormat::kR8G8B8A8UNormInt, -// .size = ISize(100, 100), -// .usage = static_cast(TextureUsage::kRenderTarget)}; - -// allocator->DidAcquireSurfaceFrame(); -// allocator->CreateTexture(desc); - -// ASSERT_TRUE(allocator->GetCachedData().size() == 0); - -// allocator->SetEnableRenderTargetTextureCache(true); -// allocator->CreateTexture(desc); - -// ASSERT_TRUE(allocator->GetCachedData().size() == 1); -// } - -// TEST(AllocatorTest, DoesNotCacheNonRenderTargetTextures) { -// auto allocator = std::make_shared(); -// auto desc = TextureDescriptor{ -// .format = PixelFormat::kR8G8B8A8UNormInt, -// .size = ISize(100, 100), -// .usage = static_cast(TextureUsage::kShaderRead)}; - -// allocator->SetEnableRenderTargetTextureCache(true); -// allocator->DidAcquireSurfaceFrame(); -// allocator->CreateTexture(desc); - -// ASSERT_TRUE(allocator->GetCachedData().size() == 0); -// } - -// TEST(AllocatorTest, CachesUsedTexturesAcrossFrames) { -// auto allocator = std::make_shared(); -// auto desc = TextureDescriptor{ -// .format = PixelFormat::kR8G8B8A8UNormInt, -// .size = ISize(100, 100), -// .usage = static_cast(TextureUsage::kRenderTarget)}; - -// allocator->DidAcquireSurfaceFrame(); -// allocator->SetEnableRenderTargetTextureCache(true); -// // Create two textures of the same exact size/shape. Both should be marked -// as -// // used this frame, so the cached data set will contain two. -// allocator->CreateTexture(desc); -// allocator->CreateTexture(desc); - -// ASSERT_TRUE(allocator->GetCachedData().size() == 2); - -// allocator->DidFinishSurfaceFrame(); -// allocator->DidAcquireSurfaceFrame(); - -// // Next frame, only create one texture. The set will still contain two, -// // but one will be removed at the end of the frame. -// allocator->CreateTexture(desc); -// ASSERT_TRUE(allocator->GetCachedData().size() == 2); - -// allocator->DidFinishSurfaceFrame(); -// ASSERT_TRUE(allocator->GetCachedData().size() == 1); -// } - } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/allocator_vk.cc b/impeller/renderer/backend/vulkan/allocator_vk.cc index 4e35a416d99ba..2720a81587060 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.cc +++ b/impeller/renderer/backend/vulkan/allocator_vk.cc @@ -167,7 +167,7 @@ AllocatorVK::AllocatorVK(std::weak_ptr context, is_valid_ = true; } -AllocatorVK::~AllocatorVK() {} +AllocatorVK::~AllocatorVK() = default; // |Allocator| bool AllocatorVK::IsValid() const { diff --git a/impeller/renderer/backend/vulkan/allocator_vk.h b/impeller/renderer/backend/vulkan/allocator_vk.h index 05a5ab51e089f..d1753636d0b0b 100644 --- a/impeller/renderer/backend/vulkan/allocator_vk.h +++ b/impeller/renderer/backend/vulkan/allocator_vk.h @@ -51,6 +51,7 @@ class AllocatorVK final : public Allocator { // |Allocator| bool IsValid() const; + // |Allocator| void DidAcquireSurfaceFrame() override; // |Allocator| diff --git a/impeller/renderer/backend/vulkan/surface_context_vk.cc b/impeller/renderer/backend/vulkan/surface_context_vk.cc index 814c406430b67..7f2fdf82003c6 100644 --- a/impeller/renderer/backend/vulkan/surface_context_vk.cc +++ b/impeller/renderer/backend/vulkan/surface_context_vk.cc @@ -74,7 +74,10 @@ std::unique_ptr SurfaceContextVK::AcquireNextSurface() { impeller::PipelineLibraryVK::Cast(*pipeline_library) .DidAcquireSurfaceFrame(); } - parent_->GetResourceAllocator()->DidAcquireSurfaceFrame(); + auto allocator = parent_->GetResourceAllocator(); + if (allocator) { + allocator->DidAcquireSurfaceFrame(); + } return surface; } diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index be4607b806430..cd3910b1a9737 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -102,10 +102,6 @@ class MockImpellerContext : public Context { MOCK_CONST_METHOD0(GetPipelineLibrary, std::shared_ptr()); - MOCK_CONST_METHOD0(DidAcquireSurfaceFrame, void()); - - MOCK_CONST_METHOD0(DidFinishSurfaceFrame, void()); - MOCK_CONST_METHOD0(CreateCommandBuffer, std::shared_ptr()); MOCK_CONST_METHOD0(GetCapabilities, diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 2732f979952c1..b779a77ded17c 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -144,10 +144,6 @@ class TestImpellerContext : public impeller::Context { return nullptr; } - void DidAcquireSurfaceFrame() const override {} - - void DidFinishSurfaceFrame() const override {} - void Shutdown() override {} mutable size_t command_buffer_count_ = 0; From cf3453fae0c08459d21bd5cc47191326f12cdd2d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 14 Aug 2023 19:48:21 -0700 Subject: [PATCH 18/20] ++ --- ci/licenses_golden/excluded_files | 1 + ci/licenses_golden/licenses_flutter | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index b9c9db6b4b3f1..7dcd084f27295 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -135,6 +135,7 @@ ../../../flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc ../../../flutter/impeller/entity/entity_unittests.cc ../../../flutter/impeller/entity/geometry/geometry_unittests.cc +../../../flutter/impeller/entity/render_target_cache_unittests.cc ../../../flutter/impeller/fixtures ../../../flutter/impeller/geometry/README.md ../../../flutter/impeller/geometry/geometry_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 5ea50c1f38d6e..87ae635df37a1 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1271,6 +1271,8 @@ ORIGIN: ../../../flutter/impeller/entity/geometry/vertices_geometry.cc + ../../. ORIGIN: ../../../flutter/impeller/entity/geometry/vertices_geometry.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/entity/inline_pass_context.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/entity/inline_pass_context.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/entity/render_target_cache.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/entity/render_target_cache.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/entity/shaders/blending/advanced_blend.glsl + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/entity/shaders/blending/advanced_blend.vert + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/entity/shaders/blending/advanced_blend_color.frag + ../../../flutter/LICENSE @@ -3995,6 +3997,8 @@ FILE: ../../../flutter/impeller/entity/geometry/vertices_geometry.cc FILE: ../../../flutter/impeller/entity/geometry/vertices_geometry.h FILE: ../../../flutter/impeller/entity/inline_pass_context.cc FILE: ../../../flutter/impeller/entity/inline_pass_context.h +FILE: ../../../flutter/impeller/entity/render_target_cache.cc +FILE: ../../../flutter/impeller/entity/render_target_cache.h FILE: ../../../flutter/impeller/entity/shaders/blending/advanced_blend.glsl FILE: ../../../flutter/impeller/entity/shaders/blending/advanced_blend.vert FILE: ../../../flutter/impeller/entity/shaders/blending/advanced_blend_color.frag From ddd2663958ae4648193afdfb8d3cc6199ad92270 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 15 Aug 2023 13:52:25 -0700 Subject: [PATCH 19/20] Update impeller/entity/render_target_cache_unittests.cc Co-authored-by: Brandon DeRosier --- impeller/entity/render_target_cache_unittests.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/entity/render_target_cache_unittests.cc b/impeller/entity/render_target_cache_unittests.cc index 184803db1ca7a..d8086b32aec44 100644 --- a/impeller/entity/render_target_cache_unittests.cc +++ b/impeller/entity/render_target_cache_unittests.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include + #include "flutter/testing/testing.h" #include "impeller/core/allocator.h" #include "impeller/core/texture_descriptor.h" From dfbb8051fbe04518428fdb8c4c3ad55035709022 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 15 Aug 2023 15:34:08 -0700 Subject: [PATCH 20/20] Update texture_descriptor.h --- impeller/core/texture_descriptor.h | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/impeller/core/texture_descriptor.h b/impeller/core/texture_descriptor.h index 458ebd08645c5..feb012bb832eb 100644 --- a/impeller/core/texture_descriptor.h +++ b/impeller/core/texture_descriptor.h @@ -78,14 +78,7 @@ struct TextureDescriptor { } constexpr bool operator!=(const TextureDescriptor& other) const { - return size != other.size || // - storage_mode != other.storage_mode || // - format != other.format || // - usage != other.usage || // - sample_count != other.sample_count || // - type != other.type || // - compression_type != other.compression_type || // - mip_count != other.mip_count; + return !(*this == other); } constexpr bool IsValid() const {