Factor out painting logic for DecorationImage (#12646)

This avoids some code duplication that existed before and will make
further modifications easier.

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

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 import 'dart:math' as math;
-import 'dart:ui' as ui show Image;
 
 import 'package:flutter/foundation.dart';
 import 'package:flutter/services.dart';
@@ -305,9 +304,9 @@ class BoxDecoration extends Decoration {
 
 /// An object that paints a [BoxDecoration] into a canvas.
 class _BoxDecorationPainter extends BoxPainter {
-  _BoxDecorationPainter(this._decoration, VoidCallback onChange)
+  _BoxDecorationPainter(this._decoration, VoidCallback onChanged)
     : assert(_decoration != null),
-      super(onChange);
+      super(onChanged);
 
   final BoxDecoration _decoration;
 
@@ -367,74 +366,27 @@ class _BoxDecorationPainter extends BoxPainter {
       _paintBox(canvas, rect, _getBackgroundPaint(rect), textDirection);
   }
 
-  ImageStream _imageStream;
-  ImageInfo _image;
-
+  DecorationImagePainter _imagePainter;
   void _paintBackgroundImage(Canvas canvas, Rect rect, ImageConfiguration configuration) {
-    // TODO(ianh): factor this out into a DecorationImage.paint method.
-    final DecorationImage backgroundImage = _decoration.image;
-    if (backgroundImage == null)
+    if (_decoration.image == null)
       return;
-
-    bool flipHorizontally = false;
-    if (backgroundImage.matchTextDirection) {
-      // We check this first so that the assert will fire immediately, not just when the
-      // image is ready.
-      assert(configuration.textDirection != null, 'matchTextDirection can only be used when a TextDirection is available.');
-      if (configuration.textDirection == TextDirection.rtl)
-        flipHorizontally = true;
-    }
-
-    final ImageStream newImageStream = backgroundImage.image.resolve(configuration);
-    if (newImageStream.key != _imageStream?.key) {
-      _imageStream?.removeListener(_imageListener);
-      _imageStream = newImageStream;
-      _imageStream.addListener(_imageListener);
-    }
-    final ui.Image image = _image?.image;
-    if (image == null)
-      return;
-
+    _imagePainter ??= _decoration.image.createPainter(onChanged);
     Path clipPath;
-    if (_decoration.shape == BoxShape.circle)
-      clipPath = new Path()..addOval(rect);
-    else if (_decoration.borderRadius != null)
-      clipPath = new Path()..addRRect(_decoration.borderRadius.resolve(configuration.textDirection).toRRect(rect));
-    if (clipPath != null) {
-      canvas.save();
-      canvas.clipPath(clipPath);
+    switch (_decoration.shape) {
+      case BoxShape.circle:
+        clipPath = new Path()..addOval(rect);
+        break;
+      case BoxShape.rectangle:
+        if (_decoration.borderRadius != null)
+          clipPath = new Path()..addRRect(_decoration.borderRadius.resolve(configuration.textDirection).toRRect(rect));
+        break;
     }
-
-    paintImage(
-      canvas: canvas,
-      rect: rect,
-      image: image,
-      colorFilter: backgroundImage.colorFilter,
-      fit: backgroundImage.fit,
-      alignment: backgroundImage.alignment.resolve(configuration.textDirection),
-      centerSlice: backgroundImage.centerSlice,
-      repeat: backgroundImage.repeat,
-      flipHorizontally: flipHorizontally,
-    );
-
-    if (clipPath != null)
-      canvas.restore();
-  }
-
-  void _imageListener(ImageInfo value, bool synchronousCall) {
-    if (_image == value)
-      return;
-    _image = value;
-    assert(onChanged != null);
-    if (!synchronousCall)
-      onChanged();
+    _imagePainter.paint(canvas, rect, clipPath, configuration);
   }
 
   @override
   void dispose() {
-    _imageStream?.removeListener(_imageListener);
-    _imageStream = null;
-    _image = null;
+    _imagePainter?.dispose();
     super.dispose();
   }
 

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

@@ -148,7 +148,7 @@ abstract class Decoration extends Diagnosticable {
 abstract class BoxPainter {
   /// Abstract const constructor. This constructor enables subclasses to provide
   /// const constructors so that they can be used in const expressions.
-  BoxPainter([this._onChanged]);
+  BoxPainter([this.onChanged]);
 
   /// Paints the [Decoration] for which this object was created on the
   /// given canvas using the given configuration.
@@ -180,13 +180,12 @@ abstract class BoxPainter {
   ///
   /// Resources might not start to load until after [paint] has been called,
   /// because they might depend on the configuration.
-  VoidCallback get onChanged => _onChanged;
-  VoidCallback _onChanged;
+  final VoidCallback onChanged;
 
-  /// Discard any resources being held by the object. This also guarantees that
-  /// the [onChanged] callback will not be called again.
+  /// Discard any resources being held by the object.
+  ///
+  /// The [onChanged] callback will not be invoked after this method has been
+  /// called.
   @mustCallSuper
-  void dispose() {
-    _onChanged = null;
-  }
+  void dispose() { }
 }

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

@@ -117,6 +117,16 @@ class DecorationImage {
   /// in the top right.
   final bool matchTextDirection;
 
+  /// Creates a [DecorationImagePainter] for this [DecorationImage].
+  ///
+  /// The `onChanged` argument must not be null. It will be called whenever the
+  /// image needs to be repainted, e.g. because it is loading incrementally or
+  /// because it is animated.
+  DecorationImagePainter createPainter(VoidCallback onChanged) {
+    assert(onChanged != null);
+    return new DecorationImagePainter._(this, onChanged);
+  }
+
   @override
   bool operator ==(dynamic other) {
     if (identical(this, other))
@@ -157,6 +167,106 @@ class DecorationImage {
   }
 }
 
+/// The painter for a [DecorationImage].
+///
+/// To obtain a painter, call [DecorationImage.createPainter].
+///
+/// To paint, call [paint]. The `onChanged` callback passed to
+/// [DecorationImage.createPainter] will be called if the image needs to paint
+/// again (e.g. because it is animated or because it had not yet loaded the
+/// first time the [paint] method was called).
+///
+/// This object should be disposed using the [dispose] method when it is no
+/// longer needed.
+class DecorationImagePainter {
+  DecorationImagePainter._(this._details, this._onChanged);
+
+  final DecorationImage _details;
+  final VoidCallback _onChanged;
+
+  ImageStream _imageStream;
+  ImageInfo _image;
+
+  void paint(Canvas canvas, Rect rect, Path clipPath, ImageConfiguration configuration) {
+    if (_details == null)
+      return;
+
+    assert(canvas != null);
+    assert(rect != null);
+    assert(configuration != null);
+
+    bool flipHorizontally = false;
+    if (_details.matchTextDirection) {
+      assert(() {
+        // We check this first so that the assert will fire immediately, not just
+        // when the image is ready.
+        if (configuration.textDirection == null) {
+          throw new FlutterError(
+            'ImageDecoration.matchTextDirection can only be used when a TextDirection is available.\n'
+            'When DecorationImagePainter.paint() was called, there was no text direction provided '
+            'in the ImageConfiguration object to match.\n'
+            'The DecorationImage was:\n'
+            '  $_details\n'
+            'The ImageConfiguration was:\n'
+            '  $configuration'
+          );
+        }
+        return true;
+      }());
+      if (configuration.textDirection == TextDirection.rtl)
+        flipHorizontally = true;
+    }
+
+    final ImageStream newImageStream = _details.image.resolve(configuration);
+    if (newImageStream.key != _imageStream?.key) {
+      _imageStream?.removeListener(_imageListener);
+      _imageStream = newImageStream;
+      _imageStream.addListener(_imageListener);
+    }
+    if (_image == null)
+      return;
+
+    if (clipPath != null) {
+      canvas.save();
+      canvas.clipPath(clipPath);
+    }
+
+    paintImage(
+      canvas: canvas,
+      rect: rect,
+      image: _image.image,
+      colorFilter: _details.colorFilter,
+      fit: _details.fit,
+      alignment: _details.alignment.resolve(configuration.textDirection),
+      centerSlice: _details.centerSlice,
+      repeat: _details.repeat,
+      flipHorizontally: flipHorizontally,
+    );
+
+    if (clipPath != null)
+      canvas.restore();
+  }
+
+  void _imageListener(ImageInfo value, bool synchronousCall) {
+    if (_image == value)
+      return;
+    _image = value;
+    assert(_onChanged != null);
+    if (!synchronousCall)
+      _onChanged();
+  }
+
+  /// Releases the resources used by this painter.
+  ///
+  /// This should be called whenever the painter is no longer needed.
+  ///
+  /// After this method has been called, the object is no longer usable.
+  @mustCallSuper
+  void dispose() {
+    _imageStream?.removeListener(_imageListener);
+  }
+}
+
 /// Paints an image into the given rectangle on the canvas.
 ///
 /// The arguments have the following meanings:

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

@@ -268,9 +268,9 @@ class ShapeDecoration extends Decoration {
 
 /// An object that paints a [ShapeDecoration] into a canvas.
 class _ShapeDecorationPainter extends BoxPainter {
-  _ShapeDecorationPainter(this._decoration, VoidCallback onChange)
+  _ShapeDecorationPainter(this._decoration, VoidCallback onChanged)
     : assert(_decoration != null),
-      super(onChange);
+      super(onChanged);
 
   final ShapeDecoration _decoration;
 
@@ -318,6 +318,7 @@ class _ShapeDecorationPainter extends BoxPainter {
       _outerPath = _decoration.shape.getOuterPath(rect);
     if (_decoration.image != null)
       _innerPath = _decoration.shape.getInnerPath(rect);
+    _lastRect = rect;
   }
 
   void _paintShadows(Canvas canvas) {
@@ -332,48 +333,17 @@ class _ShapeDecorationPainter extends BoxPainter {
       canvas.drawPath(_outerPath, _interiorPaint);
   }
 
-  ImageStream _imageStream;
-  ImageInfo _image;
-
-  void _imageListener(ImageInfo value, bool synchronousCall) {
-    if (_image == value)
-      return;
-    _image = value;
-    assert(onChanged != null);
-    if (!synchronousCall)
-      onChanged();
-  }
-
+  DecorationImagePainter _imagePainter;
   void _paintImage(Canvas canvas, ImageConfiguration configuration) {
-    final DecorationImage details = _decoration.image;
-    if (details == null)
+    if (_decoration.image == null)
       return;
-    final ImageStream newImageStream = details.image.resolve(configuration);
-    if (newImageStream.key != _imageStream?.key) {
-      _imageStream?.removeListener(_imageListener);
-      _imageStream = newImageStream;
-      _imageStream.addListener(_imageListener);
-    }
-    if (_image == null)
-      return;
-    canvas.save();
-    canvas.clipPath(_innerPath);
-    paintImage(
-      canvas: canvas,
-      rect: _lastRect,
-      image: _image.image,
-      colorFilter: details.colorFilter,
-      fit: details.fit,
-      alignment: details.alignment,
-      centerSlice: details.centerSlice,
-      repeat: details.repeat,
-    );
-    canvas.restore();
+    _imagePainter ??= _decoration.image.createPainter(onChanged);
+    _imagePainter.paint(canvas, _lastRect, _innerPath, configuration);
   }
 
   @override
   void dispose() {
-    _imageStream?.removeListener(_imageListener);
+    _imagePainter?.dispose();
     super.dispose();
   }
 

packages/flutter/test/painting/shape_decoration_test.dart

@@ -2,10 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'package:flutter/painting.dart';
+import 'dart:ui' as ui;
 
+import 'package:flutter/foundation.dart';
+import 'package:flutter/painting.dart';
+import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 
+import '../rendering/mock_canvas.dart';
+
 void main() {
   test('ShapeDecoration constructor', () {
     final Color colorR = const Color(0xffff0000);
@@ -48,4 +53,70 @@ void main() {
     expect(Decoration.lerp(a, b, 0.9).hitTest(size, const Offset(20.0, 50.0)), isTrue);
     expect(b.hitTest(size, const Offset(20.0, 50.0)), isTrue);
   });
+
+  test('ShapeDecoration.image RTL test', () {
+    final List<int> log = <int>[];
+    final ShapeDecoration decoration = new ShapeDecoration(
+      shape: const CircleBorder(),
+      image: new DecorationImage(
+        image: new TestImageProvider(),
+        alignment: AlignmentDirectional.bottomEnd,
+      ),
+    );
+    final BoxPainter painter = decoration.createBoxPainter(() { log.add(0); });
+    expect((Canvas canvas) => painter.paint(canvas, Offset.zero, const ImageConfiguration(size: const Size(100.0, 100.0))), paintsAssertion);
+    expect(
+      (Canvas canvas) {
+        return painter.paint(
+          canvas,
+          const Offset(20.0, -40.0),
+          const ImageConfiguration(
+            size: const Size(1000.0, 1000.0),
+            textDirection: TextDirection.rtl,
+          ),
+        );
+      },
+      paints
+        ..drawImageRect(source: new Rect.fromLTRB(0.0, 0.0, 100.0, 200.0), destination: new Rect.fromLTRB(20.0, 1000.0 - 40.0 - 200.0, 20.0 + 100.0, 1000.0 - 40.0))
+    );
+    expect(
+      (Canvas canvas) {
+        return painter.paint(
+          canvas,
+          Offset.zero,
+          const ImageConfiguration(
+            size: const Size(100.0, 200.0),
+            textDirection: TextDirection.ltr,
+          ),
+        );
+      },
+      isNot(paints..image()) // we always use drawImageRect
+    );
+    expect(log, isEmpty);
+  });
+}
+
+class TestImageProvider extends ImageProvider<TestImageProvider> {
+  @override
+  Future<TestImageProvider> obtainKey(ImageConfiguration configuration) {
+    return new SynchronousFuture<TestImageProvider>(this);
+  }
+
+  @override
+  ImageStreamCompleter load(TestImageProvider key) {
+    return new OneFrameImageStreamCompleter(
+      new SynchronousFuture<ImageInfo>(new ImageInfo(image: new TestImage(), scale: 1.0)),
+    );
+  }
+}
+
+class TestImage extends ui.Image {
+  @override
+  int get width => 100;
+
+  @override
+  int get height => 200;
+
+  @override
+  void dispose() { }
 }

packages/flutter/test/rendering/mock_canvas.dart

@@ -287,6 +287,24 @@ abstract class PaintPattern {
 
   /// Indicates that an image is expected next.
   ///
+  /// The next call to [Canvas.drawImage] is examined, and its arguments
+  /// compared to those passed to _this_ method.
+  ///
+  /// If no call to [Canvas.drawImage] was made, then this results in
+  /// failure.
+  ///
+  /// Any calls made between the last matched call (if any) and the
+  /// [Canvas.drawImage] call are ignored.
+  ///
+  /// The [Paint]-related arguments (`color`, `strokeWidth`, `hasMaskFilter`,
+  /// `style`) are compared against the state of the [Paint] object after the
+  /// painting has completed, not at the time of the call. If the same [Paint]
+  /// object is reused multiple times, then this may not match the actual
+  /// arguments as they were seen by the method.
+  void image({ ui.Image image, double x, double y, Color color, double strokeWidth, bool hasMaskFilter, PaintingStyle style });
+
+  /// Indicates that an image subsection is expected next.
+  ///
   /// The next call to [Canvas.drawImageRect] is examined, and its arguments
   /// compared to those passed to _this_ method.
   ///
@@ -612,6 +630,11 @@ class _TestRecordingCanvasPatternMatcher extends _TestRecordingCanvasMatcher imp
     _predicates.add(new _FunctionPaintPredicate(#drawParagraph, <dynamic>[paragraph, offset]));
   }
 
+  @override
+  void image({ ui.Image image, double x, double y, Color color, double strokeWidth, bool hasMaskFilter, PaintingStyle style }) {
+    _predicates.add(new _DrawImagePaintPredicate(image: image, x: x, y: y, color: color, strokeWidth: strokeWidth, hasMaskFilter: hasMaskFilter, style: style));
+  }
+
   @override
   void drawImageRect({ ui.Image image, Rect source, Rect destination, Color color, double strokeWidth, bool hasMaskFilter, PaintingStyle style }) {
     _predicates.add(new _DrawImageRectPaintPredicate(image: image, source: source, destination: destination, color: color, strokeWidth: strokeWidth, hasMaskFilter: hasMaskFilter, style: style));
@@ -978,6 +1001,50 @@ class _ArcPaintPredicate extends _DrawCommandPaintPredicate {
   );
 }
 
+class _DrawImagePaintPredicate extends _DrawCommandPaintPredicate {
+  _DrawImagePaintPredicate({ this.image, this.x, this.y, Color color, double strokeWidth, bool hasMaskFilter, PaintingStyle style }) : super(
+    #drawImage, 'an image', 3, 2, color: color, strokeWidth: strokeWidth, hasMaskFilter: hasMaskFilter, style: style
+  );
+
+  final ui.Image image;
+  final double x;
+  final double y;
+
+  @override
+  void verifyArguments(List<dynamic> arguments) {
+    super.verifyArguments(arguments);
+    final ui.Image imageArgument = arguments[0];
+    if (image != null && imageArgument != image)
+      throw 'It called $methodName with an image, $imageArgument, which was not exactly the expected image ($image).';
+    final Offset pointArgument = arguments[0];
+    if (x != null && y != null) {
+      final Offset point = new Offset(x, y);
+      if (point != pointArgument)
+        throw 'It called $methodName with an offset coordinate, $pointArgument, which was not exactly the expected coordinate ($point).';
+    } else {
+      if (x != null && pointArgument.dx != x)
+        throw 'It called $methodName with an offset coordinate, $pointArgument, whose x-coordinate not exactly the expected coordinate (${x.toStringAsFixed(1)}).';
+      if (y != null && pointArgument.dy != y)
+        throw 'It called $methodName with an offset coordinate, $pointArgument, whose y-coordinate not exactly the expected coordinate (${y.toStringAsFixed(1)}).';
+    }
+  }
+
+  @override
+  void debugFillDescription(List<String> description) {
+    super.debugFillDescription(description);
+    if (image != null)
+      description.add('image $image');
+    if (x != null && y != null) {
+      description.add('point ${new Offset(x, y)}');
+    } else {
+      if (x != null)
+        description.add('x-coordinate ${x.toStringAsFixed(1)}');
+      if (y != null)
+        description.add('y-coordinate ${y.toStringAsFixed(1)}');
+    }
+  }
+}
+
 class _DrawImageRectPaintPredicate extends _DrawCommandPaintPredicate {
   _DrawImageRectPaintPredicate({ this.image, this.source, this.destination, Color color, double strokeWidth, bool hasMaskFilter, PaintingStyle style }) : super(
     #drawImageRect, 'an image', 4, 3, color: color, strokeWidth: strokeWidth, hasMaskFilter: hasMaskFilter, style: style