Skip to content

Commit b7046b3

Browse files
authored
Improve and optimize non-uniform Borders. (flutter#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.
1 parent fe9c7f4 commit b7046b3

File tree

4 files changed

+153
-58
lines changed

4 files changed

+153
-58
lines changed

packages/flutter/lib/src/painting/box_border.dart

Lines changed: 125 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -242,16 +242,24 @@ abstract class BoxBorder extends ShapeBorder {
242242
}
243243
}
244244

245-
static void _paintNonUniformBorder(
245+
/// Paints a Border with different widths, styles and strokeAligns, on any
246+
/// borderRadius while using a single color.
247+
///
248+
/// See also:
249+
///
250+
/// * [paintBorder], which supports multiple colors but not borderRadius.
251+
/// * [paint], which calls this method.
252+
static void paintNonUniformBorder(
246253
Canvas canvas,
247254
Rect rect, {
248255
required BorderRadius? borderRadius,
249-
required BoxShape shape,
250256
required TextDirection? textDirection,
251-
required BorderSide left,
252-
required BorderSide top,
253-
required BorderSide right,
254-
required BorderSide bottom,
257+
BoxShape shape = BoxShape.rectangle,
258+
BorderSide top = BorderSide.none,
259+
BorderSide right = BorderSide.none,
260+
BorderSide bottom = BorderSide.none,
261+
BorderSide left = BorderSide.none,
262+
required Color color,
255263
}) {
256264
final RRect borderRect;
257265
switch (shape) {
@@ -266,7 +274,7 @@ abstract class BoxBorder extends ShapeBorder {
266274
Radius.circular(rect.width),
267275
);
268276
}
269-
final Paint paint = Paint()..color = top.color;
277+
final Paint paint = Paint()..color = color;
270278
final RRect inner = _deflateRRect(borderRect, EdgeInsets.fromLTRB(left.strokeInset, top.strokeInset, right.strokeInset, bottom.strokeInset));
271279
final RRect outer = _inflateRRect(borderRect, EdgeInsets.fromLTRB(left.strokeOutset, top.strokeOutset, right.strokeOutset, bottom.strokeOutset));
272280
canvas.drawDRRect(outer, inner, paint);
@@ -489,6 +497,32 @@ class Border extends BoxBorder {
489497
&& right.strokeAlign == topStrokeAlign;
490498
}
491499

500+
Set<Color> _distinctVisibleColors() {
501+
final Set<Color> distinctVisibleColors = <Color>{};
502+
if (top.style != BorderStyle.none) {
503+
distinctVisibleColors.add(top.color);
504+
}
505+
if (right.style != BorderStyle.none) {
506+
distinctVisibleColors.add(right.color);
507+
}
508+
if (bottom.style != BorderStyle.none) {
509+
distinctVisibleColors.add(bottom.color);
510+
}
511+
if (left.style != BorderStyle.none) {
512+
distinctVisibleColors.add(left.color);
513+
}
514+
return distinctVisibleColors;
515+
}
516+
517+
// [BoxBorder.paintNonUniformBorder] is about 20% faster than [paintBorder],
518+
// but [paintBorder] is able to draw hairline borders when width is zero
519+
// and style is [BorderStyle.solid].
520+
bool get _hasHairlineBorder =>
521+
(top.style == BorderStyle.solid && top.width == 0.0) ||
522+
(right.style == BorderStyle.solid && right.width == 0.0) ||
523+
(bottom.style == BorderStyle.solid && bottom.width == 0.0) ||
524+
(left.style == BorderStyle.solid && left.width == 0.0);
525+
492526
@override
493527
Border? add(ShapeBorder other, { bool reversed = false }) {
494528
if (other is Border &&
@@ -603,50 +637,59 @@ class Border extends BoxBorder {
603637
}
604638
}
605639

606-
// Allow painting non-uniform borders if the color and style are uniform.
607-
if (_colorIsUniform && _styleIsUniform) {
608-
switch (top.style) {
609-
case BorderStyle.none:
610-
return;
611-
case BorderStyle.solid:
612-
BoxBorder._paintNonUniformBorder(canvas, rect,
613-
shape: shape,
614-
borderRadius: borderRadius,
615-
textDirection: textDirection,
616-
left: left,
617-
top: top,
618-
right: right,
619-
bottom: bottom);
620-
return;
621-
}
640+
if (_styleIsUniform && top.style == BorderStyle.none) {
641+
return;
622642
}
623643

624-
assert(() {
625-
if (borderRadius != null) {
644+
// Allow painting non-uniform borders if the visible colors are uniform.
645+
final Set<Color> visibleColors = _distinctVisibleColors();
646+
final bool hasHairlineBorder = _hasHairlineBorder;
647+
// Paint a non uniform border if a single color is visible
648+
// and (borderRadius is present) or (border is visible and width != 0.0).
649+
if (visibleColors.length == 1 &&
650+
!hasHairlineBorder &&
651+
(shape == BoxShape.circle ||
652+
(borderRadius != null && borderRadius != BorderRadius.zero))) {
653+
BoxBorder.paintNonUniformBorder(canvas, rect,
654+
shape: shape,
655+
borderRadius: borderRadius,
656+
textDirection: textDirection,
657+
top: top.style == BorderStyle.none ? BorderSide.none : top,
658+
right: right.style == BorderStyle.none ? BorderSide.none : right,
659+
bottom: bottom.style == BorderStyle.none ? BorderSide.none : bottom,
660+
left: left.style == BorderStyle.none ? BorderSide.none : left,
661+
color: visibleColors.first);
662+
return;
663+
}
664+
665+
assert(() {
666+
if (hasHairlineBorder) {
667+
assert(borderRadius == null || borderRadius == BorderRadius.zero,
668+
'A hairline border like `BorderSide(width: 0.0, style: BorderStyle.solid)` can only be drawn when BorderRadius is zero or null.');
669+
}
670+
if (borderRadius != null && borderRadius != BorderRadius.zero) {
626671
throw FlutterError.fromParts(<DiagnosticsNode>[
627-
ErrorSummary('A borderRadius can only be given on borders with uniform colors and styles.'),
672+
ErrorSummary('A borderRadius can only be given on borders with uniform colors.'),
628673
ErrorDescription('The following is not uniform:'),
629674
if (!_colorIsUniform) ErrorDescription('BorderSide.color'),
630-
if (!_styleIsUniform) ErrorDescription('BorderSide.style'),
631675
]);
632676
}
633677
return true;
634678
}());
635679
assert(() {
636680
if (shape != BoxShape.rectangle) {
637681
throw FlutterError.fromParts(<DiagnosticsNode>[
638-
ErrorSummary('A Border can only be drawn as a circle on borders with uniform colors and styles.'),
682+
ErrorSummary('A Border can only be drawn as a circle on borders with uniform colors.'),
639683
ErrorDescription('The following is not uniform:'),
640684
if (!_colorIsUniform) ErrorDescription('BorderSide.color'),
641-
if (!_styleIsUniform) ErrorDescription('BorderSide.style'),
642685
]);
643686
}
644687
return true;
645688
}());
646689
assert(() {
647690
if (!_strokeAlignIsUniform || top.strokeAlign != BorderSide.strokeAlignInside) {
648691
throw FlutterError.fromParts(<DiagnosticsNode>[
649-
ErrorSummary('A Border can only draw strokeAlign different than BorderSide.strokeAlignInside on borders with uniform colors and styles.'),
692+
ErrorSummary('A Border can only draw strokeAlign different than BorderSide.strokeAlignInside on borders with uniform colors.'),
650693
]);
651694
}
652695
return true;
@@ -806,6 +849,31 @@ class BorderDirectional extends BoxBorder {
806849
&& end.strokeAlign == topStrokeAlign;
807850
}
808851

852+
Set<Color> _distinctVisibleColors() {
853+
final Set<Color> distinctVisibleColors = <Color>{};
854+
if (top.style != BorderStyle.none) {
855+
distinctVisibleColors.add(top.color);
856+
}
857+
if (end.style != BorderStyle.none) {
858+
distinctVisibleColors.add(end.color);
859+
}
860+
if (bottom.style != BorderStyle.none) {
861+
distinctVisibleColors.add(bottom.color);
862+
}
863+
if (start.style != BorderStyle.none) {
864+
distinctVisibleColors.add(start.color);
865+
}
866+
867+
return distinctVisibleColors;
868+
}
869+
870+
871+
bool get _hasHairlineBorder =>
872+
(top.style == BorderStyle.solid && top.width == 0.0) ||
873+
(end.style == BorderStyle.solid && end.width == 0.0) ||
874+
(bottom.style == BorderStyle.solid && bottom.width == 0.0) ||
875+
(start.style == BorderStyle.solid && start.width == 0.0);
876+
809877
@override
810878
BoxBorder? add(ShapeBorder other, { bool reversed = false }) {
811879
if (other is BorderDirectional) {
@@ -951,6 +1019,10 @@ class BorderDirectional extends BoxBorder {
9511019
}
9521020
}
9531021

1022+
if (_styleIsUniform && top.style == BorderStyle.none) {
1023+
return;
1024+
}
1025+
9541026
final BorderSide left, right;
9551027
assert(textDirection != null, 'Non-uniform BorderDirectional objects require a TextDirection when painting.');
9561028
switch (textDirection!) {
@@ -962,27 +1034,31 @@ class BorderDirectional extends BoxBorder {
9621034
right = end;
9631035
}
9641036

965-
// Allow painting non-uniform borders if the color and style are uniform.
966-
if (_colorIsUniform && _styleIsUniform) {
967-
switch (top.style) {
968-
case BorderStyle.none:
969-
return;
970-
case BorderStyle.solid:
971-
BoxBorder._paintNonUniformBorder(canvas, rect,
972-
shape: shape,
973-
borderRadius: borderRadius,
974-
textDirection: textDirection,
975-
left: left,
976-
top: top,
977-
right: right,
978-
bottom: bottom);
979-
return;
980-
}
1037+
// Allow painting non-uniform borders if the visible colors are uniform.
1038+
final Set<Color> visibleColors = _distinctVisibleColors();
1039+
final bool hasHairlineBorder = _hasHairlineBorder;
1040+
if (visibleColors.length == 1 &&
1041+
!hasHairlineBorder &&
1042+
(shape == BoxShape.circle ||
1043+
(borderRadius != null && borderRadius != BorderRadius.zero))) {
1044+
BoxBorder.paintNonUniformBorder(canvas, rect,
1045+
shape: shape,
1046+
borderRadius: borderRadius,
1047+
textDirection: textDirection,
1048+
top: top.style == BorderStyle.none ? BorderSide.none : top,
1049+
right: right.style == BorderStyle.none ? BorderSide.none : right,
1050+
bottom: bottom.style == BorderStyle.none ? BorderSide.none : bottom,
1051+
left: left.style == BorderStyle.none ? BorderSide.none : left,
1052+
color: visibleColors.first);
1053+
return;
9811054
}
9821055

983-
assert(borderRadius == null, 'A borderRadius can only be given for borders with uniform colors and styles.');
984-
assert(shape == BoxShape.rectangle, 'A Border can only be drawn as a circle on borders with uniform colors and styles.');
985-
assert(_strokeAlignIsUniform && top.strokeAlign == BorderSide.strokeAlignInside, 'A Border can only draw strokeAlign different than strokeAlignInside on borders with uniform colors and styles.');
1056+
if (hasHairlineBorder) {
1057+
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.');
1058+
}
1059+
assert(borderRadius == null, 'A borderRadius can only be given for borders with uniform colors.');
1060+
assert(shape == BoxShape.rectangle, 'A Border can only be drawn as a circle on borders with uniform colors.');
1061+
assert(_strokeAlignIsUniform && top.strokeAlign == BorderSide.strokeAlignInside, 'A Border can only draw strokeAlign different than strokeAlignInside on borders with uniform colors.');
9861062

9871063
paintBorder(canvas, rect, top: top, left: left, bottom: bottom, right: right);
9881064
}

packages/flutter/test/material/data_table_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1750,7 +1750,7 @@ void main() {
17501750
);
17511751
expect(
17521752
find.ancestor(of: find.byType(Table), matching: find.byType(Container)),
1753-
paints..drrect(color: borderColor),
1753+
paints..path(color: borderColor),
17541754
);
17551755
expect(
17561756
tester.getTopLeft(find.byType(Table)),

packages/flutter/test/material/divider_test.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,9 @@ void main() {
136136

137137
testWidgets('Vertical Divider Test 2', (WidgetTester tester) async {
138138
await tester.pumpWidget(
139-
const MaterialApp(
140-
home: Material(
139+
MaterialApp(
140+
theme: ThemeData(useMaterial3: false),
141+
home: const Material(
141142
child: SizedBox(
142143
height: 24.0,
143144
child: Row(

packages/flutter/test/painting/border_test.dart

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ void main() {
273273
expect(error.diagnostics.length, 1);
274274
expect(
275275
error.diagnostics[0].toStringDeep(),
276-
'A Border can only draw strokeAlign different than\nBorderSide.strokeAlignInside on borders with uniform colors and\nstyles.\n',
276+
'A Border can only draw strokeAlign different than\nBorderSide.strokeAlignInside on borders with uniform colors.\n',
277277
);
278278
});
279279

@@ -341,8 +341,8 @@ void main() {
341341

342342
// This falls into non-uniform border because of strokeAlign.
343343
await tester.pumpWidget(buildWidget(border: allowedBorderVariations));
344-
expect(tester.takeException(), isNull,
345-
reason: 'Border with non-uniform strokeAlign should not fail.');
344+
expect(tester.takeException(), isAssertionError,
345+
reason: 'Border with non-uniform strokeAlign should fail.');
346346

347347
await tester.pumpWidget(buildWidget(
348348
border: allowedBorderVariations,
@@ -364,8 +364,8 @@ void main() {
364364
borderRadius: BorderRadius.circular(25),
365365
),
366366
);
367-
expect(tester.takeException(), isAssertionError,
368-
reason: 'Border with non-uniform styles should fail with borderRadius.');
367+
expect(tester.takeException(), isNull,
368+
reason: 'Border with non-uniform styles should work with borderRadius.');
369369

370370
await tester.pumpWidget(
371371
buildWidget(
@@ -381,6 +381,24 @@ void main() {
381381
expect(tester.takeException(), isAssertionError,
382382
reason: 'Border with non-uniform colors should fail with borderRadius.');
383383

384+
await tester.pumpWidget(
385+
buildWidget(
386+
border: const Border(bottom: BorderSide(width: 0)),
387+
borderRadius: BorderRadius.zero,
388+
),
389+
);
390+
expect(tester.takeException(), isNull,
391+
reason: 'Border with a side.width == 0 should work without borderRadius (hairline border).');
392+
393+
await tester.pumpWidget(
394+
buildWidget(
395+
border: const Border(bottom: BorderSide(width: 0)),
396+
borderRadius: BorderRadius.circular(40),
397+
),
398+
);
399+
expect(tester.takeException(), isAssertionError,
400+
reason: 'Border with width == 0 and borderRadius should fail (hairline border).');
401+
384402
// Tests for BorderDirectional.
385403
const BorderDirectional allowedBorderDirectionalVariations = BorderDirectional(
386404
start: BorderSide(width: 5),
@@ -390,7 +408,7 @@ void main() {
390408
);
391409

392410
await tester.pumpWidget(buildWidget(border: allowedBorderDirectionalVariations));
393-
expect(tester.takeException(), isNull);
411+
expect(tester.takeException(), isAssertionError);
394412

395413
await tester.pumpWidget(buildWidget(
396414
border: allowedBorderDirectionalVariations,

0 commit comments

Comments
 (0)