Merge pull request #545 from abarth/image_resource

Use ImageResource instead of Future<sky.Image>

examples/game/lib/image_map.dart

@@ -15,7 +15,7 @@ class ImageMap {
   }
 
   Future<Image> _loadImage(String url) async {
-    Image image = await _bundle.loadImage(url);
+    Image image = await _bundle.loadImage(url).first;
     _images[url] = image;
     return image;
   }

sky/packages/sky/lib/base/image_resource.dart

+// Copyright 2015 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:async';
+import 'dart:sky' as sky;
+
+typedef void ImageListener(sky.Image image);
+
+class ImageResource {
+  ImageResource(this._futureImage) {
+    _futureImage.then((sky.Image image) {
+      _image = image;
+      _resolved = true;
+      _notifyListeners();
+    });
+  }
+
+  bool _resolved = false;
+  Future<sky.Image> _futureImage;
+  sky.Image _image;
+  final List<ImageListener> _listeners = new List<ImageListener>();
+
+  Future<sky.Image> get first => _futureImage;
+
+  void addListener(ImageListener listener) {
+    _listeners.add(listener);
+    if (_resolved)
+      listener(_image);
+  }
+
+  void removeListener(ImageListener listener) {
+    _listeners.remove(listener);
+  }
+
+  void _notifyListeners() {
+    assert(_resolved);
+    List<ImageListener> localListeners = new List<ImageListener>.from(_listeners);
+    for (ImageListener listener in localListeners)
+      listener(_image);
+  }
+}

sky/packages/sky/lib/mojo/asset_bundle.dart

@@ -9,13 +9,14 @@ import 'dart:typed_data';
 
 import 'package:mojo/core.dart' as core;
 import 'package:mojo_services/mojo/asset_bundle/asset_bundle.mojom.dart';
+import 'package:sky/base/image_resource.dart';
 import 'package:sky/mojo/net/fetch.dart';
 import 'package:sky/mojo/net/image_cache.dart' as image_cache;
 import 'package:sky/mojo/shell.dart' as shell;
 
 abstract class AssetBundle {
   void close();
-  Future<sky.Image> loadImage(String key);
+  ImageResource loadImage(String key);
   Future<String> loadString(String key);
 }
 
@@ -26,7 +27,7 @@ class NetworkAssetBundle extends AssetBundle {
 
   void close() { }
 
-  Future<sky.Image> loadImage(String key) {
+  ImageResource loadImage(String key) {
     return image_cache.load(_baseUrl.resolve(key).toString());
   }
 
@@ -53,7 +54,7 @@ class MojoAssetBundle extends AssetBundle {
   }
 
   AssetBundleProxy _bundle;
-  Map<String, Future<sky.Image>> _imageCache = new Map<String, Future<sky.Image>>();
+  Map<String, ImageResource> _imageCache = new Map<String, ImageResource>();
   Map<String, Future<String>> _stringCache = new Map<String, Future<String>>();
 
   void close() {
@@ -62,13 +63,13 @@ class MojoAssetBundle extends AssetBundle {
     _imageCache = null;
   }
 
-  Future<sky.Image> loadImage(String key) {
+  ImageResource loadImage(String key) {
     return _imageCache.putIfAbsent(key, () {
       Completer<sky.Image> completer = new Completer<sky.Image>();
       _bundle.ptr.getAsStream(key).then((response) {
         new sky.ImageDecoder(response.assetData.handle.h, completer.complete);
       });
-      return completer.future;
+      return new ImageResource(completer.future);
     });
   }
 

sky/packages/sky/lib/mojo/net/image_cache.dart

@@ -7,12 +7,12 @@ import 'dart:collection';
 import 'dart:sky' as sky;
 
 import 'package:mojo/mojo/url_response.mojom.dart';
+import 'package:sky/base/image_resource.dart';
 import 'package:sky/mojo/net/fetch.dart';
 
-final HashMap<String, Future<sky.Image>> _cache =
-    new HashMap<String, Future<sky.Image>>();
+final HashMap<String, ImageResource> _cache = new Map<String, ImageResource>();
 
-Future<sky.Image> load(String url) {
+ImageResource load(String url) {
   return _cache.putIfAbsent(url, () {
     Completer<sky.Image> completer = new Completer<sky.Image>();
     fetchUrl(url).then((UrlResponse response) {
@@ -23,6 +23,6 @@ Future<sky.Image> load(String url) {
         new sky.ImageDecoder(response.body.handle.h, completer.complete);
       }
     });
-    return completer.future;
+    return new ImageResource(completer.future);
   });
 }

sky/packages/sky/lib/painting/box_painter.dart

@@ -2,11 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
 import 'dart:math' as math;
 import 'dart:sky' as sky;
 import 'dart:sky' show Point, Offset, Size, Rect, Color, Paint, Path;
 
+import 'package:sky/base/image_resource.dart';
 import 'package:sky/base/lerp.dart';
 import 'package:sky/painting/shadows.dart';
 
@@ -168,10 +168,7 @@ enum BackgroundFit { fill, contain, cover, none, scaleDown }
 
 enum BackgroundRepeat { repeat, repeatX, repeatY, noRepeat }
 
-// TODO(jackson): We should abstract this out into a separate class
-// that handles the image caching and so forth, which has callbacks
-// for "size changed" and "image changed". This would also enable us
-// to do animated images.
+typedef void BackgroundImageChangeListener();
 
 class BackgroundImage {
   final BackgroundFit fit;
@@ -179,35 +176,48 @@ class BackgroundImage {
   final sky.ColorFilter colorFilter;
 
   BackgroundImage({
-    Future<sky.Image> image,
+    ImageResource image,
     this.fit: BackgroundFit.scaleDown,
     this.repeat: BackgroundRepeat.noRepeat,
     this.colorFilter
-  }) {
-    image.then((resolvedImage) {
-      if (resolvedImage == null)
-        return;
-      _image = resolvedImage;
-      _size = new Size(resolvedImage.width.toDouble(), resolvedImage.height.toDouble());
-      for (Function listener in _listeners) {
-        listener();
-      }
-    });
-  }
+  }) : _imageResource = image;
 
   sky.Image _image;
   sky.Image get image => _image;
 
+  ImageResource _imageResource;
   Size _size;
 
-  final List<Function> _listeners = new List<Function>();
+  final List<BackgroundImageChangeListener> _listeners =
+      new List<BackgroundImageChangeListener>();
 
-  void addChangeListener(Function listener) {
+  void addChangeListener(BackgroundImageChangeListener listener) {
+    // We add the listener to the _imageResource first so that the first change
+    // listener doesn't get callback synchronously if the image resource is
+    // already resolved.
+    if (_listeners.isEmpty)
+      _imageResource.addListener(_handleImageChanged);
     _listeners.add(listener);
   }
 
-  void removeChangeListener(Function listener) {
+  void removeChangeListener(BackgroundImageChangeListener listener) {
     _listeners.remove(listener);
+    // We need to remove ourselves as listeners from the _imageResource so that
+    // we're not kept alive by the image_cache.
+    if (_listeners.isEmpty)
+      _imageResource.removeListener(_handleImageChanged);
+  }
+
+  void _handleImageChanged(sky.Image resolvedImage) {
+    if (resolvedImage == null)
+      return;
+    _image = resolvedImage;
+    _size = new Size(resolvedImage.width.toDouble(), resolvedImage.height.toDouble());
+    final List<BackgroundImageChangeListener> localListeners =
+        new List<BackgroundImageChangeListener>.from(_listeners);
+    for (BackgroundImageChangeListener listener in localListeners) {
+      listener();
+    }
   }
 
   String toString() => 'BackgroundImage($fit, $repeat)';

sky/packages/sky/lib/rendering/box.dart

@@ -1475,20 +1475,45 @@ class RenderDecoratedBox extends RenderProxyBox {
     RenderBox child
   }) : _painter = new BoxPainter(decoration), super(child);
 
-  BoxPainter _painter;
+  final BoxPainter _painter;
+
   BoxDecoration get decoration => _painter.decoration;
   void set decoration (BoxDecoration value) {
     assert(value != null);
-    if (_painter.decoration.backgroundImage != null)
-      _painter.decoration.backgroundImage.removeChangeListener(markNeedsPaint);
-    if (value.backgroundImage != null)
-      value.backgroundImage.addChangeListener(markNeedsPaint);
     if (value == _painter.decoration)
       return;
+    _removeBackgroundImageListenerIfNeeded();
     _painter.decoration = value;
+    _addBackgroundImageListenerIfNeeded();
     markNeedsPaint();
   }
 
+  bool get _needsBackgroundImageListener {
+    return attached &&
+        _painter.decoration != null &&
+        _painter.decoration.backgroundImage != null;
+  }
+
+  void _addBackgroundImageListenerIfNeeded() {
+    if (_needsBackgroundImageListener)
+      _painter.decoration.backgroundImage.addChangeListener(markNeedsPaint);
+  }
+
+  void _removeBackgroundImageListenerIfNeeded() {
+    if (_needsBackgroundImageListener)
+      _painter.decoration.backgroundImage.removeChangeListener(markNeedsPaint);
+  }
+
+  void attach() {
+    super.attach();
+    _addBackgroundImageListenerIfNeeded();
+  }
+
+  void detach() {
+    _removeBackgroundImageListenerIfNeeded();
+    super.detach();
+  }
+
   void paint(PaintingCanvas canvas, Offset offset) {
     assert(size.width != null);
     assert(size.height != null);

sky/packages/sky/lib/widgets/basic.dart

@@ -2,11 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
 import 'dart:sky' as sky;
 
 import 'package:vector_math/vector_math.dart';
 
+import 'package:sky/base/image_resource.dart';
 import 'package:sky/mojo/asset_bundle.dart';
 import 'package:sky/mojo/net/image_cache.dart' as image_cache;
 import 'package:sky/painting/text_style.dart';
@@ -530,39 +530,44 @@ class Image extends LeafRenderObjectWrapper {
   }
 }
 
-class FutureImage extends StatefulComponent {
-  FutureImage({ Key key, this.image, this.width, this.height, this.colorFilter }) : super(key: key);
+class ImageListener extends StatefulComponent {
+  ImageListener({ Key key, this.image, this.width, this.height, this.colorFilter }) : super(key: key);
 
-  Future<sky.Image> image;
+  ImageResource image;
   double width;
   double height;
   sky.ColorFilter colorFilter;
 
   sky.Image _resolvedImage;
 
-  void _resolveImage() {
-    image.then((sky.Image resolvedImage) {
-      if (!mounted)
-        return;
-      setState(() {
-        _resolvedImage = resolvedImage;
-      });
+  void _handleImageChanged(sky.Image resolvedImage) {
+    if (!mounted)
+      return;
+    setState(() {
+      _resolvedImage = resolvedImage;
     });
   }
 
   void didMount() {
     super.didMount();
-    _resolveImage();
+    image.addListener(_handleImageChanged);
   }
 
-  void syncFields(FutureImage source) {
-    bool needToResolveImage = (image != source.image);
+  void didUnmount() {
+    super.didUnmount();
+    image.removeListener(_handleImageChanged);
+  }
+
+  void syncFields(ImageListener source) {
+    final bool needToUpdateListeners = (image != source.image) && mounted;
+    if (needToUpdateListeners)
+      image.removeListener(_handleImageChanged);
     image = source.image;
     width = source.width;
     height = source.height;
     colorFilter = source.colorFilter;
-    if (needToResolveImage)
-      _resolveImage();
+    if (needToUpdateListeners)
+      image.addListener(_handleImageChanged);
   }
 
   Widget build() {
@@ -584,7 +589,7 @@ class NetworkImage extends Component {
   final sky.ColorFilter colorFilter;
 
   Widget build() {
-    return new FutureImage(
+    return new ImageListener(
       image: image_cache.load(src),
       width: width,
       height: height,
@@ -603,7 +608,7 @@ class AssetImage extends Component {
   final sky.ColorFilter colorFilter;
 
   Widget build() {
-    return new FutureImage(
+    return new ImageListener(
       image: bundle.loadImage(name),
       width: width,
       height: height,