Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add more flexible image loading API
This adds a new `instantiateImageCodecWithSize` method, which can be
used to decode an image into a size, where the target size isn't known
until the caller is allowed to inspect the image descriptor.

This enables the use case of loading an image whose aspect ratio isn't
known at load time, and which needs to be resized to fit within a
bounding box while also maintaining its original aspect ratio.

flutter/flutter#118543
  • Loading branch information
tvolkert committed Jan 20, 2023
commit 759c9803941d6b2f914e594d2d4f181b9403bc8a
128 changes: 115 additions & 13 deletions lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2105,7 +2105,7 @@ Future<Codec> instantiateImageCodec(
bool allowUpscaling = true,
}) async {
final ImmutableBuffer buffer = await ImmutableBuffer.fromUint8List(list);
return instantiateImageCodecFromBuffer(
return await instantiateImageCodecFromBuffer(
buffer,
targetWidth: targetWidth,
targetHeight: targetHeight,
Expand All @@ -2124,6 +2124,10 @@ Future<Codec> instantiateImageCodec(
/// The data can be for either static or animated images. The following image
/// formats are supported: {@macro dart.ui.imageFormats}
///
/// The [buffer] will be disposed by this method once the codec has been created,
/// so the caller must relinquish ownership of the [buffer] when they call this
/// method.
///
/// The [targetWidth] and [targetHeight] arguments specify the size of the
/// output image, in image pixels. If they are not equal to the intrinsic
/// dimensions of the image, then the image will be scaled after being decoded.
Expand All @@ -2146,21 +2150,119 @@ Future<Codec> instantiateImageCodecFromBuffer(
int? targetWidth,
int? targetHeight,
bool allowUpscaling = true,
}) {
return instantiateImageCodecWithSize(
buffer,
getTargetSize: (ImageDescriptor descriptor) {
if (!allowUpscaling) {
if (targetWidth != null && targetWidth! > descriptor.width) {
targetWidth = descriptor.width;
}
if (targetHeight != null && targetHeight! > descriptor.height) {
targetHeight = descriptor.height;
}
}
return TargetImageSize(width: targetWidth, height: targetHeight);
},
);
}

/// Instantiates an image [Codec].
///
/// This method is a convenience wrapper around the [ImageDescriptor] API.
///
/// The [buffer] parameter is the binary image data (e.g a PNG or GIF binary data).
/// The data can be for either static or animated images. The following image
/// formats are supported: {@macro dart.ui.imageFormats}
///
/// The [buffer] will be disposed by this method once the codec has been created,
/// so the caller must relinquish ownership of the [buffer] when they call this
/// method.
///
/// The [getTargetSize] parameter, when specified, will be invoked and passed
/// the image's [ImageDescriptor] to determine the size to decode the image to.
/// The width and the height of the size it returns must be positive values
/// greater than or equal to 1, or null. It is valid to return a [TargetImageSize]
/// that specifies only one of `width` and `height` with the other remaining
/// null, in which case the omitted dimension will be scaled to maintain the
/// aspect ratio of the original dimensions. When both are null or omitted,
/// the image will be decoded at its native resolution (as will be the case if
/// the [getTargetSize] parameter is omitted).
///
/// Scaling the image to larger than its intrinsic size should usually be
/// avoided, since it causes the image to use more memory than necessary.
/// Instead, prefer scaling the [Canvas] transform.
///
/// The returned future can complete with an error if the image decoding has
/// failed.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe note here that image resizing capabilities are only available with the canvaskit renderer and not html renderer (we should probably update the api docs for instantiateImageCodec to note this too)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Future<Codec> instantiateImageCodecWithSize(
ImmutableBuffer buffer, {
TargetImageSizeProducer? getTargetSize,
}) async {
getTargetSize ??= (ImageDescriptor descriptor) => const TargetImageSize();
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using ??= here, create a static function and make it the argument default value:

TargetImageSize _defaultGetTargetSize(ImageDescriptor descriptor) {
  return const TargetImageSize();
}

TargetImageSizeProducer getTargetSize = _defaultGetTargetSize,


Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh - I did that at first, but then the package:flutter/painting.dart API wrapper has to do something like this:

  Future<ui.Codec> instantiateImageCodecWithSize(
    ui.ImmutableBuffer buffer, {
    ui.TargetImageSizeProducer? getTargetSize,
  }) {
    if (getTargetSize == null) {
      return ui.instantiateImageCodecWithSize(buffer);
    } else {
      return ui.instantiateImageCodecWithSize(buffer, getTargetSize: getTargetSize);
    }
  }

Either that, or the static function has to be public (or worse: copied). After discussing with Bob, he suggested that he frequently resorts to the method I've done here. What do you think given those tradeoffs?

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh yeah, that makes sense. I think the approach is fine. I would still use a static function instead of creating a new closure on each run though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

final ImageDescriptor descriptor = await ImageDescriptor.encoded(buffer);
if (!allowUpscaling) {
if (targetWidth != null && targetWidth > descriptor.width) {
targetWidth = descriptor.width;
}
if (targetHeight != null && targetHeight > descriptor.height) {
targetHeight = descriptor.height;
}
try {
final TargetImageSize targetSize = getTargetSize(descriptor);
assert(targetSize.width == null || targetSize.width! > 0);
assert(targetSize.height == null || targetSize.height! > 0);
return descriptor.instantiateCodec(
targetWidth: targetSize.width,
targetHeight: targetSize.height,
);
} finally {
buffer.dispose();
}
buffer.dispose();
return descriptor.instantiateCodec(
targetWidth: targetWidth,
targetHeight: targetHeight,
);
}

/// Signature for a callback that determines the size to which an image should
/// be decoded given its [ImageDescriptor].
///
/// See also:
///
/// * [instantiateImageCodecWithSize], which used this signature for its
/// `getTargetSize` argument.
typedef TargetImageSizeProducer = TargetImageSize Function(ImageDescriptor descriptor);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we generally use "Callback" as the suffix for function types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


/// A specification of the size to which an image should be decoded.
///
/// See also:
///
/// * [TargetImageSizeProducer], a callback that returns instances of this
/// class when consulted by image decoding methods such as
/// [instantiateImageCodecWithSize].
class TargetImageSize {
/// Creates a new instance of this class.
///
/// The `width` and `height` may both be null, but if they're non-null, they
/// must be positive.
const TargetImageSize({this.width, this.height})
: assert(width == null || width > 0),
assert(width == null || width > 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think you meant for this to be height


/// The width into which to load the image.
///
/// If this is non-null, the image will be decoded into the specified width.
/// If this is null and [height] is also null, the image will be decoded into
/// its intrinsic size. If this is null and [height] is non-null, the image
/// will be decoded into a width that maintains its intrinsic aspect ratio
/// while respecting the [height] value.
///
/// If this value is non-null, it must be positive.
final int? width;

/// The height into which to load the image.
///
/// If this is non-null, the image will be decoded into the specified height.
/// If this is null and [width] is also null, the image will be decoded into
/// its intrinsic size. If this is null and [width] is non-null, the image
/// will be decoded into a height that maintains its intrinsic aspect ratio
/// while respecting the [width] value.
///
/// If this value is non-null, it must be positive.
final int? height;

@override
String toString() => 'TargetImageSize($width x $height)';
}

/// Loads a single image frame from a byte array into an [Image] object.
Expand Down