Unverified Commit 2afec776 authored by Bernardo Ferrari's avatar Bernardo Ferrari Committed by GitHub

Make `UnderlineInputBorder` consistent (#124153)

This was easy to implement. I like the result, I think `borderRadius.zero` -> `borderRadius.circular` makes a nice transition, and many places (like macOS) use an effect similar to this PR, while Google doesn't use anywhere (yet). I'm curious if it is going to break goldens or google testing.

<img width="954" alt="ttt" src="https://user-images.githubusercontent.com/351125/229918871-9f2ab851-6b41-44f5-80b9-c7928a8c0014.png">

What do you think? cc @HansMuller @gspencergoog. Is this something you want, should I ask the community, or do you prefer the current one?

Side effects:
- This makes strokeAlign work with `UnderlineInputBorder` (TODO: fix `drawLine` when borderRadius is zero).
- This is faster than the current implementation (clip is slow on Skia). 🚀 
- We could just call `BoxBorder._paintNonUniformBorder` (if it weren't private). Single LOC implementation.
- Web does this by default:
![image](https://user-images.githubusercontent.com/351125/233448671-90ce62ff-be91-40ca-8007-e82b57f3272e.png)
- Apparently no tests fail and most usages around (via code search) seem to be without a borderRadius.
parent f796e62c
......@@ -244,9 +244,22 @@ class UnderlineInputBorder extends InputBorder {
TextDirection? textDirection,
}) {
if (borderRadius.bottomLeft != Radius.zero || borderRadius.bottomRight != Radius.zero) {
canvas.clipPath(getOuterPath(rect, textDirection: textDirection));
// This prevents the border from leaking the color due to anti-aliasing rounding errors.
final BorderRadius updatedBorderRadius = BorderRadius.only(
bottomLeft: borderRadius.bottomLeft.clamp(maximum: Radius.circular(rect.height / 2)),
bottomRight: borderRadius.bottomRight.clamp(maximum: Radius.circular(rect.height / 2)),
);
// We set the strokeAlign to center, so the behavior is consistent with
// drawLine and with the historical behavior of this border.
BoxBorder.paintNonUniformBorder(canvas, rect,
textDirection: textDirection,
borderRadius: updatedBorderRadius,
bottom: borderSide.copyWith(strokeAlign: BorderSide.strokeAlignCenter),
color: borderSide.color);
} else {
canvas.drawLine(rect.bottomLeft, rect.bottomRight, borderSide.toPaint());
}
canvas.drawLine(rect.bottomLeft, rect.bottomRight, borderSide.toPaint());
}
@override
......
......@@ -5000,14 +5000,15 @@ void runAllTests({ required bool useMaterial3 }) {
final RenderBox box = tester.renderObject(find.byType(InputDecorator));
// Fill is the border's outer path, a rounded rectangle
expect(box, paints..path(
expect(box, paints
..drrect(
style: PaintingStyle.fill,
color: const Color(0xFF00FF00),
includes: <Offset>[const Offset(800.0/2.0, 56/2.0)],
excludes: <Offset>[
const Offset(1.0, 56.0 - 6.0), // bottom left
const Offset(800 - 1.0, 56.0 - 6.0), // bottom right
],
inner: RRect.fromLTRBAndCorners(0.0, 0.0, 800.0, 47.5,
bottomRight: const Radius.elliptical(12.0, 11.5),
bottomLeft: const Radius.elliptical(12.0, 11.5)),
outer: RRect.fromLTRBAndCorners(0.0, 0.0, 800.0, 48.5,
bottomRight: const Radius.elliptical(12.0, 12.5),
bottomLeft: const Radius.elliptical(12.0, 12.5)),
));
});
......@@ -6934,4 +6935,40 @@ testWidgetsWithLeakTracking('OutlineInputBorder with BorderRadius.zero should dr
expect(decoration.isCollapsed, true);
});
});
testWidgets('UnderlineInputBorder clips top border to prevent anti-aliasing glitches', (WidgetTester tester) async {
const Rect canvasRect = Rect.fromLTWH(0, 0, 100, 100);
const UnderlineInputBorder border = UnderlineInputBorder(
borderRadius: BorderRadius.all(Radius.circular(12.0)),
);
expect(
(Canvas canvas) => border.paint(canvas, canvasRect),
paints
..drrect(
outer: RRect.fromLTRBAndCorners(0.0, 0.0, 100.0, 100.5,
bottomRight: const Radius.elliptical(12.0, 12.5),
bottomLeft: const Radius.elliptical(12.0, 12.5)),
inner: RRect.fromLTRBAndCorners(0.0, 0.0, 100.0, 99.5,
bottomRight: const Radius.elliptical(12.0, 11.5),
bottomLeft: const Radius.elliptical(12.0, 11.5)),
),
);
const UnderlineInputBorder border2 = UnderlineInputBorder(
borderRadius: BorderRadius.all(Radius.circular(60.0)),
);
expect(
(Canvas canvas) => border2.paint(canvas, canvasRect),
paints
..drrect(
outer: RRect.fromLTRBAndCorners(0.0, 0.0, 100.0, 100.5,
bottomRight: const Radius.elliptical(50.0, 50.5),
bottomLeft: const Radius.elliptical(50.0, 50.5)),
inner: RRect.fromLTRBAndCorners(0.0, 0.0, 100.0, 99.5,
bottomRight: const Radius.elliptical(50.0, 49.5),
bottomLeft: const Radius.elliptical(50.0, 49.5)),
),
reason: 'clamp is expected',
);
});
}
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