[CP] Fix for AnimatedOpacity affects blended color overlay render (#49190)
This PR cherry-picks the following two commits:
https://github.com/flutter/engine/commit/2dbc5d2c576ae3e80b23c28e3f4989e79821b41d
https://github.com/flutter/engine/commit/0b0fab8215362f50a034d108ad11fbb8e93c3fce
to address https://github.com/flutter/flutter/issues/139571
diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc
index 115f3c1..42b9fd6 100644
--- a/impeller/aiks/aiks_unittests.cc
+++ b/impeller/aiks/aiks_unittests.cc
@@ -2416,19 +2416,18 @@
Picture picture = canvas.EndRecordingAsPicture();
std::optional<Color> actual_color;
+ bool found_subpass = false;
picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool {
if (auto subpass = std::get_if<std::unique_ptr<EntityPass>>(&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 @@
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 f9a081a..604e4dc 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/contents/clip_contents.cc b/impeller/entity/contents/clip_contents.cc
index 21c2eb1..77bbccc 100644
--- a/impeller/entity/contents/clip_contents.cc
+++ b/impeller/entity/contents/clip_contents.cc
@@ -66,7 +66,7 @@
bool ClipContents::ShouldRender(
const Entity& entity,
- const std::optional<Rect>& stencil_coverage) const {
+ const std::optional<Rect> stencil_coverage) const {
return true;
}
@@ -163,7 +163,7 @@
bool ClipRestoreContents::ShouldRender(
const Entity& entity,
- const std::optional<Rect>& stencil_coverage) const {
+ const std::optional<Rect> stencil_coverage) const {
return true;
}
diff --git a/impeller/entity/contents/clip_contents.h b/impeller/entity/contents/clip_contents.h
index 3b0faac..bc7b89a 100644
--- a/impeller/entity/contents/clip_contents.h
+++ b/impeller/entity/contents/clip_contents.h
@@ -35,7 +35,7 @@
// |Contents|
bool ShouldRender(const Entity& entity,
- const std::optional<Rect>& stencil_coverage) const override;
+ const std::optional<Rect> stencil_coverage) const override;
// |Contents|
bool Render(const ContentContext& renderer,
@@ -76,7 +76,7 @@
// |Contents|
bool ShouldRender(const Entity& entity,
- const std::optional<Rect>& stencil_coverage) const override;
+ const std::optional<Rect> stencil_coverage) const override;
// |Contents|
bool Render(const ContentContext& renderer,
diff --git a/impeller/entity/contents/contents.cc b/impeller/entity/contents/contents.cc
index a181e22..4671d23 100644
--- a/impeller/entity/contents/contents.cc
+++ b/impeller/entity/contents/contents.cc
@@ -133,11 +133,10 @@
}
bool Contents::ShouldRender(const Entity& entity,
- const std::optional<Rect>& stencil_coverage) const {
+ const std::optional<Rect> stencil_coverage) const {
if (!stencil_coverage.has_value()) {
return false;
}
-
auto coverage = GetCoverage(entity);
if (!coverage.has_value()) {
return false;
diff --git a/impeller/entity/contents/contents.h b/impeller/entity/contents/contents.h
index b9dec5d..11b5ad0 100644
--- a/impeller/entity/contents/contents.h
+++ b/impeller/entity/contents/contents.h
@@ -113,7 +113,7 @@
const std::string& label = "Snapshot") const;
virtual bool ShouldRender(const Entity& entity,
- const std::optional<Rect>& stencil_coverage) const;
+ const std::optional<Rect> stencil_coverage) const;
//----------------------------------------------------------------------------
/// @brief Return the color source's intrinsic size, if available.
diff --git a/impeller/entity/entity.cc b/impeller/entity/entity.cc
index b7ebc2a..ae18016 100644
--- a/impeller/entity/entity.cc
+++ b/impeller/entity/entity.cc
@@ -71,7 +71,11 @@
}
bool Entity::ShouldRender(const std::optional<Rect>& stencil_coverage) const {
+#ifdef IMPELLER_CONTENT_CULLING
return contents_->ShouldRender(*this, stencil_coverage);
+#else
+ return true;
+#endif // IMPELLER_CONTENT_CULLING
}
void Entity::SetContents(std::shared_ptr<Contents> contents) {
diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc
index 64580fe..495856a 100644
--- a/impeller/entity/entity_pass.cc
+++ b/impeller/entity/entity_pass.cc
@@ -368,7 +368,7 @@
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 @@
}
// 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 @@
}
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 @@
}
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 @@
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<Color> EntityPass::GetClearColor(ISize target_size) const {
if (backdrop_filter_proc_) {
- return result;
+ return std::nullopt;
}
+ std::optional<Color> 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 d09649a..c03a837 100644
--- a/impeller/entity/entity_pass.h
+++ b/impeller/entity/entity_pass.h
@@ -135,7 +135,13 @@
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<Color> 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);
diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
index ca094aa..2b28fa3 100644
--- a/impeller/entity/entity_unittests.cc
+++ b/impeller/entity/entity_unittests.cc
@@ -1607,6 +1607,20 @@
}
}
+TEST_P(EntityTest, DoesNotCullEntitiesByDefault) {
+ auto fill = std::make_shared<SolidColorContents>();
+ fill->SetColor(Color::CornflowerBlue());
+ fill->SetGeometry(
+ Geometry::MakeRect(Rect::MakeLTRB(-1000, -1000, -900, -900)));
+
+ Entity entity;
+ entity.SetContents(fill);
+
+ // Even though the entity is offscreen, this should still render because we do
+ // not compute the coverage intersection by default.
+ EXPECT_TRUE(entity.ShouldRender(Rect::MakeLTRB(0, 0, 100, 100)));
+}
+
TEST_P(EntityTest, ClipContentsShouldRenderIsCorrect) {
// For clip ops, `ShouldRender` should always return true.
