Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] distinguish between no clear color and transparent black clear color. #49038

Merged
merged 4 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 23 additions & 5 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2926,19 +2926,18 @@ TEST_P(AiksTest, ClearColorOptimizationDoesNotApplyForBackdropFilters) {
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) {
Expand Down Expand Up @@ -4555,5 +4554,24 @@ TEST_P(AiksTest, GaussianBlurWithoutDecalSupport) {
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();

ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

} // namespace testing
} // namespace impeller
1 change: 0 additions & 1 deletion impeller/aiks/paint_pass_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
30 changes: 18 additions & 12 deletions impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,8 @@ bool EntityPass::Render(ContentContext& renderer,
if (reads_from_onscreen_backdrop) {
auto offscreen_target =
CreateRenderTarget(renderer, root_render_target.GetRenderTargetSize(),
GetClearColor(render_target.GetRenderTargetSize()));
GetClearColor(render_target.GetRenderTargetSize())
.value_or(Color::BlackTransparent()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should pull out this constant or pull out a helper function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetClearColorOrTransparentBlack()?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some options. I think "OrDefault" is more succinct. I'm fine with either though.

GetClearColor(render_target.GetRenderTargetSize())
  .value_or(kDefaultClearColor);
OrDefaultClearColor(
  GetClearColor(render_target.GetRenderTargetSize()))
GetClearColorOrDefault(render_target.GetRenderTargetSize())

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with OrDefault, Thanks!


if (!OnRender(renderer, // renderer
capture, // capture
Expand Down Expand Up @@ -447,7 +448,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 = GetClearColor(render_target.GetRenderTargetSize())
.value_or(Color::BlackTransparent());
root_render_target.SetColorAttachment(color0, 0);

EntityPassTarget pass_target(
Expand Down Expand Up @@ -600,9 +602,10 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
}

auto subpass_target = CreateRenderTarget(
renderer, // renderer
subpass_size, // size
subpass->GetClearColor(subpass_size)); // clear_color
renderer, // renderer
subpass_size, // size
subpass->GetClearColor(subpass_size)
.value_or(Color::BlackTransparent())); // clear_color

if (!subpass_target.IsValid()) {
VALIDATION_LOG << "Subpass render target is invalid.";
Expand Down Expand Up @@ -845,8 +848,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);
Expand Down Expand Up @@ -1139,21 +1141,25 @@ void EntityPass::SetBlendMode(BlendMode blend_mode) {
flood_clip_ = Entity::IsBlendModeDestructive(blend_mode);
}

Color EntityPass::GetClearColor(ISize target_size) const {
Color result = 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);
}
if (result.has_value()) {
return result->Premultiply();
}
return result.Premultiply();
return result;
}

void EntityPass::SetBackdropFilter(BackdropFilterProc proc) {
Expand Down
6 changes: 5 additions & 1 deletion impeller/entity/entity_pass.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ class EntityPass {

void SetBlendMode(BlendMode blend_mode);

Color GetClearColor(ISize size = ISize::Infinite()) const;
/// @brief Return the clear color of the pass entities.
///
/// If the returned value is std::nullopt, the clear color can be assumed to
/// be transparent black.
std::optional<Color> GetClearColor(ISize size = ISize::Infinite()) const;

void SetBackdropFilter(BackdropFilterProc proc);

Expand Down