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

DisplayList tracks maximum render op depths #52070

Merged
merged 6 commits into from
Apr 17, 2024
Merged

DisplayList tracks maximum render op depths #52070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bbfef12
DisplayList tracks maximum render op depths
flar Apr 11, 2024
b39431a
type cleanup and depth verifications in unit tests
flar Apr 12, 2024
3d099df
add content depth specific unit test
flar Apr 12, 2024
2d4c8ef
adjust depth accounting for saveLayer calls
flar Apr 16, 2024
03de2d3
drawDisplayList now includes an implicit depth value for itself
flar Apr 17, 2024
6d557ab
switch to relative depth totals for save/saveLayer
flar Apr 17, 2024
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
7 changes: 5 additions & 2 deletions display_list/display_list.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ DisplayList::DisplayList()
op_count_(0),
nested_byte_count_(0),
nested_op_count_(0),
total_depth_(0),
unique_id_(0),
bounds_({0, 0, 0, 0}),
can_apply_group_opacity_(true),
Expand All @@ -27,9 +28,10 @@ DisplayList::DisplayList()

DisplayList::DisplayList(DisplayListStorage&& storage,
size_t byte_count,
unsigned int op_count,
uint32_t op_count,
size_t nested_byte_count,
unsigned int nested_op_count,
uint32_t nested_op_count,
uint32_t total_depth,
const SkRect& bounds,
bool can_apply_group_opacity,
bool is_ui_thread_safe,
Expand All @@ -40,6 +42,7 @@ DisplayList::DisplayList(DisplayListStorage&& storage,
op_count_(op_count),
nested_byte_count_(nested_byte_count),
nested_op_count_(nested_op_count),
total_depth_(total_depth),
unique_id_(next_unique_id()),
bounds_(bounds),
can_apply_group_opacity_(can_apply_group_opacity),
Expand Down
15 changes: 10 additions & 5 deletions display_list/display_list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,12 @@ class DisplayList : public SkRefCnt {
(nested ? nested_byte_count_ : 0);
}

unsigned int op_count(bool nested = false) const {
uint32_t op_count(bool nested = false) const {
return op_count_ + (nested ? nested_op_count_ : 0);
}

uint32_t total_depth() const { return total_depth_; }

uint32_t unique_id() const { return unique_id_; }

const SkRect& bounds() const { return bounds_; }
Expand Down Expand Up @@ -310,9 +312,10 @@ class DisplayList : public SkRefCnt {
private:
DisplayList(DisplayListStorage&& ptr,
size_t byte_count,
unsigned int op_count,
uint32_t op_count,
size_t nested_byte_count,
unsigned int nested_op_count,
uint32_t nested_op_count,
uint32_t total_depth,
const SkRect& bounds,
bool can_apply_group_opacity,
bool is_ui_thread_safe,
Expand All @@ -325,10 +328,12 @@ class DisplayList : public SkRefCnt {

const DisplayListStorage storage_;
const size_t byte_count_;
const unsigned int op_count_;
const uint32_t op_count_;

const size_t nested_byte_count_;
const unsigned int nested_op_count_;
const uint32_t nested_op_count_;

const uint32_t total_depth_;

const uint32_t unique_id_;
const SkRect bounds_;
Expand Down
123 changes: 112 additions & 11 deletions display_list/display_list_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ class DisplayListTestBase : public BaseT {
DisplayListBuilder builder;
DlOpReceiver& receiver =
DisplayListTestBase<::testing::Test>::ToReceiver(builder);
unsigned int op_count = 0;
uint32_t op_count = 0;
size_t byte_count = 0;
uint32_t depth = 0;
for (size_t i = 0; i < allGroups.size(); i++) {
DisplayListInvocationGroup& group = allGroups[i];
size_t j = (i == g_index ? v_index : 0);
Expand All @@ -76,6 +77,7 @@ class DisplayListTestBase : public BaseT {
DisplayListInvocation& invocation = group.variants[j];
op_count += invocation.op_count();
byte_count += invocation.raw_byte_count();
depth += invocation.depth();
invocation.invoker(receiver);
}
sk_sp<DisplayList> dl = builder.Build();
Expand All @@ -92,6 +94,7 @@ class DisplayListTestBase : public BaseT {
}
EXPECT_EQ(dl->op_count(false), op_count) << name;
EXPECT_EQ(dl->bytes(false), byte_count + sizeof(DisplayList)) << name;
EXPECT_EQ(dl->total_depth(), depth) << name;
return dl;
}

Expand Down Expand Up @@ -231,6 +234,7 @@ TEST_F(DisplayListTest, EmptyBuild) {
auto dl = builder.Build();
EXPECT_EQ(dl->op_count(), 0u);
EXPECT_EQ(dl->bytes(), sizeof(DisplayList));
EXPECT_EQ(dl->total_depth(), 0u);
}

TEST_F(DisplayListTest, EmptyRebuild) {
Expand Down Expand Up @@ -526,6 +530,7 @@ TEST_F(DisplayListTest, UnclippedSaveLayerContentAccountsForFilter) {
auto display_list = builder.Build();

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));
Expand Down Expand Up @@ -558,6 +563,7 @@ TEST_F(DisplayListTest, ClippedSaveLayerContentAccountsForFilter) {
auto display_list = builder.Build();

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));
Expand All @@ -573,6 +579,7 @@ TEST_F(DisplayListTest, SingleOpSizes) {
auto desc = group.op_name + "(variant " + std::to_string(i + 1) + ")";
ASSERT_EQ(dl->op_count(false), invocation.op_count()) << desc;
ASSERT_EQ(dl->bytes(false), invocation.byte_count()) << desc;
EXPECT_EQ(dl->total_depth(), invocation.depth()) << desc;
}
}
}
Expand Down Expand Up @@ -611,6 +618,7 @@ TEST_F(DisplayListTest, SingleOpDisplayListsRecapturedAreEqual) {
ASSERT_EQ(copy->bytes(false), dl->bytes(false)) << desc;
ASSERT_EQ(copy->op_count(true), dl->op_count(true)) << desc;
ASSERT_EQ(copy->bytes(true), dl->bytes(true)) << desc;
EXPECT_EQ(copy->total_depth(), dl->total_depth()) << desc;
ASSERT_EQ(copy->bounds(), dl->bounds()) << desc;
ASSERT_TRUE(copy->Equals(*dl)) << desc;
ASSERT_TRUE(dl->Equals(*copy)) << desc;
Expand Down Expand Up @@ -640,6 +648,7 @@ TEST_F(DisplayListTest, SingleOpDisplayListsCompareToEachOther) {
ASSERT_EQ(listA->bytes(false), listB->bytes(false)) << desc;
ASSERT_EQ(listA->op_count(true), listB->op_count(true)) << desc;
ASSERT_EQ(listA->bytes(true), listB->bytes(true)) << desc;
EXPECT_EQ(listA->total_depth(), listB->total_depth()) << desc;
ASSERT_EQ(listA->bounds(), listB->bounds()) << desc;
ASSERT_TRUE(listA->Equals(*listB)) << desc;
ASSERT_TRUE(listB->Equals(*listA)) << desc;
Expand Down Expand Up @@ -669,7 +678,9 @@ TEST_F(DisplayListTest, SingleOpDisplayListsAreEqualWithOrWithoutRtree) {
ASSERT_EQ(dl1->bytes(false), dl2->bytes(false)) << desc;
ASSERT_EQ(dl1->op_count(true), dl2->op_count(true)) << desc;
ASSERT_EQ(dl1->bytes(true), dl2->bytes(true)) << desc;
EXPECT_EQ(dl1->total_depth(), dl2->total_depth()) << desc;
ASSERT_EQ(dl1->bounds(), dl2->bounds()) << desc;
ASSERT_EQ(dl1->total_depth(), dl2->total_depth()) << desc;
ASSERT_TRUE(DisplayListsEQ_Verbose(dl1, dl2)) << desc;
ASSERT_TRUE(DisplayListsEQ_Verbose(dl2, dl2)) << desc;
ASSERT_EQ(dl1->rtree().get(), nullptr) << desc;
Expand All @@ -691,6 +702,7 @@ TEST_F(DisplayListTest, FullRotationsAreNop) {
ASSERT_EQ(dl->bytes(true), sizeof(DisplayList));
ASSERT_EQ(dl->op_count(false), 0u);
ASSERT_EQ(dl->op_count(true), 0u);
EXPECT_EQ(dl->total_depth(), 0u);
}

TEST_F(DisplayListTest, AllBlendModeNops) {
Expand All @@ -702,6 +714,7 @@ TEST_F(DisplayListTest, AllBlendModeNops) {
ASSERT_EQ(dl->bytes(true), sizeof(DisplayList));
ASSERT_EQ(dl->op_count(false), 0u);
ASSERT_EQ(dl->op_count(true), 0u);
EXPECT_EQ(dl->total_depth(), 0u);
}

TEST_F(DisplayListTest, DisplayListsWithVaryingOpComparisons) {
Expand Down Expand Up @@ -933,13 +946,15 @@ TEST_F(DisplayListTest, NestedOpCountMetricsSameAsSkPicture) {
receiver.drawRect(SkRect::MakeXYWH(x, y, 80, 80));
}
}

DisplayListBuilder outer_builder(SkRect::MakeWH(150, 100));
DlOpReceiver& outer_receiver = ToReceiver(outer_builder);
outer_receiver.drawDisplayList(builder.Build());

auto display_list = outer_builder.Build();

ASSERT_EQ(display_list->op_count(), 1u);
ASSERT_EQ(display_list->op_count(true), 36u);
EXPECT_EQ(display_list->total_depth(), 37u);

ASSERT_EQ(picture->approximateOpCount(),
static_cast<int>(display_list->op_count()));
Expand Down Expand Up @@ -1556,6 +1571,7 @@ TEST_F(DisplayListTest, FlutterSvgIssue661BoundsWereEmpty) {
EXPECT_EQ(display_list->bounds().roundOut(), SkIRect::MakeWH(100, 100));
EXPECT_EQ(display_list->op_count(), 19u);
EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 400u);
EXPECT_EQ(display_list->total_depth(), 3u);
}

TEST_F(DisplayListTest, TranslateAffectsCurrentTransform) {
Expand Down Expand Up @@ -3084,11 +3100,13 @@ TEST_F(DisplayListTest, DrawUnorderedRoundRectPathCCW) {
TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) {
auto run_tests = [](const std::string& name,
void init(DisplayListBuilder & builder, DlPaint & paint),
uint32_t expected_op_count = 0u) {
uint32_t expected_op_count = 0u,
uint32_t expected_total_depth = 0u) {
auto run_one_test =
[init](const std::string& name,
void build(DisplayListBuilder & builder, DlPaint & paint),
uint32_t expected_op_count = 0u) {
uint32_t expected_op_count = 0u,
uint32_t expected_total_depth = 0u) {
DisplayListBuilder builder;
DlPaint paint;
init(builder, paint);
Expand All @@ -3098,26 +3116,27 @@ TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) {
FML_LOG(ERROR) << *list;
}
ASSERT_EQ(list->op_count(), expected_op_count) << name;
EXPECT_EQ(list->total_depth(), expected_total_depth) << name;
ASSERT_TRUE(list->bounds().isEmpty()) << name;
};
run_one_test(
name + " DrawColor",
[](DisplayListBuilder& builder, DlPaint& paint) {
builder.DrawColor(paint.getColor(), paint.getBlendMode());
},
expected_op_count);
expected_op_count, expected_total_depth);
run_one_test(
name + " DrawPaint",
[](DisplayListBuilder& builder, DlPaint& paint) {
builder.DrawPaint(paint);
},
expected_op_count);
expected_op_count, expected_total_depth);
run_one_test(
name + " DrawRect",
[](DisplayListBuilder& builder, DlPaint& paint) {
builder.DrawRect({10, 10, 20, 20}, paint);
},
expected_op_count);
expected_op_count, expected_total_depth);
run_one_test(
name + " Other Draw Ops",
[](DisplayListBuilder& builder, DlPaint& paint) {
Expand Down Expand Up @@ -3155,23 +3174,23 @@ TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) {
builder.DrawShadow(kTestPath1, paint.getColor(), 1, true, 1);
}
},
expected_op_count);
expected_op_count, expected_total_depth);
run_one_test(
name + " SaveLayer",
[](DisplayListBuilder& builder, DlPaint& paint) {
builder.SaveLayer(nullptr, &paint, nullptr);
builder.DrawRect({10, 10, 20, 20}, DlPaint());
builder.Restore();
},
expected_op_count);
expected_op_count, expected_total_depth);
run_one_test(
name + " inside Save",
[](DisplayListBuilder& builder, DlPaint& paint) {
builder.Save();
builder.DrawRect({10, 10, 20, 20}, paint);
builder.Restore();
},
expected_op_count);
expected_op_count, expected_total_depth);
};
run_tests("transparent color", //
[](DisplayListBuilder& builder, DlPaint& paint) {
Expand Down Expand Up @@ -3230,7 +3249,7 @@ TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) {
builder.SaveLayer(nullptr, nullptr);
paint.setBlendMode(DlBlendMode::kDst);
},
2u);
2u, 1u);
run_tests("DrawImage inside Culled SaveLayer", //
[](DisplayListBuilder& builder, DlPaint& paint) {
DlPaint save_paint;
Expand Down Expand Up @@ -3781,5 +3800,87 @@ TEST_F(DisplayListTest, SaveLayerBoundsClipDetectionSimpleClippedRect) {
EXPECT_TRUE(expector.all_bounds_checked());
}

class DepthExpector : public virtual DlOpReceiver,
virtual IgnoreAttributeDispatchHelper,
virtual IgnoreTransformDispatchHelper,
virtual IgnoreClipDispatchHelper,
virtual IgnoreDrawDispatchHelper {
public:
explicit DepthExpector(std::vector<uint32_t> expectations)
: depth_expectations_(std::move(expectations)) {}

void save() override {
// This method should not be called since we override the variant with
// the total_content_depth parameter.
FAIL() << "save(no depth parameter) method should not be called";
}

void save(uint32_t total_content_depth) override {
ASSERT_LT(index_, depth_expectations_.size());
EXPECT_EQ(depth_expectations_[index_], total_content_depth)
<< "at index " << index_;
index_++;
}

void saveLayer(const SkRect& bounds,
SaveLayerOptions options,
const DlImageFilter* backdrop) override {
// This method should not be called since we override the variant with
// the total_content_depth parameter.
FAIL() << "saveLayer(no depth parameter) method should not be called";
}

void saveLayer(const SkRect& bounds,
const SaveLayerOptions& options,
uint32_t total_content_depth,
const DlImageFilter* backdrop) override {
ASSERT_LT(index_, depth_expectations_.size());
EXPECT_EQ(depth_expectations_[index_], total_content_depth)
<< "at index " << index_;
index_++;
}

private:
size_t index_ = 0;
std::vector<uint32_t> depth_expectations_;
};

TEST_F(DisplayListTest, SaveContentDepthTest) {
DisplayListBuilder child_builder;
child_builder.DrawRect({10, 10, 20, 20}, DlPaint()); // depth 1
auto child = child_builder.Build();

DisplayListBuilder builder;
builder.DrawRect({10, 10, 20, 20}, DlPaint()); // depth 1

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

builder.DrawDisplayList(child, 1.0f); // depth 3 (content) + 4 (self)

builder.SaveLayer(nullptr, nullptr); // covers depth 5->6
{
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 7

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

builder.DrawRect({16, 16, 26, 26}, DlPaint()); // depth 10
builder.DrawRect({18, 18, 28, 28}, DlPaint()); // depth 11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test that checks the depth of clip ops added to both the root and to the root and within a SaveLayer? That way we know they close with the correct depth (max depth of everything they affect).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the "depth of clip ops"? They aren't accounted in DL. That's an Impeller thing...?

All DL does is count how many render calls are within a save/restore or an entire DL. Clips are invisible to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in the class comment in dl_op_receiver, the only places depth information is reported is the DL total_depth and the argument passed to save/saveLayer.

auto display_list = builder.Build();

EXPECT_EQ(display_list->total_depth(), 11u);

DepthExpector expector({8, 2});
display_list->Dispatch(expector);
}

} // namespace testing
} // namespace flutter
Loading