Skip to content

Conversation

@Hixie
Copy link
Contributor

@Hixie Hixie commented Jun 15, 2016

Overview

This patch refactors images to achieve the following goals:

  • it allows references to unresolved assets to be passed
    around (previously, almost every layer of the system had to know about
    whether an image came from an asset bundle or the network or
    elsewhere, and had to manually interact with the image cache).
  • it allows decorations to use the same API for declaring images as the
    widget tree.

It requires some minor changes to call sites that use images, as
discussed below.

Widgets

Change this:

      child: new AssetImage(
        name: 'my_asset.png',
        ...
      )

...to this:

      child: new Image(
        image: new AssetImage('my_asset.png'),
        ...
      )

Decorations

Change this:

      child: new DecoratedBox(
        decoration: new BoxDecoration(
          backgroundImage: new BackgroundImage(
            image: DefaultAssetBundle.of(context).loadImage('my_asset.png'),
            ...
          ),
          ...
        ),
        child: ...
      )

...to this:

      child: new DecoratedBox(
        decoration: new BoxDecoration(
          backgroundImage: new BackgroundImage(
            image: new AssetImage('my_asset.png'),
            ...
          ),
          ...
        ),
        child: ...
      )

Detailed change log

The following APIs have been replaced in this patch:

  • The AssetImage and NetworkImage widgets have been split in two,
    with identically-named ImageProvider subclasses providing the
    image-loading logic, and a single Image widget providing all the
    widget tree logic.
  • ImageResource is now ImageStream. Rather than configuring it with
    a Future<ImageInfo>, you complete it with an ImageStreamCompleter.
  • ImageCache.load and ImageCache.loadProvider are replaced by
    ImageCache.putIfAbsent.

The following APIs have changed in this patch:

  • ImageCache works in terms of arbitrary keys and caches
    ImageStreamCompleter objects using those keys. With the new model,
    you should never need to interact with the cache directly.
  • Decoration can now be const. The state has moved to the
    BoxPainter class. Instead of a list of listeners, there's now just a
    single callback and a dispose() method on the painter. The callback
    is passed in to the createBoxPainter() method. When invoked, you
    should repaint the painter.

The following new APIs are introduced:

  • AssetBundle.loadStructuredData.
  • SynchronousFuture, a variant of Future that calls the then
    callback synchronously. This enables the asynchronous and
    synchronous (in-the-cache) code paths to look identical yet for the
    latter to avoid returning to the event loop mid-paint.
  • ExactAssetImage, a variant of AssetImage that doesn't do anything clever.
  • ImageConfiguration, a class that describes parameters that configure
    the AssetImage resolver.

The following APIs are entirely removed by this patch:

  • AssetBundle.loadImage is gone. Use an AssetImage instead.
  • AssetVendor is gone. AssetImage handles everything AssetVendor
    used to handle.
  • RawImageResource and AsyncImage are gone.

The following code-level changes are performed:

  • Image, which replaces AsyncImage, NetworkImage, AssetImage,
    and RawResourceImage, lives in image.dart.
  • DecoratedBox and Container live in their own file now,
    container.dart (they reference image.dart).

Directions for future research

  • The ImageConfiguration fields are mostly aspirational. Right now
    only devicePixelRatio and bundle are implemented. locale isn't
    even plumbed through, it will require work on the localisation logic.
  • We should go through and make BoxDecoration, AssetImage, and
    NetworkImage objects const where possible.
  • This patch makes supporting animated GIFs much easier.
  • This patch makes it possible to create an abstract concept of an
    "Icon" that could be either an image or a font-based glyph (using
    IconData or similar). (see
    Icon widget (or a new widget) should support giving an icon from another font #4494)

Related issues

Fixes #4500
Fixes #4495
Obsoletes #4496

@Hixie
Copy link
Contributor Author

Hixie commented Jun 15, 2016

@abarth

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks spurious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. Fallout from my trying to fix the analyzer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@abarth
Copy link
Contributor

abarth commented Jun 16, 2016

LGTM. This patch is fabulous. I worry the SynchronousFuture is slightly too clever, but it works nicely in this patch.

Overview
========

This patch refactors images to achieve the following goals:

* it allows references to unresolved assets to be passed
  around (previously, almost every layer of the system had to know about
  whether an image came from an asset bundle or the network or
  elsewhere, and had to manually interact with the image cache).

* it allows decorations to use the same API for declaring images as the
  widget tree.

It requires some minor changes to call sites that use images, as
discussed below.

Widgets
-------

Change this:

```dart
      child: new AssetImage(
        name: 'my_asset.png',
        ...
      )
```

...to this:

```dart
      child: new Image(
        image: new AssetImage('my_asset.png'),
        ...
      )
```

Decorations
-----------

Change this:

```dart
      child: new DecoratedBox(
        decoration: new BoxDecoration(
          backgroundImage: new BackgroundImage(
            image: DefaultAssetBundle.of(context).loadImage('my_asset.png'),
            ...
          ),
          ...
        ),
        child: ...
      )
```

...to this:

```dart
      child: new DecoratedBox(
        decoration: new BoxDecoration(
          backgroundImage: new BackgroundImage(
            image: new AssetImage('my_asset.png'),
            ...
          ),
          ...
        ),
        child: ...
      )
```

DETAILED CHANGE LOG
===================

The following APIs have been replaced in this patch:

* The `AssetImage` and `NetworkImage` widgets have been split in two,
  with identically-named `ImageProvider` subclasses providing the
  image-loading logic, and a single `Image` widget providing all the
  widget tree logic.

* `ImageResource` is now `ImageStream`. Rather than configuring it with
  a `Future<ImageInfo>`, you complete it with an `ImageStreamCompleter`.

* `ImageCache.load` and `ImageCache.loadProvider` are replaced by
  `ImageCache.putIfAbsent`.

The following APIs have changed in this patch:

* `ImageCache` works in terms of arbitrary keys and caches
  `ImageStreamCompleter` objects using those keys. With the new model,
  you should never need to interact with the cache directly.

* `Decoration` can now be `const`. The state has moved to the
  `BoxPainter` class. Instead of a list of listeners, there's now just a
  single callback and a `dispose()` method on the painter. The callback
  is passed in to the `createBoxPainter()` method. When invoked, you
  should repaint the painter.

The following new APIs are introduced:

* `AssetBundle.loadStructuredData`.

* `SynchronousFuture`, a variant of `Future` that calls the `then`
  callback synchronously. This enables the asynchronous and
  synchronous (in-the-cache) code paths to look identical yet for the
  latter to avoid returning to the event loop mid-paint.

* `ExactAssetImage`, a variant of `AssetImage` that doesn't do anything clever.

* `ImageConfiguration`, a class that describes parameters that configure
  the `AssetImage` resolver.

The following APIs are entirely removed by this patch:

* `AssetBundle.loadImage` is gone. Use an `AssetImage` instead.

* `AssetVendor` is gone. `AssetImage` handles everything `AssetVendor`
  used to handle.

* `RawImageResource` and `AsyncImage` are gone.

The following code-level changes are performed:

* `Image`, which replaces `AsyncImage`, `NetworkImage`, `AssetImage`,
  and `RawResourceImage`, lives in `image.dart`.

* `DecoratedBox` and `Container` live in their own file now,
  `container.dart` (they reference `image.dart`).

DIRECTIONS FOR FUTURE RESEARCH
==============================

* The `ImageConfiguration` fields are mostly aspirational. Right now
  only `devicePixelRatio` and `bundle` are implemented. `locale` isn't
  even plumbed through, it will require work on the localisation logic.

* We should go through and make `BoxDecoration`, `AssetImage`, and
  `NetworkImage` objects `const` where possible.

* This patch makes supporting animated GIFs much easier.

* This patch makes it possible to create an abstract concept of an
  "Icon" that could be either an image or a font-based glyph (using
  `IconData` or similar). (see
  flutter#4494)

RELATED ISSUES
==============

Fixes flutter#4500
Fixes flutter#4495
Obsoletes flutter#4496
@Hixie Hixie merged commit 2dfdc84 into flutter:master Jun 16, 2016
@Hixie Hixie deleted the image-refactor branch June 16, 2016 16:49
cdotstout pushed a commit to cdotstout/flutter that referenced this pull request Apr 3, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split the configuration for selecting an image from the configuration of its painting "fit" argument dartdocs should say what the default is

2 participants