From 88ae663bce84686377ef795748aba83457667961 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Thu, 14 Dec 2023 13:37:29 -0800 Subject: [PATCH] [Impeller] distinguish between no clear color and transparent black clear color. (#49038) If we clear to transparent black, we're not forcing the pass to be constructed. Change the entity pass API so that we can tell the difference between clearing transparent black and not having a clear color. In https://github.com/flutter/flutter/issues/139571 , the app is creating a layer that is clearing to a transparent color, which is getting skipped. That invalid texture is fed into a blend which produces either black or magenta error texture. Fixes https://github.com/flutter/flutter/issues/139571 --- impeller/aiks/aiks_unittests.cc | 31 +++++++++++++++++++++---- impeller/aiks/paint_pass_delegate.cc | 1 - impeller/entity/entity_pass.cc | 34 +++++++++++++++++----------- impeller/entity/entity_pass.h | 8 ++++++- 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 115f3c1950007..42b9fd6ab0122 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -2416,19 +2416,18 @@ TEST_P(AiksTest, ClearColorOptimizationDoesNotApplyForBackdropFilters) { Picture picture = canvas.EndRecordingAsPicture(); std::optional actual_color; + bool found_subpass = false; picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool { if (auto subpass = std::get_if>(&element)) { actual_color = subpass->get()->GetClearColor(); + found_subpass = true; } // Fail if the first element isn't a subpass. return true; }); - ASSERT_TRUE(actual_color.has_value()); - if (!actual_color) { - return; - } - ASSERT_EQ(actual_color.value(), Color::BlackTransparent()); + EXPECT_TRUE(found_subpass); + EXPECT_FALSE(actual_color.has_value()); } TEST_P(AiksTest, CollapsedDrawPaintInSubpass) { @@ -3645,5 +3644,27 @@ TEST_P(AiksTest, MaskBlurWithZeroSigmaIsSkipped) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, SubpassWithClearColorOptimization) { + Canvas canvas; + + // Use a non-srcOver blend mode to ensure that we don't detect this as an + // opacity peephole optimization. + canvas.SaveLayer( + {.color = Color::Blue().WithAlpha(0.5), .blend_mode = BlendMode::kSource}, + Rect::MakeLTRB(0, 0, 200, 200)); + canvas.DrawPaint( + {.color = Color::BlackTransparent(), .blend_mode = BlendMode::kSource}); + canvas.Restore(); + + canvas.SaveLayer( + {.color = Color::Blue(), .blend_mode = BlendMode::kDestinationOver}); + canvas.Restore(); + + // This playground should appear blank on CI since we are only drawing + // transparent black. If the clear color optimization is broken, the texture + // will be filled with NaNs and may produce a magenta texture on macOS or iOS. + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + } // namespace testing } // namespace impeller diff --git a/impeller/aiks/paint_pass_delegate.cc b/impeller/aiks/paint_pass_delegate.cc index f9a081a0b55cf..604e4dc92831c 100644 --- a/impeller/aiks/paint_pass_delegate.cc +++ b/impeller/aiks/paint_pass_delegate.cc @@ -10,7 +10,6 @@ #include "impeller/entity/contents/texture_contents.h" #include "impeller/entity/entity_pass.h" #include "impeller/geometry/color.h" -#include "impeller/geometry/path_builder.h" namespace impeller { diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 64580febfcf24..495856a338bb5 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -368,7 +368,7 @@ bool EntityPass::Render(ContentContext& renderer, if (!supports_onscreen_backdrop_reads && reads_from_onscreen_backdrop) { auto offscreen_target = CreateRenderTarget( renderer, root_render_target.GetRenderTargetSize(), true, - GetClearColor(render_target.GetRenderTargetSize())); + GetClearColorOrDefault(render_target.GetRenderTargetSize())); if (!OnRender(renderer, // renderer capture, // capture @@ -475,7 +475,8 @@ bool EntityPass::Render(ContentContext& renderer, } // Set up the clear color of the root pass. - color0.clear_color = GetClearColor(render_target.GetRenderTargetSize()); + color0.clear_color = + GetClearColorOrDefault(render_target.GetRenderTargetSize()); root_render_target.SetColorAttachment(color0, 0); EntityPassTarget pass_target( @@ -628,10 +629,10 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( } auto subpass_target = CreateRenderTarget( - renderer, // renderer - subpass_size, // size - subpass->GetTotalPassReads(renderer) > 0, // readable - subpass->GetClearColor(subpass_size)); // clear_color + renderer, // renderer + subpass_size, // size + subpass->GetTotalPassReads(renderer) > 0, // readable + subpass->GetClearColorOrDefault(subpass_size)); // clear_color if (!subpass_target.IsValid()) { VALIDATION_LOG << "Subpass render target is invalid."; @@ -722,8 +723,7 @@ bool EntityPass::OnRender( } auto clear_color_size = pass_target.GetRenderTarget().GetRenderTargetSize(); - if (!collapsed_parent_pass && - !GetClearColor(clear_color_size).IsTransparent()) { + if (!collapsed_parent_pass && GetClearColor(clear_color_size).has_value()) { // Force the pass context to create at least one new pass if the clear color // is present. pass_context.GetRenderPass(pass_depth); @@ -1140,21 +1140,29 @@ void EntityPass::SetBlendMode(BlendMode blend_mode) { flood_clip_ = Entity::IsBlendModeDestructive(blend_mode); } -Color EntityPass::GetClearColor(ISize target_size) const { - Color result = Color::BlackTransparent(); +Color EntityPass::GetClearColorOrDefault(ISize size) const { + return GetClearColor(size).value_or(Color::BlackTransparent()); +} + +std::optional EntityPass::GetClearColor(ISize target_size) const { if (backdrop_filter_proc_) { - return result; + return std::nullopt; } + std::optional result = std::nullopt; for (const Element& element : elements_) { auto [entity_color, blend_mode] = ElementAsBackgroundColor(element, target_size); if (!entity_color.has_value()) { break; } - result = result.Blend(entity_color.value(), blend_mode); + result = result.value_or(Color::BlackTransparent()) + .Blend(entity_color.value(), blend_mode); } - return result.Premultiply(); + if (result.has_value()) { + return result->Premultiply(); + } + return result; } void EntityPass::SetBackdropFilter(BackdropFilterProc proc) { diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index d09649abfd9d0..c03a837047eeb 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -135,7 +135,13 @@ class EntityPass { void SetBlendMode(BlendMode blend_mode); - Color GetClearColor(ISize size = ISize::Infinite()) const; + /// @brief Return the premultiplied clear color of the pass entities, if any. + std::optional GetClearColor(ISize size = ISize::Infinite()) const; + + /// @brief Return the premultiplied clear color of the pass entities. + /// + /// If the entity pass has no clear color, this will return transparent black. + Color GetClearColorOrDefault(ISize size = ISize::Infinite()) const; void SetBackdropFilter(BackdropFilterProc proc);