From d3ae0219f073c2ba865414372aa90cbd98ec269a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 26 Feb 2024 15:30:52 -0800 Subject: [PATCH 1/6] Revert "[Impeller] disble render pass caches. (#50976)" This reverts commit a93ca854550431f35d38da8e447ea22ffc4a2cb4. --- .../vulkan/render_pass_cache_unittests.cc | 36 ++++++++++++++++++- .../renderer/backend/vulkan/render_pass_vk.cc | 18 ++++++++-- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc index bb850c3e7d83b..e2fc63ea5796a 100644 --- a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc +++ b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc @@ -11,7 +11,41 @@ namespace impeller { namespace testing { -// +using RendererTest = PlaygroundTest; + +TEST_P(RendererTest, CachesRenderPassAndFramebuffer) { + if (GetBackend() != PlaygroundBackend::kVulkan) { + GTEST_SKIP() << "Test only applies to Vulkan"; + } + + auto allocator = std::make_shared( + GetContext()->GetResourceAllocator()); + + auto render_target = RenderTarget::CreateOffscreenMSAA( + *GetContext(), *allocator, {100, 100}, 1); + auto resolve_texture = + render_target.GetColorAttachments().find(0u)->second.resolve_texture; + auto& texture_vk = TextureVK::Cast(*resolve_texture); + + EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); + EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); + + auto buffer = GetContext()->CreateCommandBuffer(); + auto render_pass = buffer->CreateRenderPass(render_target); + + EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); + EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + + render_pass->EncodeCommands(); + GetContext()->GetCommandQueue()->Submit({buffer}); + + // Can be reused without error. + auto buffer_2 = GetContext()->CreateCommandBuffer(); + auto render_pass_2 = buffer_2->CreateRenderPass(render_target); + + EXPECT_TRUE(render_pass_2->EncodeCommands()); + EXPECT_TRUE(GetContext()->GetCommandQueue()->Submit({buffer_2}).ok()); +} } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index b2f8bde4c114c..28a7013127775 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -170,16 +170,26 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, return true; }); + SharedHandleVK recycled_render_pass; + SharedHandleVK recycled_framebuffer; + if (resolve_image_vk_) { + recycled_render_pass = TextureVK::Cast(*resolve_image_vk_).GetRenderPass(); + recycled_framebuffer = TextureVK::Cast(*resolve_image_vk_).GetFramebuffer(); + } + const auto& target_size = render_target_.GetRenderTargetSize(); - render_pass_ = CreateVKRenderPass(vk_context, nullptr, command_buffer_); + render_pass_ = + CreateVKRenderPass(vk_context, recycled_render_pass, command_buffer_); if (!render_pass_) { VALIDATION_LOG << "Could not create renderpass."; is_valid_ = false; return; } - auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass_); + auto framebuffer = (recycled_framebuffer == nullptr) + ? CreateVKFramebuffer(vk_context, *render_pass_) + : recycled_framebuffer; if (!framebuffer) { VALIDATION_LOG << "Could not create framebuffer."; is_valid_ = false; @@ -190,6 +200,10 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, is_valid_ = false; return; } + if (resolve_image_vk_) { + TextureVK::Cast(*resolve_image_vk_).SetFramebuffer(framebuffer); + TextureVK::Cast(*resolve_image_vk_).SetRenderPass(render_pass_); + } auto clear_values = GetVKClearValues(render_target_); From 47250708e8c8926ea6592316fd2a9f6e8b87858d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 26 Feb 2024 16:30:14 -0800 Subject: [PATCH 2/6] [Impeller] cache entire render targets instead of just the allocations. --- impeller/aiks/aiks_unittests.cc | 20 ++- impeller/aiks/picture.cc | 14 +- .../checkerboard_contents_unittests.cc | 8 +- impeller/entity/contents/content_context.cc | 8 +- .../tiled_texture_contents_unittests.cc | 8 +- .../contents/vertices_contents_unittests.cc | 8 +- impeller/entity/entity_pass.cc | 12 +- .../entity/entity_pass_target_unittests.cc | 8 +- impeller/entity/entity_unittests.cc | 22 +-- impeller/entity/render_target_cache.cc | 87 ++++++++--- impeller/entity/render_target_cache.h | 36 +++-- .../entity/render_target_cache_unittests.cc | 142 +++++++++--------- .../renderer/backend/vulkan/context_vk.cc | 2 +- .../vulkan/render_pass_cache_unittests.cc | 40 ++--- .../renderer/backend/vulkan/render_pass_vk.cc | 5 - impeller/renderer/render_target.cc | 128 ++++++++-------- impeller/renderer/render_target.h | 140 ++++++++++------- impeller/renderer/renderer_unittests.cc | 12 +- 18 files changed, 386 insertions(+), 314 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 37dc2ba536996..8c8de9ac26ed4 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3350,8 +3350,9 @@ TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) { size_t max_mip_count = 0; for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); ++it) { - max_mip_count = - std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); + // max_mip_count = + // std::max(it->texture->GetTextureDescriptor().mip_count, + // max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); } @@ -3380,8 +3381,9 @@ TEST_P(AiksTest, GaussianBlurMipMapNestedLayer) { size_t max_mip_count = 0; for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); ++it) { - max_mip_count = - std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); + // max_mip_count = + // std::max(it->texture->GetTextureDescriptor().mip_count, + // max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); // The log is FML_DLOG, so only check in debug builds. @@ -3416,8 +3418,9 @@ TEST_P(AiksTest, GaussianBlurMipMapImageFilter) { size_t max_mip_count = 0; for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); ++it) { - max_mip_count = - std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); + // max_mip_count = + // std::max(it->render_target->GetTextureDescriptor().mip_count, + // max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); // The log is FML_DLOG, so only check in debug builds. @@ -3458,8 +3461,9 @@ TEST_P(AiksTest, GaussianBlurMipMapSolidColor) { size_t max_mip_count = 0; for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); ++it) { - max_mip_count = - std::max(it->texture->GetTextureDescriptor().mip_count, max_mip_count); + // max_mip_count = + // std::max(it->texture->GetTextureDescriptor().mip_count, + // max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); // The log is FML_DLOG, so only check in debug builds. diff --git a/impeller/aiks/picture.cc b/impeller/aiks/picture.cc index 2b811b8abf4e8..77f7fa8dbcfd0 100644 --- a/impeller/aiks/picture.cc +++ b/impeller/aiks/picture.cc @@ -60,10 +60,9 @@ std::shared_ptr Picture::RenderToTexture( RenderTargetAllocator(impeller_context->GetResourceAllocator()); RenderTarget target; if (impeller_context->GetCapabilities()->SupportsOffscreenMSAA()) { - target = RenderTarget::CreateOffscreenMSAA( - *impeller_context, // context - render_target_allocator, // allocator - size, // size + target = render_target_allocator.CreateOffscreenMSAA( + *impeller_context, // context + size, // size /*mip_count=*/1, "Picture Snapshot MSAA", // label RenderTarget:: @@ -71,10 +70,9 @@ std::shared_ptr Picture::RenderToTexture( std::nullopt // stencil_attachment_config ); } else { - target = RenderTarget::CreateOffscreen( - *impeller_context, // context - render_target_allocator, // allocator - size, // size + target = render_target_allocator.CreateOffscreen( + *impeller_context, // context + size, // size /*mip_count=*/1, "Picture Snapshot", // label RenderTarget::kDefaultColorAttachmentConfig, // color_attachment_config diff --git a/impeller/entity/contents/checkerboard_contents_unittests.cc b/impeller/entity/contents/checkerboard_contents_unittests.cc index 61f5aa1ce585a..a79cc115f544c 100644 --- a/impeller/entity/contents/checkerboard_contents_unittests.cc +++ b/impeller/entity/contents/checkerboard_contents_unittests.cc @@ -34,10 +34,10 @@ TEST_P(EntityTest, RendersWithoutError) { auto content_context = GetContentContext(); auto buffer = content_context->GetContext()->CreateCommandBuffer(); - auto render_target = RenderTarget::CreateOffscreenMSAA( - *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}, - /*mip_count=*/1); + auto render_target = + GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA( + *content_context->GetContext(), {100, 100}, + /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); auto recording_pass = std::make_shared( render_pass, GetContext(), render_target); diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 2ab8d93599fb8..f1566c1773543 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -490,13 +490,13 @@ fml::StatusOr ContentContext::MakeSubpass( : std::optional(); if (context->GetCapabilities()->SupportsOffscreenMSAA() && msaa_enabled) { - subpass_target = RenderTarget::CreateOffscreenMSAA( - *context, *GetRenderTargetCache(), texture_size, + subpass_target = GetRenderTargetCache()->CreateOffscreenMSAA( + *context, texture_size, /*mip_count=*/mip_count, SPrintF("%s Offscreen", label.c_str()), RenderTarget::kDefaultColorAttachmentConfigMSAA, depth_stencil_config); } else { - subpass_target = RenderTarget::CreateOffscreen( - *context, *GetRenderTargetCache(), texture_size, + subpass_target = GetRenderTargetCache()->CreateOffscreen( + *context, texture_size, /*mip_count=*/mip_count, SPrintF("%s Offscreen", label.c_str()), RenderTarget::kDefaultColorAttachmentConfig, depth_stencil_config); } diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index 5100a56b03a5e..b9ff3d9e01882 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -32,10 +32,10 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipeline) { auto content_context = GetContentContext(); auto buffer = content_context->GetContext()->CreateCommandBuffer(); - auto render_target = RenderTarget::CreateOffscreenMSAA( - *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}, - /*mip_count=*/1); + auto render_target = + GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA( + *content_context->GetContext(), {100, 100}, + /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); auto recording_pass = std::make_shared( render_pass, GetContext(), render_target); diff --git a/impeller/entity/contents/vertices_contents_unittests.cc b/impeller/entity/contents/vertices_contents_unittests.cc index 67b6e431748da..86b01643019c5 100644 --- a/impeller/entity/contents/vertices_contents_unittests.cc +++ b/impeller/entity/contents/vertices_contents_unittests.cc @@ -60,10 +60,10 @@ TEST_P(EntityTest, RendersDstPerColorWithAlpha) { auto content_context = GetContentContext(); auto buffer = content_context->GetContext()->CreateCommandBuffer(); - auto render_target = RenderTarget::CreateOffscreenMSAA( - *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}, - /*mip_count=*/1); + auto render_target = + GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA( + *content_context->GetContext(), {100, 100}, + /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); auto recording_pass = std::make_shared( render_pass, GetContext(), render_target); diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 1e485b9c4781e..9da1ae322568b 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -292,9 +292,8 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, RenderTarget target; if (context->GetCapabilities()->SupportsOffscreenMSAA()) { - target = RenderTarget::CreateOffscreenMSAA( + target = renderer.GetRenderTargetCache()->CreateOffscreenMSAA( /*context=*/*context, - /*allocator=*/*renderer.GetRenderTargetCache(), /*size=*/size, /*mip_count=*/mip_count, /*label=*/"EntityPass", @@ -308,10 +307,9 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer, /*stencil_attachment_config=*/ kDefaultStencilConfig); } else { - target = RenderTarget::CreateOffscreen( - *context, // context - *renderer.GetRenderTargetCache(), // allocator - size, // size + target = renderer.GetRenderTargetCache()->CreateOffscreen( + *context, // context + size, // size /*mip_count=*/mip_count, "EntityPass", // label RenderTarget::AttachmentConfig{ @@ -485,7 +483,7 @@ bool EntityPass::Render(ContentContext& renderer, // provided by the caller. else { root_render_target.SetupDepthStencilAttachments( - *renderer.GetContext(), *renderer.GetRenderTargetCache(), + *renderer.GetContext(), *renderer.GetContext()->GetResourceAllocator(), color0.texture->GetSize(), renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA(), "ImpellerOnscreen", kDefaultStencilConfig); diff --git a/impeller/entity/entity_pass_target_unittests.cc b/impeller/entity/entity_pass_target_unittests.cc index 108da36230506..c510a44d85a59 100644 --- a/impeller/entity/entity_pass_target_unittests.cc +++ b/impeller/entity/entity_pass_target_unittests.cc @@ -24,10 +24,10 @@ TEST_P(EntityPassTargetTest, SwapWithMSAATexture) { } auto content_context = GetContentContext(); auto buffer = content_context->GetContext()->CreateCommandBuffer(); - auto render_target = RenderTarget::CreateOffscreenMSAA( - *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}, - /*mip_count=*/1); + auto render_target = + GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA( + *content_context->GetContext(), {100, 100}, + /*mip_count=*/1); auto entity_pass_target = EntityPassTarget(render_target, false, false); diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index 0d0da4791cc77..bca22894ea38a 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2248,10 +2248,10 @@ TEST_P(EntityTest, RuntimeEffectCanSuccessfullyRender) { // Create a render target with a depth-stencil, similar to how EntityPass // does. - RenderTarget target = RenderTarget::CreateOffscreenMSAA( - *GetContext(), *GetContentContext()->GetRenderTargetCache(), - {GetWindowSize().width, GetWindowSize().height}, 1, - "RuntimeEffect Texture"); + RenderTarget target = + GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA( + *GetContext(), {GetWindowSize().width, GetWindowSize().height}, 1, + "RuntimeEffect Texture"); testing::MockRenderPass pass(GetContext(), target); ASSERT_TRUE(contents->Render(*GetContentContext(), entity, pass)); @@ -2597,11 +2597,11 @@ class TestRenderTargetAllocator : public RenderTargetAllocator { ~TestRenderTargetAllocator() = default; - std::shared_ptr CreateTexture( - const TextureDescriptor& desc) override { - allocated_.push_back(desc); - return RenderTargetAllocator::CreateTexture(desc); - } + // std::shared_ptr CreateTexture( + // const TextureDescriptor& desc) override { + // allocated_.push_back(desc); + // return RenderTargetAllocator::CreateTexture(desc); + // } void Start() override { RenderTargetAllocator::Start(); } @@ -2643,8 +2643,8 @@ TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) { .load_action = LoadAction::kClear, .store_action = StoreAction::kDontCare, .clear_color = Color::BlackTransparent()}; - auto rt = RenderTarget::CreateOffscreen( - *GetContext(), *test_allocator, ISize::MakeWH(1000, 1000), + auto rt = test_allocator->CreateOffscreen( + *GetContext(), ISize::MakeWH(1000, 1000), /*mip_count=*/1, "Offscreen", RenderTarget::kDefaultColorAttachmentConfig, stencil_config); auto content_context = ContentContext( diff --git a/impeller/entity/render_target_cache.cc b/impeller/entity/render_target_cache.cc index 89e16289092a8..1e86cb2808bff 100644 --- a/impeller/entity/render_target_cache.cc +++ b/impeller/entity/render_target_cache.cc @@ -11,47 +11,86 @@ RenderTargetCache::RenderTargetCache(std::shared_ptr allocator) : RenderTargetAllocator(std::move(allocator)) {} void RenderTargetCache::Start() { - for (auto& td : texture_data_) { + for (auto& td : render_pass_data_) { td.used_this_frame = false; } } void RenderTargetCache::End() { - std::vector retain; + std::vector retain; - for (const auto& td : texture_data_) { + for (const auto& td : render_pass_data_) { if (td.used_this_frame) { retain.push_back(td); } } - texture_data_.swap(retain); + render_pass_data_.swap(retain); } -size_t RenderTargetCache::CachedTextureCount() const { - return texture_data_.size(); +RenderTarget RenderTargetCache::CreateOffscreen( + const Context& context, + ISize size, + int mip_count, + const std::string& label, + RenderTarget::AttachmentConfig color_attachment_config, + std::optional stencil_attachment_config) { + auto config = RenderTargetConfig{ + .size = size, + .mip_count = static_cast(mip_count), + .has_msaa = false, + .has_depth_stencil = stencil_attachment_config.has_value(), + }; + for (auto& render_target_data : render_pass_data_) { + const auto other_config = render_target_data.render_target.ToConfig(); + if (!render_target_data.used_this_frame && other_config == config) { + render_target_data.used_this_frame = true; + return render_target_data.render_target; + } + } + RenderTarget created_target = RenderTargetAllocator::CreateOffscreen( + context, size, mip_count, label, color_attachment_config, + stencil_attachment_config); + if (!created_target.IsValid()) { + return created_target; + } + render_pass_data_.push_back( + RenderPassData{.used_this_frame = true, .render_target = created_target}); + return created_target; } -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(); - FML_DCHECK(td.texture != nullptr); - if (!td.used_this_frame && desc == other_desc) { - td.used_this_frame = true; - return td.texture; +RenderTarget RenderTargetCache::CreateOffscreenMSAA( + const Context& context, + ISize size, + int mip_count, + const std::string& label, + RenderTarget::AttachmentConfigMSAA color_attachment_config, + std::optional stencil_attachment_config) { + auto config = RenderTargetConfig{ + .size = size, + .mip_count = static_cast(mip_count), + .has_msaa = true, + .has_depth_stencil = stencil_attachment_config.has_value(), + }; + for (auto& render_target_data : render_pass_data_) { + const auto other_config = render_target_data.render_target.ToConfig(); + if (!render_target_data.used_this_frame && other_config == config) { + render_target_data.used_this_frame = true; + return render_target_data.render_target; } } - auto result = RenderTargetAllocator::CreateTexture(desc); - if (result == nullptr) { - return result; + RenderTarget created_target = RenderTargetAllocator::CreateOffscreenMSAA( + context, size, mip_count, label, color_attachment_config, + stencil_attachment_config); + if (!created_target.IsValid()) { + return created_target; } - texture_data_.push_back( - TextureData{.used_this_frame = true, .texture = result}); - return result; + render_pass_data_.push_back( + RenderPassData{.used_this_frame = true, .render_target = created_target}); + return created_target; +} + +size_t RenderTargetCache::CachedTextureCount() const { + return render_pass_data_.size(); } } // namespace impeller diff --git a/impeller/entity/render_target_cache.h b/impeller/entity/render_target_cache.h index 382781b793b7b..ccb2bd4fd3d72 100644 --- a/impeller/entity/render_target_cache.h +++ b/impeller/entity/render_target_cache.h @@ -25,20 +25,36 @@ class RenderTargetCache : public RenderTargetAllocator { // |RenderTargetAllocator| void End() override; - // |RenderTargetAllocator| - std::shared_ptr CreateTexture( - const TextureDescriptor& desc) override; + RenderTarget CreateOffscreen( + const Context& context, + ISize size, + int mip_count, + const std::string& label = "Offscreen", + RenderTarget::AttachmentConfig color_attachment_config = + RenderTarget::kDefaultColorAttachmentConfig, + std::optional stencil_attachment_config = + RenderTarget::kDefaultStencilAttachmentConfig) override; + + RenderTarget CreateOffscreenMSAA( + const Context& context, + ISize size, + int mip_count, + const std::string& label = "Offscreen MSAA", + RenderTarget::AttachmentConfigMSAA color_attachment_config = + RenderTarget::kDefaultColorAttachmentConfigMSAA, + std::optional stencil_attachment_config = + RenderTarget::kDefaultStencilAttachmentConfig) override; // visible for testing. size_t CachedTextureCount() const; private: - struct TextureData { + struct RenderPassData { bool used_this_frame; - std::shared_ptr texture; + RenderTarget render_target; }; - std::vector texture_data_; + std::vector render_pass_data_; RenderTargetCache(const RenderTargetCache&) = delete; @@ -46,13 +62,13 @@ class RenderTargetCache : public RenderTargetAllocator { public: /// Visible for testing. - std::vector::const_iterator GetTextureDataBegin() const { - return texture_data_.begin(); + std::vector::const_iterator GetTextureDataBegin() const { + return render_pass_data_.begin(); } /// Visible for testing. - std::vector::const_iterator GetTextureDataEnd() const { - return texture_data_.end(); + std::vector::const_iterator GetTextureDataEnd() const { + return render_pass_data_.end(); } }; diff --git a/impeller/entity/render_target_cache_unittests.cc b/impeller/entity/render_target_cache_unittests.cc index fc402f87fa9f5..4c4c4c4a817f2 100644 --- a/impeller/entity/render_target_cache_unittests.cc +++ b/impeller/entity/render_target_cache_unittests.cc @@ -13,77 +13,77 @@ 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 { - if (should_fail) { - return nullptr; - } - return std::make_shared(desc); - }; - - virtual std::shared_ptr OnCreateTexture( - const TextureDescriptor& desc) override { - if (should_fail) { - return nullptr; - } - return std::make_shared(desc); - }; - - bool should_fail = false; -}; - -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); -} - -TEST(RenderTargetCacheTest, DoesNotPersistFailedAllocations) { - 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(); - allocator->should_fail = true; - - ASSERT_EQ(render_target_cache.CreateTexture(desc), nullptr); - ASSERT_EQ(render_target_cache.CachedTextureCount(), 0u); -} +// 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 { +// if (should_fail) { +// return nullptr; +// } +// return std::make_shared(desc); +// }; + +// virtual std::shared_ptr OnCreateTexture( +// const TextureDescriptor& desc) override { +// if (should_fail) { +// return nullptr; +// } +// return std::make_shared(desc); +// }; + +// bool should_fail = false; +// }; + +// 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); +// } + +// TEST(RenderTargetCacheTest, DoesNotPersistFailedAllocations) { +// 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(); +// allocator->should_fail = true; + +// ASSERT_EQ(render_target_cache.CreateTexture(desc), nullptr); +// ASSERT_EQ(render_target_cache.CachedTextureCount(), 0u); +// } } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index d8fde32f3756e..6e99bdd2973ce 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -580,7 +580,7 @@ std::shared_ptr ContextVK::GetCommandQueue() const { void ContextVK::InitializeCommonlyUsedShadersIfNeeded() const { RenderTargetAllocator rt_allocator(GetResourceAllocator()); RenderTarget render_target = - RenderTarget::CreateOffscreenMSAA(*this, rt_allocator, {1, 1}, 1); + rt_allocator.CreateOffscreenMSAA(*this, {1, 1}, 1); RenderPassBuilderVK builder; for (const auto& [bind_point, color] : render_target.GetColorAttachments()) { diff --git a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc index e2fc63ea5796a..4399891f0cb55 100644 --- a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc +++ b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc @@ -18,33 +18,33 @@ TEST_P(RendererTest, CachesRenderPassAndFramebuffer) { GTEST_SKIP() << "Test only applies to Vulkan"; } - auto allocator = std::make_shared( - GetContext()->GetResourceAllocator()); + // auto allocator = std::make_shared( + // GetContext()->GetResourceAllocator()); - auto render_target = RenderTarget::CreateOffscreenMSAA( - *GetContext(), *allocator, {100, 100}, 1); - auto resolve_texture = - render_target.GetColorAttachments().find(0u)->second.resolve_texture; - auto& texture_vk = TextureVK::Cast(*resolve_texture); + // auto render_target = RenderTarget::CreateOffscreenMSAA( + // *GetContext(), *allocator, {100, 100}, 1); + // auto resolve_texture = + // render_target.GetColorAttachments().find(0u)->second.resolve_texture; + // auto& texture_vk = TextureVK::Cast(*resolve_texture); - EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); - EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); + // EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); + // EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); - auto buffer = GetContext()->CreateCommandBuffer(); - auto render_pass = buffer->CreateRenderPass(render_target); + // auto buffer = GetContext()->CreateCommandBuffer(); + // auto render_pass = buffer->CreateRenderPass(render_target); - EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); - EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + // EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); + // EXPECT_NE(texture_vk.GetRenderPass(), nullptr); - render_pass->EncodeCommands(); - GetContext()->GetCommandQueue()->Submit({buffer}); + // render_pass->EncodeCommands(); + // GetContext()->GetCommandQueue()->Submit({buffer}); - // Can be reused without error. - auto buffer_2 = GetContext()->CreateCommandBuffer(); - auto render_pass_2 = buffer_2->CreateRenderPass(render_target); + // // Can be reused without error. + // auto buffer_2 = GetContext()->CreateCommandBuffer(); + // auto render_pass_2 = buffer_2->CreateRenderPass(render_target); - EXPECT_TRUE(render_pass_2->EncodeCommands()); - EXPECT_TRUE(GetContext()->GetCommandQueue()->Submit({buffer_2}).ok()); + // EXPECT_TRUE(render_pass_2->EncodeCommands()); + // EXPECT_TRUE(GetContext()->GetCommandQueue()->Submit({buffer_2}).ok()); } } // namespace testing diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 28a7013127775..4d3a514926c0b 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -129,11 +129,6 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( TextureVK::Cast(*stencil->texture).SetLayout(barrier); } - // There may exist a previous recycled render pass that we can continue using. - // This is probably compatible with the render pass we are about to construct, - // but I have not conclusively proven this. If there are scenarios that - // produce validation warnings, we could use them to determine if we need - // additional checks at this point to determine reusability. if (recycled_renderpass != nullptr) { return recycled_renderpass; } diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index 59ffc41b029bc..6a79542c23fd6 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -15,19 +15,6 @@ 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; @@ -221,14 +208,60 @@ const std::optional& RenderTarget::GetStencilAttachment() return stencil_; } -RenderTarget RenderTarget::CreateOffscreen( +size_t RenderTarget::GetTotalAttachmentCount() const { + size_t count = 0u; + for (const auto& [_, color] : colors_) { + if (color.texture) { + count++; + } + if (color.resolve_texture) { + count++; + } + } + if (depth_.has_value()) { + count++; + } + if (stencil_.has_value()) { + count++; + } + return count; +} + +std::string RenderTarget::ToString() const { + std::stringstream stream; + + for (const auto& [index, color] : colors_) { + stream << SPrintF("Color[%zu]=(%s)", index, + ColorAttachmentToString(color).c_str()); + } + if (depth_) { + stream << ","; + stream << SPrintF("Depth=(%s)", + DepthAttachmentToString(depth_.value()).c_str()); + } + if (stencil_) { + stream << ","; + stream << SPrintF("Stencil=(%s)", + StencilAttachmentToString(stencil_.value()).c_str()); + } + return stream.str(); +} + +RenderTargetAllocator::RenderTargetAllocator( + std::shared_ptr allocator) + : allocator_(std::move(allocator)) {} + +void RenderTargetAllocator::Start() {} + +void RenderTargetAllocator::End() {} + +RenderTarget RenderTargetAllocator::CreateOffscreen( const Context& context, - RenderTargetAllocator& allocator, ISize size, int mip_count, const std::string& label, - AttachmentConfig color_attachment_config, - std::optional stencil_attachment_config) { + RenderTarget::AttachmentConfig color_attachment_config, + std::optional stencil_attachment_config) { if (size.IsEmpty()) { return {}; } @@ -247,7 +280,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 = allocator.CreateTexture(color_tex0); + color0.texture = allocator_->CreateTexture(color_tex0); if (!color0.texture) { return {}; @@ -256,7 +289,8 @@ RenderTarget RenderTarget::CreateOffscreen( target.SetColorAttachment(color0, 0u); if (stencil_attachment_config.has_value()) { - target.SetupDepthStencilAttachments(context, allocator, size, false, label, + target.SetupDepthStencilAttachments(context, *allocator_, size, false, + label, stencil_attachment_config.value()); } else { target.SetStencilAttachment(std::nullopt); @@ -265,14 +299,13 @@ RenderTarget RenderTarget::CreateOffscreen( return target; } -RenderTarget RenderTarget::CreateOffscreenMSAA( +RenderTarget RenderTargetAllocator::CreateOffscreenMSAA( const Context& context, - RenderTargetAllocator& allocator, ISize size, int mip_count, const std::string& label, - AttachmentConfigMSAA color_attachment_config, - std::optional stencil_attachment_config) { + RenderTarget::AttachmentConfigMSAA color_attachment_config, + std::optional stencil_attachment_config) { if (size.IsEmpty()) { return {}; } @@ -295,7 +328,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( color0_tex_desc.storage_mode = StorageMode::kDevicePrivate; } - auto color0_msaa_tex = allocator.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 {}; @@ -316,7 +349,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( static_cast(TextureUsage::kShaderRead); color0_resolve_tex_desc.mip_count = mip_count; - auto color0_resolve_tex = allocator.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 {}; @@ -348,7 +381,7 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( // Create MSAA stencil texture. if (stencil_attachment_config.has_value()) { - target.SetupDepthStencilAttachments(context, allocator, size, true, label, + target.SetupDepthStencilAttachments(context, *allocator_, size, true, label, stencil_attachment_config.value()); } else { target.SetDepthAttachment(std::nullopt); @@ -360,11 +393,11 @@ RenderTarget RenderTarget::CreateOffscreenMSAA( void RenderTarget::SetupDepthStencilAttachments( const Context& context, - RenderTargetAllocator& allocator, + Allocator& allocator, ISize size, bool msaa, const std::string& label, - AttachmentConfig stencil_attachment_config) { + RenderTarget::AttachmentConfig stencil_attachment_config) { TextureDescriptor depth_stencil_texture_desc; depth_stencil_texture_desc.storage_mode = stencil_attachment_config.storage_mode; @@ -402,43 +435,4 @@ void RenderTarget::SetupDepthStencilAttachments( SetStencilAttachment(std::move(stencil0)); } -size_t RenderTarget::GetTotalAttachmentCount() const { - size_t count = 0u; - for (const auto& [_, color] : colors_) { - if (color.texture) { - count++; - } - if (color.resolve_texture) { - count++; - } - } - if (depth_.has_value()) { - count++; - } - if (stencil_.has_value()) { - count++; - } - return count; -} - -std::string RenderTarget::ToString() const { - std::stringstream stream; - - for (const auto& [index, color] : colors_) { - stream << SPrintF("Color[%zu]=(%s)", index, - ColorAttachmentToString(color).c_str()); - } - if (depth_) { - stream << ","; - stream << SPrintF("Depth=(%s)", - DepthAttachmentToString(depth_.value()).c_str()); - } - if (stencil_) { - stream << ","; - stream << SPrintF("Stencil=(%s)", - StencilAttachmentToString(stencil_.value()).c_str()); - } - return stream.str(); -} - } // namespace impeller diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index fc3fd345cbc60..0bfce5137c8fa 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -9,7 +9,7 @@ #include #include -#include "flutter/fml/macros.h" +#include "flutter/fml/hash_combine.h" #include "impeller/core/allocator.h" #include "impeller/core/formats.h" #include "impeller/geometry/size.h" @@ -18,32 +18,21 @@ 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_; +struct RenderTargetConfig { + ISize size = ISize{0, 0}; + size_t mip_count = 0; + bool has_msaa = false; + bool has_depth_stencil = false; + + constexpr bool operator==(const RenderTargetConfig& o) const { + return size == o.size && mip_count == o.mip_count && + has_msaa == o.has_msaa && has_depth_stencil == o.has_depth_stencil; + } + + constexpr size_t Hash() const { + return fml::HashCombine(size.width, size.height, mip_count, has_msaa, + has_depth_stencil); + } }; class RenderTarget final { @@ -82,40 +71,20 @@ class RenderTarget final { .store_action = StoreAction::kDontCare, .clear_color = Color::BlackTransparent()}; - static RenderTarget CreateOffscreen( - const Context& context, - RenderTargetAllocator& allocator, - ISize size, - int mip_count, - const std::string& label = "Offscreen", - AttachmentConfig color_attachment_config = kDefaultColorAttachmentConfig, - std::optional stencil_attachment_config = - kDefaultStencilAttachmentConfig); - - static RenderTarget CreateOffscreenMSAA( - const Context& context, - RenderTargetAllocator& allocator, - ISize size, - int mip_count, - const std::string& label = "Offscreen MSAA", - AttachmentConfigMSAA color_attachment_config = - kDefaultColorAttachmentConfigMSAA, - std::optional stencil_attachment_config = - kDefaultStencilAttachmentConfig); - RenderTarget(); ~RenderTarget(); bool IsValid() const; - void SetupDepthStencilAttachments(const Context& context, - RenderTargetAllocator& allocator, - ISize size, - bool msaa, - const std::string& label = "Offscreen", - AttachmentConfig stencil_attachment_config = - kDefaultStencilAttachmentConfig); + void SetupDepthStencilAttachments( + const Context& context, + Allocator& allocator, + ISize size, + bool msaa, + const std::string& label = "Offscreen", + RenderTarget::AttachmentConfig stencil_attachment_config = + RenderTarget::kDefaultStencilAttachmentConfig); SampleCount GetSampleCount() const; @@ -152,12 +121,73 @@ class RenderTarget final { std::string ToString() const; + RenderTargetConfig ToConfig() const { + auto& color_attachment = GetColorAttachments().find(0)->second; + return RenderTargetConfig{ + .size = color_attachment.texture->GetSize(), + .mip_count = color_attachment.texture->GetMipCount(), + .has_msaa = color_attachment.resolve_texture != nullptr, + .has_depth_stencil = depth_.has_value() && stencil_.has_value()}; + } + private: std::map colors_; std::optional depth_; std::optional stencil_; }; +/// @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; + + virtual RenderTarget CreateOffscreen( + const Context& context, + ISize size, + int mip_count, + const std::string& label = "Offscreen", + RenderTarget::AttachmentConfig color_attachment_config = + RenderTarget::kDefaultColorAttachmentConfig, + std::optional stencil_attachment_config = + RenderTarget::kDefaultStencilAttachmentConfig); + + virtual RenderTarget CreateOffscreenMSAA( + const Context& context, + ISize size, + int mip_count, + const std::string& label = "Offscreen MSAA", + RenderTarget::AttachmentConfigMSAA color_attachment_config = + RenderTarget::kDefaultColorAttachmentConfigMSAA, + std::optional stencil_attachment_config = + RenderTarget::kDefaultStencilAttachmentConfig); + + /// @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: + void SetupDepthStencilAttachments( + Allocator& allocator, + const Context& context, + ISize size, + bool msaa, + const std::string& label = "Offscreen", + RenderTarget::AttachmentConfig stencil_attachment_config = + RenderTarget::kDefaultStencilAttachmentConfig); + + std::shared_ptr allocator_; +}; + } // namespace impeller #endif // FLUTTER_IMPELLER_RENDERER_RENDER_TARGET_H_ diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 2dbef38b3abc7..e491e4eb40010 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -1178,10 +1178,8 @@ TEST_P(RendererTest, StencilMask) { stencil_config.load_action = LoadAction::kLoad; stencil_config.store_action = StoreAction::kDontCare; stencil_config.storage_mode = StorageMode::kHostVisible; - auto render_target_allocator = - RenderTargetAllocator(context->GetResourceAllocator()); render_target.SetupDepthStencilAttachments( - *context, render_target_allocator, + *context, *context->GetResourceAllocator(), render_target.GetRenderTargetSize(), true, "stencil", stencil_config); // Fill the stencil buffer with an checkerboard pattern. const auto target_width = render_target.GetRenderTargetSize().width; @@ -1280,8 +1278,8 @@ TEST_P(RendererTest, CanLookupRenderTargetProperties) { auto render_target_cache = std::make_shared( GetContext()->GetResourceAllocator()); - auto render_target = RenderTarget::CreateOffscreen( - *context, *render_target_cache, {100, 100}, /*mip_count=*/1); + auto render_target = render_target_cache->CreateOffscreen( + *context, {100, 100}, /*mip_count=*/1); auto render_pass = cmd_buffer->CreateRenderPass(render_target); EXPECT_EQ(render_pass->GetSampleCount(), render_target.GetSampleCount()); @@ -1300,8 +1298,8 @@ TEST_P(RendererTest, auto render_target_cache = std::make_shared( GetContext()->GetResourceAllocator()); - RenderTarget render_target = RenderTarget::CreateOffscreenMSAA( - *context, *render_target_cache, {100, 100}, /*mip_count=*/1); + RenderTarget render_target = render_target_cache->CreateOffscreenMSAA( + *context, {100, 100}, /*mip_count=*/1); EXPECT_EQ(render_target.GetDepthAttachment() ->texture->GetTextureDescriptor() .format, From 99117d8c758939553941d77afc7d60ad669aee70 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 26 Feb 2024 18:24:43 -0800 Subject: [PATCH 3/6] fixes tests. --- impeller/aiks/aiks_unittests.cc | 32 ++-- impeller/entity/entity_unittests.cc | 50 ++---- impeller/entity/render_target_cache.cc | 30 ++-- impeller/entity/render_target_cache.h | 14 +- .../entity/render_target_cache_unittests.cc | 142 +++++++++--------- .../vulkan/render_pass_cache_unittests.cc | 40 ++--- 6 files changed, 139 insertions(+), 169 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 8c8de9ac26ed4..0d4fcb16aee64 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3348,11 +3348,9 @@ TEST_P(AiksTest, GaussianBlurAllocatesCorrectMipCountRenderTarget) { picture.ToImage(aiks_context, {100, 100}); size_t max_mip_count = 0; - for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); - ++it) { - // max_mip_count = - // std::max(it->texture->GetTextureDescriptor().mip_count, - // max_mip_count); + for (auto it = cache->GetRenderTargetDataBegin(); + it != cache->GetRenderTargetDataEnd(); ++it) { + max_mip_count = std::max(it->config.mip_count, max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); } @@ -3379,11 +3377,9 @@ TEST_P(AiksTest, GaussianBlurMipMapNestedLayer) { picture.ToImage(aiks_context, {100, 100}); size_t max_mip_count = 0; - for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); - ++it) { - // max_mip_count = - // std::max(it->texture->GetTextureDescriptor().mip_count, - // max_mip_count); + for (auto it = cache->GetRenderTargetDataBegin(); + it != cache->GetRenderTargetDataEnd(); ++it) { + max_mip_count = std::max(it->config.mip_count, max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); // The log is FML_DLOG, so only check in debug builds. @@ -3416,11 +3412,9 @@ TEST_P(AiksTest, GaussianBlurMipMapImageFilter) { picture.ToImage(aiks_context, {1024, 768}); size_t max_mip_count = 0; - for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); - ++it) { - // max_mip_count = - // std::max(it->render_target->GetTextureDescriptor().mip_count, - // max_mip_count); + for (auto it = cache->GetRenderTargetDataBegin(); + it != cache->GetRenderTargetDataEnd(); ++it) { + max_mip_count = std::max(it->config.mip_count, max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); // The log is FML_DLOG, so only check in debug builds. @@ -3459,11 +3453,9 @@ TEST_P(AiksTest, GaussianBlurMipMapSolidColor) { picture.ToImage(aiks_context, {1024, 768}); size_t max_mip_count = 0; - for (auto it = cache->GetTextureDataBegin(); it != cache->GetTextureDataEnd(); - ++it) { - // max_mip_count = - // std::max(it->texture->GetTextureDescriptor().mip_count, - // max_mip_count); + for (auto it = cache->GetRenderTargetDataBegin(); + it != cache->GetRenderTargetDataEnd(); ++it) { + max_mip_count = std::max(it->config.mip_count, max_mip_count); } EXPECT_EQ(max_mip_count, blur_required_mip_count); // The log is FML_DLOG, so only check in debug builds. diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index bca22894ea38a..e84d64853cfde 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -38,6 +38,7 @@ #include "impeller/entity/geometry/geometry.h" #include "impeller/entity/geometry/point_field_geometry.h" #include "impeller/entity/geometry/stroke_path_geometry.h" +#include "impeller/entity/render_target_cache.h" #include "impeller/geometry/color.h" #include "impeller/geometry/geometry_asserts.h" #include "impeller/geometry/path_builder.h" @@ -2590,33 +2591,6 @@ TEST_P(EntityTest, TextContentsCeilsGlyphScaleToDecimal) { ASSERT_EQ(TextFrame::RoundScaledFontSize(0.0f, 12), 0.0f); } -class TestRenderTargetAllocator : public RenderTargetAllocator { - public: - explicit TestRenderTargetAllocator(std::shared_ptr allocator) - : RenderTargetAllocator(std::move(allocator)) {} - - ~TestRenderTargetAllocator() = default; - - // std::shared_ptr CreateTexture( - // const TextureDescriptor& desc) override { - // allocated_.push_back(desc); - // return RenderTargetAllocator::CreateTexture(desc); - // } - - void Start() override { RenderTargetAllocator::Start(); } - - void End() override { RenderTargetAllocator::End(); } - - std::vector GetAllocatedTextureDescriptors() const { - return allocated_; - } - - void ResetDescriptors() { allocated_.clear(); } - - private: - std::vector allocated_; -}; - TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) { if (GetContext()->GetCapabilities()->SupportsFramebufferFetch()) { GTEST_SKIP() << "Backends that support framebuffer fetch dont use coverage " @@ -2636,31 +2610,29 @@ TEST_P(EntityTest, AdvancedBlendCoverageHintIsNotResetByEntityPass) { EXPECT_TRUE(coverage.has_value()); auto pass = std::make_unique(); - auto test_allocator = std::make_shared( - GetContext()->GetResourceAllocator()); + std::shared_ptr render_target_allocator = + std::make_shared(GetContext()->GetResourceAllocator()); auto stencil_config = RenderTarget::AttachmentConfig{ .storage_mode = StorageMode::kDevicePrivate, .load_action = LoadAction::kClear, .store_action = StoreAction::kDontCare, .clear_color = Color::BlackTransparent()}; - auto rt = test_allocator->CreateOffscreen( + auto rt = render_target_allocator->CreateOffscreen( *GetContext(), ISize::MakeWH(1000, 1000), /*mip_count=*/1, "Offscreen", RenderTarget::kDefaultColorAttachmentConfig, stencil_config); auto content_context = ContentContext( - GetContext(), TypographerContextSkia::Make(), test_allocator); + GetContext(), TypographerContextSkia::Make(), render_target_allocator); pass->AddEntity(std::move(entity)); - test_allocator->ResetDescriptors(); EXPECT_TRUE(pass->Render(content_context, rt)); - std::vector descriptors = - test_allocator->GetAllocatedTextureDescriptors(); - - auto contains_size = [&descriptors](ISize size) -> bool { - return std::find_if(descriptors.begin(), descriptors.end(), - [&size](auto desc) { return desc.size == size; }) != - descriptors.end(); + auto contains_size = [&render_target_allocator](ISize size) -> bool { + return std::find_if(render_target_allocator->GetRenderTargetDataBegin(), + render_target_allocator->GetRenderTargetDataEnd(), + [&size](const auto& data) { + return data.config.size == size; + }) != render_target_allocator->GetRenderTargetDataEnd(); }; EXPECT_TRUE(contains_size(ISize(1000, 1000))) diff --git a/impeller/entity/render_target_cache.cc b/impeller/entity/render_target_cache.cc index 1e86cb2808bff..3a9710a93e5e9 100644 --- a/impeller/entity/render_target_cache.cc +++ b/impeller/entity/render_target_cache.cc @@ -11,20 +11,20 @@ RenderTargetCache::RenderTargetCache(std::shared_ptr allocator) : RenderTargetAllocator(std::move(allocator)) {} void RenderTargetCache::Start() { - for (auto& td : render_pass_data_) { + for (auto& td : render_target_data_) { td.used_this_frame = false; } } void RenderTargetCache::End() { - std::vector retain; + std::vector retain; - for (const auto& td : render_pass_data_) { + for (const auto& td : render_target_data_) { if (td.used_this_frame) { retain.push_back(td); } } - render_pass_data_.swap(retain); + render_target_data_.swap(retain); } RenderTarget RenderTargetCache::CreateOffscreen( @@ -40,8 +40,8 @@ RenderTarget RenderTargetCache::CreateOffscreen( .has_msaa = false, .has_depth_stencil = stencil_attachment_config.has_value(), }; - for (auto& render_target_data : render_pass_data_) { - const auto other_config = render_target_data.render_target.ToConfig(); + for (auto& render_target_data : render_target_data_) { + const auto other_config = render_target_data.config; if (!render_target_data.used_this_frame && other_config == config) { render_target_data.used_this_frame = true; return render_target_data.render_target; @@ -53,8 +53,10 @@ RenderTarget RenderTargetCache::CreateOffscreen( if (!created_target.IsValid()) { return created_target; } - render_pass_data_.push_back( - RenderPassData{.used_this_frame = true, .render_target = created_target}); + render_target_data_.push_back( + RenderTargetData{.used_this_frame = true, + .config = config, + .render_target = created_target}); return created_target; } @@ -71,8 +73,8 @@ RenderTarget RenderTargetCache::CreateOffscreenMSAA( .has_msaa = true, .has_depth_stencil = stencil_attachment_config.has_value(), }; - for (auto& render_target_data : render_pass_data_) { - const auto other_config = render_target_data.render_target.ToConfig(); + for (auto& render_target_data : render_target_data_) { + const auto other_config = render_target_data.config; if (!render_target_data.used_this_frame && other_config == config) { render_target_data.used_this_frame = true; return render_target_data.render_target; @@ -84,13 +86,15 @@ RenderTarget RenderTargetCache::CreateOffscreenMSAA( if (!created_target.IsValid()) { return created_target; } - render_pass_data_.push_back( - RenderPassData{.used_this_frame = true, .render_target = created_target}); + render_target_data_.push_back( + RenderTargetData{.used_this_frame = true, + .config = config, + .render_target = created_target}); return created_target; } size_t RenderTargetCache::CachedTextureCount() const { - return render_pass_data_.size(); + return render_target_data_.size(); } } // namespace impeller diff --git a/impeller/entity/render_target_cache.h b/impeller/entity/render_target_cache.h index ccb2bd4fd3d72..bc60951ca9a4f 100644 --- a/impeller/entity/render_target_cache.h +++ b/impeller/entity/render_target_cache.h @@ -49,12 +49,13 @@ class RenderTargetCache : public RenderTargetAllocator { size_t CachedTextureCount() const; private: - struct RenderPassData { + struct RenderTargetData { bool used_this_frame; + RenderTargetConfig config; RenderTarget render_target; }; - std::vector render_pass_data_; + std::vector render_target_data_; RenderTargetCache(const RenderTargetCache&) = delete; @@ -62,13 +63,14 @@ class RenderTargetCache : public RenderTargetAllocator { public: /// Visible for testing. - std::vector::const_iterator GetTextureDataBegin() const { - return render_pass_data_.begin(); + std::vector::const_iterator GetRenderTargetDataBegin() + const { + return render_target_data_.begin(); } /// Visible for testing. - std::vector::const_iterator GetTextureDataEnd() const { - return render_pass_data_.end(); + std::vector::const_iterator GetRenderTargetDataEnd() const { + return render_target_data_.end(); } }; diff --git a/impeller/entity/render_target_cache_unittests.cc b/impeller/entity/render_target_cache_unittests.cc index 4c4c4c4a817f2..af69a8819453e 100644 --- a/impeller/entity/render_target_cache_unittests.cc +++ b/impeller/entity/render_target_cache_unittests.cc @@ -7,83 +7,83 @@ #include "flutter/testing/testing.h" #include "impeller/core/allocator.h" #include "impeller/core/texture_descriptor.h" +#include "impeller/entity/entity_playground.h" #include "impeller/entity/render_target_cache.h" +#include "impeller/playground/playground_test.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 { -// if (should_fail) { -// return nullptr; -// } -// return std::make_shared(desc); -// }; - -// virtual std::shared_ptr OnCreateTexture( -// const TextureDescriptor& desc) override { -// if (should_fail) { -// return nullptr; -// } -// return std::make_shared(desc); -// }; - -// bool should_fail = false; -// }; - -// 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); -// } - -// TEST(RenderTargetCacheTest, DoesNotPersistFailedAllocations) { -// 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(); -// allocator->should_fail = true; - -// ASSERT_EQ(render_target_cache.CreateTexture(desc), nullptr); -// ASSERT_EQ(render_target_cache.CachedTextureCount(), 0u); -// } +using RenderTargetCacheTest = EntityPlayground; +INSTANTIATE_PLAYGROUND_SUITE(RenderTargetCacheTest); + +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 { + if (should_fail) { + return nullptr; + } + return std::make_shared(desc); + }; + + virtual std::shared_ptr OnCreateTexture( + const TextureDescriptor& desc) override { + if (should_fail) { + return nullptr; + } + return std::make_shared(desc); + }; + + bool should_fail = false; +}; + +TEST_P(RenderTargetCacheTest, CachesUsedTexturesAcrossFrames) { + auto allocator = std::make_shared(); + auto render_target_cache = RenderTargetCache(allocator); + + render_target_cache.Start(); + // Create two render targets 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.CreateOffscreen(*GetContext(), {100, 100}, 1); + render_target_cache.CreateOffscreen(*GetContext(), {100, 100}, 1); + + EXPECT_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.CreateOffscreen(*GetContext(), {100, 100}, 1); + EXPECT_EQ(render_target_cache.CachedTextureCount(), 2u); + + render_target_cache.End(); + EXPECT_EQ(render_target_cache.CachedTextureCount(), 1u); +} + +TEST_P(RenderTargetCacheTest, DoesNotPersistFailedAllocations) { + auto allocator = std::make_shared(); + auto render_target_cache = RenderTargetCache(allocator); + + render_target_cache.Start(); + allocator->should_fail = true; + + auto render_target = + render_target_cache.CreateOffscreen(*GetContext(), {100, 100}, 1); + + EXPECT_FALSE(render_target.IsValid()); + EXPECT_EQ(render_target_cache.CachedTextureCount(), 0u); +} } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc index 4399891f0cb55..21b8bd9d05505 100644 --- a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc +++ b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc @@ -18,33 +18,33 @@ TEST_P(RendererTest, CachesRenderPassAndFramebuffer) { GTEST_SKIP() << "Test only applies to Vulkan"; } - // auto allocator = std::make_shared( - // GetContext()->GetResourceAllocator()); + auto allocator = std::make_shared( + GetContext()->GetResourceAllocator()); - // auto render_target = RenderTarget::CreateOffscreenMSAA( - // *GetContext(), *allocator, {100, 100}, 1); - // auto resolve_texture = - // render_target.GetColorAttachments().find(0u)->second.resolve_texture; - // auto& texture_vk = TextureVK::Cast(*resolve_texture); + auto render_target = + allocator->CreateOffscreenMSAA(*GetContext(), {100, 100}, 1); + auto resolve_texture = + render_target.GetColorAttachments().find(0u)->second.resolve_texture; + auto& texture_vk = TextureVK::Cast(*resolve_texture); - // EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); - // EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); + EXPECT_EQ(texture_vk.GetFramebuffer(), nullptr); + EXPECT_EQ(texture_vk.GetRenderPass(), nullptr); - // auto buffer = GetContext()->CreateCommandBuffer(); - // auto render_pass = buffer->CreateRenderPass(render_target); + auto buffer = GetContext()->CreateCommandBuffer(); + auto render_pass = buffer->CreateRenderPass(render_target); - // EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); - // EXPECT_NE(texture_vk.GetRenderPass(), nullptr); + EXPECT_NE(texture_vk.GetFramebuffer(), nullptr); + EXPECT_NE(texture_vk.GetRenderPass(), nullptr); - // render_pass->EncodeCommands(); - // GetContext()->GetCommandQueue()->Submit({buffer}); + render_pass->EncodeCommands(); + GetContext()->GetCommandQueue()->Submit({buffer}); - // // Can be reused without error. - // auto buffer_2 = GetContext()->CreateCommandBuffer(); - // auto render_pass_2 = buffer_2->CreateRenderPass(render_target); + // Can be reused without error. + auto buffer_2 = GetContext()->CreateCommandBuffer(); + auto render_pass_2 = buffer_2->CreateRenderPass(render_target); - // EXPECT_TRUE(render_pass_2->EncodeCommands()); - // EXPECT_TRUE(GetContext()->GetCommandQueue()->Submit({buffer_2}).ok()); + EXPECT_TRUE(render_pass_2->EncodeCommands()); + EXPECT_TRUE(GetContext()->GetCommandQueue()->Submit({buffer_2}).ok()); } } // namespace testing From cf10a060868f447e22f34f44c56994c924cdbbbc Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 26 Feb 2024 18:40:54 -0800 Subject: [PATCH 4/6] test fixes. --- .../entity/contents/tiled_texture_contents_unittests.cc | 8 ++++---- impeller/entity/render_target_cache_unittests.cc | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index b9ff3d9e01882..674ac4fdac20d 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -74,10 +74,10 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipelineExternalOES) { auto content_context = GetContentContext(); auto buffer = content_context->GetContext()->CreateCommandBuffer(); - auto render_target = RenderTarget::CreateOffscreenMSAA( - *content_context->GetContext(), - *GetContentContext()->GetRenderTargetCache(), {100, 100}, - /*mip_count=*/1); + auto render_target = + GetContentContext()->GetRenderTargetCache()->CreateOffscreenMSAA( + *content_context->GetContext(), {100, 100}, + /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); ASSERT_TRUE(contents.Render(*GetContentContext(), {}, *render_pass)); diff --git a/impeller/entity/render_target_cache_unittests.cc b/impeller/entity/render_target_cache_unittests.cc index af69a8819453e..1e980870fe6fe 100644 --- a/impeller/entity/render_target_cache_unittests.cc +++ b/impeller/entity/render_target_cache_unittests.cc @@ -5,6 +5,7 @@ #include #include "flutter/testing/testing.h" +#include "impeller/base/validation.h" #include "impeller/core/allocator.h" #include "impeller/core/texture_descriptor.h" #include "impeller/entity/entity_playground.h" @@ -72,6 +73,7 @@ TEST_P(RenderTargetCacheTest, CachesUsedTexturesAcrossFrames) { } TEST_P(RenderTargetCacheTest, DoesNotPersistFailedAllocations) { + ScopedValidationDisable disable; auto allocator = std::make_shared(); auto render_target_cache = RenderTargetCache(allocator); From 0ed07389fe370689b7c1e6251d13d18808c8b05d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 26 Feb 2024 19:34:42 -0800 Subject: [PATCH 5/6] use real allocator. --- impeller/entity/render_target_cache_unittests.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/entity/render_target_cache_unittests.cc b/impeller/entity/render_target_cache_unittests.cc index 1e980870fe6fe..a3a47dde96ea1 100644 --- a/impeller/entity/render_target_cache_unittests.cc +++ b/impeller/entity/render_target_cache_unittests.cc @@ -49,8 +49,8 @@ class TestAllocator : public Allocator { }; TEST_P(RenderTargetCacheTest, CachesUsedTexturesAcrossFrames) { - auto allocator = std::make_shared(); - auto render_target_cache = RenderTargetCache(allocator); + auto render_target_cache = + RenderTargetCache(GetContext()->GetResourceAllocator()); render_target_cache.Start(); // Create two render targets of the same exact size/shape. Both should be From 41035f7e670b93a6b673920f2f2bb3241bcb8a50 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 27 Feb 2024 04:39:19 +0000 Subject: [PATCH 6/6] fix scene_contents --- impeller/entity/contents/scene_contents.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/impeller/entity/contents/scene_contents.cc b/impeller/entity/contents/scene_contents.cc index f5da62830ecbc..602821a6d237b 100644 --- a/impeller/entity/contents/scene_contents.cc +++ b/impeller/entity/contents/scene_contents.cc @@ -46,9 +46,8 @@ bool SceneContents::Render(const ContentContext& renderer, RenderTarget subpass_target; if (renderer.GetContext()->GetCapabilities()->SupportsOffscreenMSAA()) { - subpass_target = RenderTarget::CreateOffscreenMSAA( + subpass_target = renderer.GetRenderTargetCache()->CreateOffscreenMSAA( *renderer.GetContext(), // context - *renderer.GetRenderTargetCache(), // allocator ISize(coverage.value().GetSize()), // size /*mip_count=*/1, "SceneContents", // label @@ -65,9 +64,8 @@ bool SceneContents::Render(const ContentContext& renderer, } // stencil_attachment_config ); } else { - subpass_target = RenderTarget::CreateOffscreen( + subpass_target = renderer.GetRenderTargetCache()->CreateOffscreen( *renderer.GetContext(), // context - *renderer.GetRenderTargetCache(), // allocator ISize(coverage.value().GetSize()), // size /*mip_count=*/1, "SceneContents", // label