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

[Impeller] take advantage of DisplayList culling #41606

Merged
merged 4 commits into from
May 1, 2023
Merged

[Impeller] take advantage of DisplayList culling #41606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
03dbc94
take advantage of DisplayList culling in Impeller
flar Apr 29, 2023
81af5eb
make cull_rect std::optional and add unit tests of Canvas clip bounds…
flar Apr 30, 2023
77ad0e7
fix warnings about DL with no RTree while running gallery
flar May 1, 2023
fa1cfa8
fix cull_rect handling in Impeller submit callback
flar May 1, 2023
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
1 change: 1 addition & 0 deletions ci/licenses_golden/excluded_files
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
../../../flutter/impeller/.gitignore
../../../flutter/impeller/README.md
../../../flutter/impeller/aiks/aiks_unittests.cc
../../../flutter/impeller/aiks/canvas_unittests.cc
../../../flutter/impeller/archivist/archivist_unittests.cc
../../../flutter/impeller/base/README.md
../../../flutter/impeller/base/base_unittests.cc
Expand Down
5 changes: 5 additions & 0 deletions display_list/display_list.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ void DisplayList::Dispatch(DlOpReceiver& receiver) const {
Dispatch(receiver, ptr, ptr + byte_count_, NopCuller::instance);
}

void DisplayList::Dispatch(DlOpReceiver& receiver,
const SkIRect& cull_rect) const {
Dispatch(receiver, SkRect::Make(cull_rect));
}

void DisplayList::Dispatch(DlOpReceiver& receiver,
const SkRect& cull_rect) const {
if (cull_rect.isEmpty()) {
Expand Down
1 change: 1 addition & 0 deletions display_list/display_list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ class DisplayList : public SkRefCnt {

void Dispatch(DlOpReceiver& ctx) const;
void Dispatch(DlOpReceiver& ctx, const SkRect& cull_rect) const;
void Dispatch(DlOpReceiver& ctx, const SkIRect& cull_rect) const;

// From historical behavior, SkPicture always included nested bytes,
// but nested ops are only included if requested. The defaults used
Expand Down
58 changes: 28 additions & 30 deletions display_list/display_list_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2532,81 +2532,79 @@ TEST_F(DisplayListTest, RTreeRenderCulling) {
main_receiver.drawRect({20, 20, 30, 30});
auto main = main_builder.Build();

auto test = [main](SkIRect cull_rect, const sk_sp<DisplayList>& expected) {
{ // Test SkIRect culling
DisplayListBuilder culling_builder;
main->Dispatch(ToReceiver(culling_builder), cull_rect);

EXPECT_TRUE(DisplayListsEQ_Verbose(culling_builder.Build(), expected));
}

{ // Test SkRect culling
DisplayListBuilder culling_builder;
main->Dispatch(ToReceiver(culling_builder), SkRect::Make(cull_rect));

EXPECT_TRUE(DisplayListsEQ_Verbose(culling_builder.Build(), expected));
}
};

{ // No rects
SkRect cull_rect = {11, 11, 19, 19};
SkIRect cull_rect = {11, 11, 19, 19};

DisplayListBuilder expected_builder;
auto expected = expected_builder.Build();

DisplayListBuilder culling_builder(cull_rect);
main->Dispatch(ToReceiver(culling_builder), cull_rect);

EXPECT_TRUE(DisplayListsEQ_Verbose(culling_builder.Build(), expected));
test(cull_rect, expected);
}

{ // Rect 1
SkRect cull_rect = {9, 9, 19, 19};
SkIRect cull_rect = {9, 9, 19, 19};

DisplayListBuilder expected_builder;
DlOpReceiver& expected_receiver = ToReceiver(expected_builder);
expected_receiver.drawRect({0, 0, 10, 10});
auto expected = expected_builder.Build();

DisplayListBuilder culling_builder(cull_rect);
main->Dispatch(ToReceiver(culling_builder), cull_rect);

EXPECT_TRUE(DisplayListsEQ_Verbose(culling_builder.Build(), expected));
test(cull_rect, expected);
}

{ // Rect 2
SkRect cull_rect = {11, 9, 21, 19};
SkIRect cull_rect = {11, 9, 21, 19};

DisplayListBuilder expected_builder;
DlOpReceiver& expected_receiver = ToReceiver(expected_builder);
expected_receiver.drawRect({20, 0, 30, 10});
auto expected = expected_builder.Build();

DisplayListBuilder culling_builder(cull_rect);
main->Dispatch(ToReceiver(culling_builder), cull_rect);

EXPECT_TRUE(DisplayListsEQ_Verbose(culling_builder.Build(), expected));
test(cull_rect, expected);
}

{ // Rect 3
SkRect cull_rect = {9, 11, 19, 21};
SkIRect cull_rect = {9, 11, 19, 21};

DisplayListBuilder expected_builder;
DlOpReceiver& expected_receiver = ToReceiver(expected_builder);
expected_receiver.drawRect({0, 20, 10, 30});
auto expected = expected_builder.Build();

DisplayListBuilder culling_builder(cull_rect);
main->Dispatch(ToReceiver(culling_builder), cull_rect);

EXPECT_TRUE(DisplayListsEQ_Verbose(culling_builder.Build(), expected));
test(cull_rect, expected);
}

{ // Rect 4
SkRect cull_rect = {11, 11, 21, 21};
SkIRect cull_rect = {11, 11, 21, 21};

DisplayListBuilder expected_builder;
DlOpReceiver& expected_receiver = ToReceiver(expected_builder);
expected_receiver.drawRect({20, 20, 30, 30});
auto expected = expected_builder.Build();

DisplayListBuilder culling_builder(cull_rect);
main->Dispatch(ToReceiver(culling_builder), cull_rect);

EXPECT_TRUE(DisplayListsEQ_Verbose(culling_builder.Build(), expected));
test(cull_rect, expected);
}

{ // All 4 rects
SkRect cull_rect = {9, 9, 21, 21};

DisplayListBuilder culling_builder(cull_rect);
main->Dispatch(ToReceiver(culling_builder), cull_rect);
SkIRect cull_rect = {9, 9, 21, 21};

EXPECT_TRUE(DisplayListsEQ_Verbose(culling_builder.Build(), main));
test(cull_rect, main);
}
}

Expand Down
3 changes: 2 additions & 1 deletion flow/surface_frame.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ SurfaceFrame::SurfaceFrame(sk_sp<SkSurface> surface,
canvas_ = &adapter_;
} else if (display_list_fallback) {
FML_DCHECK(!frame_size.isEmpty());
dl_builder_ = sk_make_sp<DisplayListBuilder>(SkRect::Make(frame_size));
dl_builder_ =
sk_make_sp<DisplayListBuilder>(SkRect::Make(frame_size), true);
canvas_ = dl_builder_.get();
}
}
Expand Down
5 changes: 4 additions & 1 deletion impeller/aiks/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ impeller_component("aiks_playground") {

impeller_component("aiks_unittests") {
testonly = true
sources = [ "aiks_unittests.cc" ]
sources = [
"aiks_unittests.cc",
"canvas_unittests.cc",
]
deps = [
":aiks",
":aiks_playground",
Expand Down
93 changes: 89 additions & 4 deletions impeller/aiks/canvas.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,25 @@
namespace impeller {

Canvas::Canvas() {
Initialize();
Initialize(std::nullopt);
}

Canvas::Canvas(Rect cull_rect) {
Initialize(cull_rect);
}

Canvas::Canvas(IRect cull_rect) {
Initialize(Rect::MakeLTRB(cull_rect.GetLeft(), cull_rect.GetTop(),
cull_rect.GetRight(), cull_rect.GetBottom()));
}

Canvas::~Canvas() = default;

void Canvas::Initialize() {
void Canvas::Initialize(std::optional<Rect> cull_rect) {
initial_cull_rect_ = cull_rect;
base_pass_ = std::make_unique<EntityPass>();
current_pass_ = base_pass_.get();
xformation_stack_.emplace_back(CanvasStackEntry{});
xformation_stack_.emplace_back(CanvasStackEntry{.cull_rect = cull_rect});
lazy_glyph_atlas_ = std::make_shared<LazyGlyphAtlas>();
FML_DCHECK(GetSaveCount() == 1u);
FML_DCHECK(base_pass_->GetSubpassesDepth() == 1u);
Expand All @@ -54,6 +64,7 @@ void Canvas::Save(
std::optional<EntityPass::BackdropFilterProc> backdrop_filter) {
auto entry = CanvasStackEntry{};
entry.xformation = xformation_stack_.back().xformation;
entry.cull_rect = xformation_stack_.back().cull_rect;
entry.stencil_depth = xformation_stack_.back().stencil_depth;
if (create_subpass) {
entry.is_subpass = true;
Expand Down Expand Up @@ -109,6 +120,15 @@ const Matrix& Canvas::GetCurrentTransformation() const {
return xformation_stack_.back().xformation;
}

const std::optional<Rect> Canvas::GetCurrentLocalCullingBounds() const {
auto cull_rect = xformation_stack_.back().cull_rect;
if (cull_rect.has_value()) {
Matrix inverse = xformation_stack_.back().xformation.Invert();
cull_rect = cull_rect.value().TransformBounds(inverse);
}
return cull_rect;
}
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to add a regular test (doesn't use OpenPlaygroundHere) in aiks_unittests which uses this method to verify the intersection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way I can have a test that creates a wide ranging DL, then dispatches it to an impeller Dispatcher with a cull_rect (and one that does that with a sub-DisplayList as well), and then checks that there are only Entities for the ops that should have made it through the culling?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a new canvas_unittests file and wrote a whole suite of tests which found several bugs in Rect::CutOut().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With respect to making sure that the Entities and Passes are culled by the cull rect, we already have tests of the DisplayList RTree mechanism that make sure that the culled Dispatch variant correctly omits ops in the playback, and with the new tests in canvas_unittests.cc that check that Impeller's Canvas manages the cull rect according to the desired behavior - is there anything left to test here?

Copy link
Member

@bdero bdero May 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'd say this level of test is beyond the call of duty. 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, given that it found errors in an existing geom method, isn't it just "at the call of duty"?

Copy link
Member

Choose a reason for hiding this comment

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

We already cover rect methods in geometry_unittests.cc. It sounds like I probably missed a case there.


void Canvas::Translate(const Vector3& offset) {
Concat(Matrix::MakeTranslation(offset));
}
Expand Down Expand Up @@ -258,16 +278,56 @@ void Canvas::DrawCircle(Point center, Scalar radius, const Paint& paint) {

void Canvas::ClipPath(const Path& path, Entity::ClipOperation clip_op) {
ClipGeometry(Geometry::MakeFillPath(path), clip_op);
if (clip_op == Entity::ClipOperation::kIntersect) {
auto bounds = path.GetBoundingBox();
if (bounds.has_value()) {
IntersectCulling(bounds.value());
}
}
}

void Canvas::ClipRect(const Rect& rect, Entity::ClipOperation clip_op) {
ClipGeometry(Geometry::MakeRect(rect), clip_op);
switch (clip_op) {
case Entity::ClipOperation::kIntersect:
IntersectCulling(rect);
break;
case Entity::ClipOperation::kDifference:
SubtractCulling(rect);
break;
}
}

void Canvas::ClipRRect(const Rect& rect,
Scalar corner_radius,
Entity::ClipOperation clip_op) {
ClipGeometry(Geometry::MakeRRect(rect, corner_radius), clip_op);
switch (clip_op) {
case Entity::ClipOperation::kIntersect:
IntersectCulling(rect);
break;
case Entity::ClipOperation::kDifference:
if (corner_radius <= 0) {
SubtractCulling(rect);
} else {
// We subtract the inner "tall" and "wide" rectangle pieces
// that fit inside the corners which cover the greatest area
// without involving the curved corners
// Since this is a subtract operation, we can subtract each
// rectangle piece individually without fear of interference.
if (corner_radius * 2 < rect.size.width) {
SubtractCulling(Rect::MakeLTRB(
rect.GetLeft() + corner_radius, rect.GetTop(),
rect.GetRight() - corner_radius, rect.GetBottom()));
}
if (corner_radius * 2 < rect.size.height) {
SubtractCulling(Rect::MakeLTRB(
rect.GetLeft(), rect.GetTop() + corner_radius, //
rect.GetRight(), rect.GetBottom() - corner_radius));
}
}
break;
}
}

void Canvas::ClipGeometry(std::unique_ptr<Geometry> geometry,
Expand All @@ -287,6 +347,31 @@ void Canvas::ClipGeometry(std::unique_ptr<Geometry> geometry,
xformation_stack_.back().contains_clips = true;
}

void Canvas::IntersectCulling(Rect clip_rect) {
clip_rect = clip_rect.TransformBounds(GetCurrentTransformation());
std::optional<Rect>& cull_rect = xformation_stack_.back().cull_rect;
if (cull_rect.has_value()) {
cull_rect = cull_rect
.value() //
.Intersection(clip_rect) //
.value_or(Rect{});
} else {
cull_rect = clip_rect;
}
}

void Canvas::SubtractCulling(Rect clip_rect) {
std::optional<Rect>& cull_rect = xformation_stack_.back().cull_rect;
if (cull_rect.has_value()) {
clip_rect = clip_rect.TransformBounds(GetCurrentTransformation());
cull_rect = cull_rect
.value() //
.Cutout(clip_rect) //
.value_or(Rect{});
}
// else (no cull) diff (any clip) is non-rectangular
}

void Canvas::RestoreClip() {
Entity entity;
entity.SetTransformation(GetCurrentTransformation());
Expand Down Expand Up @@ -364,7 +449,7 @@ Picture Canvas::EndRecordingAsPicture() {
picture.pass = std::move(base_pass_);

Reset();
Initialize();
Initialize(initial_cull_rect_);

return picture;
}
Expand Down
21 changes: 20 additions & 1 deletion impeller/aiks/canvas.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ namespace impeller {

class Entity;

struct CanvasStackEntry {
Matrix xformation;
// |cull_rect| is conservative screen-space bounds of the clipped output area
std::optional<Rect> cull_rect;
size_t stencil_depth = 0u;
bool is_subpass = false;
bool contains_clips = false;
};

class Canvas {
public:
struct DebugOptions {
Expand All @@ -40,6 +49,10 @@ class Canvas {

Canvas();

explicit Canvas(Rect cull_rect);

explicit Canvas(IRect cull_rect);

~Canvas();

void Save();
Expand All @@ -57,6 +70,8 @@ class Canvas {

const Matrix& GetCurrentTransformation() const;

const std::optional<Rect> GetCurrentLocalCullingBounds() const;

void ResetTransform();

void Transform(const Matrix& xformation);
Expand Down Expand Up @@ -135,8 +150,9 @@ class Canvas {
EntityPass* current_pass_ = nullptr;
std::deque<CanvasStackEntry> xformation_stack_;
std::shared_ptr<LazyGlyphAtlas> lazy_glyph_atlas_;
std::optional<Rect> initial_cull_rect_;

void Initialize();
void Initialize(std::optional<Rect> cull_rect);

void Reset();

Expand All @@ -147,6 +163,9 @@ class Canvas {
void ClipGeometry(std::unique_ptr<Geometry> geometry,
Entity::ClipOperation clip_op);

void IntersectCulling(Rect clip_bounds);
void SubtractCulling(Rect clip_bounds);

void Save(bool create_subpass,
BlendMode = BlendMode::kSourceOver,
std::optional<EntityPass::BackdropFilterProc> backdrop_filter =
Expand Down
Loading