Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 4a4ef50

Browse files
authored
Revert "[Impeller] Absorb DrawPaints at the beginning of EntityPasses (#40675)"
This reverts commit 29c2845.
1 parent 0ea89c3 commit 4a4ef50

File tree

6 files changed

+23
-79
lines changed

6 files changed

+23
-79
lines changed

impeller/aiks/aiks_unittests.cc

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1993,17 +1993,5 @@ TEST_P(AiksTest, OpacityPeepHoleApplicationTest) {
19931993
ASSERT_TRUE(delegate->CanCollapseIntoParentPass(entity_pass.get()));
19941994
}
19951995

1996-
TEST_P(AiksTest, DrawPaintAbsorbsClears) {
1997-
Canvas canvas;
1998-
canvas.DrawPaint({.color = Color::Red(), .blend_mode = BlendMode::kSource});
1999-
canvas.DrawPaint(
2000-
{.color = Color::CornflowerBlue(), .blend_mode = BlendMode::kSource});
2001-
2002-
Picture picture = canvas.EndRecordingAsPicture();
2003-
2004-
ASSERT_EQ(picture.pass->GetElementCount(), 0u);
2005-
ASSERT_EQ(picture.pass->GetClearColor(), Color::CornflowerBlue());
2006-
}
2007-
20081996
} // namespace testing
20091997
} // namespace impeller

impeller/aiks/canvas.cc

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -167,18 +167,6 @@ void Canvas::DrawPath(const Path& path, const Paint& paint) {
167167
}
168168

169169
void Canvas::DrawPaint(const Paint& paint) {
170-
bool is_clear =
171-
paint.blend_mode == BlendMode::kSource ||
172-
(paint.blend_mode == BlendMode::kSourceOver && paint.color.alpha == 1);
173-
if (xformation_stack_.size() == 1 && // If we're recording the root pass,
174-
GetCurrentPass().GetElementCount() == 0 && // and this is the first item,
175-
is_clear // and the backdrop is being replaced
176-
) {
177-
// Then we can absorb this drawPaint as the clear color of the pass.
178-
GetCurrentPass().SetClearColor(paint.color);
179-
return;
180-
}
181-
182170
Entity entity;
183171
entity.SetTransformation(GetCurrentTransformation());
184172
entity.SetStencilDepth(GetStencilDepth());

impeller/aiks/paint_pass_delegate.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass(
8585
// command wrapped in save layer. This would indicate something like an
8686
// Opacity or FadeTransition wrapping a very simple widget, like in the
8787
// CupertinoPicker.
88-
if (entity_pass->GetElementCount() > 3) {
88+
if (entity_pass->GetEntityCount() > 3) {
8989
// Single paint command with a save layer would be:
9090
// 1. clip
9191
// 2. draw command

impeller/entity/entity_pass.cc

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,7 @@ EntityPass* EntityPass::AddSubpass(std::unique_ptr<EntityPass> pass) {
144144

145145
static EntityPassTarget CreateRenderTarget(ContentContext& renderer,
146146
ISize size,
147-
bool readable,
148-
const Color& clear_color) {
147+
bool readable) {
149148
auto context = renderer.GetContext();
150149

151150
/// All of the load/store actions are managed by `InlinePassContext` when
@@ -164,7 +163,7 @@ static EntityPassTarget CreateRenderTarget(ContentContext& renderer,
164163
.resolve_storage_mode = StorageMode::kDevicePrivate,
165164
.load_action = LoadAction::kDontCare,
166165
.store_action = StoreAction::kMultisampleResolve,
167-
.clear_color = clear_color}, // color_attachment_config
166+
}, // color_attachment_config
168167
RenderTarget::AttachmentConfig{
169168
.storage_mode = readable ? StorageMode::kDevicePrivate
170169
: StorageMode::kDeviceTransient,
@@ -204,18 +203,13 @@ uint32_t EntityPass::GetTotalPassReads(ContentContext& renderer) const {
204203

205204
bool EntityPass::Render(ContentContext& renderer,
206205
const RenderTarget& render_target) const {
207-
if (render_target.GetColorAttachments().empty()) {
208-
VALIDATION_LOG << "The root RenderTarget must have a color attachment.";
209-
return false;
210-
}
211-
212206
StencilCoverageStack stencil_coverage_stack = {StencilCoverageLayer{
213207
.coverage = Rect::MakeSize(render_target.GetRenderTargetSize()),
214208
.stencil_depth = 0}};
215209

216210
if (GetTotalPassReads(renderer) > 0) {
217-
auto offscreen_target = CreateRenderTarget(
218-
renderer, render_target.GetRenderTargetSize(), true, clear_color_);
211+
auto offscreen_target =
212+
CreateRenderTarget(renderer, render_target.GetRenderTargetSize(), true);
219213

220214
if (!OnRender(renderer, // renderer
221215
offscreen_target.GetRenderTarget()
@@ -275,25 +269,18 @@ bool EntityPass::Render(ContentContext& renderer,
275269
return true;
276270
}
277271

278-
// Set up the clear color of the root pass.
279-
auto color0 = render_target.GetColorAttachments().find(0)->second;
280-
color0.clear_color = clear_color_;
281-
282-
auto root_render_target = render_target;
283-
root_render_target.SetColorAttachment(color0, 0);
284-
285272
EntityPassTarget pass_target(
286-
root_render_target,
273+
render_target,
287274
renderer.GetDeviceCapabilities().SupportsReadFromResolve());
288275

289-
return OnRender( //
290-
renderer, // renderer
291-
root_render_target.GetRenderTargetSize(), // root_pass_size
292-
pass_target, // pass_target
293-
Point(), // global_pass_position
294-
Point(), // local_pass_position
295-
0, // pass_depth
296-
stencil_coverage_stack); // stencil_coverage_stack
276+
return OnRender( //
277+
renderer, // renderer
278+
render_target.GetRenderTargetSize(), // root_pass_size
279+
pass_target, // pass_target
280+
Point(), // global_pass_position
281+
Point(), // local_pass_position
282+
0, // pass_depth
283+
stencil_coverage_stack); // stencil_coverage_stack
297284
}
298285

299286
EntityPass::EntityResult EntityPass::GetEntityForElement(
@@ -399,10 +386,9 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
399386
subpass_coverage->Intersection(Rect::MakeSize(root_pass_size));
400387

401388
auto subpass_target =
402-
CreateRenderTarget(renderer, //
403-
ISize(subpass_coverage->size), //
404-
subpass->GetTotalPassReads(renderer) > 0, //
405-
clear_color_);
389+
CreateRenderTarget(renderer, //
390+
ISize(subpass_coverage->size), //
391+
subpass->GetTotalPassReads(renderer) > 0);
406392

407393
auto subpass_texture =
408394
subpass_target.GetRenderTarget().GetRenderTargetTexture();
@@ -715,7 +701,7 @@ bool EntityPass::IterateUntilSubpass(
715701
return false;
716702
}
717703

718-
size_t EntityPass::GetElementCount() const {
704+
size_t EntityPass::GetEntityCount() const {
719705
return elements_.size();
720706
}
721707

@@ -753,14 +739,6 @@ void EntityPass::SetBlendMode(BlendMode blend_mode) {
753739
cover_whole_screen_ = Entity::IsBlendModeDestructive(blend_mode);
754740
}
755741

756-
void EntityPass::SetClearColor(Color clear_color) {
757-
clear_color_ = clear_color;
758-
}
759-
760-
Color EntityPass::GetClearColor() const {
761-
return clear_color_;
762-
}
763-
764742
void EntityPass::SetBackdropFilter(std::optional<BackdropFilterProc> proc) {
765743
if (superpass_) {
766744
VALIDATION_LOG << "Backdrop filters cannot be set on EntityPasses that "

impeller/entity/entity_pass.h

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,15 @@ class EntityPass {
7777
/// @return Returns whether a subpass was encountered.
7878
bool IterateUntilSubpass(const std::function<bool(Entity&)>& iterator);
7979

80-
/// @brief Return the number of elements on this pass.
81-
size_t GetElementCount() const;
80+
/// @brief Return the number of entities on this pass.
81+
size_t GetEntityCount() const;
8282

8383
void SetTransformation(Matrix xformation);
8484

8585
void SetStencilDepth(size_t stencil_depth);
8686

8787
void SetBlendMode(BlendMode blend_mode);
8888

89-
void SetClearColor(Color clear_color);
90-
91-
Color GetClearColor() const;
92-
9389
void SetBackdropFilter(std::optional<BackdropFilterProc> proc);
9490

9591
std::optional<Rect> GetSubpassCoverage(
@@ -208,7 +204,6 @@ class EntityPass {
208204
size_t stencil_depth_ = 0u;
209205
BlendMode blend_mode_ = BlendMode::kSourceOver;
210206
bool cover_whole_screen_ = false;
211-
Color clear_color_ = Color::BlackTransparent();
212207

213208
/// These values are incremented whenever something is added to the pass that
214209
/// requires reading from the backdrop texture. Currently, this can happen in

impeller/renderer/render_target.h

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,35 +23,30 @@ class RenderTarget final {
2323
StorageMode storage_mode;
2424
LoadAction load_action;
2525
StoreAction store_action;
26-
Color clear_color;
2726
};
2827

2928
struct AttachmentConfigMSAA {
3029
StorageMode storage_mode;
3130
StorageMode resolve_storage_mode;
3231
LoadAction load_action;
3332
StoreAction store_action;
34-
Color clear_color;
3533
};
3634

3735
static constexpr AttachmentConfig kDefaultColorAttachmentConfig = {
3836
.storage_mode = StorageMode::kDevicePrivate,
3937
.load_action = LoadAction::kClear,
40-
.store_action = StoreAction::kStore,
41-
.clear_color = Color::BlackTransparent()};
38+
.store_action = StoreAction::kStore};
4239

4340
static constexpr AttachmentConfigMSAA kDefaultColorAttachmentConfigMSAA = {
4441
.storage_mode = StorageMode::kDeviceTransient,
4542
.resolve_storage_mode = StorageMode::kDevicePrivate,
4643
.load_action = LoadAction::kClear,
47-
.store_action = StoreAction::kMultisampleResolve,
48-
.clear_color = Color::BlackTransparent()};
44+
.store_action = StoreAction::kMultisampleResolve};
4945

5046
static constexpr AttachmentConfig kDefaultStencilAttachmentConfig = {
5147
.storage_mode = StorageMode::kDeviceTransient,
5248
.load_action = LoadAction::kClear,
53-
.store_action = StoreAction::kDontCare,
54-
.clear_color = Color::BlackTransparent()};
49+
.store_action = StoreAction::kDontCare};
5550

5651
static RenderTarget CreateOffscreen(
5752
const Context& context,

0 commit comments

Comments
 (0)