diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index b78f9b476103c..2aaa5a08b6f91 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1695,28 +1695,45 @@ class Codec extends NativeFieldWrapperClass2 { void dispose() native 'Codec_dispose'; } -/// Instantiates an image codec [Codec] object. +/// Instantiates an image [Codec]. /// -/// [list] is the binary image data (e.g a PNG or GIF binary data). +/// The `list` 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 flutter.dart:ui.imageFormats} /// -/// 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. If only one dimension -/// is specified, the omitted dimension will be scaled to maintain the original -/// aspect ratio. If both are not specified, then the image maintains its real -/// size. +/// 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. +/// If the `allowUpscaling` parameter is not set to true, both dimensions will +/// be capped at the intrinsic dimensions of the image, even if only one of +/// them would have exceeded those intrinsic dimensions. If exactly one of these +/// two arguments is specified, then the aspect ratio will be maintained while +/// forcing the image to match the other given dimension. If neither is +/// specified, then the image maintains its intrinsic size. +/// +/// 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. If the image must be scaled +/// up, the `allowUpscaling` parameter must be set to true. /// /// The returned future can complete with an error if the image decoding has /// failed. -Future instantiateImageCodec(Uint8List list, { +Future instantiateImageCodec( + Uint8List list, { int? targetWidth, int? targetHeight, + bool allowUpscaling = true, }) { - return _futurize( - (_Callback callback) => _instantiateImageCodec(list, callback, null, targetWidth ?? _kDoNotResizeDimension, targetHeight ?? _kDoNotResizeDimension) - ); + return _futurize((_Callback callback) { + return _instantiateImageCodec( + list, + callback, + null, + targetWidth ?? _kDoNotResizeDimension, + targetHeight ?? _kDoNotResizeDimension, + allowUpscaling, + ); + }); } /// Instantiates a [Codec] object for an image binary data. @@ -1730,8 +1747,14 @@ Future instantiateImageCodec(Uint8List list, { /// If both are equal to [_kDoNotResizeDimension], then the image maintains its real size. /// /// Returns an error message if the instantiation has failed, null otherwise. -String? _instantiateImageCodec(Uint8List list, _Callback callback, _ImageInfo? imageInfo, int targetWidth, int targetHeight) - native 'instantiateImageCodec'; +String? _instantiateImageCodec( + Uint8List list, + _Callback callback, + _ImageInfo? imageInfo, + int targetWidth, + int targetHeight, + bool allowUpscaling, +) native 'instantiateImageCodec'; /// Loads a single image frame from a byte array into an [Image] object. /// @@ -1751,30 +1774,56 @@ Future _decodeImageFromListAsync(Uint8List list, /// Convert an array of pixel values into an [Image] object. /// -/// [pixels] is the pixel data in the encoding described by [format]. +/// The `pixels` parameter is the pixel data in the encoding described by +/// `format`. /// -/// [rowBytes] is the number of bytes consumed by each row of pixels in the -/// data buffer. If unspecified, it defaults to [width] multiplied by the -/// number of bytes per pixel in the provided [format]. +/// The `rowBytes` parameter is the number of bytes consumed by each row of +/// pixels in the data buffer. If unspecified, it defaults to `width` multiplied +/// by the number of bytes per pixel in the provided `format`. /// -/// 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. If exactly one of -/// these two arguments is specified, then the aspect ratio will be maintained -/// while forcing the image to match the other given dimension. If neither is -/// specified, then the image maintains its real size. +/// 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. +/// If the `allowUpscaling` parameter is not set to true, both dimensions will +/// be capped at the intrinsic dimensions of the image, even if only one of +/// them would have exceeded those intrinsic dimensions. If exactly one of these +/// two arguments is specified, then the aspect ratio will be maintained while +/// forcing the image to match the other given dimension. If neither is +/// specified, then the image maintains its intrinsic size. +/// +/// 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. If the image must be scaled +/// up, the `allowUpscaling` parameter must be set to true. void decodeImageFromPixels( Uint8List pixels, int width, int height, PixelFormat format, - ImageDecoderCallback callback, - {int? rowBytes, int? targetWidth, int? targetHeight} -) { + ImageDecoderCallback callback, { + int? rowBytes, + int? targetWidth, + int? targetHeight, + bool allowUpscaling = true, +}) { + if (targetWidth != null) { + assert(allowUpscaling || targetWidth <= width); + } + if (targetHeight != null) { + assert(allowUpscaling || targetHeight <= height); + } + final _ImageInfo imageInfo = _ImageInfo(width, height, format.index, rowBytes); - final Future codecFuture = _futurize( - (_Callback callback) => _instantiateImageCodec(pixels, callback, imageInfo, targetWidth ?? _kDoNotResizeDimension, targetHeight ?? _kDoNotResizeDimension) - ); + final Future codecFuture = _futurize((_Callback callback) { + return _instantiateImageCodec( + pixels, + callback, + imageInfo, + targetWidth ?? _kDoNotResizeDimension, + targetHeight ?? _kDoNotResizeDimension, + allowUpscaling, + ); + }); codecFuture.then((Codec codec) => codec.getNextFrame()) .then((FrameInfo frameInfo) => callback(frameInfo.image)); } diff --git a/lib/ui/painting/codec.cc b/lib/ui/painting/codec.cc index b283f124b863f..a2abdf29a0f7f 100644 --- a/lib/ui/painting/codec.cc +++ b/lib/ui/painting/codec.cc @@ -191,10 +191,12 @@ static void InstantiateImageCodec(Dart_NativeArguments args) { } } - const int targetWidth = + const int target_width = tonic::DartConverter::FromDart(Dart_GetNativeArgument(args, 3)); - const int targetHeight = + const int target_height = tonic::DartConverter::FromDart(Dart_GetNativeArgument(args, 4)); + const bool allow_upscaling = + tonic::DartConverter::FromDart(Dart_GetNativeArgument(args, 5)); std::unique_ptr codec; bool single_frame; @@ -215,12 +217,15 @@ static void InstantiateImageCodec(Dart_NativeArguments args) { ImageDecoder::ImageDescriptor descriptor; descriptor.decompressed_image_info = image_info; - if (targetWidth > 0) { - descriptor.target_width = targetWidth; + if (target_width > 0) { + descriptor.target_width = target_width; } - if (targetHeight > 0) { - descriptor.target_height = targetHeight; + if (target_height > 0) { + descriptor.target_height = target_height; } + descriptor.image_upscaling = allow_upscaling + ? ImageUpscalingMode::kAllowed + : ImageUpscalingMode::kNotAllowed; descriptor.data = std::move(buffer); ui_codec = fml::MakeRefCounted(std::move(descriptor)); @@ -247,7 +252,7 @@ void Codec::dispose() { void Codec::RegisterNatives(tonic::DartLibraryNatives* natives) { natives->Register({ - {"instantiateImageCodec", InstantiateImageCodec, 5, true}, + {"instantiateImageCodec", InstantiateImageCodec, 6, true}, }); natives->Register({FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); } diff --git a/lib/ui/painting/image_decoder.cc b/lib/ui/painting/image_decoder.cc index 5b322e1d8deb0..c778d1ba22a56 100644 --- a/lib/ui/painting/image_decoder.cc +++ b/lib/ui/painting/image_decoder.cc @@ -42,11 +42,23 @@ static double AspectRatio(const SkISize& size) { // intrinsic dimensions of the image. static SkISize GetResizedDimensions(SkISize current_size, std::optional target_width, - std::optional target_height) { + std::optional target_height, + ImageUpscalingMode image_upscaling) { if (current_size.isEmpty()) { return SkISize::MakeEmpty(); } + if (image_upscaling == ImageUpscalingMode::kNotAllowed) { + if (target_width) { + target_width = std::min(current_size.width(), + static_cast(target_width.value())); + } + if (target_height) { + target_height = std::min(current_size.height(), + static_cast(target_height.value())); + } + } + if (target_width && target_height) { return SkISize::Make(target_width.value(), target_height.value()); } @@ -83,18 +95,8 @@ static sk_sp ResizeRasterImage(sk_sp image, return image->makeRasterImage(); } - if (resized_dimensions.width() > image->dimensions().width() || - resized_dimensions.height() > image->dimensions().height()) { - FML_LOG(WARNING) << "Image is being upsized from " - << image->dimensions().width() << "x" - << image->dimensions().height() << " to " - << resized_dimensions.width() << "x" - << resized_dimensions.height() - << ". Are cache(Height|Width) used correctly?"; - // TOOD(48885): consider exiting here, there's no good reason to support - // upsampling in a "caching"-optimization context.. - } - + // TODO(dnfield): remove this in favor of clearer constraints. + // https://github.com/flutter/flutter/issues/59578 const bool aspect_ratio_changed = std::abs(AspectRatio(resized_dimensions) - AspectRatio(image->dimensions())) > kAspectRatioChangedThreshold; @@ -140,6 +142,7 @@ static sk_sp ImageFromDecompressedData( ImageDecoder::ImageInfo info, std::optional target_width, std::optional target_height, + ImageUpscalingMode image_upscaling, const fml::tracing::TraceFlow& flow) { TRACE_EVENT0("flutter", __FUNCTION__); flow.Step(__FUNCTION__); @@ -155,8 +158,8 @@ static sk_sp ImageFromDecompressedData( return image->makeRasterImage(); } - auto resized_dimensions = - GetResizedDimensions(image->dimensions(), target_width, target_height); + auto resized_dimensions = GetResizedDimensions( + image->dimensions(), target_width, target_height, image_upscaling); return ResizeRasterImage(std::move(image), resized_dimensions, flow); } @@ -164,6 +167,7 @@ static sk_sp ImageFromDecompressedData( sk_sp ImageFromCompressedData(sk_sp data, std::optional target_width, std::optional target_height, + ImageUpscalingMode image_upscaling, const fml::tracing::TraceFlow& flow) { TRACE_EVENT0("flutter", __FUNCTION__); flow.Step(__FUNCTION__); @@ -185,8 +189,8 @@ sk_sp ImageFromCompressedData(sk_sp data, auto image_generator = SkCodecImageGenerator::MakeFromCodec(std::move(codec)); const auto& source_dimensions = image_generator->getInfo().dimensions(); - auto resized_dimensions = - GetResizedDimensions(source_dimensions, target_width, target_height); + auto resized_dimensions = GetResizedDimensions( + source_dimensions, target_width, target_height, image_upscaling); // No resize needed. if (resized_dimensions == source_dimensions) { @@ -344,11 +348,13 @@ void ImageDecoder::Decode(ImageDescriptor descriptor, descriptor.decompressed_image_info.value(), // descriptor.target_width, // descriptor.target_height, // + descriptor.image_upscaling, // flow // ) : ImageFromCompressedData(std::move(descriptor.data), // descriptor.target_width, // descriptor.target_height, // + descriptor.image_upscaling, // flow); if (!decompressed) { diff --git a/lib/ui/painting/image_decoder.h b/lib/ui/painting/image_decoder.h index b8190acdf5271..81e216a8a6d79 100644 --- a/lib/ui/painting/image_decoder.h +++ b/lib/ui/painting/image_decoder.h @@ -23,6 +23,9 @@ namespace flutter { +// Whether to allow for image upscaling when resizing an image. +enum class ImageUpscalingMode { kAllowed, kNotAllowed }; + // An object that coordinates image decompression and texture upload across // multiple threads/components in the shell. This object must be created, // accessed and collected on the UI thread (typically the engine or its runtime @@ -47,6 +50,7 @@ class ImageDecoder { std::optional decompressed_image_info; std::optional target_width; std::optional target_height; + ImageUpscalingMode image_upscaling = ImageUpscalingMode::kNotAllowed; }; using ImageResult = std::function)>; @@ -72,6 +76,7 @@ class ImageDecoder { sk_sp ImageFromCompressedData(sk_sp data, std::optional target_width, std::optional target_height, + ImageUpscalingMode image_upscaling, const fml::tracing::TraceFlow& flow); } // namespace flutter diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index d90378dd625fe..6c17157373fd6 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -364,6 +364,7 @@ TEST_F(ImageDecoderFixtureTest, CanDecodeWithResizes) { ImageDecoder::ImageDescriptor image_descriptor; image_descriptor.target_width = target_width; image_descriptor.target_height = target_height; + image_descriptor.image_upscaling = ImageUpscalingMode::kNotAllowed; image_descriptor.data = OpenFixtureAsSkData("DashInNooglerHat.jpg"); ASSERT_TRUE(image_descriptor.data); @@ -463,6 +464,7 @@ TEST_F(ImageDecoderFixtureTest, CanResizeWithoutDecode) { ImageDecoder::ImageDescriptor image_descriptor; image_descriptor.target_width = target_width; image_descriptor.target_height = target_height; + image_descriptor.image_upscaling = ImageUpscalingMode::kNotAllowed; image_descriptor.data = decompressed_data; image_descriptor.decompressed_image_info = info; @@ -530,11 +532,51 @@ TEST(ImageDecoderTest, VerifySimpleDecoding) { ASSERT_TRUE(image != nullptr); ASSERT_EQ(SkISize::Make(600, 200), image->dimensions()); - ASSERT_EQ(ImageFromCompressedData(data, 6, 2, fml::tracing::TraceFlow("")) + ASSERT_EQ(ImageFromCompressedData(data, 6, 2, ImageUpscalingMode::kNotAllowed, + fml::tracing::TraceFlow("")) ->dimensions(), SkISize::Make(6, 2)); } +TEST(ImageDecoderTest, VerifySimpleDecodingNoUpscaling) { + auto data = OpenFixtureAsSkData("Horizontal.jpg"); + auto image = SkImage::MakeFromEncoded(data); + ASSERT_TRUE(image != nullptr); + ASSERT_EQ(SkISize::Make(600, 200), image->dimensions()); + + ASSERT_EQ( + ImageFromCompressedData(data, 900, 300, ImageUpscalingMode::kNotAllowed, + fml::tracing::TraceFlow("")) + ->dimensions(), + SkISize::Make(600, 200)); +} + +TEST(ImageDecoderTest, VerifySimpleDecodingNoUpscalingOneDimension) { + auto data = OpenFixtureAsSkData("Horizontal.jpg"); + auto image = SkImage::MakeFromEncoded(data); + ASSERT_TRUE(image != nullptr); + ASSERT_EQ(SkISize::Make(600, 200), image->dimensions()); + + ASSERT_EQ( + ImageFromCompressedData(data, 1200, 200, ImageUpscalingMode::kNotAllowed, + fml::tracing::TraceFlow("")) + ->dimensions(), + SkISize::Make(600, 200)); +} + +TEST(ImageDecoderTest, VerifySimpleDecodingWithUpscaling) { + auto data = OpenFixtureAsSkData("Horizontal.jpg"); + auto image = SkImage::MakeFromEncoded(data); + ASSERT_TRUE(image != nullptr); + ASSERT_EQ(SkISize::Make(600, 200), image->dimensions()); + + ASSERT_EQ( + ImageFromCompressedData(data, 900, 300, ImageUpscalingMode::kAllowed, + fml::tracing::TraceFlow("")) + ->dimensions(), + SkISize::Make(900, 300)); +} + TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { auto data = OpenFixtureAsSkData("Horizontal.jpg"); auto image = SkImage::MakeFromEncoded(data); @@ -544,6 +586,7 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { auto decode = [data](std::optional target_width, std::optional target_height) { return ImageFromCompressedData(data, target_width, target_height, + ImageUpscalingMode::kNotAllowed, fml::tracing::TraceFlow("")); }; diff --git a/lib/web_ui/lib/src/ui/painting.dart b/lib/web_ui/lib/src/ui/painting.dart index 46c41b4b0d31a..0facfaeee8d14 100644 --- a/lib/web_ui/lib/src/ui/painting.dart +++ b/lib/web_ui/lib/src/ui/painting.dart @@ -1581,6 +1581,7 @@ Future instantiateImageCodec( Uint8List list, { int? targetWidth, int? targetHeight, + bool allowUpscaling = true, }) { return _futurize((engine.Callback callback) => // TODO: Implement targetWidth and targetHeight support. @@ -1647,9 +1648,12 @@ void decodeImageFromPixels( int width, int height, PixelFormat format, - ImageDecoderCallback callback, - {int? rowBytes, int? targetWidth, int? targetHeight} -) { + ImageDecoderCallback callback, { + int? rowBytes, + int? targetWidth, + int? targetHeight, + bool allowUpscaling = true, +}) { final _ImageInfo imageInfo = _ImageInfo(width, height, format.index, rowBytes); final Future codecFuture = _futurize( diff --git a/testing/dart/image_resize_test.dart b/testing/dart/image_resize_test.dart index 8ef592a25e354..484dfc48bb8c6 100644 --- a/testing/dart/image_resize_test.dart +++ b/testing/dart/image_resize_test.dart @@ -12,6 +12,12 @@ import 'package:path/path.dart' as path; import 'package:test/test.dart'; void main() { + bool assertsEnabled = false; + assert(() { + assertsEnabled = true; + return true; + }()); + test('no resize by default', () async { final Uint8List bytes = await readFile('4x4.png'); final Codec codec = await instantiateImageCodec(bytes); @@ -44,7 +50,7 @@ void main() { test('upscale image by 5x', () async { final Uint8List bytes = await readFile('4x4.png'); - final Codec codec = await instantiateImageCodec(bytes, targetWidth: 10); + final Codec codec = await instantiateImageCodec(bytes, targetWidth: 10, allowUpscaling: true); final FrameInfo frame = await codec.getNextFrame(); final int codecHeight = frame.image.height; final int codecWidth = frame.image.width; @@ -52,10 +58,20 @@ void main() { expect(codecWidth, 10); }); + test('upscale image by 5x - no upscaling', () async { + final Uint8List bytes = await readFile('4x4.png'); + final Codec codec = await instantiateImageCodec(bytes, targetWidth: 10, allowUpscaling: false); + final FrameInfo frame = await codec.getNextFrame(); + final int codecHeight = frame.image.height; + final int codecWidth = frame.image.width; + expect(codecHeight, 2); + expect(codecWidth, 2); + }); + test('upscale image varying width and height', () async { final Uint8List bytes = await readFile('4x4.png'); final Codec codec = - await instantiateImageCodec(bytes, targetWidth: 10, targetHeight: 1); + await instantiateImageCodec(bytes, targetWidth: 10, targetHeight: 1, allowUpscaling: true); final FrameInfo frame = await codec.getNextFrame(); final int codecHeight = frame.image.height; final int codecWidth = frame.image.width; @@ -63,6 +79,17 @@ void main() { expect(codecWidth, 10); }); + test('upscale image varying width and height - no upscaling', () async { + final Uint8List bytes = await readFile('4x4.png'); + final Codec codec = + await instantiateImageCodec(bytes, targetWidth: 10, targetHeight: 1, allowUpscaling: false); + final FrameInfo frame = await codec.getNextFrame(); + final int codecHeight = frame.image.height; + final int codecWidth = frame.image.width; + expect(codecHeight, 1); + expect(codecWidth, 2); + }); + test('pixels: no resize by default', () async { final BlackSquare blackSquare = BlackSquare.create(); final Image resized = await blackSquare.resize(); @@ -86,19 +113,60 @@ void main() { test('pixels: upscale image by 5x', () async { final BlackSquare blackSquare = BlackSquare.create(); - final Image resized = await blackSquare.resize(targetWidth: 10); + final Image resized = await blackSquare.resize(targetWidth: 10, allowUpscaling: true); expect(resized.height, 10); expect(resized.width, 10); }); + test('pixels: upscale image by 5x - no upscaling', () async { + final BlackSquare blackSquare = BlackSquare.create(); + bool threw = false; + try { + decodeImageFromPixels( + blackSquare.pixels, + blackSquare.width, + blackSquare.height, + PixelFormat.rgba8888, + (Image image) => null, + targetHeight: 10, + allowUpscaling: false, + ); + } catch (e) { + expect(e is AssertionError, true); + threw = true; + } + expect(threw, true); + }, skip: !assertsEnabled); + test('pixels: upscale image varying width and height', () async { final BlackSquare blackSquare = BlackSquare.create(); final Image resized = - await blackSquare.resize(targetHeight: 1, targetWidth: 10); + await blackSquare.resize(targetHeight: 1, targetWidth: 10, allowUpscaling: true); expect(resized.height, 1); expect(resized.width, 10); }); + test('pixels: upscale image varying width and height - no upscaling', () async { + final BlackSquare blackSquare = BlackSquare.create(); + bool threw = false; + try { + decodeImageFromPixels( + blackSquare.pixels, + blackSquare.width, + blackSquare.height, + PixelFormat.rgba8888, + (Image image) => null, + targetHeight: 10, + targetWidth: 1, + allowUpscaling: false, + ); + } catch (e) { + expect(e is AssertionError, true); + threw = true; + } + expect(threw, true); + }, skip: !assertsEnabled); + test('pixels: large negative dimensions', () async { final BlackSquare blackSquare = BlackSquare.create(); final Image resized = @@ -117,11 +185,18 @@ class BlackSquare { return BlackSquare._(width, height, pixels); } - Future resize({int targetWidth, int targetHeight}) async { + Future resize({int targetWidth, int targetHeight, bool allowUpscaling = false}) async { final Completer imageCompleter = Completer(); - decodeImageFromPixels(pixels, width, height, PixelFormat.rgba8888, - (Image image) => imageCompleter.complete(image), - targetHeight: targetHeight, targetWidth: targetWidth); + decodeImageFromPixels( + pixels, + width, + height, + PixelFormat.rgba8888, + (Image image) => imageCompleter.complete(image), + targetHeight: targetHeight, + targetWidth: targetWidth, + allowUpscaling: allowUpscaling, + ); return await imageCompleter.future; }