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

Unable to use ResizeImage effectively when you don't know the aspect ratio of the image ahead of time #118543

Closed
tvolkert opened this issue Jan 16, 2023 · 1 comment · Fixed by #121154
Assignees
Labels
a: images Loading, displaying, rendering images c: performance Relates to speed or footprint issues (see "perf:" labels) framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list perf: memory Performance issues related to memory

Comments

@tvolkert
Copy link
Contributor

tvolkert commented Jan 16, 2023

Steps to reproduce

  1. Load images over the web from an API that will give random images (and they might be WAY oversized).
  2. Try to resize them so as to not take up too much memory (either in the image cache or when uploading to the GPU). Resize them to fit within a bounding box, but also to maintain their aspect ratio

Expected behavior

You expect to be able to satisfy the use case and only cache the resized images.

Actual behavior

ResizeImage takes a width, a height, or both. You can omit one to have to maintain its aspect ratio and respect the dimension that you gave it. However, this doesn't meet the needs of the use case, because since you don't know the size of the original image, you don't know which dimension will be the limiting dimension. This means you need to first decode the original image into its intrinsic size in order to know which dimensions to give to ResizeImage. But this "pre-decoding" will inadvertently cache the image in its original size.

All in all, it would be very useful to provide an easy way to say "if you tell me the image descriptor info, I'll tell you what target size I want" and to use that facility in ResizeImage

@tvolkert tvolkert added c: performance Relates to speed or footprint issues (see "perf:" labels) a: images Loading, displaying, rendering images perf: memory Performance issues related to memory labels Jan 16, 2023
@tvolkert tvolkert self-assigned this Jan 16, 2023
tvolkert added a commit to tvolkert/engine that referenced this issue Jan 16, 2023
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
tvolkert added a commit to tvolkert/engine that referenced this issue Jan 20, 2023
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
auto-submit bot pushed a commit to flutter/engine that referenced this issue Jan 23, 2023
* 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

* Add test

* Fixed test failure

* Update

* Respond to review comments

* Add web implementation

* Fixed typo

* Review comments

Also changed the TargetImageSizeCallback to just take intrinsic
width & height, rather than the full image descriptor.

* Forgot to remove the _SizeOnlyImageDescriptor class - it's no longer needed

* Forgot to update test
@darshankawar darshankawar added the framework flutter/packages/flutter repository. See also f: labels. label Jan 24, 2023
@goderbauer goderbauer added the P2 Important issues not at the top of the work list label Jan 24, 2023
tvolkert added a commit to tvolkert/flutter that referenced this issue Jan 25, 2023
ricardoamador pushed a commit to ricardoamador/engine that referenced this issue Jan 25, 2023
* 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

* Add test

* Fixed test failure

* Update

* Respond to review comments

* Add web implementation

* Fixed typo

* Review comments

Also changed the TargetImageSizeCallback to just take intrinsic
width & height, rather than the full image descriptor.

* Forgot to remove the _SizeOnlyImageDescriptor class - it's no longer needed

* Forgot to update test
tvolkert added a commit that referenced this issue Jan 26, 2023
This updates the framework to provide higher level wrappers around ui.instantiateImageCodecWithSize(). Functionally, this doesn't change anything (other than deprecating the older loadBuffer() method in favor of loadImage()), but it sets the stage for a simpler change that will allow us to provide a more flexible way to load sized images.

#118543
@tvolkert tvolkert mentioned this issue Feb 21, 2023
8 tasks
tvolkert added a commit to tvolkert/flutter that referenced this issue Feb 22, 2023
This adds a new `ResizeImage.policy` property that controls how `ResizeImage`
will interpret its `width` and `height` properties. The existing behavior is
preserved via `ResizeImagePolicy.exact` (default), but there is now the option
to use `ResizeImagePolicy.fit`, which satisfies the use case outlined in
flutter#118543.

The API doc assets were added in flutter/assets-for-api-docs#209

Fixes flutter#118543
auto-submit bot pushed a commit that referenced this issue Feb 23, 2023
* Add ResizeImage.policy

This adds a new `ResizeImage.policy` property that controls how `ResizeImage`
will interpret its `width` and `height` properties. The existing behavior is
preserved via `ResizeImagePolicy.exact` (default), but there is now the option
to use `ResizeImagePolicy.fit`, which satisfies the use case outlined in
#118543.

The API doc assets were added in flutter/assets-for-api-docs#209

Fixes #118543

* Docuemnt public member

* Remove protected annotation from overrides - was failing tests

* Fixed analysis of code in Dartdoc

* More dartdoc code analysis fixes

* One more fix

* Review comments
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: images Loading, displaying, rendering images c: performance Relates to speed or footprint issues (see "perf:" labels) framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list perf: memory Performance issues related to memory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants