From de3dedaa7448960ef0bcf5f18c747bd707d71f12 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 5 Aug 2025 16:53:14 -0700 Subject: [PATCH] Add radius clamping to web `RSuperellipse` (#172254) This PR fixes rendering errors on Web when the provided corner radii sum up larger than the size. It implements radius scaling using the same algorithm as in [the C++ implementation](https://github.com/flutter/flutter/blob/b2d4210b3795413c2360968b685743a6df60ff50/engine/src/flutter/impeller/geometry/rounding_radii.cc). Before: (error emerges for r>100, since the height is 200) image After: (it stays this way for r>100) image It also fixes a bug that uses an incorrect starting point. Both changes are backed by the new test cases in `rounded_superellipse_border_test.dart`. ## 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]. [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 --- .../src/flutter/lib/web_ui/lib/geometry.dart | 91 ++++++++++++++++++- .../lib/web_ui/lib/rsuperellipse_param.dart | 9 +- .../rounded_superellipse_border_test.dart | 24 +++++ 3 files changed, 119 insertions(+), 5 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/geometry.dart b/engine/src/flutter/lib/web_ui/lib/geometry.dart index ea4140e167a..d0d50e290f7 100644 --- a/engine/src/flutter/lib/web_ui/lib/geometry.dart +++ b/engine/src/flutter/lib/web_ui/lib/geometry.dart @@ -1164,6 +1164,18 @@ class RSuperellipse extends _RRectLike { @override final bool _uniformRadii; + static (double, double) _normalizeEmptyToZero(double inputX, double inputY) { + return (inputX > 0 && inputY > 0) ? (inputX, inputY) : (0, 0); + } + + static double _adjustScale(double radius1, double radius2, double dimension, double scale) { + assert(radius1 >= 0.0 && radius2 >= 0.0 && dimension > 0.0); + if (radius1 + radius2 > dimension) { + return math.min(scale, dimension / (radius1 + radius2)); + } + return scale; + } + /// (Web only) Returns a [Path] for this shape and an [Offset] for its /// placement. /// @@ -1180,9 +1192,9 @@ class RSuperellipse extends _RRectLike { /// `path` and the `offset` to the `addPath` method. (Path, Offset) toPathOffset() { if (_uniformRadii) { - return (_RSuperellipseCache.instance.get(width, height, tlRadius), center); + return (_RSuperellipseCache.instance.get(width, height, _scaledUniformRadii()), center); } else { - return (_RSuperellipsePathBuilder.exact(this).path, Offset.zero); + return (_RSuperellipsePathBuilder.exact(_toScaledRadii()).path, Offset.zero); } } @@ -1211,6 +1223,81 @@ class RSuperellipse extends _RRectLike { ); } + /// Returns a [RSuperellipse] whose corner radii are scaled based on this one, + /// ensuring that the sum of the corner radii on each side does not exceed the + /// width or height of the given bounds. + /// + /// See the [Skia scaling + /// implementation](https://github.com/google/skia/blob/main/src/core/SkRRect.cpp) + /// for more details. + RSuperellipse _toScaledRadii() { + if (!(width > 0 && height > 0)) { + return RSuperellipse.fromLTRBXY(left, top, right, bottom, 0.0, 0.0); + } + + // If any corner is flat or has a negative value, normalize it to zeros + // We do this first so that the unnecessary non-flat part of that radius + // does not contribute to the global scaling below. + final (double tlRadiusX, double tlRadiusY) = _normalizeEmptyToZero( + this.tlRadiusX, + this.tlRadiusY, + ); + final (double trRadiusX, double trRadiusY) = _normalizeEmptyToZero( + this.trRadiusX, + this.trRadiusY, + ); + final (double blRadiusX, double blRadiusY) = _normalizeEmptyToZero( + this.blRadiusX, + this.blRadiusY, + ); + final (double brRadiusX, double brRadiusY) = _normalizeEmptyToZero( + this.brRadiusX, + this.brRadiusY, + ); + + // Now determine a global scale to apply to all of the radii to ensure + // that none of the adjacent pairs of radius values sum to larger than + // the corresponding dimension of the rectangle. + double scale = 1.0; + scale = _adjustScale(tlRadiusX, trRadiusX, width, scale); + scale = _adjustScale(blRadiusX, brRadiusX, width, scale); + scale = _adjustScale(tlRadiusY, blRadiusY, height, scale); + scale = _adjustScale(trRadiusY, brRadiusY, height, scale); + if (scale < 1.0) { + return _create( + left: left, + top: top, + right: right, + bottom: bottom, + tlRadiusX: tlRadiusX * scale, + tlRadiusY: tlRadiusY * scale, + trRadiusX: trRadiusX * scale, + trRadiusY: trRadiusY * scale, + brRadiusX: brRadiusX * scale, + brRadiusY: brRadiusY * scale, + blRadiusX: blRadiusX * scale, + blRadiusY: blRadiusY * scale, + uniformRadii: _uniformRadii, + ); + } else { + return this; + } + } + + // A variation of `_toScaledRadii` that deals with uniform radii and returns a + // `Radius`. + Radius _scaledUniformRadii() { + assert(_uniformRadii); + if (!(width > 0 && height > 0)) { + return Radius.zero; + } + final (double radiusX, double radiusY) = _normalizeEmptyToZero(tlRadiusX, tlRadiusY); + double scale = 1.0; + scale = _adjustScale(radiusX, radiusX, width, scale); + scale = _adjustScale(radiusY, radiusY, height, scale); + return Radius.elliptical(radiusX * scale, radiusY * scale); + } + static const RSuperellipse zero = RSuperellipse._raw(); bool contains(Offset point) { diff --git a/engine/src/flutter/lib/web_ui/lib/rsuperellipse_param.dart b/engine/src/flutter/lib/web_ui/lib/rsuperellipse_param.dart index d376e952b92..5c7009670ac 100644 --- a/engine/src/flutter/lib/web_ui/lib/rsuperellipse_param.dart +++ b/engine/src/flutter/lib/web_ui/lib/rsuperellipse_param.dart @@ -336,6 +336,8 @@ class _RSuperellipseQuadrant { final _RSuperellipseOctant top; final _RSuperellipseOctant right; + bool get isSharpCorner => top.seN < 2 || right.seN < 2; + void addToPath( _RSuperellipsePath path, { required bool reverse, @@ -345,7 +347,7 @@ class _RSuperellipseQuadrant { _Transform.makeTranslate(offset), _Transform.makeScale(signedScale.scale(extraScale.width, extraScale.height)), ); - if (top.seN < 2 || right.seN < 2) { + if (isSharpCorner) { if (!reverse) { final _Transform transformOctant = _Transform.makeComposite( transform, @@ -406,13 +408,14 @@ class _RSuperellipsePathBuilder { // Build a path for an RSuperellipse with arbitrary position and radii. _RSuperellipsePathBuilder.exact(RSuperellipse r) : path = Path() { final _RSuperellipsePath p = _RSuperellipsePath(path); - final Offset start = Offset((r.left + r.right) / 2, r.top); - path.moveTo(start.dx, start.dy); final double topSplit = _split(r.left, r.right, r.tlRadiusX, r.trRadiusX); final double rightSplit = _split(r.top, r.bottom, r.trRadiusY, r.brRadiusY); final double bottomSplit = _split(r.left, r.right, r.blRadiusX, r.brRadiusX); final double leftSplit = _split(r.top, r.bottom, r.tlRadiusY, r.blRadiusY); + + final Offset start = Offset(topSplit, r.top); + path.moveTo(start.dx, start.dy); _RSuperellipseQuadrant( Offset(topSplit, rightSplit), Offset(r.right, r.top), diff --git a/packages/flutter/test/painting/rounded_superellipse_border_test.dart b/packages/flutter/test/painting/rounded_superellipse_border_test.dart index 9c051a8b025..1628c508f15 100644 --- a/packages/flutter/test/painting/rounded_superellipse_border_test.dart +++ b/packages/flutter/test/painting/rounded_superellipse_border_test.dart @@ -253,6 +253,30 @@ void main() { matchesGoldenFile('painting.rounded_superellipse_border.all_elliptical.png'), ); + await tester.pumpWidget( + containerWithBorder(const Size(120, 300), const BorderRadius.all(Radius.circular(600))), + ); + await expectLater( + find.byType(Container), + matchesGoldenFile('painting.rounded_superellipse_border.clamping_uniform.png'), + ); + + await tester.pumpWidget( + containerWithBorder( + const Size(120, 300), + const BorderRadius.only( + topLeft: Radius.elliptical(1000, 1000), + topRight: Radius.elliptical(0, 1000), + bottomRight: Radius.elliptical(800, 1000), + bottomLeft: Radius.elliptical(100, 500), + ), + ), + ); + await expectLater( + find.byType(Container), + matchesGoldenFile('painting.rounded_superellipse_border.clamping_non_uniform.png'), + ); + // Regression test for https://github.com/flutter/flutter/issues/170593 await tester.pumpWidget( containerWithBorder(