Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make upscaling images opt-in #59856

Merged
merged 6 commits into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
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,
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a sentence about this parameter to the doc comment above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the docstring in general here - there were some old no-longer-existant things in there and some formatting issues, as well as something that's changed about aspect ratio preservation. PTAL

}) {
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