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

Revert "DisplayListBuilder internal reorganization with better rendering op overlap detection" #52725

Merged
merged 1 commit into from
May 10, 2024
Merged

Revert "DisplayListBuilder internal reorganization with better rendering op overlap detection" #52725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39927,7 +39927,6 @@ ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.cc + ../../../flutt
ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/geometry/dl_geometry_types.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/geometry/dl_region.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/geometry/dl_region.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/geometry/dl_rtree.cc + ../../../flutter/LICENSE
Expand All @@ -39945,8 +39944,8 @@ ORIGIN: ../../../flutter/display_list/skia/dl_sk_dispatcher.h + ../../../flutter
ORIGIN: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/skia/dl_sk_types.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/utils/dl_accumulation_rect.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/utils/dl_accumulation_rect.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/utils/dl_bounds_accumulator.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/utils/dl_bounds_accumulator.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/utils/dl_comparable.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -42809,7 +42808,6 @@ FILE: ../../../flutter/display_list/effects/dl_path_effect.cc
FILE: ../../../flutter/display_list/effects/dl_path_effect.h
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.cc
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.h
FILE: ../../../flutter/display_list/geometry/dl_geometry_types.h
FILE: ../../../flutter/display_list/geometry/dl_region.cc
FILE: ../../../flutter/display_list/geometry/dl_region.h
FILE: ../../../flutter/display_list/geometry/dl_rtree.cc
Expand All @@ -42827,8 +42825,8 @@ FILE: ../../../flutter/display_list/skia/dl_sk_dispatcher.h
FILE: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.cc
FILE: ../../../flutter/display_list/skia/dl_sk_paint_dispatcher.h
FILE: ../../../flutter/display_list/skia/dl_sk_types.h
FILE: ../../../flutter/display_list/utils/dl_accumulation_rect.cc
FILE: ../../../flutter/display_list/utils/dl_accumulation_rect.h
FILE: ../../../flutter/display_list/utils/dl_bounds_accumulator.cc
FILE: ../../../flutter/display_list/utils/dl_bounds_accumulator.h
FILE: ../../../flutter/display_list/utils/dl_comparable.h
FILE: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.cc
FILE: ../../../flutter/display_list/utils/dl_matrix_clip_tracker.h
Expand Down
5 changes: 2 additions & 3 deletions display_list/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ source_set("display_list") {
"effects/dl_path_effect.h",
"effects/dl_runtime_effect.cc",
"effects/dl_runtime_effect.h",
"geometry/dl_geometry_types.h",
"geometry/dl_region.cc",
"geometry/dl_region.h",
"geometry/dl_rtree.cc",
Expand All @@ -78,8 +77,8 @@ source_set("display_list") {
"skia/dl_sk_paint_dispatcher.cc",
"skia/dl_sk_paint_dispatcher.h",
"skia/dl_sk_types.h",
"utils/dl_accumulation_rect.cc",
"utils/dl_accumulation_rect.h",
"utils/dl_bounds_accumulator.cc",
"utils/dl_bounds_accumulator.h",
"utils/dl_matrix_clip_tracker.cc",
"utils/dl_matrix_clip_tracker.h",
"utils/dl_receiver_utils.cc",
Expand Down
2 changes: 1 addition & 1 deletion display_list/benchmarking/dl_complexity_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TEST(DisplayListComplexity, StrokeWidth) {
auto display_list_stroke_0 = builder_stroke_0.Build();

DisplayListBuilder builder_stroke_1;
builder_stroke_1.DrawLine(SkPoint::Make(0, 0), SkPoint::Make(100, 100),
builder_stroke_0.DrawLine(SkPoint::Make(0, 0), SkPoint::Make(100, 100),
DlPaint().setStrokeWidth(1.0f));
auto display_list_stroke_1 = builder_stroke_1.Build();

Expand Down
99 changes: 10 additions & 89 deletions display_list/display_list_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,13 +532,13 @@ TEST_F(DisplayListTest, UnclippedSaveLayerContentAccountsForFilter) {
builder.Restore();
auto display_list = builder.Build();

EXPECT_EQ(display_list->op_count(), 6u);
ASSERT_EQ(display_list->op_count(), 6u);
EXPECT_EQ(display_list->total_depth(), 2u);

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

TEST_F(DisplayListTest, ClippedSaveLayerContentAccountsForFilter) {
Expand All @@ -565,72 +565,13 @@ TEST_F(DisplayListTest, ClippedSaveLayerContentAccountsForFilter) {
builder.Restore();
auto display_list = builder.Build();

EXPECT_EQ(display_list->op_count(), 6u);
ASSERT_EQ(display_list->op_count(), 6u);
EXPECT_EQ(display_list->total_depth(), 2u);

SkRect result_rect = draw_rect.makeOutset(30.0f, 30.0f);
ASSERT_TRUE(result_rect.intersect(clip_rect));
ASSERT_EQ(result_rect, SkRect::MakeLTRB(100.0f, 110.0f, 129.0f, 190.0f));
EXPECT_EQ(display_list->bounds(), result_rect);
}

TEST_F(DisplayListTest, OOBSaveLayerContentCulledWithBlurFilter) {
SkRect cull_rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f);
SkRect draw_rect = SkRect::MakeLTRB(25.0f, 25.0f, 99.0f, 75.0f);
auto filter = DlBlurImageFilter::Make(10.0f, 10.0f, DlTileMode::kDecal);
DlPaint layer_paint = DlPaint().setImageFilter(filter);

// We want a draw rect that is outside the layer bounds even though its
// filtered output might be inside. The drawn rect should be culled by
// the expectations of the layer bounds even though it is close enough
// to be visible due to filtering.
ASSERT_FALSE(cull_rect.intersects(draw_rect));
SkRect mapped_rect;
ASSERT_TRUE(filter->map_local_bounds(draw_rect, mapped_rect));
ASSERT_TRUE(mapped_rect.intersects(cull_rect));

DisplayListBuilder builder;
builder.SaveLayer(&cull_rect, &layer_paint);
{ //
builder.DrawRect(draw_rect, DlPaint());
}
builder.Restore();
auto display_list = builder.Build();

EXPECT_EQ(display_list->op_count(), 2u);
EXPECT_EQ(display_list->total_depth(), 1u);

EXPECT_TRUE(display_list->bounds().isEmpty()) << display_list->bounds();
}

TEST_F(DisplayListTest, OOBSaveLayerContentCulledWithMatrixFilter) {
SkRect cull_rect = SkRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f);
SkRect draw_rect = SkRect::MakeLTRB(25.0f, 125.0f, 75.0f, 175.0f);
auto filter = DlMatrixImageFilter::Make(SkMatrix::Translate(100.0f, 0.0f),
DlImageSampling::kLinear);
DlPaint layer_paint = DlPaint().setImageFilter(filter);

// We want a draw rect that is outside the layer bounds even though its
// filtered output might be inside. The drawn rect should be culled by
// the expectations of the layer bounds even though it is close enough
// to be visible due to filtering.
ASSERT_FALSE(cull_rect.intersects(draw_rect));
SkRect mapped_rect;
ASSERT_TRUE(filter->map_local_bounds(draw_rect, mapped_rect));
ASSERT_TRUE(mapped_rect.intersects(cull_rect));

DisplayListBuilder builder;
builder.SaveLayer(&cull_rect, &layer_paint);
{ //
builder.DrawRect(draw_rect, DlPaint());
}
builder.Restore();
auto display_list = builder.Build();

EXPECT_EQ(display_list->op_count(), 2u);
EXPECT_EQ(display_list->total_depth(), 1u);

EXPECT_TRUE(display_list->bounds().isEmpty()) << display_list->bounds();
ASSERT_EQ(display_list->bounds(), result_rect);
}

TEST_F(DisplayListTest, SingleOpSizes) {
Expand Down Expand Up @@ -1203,33 +1144,14 @@ TEST_F(DisplayListTest, SingleOpsMightSupportGroupOpacityBlendMode) {

TEST_F(DisplayListTest, OverlappingOpsDoNotSupportGroupOpacity) {
DisplayListBuilder builder;
DlOpReceiver& receiver = ToReceiver(builder);
for (int i = 0; i < 10; i++) {
builder.DrawRect(SkRect::MakeXYWH(i * 10, 0, 30, 30), DlPaint());
receiver.drawRect(SkRect::MakeXYWH(i * 10, 0, 30, 30));
}
auto display_list = builder.Build();
EXPECT_FALSE(display_list->can_apply_group_opacity());
}

TEST_F(DisplayListTest, LineOfNonOverlappingOpsSupportGroupOpacity) {
DisplayListBuilder builder;
for (int i = 0; i < 10; i++) {
builder.DrawRect(SkRect::MakeXYWH(i * 30, 0, 30, 30), DlPaint());
}
auto display_list = builder.Build();
EXPECT_TRUE(display_list->can_apply_group_opacity());
}

TEST_F(DisplayListTest, CrossOfNonOverlappingOpsSupportGroupOpacity) {
DisplayListBuilder builder;
builder.DrawRect(SkRect::MakeLTRB(200, 200, 300, 300), DlPaint()); // center
builder.DrawRect(SkRect::MakeLTRB(100, 200, 200, 300), DlPaint()); // left
builder.DrawRect(SkRect::MakeLTRB(200, 100, 300, 200), DlPaint()); // above
builder.DrawRect(SkRect::MakeLTRB(300, 200, 400, 300), DlPaint()); // right
builder.DrawRect(SkRect::MakeLTRB(200, 300, 300, 400), DlPaint()); // below
auto display_list = builder.Build();
EXPECT_TRUE(display_list->can_apply_group_opacity());
}

TEST_F(DisplayListTest, SaveLayerFalseSupportsGroupOpacityOverlappingChidren) {
DisplayListBuilder builder;
DlOpReceiver& receiver = ToReceiver(builder);
Expand Down Expand Up @@ -1326,8 +1248,7 @@ class SaveLayerOptionsExpector : public virtual DlOpReceiver,
void saveLayer(const SkRect& bounds,
const SaveLayerOptions options,
const DlImageFilter* backdrop) override {
EXPECT_EQ(options, expected_[save_layer_count_])
<< "index " << save_layer_count_;
EXPECT_EQ(options, expected_[save_layer_count_]);
save_layer_count_++;
}

Expand Down Expand Up @@ -3937,7 +3858,7 @@ TEST_F(DisplayListTest, SaveContentDepthTest) {

builder.Save(); // covers depth 1->9
{
builder.Translate(5, 5); // triggers deferred save at depth 1
builder.Translate(5, 5);
builder.DrawRect({10, 10, 20, 20}, DlPaint()); // depth 2

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

builder.DrawRect({16, 16, 26, 26}, DlPaint()); // depth 8
builder.DrawRect({18, 18, 28, 28}, DlPaint()); // depth 9
}
builder.Restore(); // save is restored with depth 9
builder.Restore();

builder.DrawRect({16, 16, 26, 26}, DlPaint()); // depth 10
builder.DrawRect({18, 18, 28, 28}, DlPaint()); // depth 11
Expand Down
Loading