Improve and optimize non-uniform Borders. (#124417)
~~Using the same priority order as a Border without borderRadius, it is possible to draw them on top of each other. This is better than the current behavior (crash!) and would work well for a "one color on top, another on bottom" scenario.~~
~~With this, if approved, we move the current number of possible exceptions from 4 to 1 (`BoxShape.circle` + `borderRadius`).~~
~~It is kind of odd how `borderRadius.zero` to `borderRadius != BorderRadius.zero` change, but I think it is better than crashing. Alternatively, we just remove the "original function" and see if any goldens are affected.~~
<img width="448" alt="image" src="https://user-images.githubusercontent.com/351125/236550350-7499d758-5b44-40e6-9105-32671eb21998.png">
Another one for @gspencergoog. If this works, we could make the paint method public and re-use in the InputBorder PR (if that's also approved). Single line fix.
diff --git a/packages/flutter/lib/src/painting/box_border.dart b/packages/flutter/lib/src/painting/box_border.dart
index dfc9a60..633be6a 100644
--- a/packages/flutter/lib/src/painting/box_border.dart
+++ b/packages/flutter/lib/src/painting/box_border.dart
@@ -242,16 +242,24 @@
}
}
- static void _paintNonUniformBorder(
+ /// Paints a Border with different widths, styles and strokeAligns, on any
+ /// borderRadius while using a single color.
+ ///
+ /// See also:
+ ///
+ /// * [paintBorder], which supports multiple colors but not borderRadius.
+ /// * [paint], which calls this method.
+ static void paintNonUniformBorder(
Canvas canvas,
Rect rect, {
required BorderRadius? borderRadius,
- required BoxShape shape,
required TextDirection? textDirection,
- required BorderSide left,
- required BorderSide top,
- required BorderSide right,
- required BorderSide bottom,
+ BoxShape shape = BoxShape.rectangle,
+ BorderSide top = BorderSide.none,
+ BorderSide right = BorderSide.none,
+ BorderSide bottom = BorderSide.none,
+ BorderSide left = BorderSide.none,
+ required Color color,
}) {
final RRect borderRect;
switch (shape) {
@@ -266,7 +274,7 @@
Radius.circular(rect.width),
);
}
- final Paint paint = Paint()..color = top.color;
+ final Paint paint = Paint()..color = color;
final RRect inner = _deflateRRect(borderRect, EdgeInsets.fromLTRB(left.strokeInset, top.strokeInset, right.strokeInset, bottom.strokeInset));
final RRect outer = _inflateRRect(borderRect, EdgeInsets.fromLTRB(left.strokeOutset, top.strokeOutset, right.strokeOutset, bottom.strokeOutset));
canvas.drawDRRect(outer, inner, paint);
@@ -489,6 +497,32 @@
&& right.strokeAlign == topStrokeAlign;
}
+ Set<Color> _distinctVisibleColors() {
+ final Set<Color> distinctVisibleColors = <Color>{};
+ if (top.style != BorderStyle.none) {
+ distinctVisibleColors.add(top.color);
+ }
+ if (right.style != BorderStyle.none) {
+ distinctVisibleColors.add(right.color);
+ }
+ if (bottom.style != BorderStyle.none) {
+ distinctVisibleColors.add(bottom.color);
+ }
+ if (left.style != BorderStyle.none) {
+ distinctVisibleColors.add(left.color);
+ }
+ return distinctVisibleColors;
+ }
+
+ // [BoxBorder.paintNonUniformBorder] is about 20% faster than [paintBorder],
+ // but [paintBorder] is able to draw hairline borders when width is zero
+ // and style is [BorderStyle.solid].
+ bool get _hasHairlineBorder =>
+ (top.style == BorderStyle.solid && top.width == 0.0) ||
+ (right.style == BorderStyle.solid && right.width == 0.0) ||
+ (bottom.style == BorderStyle.solid && bottom.width == 0.0) ||
+ (left.style == BorderStyle.solid && left.width == 0.0);
+
@override
Border? add(ShapeBorder other, { bool reversed = false }) {
if (other is Border &&
@@ -603,31 +637,41 @@
}
}
- // Allow painting non-uniform borders if the color and style are uniform.
- if (_colorIsUniform && _styleIsUniform) {
- switch (top.style) {
- case BorderStyle.none:
- return;
- case BorderStyle.solid:
- BoxBorder._paintNonUniformBorder(canvas, rect,
- shape: shape,
- borderRadius: borderRadius,
- textDirection: textDirection,
- left: left,
- top: top,
- right: right,
- bottom: bottom);
- return;
- }
+ if (_styleIsUniform && top.style == BorderStyle.none) {
+ return;
}
- assert(() {
- if (borderRadius != null) {
+ // Allow painting non-uniform borders if the visible colors are uniform.
+ final Set<Color> visibleColors = _distinctVisibleColors();
+ final bool hasHairlineBorder = _hasHairlineBorder;
+ // Paint a non uniform border if a single color is visible
+ // and (borderRadius is present) or (border is visible and width != 0.0).
+ if (visibleColors.length == 1 &&
+ !hasHairlineBorder &&
+ (shape == BoxShape.circle ||
+ (borderRadius != null && borderRadius != BorderRadius.zero))) {
+ BoxBorder.paintNonUniformBorder(canvas, rect,
+ shape: shape,
+ borderRadius: borderRadius,
+ textDirection: textDirection,
+ top: top.style == BorderStyle.none ? BorderSide.none : top,
+ right: right.style == BorderStyle.none ? BorderSide.none : right,
+ bottom: bottom.style == BorderStyle.none ? BorderSide.none : bottom,
+ left: left.style == BorderStyle.none ? BorderSide.none : left,
+ color: visibleColors.first);
+ return;
+ }
+
+ assert(() {
+ if (hasHairlineBorder) {
+ assert(borderRadius == null || borderRadius == BorderRadius.zero,
+ 'A hairline border like `BorderSide(width: 0.0, style: BorderStyle.solid)` can only be drawn when BorderRadius is zero or null.');
+ }
+ if (borderRadius != null && borderRadius != BorderRadius.zero) {
throw FlutterError.fromParts(<DiagnosticsNode>[
- ErrorSummary('A borderRadius can only be given on borders with uniform colors and styles.'),
+ ErrorSummary('A borderRadius can only be given on borders with uniform colors.'),
ErrorDescription('The following is not uniform:'),
if (!_colorIsUniform) ErrorDescription('BorderSide.color'),
- if (!_styleIsUniform) ErrorDescription('BorderSide.style'),
]);
}
return true;
@@ -635,10 +679,9 @@
assert(() {
if (shape != BoxShape.rectangle) {
throw FlutterError.fromParts(<DiagnosticsNode>[
- ErrorSummary('A Border can only be drawn as a circle on borders with uniform colors and styles.'),
+ ErrorSummary('A Border can only be drawn as a circle on borders with uniform colors.'),
ErrorDescription('The following is not uniform:'),
if (!_colorIsUniform) ErrorDescription('BorderSide.color'),
- if (!_styleIsUniform) ErrorDescription('BorderSide.style'),
]);
}
return true;
@@ -646,7 +689,7 @@
assert(() {
if (!_strokeAlignIsUniform || top.strokeAlign != BorderSide.strokeAlignInside) {
throw FlutterError.fromParts(<DiagnosticsNode>[
- ErrorSummary('A Border can only draw strokeAlign different than BorderSide.strokeAlignInside on borders with uniform colors and styles.'),
+ ErrorSummary('A Border can only draw strokeAlign different than BorderSide.strokeAlignInside on borders with uniform colors.'),
]);
}
return true;
@@ -806,6 +849,31 @@
&& end.strokeAlign == topStrokeAlign;
}
+ Set<Color> _distinctVisibleColors() {
+ final Set<Color> distinctVisibleColors = <Color>{};
+ if (top.style != BorderStyle.none) {
+ distinctVisibleColors.add(top.color);
+ }
+ if (end.style != BorderStyle.none) {
+ distinctVisibleColors.add(end.color);
+ }
+ if (bottom.style != BorderStyle.none) {
+ distinctVisibleColors.add(bottom.color);
+ }
+ if (start.style != BorderStyle.none) {
+ distinctVisibleColors.add(start.color);
+ }
+
+ return distinctVisibleColors;
+ }
+
+
+ bool get _hasHairlineBorder =>
+ (top.style == BorderStyle.solid && top.width == 0.0) ||
+ (end.style == BorderStyle.solid && end.width == 0.0) ||
+ (bottom.style == BorderStyle.solid && bottom.width == 0.0) ||
+ (start.style == BorderStyle.solid && start.width == 0.0);
+
@override
BoxBorder? add(ShapeBorder other, { bool reversed = false }) {
if (other is BorderDirectional) {
@@ -951,6 +1019,10 @@
}
}
+ if (_styleIsUniform && top.style == BorderStyle.none) {
+ return;
+ }
+
final BorderSide left, right;
assert(textDirection != null, 'Non-uniform BorderDirectional objects require a TextDirection when painting.');
switch (textDirection!) {
@@ -962,27 +1034,31 @@
right = end;
}
- // Allow painting non-uniform borders if the color and style are uniform.
- if (_colorIsUniform && _styleIsUniform) {
- switch (top.style) {
- case BorderStyle.none:
- return;
- case BorderStyle.solid:
- BoxBorder._paintNonUniformBorder(canvas, rect,
- shape: shape,
- borderRadius: borderRadius,
- textDirection: textDirection,
- left: left,
- top: top,
- right: right,
- bottom: bottom);
- return;
- }
+ // Allow painting non-uniform borders if the visible colors are uniform.
+ final Set<Color> visibleColors = _distinctVisibleColors();
+ final bool hasHairlineBorder = _hasHairlineBorder;
+ if (visibleColors.length == 1 &&
+ !hasHairlineBorder &&
+ (shape == BoxShape.circle ||
+ (borderRadius != null && borderRadius != BorderRadius.zero))) {
+ BoxBorder.paintNonUniformBorder(canvas, rect,
+ shape: shape,
+ borderRadius: borderRadius,
+ textDirection: textDirection,
+ top: top.style == BorderStyle.none ? BorderSide.none : top,
+ right: right.style == BorderStyle.none ? BorderSide.none : right,
+ bottom: bottom.style == BorderStyle.none ? BorderSide.none : bottom,
+ left: left.style == BorderStyle.none ? BorderSide.none : left,
+ color: visibleColors.first);
+ return;
}
- assert(borderRadius == null, 'A borderRadius can only be given for borders with uniform colors and styles.');
- assert(shape == BoxShape.rectangle, 'A Border can only be drawn as a circle on borders with uniform colors and styles.');
- assert(_strokeAlignIsUniform && top.strokeAlign == BorderSide.strokeAlignInside, 'A Border can only draw strokeAlign different than strokeAlignInside on borders with uniform colors and styles.');
+ if (hasHairlineBorder) {
+ assert(borderRadius == null || borderRadius == BorderRadius.zero, 'A side like `BorderSide(width: 0.0, style: BorderStyle.solid)` can only be drawn when BorderRadius is zero or null.');
+ }
+ assert(borderRadius == null, 'A borderRadius can only be given for borders with uniform colors.');
+ assert(shape == BoxShape.rectangle, 'A Border can only be drawn as a circle on borders with uniform colors.');
+ assert(_strokeAlignIsUniform && top.strokeAlign == BorderSide.strokeAlignInside, 'A Border can only draw strokeAlign different than strokeAlignInside on borders with uniform colors.');
paintBorder(canvas, rect, top: top, left: left, bottom: bottom, right: right);
}
diff --git a/packages/flutter/test/material/data_table_test.dart b/packages/flutter/test/material/data_table_test.dart
index 2863d8a..fed5e1a 100644
--- a/packages/flutter/test/material/data_table_test.dart
+++ b/packages/flutter/test/material/data_table_test.dart
@@ -1750,7 +1750,7 @@
);
expect(
find.ancestor(of: find.byType(Table), matching: find.byType(Container)),
- paints..drrect(color: borderColor),
+ paints..path(color: borderColor),
);
expect(
tester.getTopLeft(find.byType(Table)),
diff --git a/packages/flutter/test/material/divider_test.dart b/packages/flutter/test/material/divider_test.dart
index c3326e7..9249ea6 100644
--- a/packages/flutter/test/material/divider_test.dart
+++ b/packages/flutter/test/material/divider_test.dart
@@ -136,8 +136,9 @@
testWidgets('Vertical Divider Test 2', (WidgetTester tester) async {
await tester.pumpWidget(
- const MaterialApp(
- home: Material(
+ MaterialApp(
+ theme: ThemeData(useMaterial3: false),
+ home: const Material(
child: SizedBox(
height: 24.0,
child: Row(
diff --git a/packages/flutter/test/painting/border_test.dart b/packages/flutter/test/painting/border_test.dart
index f2f2aeb..e4298f8 100644
--- a/packages/flutter/test/painting/border_test.dart
+++ b/packages/flutter/test/painting/border_test.dart
@@ -273,7 +273,7 @@
expect(error.diagnostics.length, 1);
expect(
error.diagnostics[0].toStringDeep(),
- 'A Border can only draw strokeAlign different than\nBorderSide.strokeAlignInside on borders with uniform colors and\nstyles.\n',
+ 'A Border can only draw strokeAlign different than\nBorderSide.strokeAlignInside on borders with uniform colors.\n',
);
});
@@ -341,8 +341,8 @@
// This falls into non-uniform border because of strokeAlign.
await tester.pumpWidget(buildWidget(border: allowedBorderVariations));
- expect(tester.takeException(), isNull,
- reason: 'Border with non-uniform strokeAlign should not fail.');
+ expect(tester.takeException(), isAssertionError,
+ reason: 'Border with non-uniform strokeAlign should fail.');
await tester.pumpWidget(buildWidget(
border: allowedBorderVariations,
@@ -364,8 +364,8 @@
borderRadius: BorderRadius.circular(25),
),
);
- expect(tester.takeException(), isAssertionError,
- reason: 'Border with non-uniform styles should fail with borderRadius.');
+ expect(tester.takeException(), isNull,
+ reason: 'Border with non-uniform styles should work with borderRadius.');
await tester.pumpWidget(
buildWidget(
@@ -381,6 +381,24 @@
expect(tester.takeException(), isAssertionError,
reason: 'Border with non-uniform colors should fail with borderRadius.');
+ await tester.pumpWidget(
+ buildWidget(
+ border: const Border(bottom: BorderSide(width: 0)),
+ borderRadius: BorderRadius.zero,
+ ),
+ );
+ expect(tester.takeException(), isNull,
+ reason: 'Border with a side.width == 0 should work without borderRadius (hairline border).');
+
+ await tester.pumpWidget(
+ buildWidget(
+ border: const Border(bottom: BorderSide(width: 0)),
+ borderRadius: BorderRadius.circular(40),
+ ),
+ );
+ expect(tester.takeException(), isAssertionError,
+ reason: 'Border with width == 0 and borderRadius should fail (hairline border).');
+
// Tests for BorderDirectional.
const BorderDirectional allowedBorderDirectionalVariations = BorderDirectional(
start: BorderSide(width: 5),
@@ -390,7 +408,7 @@
);
await tester.pumpWidget(buildWidget(border: allowedBorderDirectionalVariations));
- expect(tester.takeException(), isNull);
+ expect(tester.takeException(), isAssertionError);
await tester.pumpWidget(buildWidget(
border: allowedBorderDirectionalVariations,
