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

Commit 9464c00

Browse files
author
jonahwilliams
committed
move clip skipping logic into clip stack.
1 parent 1783fc8 commit 9464c00

File tree

8 files changed

+97
-78
lines changed

8 files changed

+97
-78
lines changed

‎impeller/aiks/experimental_canvas.cc‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,8 @@ void ExperimentalCanvas::SaveLayer(
432432
entry.rendering_mode = Entity::RenderingMode::kSubpassAppendSnapshotTransform;
433433
transform_stack_.emplace_back(entry);
434434

435-
// The DisplayList bounds/rtree doesn't account for filters applied to parent
436-
// layers, and so sub-DisplayLists are getting culled as if no filters are
437-
// applied.
438-
// See also: https://github.com/flutter/flutter/issues/139294
435+
// The current clip aiks clip culling can not handle image filters.
436+
// Remove this once we've migrated to exp canvas and removed it.
439437
if (paint.image_filter) {
440438
transform_stack_.back().cull_rect = std::nullopt;
441439
}
@@ -761,6 +759,7 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) {
761759
auto transform = entity.GetTransform();
762760
entity.SetTransform(
763761
Matrix::MakeTranslation(Vector3(-GetGlobalPassPosition())) * transform);
762+
764763
// Ideally the clip depth would be greater than the current rendering
765764
// depth because any rendering calls that follow this clip operation will
766765
// pre-increment the depth and then be rendering above our clip depth,
@@ -783,14 +782,6 @@ void ExperimentalCanvas::AddClipEntityToCurrentPass(Entity entity) {
783782
current_clip_coverage->Shift(-GetGlobalPassPosition());
784783
}
785784

786-
// Skip rendering the clip if it is fully contained by the current render
787-
// target.
788-
if (entity.GetContents()->CanSkip(
789-
render_passes_.back().inline_pass_context->GetTexture()->GetSize(),
790-
entity.GetTransform())) {
791-
return;
792-
}
793-
794785
auto clip_coverage = entity.GetClipCoverage(current_clip_coverage);
795786
if (clip_coverage.coverage.has_value()) {
796787
clip_coverage.coverage =

‎impeller/entity/contents/clip_contents.cc‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -82,20 +82,6 @@ bool ClipContents::CanInheritOpacity(const Entity& entity) const {
8282

8383
void ClipContents::SetInheritedOpacity(Scalar opacity) {}
8484

85-
bool ClipContents::CanSkip(ISize render_pass_size, const Matrix& ctm) const {
86-
if (clip_op_ == Entity::ClipOperation::kIntersect &&
87-
geometry_->IsAxisAlignedRect() && ctm.IsTranslationScaleOnly()) {
88-
std::optional<Rect> coverage = geometry_->GetCoverage(ctm);
89-
if (coverage.has_value() &&
90-
coverage->Contains(Rect::MakeSize(render_pass_size))) {
91-
// Skip axis-aligned intersect clips that cover the whole render target
92-
// since they won't draw anything to the depth buffer.
93-
return true;
94-
}
95-
}
96-
return false;
97-
}
98-
9985
bool ClipContents::Render(const ContentContext& renderer,
10086
const Entity& entity,
10187
RenderPass& pass) const {
@@ -105,10 +91,6 @@ bool ClipContents::Render(const ContentContext& renderer,
10591

10692
using VS = ClipPipeline::VertexShader;
10793

108-
if (CanSkip(pass.GetRenderTargetSize(), entity.GetTransform())) {
109-
return true;
110-
}
111-
11294
VS::FrameInfo info;
11395
info.depth = GetShaderClipDepth(entity);
11496

‎impeller/entity/contents/clip_contents.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ class ClipContents final : public Contents {
2525

2626
void SetClipOperation(Entity::ClipOperation clip_op);
2727

28-
bool CanSkip(ISize render_pass_size, const Matrix& transform) const override;
29-
3028
// |Contents|
3129
std::optional<Rect> GetCoverage(const Entity& entity) const override;
3230

‎impeller/entity/contents/contents.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,4 @@ void Contents::SetColorSourceSize(Size size) {
182182
color_source_size_ = size;
183183
}
184184

185-
bool Contents::CanSkip(ISize render_pass_size, const Matrix& transform) const {
186-
return false;
187-
}
188-
189185
} // namespace impeller

‎impeller/entity/contents/contents.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,6 @@ class Contents {
8383
///
8484
void SetCoverageHint(std::optional<Rect> coverage_hint);
8585

86-
//----------------------------------------------------------------------------
87-
/// @brief Whether this entity can be entirely skipped during rendering.
88-
///
89-
/// TODO(jonahwilliams): remove this method which was only added for clipping
90-
/// once experimental canvas lands.
91-
virtual bool CanSkip(ISize render_pass_size, const Matrix& transform) const;
92-
9386
const std::optional<Rect>& GetCoverageHint() const;
9487

9588
//----------------------------------------------------------------------------

‎impeller/entity/entity_pass_clip_stack.cc‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState(
6060
case Contents::ClipCoverage::Type::kNoChange:
6161
break;
6262
case Contents::ClipCoverage::Type::kAppend: {
63-
auto op = CurrentClipCoverage();
63+
auto maybe_coverage = CurrentClipCoverage();
6464

6565
// Compute the previous clip height.
6666
size_t previous_clip_height = 0;
@@ -72,6 +72,23 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState(
7272
previous_clip_height = clip_height_floor;
7373
}
7474

75+
if (!maybe_coverage.has_value()) {
76+
// Running this append op won't impact the clip buffer because the
77+
// whole screen is already being clipped, so skip it.
78+
return result;
79+
}
80+
auto op = maybe_coverage.value();
81+
82+
// If the new clip coverage is bigger than the existing coverage, we
83+
// do not need to change the clip region.
84+
if (global_clip_coverage.coverage.has_value() &&
85+
global_clip_coverage.coverage.value().Contains(op)) {
86+
subpass_state.clip_coverage.push_back(ClipCoverageLayer{
87+
.coverage = op, .clip_height = previous_clip_height + 1});
88+
89+
return result;
90+
}
91+
7592
subpass_state.clip_coverage.push_back(
7693
ClipCoverageLayer{.coverage = global_clip_coverage.coverage,
7794
.clip_height = previous_clip_height + 1});
@@ -81,11 +98,6 @@ EntityPassClipStack::ClipStateResult EntityPassClipStack::ApplyClipState(
8198
subpass_state.clip_coverage.front().clip_height +
8299
subpass_state.clip_coverage.size() - 1);
83100

84-
if (!op.has_value()) {
85-
// Running this append op won't impact the clip buffer because the
86-
// whole screen is already being clipped, so skip it.
87-
return result;
88-
}
89101
} break;
90102
case Contents::ClipCoverage::Type::kRestore: {
91103
ClipRestoreContents* restore_contents =

‎impeller/entity/entity_pass_unittests.cc‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,82 @@ TEST(EntityPassClipStackTest, AppendAndRestoreClipCoverage) {
130130
EXPECT_EQ(recorder.GetReplayEntities().size(), 0u);
131131
}
132132

133+
// Append two clip coverages, the second is larger the first. This
134+
// should result in the second clip not requiring any update.
135+
TEST(EntityPassClipStackTest, AppendLargerClipCoverage) {
136+
EntityPassClipStack recorder =
137+
EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));
138+
139+
ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
140+
141+
// Push a clip.
142+
Entity entity;
143+
EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
144+
Contents::ClipCoverage{
145+
.type = Contents::ClipCoverage::Type::kAppend,
146+
.coverage = Rect::MakeLTRB(50, 50, 55, 55),
147+
},
148+
entity, 0, Point(0, 0));
149+
EXPECT_TRUE(result.should_render);
150+
EXPECT_TRUE(result.clip_did_change);
151+
152+
// Push a clip with larger coverage than the previous state.
153+
result = recorder.ApplyClipState(
154+
Contents::ClipCoverage{
155+
.type = Contents::ClipCoverage::Type::kAppend,
156+
.coverage = Rect::MakeLTRB(0, 0, 100, 100),
157+
},
158+
entity, 0, Point(0, 0));
159+
160+
EXPECT_FALSE(result.should_render);
161+
EXPECT_FALSE(result.clip_did_change);
162+
}
163+
164+
TEST(EntityPassClipStackTest, AppendDecreasingSizeClipCoverage) {
165+
EntityPassClipStack recorder =
166+
EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));
167+
168+
ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
169+
170+
// Push Clips that shrink in size. All should be applied.
171+
Entity entity;
172+
173+
for (auto i = 1; i < 20; i++) {
174+
EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
175+
Contents::ClipCoverage{
176+
.type = Contents::ClipCoverage::Type::kAppend,
177+
.coverage = Rect::MakeLTRB(i, i, 100 - i, 100 - i),
178+
},
179+
entity, 0, Point(0, 0));
180+
EXPECT_TRUE(result.should_render);
181+
EXPECT_TRUE(result.clip_did_change);
182+
EXPECT_EQ(recorder.CurrentClipCoverage(),
183+
Rect::MakeLTRB(i, i, 100 - i, 100 - i));
184+
}
185+
}
186+
187+
TEST(EntityPassClipStackTest, AppendIncreasingSizeClipCoverage) {
188+
EntityPassClipStack recorder =
189+
EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));
190+
191+
ASSERT_EQ(recorder.GetClipCoverageLayers().size(), 1u);
192+
193+
// Push Clips that grow in size. All should be skipped.
194+
Entity entity;
195+
196+
for (auto i = 1; i < 20; i++) {
197+
EntityPassClipStack::ClipStateResult result = recorder.ApplyClipState(
198+
Contents::ClipCoverage{
199+
.type = Contents::ClipCoverage::Type::kAppend,
200+
.coverage = Rect::MakeLTRB(0 - i, 0 - i, 100 + i, 100 + i),
201+
},
202+
entity, 0, Point(0, 0));
203+
EXPECT_FALSE(result.should_render);
204+
EXPECT_FALSE(result.clip_did_change);
205+
EXPECT_EQ(recorder.CurrentClipCoverage(), Rect::MakeLTRB(0, 0, 100, 100));
206+
}
207+
}
208+
133209
TEST(EntityPassClipStackTest, UnbalancedRestore) {
134210
EntityPassClipStack recorder =
135211
EntityPassClipStack(Rect::MakeLTRB(0, 0, 100, 100));

‎impeller/entity/entity_unittests.cc‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1532,35 +1532,6 @@ TEST_P(EntityTest, ClipContentsShouldRenderIsCorrect) {
15321532
}
15331533
}
15341534

1535-
TEST_P(EntityTest, ClipContentsCanSkip) {
1536-
// Intersect Clips can be skipped if the render target coverage is fully
1537-
// inside of them.
1538-
1539-
auto clip = std::make_shared<ClipContents>();
1540-
clip->SetGeometry(Geometry::MakeRect(Rect::MakeLTRB(0, 0, 100, 100)));
1541-
clip->SetClipOperation(Entity::ClipOperation::kIntersect);
1542-
1543-
// Fully Contained
1544-
EXPECT_TRUE(clip->CanSkip(ISize::MakeWH(50, 50), {}));
1545-
1546-
// Not Intersect
1547-
clip->SetClipOperation(Entity::ClipOperation::kDifference);
1548-
EXPECT_FALSE(clip->CanSkip(ISize::MakeWH(50, 50), {}));
1549-
1550-
// Not Contained
1551-
clip->SetClipOperation(Entity::ClipOperation::kIntersect);
1552-
EXPECT_FALSE(clip->CanSkip(ISize::MakeWH(200, 200), {}));
1553-
1554-
// Not Scale Translate
1555-
clip->SetClipOperation(Entity::ClipOperation::kIntersect);
1556-
EXPECT_FALSE(
1557-
clip->CanSkip(ISize::MakeWH(50, 50), Matrix::MakeRotationX(Radians(2))));
1558-
1559-
// Not Axis Aligned Rectangle.
1560-
clip->SetGeometry(Geometry::MakeOval(Rect::MakeLTRB(0, 0, 50, 50)));
1561-
EXPECT_FALSE(clip->CanSkip(ISize::MakeWH(50, 50), {}));
1562-
}
1563-
15641535
TEST_P(EntityTest, ClipContentsGetClipCoverageIsCorrect) {
15651536
// Intersection: No stencil coverage, no geometry.
15661537
{

0 commit comments

Comments
 (0)