From 80d6095d98f362cd1a117dd093336ae52c05785b Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Fri, 12 Jan 2018 15:03:51 -0800 Subject: [PATCH] Fix ShapeDecoration (#14009) And add a test that catches these problems. --- .../flutter/lib/src/painting/borders.dart | 4 +- .../lib/src/painting/circle_border.dart | 2 +- .../flutter/lib/src/painting/decoration.dart | 9 ++ .../lib/src/painting/image_provider.dart | 2 +- .../lib/src/painting/shape_decoration.dart | 6 +- .../test/painting/mocks_for_image_cache.dart | 5 +- .../test/widgets/shape_decoration_test.dart | 137 ++++++++++++++++++ 7 files changed, 156 insertions(+), 9 deletions(-) create mode 100644 packages/flutter/test/widgets/shape_decoration_test.dart diff --git a/packages/flutter/lib/src/painting/borders.dart b/packages/flutter/lib/src/painting/borders.dart index 0bc4338da7..6f6c8e6f5d 100644 --- a/packages/flutter/lib/src/painting/borders.dart +++ b/packages/flutter/lib/src/painting/borders.dart @@ -596,12 +596,12 @@ class _CompoundBorder extends ShapeBorder { Path getInnerPath(Rect rect, { TextDirection textDirection }) { for (int index = 0; index < borders.length - 1; index += 1) rect = borders[index].dimensions.resolve(textDirection).deflateRect(rect); - return borders.last.getInnerPath(rect); + return borders.last.getInnerPath(rect, textDirection: textDirection); } @override Path getOuterPath(Rect rect, { TextDirection textDirection }) { - return borders.first.getOuterPath(rect); + return borders.first.getOuterPath(rect, textDirection: textDirection); } @override diff --git a/packages/flutter/lib/src/painting/circle_border.dart b/packages/flutter/lib/src/painting/circle_border.dart index 76396c23c3..575b05bfb2 100644 --- a/packages/flutter/lib/src/painting/circle_border.dart +++ b/packages/flutter/lib/src/painting/circle_border.dart @@ -25,7 +25,7 @@ class CircleBorder extends ShapeBorder { /// Create a circle border. /// /// The [side] argument must not be null. - const CircleBorder({ this.side = BorderSide.none }) : assert(side != null); + const CircleBorder({ this.side: BorderSide.none }) : assert(side != null); /// The style of this border. final BorderSide side; diff --git a/packages/flutter/lib/src/painting/decoration.dart b/packages/flutter/lib/src/painting/decoration.dart index e27252d58f..2c7d899208 100644 --- a/packages/flutter/lib/src/painting/decoration.dart +++ b/packages/flutter/lib/src/painting/decoration.dart @@ -162,6 +162,11 @@ abstract class Decoration extends Diagnosticable { /// `textDirection` argument should therefore be provided. If it is known that /// the decoration is not affected by the text direction, then the argument /// may be omitted or set to null. + /// + /// When a [Decoration] is painted in a [Container] or [DecoratedBox] (which + /// is what [Container] uses), the `textDirection` parameter will be populated + /// based on the ambient [Directionality] (by way of the [RenderDecoratedBox] + /// renderer). bool hitTest(Size size, Offset position, { TextDirection textDirection }) => true; /// Returns a [BoxPainter] that will paint this decoration. @@ -207,6 +212,10 @@ abstract class BoxPainter { /// Implementations should paint their decorations on the canvas in a /// rectangle whose top left corner is at the given `offset` and whose size is /// given by `configuration.size`. + /// + /// When a [Decoration] is painted in a [Container] or [DecoratedBox] (which + /// is what [Container] uses), the [ImageConfiguration.textDirection] property + /// will be populated based on the ambient [Directionality]. void paint(Canvas canvas, Offset offset, ImageConfiguration configuration); /// Callback that is invoked if an asynchronously-loading resource used by the diff --git a/packages/flutter/lib/src/painting/image_provider.dart b/packages/flutter/lib/src/painting/image_provider.dart index a91502e144..c2df735b52 100644 --- a/packages/flutter/lib/src/painting/image_provider.dart +++ b/packages/flutter/lib/src/painting/image_provider.dart @@ -118,7 +118,7 @@ class ImageConfiguration { if (devicePixelRatio != null) { if (hasArguments) result.write(', '); - result.write('devicePixelRatio: $devicePixelRatio'); + result.write('devicePixelRatio: ${devicePixelRatio.toStringAsFixed(1)}'); hasArguments = true; } if (locale != null) { diff --git a/packages/flutter/lib/src/painting/shape_decoration.dart b/packages/flutter/lib/src/painting/shape_decoration.dart index 79b4825712..33d3138a63 100644 --- a/packages/flutter/lib/src/painting/shape_decoration.dart +++ b/packages/flutter/lib/src/painting/shape_decoration.dart @@ -323,13 +323,13 @@ class _ShapeDecorationPainter extends BoxPainter { } for (int index = 0; index < _shadowCount; index += 1) { final BoxShadow shadow = _decoration.shadows[index]; - _shadowPaths[index] = _decoration.shape.getOuterPath(rect.shift(shadow.offset).inflate(shadow.spreadRadius)); + _shadowPaths[index] = _decoration.shape.getOuterPath(rect.shift(shadow.offset).inflate(shadow.spreadRadius), textDirection: textDirection); } } if (_interiorPaint != null || _shadowCount != null) - _outerPath = _decoration.shape.getOuterPath(rect); + _outerPath = _decoration.shape.getOuterPath(rect, textDirection: textDirection); if (_decoration.image != null) - _innerPath = _decoration.shape.getInnerPath(rect); + _innerPath = _decoration.shape.getInnerPath(rect, textDirection: textDirection); _lastRect = rect; _lastTextDirection = textDirection; diff --git a/packages/flutter/test/painting/mocks_for_image_cache.dart b/packages/flutter/test/painting/mocks_for_image_cache.dart index b910a8e24a..8cb612ea04 100644 --- a/packages/flutter/test/painting/mocks_for_image_cache.dart +++ b/packages/flutter/test/painting/mocks_for_image_cache.dart @@ -24,9 +24,10 @@ class TestImageInfo implements ImageInfo { } class TestImageProvider extends ImageProvider { - const TestImageProvider(this.key, this.imageValue); + const TestImageProvider(this.key, this.imageValue, { this.image }); final int key; final int imageValue; + final ui.Image image; @override Future obtainKey(ImageConfiguration configuration) { @@ -36,7 +37,7 @@ class TestImageProvider extends ImageProvider { @override ImageStreamCompleter load(int key) { return new OneFrameImageStreamCompleter( - new SynchronousFuture(new TestImageInfo(imageValue)) + new SynchronousFuture(new TestImageInfo(imageValue, image: image)) ); } diff --git a/packages/flutter/test/widgets/shape_decoration_test.dart b/packages/flutter/test/widgets/shape_decoration_test.dart new file mode 100644 index 0000000000..3c8dc18d9c --- /dev/null +++ b/packages/flutter/test/widgets/shape_decoration_test.dart @@ -0,0 +1,137 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:typed_data'; +import 'dart:ui' as ui show Image; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import '../painting/image_data.dart'; +import '../painting/mocks_for_image_cache.dart'; +import '../rendering/mock_canvas.dart'; + +Future main() async { + final ui.Image rawImage = await decodeImageFromList(new Uint8List.fromList(kTransparentImage)); + final ImageProvider image = new TestImageProvider(0, 0, image: rawImage); + testWidgets('ShapeDecoration.image', (WidgetTester tester) async { + await tester.pumpWidget( + new MaterialApp( + home: new DecoratedBox( + decoration: new ShapeDecoration( + shape: new Border.all(width: 1.0, color: Colors.white) + + new Border.all(width: 1.0, color: Colors.black), + image: new DecorationImage( + image: image, + ), + ), + ), + ), + ); + expect( + find.byType(DecoratedBox), + paints + ..drawImageRect(image: rawImage) + ..rect(color: Colors.black) + ..rect(color: Colors.white) + ); + }); + + testWidgets('ShapeDecoration.color', (WidgetTester tester) async { + await tester.pumpWidget( + new MaterialApp( + home: new DecoratedBox( + decoration: new ShapeDecoration( + shape: new Border.all(width: 1.0, color: Colors.white) + + new Border.all(width: 1.0, color: Colors.black), + color: Colors.blue, + ), + ), + ), + ); + expect( + find.byType(DecoratedBox), + paints + ..path(color: new Color(Colors.blue.value)) + ..rect(color: Colors.black) + ..rect(color: Colors.white) + ); + }); + + testWidgets('TestBorder and Directionality - 1', (WidgetTester tester) async { + final List log = []; + await tester.pumpWidget( + new MaterialApp( + home: new DecoratedBox( + decoration: new ShapeDecoration( + shape: new TestBorder(log.add), + color: Colors.green, + ), + ), + ), + ); + expect( + log, + [ + 'getOuterPath Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) TextDirection.ltr', + 'paint Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) TextDirection.ltr' + ], + ); + }); + + testWidgets('TestBorder and Directionality - 2', (WidgetTester tester) async { + final List log = []; + await tester.pumpWidget( + new Directionality( + textDirection: TextDirection.rtl, + child: new DecoratedBox( + decoration: new ShapeDecoration( + shape: new TestBorder(log.add), + image: new DecorationImage( + image: image, + ), + ), + ), + ), + ); + expect( + log, + [ + 'getInnerPath Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) TextDirection.rtl', + 'paint Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) TextDirection.rtl' + ], + ); + }); +} + +typedef void Logger(String caller); + +class TestBorder extends ShapeBorder { + const TestBorder(this.onLog) : assert(onLog != null); + + final Logger onLog; + + @override + EdgeInsetsGeometry get dimensions => const EdgeInsetsDirectional.only(start: 1.0); + + @override + ShapeBorder scale(double t) => new TestBorder(onLog); + + @override + Path getInnerPath(Rect rect, { TextDirection textDirection }) { + onLog('getInnerPath $rect $textDirection'); + return new Path(); + } + + @override + Path getOuterPath(Rect rect, { TextDirection textDirection }) { + onLog('getOuterPath $rect $textDirection'); + return new Path(); + } + + @override + void paint(Canvas canvas, Rect rect, { TextDirection textDirection }) { + onLog('paint $rect $textDirection'); + } +}