Skip to content

F
Front-End
Unverified Commit b7046b32 authored by Bernardo Ferrari's avatar Bernardo Ferrari Committed by GitHub
Browse files
Options

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.
parent fe9c7f4f
Hide whitespace changes
Inline Side-by-side
Showing with 153 additions and 58 deletions
packages/flutter/lib/src/painting/box_border.dart
View file @ b7046b32
......@@ -242,16 +242,24 @@ abstract class BoxBorder extends ShapeBorder {
}
}
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 @@ abstract class BoxBorder extends ShapeBorder {
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 @@ class Border extends BoxBorder {
&& 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 @@ class Border extends BoxBorder {
}
}
// 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 @@ class Border extends BoxBorder {
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 @@ class Border extends BoxBorder {
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 @@ class BorderDirectional extends BoxBorder {
&& 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 @@ class BorderDirectional extends BoxBorder {
}
}
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 @@ class BorderDirectional extends BoxBorder {
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);
}
......
packages/flutter/test/material/data_table_test.dart
View file @ b7046b32
......@@ -1750,7 +1750,7 @@ void main() {
);
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)),
......
packages/flutter/test/material/divider_test.dart
View file @ b7046b32
......@@ -136,8 +136,9 @@ void main() {
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(
......
packages/flutter/test/painting/border_test.dart
View file @ b7046b32
......@@ -273,7 +273,7 @@ void main() {
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 @@ void main() {
// 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 @@ void main() {
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 @@ void main() {
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 @@ void main() {
);
await tester.pumpWidget(buildWidget(border: allowedBorderDirectionalVariations));
expect(tester.takeException(), isNull);
expect(tester.takeException(), isAssertionError);
await tester.pumpWidget(buildWidget(
border: allowedBorderDirectionalVariations,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment

HIAST - GitLab