diff --git a/impeller/display_list/canvas.cc b/impeller/display_list/canvas.cc index 7b26f02892904..4aaa83372bf1a 100644 --- a/impeller/display_list/canvas.cc +++ b/impeller/display_list/canvas.cc @@ -14,6 +14,7 @@ #include "flutter/fml/logging.h" #include "flutter/fml/trace_event.h" #include "impeller/base/validation.h" +#include "impeller/core/formats.h" #include "impeller/display_list/color_filter.h" #include "impeller/display_list/image_filter.h" #include "impeller/display_list/skia_conversions.h" @@ -856,7 +857,7 @@ void Canvas::DrawAtlas(const std::shared_ptr& atlas_contents, void Canvas::SetupRenderPass() { renderer_.GetRenderTargetCache()->Start(); - auto color0 = render_target_.GetColorAttachments().find(0u)->second; + ColorAttachment color0 = render_target_.GetColorAttachment(0); auto& stencil_attachment = render_target_.GetStencilAttachment(); auto& depth_attachment = render_target_.GetDepthAttachment(); @@ -1463,8 +1464,7 @@ void Canvas::AddRenderEntityToCurrentPass(Entity& entity, bool reuse_depth) { RenderTarget& render_target = render_passes_.back() .inline_pass_context->GetPassTarget() .GetRenderTarget(); - ColorAttachment attachment = - render_target.GetColorAttachments().find(0u)->second; + ColorAttachment attachment = render_target.GetColorAttachment(0); // Attachment.clear color needs to be premultiplied at all times, but the // Color::Blend function requires unpremultiplied colors. attachment.clear_color = attachment.clear_color.Unpremultiply() @@ -1591,8 +1591,7 @@ std::shared_ptr Canvas::FlipBackdrop(Point global_pass_position, } if (should_use_onscreen) { - ColorAttachment color0 = - render_target_.GetColorAttachments().find(0u)->second; + ColorAttachment color0 = render_target_.GetColorAttachment(0); // When MSAA is being used, we end up overriding the entire backdrop by // drawing the previous pass texture, and so we don't have to clear it and // can use kDontCare. diff --git a/impeller/entity/entity_pass_target.cc b/impeller/entity/entity_pass_target.cc index e9284de522675..db9c917b27add 100644 --- a/impeller/entity/entity_pass_target.cc +++ b/impeller/entity/entity_pass_target.cc @@ -19,7 +19,7 @@ EntityPassTarget::EntityPassTarget(const RenderTarget& render_target, std::shared_ptr EntityPassTarget::Flip( const ContentContext& renderer) { - auto color0 = target_.GetColorAttachments().find(0)->second; + ColorAttachment color0 = target_.GetColorAttachment(0); if (!color0.resolve_texture) { VALIDATION_LOG << "EntityPassTarget Flip should never be called for a " "non-MSAA target."; diff --git a/impeller/entity/entity_pass_target_unittests.cc b/impeller/entity/entity_pass_target_unittests.cc index cd22acbffdb5a..4a72138a70e19 100644 --- a/impeller/entity/entity_pass_target_unittests.cc +++ b/impeller/entity/entity_pass_target_unittests.cc @@ -31,20 +31,14 @@ TEST_P(EntityPassTargetTest, SwapWithMSAATexture) { auto entity_pass_target = EntityPassTarget(render_target, false, false); - auto color0 = entity_pass_target.GetRenderTarget() - .GetColorAttachments() - .find(0u) - ->second; + auto color0 = entity_pass_target.GetRenderTarget().GetColorAttachment(0); auto msaa_tex = color0.texture; auto resolve_tex = color0.resolve_texture; FML_DCHECK(content_context); entity_pass_target.Flip(*content_context); - color0 = entity_pass_target.GetRenderTarget() - .GetColorAttachments() - .find(0u) - ->second; + color0 = entity_pass_target.GetRenderTarget().GetColorAttachment(0); ASSERT_EQ(msaa_tex, color0.texture); ASSERT_NE(resolve_tex, color0.resolve_texture); @@ -89,10 +83,7 @@ TEST_P(EntityPassTargetTest, SwapWithMSAAImplicitResolve) { auto entity_pass_target = EntityPassTarget(render_target, false, true); - auto color0 = entity_pass_target.GetRenderTarget() - .GetColorAttachments() - .find(0u) - ->second; + auto color0 = entity_pass_target.GetRenderTarget().GetColorAttachment(0); auto msaa_tex = color0.texture; auto resolve_tex = color0.resolve_texture; @@ -101,10 +92,7 @@ TEST_P(EntityPassTargetTest, SwapWithMSAAImplicitResolve) { FML_DCHECK(content_context); entity_pass_target.Flip(*content_context); - color0 = entity_pass_target.GetRenderTarget() - .GetColorAttachments() - .find(0u) - ->second; + color0 = entity_pass_target.GetRenderTarget().GetColorAttachment(0); ASSERT_NE(msaa_tex, color0.texture); ASSERT_NE(resolve_tex, color0.resolve_texture); diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index ac86af2139143..82d94a614ec5d 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -86,20 +86,13 @@ const std::shared_ptr& InlinePassContext::GetRenderPass() { return pass_; } - if (pass_target_.GetRenderTarget().GetColorAttachments().empty()) { - VALIDATION_LOG << "Color attachment unexpectedly missing from the " - "EntityPass render target."; - return pass_; - } - command_buffer_->SetLabel("EntityPass Command Buffer"); { // If the pass target has a resolve texture, then we're using MSAA. - bool is_msaa = pass_target_.GetRenderTarget() - .GetColorAttachments() - .find(0) - ->second.resolve_texture != nullptr; + bool is_msaa = + pass_target_.GetRenderTarget().GetColorAttachment(0).resolve_texture != + nullptr; if (pass_count_ > 0 && is_msaa) { pass_target_.Flip(renderer_); } @@ -107,8 +100,7 @@ const std::shared_ptr& InlinePassContext::GetRenderPass() { // Find the color attachment a second time, since the target may have just // flipped. - auto color0 = - pass_target_.GetRenderTarget().GetColorAttachments().find(0)->second; + ColorAttachment color0 = pass_target_.GetRenderTarget().GetColorAttachment(0); bool is_msaa = color0.resolve_texture != nullptr; if (pass_count_ > 0) { diff --git a/impeller/entity/render_target_cache.cc b/impeller/entity/render_target_cache.cc index dbfe0a9b50c63..43c85fcbed790 100644 --- a/impeller/entity/render_target_cache.cc +++ b/impeller/entity/render_target_cache.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/entity/render_target_cache.h" +#include "impeller/core/formats.h" #include "impeller/renderer/render_target.h" namespace impeller { @@ -52,10 +53,10 @@ RenderTarget RenderTargetCache::CreateOffscreen( 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; - auto color0 = render_target_data.render_target.GetColorAttachments() - .find(0u) - ->second; - auto depth = render_target_data.render_target.GetDepthAttachment(); + ColorAttachment color0 = + render_target_data.render_target.GetColorAttachment(0); + std::optional depth = + render_target_data.render_target.GetDepthAttachment(); std::shared_ptr depth_tex = depth ? depth->texture : nullptr; return RenderTargetAllocator::CreateOffscreen( context, size, mip_count, label, color_attachment_config, @@ -102,10 +103,10 @@ RenderTarget RenderTargetCache::CreateOffscreenMSAA( 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; - auto color0 = render_target_data.render_target.GetColorAttachments() - .find(0u) - ->second; - auto depth = render_target_data.render_target.GetDepthAttachment(); + ColorAttachment color0 = + render_target_data.render_target.GetColorAttachment(0); + std::optional depth = + render_target_data.render_target.GetDepthAttachment(); std::shared_ptr depth_tex = depth ? depth->texture : nullptr; return RenderTargetAllocator::CreateOffscreenMSAA( context, size, mip_count, label, color_attachment_config, diff --git a/impeller/entity/render_target_cache_unittests.cc b/impeller/entity/render_target_cache_unittests.cc index 73493e7641c85..2684e322c587e 100644 --- a/impeller/entity/render_target_cache_unittests.cc +++ b/impeller/entity/render_target_cache_unittests.cc @@ -7,6 +7,7 @@ #include "flutter/testing/testing.h" #include "impeller/base/validation.h" #include "impeller/core/allocator.h" +#include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" #include "impeller/entity/entity_playground.h" #include "impeller/entity/render_target_cache.h" @@ -104,8 +105,8 @@ TEST_P(RenderTargetCacheTest, CachedTextureGetsNewAttachmentConfig) { *GetContext(), {100, 100}, 1, "Offscreen2", color_attachment_config); render_target_cache.End(); - auto color1 = target1.GetColorAttachments().find(0)->second; - auto color2 = target2.GetColorAttachments().find(0)->second; + ColorAttachment color1 = target1.GetColorAttachment(0); + ColorAttachment color2 = target2.GetColorAttachment(0); // The second color attachment should reuse the first attachment's texture // but with attributes from the second AttachmentConfig. EXPECT_EQ(color2.texture, color1.texture); diff --git a/impeller/playground/playground.cc b/impeller/playground/playground.cc index f588a54049480..1ef9f17f134d8 100644 --- a/impeller/playground/playground.cc +++ b/impeller/playground/playground.cc @@ -284,12 +284,7 @@ bool Playground::OpenPlaygroundHere( } buffer->SetLabel("ImGui Command Buffer"); - if (render_target.GetColorAttachments().empty()) { - VALIDATION_LOG << "render target attachments are empty."; - return false; - } - - auto color0 = render_target.GetColorAttachments().find(0)->second; + auto color0 = render_target.GetColorAttachment(0); color0.load_action = LoadAction::kLoad; if (color0.resolve_texture) { color0.texture = color0.resolve_texture; @@ -297,7 +292,6 @@ bool Playground::OpenPlaygroundHere( color0.store_action = StoreAction::kStore; } render_target.SetColorAttachment(color0, 0); - render_target.SetStencilAttachment(std::nullopt); render_target.SetDepthAttachment(std::nullopt); diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 594fcfab0732f..0da1c79c7bd6c 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -11,6 +11,7 @@ #include "fml/closure.h" #include "fml/logging.h" #include "impeller/base/validation.h" +#include "impeller/core/formats.h" #include "impeller/renderer/backend/gles/context_gles.h" #include "impeller/renderer/backend/gles/device_buffer_gles.h" #include "impeller/renderer/backend/gles/formats_gles.h" @@ -537,9 +538,11 @@ bool RenderPassGLES::OnEncodeCommands(const Context& context) const { if (!render_target.HasColorAttachment(0u)) { return false; } - const auto& color0 = render_target.GetColorAttachments().at(0u); - const auto& depth0 = render_target.GetDepthAttachment(); - const auto& stencil0 = render_target.GetStencilAttachment(); + const ColorAttachment& color0 = render_target.GetColorAttachment(0); + const std::optional& depth0 = + render_target.GetDepthAttachment(); + const std::optional& stencil0 = + render_target.GetStencilAttachment(); auto pass_data = std::make_shared(); pass_data->label = label_; diff --git a/impeller/renderer/backend/gles/test/surface_gles_unittests.cc b/impeller/renderer/backend/gles/test/surface_gles_unittests.cc index dbef03075a088..a039e03786c22 100644 --- a/impeller/renderer/backend/gles/test/surface_gles_unittests.cc +++ b/impeller/renderer/backend/gles/test/surface_gles_unittests.cc @@ -22,7 +22,7 @@ TEST_P(SurfaceGLESTest, CanWrapNonZeroFBO) { ASSERT_TRUE(surface->IsValid()); ASSERT_TRUE(surface->GetRenderTarget().HasColorAttachment(0)); const auto& texture = TextureGLES::Cast( - *(surface->GetRenderTarget().GetColorAttachments().at(0).texture)); + *(surface->GetRenderTarget().GetColorAttachment(0).texture)); auto wrapped = texture.GetFBO(); ASSERT_TRUE(wrapped.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm index 80cf59bb5f1f7..cd0fbbeb0219c 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -104,15 +104,15 @@ static bool ConfigureStencilAttachment( const RenderTarget& desc) { auto result = [MTLRenderPassDescriptor renderPassDescriptor]; - const auto& colors = desc.GetColorAttachments(); - - for (const auto& color : colors) { - if (!ConfigureColorAttachment(color.second, - result.colorAttachments[color.first])) { - VALIDATION_LOG << "Could not configure color attachment at index " - << color.first; - return nil; - } + bool configured_attachment = desc.IterateAllColorAttachments( + [&result](size_t index, const ColorAttachment& attachment) -> bool { + return ConfigureColorAttachment(attachment, + result.colorAttachments[index]); + }); + + if (!configured_attachment) { + VALIDATION_LOG << "Could not configure color attachments"; + return nil; } const auto& depth = desc.GetDepthAttachment(); diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index d793b3e9c61fd..62da661cf7e84 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -7,6 +7,7 @@ #include #include "fml/concurrent_message_loop.h" +#include "impeller/core/formats.h" #include "impeller/renderer/backend/vulkan/command_queue_vk.h" #include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" #include "impeller/renderer/backend/vulkan/render_pass_builder_vk.h" @@ -666,15 +667,18 @@ void ContextVK::InitializeCommonlyUsedShadersIfNeeded() const { rt_allocator.CreateOffscreenMSAA(*this, {1, 1}, 1); RenderPassBuilderVK builder; - for (const auto& [bind_point, color] : render_target.GetColorAttachments()) { - builder.SetColorAttachment( - bind_point, // - color.texture->GetTextureDescriptor().format, // - color.texture->GetTextureDescriptor().sample_count, // - color.load_action, // - color.store_action // - ); - } + + render_target.IterateAllColorAttachments( + [&builder](size_t index, const ColorAttachment& attachment) -> bool { + builder.SetColorAttachment( + index, // + attachment.texture->GetTextureDescriptor().format, // + attachment.texture->GetTextureDescriptor().sample_count, // + attachment.load_action, // + attachment.store_action // + ); + return true; + }); if (auto depth = render_target.GetDepthAttachment(); depth.has_value()) { builder.SetDepthStencilAttachment( diff --git a/impeller/renderer/backend/vulkan/render_pass_builder_vk.cc b/impeller/renderer/backend/vulkan/render_pass_builder_vk.cc index 77f5a3a1e9b14..32f62dddf1907 100644 --- a/impeller/renderer/backend/vulkan/render_pass_builder_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_builder_vk.cc @@ -48,14 +48,27 @@ RenderPassBuilderVK& RenderPassBuilderVK::SetColorAttachment( desc.initialLayout = vk::ImageLayout::eUndefined; } desc.finalLayout = vk::ImageLayout::eGeneral; - colors_[index] = desc; - if (StoreActionPerformsResolve(store_action)) { - desc.storeOp = ToVKAttachmentStoreOp(store_action, true); - desc.samples = vk::SampleCountFlagBits::e1; - resolves_[index] = desc; + const bool performs_resolves = StoreActionPerformsResolve(store_action); + if (index == 0u) { + color0_ = desc; + + if (performs_resolves) { + desc.storeOp = ToVKAttachmentStoreOp(store_action, true); + desc.samples = vk::SampleCountFlagBits::e1; + color0_resolve_ = desc; + } else { + color0_resolve_ = std::nullopt; + } } else { - resolves_.erase(index); + colors_[index] = desc; + if (performs_resolves) { + desc.storeOp = ToVKAttachmentStoreOp(store_action, true); + desc.samples = vk::SampleCountFlagBits::e1; + resolves_[index] = desc; + } else { + resolves_.erase(index); + } } return *this; } @@ -100,8 +113,11 @@ vk::UniqueRenderPass RenderPassBuilderVK::Build( const vk::Device& device) const { // This must be less than `VkPhysicalDeviceLimits::maxColorAttachments` but we // are not checking. - const auto color_attachments_count = + auto color_attachments_count = colors_.empty() ? 0u : colors_.rbegin()->first + 1u; + if (color0_.has_value()) { + color_attachments_count++; + } std::vector attachments; @@ -111,6 +127,22 @@ vk::UniqueRenderPass RenderPassBuilderVK::Build( kUnusedAttachmentReference); vk::AttachmentReference depth_stencil_ref = kUnusedAttachmentReference; + if (color0_.has_value()) { + vk::AttachmentReference color_ref; + color_ref.attachment = attachments.size(); + color_ref.layout = vk::ImageLayout::eGeneral; + color_refs[0] = color_ref; + attachments.push_back(color0_.value()); + + if (color0_resolve_.has_value()) { + vk::AttachmentReference resolve_ref; + resolve_ref.attachment = attachments.size(); + resolve_ref.layout = vk::ImageLayout::eGeneral; + resolve_refs[0] = resolve_ref; + attachments.push_back(color0_resolve_.value()); + } + } + for (const auto& color : colors_) { vk::AttachmentReference color_ref; color_ref.attachment = attachments.size(); @@ -207,4 +239,16 @@ RenderPassBuilderVK::GetDepthStencil() const { return depth_stencil_; } +// Visible for testing. +std::optional RenderPassBuilderVK::GetColor0() + const { + return color0_; +} + +// Visible for testing. +std::optional RenderPassBuilderVK::GetColor0Resolve() + const { + return color0_resolve_; +} + } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/render_pass_builder_vk.h b/impeller/renderer/backend/vulkan/render_pass_builder_vk.h index c3ed390e961b9..4e9632d9a6d13 100644 --- a/impeller/renderer/backend/vulkan/render_pass_builder_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_builder_vk.h @@ -8,6 +8,7 @@ #include #include +#include "flutter/fml/macros.h" #include "impeller/core/formats.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" @@ -54,10 +55,20 @@ class RenderPassBuilderVK { // Visible for testing. const std::optional& GetDepthStencil() const; + // Visible for testing. + std::optional GetColor0() const; + + // Visible for testing. + std::optional GetColor0Resolve() const; + private: + std::optional color0_; + std::optional color0_resolve_; + std::optional depth_stencil_; + + // Color attachment 0 is stored in the field above and not in these maps. std::map colors_; std::map resolves_; - std::optional depth_stencil_; }; //------------------------------------------------------------------------------ diff --git a/impeller/renderer/backend/vulkan/render_pass_builder_vk_unittests.cc b/impeller/renderer/backend/vulkan/render_pass_builder_vk_unittests.cc index 658ae2f54ac88..2aa44c42de0db 100644 --- a/impeller/renderer/backend/vulkan/render_pass_builder_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/render_pass_builder_vk_unittests.cc @@ -40,9 +40,12 @@ TEST(RenderPassBuilder, RenderPassWithLoadOpUsesCurrentLayout) { EXPECT_TRUE(!!render_pass); - auto maybe_color = builder.GetColorAttachments().find(0u); - ASSERT_NE(maybe_color, builder.GetColorAttachments().end()); - auto color = maybe_color->second; + std::optional maybe_color = builder.GetColor0(); + ASSERT_TRUE(maybe_color.has_value()); + if (!maybe_color.has_value()) { + return; + } + vk::AttachmentDescription color = maybe_color.value(); EXPECT_EQ(color.initialLayout, vk::ImageLayout::eColorAttachmentOptimal); EXPECT_EQ(color.finalLayout, vk::ImageLayout::eGeneral); @@ -66,21 +69,25 @@ TEST(RenderPassBuilder, CreatesRenderPassWithCombinedDepthStencil) { EXPECT_TRUE(!!render_pass); - auto maybe_color = builder.GetColorAttachments().find(0u); - ASSERT_NE(maybe_color, builder.GetColorAttachments().end()); - auto color = maybe_color->second; + std::optional maybe_color = builder.GetColor0(); + ASSERT_TRUE(maybe_color.has_value()); + if (!maybe_color.has_value()) { + return; + } + vk::AttachmentDescription color = maybe_color.value(); EXPECT_EQ(color.initialLayout, vk::ImageLayout::eUndefined); EXPECT_EQ(color.finalLayout, vk::ImageLayout::eGeneral); EXPECT_EQ(color.loadOp, vk::AttachmentLoadOp::eClear); EXPECT_EQ(color.storeOp, vk::AttachmentStoreOp::eStore); - auto maybe_depth_stencil = builder.GetDepthStencil(); + std::optional maybe_depth_stencil = + builder.GetDepthStencil(); ASSERT_TRUE(maybe_depth_stencil.has_value()); if (!maybe_depth_stencil.has_value()) { return; } - auto depth_stencil = maybe_depth_stencil.value(); + vk::AttachmentDescription depth_stencil = maybe_depth_stencil.value(); EXPECT_EQ(depth_stencil.initialLayout, vk::ImageLayout::eUndefined); EXPECT_EQ(depth_stencil.finalLayout, @@ -106,12 +113,13 @@ TEST(RenderPassBuilder, CreatesRenderPassWithOnlyStencil) { EXPECT_TRUE(!!render_pass); - auto maybe_depth_stencil = builder.GetDepthStencil(); + std::optional maybe_depth_stencil = + builder.GetDepthStencil(); ASSERT_TRUE(maybe_depth_stencil.has_value()); if (!maybe_depth_stencil.has_value()) { return; } - auto depth_stencil = maybe_depth_stencil.value(); + vk::AttachmentDescription depth_stencil = maybe_depth_stencil.value(); EXPECT_EQ(depth_stencil.initialLayout, vk::ImageLayout::eUndefined); EXPECT_EQ(depth_stencil.finalLayout, @@ -135,9 +143,12 @@ TEST(RenderPassBuilder, CreatesMSAAResolveWithCorrectStore) { EXPECT_TRUE(!!render_pass); - auto maybe_color = builder.GetColorAttachments().find(0u); - ASSERT_NE(maybe_color, builder.GetColorAttachments().end()); - auto color = maybe_color->second; + auto maybe_color = builder.GetColor0(); + ASSERT_TRUE(maybe_color.has_value()); + if (!maybe_color.has_value()) { + return; + } + vk::AttachmentDescription color = maybe_color.value(); // MSAA Texture. EXPECT_EQ(color.initialLayout, vk::ImageLayout::eUndefined); @@ -145,9 +156,12 @@ TEST(RenderPassBuilder, CreatesMSAAResolveWithCorrectStore) { EXPECT_EQ(color.loadOp, vk::AttachmentLoadOp::eClear); EXPECT_EQ(color.storeOp, vk::AttachmentStoreOp::eDontCare); - auto maybe_resolve = builder.GetResolves().find(0u); - ASSERT_NE(maybe_resolve, builder.GetResolves().end()); - auto resolve = maybe_resolve->second; + auto maybe_resolve = builder.GetColor0Resolve(); + ASSERT_TRUE(maybe_resolve.has_value()); + if (!maybe_resolve.has_value()) { + return; + } + vk::AttachmentDescription resolve = maybe_resolve.value(); // MSAA Resolve Texture. EXPECT_EQ(resolve.initialLayout, vk::ImageLayout::eUndefined); diff --git a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc index 055c91a3ccb10..f4b470ebe2a3f 100644 --- a/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc +++ b/impeller/renderer/backend/vulkan/render_pass_cache_unittests.cc @@ -23,8 +23,7 @@ TEST_P(RendererTest, CachesRenderPassAndFramebuffer) { auto render_target = allocator->CreateOffscreenMSAA(*GetContext(), {100, 100}, 1); - auto resolve_texture = - render_target.GetColorAttachments().find(0u)->second.resolve_texture; + auto resolve_texture = render_target.GetColorAttachment(0).resolve_texture; auto& texture_vk = TextureVK::Cast(*resolve_texture); EXPECT_EQ(texture_vk.GetCachedFramebuffer(), nullptr); diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 3808d84fd34c2..1bbebd409cce6 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -56,12 +56,14 @@ static std::vector GetVKClearValues( const RenderTarget& target) { std::vector clears; - for (const auto& [_, color] : target.GetColorAttachments()) { - clears.emplace_back(VKClearValueFromColor(color.clear_color)); - if (color.resolve_texture) { - clears.emplace_back(VKClearValueFromColor(color.clear_color)); - } - } + target.IterateAllColorAttachments( + [&clears](size_t index, const ColorAttachment& attachment) -> bool { + clears.emplace_back(VKClearValueFromColor(attachment.clear_color)); + if (attachment.resolve_texture) { + clears.emplace_back(VKClearValueFromColor(attachment.clear_color)); + } + return true; + }); const auto& depth = target.GetDepthAttachment(); const auto& stencil = target.GetStencilAttachment(); @@ -83,22 +85,24 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( const std::shared_ptr& command_buffer) const { RenderPassBuilderVK builder; - for (const auto& [bind_point, color] : render_target_.GetColorAttachments()) { - builder.SetColorAttachment( - bind_point, // - color.texture->GetTextureDescriptor().format, // - color.texture->GetTextureDescriptor().sample_count, // - color.load_action, // - color.store_action, // - TextureVK::Cast(*color.texture).GetLayout() // - ); - TextureVK::Cast(*color.texture) - .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); - if (color.resolve_texture) { - TextureVK::Cast(*color.resolve_texture) - .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); - } - } + render_target_.IterateAllColorAttachments( + [&](size_t bind_point, const ColorAttachment& attachment) -> bool { + builder.SetColorAttachment( + bind_point, // + attachment.texture->GetTextureDescriptor().format, // + attachment.texture->GetTextureDescriptor().sample_count, // + attachment.load_action, // + attachment.store_action, // + TextureVK::Cast(*attachment.texture).GetLayout() // + ); + TextureVK::Cast(*attachment.texture) + .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); + if (attachment.resolve_texture) { + TextureVK::Cast(*attachment.resolve_texture) + .SetLayoutWithoutEncoding(vk::ImageLayout::eGeneral); + } + return true; + }); if (auto depth = render_target_.GetDepthAttachment(); depth.has_value()) { builder.SetDepthStencilAttachment( @@ -137,10 +141,9 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, const RenderTarget& target, std::shared_ptr command_buffer) : RenderPass(context, target), command_buffer_(std::move(command_buffer)) { - color_image_vk_ = - render_target_.GetColorAttachments().find(0u)->second.texture; - resolve_image_vk_ = - render_target_.GetColorAttachments().find(0u)->second.resolve_texture; + const ColorAttachment& color0 = render_target_.GetColorAttachment(0); + color_image_vk_ = color0.texture; + resolve_image_vk_ = color0.resolve_texture; const auto& vk_context = ContextVK::Cast(*context); command_buffer_vk_ = command_buffer_->GetCommandBuffer(); @@ -254,16 +257,19 @@ SharedHandleVK RenderPassVK::CreateVKFramebuffer( // This bit must be consistent to ensure compatibility with the pass created // earlier. Follow this order: Color attachments, then depth-stencil, then // stencil. - for (const auto& [_, color] : render_target_.GetColorAttachments()) { - // The bind point doesn't matter here since that information is present in - // the render pass. - attachments.emplace_back( - TextureVK::Cast(*color.texture).GetRenderTargetView()); - if (color.resolve_texture) { - attachments.emplace_back( - TextureVK::Cast(*color.resolve_texture).GetRenderTargetView()); - } - } + render_target_.IterateAllColorAttachments( + [&attachments](size_t index, const ColorAttachment& attachment) -> bool { + // The bind point doesn't matter here since that information is present + // in the render pass. + attachments.emplace_back( + TextureVK::Cast(*attachment.texture).GetRenderTargetView()); + if (attachment.resolve_texture) { + attachments.emplace_back(TextureVK::Cast(*attachment.resolve_texture) + .GetRenderTargetView()); + } + return true; + }); + if (auto depth = render_target_.GetDepthAttachment(); depth.has_value()) { attachments.emplace_back( TextureVK::Cast(*depth->texture).GetRenderTargetView()); diff --git a/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc b/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc index 7589f849a15e4..4347df46aa561 100644 --- a/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc +++ b/impeller/renderer/backend/vulkan/test/swapchain_unittests.cc @@ -86,8 +86,7 @@ TEST(SwapchainTest, CachesRenderPassOnSwapchainImage) { auto& depth = render_target.GetDepthAttachment(); depth_stencil_textures.push_back(depth.has_value() ? depth->texture : nullptr); - msaa_textures.push_back( - render_target.GetColorAttachments().find(0u)->second.texture); + msaa_textures.push_back(render_target.GetColorAttachment(0).texture); } for (auto i = 1; i < 3; i++) { diff --git a/impeller/renderer/render_target.cc b/impeller/renderer/render_target.cc index dd70f9f202a9d..b3621e706b659 100644 --- a/impeller/renderer/render_target.cc +++ b/impeller/renderer/render_target.cc @@ -28,6 +28,7 @@ bool RenderTarget::IsValid() const { return false; } +#ifndef NDEBUG // Validate that all attachments are of the same size. { std::optional size; @@ -87,12 +88,34 @@ bool RenderTarget::IsValid() const { return false; } } +#endif // NDEBUG return true; } +bool RenderTarget::IterateAllColorAttachments( + const std::function& + iterator) const { + if (color0_.has_value()) { + if (!iterator(0, color0_.value())) { + return false; + } + } + for (const auto& [index, attachment] : colors_) { + if (!iterator(index, attachment)) { + return false; + } + } + return true; +} + void RenderTarget::IterateAllAttachments( const std::function& iterator) const { + if (color0_.has_value()) { + if (!iterator(color0_.value())) { + return; + } + } for (const auto& color : colors_) { if (!iterator(color.second)) { return; @@ -113,13 +136,16 @@ void RenderTarget::IterateAllAttachments( } SampleCount RenderTarget::GetSampleCount() const { - if (auto found = colors_.find(0u); found != colors_.end()) { - return found->second.texture->GetTextureDescriptor().sample_count; + if (color0_.has_value()) { + return color0_.value().texture->GetTextureDescriptor().sample_count; } return SampleCount::kCount1; } bool RenderTarget::HasColorAttachment(size_t index) const { + if (index == 0u) { + return color0_.has_value(); + } if (auto found = colors_.find(index); found != colors_.end()) { return true; } @@ -127,6 +153,12 @@ bool RenderTarget::HasColorAttachment(size_t index) const { } std::optional RenderTarget::GetColorAttachmentSize(size_t index) const { + if (index == 0u) { + if (color0_.has_value()) { + return color0_.value().texture->GetSize(); + } + return std::nullopt; + } auto found = colors_.find(index); if (found == colors_.end()) { @@ -142,12 +174,10 @@ ISize RenderTarget::GetRenderTargetSize() const { } std::shared_ptr RenderTarget::GetRenderTargetTexture() const { - auto found = colors_.find(0u); - if (found == colors_.end()) { + if (!color0_.has_value()) { return nullptr; } - return found->second.resolve_texture ? found->second.resolve_texture - : found->second.texture; + return color0_->resolve_texture ? color0_->resolve_texture : color0_->texture; } PixelFormat RenderTarget::GetRenderTargetPixelFormat() const { @@ -169,7 +199,12 @@ size_t RenderTarget::GetMaxColorAttacmentBindIndex() const { RenderTarget& RenderTarget::SetColorAttachment( const ColorAttachment& attachment, size_t index) { - if (attachment.IsValid()) { + if (!attachment.IsValid()) { + return *this; + } + if (index == 0u) { + color0_ = attachment; + } else { colors_[index] = attachment; } return *this; @@ -195,9 +230,18 @@ RenderTarget& RenderTarget::SetStencilAttachment( return *this; } -const std::map& RenderTarget::GetColorAttachments() - const { - return colors_; +ColorAttachment RenderTarget::GetColorAttachment(size_t index) const { + if (index == 0) { + if (color0_.has_value()) { + return color0_.value(); + } + return ColorAttachment{}; + } + std::map::const_iterator it = colors_.find(index); + if (it != colors_.end()) { + return it->second; + } + return ColorAttachment{}; } const std::optional& RenderTarget::GetDepthAttachment() const { @@ -219,6 +263,9 @@ size_t RenderTarget::GetTotalAttachmentCount() const { count++; } } + if (color0_.has_value()) { + count++; + } if (depth_.has_value()) { count++; } @@ -231,6 +278,10 @@ size_t RenderTarget::GetTotalAttachmentCount() const { std::string RenderTarget::ToString() const { std::stringstream stream; + if (color0_.has_value()) { + stream << SPrintF("Color[%d]=(%s)", 0, + ColorAttachmentToString(color0_.value()).c_str()); + } for (const auto& [index, color] : colors_) { stream << SPrintF("Color[%zu]=(%s)", index, ColorAttachmentToString(color).c_str()); @@ -248,6 +299,18 @@ std::string RenderTarget::ToString() const { return stream.str(); } +RenderTargetConfig RenderTarget::ToConfig() const { + if (!color0_.has_value()) { + return RenderTargetConfig{}; + } + const auto& color_attachment = color0_.value(); + 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()}; +} + RenderTargetAllocator::RenderTargetAllocator( std::shared_ptr allocator) : allocator_(std::move(allocator)) {} diff --git a/impeller/renderer/render_target.h b/impeller/renderer/render_target.h index 77f5bf43f5e38..a7a1e0bd32c9a 100644 --- a/impeller/renderer/render_target.h +++ b/impeller/renderer/render_target.h @@ -102,6 +102,13 @@ class RenderTarget final { RenderTarget& SetColorAttachment(const ColorAttachment& attachment, size_t index); + /// @brief Get the color attachment at [index]. + /// + /// This function does not validate whether or not the attachment was + /// previously defined and will return a default constructed attachment + /// if none is set. + ColorAttachment GetColorAttachment(size_t index) const; + RenderTarget& SetDepthAttachment(std::optional attachment); RenderTarget& SetStencilAttachment( @@ -109,32 +116,32 @@ class RenderTarget final { size_t GetMaxColorAttacmentBindIndex() const; - const std::map& GetColorAttachments() const; - const std::optional& GetDepthAttachment() const; const std::optional& GetStencilAttachment() const; size_t GetTotalAttachmentCount() const; + bool IterateAllColorAttachments( + const std::function& iterator) + const; + void IterateAllAttachments( const std::function& iterator) const; 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()}; - } + RenderTargetConfig ToConfig() const; private: - std::map colors_; + std::optional color0_; std::optional depth_; std::optional stencil_; + // Note: color0 is stored in the color0_ field above and not in this map, + // to avoid heap allocations for the commonly created render target formats + // in Flutter. + std::map colors_; }; /// @brief a wrapper around the impeller [Allocator] instance that can be used diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 633ad6ddba897..0e84ea50cfd17 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -589,10 +589,6 @@ TEST_P(RendererTest, CanBlitTextureToTexture) { } pass->SetLabel("Playground Blit Pass"); - if (render_target.GetColorAttachments().empty()) { - return false; - } - // Blit `bridge` to the top left corner of the texture. pass->AddCopy(bridge, texture); @@ -709,10 +705,6 @@ TEST_P(RendererTest, CanBlitTextureToBuffer) { } pass->SetLabel("Playground Blit Pass"); - if (render_target.GetColorAttachments().empty()) { - return false; - } - // Blit `bridge` to the top left corner of the texture. pass->AddCopy(bridge, device_buffer); pass->EncodeCommands(context->GetResourceAllocator()); diff --git a/lib/gpu/render_pass.cc b/lib/gpu/render_pass.cc index 3987d823f5a22..a566567a9484f 100644 --- a/lib/gpu/render_pass.cc +++ b/lib/gpu/render_pass.cc @@ -105,10 +105,13 @@ RenderPass::GetOrCreatePipeline() { pipeline_desc.SetSampleCount(render_target_.GetSampleCount()); - for (const auto& it : render_target_.GetColorAttachments()) { - auto& color = GetColorAttachmentDescriptor(it.first); - color.format = render_target_.GetRenderTargetPixelFormat(); - } + render_target_.IterateAllColorAttachments( + [&](size_t index, const impeller::ColorAttachment& attachment) -> bool { + auto& color = GetColorAttachmentDescriptor(index); + color.format = render_target_.GetRenderTargetPixelFormat(); + return true; + }); + pipeline_desc.SetColorAttachmentDescriptors(color_descriptors_); {