Skip to content

Commit c953c83

Browse files
authored
[Impeller] switch Rect fields to LTRB implementation (flutter#49816)
Switches the Rect class implementation to an LTRB (Left, Top, Right, Bottom) set of fields for efficiency in performing rectangle operations. Additionally, the methods have been somewhat hardened to the following issues: - NaN values should always act as if the rect is empty - Helps with flutter#132770 - Saturated math methods are added to avoid overflow for integer rects - Normalized treatment of empty rectangles in rect/rect operations - empty.Union(anything) == anything - empty.Intersection(anything) == empty - empty.IntersectsWith(anything) == false - empty.Contains(anything) == false - non-empty.Contains(empty) == true - MakeMaximum now returns finite rectangles which should produce non-nan values from all getters - A lot more testing of all of these behaviors within the unit tests In addition, the new rectangle is closer to what DisplayList and the Skia backend have been using all along so we might be able to switch to using Impeller geometry inside DisplayList to reduce overhead for both backends.
1 parent ebee709 commit c953c83

File tree

12 files changed

+4072
-771
lines changed

12 files changed

+4072
-771
lines changed

ci/licenses_golden/excluded_files

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@
161161
../../../flutter/impeller/geometry/matrix_unittests.cc
162162
../../../flutter/impeller/geometry/path_unittests.cc
163163
../../../flutter/impeller/geometry/rect_unittests.cc
164+
../../../flutter/impeller/geometry/saturated_math_unittests.cc
164165
../../../flutter/impeller/geometry/size_unittests.cc
165166
../../../flutter/impeller/geometry/trig_unittests.cc
166167
../../../flutter/impeller/golden_tests/README.md

ci/licenses_golden/licenses_flutter

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5292,6 +5292,7 @@ ORIGIN: ../../../flutter/impeller/geometry/quaternion.cc + ../../../flutter/LICE
52925292
ORIGIN: ../../../flutter/impeller/geometry/quaternion.h + ../../../flutter/LICENSE
52935293
ORIGIN: ../../../flutter/impeller/geometry/rect.cc + ../../../flutter/LICENSE
52945294
ORIGIN: ../../../flutter/impeller/geometry/rect.h + ../../../flutter/LICENSE
5295+
ORIGIN: ../../../flutter/impeller/geometry/saturated_math.h + ../../../flutter/LICENSE
52955296
ORIGIN: ../../../flutter/impeller/geometry/scalar.h + ../../../flutter/LICENSE
52965297
ORIGIN: ../../../flutter/impeller/geometry/shear.cc + ../../../flutter/LICENSE
52975298
ORIGIN: ../../../flutter/impeller/geometry/shear.h + ../../../flutter/LICENSE
@@ -8129,6 +8130,7 @@ FILE: ../../../flutter/impeller/geometry/quaternion.cc
81298130
FILE: ../../../flutter/impeller/geometry/quaternion.h
81308131
FILE: ../../../flutter/impeller/geometry/rect.cc
81318132
FILE: ../../../flutter/impeller/geometry/rect.h
8133+
FILE: ../../../flutter/impeller/geometry/saturated_math.h
81328134
FILE: ../../../flutter/impeller/geometry/scalar.h
81338135
FILE: ../../../flutter/impeller/geometry/shear.cc
81348136
FILE: ../../../flutter/impeller/geometry/shear.h

impeller/entity/geometry/stroke_path_geometry.cc

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,6 @@ std::optional<Rect> StrokePathGeometry::GetCoverage(
508508
if (!path_bounds.has_value()) {
509509
return std::nullopt;
510510
}
511-
auto path_coverage = path_bounds->TransformBounds(transform);
512511

513512
Scalar max_radius = 0.5;
514513
if (stroke_cap_ == Cap::kSquare) {
@@ -522,12 +521,8 @@ std::optional<Rect> StrokePathGeometry::GetCoverage(
522521
return std::nullopt;
523522
}
524523
Scalar min_size = 1.0f / sqrt(std::abs(determinant));
525-
Vector2 max_radius_xy =
526-
transform
527-
.TransformDirection(Vector2(max_radius, max_radius) *
528-
std::max(stroke_width_, min_size))
529-
.Abs();
530-
return path_coverage.Expand(max_radius_xy);
524+
max_radius *= std::max(stroke_width_, min_size);
525+
return path_bounds->Expand(max_radius).TransformBounds(transform);
531526
}
532527

533528
} // namespace impeller

impeller/geometry/BUILD.gn

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ impeller_component("geometry") {
2929
"quaternion.h",
3030
"rect.cc",
3131
"rect.h",
32+
"saturated_math.h",
3233
"scalar.h",
3334
"shear.cc",
3435
"shear.h",
@@ -66,6 +67,7 @@ impeller_component("geometry_unittests") {
6667
"matrix_unittests.cc",
6768
"path_unittests.cc",
6869
"rect_unittests.cc",
70+
"saturated_math_unittests.cc",
6971
"size_unittests.cc",
7072
"trig_unittests.cc",
7173
]

impeller/geometry/geometry_asserts.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ inline ::testing::AssertionResult QuaternionNear(impeller::Quaternion a,
5454
}
5555

5656
inline ::testing::AssertionResult RectNear(impeller::Rect a, impeller::Rect b) {
57-
auto equal = NumberNear(a.GetX(), b.GetX()) &&
58-
NumberNear(a.GetY(), b.GetY()) &&
59-
NumberNear(a.GetWidth(), b.GetWidth()) &&
60-
NumberNear(a.GetHeight(), b.GetHeight());
57+
auto equal = NumberNear(a.GetLeft(), b.GetLeft()) &&
58+
NumberNear(a.GetTop(), b.GetTop()) &&
59+
NumberNear(a.GetRight(), b.GetRight()) &&
60+
NumberNear(a.GetBottom(), b.GetBottom());
6161

6262
return equal ? ::testing::AssertionSuccess()
6363
: ::testing::AssertionFailure()

0 commit comments

Comments
 (0)