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

Commit f312281

Browse files
authored
Revert "DisplayListBuilder internal reorganization with better rendering op overlap detection" (#52725)
Reverts #52646 Some of the golden changes in G3 as a result of that PR look like bugs in the bounds calculations...
1 parent cab5a2a commit f312281

16 files changed

+909
-1074
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39927,7 +39927,6 @@ 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
3993139930
ORIGIN: ../../../flutter/display_list/geometry/dl_region.cc + ../../../flutter/LICENSE
3993239931
ORIGIN: ../../../flutter/display_list/geometry/dl_region.h + ../../../flutter/LICENSE
3993339932
ORIGIN: ../../../flutter/display_list/geometry/dl_rtree.cc + ../../../flutter/LICENSE
@@ -39945,8 +39944,8 @@ ORIGIN: ../../../flutter/display_list/skia/dl_sk_dispatcher.h + ../../../flutter
3994539944
ORIGIN: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.cc + ../../../flutter/LICENSE
3994639945
ORIGIN: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.h + ../../../flutter/LICENSE
3994739946
ORIGIN: ../../../flutter/display_list/skia/dl_sk_types.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
39947+
ORIGIN: ../../../flutter/display_list/utils/dl_bounds_accumulator.cc + ../../../flutter/LICENSE
39948+
ORIGIN: ../../../flutter/display_list/utils/dl_bounds_accumulator.h + ../../../flutter/LICENSE
3995039949
ORIGIN: ../../../flutter/display_list/utils/dl_comparable.h + ../../../flutter/LICENSE
3995139950
ORIGIN: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.cc + ../../../flutter/LICENSE
3995239951
ORIGIN: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.h + ../../../flutter/LICENSE
@@ -42803,7 +42802,6 @@ FILE: ../../../flutter/display_list/effects/dl_path_effect.cc
4280342802
FILE: ../../../flutter/display_list/effects/dl_path_effect.h
4280442803
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.cc
4280542804
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.h
42806-
FILE: ../../../flutter/display_list/geometry/dl_geometry_types.h
4280742805
FILE: ../../../flutter/display_list/geometry/dl_region.cc
4280842806
FILE: ../../../flutter/display_list/geometry/dl_region.h
4280942807
FILE: ../../../flutter/display_list/geometry/dl_rtree.cc
@@ -42821,8 +42819,8 @@ FILE: ../../../flutter/display_list/skia/dl_sk_dispatcher.h
4282142819
FILE: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.cc
4282242820
FILE: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.h
4282342821
FILE: ../../../flutter/display_list/skia/dl_sk_types.h
42824-
FILE: ../../../flutter/display_list/utils/dl_accumulation_rect.cc
42825-
FILE: ../../../flutter/display_list/utils/dl_accumulation_rect.h
42822+
FILE: ../../../flutter/display_list/utils/dl_bounds_accumulator.cc
42823+
FILE: ../../../flutter/display_list/utils/dl_bounds_accumulator.h
4282642824
FILE: ../../../flutter/display_list/utils/dl_comparable.h
4282742825
FILE: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.cc
4282842826
FILE: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.h

display_list/BUILD.gn

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ 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",
6463
"geometry/dl_region.cc",
6564
"geometry/dl_region.h",
6665
"geometry/dl_rtree.cc",
@@ -78,8 +77,8 @@ source_set("display_list") {
7877
"skia/dl_sk_paint_dispatcher.cc",
7978
"skia/dl_sk_paint_dispatcher.h",
8079
"skia/dl_sk_types.h",
81-
"utils/dl_accumulation_rect.cc",
82-
"utils/dl_accumulation_rect.h",
80+
"utils/dl_bounds_accumulator.cc",
81+
"utils/dl_bounds_accumulator.h",
8382
"utils/dl_matrix_clip_tracker.cc",
8483
"utils/dl_matrix_clip_tracker.h",
8584
"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_1.DrawLine(SkPoint::Make(0, 0), SkPoint::Make(100, 100),
105+
builder_stroke_0.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: 10 additions & 89 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-
EXPECT_EQ(display_list->op_count(), 6u);
535+
ASSERT_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-
EXPECT_EQ(display_list->bounds(), result_rect);
541+
ASSERT_EQ(display_list->bounds(), result_rect);
542542
}
543543

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

568-
EXPECT_EQ(display_list->op_count(), 6u);
568+
ASSERT_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-
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();
574+
ASSERT_EQ(display_list->bounds(), result_rect);
634575
}
635576

636577
TEST_F(DisplayListTest, SingleOpSizes) {
@@ -1203,33 +1144,14 @@ TEST_F(DisplayListTest, SingleOpsMightSupportGroupOpacityBlendMode) {
12031144

12041145
TEST_F(DisplayListTest, OverlappingOpsDoNotSupportGroupOpacity) {
12051146
DisplayListBuilder builder;
1147+
DlOpReceiver& receiver = ToReceiver(builder);
12061148
for (int i = 0; i < 10; i++) {
1207-
builder.DrawRect(SkRect::MakeXYWH(i * 10, 0, 30, 30), DlPaint());
1149+
receiver.drawRect(SkRect::MakeXYWH(i * 10, 0, 30, 30));
12081150
}
12091151
auto display_list = builder.Build();
12101152
EXPECT_FALSE(display_list->can_apply_group_opacity());
12111153
}
12121154

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-
12331155
TEST_F(DisplayListTest, SaveLayerFalseSupportsGroupOpacityOverlappingChidren) {
12341156
DisplayListBuilder builder;
12351157
DlOpReceiver& receiver = ToReceiver(builder);
@@ -1326,8 +1248,7 @@ class SaveLayerOptionsExpector : public virtual DlOpReceiver,
13261248
void saveLayer(const SkRect& bounds,
13271249
const SaveLayerOptions options,
13281250
const DlImageFilter* backdrop) override {
1329-
EXPECT_EQ(options, expected_[save_layer_count_])
1330-
<< "index " << save_layer_count_;
1251+
EXPECT_EQ(options, expected_[save_layer_count_]);
13311252
save_layer_count_++;
13321253
}
13331254

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

39383859
builder.Save(); // covers depth 1->9
39393860
{
3940-
builder.Translate(5, 5); // triggers deferred save at depth 1
3861+
builder.Translate(5, 5);
39413862
builder.DrawRect({10, 10, 20, 20}, DlPaint()); // depth 2
39423863

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

39523873
builder.DrawRect({16, 16, 26, 26}, DlPaint()); // depth 8
39533874
builder.DrawRect({18, 18, 28, 28}, DlPaint()); // depth 9
39543875
}
3955-
builder.Restore(); // save is restored with depth 9
3876+
builder.Restore();
39563877

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

0 commit comments

Comments
 (0)