Unverified Commit 1a010de8 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Make Container always clip a decoration if it has a clip edge (#64840)

This allows us to rationalize the nullability of some of our APIs.
Prior to this we'd end up in a situation where Container assumed that
getClipPath would be non-null but Decoration was ok with return null.
parent 31e5db7a
...@@ -64,17 +64,6 @@ class UnderlineTabIndicator extends Decoration { ...@@ -64,17 +64,6 @@ class UnderlineTabIndicator extends Decoration {
_UnderlinePainter createBoxPainter([ VoidCallback onChanged ]) { _UnderlinePainter createBoxPainter([ VoidCallback onChanged ]) {
return _UnderlinePainter(this, onChanged); return _UnderlinePainter(this, onChanged);
} }
}
class _UnderlinePainter extends BoxPainter {
_UnderlinePainter(this.decoration, VoidCallback onChanged)
: assert(decoration != null),
super(onChanged);
final UnderlineTabIndicator decoration;
BorderSide get borderSide => decoration.borderSide;
EdgeInsetsGeometry get insets => decoration.insets;
Rect _indicatorRectFor(Rect rect, TextDirection textDirection) { Rect _indicatorRectFor(Rect rect, TextDirection textDirection) {
assert(rect != null); assert(rect != null);
...@@ -88,14 +77,27 @@ class _UnderlinePainter extends BoxPainter { ...@@ -88,14 +77,27 @@ class _UnderlinePainter extends BoxPainter {
); );
} }
@override
Path getClipPath(Rect rect, TextDirection textDirection) {
return Path()..addRect(_indicatorRectFor(rect, textDirection));
}
}
class _UnderlinePainter extends BoxPainter {
_UnderlinePainter(this.decoration, VoidCallback onChanged)
: assert(decoration != null),
super(onChanged);
final UnderlineTabIndicator decoration;
@override @override
void paint(Canvas canvas, Offset offset, ImageConfiguration configuration) { void paint(Canvas canvas, Offset offset, ImageConfiguration configuration) {
assert(configuration != null); assert(configuration != null);
assert(configuration.size != null); assert(configuration.size != null);
final Rect rect = offset & configuration.size; final Rect rect = offset & configuration.size;
final TextDirection textDirection = configuration.textDirection; final TextDirection textDirection = configuration.textDirection;
final Rect indicator = _indicatorRectFor(rect, textDirection).deflate(borderSide.width / 2.0); final Rect indicator = decoration._indicatorRectFor(rect, textDirection).deflate(decoration.borderSide.width / 2.0);
final Paint paint = borderSide.toPaint()..strokeCap = StrokeCap.square; final Paint paint = decoration.borderSide.toPaint()..strokeCap = StrokeCap.square;
canvas.drawLine(indicator.bottomLeft, indicator.bottomRight, paint); canvas.drawLine(indicator.bottomLeft, indicator.bottomRight, paint);
} }
} }
...@@ -215,21 +215,18 @@ class BoxDecoration extends Decoration { ...@@ -215,21 +215,18 @@ class BoxDecoration extends Decoration {
EdgeInsetsGeometry? get padding => border?.dimensions; EdgeInsetsGeometry? get padding => border?.dimensions;
@override @override
Path? getClipPath(Rect rect, TextDirection textDirection) { Path getClipPath(Rect rect, TextDirection textDirection) {
Path? clipPath;
switch (shape) { switch (shape) {
case BoxShape.circle: case BoxShape.circle:
final Offset center = rect.center; final Offset center = rect.center;
final double radius = rect.shortestSide / 2.0; final double radius = rect.shortestSide / 2.0;
final Rect square = Rect.fromCircle(center: center, radius: radius); final Rect square = Rect.fromCircle(center: center, radius: radius);
clipPath = Path()..addOval(square); return Path()..addOval(square);
break;
case BoxShape.rectangle: case BoxShape.rectangle:
if (borderRadius != null) if (borderRadius != null)
clipPath = Path()..addRRect(borderRadius!.resolve(textDirection).toRRect(rect)); return Path()..addRRect(borderRadius!.resolve(textDirection).toRRect(rect));
break; return Path()..addRect(rect);
} }
return clipPath;
} }
/// Returns a new box decoration that is scaled by the given factor. /// Returns a new box decoration that is scaled by the given factor.
......
...@@ -169,7 +169,18 @@ abstract class Decoration with Diagnosticable { ...@@ -169,7 +169,18 @@ abstract class Decoration with Diagnosticable {
BoxPainter createBoxPainter([ VoidCallback onChanged ]); BoxPainter createBoxPainter([ VoidCallback onChanged ]);
/// Returns a closed [Path] that describes the outer edge of this decoration. /// Returns a closed [Path] that describes the outer edge of this decoration.
Path? getClipPath(Rect rect, TextDirection textDirection) => null; ///
/// The default implementation throws. Subclasses must override this implementation
/// to describe the clip path that should be applied to the decoration when it is
/// used in a [Container] with an explicit [Clip] behavior.
///
/// See also:
///
/// * [Container.clipBehavior], which, if set, uses this method to determine
/// the clip path to use.
Path getClipPath(Rect rect, TextDirection textDirection) {
throw UnsupportedError('${objectRuntimeType(this, 'This Decoration subclass')} does not expect to be used for clipping.');
}
} }
/// A stateful class that can paint a particular [Decoration]. /// A stateful class that can paint a particular [Decoration].
......
...@@ -173,6 +173,11 @@ class FlutterLogoDecoration extends Decoration { ...@@ -173,6 +173,11 @@ class FlutterLogoDecoration extends Decoration {
return _FlutterLogoPainter(this); return _FlutterLogoPainter(this);
} }
@override
Path getClipPath(Rect rect, TextDirection textDirection) {
return Path()..addRect(rect);
}
@override @override
bool operator ==(Object other) { bool operator ==(Object other) {
assert(debugAssertIsValid()); assert(debugAssertIsValid());
......
...@@ -1523,9 +1523,9 @@ class RenderClipOval extends _RenderCustomClip<Rect> { ...@@ -1523,9 +1523,9 @@ class RenderClipOval extends _RenderCustomClip<Rect> {
super(child: child, clipper: clipper, clipBehavior: clipBehavior); super(child: child, clipper: clipper, clipBehavior: clipBehavior);
Rect? _cachedRect; Rect? _cachedRect;
Path? _cachedPath; late Path _cachedPath;
Path? _getClipPath(Rect rect) { Path _getClipPath(Rect rect) {
if (rect != _cachedRect) { if (rect != _cachedRect) {
_cachedRect = rect; _cachedRect = rect;
_cachedPath = Path()..addOval(_cachedRect!); _cachedPath = Path()..addOval(_cachedRect!);
...@@ -1558,7 +1558,7 @@ class RenderClipOval extends _RenderCustomClip<Rect> { ...@@ -1558,7 +1558,7 @@ class RenderClipOval extends _RenderCustomClip<Rect> {
needsCompositing, needsCompositing,
offset, offset,
_clip!, _clip!,
_getClipPath(_clip!)!, _getClipPath(_clip!),
super.paint, super.paint,
clipBehavior: clipBehavior, clipBehavior: clipBehavior,
oldLayer: layer as ClipPathLayer, oldLayer: layer as ClipPathLayer,
...@@ -1573,7 +1573,7 @@ class RenderClipOval extends _RenderCustomClip<Rect> { ...@@ -1573,7 +1573,7 @@ class RenderClipOval extends _RenderCustomClip<Rect> {
assert(() { assert(() {
if (child != null) { if (child != null) {
super.debugPaintSize(context, offset); super.debugPaintSize(context, offset);
context.canvas.drawPath(_getClipPath(_clip!)!.shift(offset), _debugPaint!); context.canvas.drawPath(_getClipPath(_clip!).shift(offset), _debugPaint!);
_debugText!.paint(context.canvas, offset + Offset((_clip!.width - _debugText!.width) / 2.0, -_debugText!.text!.style!.fontSize! * 1.1)); _debugText!.paint(context.canvas, offset + Offset((_clip!.width - _debugText!.width) / 2.0, -_debugText!.text!.style!.fontSize! * 1.1));
} }
return true; return true;
......
...@@ -281,6 +281,7 @@ class Container extends StatelessWidget { ...@@ -281,6 +281,7 @@ class Container extends StatelessWidget {
assert(decoration == null || decoration.debugAssertIsValid()), assert(decoration == null || decoration.debugAssertIsValid()),
assert(constraints == null || constraints.debugAssertIsValid()), assert(constraints == null || constraints.debugAssertIsValid()),
assert(clipBehavior != null), assert(clipBehavior != null),
assert(decoration != null || clipBehavior == Clip.none),
assert(color == null || decoration == null, assert(color == null || decoration == null,
'Cannot provide both a color and a decoration\n' 'Cannot provide both a color and a decoration\n'
'To provide both, use "decoration: BoxDecoration(color: color)".' 'To provide both, use "decoration: BoxDecoration(color: color)".'
...@@ -361,9 +362,14 @@ class Container extends StatelessWidget { ...@@ -361,9 +362,14 @@ class Container extends StatelessWidget {
/// The transformation matrix to apply before painting the container. /// The transformation matrix to apply before painting the container.
final Matrix4 transform; final Matrix4 transform;
/// The clip behavior when [Container.decoration] has a clipPath. /// The clip behavior when [Container.decoration] is not null.
/// ///
/// Defaults to [Clip.none]. /// Defaults to [Clip.none]. Must be [Clip.none] if [decoration] is null.
///
/// If a clip is to be applied, the [Decoration.getClipPath] method
/// for the provided decoration must return a clip path. (This is not
/// supported by all decorations; the default implementation of that
/// method throws an [UnsupportedError].)
final Clip clipBehavior; final Clip clipBehavior;
EdgeInsetsGeometry get _paddingIncludingDecoration { EdgeInsetsGeometry get _paddingIncludingDecoration {
...@@ -398,6 +404,7 @@ class Container extends StatelessWidget { ...@@ -398,6 +404,7 @@ class Container extends StatelessWidget {
current = ColoredBox(color: color, child: current); current = ColoredBox(color: color, child: current);
if (clipBehavior != Clip.none) { if (clipBehavior != Clip.none) {
assert(decoration != null);
current = ClipPath( current = ClipPath(
clipper: _DecorationClipper( clipper: _DecorationClipper(
textDirection: Directionality.of(context), textDirection: Directionality.of(context),
...@@ -453,7 +460,7 @@ class _DecorationClipper extends CustomClipper<Path> { ...@@ -453,7 +460,7 @@ class _DecorationClipper extends CustomClipper<Path> {
_DecorationClipper({ _DecorationClipper({
TextDirection textDirection, TextDirection textDirection,
@required this.decoration @required this.decoration
}) : assert (decoration != null), }) : assert(decoration != null),
textDirection = textDirection ?? TextDirection.ltr; textDirection = textDirection ?? TextDirection.ltr;
final TextDirection textDirection; final TextDirection textDirection;
......
...@@ -502,6 +502,36 @@ void main() { ...@@ -502,6 +502,36 @@ void main() {
); );
}); });
testWidgets('getClipPath() works for lots of kinds of decorations', (WidgetTester tester) async {
Future<void> test(Decoration decoration) async {
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.rtl,
child: Center(
child: SizedBox(
width: 100.0,
height: 100.0,
child: RepaintBoundary(
child: Container(
clipBehavior: Clip.hardEdge,
decoration: decoration,
child: ColoredBox(
color: Colors.yellow.withOpacity(0.5),
),
),
),
),
),
),
);
await expectLater(find.byType(Container), matchesGoldenFile('container_test.getClipPath.${decoration.runtimeType}.png'));
}
await test(const BoxDecoration());
await test(const UnderlineTabIndicator());
await test(const ShapeDecoration(shape: StadiumBorder()));
await test(const FlutterLogoDecoration());
});
testWidgets('Container is hittable only when having decorations', (WidgetTester tester) async { testWidgets('Container is hittable only when having decorations', (WidgetTester tester) async {
bool tapped = false; bool tapped = false;
await tester.pumpWidget(GestureDetector( await tester.pumpWidget(GestureDetector(
......
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