From c94699388a52236f2bd8e2dde20cb4794dc77188 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Fri, 9 Sep 2022 15:54:37 -0700 Subject: [PATCH 01/10] Clamp RRect radii when deflating --- lib/ui/geometry.dart | 75 +++++++++-------- testing/dart/geometry_test.dart | 144 ++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 33 deletions(-) diff --git a/lib/ui/geometry.dart b/lib/ui/geometry.dart index 2b7d61a47651a..259921f053bf5 100644 --- a/lib/ui/geometry.dart +++ b/lib/ui/geometry.dart @@ -1237,7 +1237,15 @@ class RRect { assert(brRadiusX != null), assert(brRadiusY != null), assert(blRadiusX != null), - assert(blRadiusY != 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); Float32List _getValue32() { final Float32List result = Float32List(12); @@ -1333,14 +1341,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), ); } @@ -1503,6 +1511,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( @@ -1621,14 +1630,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 { @@ -1638,14 +1647,14 @@ 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( @@ -1653,14 +1662,14 @@ class RRect { 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)), ); } } diff --git a/testing/dart/geometry_test.dart b/testing/dart/geometry_test.dart index 27c22ff7607d1..8e407bef2e78b 100644 --- a/testing/dart/geometry_test.dart +++ b/testing/dart/geometry_test.dart @@ -376,4 +376,148 @@ void main() { expect(rrectMix2.trRadius, equals(const Radius.elliptical(10, 6))); expect(rrectMix2.blRadius, equals(const Radius.elliptical(10, 6))); }); + + test('RRect asserts when corner radii are negative', () { + bool asserted = false; + try { + RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + topLeft: const Radius.circular(-1), + ); + } on AssertionError { + asserted = true; + } + expect(asserted, isTrue, reason: "Constructing an RRect with negative topLeft radii didn't assert"); + asserted = false; + + try { + RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + topRight: const Radius.circular(-2), + ); + } on AssertionError { + asserted = true; + } + expect(asserted, isTrue, reason: "Constructing an RRect with negative topRight radii didn't assert"); + asserted = false; + + try { + RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + bottomLeft: const Radius.circular(-3), + ); + } on AssertionError { + asserted = true; + } + expect(asserted, isTrue, reason: "Constructing an RRect with negative bottomLeft radii didn't assert"); + asserted = false; + + try { + RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + bottomRight: const Radius.circular(-4), + ); + } on AssertionError { + asserted = true; + } + expect(asserted, isTrue, reason: "Constructing an RRect with negative bottomRight radii didn't assert"); + }); + + test('RRect.inflate clamps when deflating past zero', () { + RRect rrect = RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + topLeft: const Radius.circular(1), + topRight: const Radius.circular(2), + bottomLeft: const Radius.circular(3), + bottomRight: const Radius.circular(4), + ).inflate(-1); + + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 1); + expect(rrect.trRadiusY, 1); + expect(rrect.blRadiusX, 2); + expect(rrect.blRadiusY, 2); + expect(rrect.brRadiusX, 3); + expect(rrect.brRadiusY, 3); + + rrect = rrect.inflate(-1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 1); + expect(rrect.blRadiusY, 1); + expect(rrect.brRadiusX, 2); + expect(rrect.brRadiusY, 2); + + rrect = rrect.inflate(-1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 0); + expect(rrect.blRadiusY, 0); + expect(rrect.brRadiusX, 1); + expect(rrect.brRadiusY, 1); + + rrect = rrect.inflate(-1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 0); + expect(rrect.blRadiusY, 0); + expect(rrect.brRadiusX, 0); + expect(rrect.brRadiusY, 0); + }); + + test('RRect.deflate clamps when deflating past zero', () { + RRect rrect = RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + topLeft: const Radius.circular(1), + topRight: const Radius.circular(2), + bottomLeft: const Radius.circular(3), + bottomRight: const Radius.circular(4), + ).deflate(1); + + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 1); + expect(rrect.trRadiusY, 1); + expect(rrect.blRadiusX, 2); + expect(rrect.blRadiusY, 2); + expect(rrect.brRadiusX, 3); + expect(rrect.brRadiusY, 3); + + rrect = rrect.deflate(1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 1); + expect(rrect.blRadiusY, 1); + expect(rrect.brRadiusX, 2); + expect(rrect.brRadiusY, 2); + + rrect = rrect.deflate(1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 0); + expect(rrect.blRadiusY, 0); + expect(rrect.brRadiusX, 1); + expect(rrect.brRadiusY, 1); + + rrect = rrect.deflate(1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 0); + expect(rrect.blRadiusY, 0); + expect(rrect.brRadiusX, 0); + expect(rrect.brRadiusY, 0); + }); } From 347fc647971d509b2e62570a186a1850a98a3148 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Fri, 9 Sep 2022 19:33:05 -0700 Subject: [PATCH 02/10] Fix test --- testing/dart/geometry_test.dart | 40 ++++++++++----------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/testing/dart/geometry_test.dart b/testing/dart/geometry_test.dart index 8e407bef2e78b..022cd35421b8f 100644 --- a/testing/dart/geometry_test.dart +++ b/testing/dart/geometry_test.dart @@ -377,50 +377,34 @@ void main() { expect(rrectMix2.blRadius, equals(const Radius.elliptical(10, 6))); }); - test('RRect asserts when corner radii are negative', () { - bool asserted = false; - try { + test('RRect asserts when corner radii are negative', () async { + expect(() async { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), topLeft: const Radius.circular(-1), ); - } on AssertionError { - asserted = true; - } - expect(asserted, isTrue, reason: "Constructing an RRect with negative topLeft radii didn't assert"); - asserted = false; + }, throwsA(AssertionError)); - try { + expect(() async { RRect.fromRectAndCorners( - const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), topRight: const Radius.circular(-2), ); - } on AssertionError { - asserted = true; - } - expect(asserted, isTrue, reason: "Constructing an RRect with negative topRight radii didn't assert"); - asserted = false; + }, throwsA(AssertionError)); - try { + expect(() async { RRect.fromRectAndCorners( - const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), bottomLeft: const Radius.circular(-3), ); - } on AssertionError { - asserted = true; - } - expect(asserted, isTrue, reason: "Constructing an RRect with negative bottomLeft radii didn't assert"); - asserted = false; + }, throwsA(AssertionError)); - try { + expect(() async { RRect.fromRectAndCorners( - const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), bottomRight: const Radius.circular(-4), ); - } on AssertionError { - asserted = true; - } - expect(asserted, isTrue, reason: "Constructing an RRect with negative bottomRight radii didn't assert"); + }, throwsA(AssertionError)); }); test('RRect.inflate clamps when deflating past zero', () { From bf45418683d65b8f2306c49412f7864e772d93b2 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 12 Sep 2022 09:29:06 -0700 Subject: [PATCH 03/10] Correctly test assertion throwing --- testing/dart/geometry_test.dart | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/testing/dart/geometry_test.dart b/testing/dart/geometry_test.dart index 022cd35421b8f..ec1eff9b18cdc 100644 --- a/testing/dart/geometry_test.dart +++ b/testing/dart/geometry_test.dart @@ -377,34 +377,34 @@ void main() { expect(rrectMix2.blRadius, equals(const Radius.elliptical(10, 6))); }); - test('RRect asserts when corner radii are negative', () async { - expect(() async { + test('RRect asserts when corner radii are negative', () { + expect(() { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), topLeft: const Radius.circular(-1), ); - }, throwsA(AssertionError)); + }, throwsAssertionError); - expect(() async { + expect(() { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), topRight: const Radius.circular(-2), ); - }, throwsA(AssertionError)); + }, throwsAssertionError); - expect(() async { + expect(() { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), bottomLeft: const Radius.circular(-3), ); - }, throwsA(AssertionError)); + }, throwsAssertionError); - expect(() async { + expect(() { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), bottomRight: const Radius.circular(-4), ); - }, throwsA(AssertionError)); + }, throwsAssertionError); }); test('RRect.inflate clamps when deflating past zero', () { From c1c586e45ae763c024c7eb5f2c963e0fa289e728 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 12 Sep 2022 11:07:03 -0700 Subject: [PATCH 04/10] revert to other method of assertion detection --- testing/dart/geometry_test.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/testing/dart/geometry_test.dart b/testing/dart/geometry_test.dart index ec1eff9b18cdc..93f4fcef3278a 100644 --- a/testing/dart/geometry_test.dart +++ b/testing/dart/geometry_test.dart @@ -383,28 +383,28 @@ void main() { const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), topLeft: const Radius.circular(-1), ); - }, throwsAssertionError); + }, throwsA(isInstanceOf())); expect(() { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), topRight: const Radius.circular(-2), ); - }, throwsAssertionError); + }, throwsA(isInstanceOf())); expect(() { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), bottomLeft: const Radius.circular(-3), ); - }, throwsAssertionError); + }, throwsA(isInstanceOf())); expect(() { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), bottomRight: const Radius.circular(-4), ); - }, throwsAssertionError); + }, throwsA(isInstanceOf())); }); test('RRect.inflate clamps when deflating past zero', () { From b06f132ccec5511527a4e30cdf5f4638834e0d16 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 12 Sep 2022 12:16:09 -0700 Subject: [PATCH 05/10] Check for enabled assertions --- testing/dart/geometry_test.dart | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/testing/dart/geometry_test.dart b/testing/dart/geometry_test.dart index 93f4fcef3278a..8dc7f371eada6 100644 --- a/testing/dart/geometry_test.dart +++ b/testing/dart/geometry_test.dart @@ -378,6 +378,15 @@ void main() { }); test('RRect asserts when corner radii are negative', () { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + if (!assertsEnabled) { + return; + } + expect(() { RRect.fromRectAndCorners( const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), From 3ce047b23a948c7fcc0e1c0d82149274e48d904d Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 12 Sep 2022 15:08:42 -0700 Subject: [PATCH 06/10] Add to web_ui too --- lib/web_ui/lib/geometry.dart | 72 +++++++++------- lib/web_ui/test/geometry_test.dart | 134 +++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 32 deletions(-) diff --git a/lib/web_ui/lib/geometry.dart b/lib/web_ui/lib/geometry.dart index 412f19137f2f9..7d17c73f14131 100644 --- a/lib/web_ui/lib/geometry.dart +++ b/lib/web_ui/lib/geometry.dart @@ -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; @@ -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), ); } @@ -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 { @@ -852,14 +860,14 @@ 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( @@ -867,14 +875,14 @@ class RRect { 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)), ); } } diff --git a/lib/web_ui/test/geometry_test.dart b/lib/web_ui/test/geometry_test.dart index 40b2ffd4c9d9c..24150d801cdf4 100644 --- a/lib/web_ui/test/geometry_test.dart +++ b/lib/web_ui/test/geometry_test.dart @@ -157,4 +157,138 @@ void testMain() { expect(rrectMix2.trRadius, equals(const Radius.elliptical(10, 6))); expect(rrectMix2.blRadius, equals(const Radius.elliptical(10, 6))); }); + test('RRect asserts when corner radii are negative', () { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + if (!assertsEnabled) { + return; + } + + expect(() { + RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + topLeft: const Radius.circular(-1), + ); + }, throwsA(isA())); + + expect(() { + RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + topRight: const Radius.circular(-2), + ); + }, throwsA(isA())); + + expect(() { + RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + bottomLeft: const Radius.circular(-3), + ); + }, throwsA(isA())); + + expect(() { + RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + bottomRight: const Radius.circular(-4), + ); + }, throwsA(isA())); + }); + test('RRect.inflate clamps when deflating past zero', () { + RRect rrect = RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + topLeft: const Radius.circular(1), + topRight: const Radius.circular(2), + bottomLeft: const Radius.circular(3), + bottomRight: const Radius.circular(4), + ).inflate(-1); + + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 1); + expect(rrect.trRadiusY, 1); + expect(rrect.blRadiusX, 2); + expect(rrect.blRadiusY, 2); + expect(rrect.brRadiusX, 3); + expect(rrect.brRadiusY, 3); + + rrect = rrect.inflate(-1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 1); + expect(rrect.blRadiusY, 1); + expect(rrect.brRadiusX, 2); + expect(rrect.brRadiusY, 2); + + rrect = rrect.inflate(-1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 0); + expect(rrect.blRadiusY, 0); + expect(rrect.brRadiusX, 1); + expect(rrect.brRadiusY, 1); + + rrect = rrect.inflate(-1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 0); + expect(rrect.blRadiusY, 0); + expect(rrect.brRadiusX, 0); + expect(rrect.brRadiusY, 0); + }); + test('RRect.deflate clamps when deflating past zero', () { + RRect rrect = RRect.fromRectAndCorners( + const Rect.fromLTRB(10.0, 20.0, 30.0, 40.0), + topLeft: const Radius.circular(1), + topRight: const Radius.circular(2), + bottomLeft: const Radius.circular(3), + bottomRight: const Radius.circular(4), + ).deflate(1); + + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 1); + expect(rrect.trRadiusY, 1); + expect(rrect.blRadiusX, 2); + expect(rrect.blRadiusY, 2); + expect(rrect.brRadiusX, 3); + expect(rrect.brRadiusY, 3); + + rrect = rrect.deflate(1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 1); + expect(rrect.blRadiusY, 1); + expect(rrect.brRadiusX, 2); + expect(rrect.brRadiusY, 2); + + rrect = rrect.deflate(1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 0); + expect(rrect.blRadiusY, 0); + expect(rrect.brRadiusX, 1); + expect(rrect.brRadiusY, 1); + + rrect = rrect.deflate(1); + expect(rrect.tlRadiusX, 0); + expect(rrect.tlRadiusY, 0); + expect(rrect.trRadiusX, 0); + expect(rrect.trRadiusY, 0); + expect(rrect.blRadiusX, 0); + expect(rrect.blRadiusY, 0); + expect(rrect.brRadiusX, 0); + expect(rrect.brRadiusY, 0); + }); } From 94e2887027fefd7e3ed89229cedd22b02eca1de8 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 12 Sep 2022 15:54:47 -0700 Subject: [PATCH 07/10] Fix recording canvas to not expect negative values anymore --- lib/web_ui/lib/src/engine/html/recording_canvas.dart | 12 ++---------- lib/web_ui/test/engine/recording_canvas_test.dart | 5 ++--- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/recording_canvas.dart b/lib/web_ui/lib/src/engine/html/recording_canvas.dart index b1f86da42f8fe..a7638d1dfcbcf 100644 --- a/lib/web_ui/lib/src/engine/html/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/html/recording_canvas.dart @@ -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]. /// diff --git a/lib/web_ui/test/engine/recording_canvas_test.dart b/lib/web_ui/test/engine/recording_canvas_test.dart index 75b59a813fb63..ea866cd667cb8 100644 --- a/lib/web_ui/test/engine/recording_canvas_test.dart +++ b/lib/web_ui/test/engine/recording_canvas_test.dart @@ -129,9 +129,8 @@ void testMain() { 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(); From 018367cc47fcceeb0edf0f3fea7de803048740af Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 12 Sep 2022 15:54:47 -0700 Subject: [PATCH 08/10] Fix recording canvas to not expect negative values anymore --- lib/web_ui/test/engine/recording_canvas_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/engine/recording_canvas_test.dart b/lib/web_ui/test/engine/recording_canvas_test.dart index ea866cd667cb8..e5ae2c462afe0 100644 --- a/lib/web_ui/test/engine/recording_canvas_test.dart +++ b/lib/web_ui/test/engine/recording_canvas_test.dart @@ -121,7 +121,7 @@ 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), From 8bfd7fe43767880ed29eb8063a1571b4dd9e1cde Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 13 Sep 2022 14:54:50 -0700 Subject: [PATCH 09/10] Add missing reference to math.dart --- lib/ui/dart_ui.gni | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ui/dart_ui.gni b/lib/ui/dart_ui.gni index 96a7b06fbd918..7059f7ab2ae08 100644 --- a/lib/ui/dart_ui.gni +++ b/lib/ui/dart_ui.gni @@ -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", "//flutter/lib/ui/natives.dart", "//flutter/lib/ui/painting.dart", "//flutter/lib/ui/platform_dispatcher.dart", From c52f063bf61e27c4afc7c2a929a2a2b93aa23ad1 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Wed, 14 Sep 2022 15:16:18 -0700 Subject: [PATCH 10/10] Mention in public constructors that negative values assert. --- lib/ui/geometry.dart | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/ui/geometry.dart b/lib/ui/geometry.dart index 259921f053bf5..7a395253e2356 100644 --- a/lib/ui/geometry.dart +++ b/lib/ui/geometry.dart @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, @@ -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, {