Skip to content

Commit

Permalink
[DisplayList] Optimize ClipRRect and ClipPath to ClipOval when approp…
Browse files Browse the repository at this point in the history
…riate (flutter#54088)

Fixes: flutter/flutter#151850

Re-adding an optimization originally included in flutter#53642 that detects when ClipRRect and ClipPath operations are actually ovals and redirects them to the ClipOval path during recording to save space and reduce the need for dispatchers to do the same detections and optimizations.
  • Loading branch information
flar authored Jul 26, 2024
1 parent 0844c0a commit e28f875
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 22 deletions.
19 changes: 5 additions & 14 deletions display_list/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4863,8 +4863,6 @@ TEST_F(DisplayListTest, ClipPathOvalNonCulling) {
auto non_encompassing_oval = clip.makeOutset(2.071f, 2.071f);
SkPath path;
path.addOval(non_encompassing_oval);
SkRRect rrect;
rrect.setOval(non_encompassing_oval);

DisplayListBuilder cull_builder;
cull_builder.ClipRect(clip, ClipOp::kIntersect, false);
Expand All @@ -4873,9 +4871,8 @@ TEST_F(DisplayListTest, ClipPathOvalNonCulling) {

CLIP_EXPECTOR(expector);
expector.addExpectation(clip, ClipOp::kIntersect, false);
// Builder will not cull this clip, but it will turn it into a ClipRRect
// Eventually it should turn it into a ClipOval
expector.addExpectation(rrect, ClipOp::kIntersect, false);
// Builder will not cull this clip, but it will turn it into a ClipOval
expector.addOvalExpectation(non_encompassing_oval, ClipOp::kIntersect, false);
cull_dl->Dispatch(expector);
}

Expand Down Expand Up @@ -5088,9 +5085,7 @@ TEST_F(DisplayListTest, ClipOvalRRectPromoteToClipOval) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();

// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}

TEST_F(DisplayListTest, ClipRectPathPromoteToClipRect) {
Expand Down Expand Up @@ -5132,9 +5127,7 @@ TEST_F(DisplayListTest, ClipOvalPathPromoteToClipOval) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();

// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}

TEST_F(DisplayListTest, ClipRRectPathPromoteToClipRRect) {
Expand Down Expand Up @@ -5273,9 +5266,7 @@ TEST_F(DisplayListTest, ClipOvalRRectPathPromoteToClipOval) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();

// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}

TEST_F(DisplayListTest, BoundedRenderOpsDoNotReportUnbounded) {
Expand Down
15 changes: 9 additions & 6 deletions display_list/dl_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,11 @@ void DisplayListBuilder::ClipRRect(const SkRRect& rrect,
ClipOp clip_op,
bool is_aa) {
if (rrect.isRect()) {
clipRect(rrect.rect(), clip_op, is_aa);
ClipRect(rrect.rect(), clip_op, is_aa);
return;
}
if (rrect.isOval()) {
ClipOval(rrect.rect(), clip_op, is_aa);
return;
}
if (current_info().is_nop) {
Expand Down Expand Up @@ -1043,17 +1047,16 @@ void DisplayListBuilder::ClipPath(const SkPath& path,
if (!path.isInverseFillType()) {
SkRect rect;
if (path.isRect(&rect)) {
this->clipRect(rect, clip_op, is_aa);
ClipRect(rect, clip_op, is_aa);
return;
}
SkRRect rrect;
if (path.isOval(&rect)) {
rrect.setOval(rect);
this->clipRRect(rrect, clip_op, is_aa);
ClipOval(rect, clip_op, is_aa);
return;
}
SkRRect rrect;
if (path.isRRect(&rrect)) {
this->clipRRect(rrect, clip_op, is_aa);
ClipRRect(rrect, clip_op, is_aa);
return;
}
}
Expand Down
4 changes: 2 additions & 2 deletions display_list/testing/dl_test_snippets.cc
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,8 @@ std::vector<DisplayListInvocationGroup> CreateAllClipOps() {
[](DlOpReceiver& r) {
r.clipPath(kTestPathRect, DlCanvas::ClipOp::kIntersect, true);
}},
// clipPath(oval) becomes clipRRect
{1, 64, 0,
// clipPath(oval) becomes clipOval
{1, 24, 0,
[](DlOpReceiver& r) {
r.clipPath(kTestPathOval, DlCanvas::ClipOp::kIntersect, true);
}},
Expand Down

0 comments on commit e28f875

Please sign in to comment.