diff --git a/engine/src/flutter/display_list/display_list_unittests.cc b/engine/src/flutter/display_list/display_list_unittests.cc index 9794eab650..5c9397d4cc 100644 --- a/engine/src/flutter/display_list/display_list_unittests.cc +++ b/engine/src/flutter/display_list/display_list_unittests.cc @@ -4420,7 +4420,9 @@ class ClipExpector : public virtual DlOpReceiver, template void check(T shape, ClipOp clip_op, bool is_aa) { - ASSERT_LT(index_, clip_expectations_.size()) << label(); + ASSERT_LT(index_, clip_expectations_.size()) + << label() << std::endl + << "extra clip shape = " << shape; auto expected = clip_expectations_[index_]; EXPECT_EQ(expected.clip_op, clip_op) << label(); EXPECT_EQ(expected.is_aa, is_aa) << label(); @@ -4443,6 +4445,30 @@ class ClipExpector : public virtual DlOpReceiver, } }; +TEST_F(DisplayListTest, ClipRectCullingPixel6a) { + // These particular values create bit errors if we use the path that + // tests for inclusion in local space, but work OK if we use a forward + // path that tests for inclusion in device space, due to the fact that + // the extra matrix inversion is just enough math to cause the transform + // to place the local space cull corners just outside the original rect. + // The test in device space only works under a simple scale, such as we + // use for DPR adjustments (and which are not always inversion friendly). + + auto frame = SkRect::MakeLTRB(0.0f, 0.0f, 1080.0f, 2400.0f); + DlScalar DPR = 2.625f; + auto clip = SkRect::MakeLTRB(0.0f, 0.0f, 1080.0f / DPR, 2400.0f / DPR); + + DisplayListBuilder cull_builder; + cull_builder.ClipRect(frame, ClipOp::kIntersect, false); + cull_builder.Scale(DPR, DPR); + cull_builder.ClipRect(clip, ClipOp::kIntersect, false); + auto cull_dl = cull_builder.Build(); + + CLIP_EXPECTOR(expector); + expector.addExpectation(frame, ClipOp::kIntersect, false); + cull_dl->Dispatch(expector); +} + TEST_F(DisplayListTest, ClipRectCulling) { auto clip = SkRect::MakeLTRB(10.0f, 10.0f, 20.0f, 20.0f); diff --git a/engine/src/flutter/display_list/utils/dl_matrix_clip_tracker.cc b/engine/src/flutter/display_list/utils/dl_matrix_clip_tracker.cc index a5579f570b..748cf07a83 100644 --- a/engine/src/flutter/display_list/utils/dl_matrix_clip_tracker.cc +++ b/engine/src/flutter/display_list/utils/dl_matrix_clip_tracker.cc @@ -239,6 +239,12 @@ bool DisplayListMatrixClipState::rect_covers_cull(const DlRect& content) const { if (cull_rect_.IsEmpty()) { return true; } + if (matrix_.IsAligned2D()) { + // This transform-to-device calculation is faster and more accurate + // for rect-to-rect aligned transformations, but not accurate under + // (non-quadrant) rotations and skews. + return content.TransformAndClipBounds(matrix_).Contains(cull_rect_); + } DlPoint corners[4]; if (!getLocalCullCorners(corners)) { return false; diff --git a/engine/src/flutter/display_list/utils/dl_matrix_clip_tracker_unittests.cc b/engine/src/flutter/display_list/utils/dl_matrix_clip_tracker_unittests.cc index 1ac039db12..7a76a99a62 100644 --- a/engine/src/flutter/display_list/utils/dl_matrix_clip_tracker_unittests.cc +++ b/engine/src/flutter/display_list/utils/dl_matrix_clip_tracker_unittests.cc @@ -779,6 +779,33 @@ TEST(DisplayListMatrixClipState, RectCoverage) { EXPECT_FALSE(state.rect_covers_cull(rect.Expand(0.0f, 0.0f, 0.0f, -0.1f))); } +TEST(DisplayListMatrixClipState, RectCoverageAccuracy) { + // These particular values create bit errors if we use the path that + // tests for inclusion in local space, but work OK if we use a forward + // path that tests for inclusion in device space, due to the fact that + // the extra matrix inversion is just enough math to cause the transform + // to place the local space cull corners just outside the original rect. + // The test in device space only works under a simple scale, such as we + // use for DPR adjustments (and which are not always inversion friendly). + + DlRect cull = DlRect::MakeLTRB(0.0f, 0.0f, 1080.0f, 2400.0f); + DlScalar DPR = 2.625; + DlRect rect = DlRect::MakeLTRB(0.0f, 0.0f, 1080.0f / DPR, 2400.0f / DPR); + + DisplayListMatrixClipState state(cull); + state.scale(DPR, DPR); + + 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);