Skip to content

Commit 0579b6f

Browse files
committed
Reapply "DisplayListBuilder internal reorganization with better rendering op overlap detection" (flutter#52725)
This reverts commit f312281.
1 parent 96c01fa commit 0579b6f

16 files changed

+1074
-909
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41973,6 +41973,7 @@ ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.cc + ../../../flutt
4197341973
ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.h + ../../../flutter/LICENSE
4197441974
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.cc + ../../../flutter/LICENSE
4197541975
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.h + ../../../flutter/LICENSE
41976+
ORIGIN: ../../../flutter/display_list/geometry/dl_geometry_types.h + ../../../flutter/LICENSE
4197641977
ORIGIN: ../../../flutter/display_list/geometry/dl_region.cc + ../../../flutter/LICENSE
4197741978
ORIGIN: ../../../flutter/display_list/geometry/dl_region.h + ../../../flutter/LICENSE
4197841979
ORIGIN: ../../../flutter/display_list/geometry/dl_rtree.cc + ../../../flutter/LICENSE
@@ -41990,8 +41991,8 @@ ORIGIN: ../../../flutter/display_list/skia/dl_sk_dispatcher.h + ../../../flutter
4199041991
ORIGIN: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.cc + ../../../flutter/LICENSE
4199141992
ORIGIN: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.h + ../../../flutter/LICENSE
4199241993
ORIGIN: ../../../flutter/display_list/skia/dl_sk_types.h + ../../../flutter/LICENSE
41993-
ORIGIN: ../../../flutter/display_list/utils/dl_bounds_accumulator.cc + ../../../flutter/LICENSE
41994-
ORIGIN: ../../../flutter/display_list/utils/dl_bounds_accumulator.h + ../../../flutter/LICENSE
41994+
ORIGIN: ../../../flutter/display_list/utils/dl_accumulation_rect.cc + ../../../flutter/LICENSE
41995+
ORIGIN: ../../../flutter/display_list/utils/dl_accumulation_rect.h + ../../../flutter/LICENSE
4199541996
ORIGIN: ../../../flutter/display_list/utils/dl_comparable.h + ../../../flutter/LICENSE
4199641997
ORIGIN: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.cc + ../../../flutter/LICENSE
4199741998
ORIGIN: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.h + ../../../flutter/LICENSE
@@ -44840,6 +44841,7 @@ FILE: ../../../flutter/display_list/effects/dl_path_effect.cc
4484044841
FILE: ../../../flutter/display_list/effects/dl_path_effect.h
4484144842
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.cc
4484244843
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.h
44844+
FILE: ../../../flutter/display_list/geometry/dl_geometry_types.h
4484344845
FILE: ../../../flutter/display_list/geometry/dl_region.cc
4484444846
FILE: ../../../flutter/display_list/geometry/dl_region.h
4484544847
FILE: ../../../flutter/display_list/geometry/dl_rtree.cc
@@ -44857,8 +44859,8 @@ FILE: ../../../flutter/display_list/skia/dl_sk_dispatcher.h
4485744859
FILE: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.cc
4485844860
FILE: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.h
4485944861
FILE: ../../../flutter/display_list/skia/dl_sk_types.h
44860-
FILE: ../../../flutter/display_list/utils/dl_bounds_accumulator.cc
44861-
FILE: ../../../flutter/display_list/utils/dl_bounds_accumulator.h
44862+
FILE: ../../../flutter/display_list/utils/dl_accumulation_rect.cc
44863+
FILE: ../../../flutter/display_list/utils/dl_accumulation_rect.h
4486244864
FILE: ../../../flutter/display_list/utils/dl_comparable.h
4486344865
FILE: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.cc
4486444866
FILE: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.h

display_list/BUILD.gn

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ source_set("display_list") {
6060
"effects/dl_path_effect.h",
6161
"effects/dl_runtime_effect.cc",
6262
"effects/dl_runtime_effect.h",
63+
"geometry/dl_geometry_types.h",
6364
"geometry/dl_region.cc",
6465
"geometry/dl_region.h",
6566
"geometry/dl_rtree.cc",
@@ -77,8 +78,8 @@ source_set("display_list") {
7778
"skia/dl_sk_paint_dispatcher.cc",
7879
"skia/dl_sk_paint_dispatcher.h",
7980
"skia/dl_sk_types.h",
80-
"utils/dl_bounds_accumulator.cc",
81-
"utils/dl_bounds_accumulator.h",
81+
"utils/dl_accumulation_rect.cc",
82+
"utils/dl_accumulation_rect.h",
8283
"utils/dl_matrix_clip_tracker.cc",
8384
"utils/dl_matrix_clip_tracker.h",
8485
"utils/dl_receiver_utils.cc",

display_list/benchmarking/dl_complexity_unittests.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ TEST(DisplayListComplexity, StrokeWidth) {
102102
auto display_list_stroke_0 = builder_stroke_0.Build();
103103

104104
DisplayListBuilder builder_stroke_1;
105-
builder_stroke_0.DrawLine(SkPoint::Make(0, 0), SkPoint::Make(100, 100),
105+
builder_stroke_1.DrawLine(SkPoint::Make(0, 0), SkPoint::Make(100, 100),
106106
DlPaint().setStrokeWidth(1.0f));
107107
auto display_list_stroke_1 = builder_stroke_1.Build();
108108

display_list/display_list_unittests.cc

Lines changed: 89 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -532,13 +532,13 @@ TEST_F(DisplayListTest, UnclippedSaveLayerContentAccountsForFilter) {
532532
builder.Restore();
533533
auto display_list = builder.Build();
534534

535-
ASSERT_EQ(display_list->op_count(), 6u);
535+
EXPECT_EQ(display_list->op_count(), 6u);
536536
EXPECT_EQ(display_list->total_depth(), 2u);
537537

538538
SkRect result_rect = draw_rect.makeOutset(30.0f, 30.0f);
539539
ASSERT_TRUE(result_rect.intersect(clip_rect));
540540
ASSERT_EQ(result_rect, SkRect::MakeLTRB(100.0f, 110.0f, 131.0f, 190.0f));
541-
ASSERT_EQ(display_list->bounds(), result_rect);
541+
EXPECT_EQ(display_list->bounds(), result_rect);
542542
}
543543

544544
TEST_F(DisplayListTest, ClippedSaveLayerContentAccountsForFilter) {
@@ -565,13 +565,72 @@ TEST_F(DisplayListTest, ClippedSaveLayerContentAccountsForFilter) {
565565
builder.Restore();
566566
auto display_list = builder.Build();
567567

568-
ASSERT_EQ(display_list->op_count(), 6u);
568+
EXPECT_EQ(display_list->op_count(), 6u);
569569
EXPECT_EQ(display_list->total_depth(), 2u);
570570

571571
SkRect result_rect = draw_rect.makeOutset(30.0f, 30.0f);
572572
ASSERT_TRUE(result_rect.intersect(clip_rect));
573573
ASSERT_EQ(result_rect, SkRect::MakeLTRB(100.0f, 110.0f, 129.0f, 190.0f));
574-
ASSERT_EQ(display_list->bounds(), result_rect);
574+
EXPECT_EQ(display_list->bounds(), result_rect);
575+
}
576+
577+
TEST_F(DisplayListTest, OOBSaveLayerContentCulledWithBlurFilter) {
578+
SkRect cull_rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f);
579+
SkRect draw_rect = SkRect::MakeLTRB(25.0f, 25.0f, 99.0f, 75.0f);
580+
auto filter = DlBlurImageFilter::Make(10.0f, 10.0f, DlTileMode::kDecal);
581+
DlPaint layer_paint = DlPaint().setImageFilter(filter);
582+
583+
// We want a draw rect that is outside the layer bounds even though its
584+
// filtered output might be inside. The drawn rect should be culled by
585+
// the expectations of the layer bounds even though it is close enough
586+
// to be visible due to filtering.
587+
ASSERT_FALSE(cull_rect.intersects(draw_rect));
588+
SkRect mapped_rect;
589+
ASSERT_TRUE(filter->map_local_bounds(draw_rect, mapped_rect));
590+
ASSERT_TRUE(mapped_rect.intersects(cull_rect));
591+
592+
DisplayListBuilder builder;
593+
builder.SaveLayer(&cull_rect, &layer_paint);
594+
{ //
595+
builder.DrawRect(draw_rect, DlPaint());
596+
}
597+
builder.Restore();
598+
auto display_list = builder.Build();
599+
600+
EXPECT_EQ(display_list->op_count(), 2u);
601+
EXPECT_EQ(display_list->total_depth(), 1u);
602+
603+
EXPECT_TRUE(display_list->bounds().isEmpty()) << display_list->bounds();
604+
}
605+
606+
TEST_F(DisplayListTest, OOBSaveLayerContentCulledWithMatrixFilter) {
607+
SkRect cull_rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f);
608+
SkRect draw_rect = SkRect::MakeLTRB(25.0f, 125.0f, 75.0f, 175.0f);
609+
auto filter = DlMatrixImageFilter::Make(SkMatrix::Translate(100.0f, 0.0f),
610+
DlImageSampling::kLinear);
611+
DlPaint layer_paint = DlPaint().setImageFilter(filter);
612+
613+
// We want a draw rect that is outside the layer bounds even though its
614+
// filtered output might be inside. The drawn rect should be culled by
615+
// the expectations of the layer bounds even though it is close enough
616+
// to be visible due to filtering.
617+
ASSERT_FALSE(cull_rect.intersects(draw_rect));
618+
SkRect mapped_rect;
619+
ASSERT_TRUE(filter->map_local_bounds(draw_rect, mapped_rect));
620+
ASSERT_TRUE(mapped_rect.intersects(cull_rect));
621+
622+
DisplayListBuilder builder;
623+
builder.SaveLayer(&cull_rect, &layer_paint);
624+
{ //
625+
builder.DrawRect(draw_rect, DlPaint());
626+
}
627+
builder.Restore();
628+
auto display_list = builder.Build();
629+
630+
EXPECT_EQ(display_list->op_count(), 2u);
631+
EXPECT_EQ(display_list->total_depth(), 1u);
632+
633+
EXPECT_TRUE(display_list->bounds().isEmpty()) << display_list->bounds();
575634
}
576635

577636
TEST_F(DisplayListTest, SingleOpSizes) {
@@ -1144,14 +1203,33 @@ TEST_F(DisplayListTest, SingleOpsMightSupportGroupOpacityBlendMode) {
11441203

11451204
TEST_F(DisplayListTest, OverlappingOpsDoNotSupportGroupOpacity) {
11461205
DisplayListBuilder builder;
1147-
DlOpReceiver& receiver = ToReceiver(builder);
11481206
for (int i = 0; i < 10; i++) {
1149-
receiver.drawRect(SkRect::MakeXYWH(i * 10, 0, 30, 30));
1207+
builder.DrawRect(SkRect::MakeXYWH(i * 10, 0, 30, 30), DlPaint());
11501208
}
11511209
auto display_list = builder.Build();
11521210
EXPECT_FALSE(display_list->can_apply_group_opacity());
11531211
}
11541212

1213+
TEST_F(DisplayListTest, LineOfNonOverlappingOpsSupportGroupOpacity) {
1214+
DisplayListBuilder builder;
1215+
for (int i = 0; i < 10; i++) {
1216+
builder.DrawRect(SkRect::MakeXYWH(i * 30, 0, 30, 30), DlPaint());
1217+
}
1218+
auto display_list = builder.Build();
1219+
EXPECT_TRUE(display_list->can_apply_group_opacity());
1220+
}
1221+
1222+
TEST_F(DisplayListTest, CrossOfNonOverlappingOpsSupportGroupOpacity) {
1223+
DisplayListBuilder builder;
1224+
builder.DrawRect(SkRect::MakeLTRB(200, 200, 300, 300), DlPaint()); // center
1225+
builder.DrawRect(SkRect::MakeLTRB(100, 200, 200, 300), DlPaint()); // left
1226+
builder.DrawRect(SkRect::MakeLTRB(200, 100, 300, 200), DlPaint()); // above
1227+
builder.DrawRect(SkRect::MakeLTRB(300, 200, 400, 300), DlPaint()); // right
1228+
builder.DrawRect(SkRect::MakeLTRB(200, 300, 300, 400), DlPaint()); // below
1229+
auto display_list = builder.Build();
1230+
EXPECT_TRUE(display_list->can_apply_group_opacity());
1231+
}
1232+
11551233
TEST_F(DisplayListTest, SaveLayerFalseSupportsGroupOpacityOverlappingChidren) {
11561234
DisplayListBuilder builder;
11571235
DlOpReceiver& receiver = ToReceiver(builder);
@@ -1248,7 +1326,8 @@ class SaveLayerOptionsExpector : public virtual DlOpReceiver,
12481326
void saveLayer(const SkRect& bounds,
12491327
const SaveLayerOptions options,
12501328
const DlImageFilter* backdrop) override {
1251-
EXPECT_EQ(options, expected_[save_layer_count_]);
1329+
EXPECT_EQ(options, expected_[save_layer_count_])
1330+
<< "index " << save_layer_count_;
12521331
save_layer_count_++;
12531332
}
12541333

@@ -3858,7 +3937,7 @@ TEST_F(DisplayListTest, SaveContentDepthTest) {
38583937

38593938
builder.Save(); // covers depth 1->9
38603939
{
3861-
builder.Translate(5, 5);
3940+
builder.Translate(5, 5); // triggers deferred save at depth 1
38623941
builder.DrawRect({10, 10, 20, 20}, DlPaint()); // depth 2
38633942

38643943
builder.DrawDisplayList(child, 1.0f); // depth 3 (content) + 4 (self)
@@ -3868,12 +3947,12 @@ TEST_F(DisplayListTest, SaveContentDepthTest) {
38683947
builder.DrawRect({12, 12, 22, 22}, DlPaint()); // depth 5
38693948
builder.DrawRect({14, 14, 24, 24}, DlPaint()); // depth 6
38703949
}
3871-
builder.Restore(); // layer is restored with depth 7
3950+
builder.Restore(); // layer is restored with depth 6
38723951

38733952
builder.DrawRect({16, 16, 26, 26}, DlPaint()); // depth 8
38743953
builder.DrawRect({18, 18, 28, 28}, DlPaint()); // depth 9
38753954
}
3876-
builder.Restore();
3955+
builder.Restore(); // save is restored with depth 9
38773956

38783957
builder.DrawRect({16, 16, 26, 26}, DlPaint()); // depth 10
38793958
builder.DrawRect({18, 18, 28, 28}, DlPaint()); // depth 11

0 commit comments

Comments
 (0)