Skip to content

Commit

Permalink
Make upscaling images opt-in (flutter#59856)
Browse files Browse the repository at this point in the history
* Make upscaling images opt-in
  • Loading branch information
dnfield authored and mingwandroid committed Sep 6, 2020
1 parent 6ed5f3d commit 0852c51
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 28 deletions.
27 changes: 19 additions & 8 deletions packages/flutter/lib/src/painting/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,26 +71,37 @@ mixin PaintingBinding on BindingBase, ServicesBinding {
@protected
ImageCache createImageCache() => ImageCache();

/// Calls through to [dart:ui] with [decodedCacheRatioCap] from [ImageCache].
/// Calls through to [dart:ui] from [ImageCache].
///
/// The [cacheWidth] and [cacheHeight] parameters, when specified, indicate the
/// size to decode the image to.
/// The `cacheWidth` and `cacheHeight` parameters, when specified, indicate
/// the size to decode the image to.
///
/// Both [cacheWidth] and [cacheHeight] must be positive values greater than or
/// equal to 1 or null. It is valid to specify only one of [cacheWidth] and
/// [cacheHeight] with the other remaining null, in which case the omitted
/// dimension will decode to its original size. When both are null or omitted,
/// the image will be decoded at its native resolution.
/// Both `cacheWidth` and `cacheHeight` must be positive values greater than
/// or equal to 1, or null. It is valid to specify only one of `cacheWidth`
/// and `cacheHeight` 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.
///
/// The `allowUpscaling` parameter determines whether the `cacheWidth` or
/// `cacheHeight` parameters are clamped to the intrinsic width and height of
/// the original image. By default, the dimensions are clamped to avoid
/// unnecessary memory usage for images. Callers that wish to display an image
/// above its native resolution should prefer scaling the canvas the image is
/// drawn into.
Future<ui.Codec> instantiateImageCodec(Uint8List bytes, {
int cacheWidth,
int cacheHeight,
bool allowUpscaling = false,
}) {
assert(cacheWidth == null || cacheWidth > 0);
assert(cacheHeight == null || cacheHeight > 0);
assert(allowUpscaling != null);
return ui.instantiateImageCodec(
bytes,
targetWidth: cacheWidth,
targetHeight: cacheHeight,
allowUpscaling: allowUpscaling,
);
}

Expand Down
32 changes: 23 additions & 9 deletions packages/flutter/lib/src/painting/image_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,15 @@ class ImageConfiguration {

/// Performs the decode process for use in [ImageProvider.load].
///
/// This callback allows decoupling of the `cacheWidth` and `cacheHeight`
/// parameters from implementations of [ImageProvider] that do not use them.
/// This callback allows decoupling of the `cacheWidth`, `cacheHeight`, and
/// `allowUpscaling` parameters from implementations of [ImageProvider] that do
/// not expose them.
///
/// See also:
///
/// * [ResizeImage], which uses this to override the `cacheWidth` and `cacheHeight` parameters.
typedef DecoderCallback = Future<ui.Codec> Function(Uint8List bytes, {int cacheWidth, int cacheHeight});
/// * [ResizeImage], which uses this to override the `cacheWidth`,
/// `cacheHeight`, and `allowUpscaling` parameters.
typedef DecoderCallback = Future<ui.Codec> Function(Uint8List bytes, {int cacheWidth, int cacheHeight, bool allowUpscaling});

/// Identifies an image without committing to the precise final asset. This
/// allows a set of images to be identified and for the precise image to later
Expand Down Expand Up @@ -718,7 +720,9 @@ class ResizeImage extends ImageProvider<_SizeAwareCacheKey> {
this.imageProvider, {
this.width,
this.height,
}) : assert(width != null || height != null);
this.allowUpscaling = false,
}) : assert(width != null || height != null),
assert(allowUpscaling != null);

/// The [ImageProvider] that this class wraps.
final ImageProvider imageProvider;
Expand All @@ -729,6 +733,15 @@ class ResizeImage extends ImageProvider<_SizeAwareCacheKey> {
/// The height the image should decode to and cache.
final int height;

/// Whether the [width] and [height] parameters should be clamped to the
/// intrinsic width and height of the image.
///
/// In general, it is better for memory usage to avoid scaling the image
/// beyond its intrinsic dimensions when decoding it. If there is a need to
/// scale an image larger, it is better to apply a scale to the canvas, or
/// to use an appropriate [Image.fit].
final bool allowUpscaling;

/// Composes the `provider` in a [ResizeImage] only when `cacheWidth` and
/// `cacheHeight` are not both null.
///
Expand All @@ -743,12 +756,13 @@ class ResizeImage extends ImageProvider<_SizeAwareCacheKey> {

@override
ImageStreamCompleter load(_SizeAwareCacheKey key, DecoderCallback decode) {
final DecoderCallback decodeResize = (Uint8List bytes, {int cacheWidth, int cacheHeight}) {
final DecoderCallback decodeResize = (Uint8List bytes, {int cacheWidth, int cacheHeight, bool allowUpscaling}) {
assert(
cacheWidth == null && cacheHeight == null,
'ResizeImage cannot be composed with another ImageProvider that applies cacheWidth or cacheHeight.'
cacheWidth == null && cacheHeight == null && allowUpscaling == null,
'ResizeImage cannot be composed with another ImageProvider that applies '
'cacheWidth, cacheHeight, or allowUpscaling.'
);
return decode(bytes, cacheWidth: width, cacheHeight: height);
return decode(bytes, cacheWidth: width, cacheHeight: height, allowUpscaling: this.allowUpscaling);
};
return imageProvider.load(key.providerCacheKey, decodeResize);
}
Expand Down
15 changes: 15 additions & 0 deletions packages/flutter/test/painting/image_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,21 @@

// @dart = 2.8


/// A 50x50 blue square png.
const List<int> kBlueSquare = <int>[
0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49,
0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x32, 0x00, 0x00, 0x00, 0x32, 0x08, 0x06,
0x00, 0x00, 0x00, 0x1e, 0x3f, 0x88, 0xb1, 0x00, 0x00, 0x00, 0x48, 0x49, 0x44,
0x41, 0x54, 0x78, 0xda, 0xed, 0xcf, 0x31, 0x0d, 0x00, 0x30, 0x08, 0x00, 0xb0,
0x61, 0x63, 0x2f, 0xfe, 0x2d, 0x61, 0x05, 0x34, 0xf0, 0x92, 0xd6, 0x41, 0x23,
0x7f, 0xf5, 0x3b, 0x20, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44,
0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44,
0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44, 0x44,
0x44, 0x44, 0x44, 0x36, 0x06, 0x03, 0x6e, 0x69, 0x47, 0x12, 0x8e, 0xea, 0xaa,
0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
];

const List<int> kTransparentImage = <int>[
0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, 0x00, 0x00, 0x0D, 0x49,
0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x06,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import 'mocks_for_image_cache.dart';
void main() {
TestRenderingFlutterBinding();

final DecoderCallback _basicDecoder = (Uint8List bytes, {int cacheWidth, int cacheHeight}) {
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight);
final DecoderCallback _basicDecoder = (Uint8List bytes, {int cacheWidth, int cacheHeight, bool allowUpscaling}) {
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight, allowUpscaling: allowUpscaling ?? false);
};

FlutterExceptionHandler oldError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import 'image_data.dart';
void main() {
TestRenderingFlutterBinding();

final DecoderCallback _basicDecoder = (Uint8List bytes, {int cacheWidth, int cacheHeight}) {
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight);
final DecoderCallback _basicDecoder = (Uint8List bytes, {int cacheWidth, int cacheHeight, bool allowUpscaling}) {
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight, allowUpscaling: allowUpscaling);
};

_MockHttpClient httpClient;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,40 @@ void main() {
PaintingBinding.instance.imageCache.clearLiveImages();
});

test('ResizeImage resizes to the correct dimensions', () async {
test('ResizeImage resizes to the correct dimensions (up)', () async {
final Uint8List bytes = Uint8List.fromList(kTransparentImage);
final MemoryImage imageProvider = MemoryImage(bytes);
final Size rawImageSize = await _resolveAndGetSize(imageProvider);
expect(rawImageSize, const Size(1, 1));

const Size resizeDims = Size(14, 7);
final ResizeImage resizedImage = ResizeImage(MemoryImage(bytes), width: resizeDims.width.round(), height: resizeDims.height.round(), allowUpscaling: true);
const ImageConfiguration resizeConfig = ImageConfiguration(size: resizeDims);
final Size resizedImageSize = await _resolveAndGetSize(resizedImage, configuration: resizeConfig);
expect(resizedImageSize, resizeDims);
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56312


test('ResizeImage resizes to the correct dimensions (down)', () async {
final Uint8List bytes = Uint8List.fromList(kBlueSquare);
final MemoryImage imageProvider = MemoryImage(bytes);
final Size rawImageSize = await _resolveAndGetSize(imageProvider);
expect(rawImageSize, const Size(50, 50));

const Size resizeDims = Size(25, 25);
final ResizeImage resizedImage = ResizeImage(MemoryImage(bytes), width: resizeDims.width.round(), height: resizeDims.height.round(), allowUpscaling: true);
const ImageConfiguration resizeConfig = ImageConfiguration(size: resizeDims);
final Size resizedImageSize = await _resolveAndGetSize(resizedImage, configuration: resizeConfig);
expect(resizedImageSize, resizeDims);
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/56312

test('ResizeImage resizes to the correct dimensions - no upscaling', () async {
final Uint8List bytes = Uint8List.fromList(kTransparentImage);
final MemoryImage imageProvider = MemoryImage(bytes);
final Size rawImageSize = await _resolveAndGetSize(imageProvider);
expect(rawImageSize, const Size(1, 1));

const Size resizeDims = Size(1, 1);
final ResizeImage resizedImage = ResizeImage(MemoryImage(bytes), width: resizeDims.width.round(), height: resizeDims.height.round());
const ImageConfiguration resizeConfig = ImageConfiguration(size: resizeDims);
final Size resizedImageSize = await _resolveAndGetSize(resizedImage, configuration: resizeConfig);
Expand Down Expand Up @@ -73,10 +100,11 @@ void main() {
final MemoryImage memoryImage = MemoryImage(bytes);
final ResizeImage resizeImage = ResizeImage(memoryImage, width: 123, height: 321);

final DecoderCallback decode = (Uint8List bytes, {int cacheWidth, int cacheHeight}) {
final DecoderCallback decode = (Uint8List bytes, {int cacheWidth, int cacheHeight, bool allowUpscaling}) {
expect(cacheWidth, 123);
expect(cacheHeight, 321);
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight);
expect(allowUpscaling, false);
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight, allowUpscaling: allowUpscaling);
};

resizeImage.load(await resizeImage.obtainKey(ImageConfiguration.empty), decode);
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter/test/painting/painting_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class PaintingBindingSpy extends BindingBase with SchedulerBinding, ServicesBind
int get instantiateImageCodecCalledCount => counter;

@override
Future<ui.Codec> instantiateImageCodec(Uint8List list, {int cacheWidth, int cacheHeight}) {
Future<ui.Codec> instantiateImageCodec(Uint8List list, {int cacheWidth, int cacheHeight, bool allowUpscaling = false}) {
counter++;
return ui.instantiateImageCodec(list);
}
Expand Down
8 changes: 5 additions & 3 deletions packages/flutter/test/widgets/fade_in_image_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -323,11 +323,12 @@ Future<void> main() async {
);

bool called = false;
final DecoderCallback decode = (Uint8List bytes, {int cacheWidth, int cacheHeight}) {
final DecoderCallback decode = (Uint8List bytes, {int cacheWidth, int cacheHeight, bool allowUpscaling}) {
expect(cacheWidth, 20);
expect(cacheHeight, 30);
expect(allowUpscaling, false);
called = true;
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight);
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight, allowUpscaling: allowUpscaling);
};
final ImageProvider resizeImage = image.placeholder;
expect(image.placeholder, isA<ResizeImage>());
Expand All @@ -345,9 +346,10 @@ Future<void> main() async {
);

bool called = false;
final DecoderCallback decode = (Uint8List bytes, {int cacheWidth, int cacheHeight}) {
final DecoderCallback decode = (Uint8List bytes, {int cacheWidth, int cacheHeight, bool allowUpscaling}) {
expect(cacheWidth, null);
expect(cacheHeight, null);
expect(allowUpscaling, null);
called = true;
return PaintingBinding.instance.instantiateImageCodec(bytes, cacheWidth: cacheWidth, cacheHeight: cacheHeight);
};
Expand Down

0 comments on commit 0852c51

Please sign in to comment.