Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Clamp RRect radii when deflating, assert on negative radii #36062

Merged
auto-submit merged 10 commits into from
Sep 15, 2022
Merged

Clamp RRect radii when deflating, assert on negative radii #36062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c946993
Clamp RRect radii when deflating
gspencergoog Sep 9, 2022
347fc64
Fix test
gspencergoog Sep 10, 2022
bf45418
Correctly test assertion throwing
gspencergoog Sep 12, 2022
c1c586e
revert to other method of assertion detection
gspencergoog Sep 12, 2022
b06f132
Check for enabled assertions
gspencergoog Sep 12, 2022
3ce047b
Add to web_ui too
gspencergoog Sep 12, 2022
94e2887
Fix recording canvas to not expect negative values anymore
gspencergoog Sep 12, 2022
018367c
Fix recording canvas to not expect negative values anymore
gspencergoog Sep 12, 2022
8bfd7fe
Add missing reference to math.dart
gspencergoog Sep 13, 2022
c52f063
Mention in public constructors that negative values assert.
gspencergoog Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/ui/dart_ui.gni
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ dart_ui_files = [
"//flutter/lib/ui/isolate_name_server.dart",
"//flutter/lib/ui/key.dart",
"//flutter/lib/ui/lerp.dart",
"//flutter/lib/ui/math.dart",
Copy link
Contributor Author

@gspencergoog gspencergoog Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was added in #36106, but I forgot to add it here. To be honest, I have no idea why it didn't blow up the framework build, but it didn't. Anyhow, adding it in now so that we can finish moving clampDouble into dart:ui.

"//flutter/lib/ui/natives.dart",
"//flutter/lib/ui/painting.dart",
"//flutter/lib/ui/platform_dispatcher.dart",
Expand Down
108 changes: 69 additions & 39 deletions lib/ui/geometry.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,8 +1087,16 @@ class Radius {
class RRect {
/// Construct a rounded rectangle from its left, top, right, and bottom edges,
/// and the same radii along its horizontal axis and its vertical axis.
const RRect.fromLTRBXY(double left, double top, double right, double bottom,
double radiusX, double radiusY) : this._raw(
///
/// Will assert in debug mode if `radiusX` or `radiusY` are negative.
const RRect.fromLTRBXY(
double left,
double top,
double right,
double bottom,
double radiusX,
double radiusY,
) : this._raw(
top: top,
left: left,
right: right,
Expand All @@ -1105,8 +1113,15 @@ class RRect {

/// Construct a rounded rectangle from its left, top, right, and bottom edges,
/// and the same radius in each corner.
RRect.fromLTRBR(double left, double top, double right, double bottom,
Radius radius)
///
/// Will assert in debug mode if the `radius` is negative in either x or y.
RRect.fromLTRBR(
double left,
double top,
double right,
double bottom,
Radius radius,
)
: this._raw(
top: top,
left: left,
Expand All @@ -1124,6 +1139,8 @@ class RRect {

/// Construct a rounded rectangle from its bounding box and the same radii
/// along its horizontal axis and its vertical axis.
///
/// Will assert in debug mode if `radiusX` or `radiusY` are negative.
RRect.fromRectXY(Rect rect, double radiusX, double radiusY)
: this._raw(
top: rect.top,
Expand All @@ -1142,6 +1159,8 @@ class RRect {

/// Construct a rounded rectangle from its bounding box and a radius that is
/// the same in each corner.
///
/// Will assert in debug mode if the `radius` is negative in either x or y.
RRect.fromRectAndRadius(Rect rect, Radius radius)
: this._raw(
top: rect.top,
Expand All @@ -1161,7 +1180,8 @@ class RRect {
/// Construct a rounded rectangle from its left, top, right, and bottom edges,
/// and topLeft, topRight, bottomRight, and bottomLeft radii.
///
/// The corner radii default to [Radius.zero], i.e. right-angled corners.
/// The corner radii default to [Radius.zero], i.e. right-angled corners. Will
/// assert in debug mode if any of the radii are negative in either x or y.
RRect.fromLTRBAndCorners(
double left,
double top,
Expand Down Expand Up @@ -1189,7 +1209,8 @@ class RRect {
/// Construct a rounded rectangle from its bounding box and and topLeft,
/// topRight, bottomRight, and bottomLeft radii.
///
/// The corner radii default to [Radius.zero], i.e. right-angled corners
/// The corner radii default to [Radius.zero], i.e. right-angled corners. Will
/// assert in debug mode if any of the radii are negative in either x or y.
RRect.fromRectAndCorners(
Rect rect,
{
Expand Down Expand Up @@ -1237,7 +1258,15 @@ class RRect {
assert(brRadiusX != null),
assert(brRadiusY != null),
assert(blRadiusX != null),
assert(blRadiusY != null);
assert(blRadiusY != null),
assert(tlRadiusX >= 0),
Copy link
Member

@goderbauer goderbauer Sep 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mention this in the docs for the public constructors that feed into this?

Copy link
Contributor Author

@gspencergoog gspencergoog Sep 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

assert(tlRadiusY >= 0),
assert(trRadiusX >= 0),
assert(trRadiusY >= 0),
assert(brRadiusX >= 0),
assert(brRadiusY >= 0),
assert(blRadiusX >= 0),
assert(blRadiusY >= 0);

Float32List _getValue32() {
final Float32List result = Float32List(12);
Expand Down Expand Up @@ -1333,14 +1362,14 @@ class RRect {
top: top - delta,
right: right + delta,
bottom: bottom + delta,
tlRadiusX: tlRadiusX + delta,
tlRadiusY: tlRadiusY + delta,
trRadiusX: trRadiusX + delta,
trRadiusY: trRadiusY + delta,
blRadiusX: blRadiusX + delta,
blRadiusY: blRadiusY + delta,
brRadiusX: brRadiusX + delta,
brRadiusY: brRadiusY + delta,
tlRadiusX: math.max(0, tlRadiusX + delta),
tlRadiusY: math.max(0, tlRadiusY + delta),
trRadiusX: math.max(0, trRadiusX + delta),
trRadiusY: math.max(0, trRadiusY + delta),
blRadiusX: math.max(0, blRadiusX + delta),
blRadiusY: math.max(0, blRadiusY + delta),
brRadiusX: math.max(0, brRadiusX + delta),
brRadiusY: math.max(0, brRadiusY + delta),
);
}

Expand Down Expand Up @@ -1503,6 +1532,7 @@ class RRect {
scale = _getMin(scale, tlRadiusX, trRadiusX, width);
scale = _getMin(scale, trRadiusY, brRadiusY, height);
scale = _getMin(scale, brRadiusX, blRadiusX, width);
assert(scale >= 0);

if (scale < 1.0) {
return RRect._raw(
Expand Down Expand Up @@ -1621,14 +1651,14 @@ class RRect {
top: a.top * k,
right: a.right * k,
bottom: a.bottom * k,
tlRadiusX: a.tlRadiusX * k,
tlRadiusY: a.tlRadiusY * k,
trRadiusX: a.trRadiusX * k,
trRadiusY: a.trRadiusY * k,
brRadiusX: a.brRadiusX * k,
brRadiusY: a.brRadiusY * k,
blRadiusX: a.blRadiusX * k,
blRadiusY: a.blRadiusY * k,
tlRadiusX: math.max(0, a.tlRadiusX * k),
tlRadiusY: math.max(0, a.tlRadiusY * k),
trRadiusX: math.max(0, a.trRadiusX * k),
trRadiusY: math.max(0, a.trRadiusY * k),
brRadiusX: math.max(0, a.brRadiusX * k),
brRadiusY: math.max(0, a.brRadiusY * k),
blRadiusX: math.max(0, a.blRadiusX * k),
blRadiusY: math.max(0, a.blRadiusY * k),
);
}
} else {
Expand All @@ -1638,29 +1668,29 @@ class RRect {
top: b.top * t,
right: b.right * t,
bottom: b.bottom * t,
tlRadiusX: b.tlRadiusX * t,
tlRadiusY: b.tlRadiusY * t,
trRadiusX: b.trRadiusX * t,
trRadiusY: b.trRadiusY * t,
brRadiusX: b.brRadiusX * t,
brRadiusY: b.brRadiusY * t,
blRadiusX: b.blRadiusX * t,
blRadiusY: b.blRadiusY * t,
tlRadiusX: math.max(0, b.tlRadiusX * t),
tlRadiusY: math.max(0, b.tlRadiusY * t),
trRadiusX: math.max(0, b.trRadiusX * t),
trRadiusY: math.max(0, b.trRadiusY * t),
brRadiusX: math.max(0, b.brRadiusX * t),
brRadiusY: math.max(0, b.brRadiusY * t),
blRadiusX: math.max(0, b.blRadiusX * t),
blRadiusY: math.max(0, b.blRadiusY * t),
);
} else {
return RRect._raw(
left: _lerpDouble(a.left, b.left, t),
top: _lerpDouble(a.top, b.top, t),
right: _lerpDouble(a.right, b.right, t),
bottom: _lerpDouble(a.bottom, b.bottom, t),
tlRadiusX: _lerpDouble(a.tlRadiusX, b.tlRadiusX, t),
tlRadiusY: _lerpDouble(a.tlRadiusY, b.tlRadiusY, t),
trRadiusX: _lerpDouble(a.trRadiusX, b.trRadiusX, t),
trRadiusY: _lerpDouble(a.trRadiusY, b.trRadiusY, t),
brRadiusX: _lerpDouble(a.brRadiusX, b.brRadiusX, t),
brRadiusY: _lerpDouble(a.brRadiusY, b.brRadiusY, t),
blRadiusX: _lerpDouble(a.blRadiusX, b.blRadiusX, t),
blRadiusY: _lerpDouble(a.blRadiusY, b.blRadiusY, t),
tlRadiusX: math.max(0, _lerpDouble(a.tlRadiusX, b.tlRadiusX, t)),
tlRadiusY: math.max(0, _lerpDouble(a.tlRadiusY, b.tlRadiusY, t)),
trRadiusX: math.max(0, _lerpDouble(a.trRadiusX, b.trRadiusX, t)),
trRadiusY: math.max(0, _lerpDouble(a.trRadiusY, b.trRadiusY, t)),
brRadiusX: math.max(0, _lerpDouble(a.brRadiusX, b.brRadiusX, t)),
brRadiusY: math.max(0, _lerpDouble(a.brRadiusY, b.brRadiusY, t)),
blRadiusX: math.max(0, _lerpDouble(a.blRadiusX, b.blRadiusX, t)),
blRadiusY: math.max(0, _lerpDouble(a.blRadiusY, b.blRadiusY, t)),
);
}
}
Expand Down
72 changes: 40 additions & 32 deletions lib/web_ui/lib/geometry.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,14 @@ class RRect {
assert(brRadiusY != null),
assert(blRadiusX != null),
assert(blRadiusY != null),
assert(tlRadiusX >= 0),
assert(tlRadiusY >= 0),
assert(trRadiusX >= 0),
assert(trRadiusY >= 0),
assert(brRadiusX >= 0),
assert(brRadiusY >= 0),
assert(blRadiusX >= 0),
assert(blRadiusY >= 0),
webOnlyUniformRadii = uniformRadii;

final double left;
Expand Down Expand Up @@ -623,14 +631,14 @@ class RRect {
top: top - delta,
right: right + delta,
bottom: bottom + delta,
tlRadiusX: tlRadiusX + delta,
tlRadiusY: tlRadiusY + delta,
trRadiusX: trRadiusX + delta,
trRadiusY: trRadiusY + delta,
blRadiusX: blRadiusX + delta,
blRadiusY: blRadiusY + delta,
brRadiusX: brRadiusX + delta,
brRadiusY: brRadiusY + delta,
tlRadiusX: math.max(0, tlRadiusX + delta),
tlRadiusY: math.max(0, tlRadiusY + delta),
trRadiusX: math.max(0, trRadiusX + delta),
trRadiusY: math.max(0, trRadiusY + delta),
blRadiusX: math.max(0, blRadiusX + delta),
blRadiusY: math.max(0, blRadiusY + delta),
brRadiusX: math.max(0, brRadiusX + delta),
brRadiusY: math.max(0, brRadiusY + delta),
);
}

Expand Down Expand Up @@ -835,14 +843,14 @@ class RRect {
top: a.top * k,
right: a.right * k,
bottom: a.bottom * k,
tlRadiusX: a.tlRadiusX * k,
tlRadiusY: a.tlRadiusY * k,
trRadiusX: a.trRadiusX * k,
trRadiusY: a.trRadiusY * k,
brRadiusX: a.brRadiusX * k,
brRadiusY: a.brRadiusY * k,
blRadiusX: a.blRadiusX * k,
blRadiusY: a.blRadiusY * k,
tlRadiusX: math.max(0, a.tlRadiusX * k),
tlRadiusY: math.max(0, a.tlRadiusY * k),
trRadiusX: math.max(0, a.trRadiusX * k),
trRadiusY: math.max(0, a.trRadiusY * k),
brRadiusX: math.max(0, a.brRadiusX * k),
brRadiusY: math.max(0, a.brRadiusY * k),
blRadiusX: math.max(0, a.blRadiusX * k),
blRadiusY: math.max(0, a.blRadiusY * k),
);
}
} else {
Expand All @@ -852,29 +860,29 @@ class RRect {
top: b.top * t,
right: b.right * t,
bottom: b.bottom * t,
tlRadiusX: b.tlRadiusX * t,
tlRadiusY: b.tlRadiusY * t,
trRadiusX: b.trRadiusX * t,
trRadiusY: b.trRadiusY * t,
brRadiusX: b.brRadiusX * t,
brRadiusY: b.brRadiusY * t,
blRadiusX: b.blRadiusX * t,
blRadiusY: b.blRadiusY * t,
tlRadiusX: math.max(0, b.tlRadiusX * t),
tlRadiusY: math.max(0, b.tlRadiusY * t),
trRadiusX: math.max(0, b.trRadiusX * t),
trRadiusY: math.max(0, b.trRadiusY * t),
brRadiusX: math.max(0, b.brRadiusX * t),
brRadiusY: math.max(0, b.brRadiusY * t),
blRadiusX: math.max(0, b.blRadiusX * t),
blRadiusY: math.max(0, b.blRadiusY * t),
);
} else {
return RRect._raw(
left: _lerpDouble(a.left, b.left, t),
top: _lerpDouble(a.top, b.top, t),
right: _lerpDouble(a.right, b.right, t),
bottom: _lerpDouble(a.bottom, b.bottom, t),
tlRadiusX: _lerpDouble(a.tlRadiusX, b.tlRadiusX, t),
tlRadiusY: _lerpDouble(a.tlRadiusY, b.tlRadiusY, t),
trRadiusX: _lerpDouble(a.trRadiusX, b.trRadiusX, t),
trRadiusY: _lerpDouble(a.trRadiusY, b.trRadiusY, t),
brRadiusX: _lerpDouble(a.brRadiusX, b.brRadiusX, t),
brRadiusY: _lerpDouble(a.brRadiusY, b.brRadiusY, t),
blRadiusX: _lerpDouble(a.blRadiusX, b.blRadiusX, t),
blRadiusY: _lerpDouble(a.blRadiusY, b.blRadiusY, t),
tlRadiusX: math.max(0, _lerpDouble(a.tlRadiusX, b.tlRadiusX, t)),
tlRadiusY: math.max(0, _lerpDouble(a.tlRadiusY, b.tlRadiusY, t)),
trRadiusX: math.max(0, _lerpDouble(a.trRadiusX, b.trRadiusX, t)),
trRadiusY: math.max(0, _lerpDouble(a.trRadiusY, b.trRadiusY, t)),
brRadiusX: math.max(0, _lerpDouble(a.brRadiusX, b.brRadiusX, t)),
brRadiusY: math.max(0, _lerpDouble(a.brRadiusY, b.brRadiusY, t)),
blRadiusX: math.max(0, _lerpDouble(a.blRadiusX, b.blRadiusX, t)),
blRadiusY: math.max(0, _lerpDouble(a.blRadiusY, b.blRadiusY, t)),
);
}
}
Expand Down
12 changes: 2 additions & 10 deletions lib/web_ui/lib/src/engine/html/recording_canvas.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,8 @@ import 'shaders/image_shader.dart';
/// Enable this to print every command applied by a canvas.
const bool _debugDumpPaintCommands = false;

// Returns the squared length of the x, y (of a border radius)
// It normalizes x, y values before working with them, by
// assuming anything < 0 to be 0, because flutter may pass
// negative radii (which Skia assumes to be 0), see:
// https://skia.org/user/api/SkRRect_Reference#SkRRect_inset
double _measureBorderRadius(double x, double y) {
final double clampedX = x < 0 ? 0 : x;
final double clampedY = y < 0 ? 0 : y;
return clampedX * clampedX + clampedY * clampedY;
}
// Returns the squared length of the x, y (of a border radius).
double _measureBorderRadius(double x, double y) => x*x + y*y;

/// Records canvas commands to be applied to a [EngineCanvas].
///
Expand Down
7 changes: 3 additions & 4 deletions lib/web_ui/test/engine/recording_canvas_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,16 @@ void testMain() {
expect(mockCanvas.methodCallLog.single.methodName, 'endOfPaint');
});

test('negative corners in inner RRect get passed through to draw', () {
test('deflated corners in inner RRect get passed through to draw', () {
// This comes from github issue #40728
final RRect outer = RRect.fromRectAndCorners(
const Rect.fromLTWH(0, 0, 88, 48),
topLeft: const Radius.circular(6),
bottomLeft: const Radius.circular(6));
final RRect inner = outer.deflate(1);

// If these assertions fail, check [_measureBorderRadius] in recording_canvas.dart
expect(inner.brRadius, equals(const Radius.circular(-1)));
expect(inner.trRadius, equals(const Radius.circular(-1)));
expect(inner.brRadius, equals(Radius.zero));
expect(inner.trRadius, equals(Radius.zero));

underTest.drawDRRect(outer, inner, somePaint);
underTest.endRecording();
Expand Down
Loading