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

Commit 4bac4a6

Browse files
authored
DisplayListBuilder internal reorganization with better rendering op overlap detection (#52646)
This commit makes some long-needed internal improvements to the way that the DisplayListBuilder manages its per-save/saveLayer data. The information for layer bounds and matrix/clips is now maintained in the layer info structure itself rather than shared across a number of stack structures which required careful alignment of the 3 different stacks and made it more difficult to compare and update adjacent layers during save and restore operations. The new code stores all information for a layer within a single structure and the save and restore operations can be more clear about which information they are getting or setting in the current and parent layers. In addition, the layer bounds accumulations were updated to have a more flexible algorithm for detecting overlap of rendering operations for the opacity peephole optimization. Previously, more than one rendering op on a layer would prevent opacity peephole optimizations, but now the condition will be recognized until the first rendering op that overlaps the bounds of the previous rendering operations. This will help for some potentially common cases of 2 non-overlapping ops or even a list of rendering operations laid out in a row.
1 parent 2aa6980 commit 4bac4a6

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
@@ -39927,6 +39927,7 @@ ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.cc + ../../../flutt
3992739927
ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.h + ../../../flutter/LICENSE
3992839928
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.cc + ../../../flutter/LICENSE
3992939929
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.h + ../../../flutter/LICENSE
39930+
ORIGIN: ../../../flutter/display_list/geometry/dl_geometry_types.h + ../../../flutter/LICENSE
3993039931
ORIGIN: ../../../flutter/display_list/geometry/dl_region.cc + ../../../flutter/LICENSE
3993139932
ORIGIN: ../../../flutter/display_list/geometry/dl_region.h + ../../../flutter/LICENSE
3993239933
ORIGIN: ../../../flutter/display_list/geometry/dl_rtree.cc + ../../../flutter/LICENSE
@@ -39944,8 +39945,8 @@ ORIGIN: ../../../flutter/display_list/skia/dl_sk_dispatcher.h + ../../../flutter
3994439945
ORIGIN: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.cc + ../../../flutter/LICENSE
3994539946
ORIGIN: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.h + ../../../flutter/LICENSE
3994639947
ORIGIN: ../../../flutter/display_list/skia/dl_sk_types.h + ../../../flutter/LICENSE
39947-
ORIGIN: ../../../flutter/display_list/utils/dl_bounds_accumulator.cc + ../../../flutter/LICENSE
39948-
ORIGIN: ../../../flutter/display_list/utils/dl_bounds_accumulator.h + ../../../flutter/LICENSE
39948+
ORIGIN: ../../../flutter/display_list/utils/dl_accumulation_rect.cc + ../../../flutter/LICENSE
39949+
ORIGIN: ../../../flutter/display_list/utils/dl_accumulation_rect.h + ../../../flutter/LICENSE
3994939950
ORIGIN: ../../../flutter/display_list/utils/dl_comparable.h + ../../../flutter/LICENSE
3995039951
ORIGIN: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.cc + ../../../flutter/LICENSE
3995139952
ORIGIN: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.h + ../../../flutter/LICENSE
@@ -42804,6 +42805,7 @@ FILE: ../../../flutter/display_list/effects/dl_path_effect.cc
4280442805
FILE: ../../../flutter/display_list/effects/dl_path_effect.h
4280542806
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.cc
4280642807
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.h
42808+
FILE: ../../../flutter/display_list/geometry/dl_geometry_types.h
4280742809
FILE: ../../../flutter/display_list/geometry/dl_region.cc
4280842810
FILE: ../../../flutter/display_list/geometry/dl_region.h
4280942811
FILE: ../../../flutter/display_list/geometry/dl_rtree.cc
@@ -42821,8 +42823,8 @@ FILE: ../../../flutter/display_list/skia/dl_sk_dispatcher.h
4282142823
FILE: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.cc
4282242824
FILE: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.h
4282342825
FILE: ../../../flutter/display_list/skia/dl_sk_types.h
42824-
FILE: ../../../flutter/display_list/utils/dl_bounds_accumulator.cc
42825-
FILE: ../../../flutter/display_list/utils/dl_bounds_accumulator.h
42826+
FILE: ../../../flutter/display_list/utils/dl_accumulation_rect.cc
42827+
FILE: ../../../flutter/display_list/utils/dl_accumulation_rect.h
4282642828
FILE: ../../../flutter/display_list/utils/dl_comparable.h
4282742829
FILE: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.cc
4282842830
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)