Skip to content

Commit 0e88c72

Browse files
Jonah Williamszanderso
authored andcommitted
[Impeller] distinguish between no clear color and transparent black clear color. (flutter#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 flutter/flutter#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 flutter/flutter#139571
1 parent f2ea655 commit 0e88c72

File tree

4 files changed

+54
-20
lines changed

4 files changed

+54
-20
lines changed

impeller/aiks/aiks_unittests.cc

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2416,19 +2416,18 @@ TEST_P(AiksTest, ClearColorOptimizationDoesNotApplyForBackdropFilters) {
24162416
Picture picture = canvas.EndRecordingAsPicture();
24172417

24182418
std::optional<Color> actual_color;
2419+
bool found_subpass = false;
24192420
picture.pass->IterateAllElements([&](EntityPass::Element& element) -> bool {
24202421
if (auto subpass = std::get_if<std::unique_ptr<EntityPass>>(&element)) {
24212422
actual_color = subpass->get()->GetClearColor();
2423+
found_subpass = true;
24222424
}
24232425
// Fail if the first element isn't a subpass.
24242426
return true;
24252427
});
24262428

2427-
ASSERT_TRUE(actual_color.has_value());
2428-
if (!actual_color) {
2429-
return;
2430-
}
2431-
ASSERT_EQ(actual_color.value(), Color::BlackTransparent());
2429+
EXPECT_TRUE(found_subpass);
2430+
EXPECT_FALSE(actual_color.has_value());
24322431
}
24332432

24342433
TEST_P(AiksTest, CollapsedDrawPaintInSubpass) {
@@ -3645,5 +3644,27 @@ TEST_P(AiksTest, MaskBlurWithZeroSigmaIsSkipped) {
36453644
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
36463645
}
36473646

3647+
TEST_P(AiksTest, SubpassWithClearColorOptimization) {
3648+
Canvas canvas;
3649+
3650+
// Use a non-srcOver blend mode to ensure that we don't detect this as an
3651+
// opacity peephole optimization.
3652+
canvas.SaveLayer(
3653+
{.color = Color::Blue().WithAlpha(0.5), .blend_mode = BlendMode::kSource},
3654+
Rect::MakeLTRB(0, 0, 200, 200));
3655+
canvas.DrawPaint(
3656+
{.color = Color::BlackTransparent(), .blend_mode = BlendMode::kSource});
3657+
canvas.Restore();
3658+
3659+
canvas.SaveLayer(
3660+
{.color = Color::Blue(), .blend_mode = BlendMode::kDestinationOver});
3661+
canvas.Restore();
3662+
3663+
// This playground should appear blank on CI since we are only drawing
3664+
// transparent black. If the clear color optimization is broken, the texture
3665+
// will be filled with NaNs and may produce a magenta texture on macOS or iOS.
3666+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
3667+
}
3668+
36483669
} // namespace testing
36493670
} // namespace impeller

impeller/aiks/paint_pass_delegate.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
#include "impeller/entity/contents/texture_contents.h"
1111
#include "impeller/entity/entity_pass.h"
1212
#include "impeller/geometry/color.h"
13-
#include "impeller/geometry/path_builder.h"
1413

1514
namespace impeller {
1615

impeller/entity/entity_pass.cc

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ bool EntityPass::Render(ContentContext& renderer,
368368
if (!supports_onscreen_backdrop_reads && reads_from_onscreen_backdrop) {
369369
auto offscreen_target = CreateRenderTarget(
370370
renderer, root_render_target.GetRenderTargetSize(), true,
371-
GetClearColor(render_target.GetRenderTargetSize()));
371+
GetClearColorOrDefault(render_target.GetRenderTargetSize()));
372372

373373
if (!OnRender(renderer, // renderer
374374
capture, // capture
@@ -475,7 +475,8 @@ bool EntityPass::Render(ContentContext& renderer,
475475
}
476476

477477
// Set up the clear color of the root pass.
478-
color0.clear_color = GetClearColor(render_target.GetRenderTargetSize());
478+
color0.clear_color =
479+
GetClearColorOrDefault(render_target.GetRenderTargetSize());
479480
root_render_target.SetColorAttachment(color0, 0);
480481

481482
EntityPassTarget pass_target(
@@ -628,10 +629,10 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
628629
}
629630

630631
auto subpass_target = CreateRenderTarget(
631-
renderer, // renderer
632-
subpass_size, // size
633-
subpass->GetTotalPassReads(renderer) > 0, // readable
634-
subpass->GetClearColor(subpass_size)); // clear_color
632+
renderer, // renderer
633+
subpass_size, // size
634+
subpass->GetTotalPassReads(renderer) > 0, // readable
635+
subpass->GetClearColorOrDefault(subpass_size)); // clear_color
635636

636637
if (!subpass_target.IsValid()) {
637638
VALIDATION_LOG << "Subpass render target is invalid.";
@@ -722,8 +723,7 @@ bool EntityPass::OnRender(
722723
}
723724
auto clear_color_size = pass_target.GetRenderTarget().GetRenderTargetSize();
724725

725-
if (!collapsed_parent_pass &&
726-
!GetClearColor(clear_color_size).IsTransparent()) {
726+
if (!collapsed_parent_pass && GetClearColor(clear_color_size).has_value()) {
727727
// Force the pass context to create at least one new pass if the clear color
728728
// is present.
729729
pass_context.GetRenderPass(pass_depth);
@@ -1140,21 +1140,29 @@ void EntityPass::SetBlendMode(BlendMode blend_mode) {
11401140
flood_clip_ = Entity::IsBlendModeDestructive(blend_mode);
11411141
}
11421142

1143-
Color EntityPass::GetClearColor(ISize target_size) const {
1144-
Color result = Color::BlackTransparent();
1143+
Color EntityPass::GetClearColorOrDefault(ISize size) const {
1144+
return GetClearColor(size).value_or(Color::BlackTransparent());
1145+
}
1146+
1147+
std::optional<Color> EntityPass::GetClearColor(ISize target_size) const {
11451148
if (backdrop_filter_proc_) {
1146-
return result;
1149+
return std::nullopt;
11471150
}
11481151

1152+
std::optional<Color> result = std::nullopt;
11491153
for (const Element& element : elements_) {
11501154
auto [entity_color, blend_mode] =
11511155
ElementAsBackgroundColor(element, target_size);
11521156
if (!entity_color.has_value()) {
11531157
break;
11541158
}
1155-
result = result.Blend(entity_color.value(), blend_mode);
1159+
result = result.value_or(Color::BlackTransparent())
1160+
.Blend(entity_color.value(), blend_mode);
11561161
}
1157-
return result.Premultiply();
1162+
if (result.has_value()) {
1163+
return result->Premultiply();
1164+
}
1165+
return result;
11581166
}
11591167

11601168
void EntityPass::SetBackdropFilter(BackdropFilterProc proc) {

impeller/entity/entity_pass.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,13 @@ class EntityPass {
135135

136136
void SetBlendMode(BlendMode blend_mode);
137137

138-
Color GetClearColor(ISize size = ISize::Infinite()) const;
138+
/// @brief Return the premultiplied clear color of the pass entities, if any.
139+
std::optional<Color> GetClearColor(ISize size = ISize::Infinite()) const;
140+
141+
/// @brief Return the premultiplied clear color of the pass entities.
142+
///
143+
/// If the entity pass has no clear color, this will return transparent black.
144+
Color GetClearColorOrDefault(ISize size = ISize::Infinite()) const;
139145

140146
void SetBackdropFilter(BackdropFilterProc proc);
141147

0 commit comments

Comments
 (0)