From 60aa66885f08d21ced1497f4e0bc063c20400bd6 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 5 Jun 2024 00:06:27 -0700 Subject: [PATCH 1/2] [DisplayList] cull clip operations that can be trivially and safely ignored --- display_list/display_list_unittests.cc | 346 +++++++++++++++++- display_list/dl_builder.cc | 68 ++-- display_list/dl_builder.h | 4 + display_list/testing/dl_test_snippets.cc | 11 +- display_list/utils/dl_matrix_clip_tracker.cc | 139 ++++++- display_list/utils/dl_matrix_clip_tracker.h | 11 + .../utils/dl_matrix_clip_tracker_unittests.cc | 174 +++++++++ .../layers/backdrop_filter_layer_unittests.cc | 2 +- impeller/geometry/rect.h | 19 + 9 files changed, 734 insertions(+), 40 deletions(-) diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index ef423aa7f29f8..88daac0062181 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -680,6 +680,7 @@ TEST_F(DisplayListTest, SingleOpDisplayListsRecapturedAreEqual) { sk_sp copy = copy_builder.Build(); auto desc = group.op_name + "(variant " + std::to_string(i + 1) + " == copy)"; + DisplayListsEQ_Verbose(dl, copy); ASSERT_EQ(copy->op_count(false), dl->op_count(false)) << desc; ASSERT_EQ(copy->bytes(false), dl->bytes(false)) << desc; ASSERT_EQ(copy->op_count(true), dl->op_count(true)) << desc; @@ -3383,7 +3384,7 @@ TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) { } TEST_F(DisplayListTest, ImpellerPathPreferenceIsHonored) { - class Tester : virtual public DlOpReceiver, + class Tester : public virtual DlOpReceiver, public IgnoreClipDispatchHelper, public IgnoreDrawDispatchHelper, public IgnoreAttributeDispatchHelper, @@ -3962,6 +3963,10 @@ class DepthExpector : public virtual DlOpReceiver, index_++; } + bool all_depths_checked() const { + return index_ == depth_expectations_.size(); + } + private: size_t index_ = 0; std::vector depth_expectations_; @@ -4331,5 +4336,344 @@ TEST_F(DisplayListTest, DrawDisplayListForwardsBackdropFlag) { EXPECT_TRUE(parent_dl->root_has_backdrop_filter()); } +#define CLIP_EXPECTOR(name) ClipExpector name(__FILE__, __LINE__) + +class ClipExpector : public virtual DlOpReceiver, + virtual IgnoreAttributeDispatchHelper, + virtual IgnoreTransformDispatchHelper, + virtual IgnoreDrawDispatchHelper { + public: + struct Expectation { + std::variant shape; + ClipOp clip_op; + bool is_aa; + + std::string shape_name() { + switch (shape.index()) { + case 0: + return "SkRect"; + case 1: + return "SkRRect"; + case 2: + return "SkPath"; + default: + return "Unknown"; + } + } + }; + + // file and line supplied automatically from CLIP_EXPECTOR macro + explicit ClipExpector(const std::string& file, int line) + : file_(file), line_(line) {} + + ~ClipExpector() { // + EXPECT_EQ(index_, clip_expectations_.size()) << label(); + } + + ClipExpector& addExpectation(const SkRect& rect, + ClipOp clip_op = ClipOp::kIntersect, + bool is_aa = false) { + clip_expectations_.push_back({ + .shape = rect, + .clip_op = clip_op, + .is_aa = is_aa, + }); + return *this; + } + + ClipExpector& addExpectation(const SkRRect& rrect, + ClipOp clip_op = ClipOp::kIntersect, + bool is_aa = false) { + clip_expectations_.push_back({ + .shape = rrect, + .clip_op = clip_op, + .is_aa = is_aa, + }); + return *this; + } + + ClipExpector& addExpectation(const SkPath& path, + ClipOp clip_op = ClipOp::kIntersect, + bool is_aa = false) { + clip_expectations_.push_back({ + .shape = path, + .clip_op = clip_op, + .is_aa = is_aa, + }); + return *this; + } + + void clipRect(const SkRect& rect, + DlCanvas::ClipOp clip_op, + bool is_aa) override { + check(rect, clip_op, is_aa); + } + void clipRRect(const SkRRect& rrect, + DlCanvas::ClipOp clip_op, + bool is_aa) override { + check(rrect, clip_op, is_aa); + } + void clipPath(const SkPath& path, + DlCanvas::ClipOp clip_op, + bool is_aa) override { + check(path, clip_op, is_aa); + } + + private: + size_t index_ = 0; + std::vector clip_expectations_; + + template + void check(T shape, ClipOp clip_op, bool is_aa) { + ASSERT_LT(index_, clip_expectations_.size()) << label(); + auto expected = clip_expectations_[index_]; + EXPECT_EQ(expected.clip_op, clip_op) << label(); + EXPECT_EQ(expected.is_aa, is_aa) << label(); + if (!std::holds_alternative(expected.shape)) { + EXPECT_TRUE(std::holds_alternative(expected.shape)) + << label() << ", expected type: " << expected.shape_name(); + } else { + EXPECT_EQ(std::get(expected.shape), shape) << label(); + } + index_++; + } + + const std::string file_; + const int line_; + + std::string label() { + return "at index " + std::to_string(index_) + // + ", from " + file_ + // + ":" + std::to_string(line_); + } +}; + +TEST_F(DisplayListTest, ClipRectCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipRect(clip.makeOutset(1.0f, 1.0f), ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipRectNonCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto smaller_clip = clip.makeInset(1.0f, 1.0f); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipRect(smaller_clip, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + expector.addExpectation(smaller_clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipRectNestedCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto larger_clip = clip.makeOutset(1.0f, 1.0f); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.Save(); + cull_builder.ClipRect(larger_clip, ClipOp::kIntersect, false); + cull_builder.Restore(); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipRectNestedNonCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto larger_clip = clip.makeOutset(1.0f, 1.0f); + + DisplayListBuilder cull_builder; + cull_builder.Save(); + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.Restore(); + // Should not be culled because we have restored the prior clip + cull_builder.ClipRect(larger_clip, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + expector.addExpectation(larger_clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipRectNestedCullingComplex) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto smaller_clip = clip.makeInset(1.0f, 1.0f); + auto smallest_clip = clip.makeInset(2.0f, 2.0f); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.Save(); + cull_builder.ClipRect(smallest_clip, ClipOp::kIntersect, false); + cull_builder.ClipRect(smaller_clip, ClipOp::kIntersect, false); + cull_builder.Restore(); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + expector.addExpectation(smallest_clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipRectNestedNonCullingComplex) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto smaller_clip = clip.makeInset(1.0f, 1.0f); + auto smallest_clip = clip.makeInset(2.0f, 2.0f); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.Save(); + cull_builder.ClipRect(smallest_clip, ClipOp::kIntersect, false); + cull_builder.Restore(); + // Would not be culled if it was inside the clip + cull_builder.ClipRect(smaller_clip, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + expector.addExpectation(smallest_clip, ClipOp::kIntersect, false); + expector.addExpectation(smaller_clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipRRectCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto rrect = SkRRect::MakeRectXY(clip.makeOutset(2.0f, 2.0f), 2.0f, 2.0f); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipRRect(rrect, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipRRectNonCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto rrect = SkRRect::MakeRectXY(clip.makeOutset(2.0f, 2.0f), 12.0f, 12.0f); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipRRect(rrect, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + expector.addExpectation(rrect, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipPathNonCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + SkPath path; + path.moveTo(0.0f, 0.0f); + path.lineTo(1000.0f, 0.0f); + path.lineTo(0.0f, 1000.0f); + path.close(); + + // Double checking that the path does indeed contain the clip. But, + // sadly, the Builder will not check paths for coverage to this level + // of detail. (In particular, path containment of the corners is not + // authoritative of true containment, but we know in this case that + // a triangle contains a rect if it contains all 4 corners...) + ASSERT_TRUE(path.contains(clip.fLeft, clip.fTop)); + ASSERT_TRUE(path.contains(clip.fRight, clip.fTop)); + ASSERT_TRUE(path.contains(clip.fRight, clip.fBottom)); + ASSERT_TRUE(path.contains(clip.fLeft, clip.fBottom)); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipPath(path, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + expector.addExpectation(path, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipPathRectCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + SkPath path; + path.addRect(clip.makeOutset(1.0f, 1.0f)); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipPath(path, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipPathRectNonCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto smaller_clip = clip.makeInset(1.0f, 1.0f); + SkPath path; + path.addRect(smaller_clip); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipPath(path, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + // Builder will not cull this clip, but it will turn it into a ClipRect + expector.addExpectation(smaller_clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipPathRRectCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto rrect = SkRRect::MakeRectXY(clip.makeOutset(2.0f, 2.0f), 2.0f, 2.0f); + SkPath path; + path.addRRect(rrect); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipPath(path, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + +TEST_F(DisplayListTest, ClipPathRRectNonCulling) { + auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); + auto rrect = SkRRect::MakeRectXY(clip.makeOutset(2.0f, 2.0f), 12.0f, 12.0f); + SkPath path; + path.addRRect(rrect); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + cull_builder.ClipPath(path, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(clip, ClipOp::kIntersect, false); + // Builder will not cull this clip, but it will turn it into a ClipRRect + expector.addExpectation(rrect, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + } // namespace testing } // namespace flutter diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc index d5689ed4fce9e..2a048c709aaf2 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -956,13 +956,22 @@ void DisplayListBuilder::ClipRect(const SkRect& rect, if (!rect.isFinite()) { return; } + if (current_info().is_nop) { + return; + } + if (current_info().has_valid_clip && + clip_op == DlCanvas::ClipOp::kIntersect && + layer_local_state().rect_covers_cull(rect)) { + return; + } global_state().clipRect(rect, clip_op, is_aa); - if (current_info().is_nop || - current_info().global_state.is_cull_rect_empty()) { + layer_local_state().clipRect(rect, clip_op, is_aa); + if (global_state().is_cull_rect_empty() || + layer_local_state().is_cull_rect_empty()) { current_info().is_nop = true; return; } - layer_local_state().clipRect(rect, clip_op, is_aa); + current_info().has_valid_clip = true; checkForDeferredSave(); switch (clip_op) { case ClipOp::kIntersect: @@ -978,28 +987,40 @@ void DisplayListBuilder::ClipRRect(const SkRRect& rrect, bool is_aa) { if (rrect.isRect()) { clipRect(rrect.rect(), clip_op, is_aa); - } else { - global_state().clipRRect(rrect, clip_op, is_aa); - if (current_info().is_nop || - current_info().global_state.is_cull_rect_empty()) { - current_info().is_nop = true; - return; - } - layer_local_state().clipRRect(rrect, clip_op, is_aa); - checkForDeferredSave(); - switch (clip_op) { - case ClipOp::kIntersect: - Push(0, rrect, is_aa); - break; - case ClipOp::kDifference: - Push(0, rrect, is_aa); - break; - } + return; + } + if (current_info().is_nop) { + return; + } + if (current_info().has_valid_clip && + clip_op == DlCanvas::ClipOp::kIntersect && + layer_local_state().rrect_covers_cull(rrect)) { + return; + } + global_state().clipRRect(rrect, clip_op, is_aa); + layer_local_state().clipRRect(rrect, clip_op, is_aa); + if (global_state().is_cull_rect_empty() || + layer_local_state().is_cull_rect_empty()) { + current_info().is_nop = true; + return; + } + current_info().has_valid_clip = true; + checkForDeferredSave(); + switch (clip_op) { + case ClipOp::kIntersect: + Push(0, rrect, is_aa); + break; + case ClipOp::kDifference: + Push(0, rrect, is_aa); + break; } } void DisplayListBuilder::ClipPath(const SkPath& path, ClipOp clip_op, bool is_aa) { + if (current_info().is_nop) { + return; + } if (!path.isInverseFillType()) { SkRect rect; if (path.isRect(&rect)) { @@ -1018,12 +1039,13 @@ void DisplayListBuilder::ClipPath(const SkPath& path, } } global_state().clipPath(path, clip_op, is_aa); - if (current_info().is_nop || - current_info().global_state.is_cull_rect_empty()) { + layer_local_state().clipPath(path, clip_op, is_aa); + if (global_state().is_cull_rect_empty() || + layer_local_state().is_cull_rect_empty()) { current_info().is_nop = true; return; } - layer_local_state().clipPath(path, clip_op, is_aa); + current_info().has_valid_clip = true; checkForDeferredSave(); switch (clip_op) { case ClipOp::kIntersect: diff --git a/display_list/dl_builder.h b/display_list/dl_builder.h index f492680003a5e..793856d64aad9 100644 --- a/display_list/dl_builder.h +++ b/display_list/dl_builder.h @@ -566,6 +566,7 @@ class DisplayListBuilder final : public virtual DlCanvas, // For constructor (root layer) initialization explicit SaveInfo(const DlRect& cull_rect) : is_save_layer(true), + has_valid_clip(false), global_state(cull_rect), layer_state(cull_rect), layer_info(new LayerInfo(nullptr, 0u)) {} @@ -576,6 +577,7 @@ class DisplayListBuilder final : public virtual DlCanvas, explicit SaveInfo(const SaveInfo* parent_info) : is_save_layer(false), has_deferred_save_op(true), + has_valid_clip(parent_info->has_valid_clip), global_state(parent_info->global_state), layer_state(parent_info->layer_state), layer_info(parent_info->layer_info) {} @@ -585,6 +587,7 @@ class DisplayListBuilder final : public virtual DlCanvas, const std::shared_ptr& filter, int rtree_rect_index) : is_save_layer(true), + has_valid_clip(false), global_state(parent_info->global_state), layer_state(kMaxCullRect), layer_info(new LayerInfo(filter, rtree_rect_index)) {} @@ -593,6 +596,7 @@ class DisplayListBuilder final : public virtual DlCanvas, bool has_deferred_save_op = false; bool is_nop = false; + bool has_valid_clip; // The depth when the save call is recorded, used to compute the total // depth of its content when the associated restore is called. diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index 56b36804f6a0b..5e744519f67dc 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -287,19 +287,22 @@ std::vector CreateAllSaveRestoreOps() { r.drawRect({10, 10, 20, 20}); r.restore(); }}, + // For saveLayer calls with bounds, we need at least one unclipped + // draw command so that the bounds are not reduced in size to the + // clip dimensions on the re-dispatch. {5, 120, 3, [](DlOpReceiver& r) { r.saveLayer(&kTestBounds, SaveLayerOptions::kNoAttributes); + r.drawRect(kTestBounds); r.clipRect({0, 0, 25, 25}, DlCanvas::ClipOp::kIntersect, true); - r.drawRect({5, 5, 15, 15}); r.drawRect({10, 10, 20, 20}); r.restore(); }}, {5, 120, 3, [](DlOpReceiver& r) { r.saveLayer(&kTestBounds, SaveLayerOptions::kWithAttributes); + r.drawRect(kTestBounds); r.clipRect({0, 0, 25, 25}, DlCanvas::ClipOp::kIntersect, true); - r.drawRect({5, 5, 15, 15}); r.drawRect({10, 10, 20, 20}); r.restore(); }}, @@ -325,8 +328,8 @@ std::vector CreateAllSaveRestoreOps() { [](DlOpReceiver& r) { r.saveLayer(&kTestBounds, SaveLayerOptions::kNoAttributes, &kTestCFImageFilter1); + r.drawRect(kTestBounds); r.clipRect({0, 0, 25, 25}, DlCanvas::ClipOp::kIntersect, true); - r.drawRect({5, 5, 15, 15}); r.drawRect({10, 10, 20, 20}); r.restore(); }}, @@ -334,8 +337,8 @@ std::vector CreateAllSaveRestoreOps() { [](DlOpReceiver& r) { r.saveLayer(&kTestBounds, SaveLayerOptions::kWithAttributes, &kTestCFImageFilter1); + r.drawRect(kTestBounds); r.clipRect({0, 0, 25, 25}, DlCanvas::ClipOp::kIntersect, true); - r.drawRect({5, 5, 15, 15}); r.drawRect({10, 10, 20, 20}); r.restore(); }}, diff --git a/display_list/utils/dl_matrix_clip_tracker.cc b/display_list/utils/dl_matrix_clip_tracker.cc index 63b953ea97a99..a5579f570b885 100644 --- a/display_list/utils/dl_matrix_clip_tracker.cc +++ b/display_list/utils/dl_matrix_clip_tracker.cc @@ -69,23 +69,40 @@ bool DisplayListMatrixClipState::mapAndClipRect(const SkRect& src, void DisplayListMatrixClipState::clipRect(const DlRect& rect, ClipOp op, bool is_aa) { - adjustCullRect(rect, op, is_aa); + if (rect.IsFinite()) { + adjustCullRect(rect, op, is_aa); + } } void DisplayListMatrixClipState::clipRRect(const SkRRect& rrect, ClipOp op, bool is_aa) { - SkRect bounds = rrect.getBounds(); + DlRect bounds = ToDlRect(rrect.getBounds()); + if (rrect.isRect()) { + return clipRect(bounds, op, is_aa); + } switch (op) { case ClipOp::kIntersect: + adjustCullRect(bounds, op, is_aa); break; - case ClipOp::kDifference: - if (!rrect.isRect()) { + case ClipOp::kDifference: { + if (rrect_covers_cull(rrect)) { + cull_rect_ = DlRect(); return; } + auto upper_left = rrect.radii(SkRRect::kUpperLeft_Corner); + auto upper_right = rrect.radii(SkRRect::kUpperRight_Corner); + auto lower_left = rrect.radii(SkRRect::kLowerLeft_Corner); + auto lower_right = rrect.radii(SkRRect::kLowerRight_Corner); + DlRect safe = bounds.Expand(-std::max(upper_left.fX, lower_left.fX), 0, + -std::max(upper_right.fX, lower_right.fX), 0); + adjustCullRect(safe, op, is_aa); + safe = bounds.Expand(0, -std::max(upper_left.fY, upper_right.fY), // + 0, -std::max(lower_left.fY, lower_right.fY)); + adjustCullRect(safe, op, is_aa); break; + } } - adjustCullRect(ToDlRect(bounds), op, is_aa); } void DisplayListMatrixClipState::clipPath(const SkPath& path, @@ -104,18 +121,17 @@ void DisplayListMatrixClipState::clipPath(const SkPath& path, } } - SkRect bounds; + DlRect bounds = ToDlRect(path.getBounds()); + if (path.isRect(nullptr)) { + return clipRect(bounds, op, is_aa); + } switch (op) { case ClipOp::kIntersect: - bounds = path.getBounds(); + adjustCullRect(bounds, op, is_aa); break; case ClipOp::kDifference: - if (!path.isRect(&bounds)) { - return; - } break; } - adjustCullRect(ToDlRect(bounds), op, is_aa); } bool DisplayListMatrixClipState::content_culled( @@ -216,4 +232,105 @@ SkRect DisplayListMatrixClipState::local_cull_rect() const { return ToSkRect(cull_rect_.TransformBounds(inverse)); } +bool DisplayListMatrixClipState::rect_covers_cull(const DlRect& content) const { + if (content.IsEmpty()) { + return false; + } + if (cull_rect_.IsEmpty()) { + return true; + } + DlPoint corners[4]; + if (!getLocalCullCorners(corners)) { + return false; + } + for (auto corner : corners) { + if (!content.ContainsInclusive(corner)) { + return false; + } + } + return true; +} + +bool DisplayListMatrixClipState::oval_covers_cull(const DlRect& bounds) const { + if (bounds.IsEmpty()) { + return false; + } + if (cull_rect_.IsEmpty()) { + return true; + } + DlPoint corners[4]; + if (!getLocalCullCorners(corners)) { + return false; + } + DlPoint center = bounds.GetCenter(); + DlSize scale = 2.0 / bounds.GetSize(); + for (auto corner : corners) { + if (!bounds.Contains(corner)) { + return false; + } + if (((corner - center) * scale).GetLengthSquared() >= 1.0) { + return false; + } + } + return true; +} + +bool DisplayListMatrixClipState::rrect_covers_cull( + const SkRRect& content) const { + if (content.isEmpty()) { + return false; + } + if (cull_rect_.IsEmpty()) { + return true; + } + if (content.isRect()) { + return rect_covers_cull(content.getBounds()); + } + if (content.isOval()) { + return oval_covers_cull(content.getBounds()); + } + if (!content.isSimple()) { + return false; + } + DlPoint corners[4]; + if (!getLocalCullCorners(corners)) { + return false; + } + auto outer = content.getBounds(); + DlScalar x_center = outer.centerX(); + DlScalar y_center = outer.centerY(); + auto radii = content.getSimpleRadii(); + DlScalar inner_x = outer.width() * 0.5f - radii.fX; + DlScalar inner_y = outer.height() * 0.5f - radii.fY; + DlScalar scale_x = 1.0 / radii.fX; + DlScalar scale_y = 1.0 / radii.fY; + for (auto corner : corners) { + if (!outer.contains(corner.x, corner.y)) { + return false; + } + DlScalar x_rel = std::abs(corner.x - x_center) - inner_x; + DlScalar y_rel = std::abs(corner.y - y_center) - inner_y; + if (x_rel > 0.0f && y_rel > 0.0f) { + x_rel *= scale_x; + y_rel *= scale_y; + if (x_rel * x_rel + y_rel * y_rel >= 1.0f) { + return false; + } + } + } + return true; +} + +bool DisplayListMatrixClipState::getLocalCullCorners(DlPoint corners[4]) const { + if (!is_matrix_invertable()) { + return false; + } + DlMatrix inverse = matrix_.Invert(); + corners[0] = inverse * cull_rect_.GetLeftTop(); + corners[1] = inverse * cull_rect_.GetRightTop(); + corners[2] = inverse * cull_rect_.GetRightBottom(); + corners[3] = inverse * cull_rect_.GetLeftBottom(); + return true; +} + } // namespace flutter diff --git a/display_list/utils/dl_matrix_clip_tracker.h b/display_list/utils/dl_matrix_clip_tracker.h index cd4d1c582cbbc..59c9880bd3bd7 100644 --- a/display_list/utils/dl_matrix_clip_tracker.h +++ b/display_list/utils/dl_matrix_clip_tracker.h @@ -63,6 +63,16 @@ class DisplayListMatrixClipState { SkRect local_cull_rect() const; SkRect device_cull_rect() const { return ToSkRect(cull_rect_); } + bool rect_covers_cull(const DlRect& content) const; + bool rect_covers_cull(const SkRect& content) const { + return rect_covers_cull(ToDlRect(content)); + } + bool oval_covers_cull(const DlRect& content_bounds) const; + bool oval_covers_cull(const SkRect& content_bounds) const { + return oval_covers_cull(ToDlRect(content_bounds)); + } + bool rrect_covers_cull(const SkRRect& content) const; + bool content_culled(const DlRect& content_bounds) const; bool content_culled(const SkRect& content_bounds) const { return content_culled(ToDlRect(content_bounds)); @@ -146,6 +156,7 @@ class DisplayListMatrixClipState { DlRect cull_rect_; DlMatrix matrix_; + bool getLocalCullCorners(DlPoint corners[4]) const; void adjustCullRect(const DlRect& clip, ClipOp op, bool is_aa); }; diff --git a/display_list/utils/dl_matrix_clip_tracker_unittests.cc b/display_list/utils/dl_matrix_clip_tracker_unittests.cc index d4504fbc05d61..1ac039db126e5 100644 --- a/display_list/utils/dl_matrix_clip_tracker_unittests.cc +++ b/display_list/utils/dl_matrix_clip_tracker_unittests.cc @@ -764,5 +764,179 @@ TEST(DisplayListMatrixClipState, MapAndClipRectScale) { } } +TEST(DisplayListMatrixClipState, RectCoverage) { + DlRect rect = DlRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DisplayListMatrixClipState state(rect); + + EXPECT_TRUE(state.rect_covers_cull(rect)); + EXPECT_TRUE(state.rect_covers_cull(rect.Expand(0.1f, 0.0f, 0.0f, 0.0f))); + EXPECT_TRUE(state.rect_covers_cull(rect.Expand(0.0f, 0.1f, 0.0f, 0.0f))); + EXPECT_TRUE(state.rect_covers_cull(rect.Expand(0.0f, 0.0f, 0.1f, 0.0f))); + EXPECT_TRUE(state.rect_covers_cull(rect.Expand(0.0f, 0.0f, 0.0f, 0.1f))); + EXPECT_FALSE(state.rect_covers_cull(rect.Expand(-0.1f, 0.0f, 0.0f, 0.0f))); + EXPECT_FALSE(state.rect_covers_cull(rect.Expand(0.0f, -0.1f, 0.0f, 0.0f))); + EXPECT_FALSE(state.rect_covers_cull(rect.Expand(0.0f, 0.0f, -0.1f, 0.0f))); + EXPECT_FALSE(state.rect_covers_cull(rect.Expand(0.0f, 0.0f, 0.0f, -0.1f))); +} + +TEST(DisplayListMatrixClipState, RectCoverageUnderScale) { + DlRect rect = DlRect::MakeLTRB(100.0f, 100.0f, 200.0f, 200.0f); + DisplayListMatrixClipState state(rect); + state.scale(2.0f, 2.0f); + + EXPECT_FALSE(state.rect_covers_cull(DlRect::MakeLTRB(100, 100, 200, 200))); + EXPECT_TRUE(state.rect_covers_cull(DlRect::MakeLTRB(50, 50, 100, 100))); + EXPECT_TRUE(state.rect_covers_cull(DlRect::MakeLTRB(49, 50, 100, 100))); + EXPECT_TRUE(state.rect_covers_cull(DlRect::MakeLTRB(50, 49, 100, 100))); + EXPECT_TRUE(state.rect_covers_cull(DlRect::MakeLTRB(50, 50, 101, 100))); + EXPECT_TRUE(state.rect_covers_cull(DlRect::MakeLTRB(50, 50, 100, 101))); + EXPECT_FALSE(state.rect_covers_cull(DlRect::MakeLTRB(51, 50, 100, 100))); + EXPECT_FALSE(state.rect_covers_cull(DlRect::MakeLTRB(50, 51, 100, 100))); + EXPECT_FALSE(state.rect_covers_cull(DlRect::MakeLTRB(50, 50, 99, 100))); + EXPECT_FALSE(state.rect_covers_cull(DlRect::MakeLTRB(50, 50, 100, 99))); +} + +TEST(DisplayListMatrixClipState, RectCoverageUnderRotation) { + DlRect rect = DlRect::MakeLTRB(-1.0f, -1.0f, 1.0f, 1.0f); + DlRect cull = rect.Scale(impeller::kSqrt2 * 25); + DlRect test = rect.Scale(50.0f); + DlRect test_true = test.Expand(0.002f); + DlRect test_false = test.Expand(-0.002f); + + for (int i = 0; i <= 360; i++) { + DisplayListMatrixClipState state(cull); + state.rotate(i); + EXPECT_TRUE(state.rect_covers_cull(test_true)) + << " testing " << test_true << std::endl + << " contains " << state.local_cull_rect() << std::endl + << " at " << i << " degrees"; + if ((i % 90) == 45) { + // The cull rect is largest when viewed at multiples of 45 + // degrees so we will fail to contain it at those angles + EXPECT_FALSE(state.rect_covers_cull(test_false)) + << " testing " << test_false << std::endl + << " contains " << state.local_cull_rect() << std::endl + << " at " << i << " degrees"; + } else { + // At other angles, the cull rect is not quite so big as to encroach + // upon the expanded test rectangle. + EXPECT_TRUE(state.rect_covers_cull(test_false)) + << " testing " << test_false << std::endl + << " contains " << state.local_cull_rect() << std::endl + << " at " << i << " degrees"; + } + } +} + +TEST(DisplayListMatrixClipState, OvalCoverage) { + DlRect cull = DlRect::MakeLTRB(-50.0f, -50.0f, 50.0f, 50.0f); + DisplayListMatrixClipState state(cull); + // The cull rect corners will be at (50, 50) so the oval needs to have + // a radius large enough to cover that - sqrt(2*50*50) == sqrt(2) * 50 + // We pad by an ever so slight 0.02f to account for round off error and + // then use larger expansion/contractions of 0.1f to cover/not-cover it. + DlRect test = cull.Scale(impeller::kSqrt2).Expand(0.02f); + + EXPECT_TRUE(state.oval_covers_cull(test)); + EXPECT_TRUE(state.oval_covers_cull(test.Expand(0.1f, 0.0f, 0.0f, 0.0f))); + EXPECT_TRUE(state.oval_covers_cull(test.Expand(0.0f, 0.1f, 0.0f, 0.0f))); + EXPECT_TRUE(state.oval_covers_cull(test.Expand(0.0f, 0.0f, 0.1f, 0.0f))); + EXPECT_TRUE(state.oval_covers_cull(test.Expand(0.0f, 0.0f, 0.0f, 0.1f))); + EXPECT_FALSE(state.oval_covers_cull(test.Expand(-0.1f, 0.0f, 0.0f, 0.0f))); + EXPECT_FALSE(state.oval_covers_cull(test.Expand(0.0f, -0.1f, 0.0f, 0.0f))); + EXPECT_FALSE(state.oval_covers_cull(test.Expand(0.0f, 0.0f, -0.1f, 0.0f))); + EXPECT_FALSE(state.oval_covers_cull(test.Expand(0.0f, 0.0f, 0.0f, -0.1f))); +} + +TEST(DisplayListMatrixClipState, OvalCoverageUnderScale) { + DlRect cull = DlRect::MakeLTRB(-50.0f, -50.0f, 50.0f, 50.0f); + DisplayListMatrixClipState state(cull); + state.scale(2.0f, 2.0f); + // The cull rect corners will be at (50, 50) so the oval needs to have + // a radius large enough to cover that - sqrt(2*50*50) == sqrt(2) * 50 + // We pad by an ever so slight 0.02f to account for round off error and + // then use larger expansion/contractions of 0.1f to cover/not-cover it. + // We combine that with an additional scale 0.5f since we are viewing + // the cull rect under a 2.0 scale. + DlRect test = cull.Scale(0.5f * impeller::kSqrt2).Expand(0.02f); + + EXPECT_TRUE(state.oval_covers_cull(test)); + EXPECT_TRUE(state.oval_covers_cull(test.Expand(0.1f, 0.0f, 0.0f, 0.0f))); + EXPECT_TRUE(state.oval_covers_cull(test.Expand(0.0f, 0.1f, 0.0f, 0.0f))); + EXPECT_TRUE(state.oval_covers_cull(test.Expand(0.0f, 0.0f, 0.1f, 0.0f))); + EXPECT_TRUE(state.oval_covers_cull(test.Expand(0.0f, 0.0f, 0.0f, 0.1f))); + EXPECT_FALSE(state.oval_covers_cull(test.Expand(-0.1f, 0.0f, 0.0f, 0.0f))); + EXPECT_FALSE(state.oval_covers_cull(test.Expand(0.0f, -0.1f, 0.0f, 0.0f))); + EXPECT_FALSE(state.oval_covers_cull(test.Expand(0.0f, 0.0f, -0.1f, 0.0f))); + EXPECT_FALSE(state.oval_covers_cull(test.Expand(0.0f, 0.0f, 0.0f, -0.1f))); +} + +TEST(DisplayListMatrixClipState, OvalCoverageUnderRotation) { + DlRect unit = DlRect::MakeLTRB(-1.0f, -1.0f, 1.0f, 1.0f); + DlRect cull = unit.Scale(50.0f); + // See above, test bounds need to be sqrt(2) larger for the inscribed + // oval to contain the cull rect. These tests are simpler than the scaled + // rectangle coverage tests because this expanded test oval will + // precisely cover the cull rect at all angles. + DlRect test = cull.Scale(impeller::kSqrt2); + DlRect test_true = test.Expand(0.002f); + DlRect test_false = test.Expand(-0.002f); + + for (int i = 0; i <= 360; i++) { + DisplayListMatrixClipState state(cull); + state.rotate(i); + EXPECT_TRUE(state.oval_covers_cull(test_true)) + << " testing " << test_true << std::endl + << " contains " << state.local_cull_rect() << std::endl + << " at " << i << " degrees"; + EXPECT_FALSE(state.oval_covers_cull(test_false)) + << " testing " << test_false << std::endl + << " contains " << state.local_cull_rect() << std::endl + << " at " << i << " degrees"; + } +} + +TEST(DisplayListMatrixClipState, RRectCoverage) { + SkRect cull = SkRect::MakeLTRB(-50.0f, -50.0f, 50.0f, 50.0f); + DisplayListMatrixClipState state(cull); + // test_bounds need to contain + SkRect test = cull.makeOutset(2.0f, 2.0f); + + // RRect of cull with no corners covers + EXPECT_TRUE(state.rrect_covers_cull(SkRRect::MakeRectXY(cull, 0.0f, 0.0f))); + // RRect of cull with even the tiniest corners does not cover + EXPECT_FALSE( + state.rrect_covers_cull(SkRRect::MakeRectXY(cull, 0.01f, 0.01f))); + + // Expanded by 2.0 and then with a corner of 2.0 obviously still covers + EXPECT_TRUE(state.rrect_covers_cull(SkRRect::MakeRectXY(test, 2.0f, 2.0f))); + // The corner point of the cull rect is at (c-2, c-2) relative to the + // corner of the rrect bounds so we compute its disance to the center + // of the circular part and compare it to the radius of the corner (c) + // to find the corner radius where it will start to leave the rounded + // rectangle: + // + // +----------- + + // | __---^^ | + // | +/------- + | + // | / \ | c + // | /| \ c-2 | + // |/ | \ | | + // || | * + + + // + // sqrt(2*(c-2)*(c-2)) > c + // 2*(c-2)*(c-2) > c*c + // 2*(cc - 4c + 4) > cc + // 2cc - 8c + 8 > cc + // cc - 8c + 8 > 0 + // c > 8 +/- sqrt(64 - 32) / 2 + // c > ~6.828 + // corners set to 6.82 should still cover the cull rect + EXPECT_TRUE(state.rrect_covers_cull(SkRRect::MakeRectXY(test, 6.82f, 6.82f))); + // but corners set to 6.83 should not cover the cull rect + EXPECT_FALSE( + state.rrect_covers_cull(SkRRect::MakeRectXY(test, 6.84f, 6.84f))); +} + } // namespace testing } // namespace flutter diff --git a/flow/layers/backdrop_filter_layer_unittests.cc b/flow/layers/backdrop_filter_layer_unittests.cc index 8dacf8ed3216f..a796bffb2a0cb 100644 --- a/flow/layers/backdrop_filter_layer_unittests.cc +++ b/flow/layers/backdrop_filter_layer_unittests.cc @@ -394,7 +394,7 @@ TEST_F(BackdropFilterLayerTest, OpacityInheritance) { clip->Paint(display_list_paint_context()); - DisplayListBuilder expected_builder(clip_rect); + DisplayListBuilder expected_builder; /* ClipRectLayer::Paint */ { expected_builder.Save(); { diff --git a/impeller/geometry/rect.h b/impeller/geometry/rect.h index 4f7bbba55f7ff..328a911013e56 100644 --- a/impeller/geometry/rect.h +++ b/impeller/geometry/rect.h @@ -226,6 +226,25 @@ struct TRect { p.y < bottom_; } + /// @brief Returns true iff the provided point |p| is inside the + /// closed-range interior of this rectangle. + /// + /// Unlike the regular |Contains(TPoint)| method, this method + /// considers all points along the boundary of the rectangle + /// to be contained within the rectangle - useful for testing + /// if vertices that define a filled shape would carry the + /// interior of that shape outside the bounds of the rectangle. + /// Since both geometries are defining half-open spaces, their + /// defining geometry needs to consider their boundaries to + /// be equivalent with respect to interior and exterior. + [[nodiscard]] constexpr bool ContainsInclusive(const TPoint& p) const { + return !this->IsEmpty() && // + p.x >= left_ && // + p.y >= top_ && // + p.x <= right_ && // + p.y <= bottom_; + } + /// @brief Returns true iff this rectangle is not empty and it also /// contains every point considered inside the provided /// rectangle |o| (as determined by |Contains(TPoint)|). From 5a8a5503656033da93c047eb964e49ed0888196c Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 13 Jun 2024 11:24:14 -0700 Subject: [PATCH 2/2] test case for impeller::Rect::ContainsInclusive --- impeller/geometry/rect_unittests.cc | 169 ++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/impeller/geometry/rect_unittests.cc b/impeller/geometry/rect_unittests.cc index b37ff2668d33a..5634de40854ed 100644 --- a/impeller/geometry/rect_unittests.cc +++ b/impeller/geometry/rect_unittests.cc @@ -2259,6 +2259,175 @@ TEST(RectTest, IRectContainsIPoint) { } } +TEST(RectTest, RectContainsInclusivePoint) { + auto check_nans = [](const Rect& rect, const Point& point, + const std::string& label) { + ASSERT_TRUE(rect.IsFinite()) << label; + ASSERT_TRUE(point.IsFinite()) << label; + + for (int i = 1; i < 16; i++) { + EXPECT_FALSE(swap_nan(rect, i).ContainsInclusive(point)) + << label << ", index = " << i; + for (int j = 1; j < 4; j++) { + EXPECT_FALSE(swap_nan(rect, i).ContainsInclusive(swap_nan(point, j))) + << label << ", indices = " << i << ", " << j; + } + } + }; + + auto check_empty_flips = [](const Rect& rect, const Point& point, + const std::string& label) { + ASSERT_FALSE(rect.IsEmpty()); + + EXPECT_FALSE(flip_lr(rect).ContainsInclusive(point)) << label; + EXPECT_FALSE(flip_tb(rect).ContainsInclusive(point)) << label; + EXPECT_FALSE(flip_lrtb(rect).ContainsInclusive(point)) << label; + }; + + auto test_inside = [&check_nans, &check_empty_flips](const Rect& rect, + const Point& point) { + ASSERT_FALSE(rect.IsEmpty()) << rect; + + std::stringstream stream; + stream << rect << " contains " << point; + auto label = stream.str(); + + EXPECT_TRUE(rect.ContainsInclusive(point)) << label; + check_empty_flips(rect, point, label); + check_nans(rect, point, label); + }; + + auto test_outside = [&check_nans, &check_empty_flips](const Rect& rect, + const Point& point) { + ASSERT_FALSE(rect.IsEmpty()) << rect; + + std::stringstream stream; + stream << rect << " contains " << point; + auto label = stream.str(); + + EXPECT_FALSE(rect.ContainsInclusive(point)) << label; + check_empty_flips(rect, point, label); + check_nans(rect, point, label); + }; + + { + // Origin is inclusive + auto r = Rect::MakeXYWH(100, 100, 100, 100); + auto p = Point(100, 100); + + test_inside(r, p); + } + { + // Size is inclusive + auto r = Rect::MakeXYWH(100, 100, 100, 100); + auto p = Point(200, 200); + + test_inside(r, p); + } + { + // Size + epsilon is exclusive + auto r = Rect::MakeXYWH(100, 100, 100, 100); + auto p = Point(200 + kEhCloseEnough, 200 + kEhCloseEnough); + + test_outside(r, p); + } + { + auto r = Rect::MakeXYWH(100, 100, 100, 100); + auto p = Point(99, 99); + + test_outside(r, p); + } + { + auto r = Rect::MakeXYWH(100, 100, 100, 100); + auto p = Point(199, 199); + + test_inside(r, p); + } + + { + auto r = Rect::MakeMaximum(); + auto p = Point(199, 199); + + test_inside(r, p); + } +} + +TEST(RectTest, IRectContainsInclusiveIPoint) { + auto check_empty_flips = [](const IRect& rect, const IPoint& point, + const std::string& label) { + ASSERT_FALSE(rect.IsEmpty()); + + EXPECT_FALSE(flip_lr(rect).ContainsInclusive(point)) << label; + EXPECT_FALSE(flip_tb(rect).ContainsInclusive(point)) << label; + EXPECT_FALSE(flip_lrtb(rect).ContainsInclusive(point)) << label; + }; + + auto test_inside = [&check_empty_flips](const IRect& rect, + const IPoint& point) { + ASSERT_FALSE(rect.IsEmpty()) << rect; + + std::stringstream stream; + stream << rect << " contains " << point; + auto label = stream.str(); + + EXPECT_TRUE(rect.ContainsInclusive(point)) << label; + check_empty_flips(rect, point, label); + }; + + auto test_outside = [&check_empty_flips](const IRect& rect, + const IPoint& point) { + ASSERT_FALSE(rect.IsEmpty()) << rect; + + std::stringstream stream; + stream << rect << " contains " << point; + auto label = stream.str(); + + EXPECT_FALSE(rect.ContainsInclusive(point)) << label; + check_empty_flips(rect, point, label); + }; + + { + // Origin is inclusive + auto r = IRect::MakeXYWH(100, 100, 100, 100); + auto p = IPoint(100, 100); + + test_inside(r, p); + } + { + // Size is inclusive + auto r = IRect::MakeXYWH(100, 100, 100, 100); + auto p = IPoint(200, 200); + + test_inside(r, p); + } + { + // Size + "epsilon" is exclusive + auto r = IRect::MakeXYWH(100, 100, 100, 100); + auto p = IPoint(201, 201); + + test_outside(r, p); + } + { + auto r = IRect::MakeXYWH(100, 100, 100, 100); + auto p = IPoint(99, 99); + + test_outside(r, p); + } + { + auto r = IRect::MakeXYWH(100, 100, 100, 100); + auto p = IPoint(199, 199); + + test_inside(r, p); + } + + { + auto r = IRect::MakeMaximum(); + auto p = IPoint(199, 199); + + test_inside(r, p); + } +} + TEST(RectTest, RectContainsRect) { auto check_nans = [](const Rect& a, const Rect& b, const std::string& label) { ASSERT_TRUE(a.IsFinite()) << label;