[Engine] Fix an edge case of RoundSuperellipseParam::Contains with sharp corners (#167977)

This PR fixes an edge case where `RoundSuperellipseParam::Contains`
would incorrectly judge a point if the point is outside a sharp corner.

Although this case would have been rejected by the rect filter,
`RSuperellipse.contains` (which is called by `dart:ui`) did not use a
rect filter, causing this bug to affect Dart only. This PR also adds
rect filter to it.

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
Tong Mu 2025-05-22 15:40:54 -07:00 committed by GitHub
parent df3d1bdc3d
commit edabadf4e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 62 additions and 16 deletions

View File

@ -17,9 +17,11 @@ Scalar Split(Scalar left, Scalar right, Scalar ratio_left, Scalar ratio_right) {
return (left * ratio_right + right * ratio_left) / (ratio_left + ratio_right);
}
// Return the same Point, but each NaN coordinate is replaced by 1.
inline Point ReplanceNaNWithOne(Point in) {
return Point{std::isnan(in.x) ? 1 : in.x, std::isnan(in.y) ? 1 : in.y};
// Return the same Point, but each NaN coordinate is replaced by that of
// `default_size`.
inline Point ReplanceNaNWithDefault(Point in, Size default_size) {
return Point{std::isnan(in.x) ? default_size.width : in.x,
std::isnan(in.y) ? default_size.height : in.y};
}
// Swap the x and y coordinate of a point.
@ -192,11 +194,15 @@ RoundSuperellipseParam::Octant ComputeOctant(Point center,
// radii.
//
// The `corner` is the coordinate of the corner point in the same coordinate
// space as `center`, which specifies both the half size of the bounding box and
// which quadrant the curve should be.
// space as `center`, which specifies the half size of the bounding box.
//
// The `sign` is a vector of {±1, ±1} that specifies which quadrant the curve
// should be, which should have the same sign as `corner - center` except that
// the latter may have a 0.
RoundSuperellipseParam::Quadrant ComputeQuadrant(Point center,
Point corner,
Size in_radii) {
Size in_radii,
Size sign) {
Point corner_vector = corner - center;
Size radii = in_radii.Abs();
@ -207,7 +213,8 @@ RoundSuperellipseParam::Quadrant ComputeQuadrant(Point center,
Scalar norm_radius = radii.MinDimension();
Size forward_scale = norm_radius == 0 ? Size{1, 1} : radii / norm_radius;
Point norm_half_size = corner_vector.Abs() / forward_scale;
Point signed_scale = ReplanceNaNWithOne(corner_vector / norm_half_size);
Point signed_scale =
ReplanceNaNWithDefault(corner_vector / norm_half_size, sign);
// Each quadrant curve is composed of two octant curves, each of which belongs
// to a square-like rounded rectangle. For the two octants to connect at the
@ -469,7 +476,7 @@ RoundSuperellipseParam RoundSuperellipseParam::MakeBoundsRadii(
// treatment on border containment and therefore is not `all_corners_same`.
return RoundSuperellipseParam{
.top_right = ComputeQuadrant(bounds.GetCenter(), bounds.GetRightTop(),
radii.top_right),
radii.top_right, {-1, 1}),
.all_corners_same = true,
};
}
@ -484,15 +491,18 @@ RoundSuperellipseParam RoundSuperellipseParam::MakeBoundsRadii(
radii.top_left.height, radii.bottom_left.height);
return RoundSuperellipseParam{
.top_right = ComputeQuadrant(Point{top_split, right_split},
bounds.GetRightTop(), radii.top_right),
.top_right =
ComputeQuadrant(Point{top_split, right_split}, bounds.GetRightTop(),
radii.top_right, {1, -1}),
.bottom_right =
ComputeQuadrant(Point{bottom_split, right_split},
bounds.GetRightBottom(), radii.bottom_right),
.bottom_left = ComputeQuadrant(Point{bottom_split, left_split},
bounds.GetLeftBottom(), radii.bottom_left),
.top_left = ComputeQuadrant(Point{top_split, left_split},
bounds.GetLeftTop(), radii.top_left),
bounds.GetRightBottom(), radii.bottom_right, {1, 1}),
.bottom_left =
ComputeQuadrant(Point{bottom_split, left_split},
bounds.GetLeftBottom(), radii.bottom_left, {-1, 1}),
.top_left =
ComputeQuadrant(Point{top_split, left_split}, bounds.GetLeftTop(),
radii.top_left, {-1, -1}),
.all_corners_same = false,
};
}

View File

@ -5,6 +5,7 @@
#include "gtest/gtest.h"
#include "flutter/impeller/geometry/round_superellipse.h"
#include "flutter/impeller/geometry/round_superellipse_param.h"
#include "flutter/impeller/geometry/geometry_asserts.h"
@ -692,5 +693,22 @@ TEST(RoundSuperellipseTest, SlimDiagnalContains) {
#undef CHECK_POINT_AND_MIRRORS
}
TEST(RoundSuperellipseTest, PointsOutsideOfSharpCorner) {
Rect bounds = Rect::MakeLTRB(196.0f, 0.0f, 294.0f, 28.0f);
// Regression test for a case where RoundSuperellipseParam::Contains
// previously failed. Although the bounding rect filter of
// `RoundSuperellipse::Contains` would reject this point, this test ensures
// the internal logic of RoundSuperellipseParam::Contains is now correct.
auto rr = RoundSuperellipseParam::MakeBoundsRadii(
bounds, {
.top_left = Size(0.0, 0.0),
.top_right = Size(3.0, 3.0),
.bottom_left = Size(0.0, 0.0),
.bottom_right = Size(3.0, 3.0),
});
EXPECT_FALSE(rr.Contains(Point{147.0, 14.0}));
}
} // namespace testing
} // namespace impeller

View File

@ -81,7 +81,11 @@ impeller::RoundSuperellipseParam RSuperellipse::param() const {
}
bool RSuperellipse::contains(double x, double y) {
return param().Contains(DlPoint(SafeNarrow(x), SafeNarrow(y)));
DlPoint point(SafeNarrow(x), SafeNarrow(y));
if (!bounds_.Contains(point)) {
return false;
}
return param().Contains(point);
}
} // namespace flutter

View File

@ -704,6 +704,20 @@ void main() {
checkDiagnalPoints(const Offset(21.05, -20.92));
checkDiagnalPoints(const Offset(40.0, 5.68));
});
test('RSuperellipse.contains is correct for points outside of a sharp corner', () {
expect(
RSuperellipse.fromLTRBAndCorners(
196.0,
0.0,
294.0,
28.0,
topRight: const Radius.circular(3.0),
bottomRight: const Radius.circular(3.0),
).contains(const Offset(147.0, 14.0)),
isFalse,
);
});
}
void checkPointWithOffset(RSuperellipse rse, Offset inPoint, Offset outwardOffset) {