firka/flutter
firka/flutter
4
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Better clipRect culling accuracy under scaling transforms (flutter/engine#53508)

Browse Source 
The clipRect culling in DisplayListBuilder can sometimes fail to cull a clip due to round-off error in the math and this case happens to trigger when trying to cull against the initial frame bounds of a Pixel 6a with its uncommon 2.625 DPR as a scaling transform. An optimization in the culling tests for axis-aligned transforms (very common in Flutter) happens to do the right thing for this odd case, likely due to less math to incur round-off errors.
This commit is contained in:
Jim Graham 2024-06-21 14:45:14 -07:00 committed by GitHub
parent c77583f85e
commit 3bd63a65bb
3 changed files with 60 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
engine/src/flutter/display_list/display_list_unittests.cc
View File

@ -4420,7 +4420,9 @@ class ClipExpector : public virtual DlOpReceiver,
template <typename T>
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);

6
engine/src/flutter/display_list/utils/dl_matrix_clip_tracker.cc
View File

@ -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;

27
engine/src/flutter/display_list/utils/dl_matrix_clip_tracker_unittests.cc
View File

@ -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);