Revisit/Simplify cached image resizing APIs

Use case

ImageProviders and ResizeImage in particular have an API for resizing images: cache(Width|Height). The name "cache" suggests this API is mostly meant to help users reduce their memory footprint.

There's a couple of issues:

It's hard to use for generic case when your depending on the image dimensions your horizontally or vertically constraint. It's easy to accidentally change the aspect ratio or accidentally upscale the image. Both are likely unintended given that this is not a generic resizing API. Upscaling a low-res image to the final layout size should likely be handled by the gpu.
the name cache is misleading. This is a simple image resize and helps with memory even when you disable the image cache.

Proposal

replace cache(Height|Wdith) with something like downScaleLongEdgeToMax or max(Height|Width). Really any API that prevents accidental upscaling and aspect ratio changes.

@iskakaushik

/cc @iskakaushik @mklim

Currently if cacheWidth and cacheHeight are not specified, we decode and cache the image at its original size. Using ResizeImage is opt-in; could we possibly make it opt-out instead?

If cacheWidth and cacheHeight aren't explicitly specified, could we deduce them from width and height?
Could we at least emit warnings or other diagnostics when the decoded image size turns out to be much larger than the displayed size?

Additionally, today I found that using the cacheWidth and cacheHeight parameters causes the resulting image to have far less details than without using the parameters, although I set them to be the same as the image's width and height:

Image.asset(
  'assets/search_initial.png',
  width: 300,
  height: 300,
  cacheWidth: 300,
  cacheHeight: 300,
)

I was under the assumption that if I supply the same values to the cacheWidth / cacheHeight parameters as I do to the width / height parameters, then the quality should be exactly the same, but the cache uses the given dimensions instead of a possibly much larger native size.

I thought the cache parameters are just a way to tell the engine that I will never draw the image in a higher resolution, so it is ok to cache it at the specified resolution in order to reduce memory footprint.

Am I completely wrong here or are the cache parameters maybe simply given in native pixels instead of device independent pixels?

@ignatz

Thanks for the feedback everyone.

Seems like the problems with the API are:

Hard to know if the api will alter with the aspect ratio of the image.
cacheWidth seems to imply that this will only help with memory if image cache is enabled.
It's not clear why it isn't the default on given that we can deduce the width and height from the widget.

@ignatz , regarding 1, I like you suggestion of maxWidth and maxHeight. The one concern I have is that as it stands right now that API does provide the flexibility to resize violating the aspect ratio, which will not be possible if we move to max{Width, Height}. Though this might be a worthy trade-off. Let me think about this.

For 2 (though this might change depending on 1) the docs indicate that this api is primarily intended to reduce the memory usage of [ImageCache]. If an app is concerned with memory usage it will likely enable ImageCache, so this might not be too big of a concern. Maybe we could add something to the effect of this will also reduce the memory footprint when [ImageCache] is disabled.

@jamesderlin , regarding 3, making this the default behavior is something I was planning on doing once the api matures. Currently it still needs some more polishing as it's evident from this issue. I've filed #56198 to track making this default-on. As far as the logging is concerned, please file another issue as this one is regarding API design.

@nioncode there seems to be some inconsistencies in the downsampling we do when the parameters are present vs what skia uses to downsample a native image to a smaller canvas. Though this is not completely unexpected, I'm surprised that this is resulting in images with far less detail. Given that this doesn't deal with the api design itself but rather the implementation of it, would you mind opening another issue for this? (along with the device details and maybe a sample image). Thanks.

Deduplicating this to #59578, which is slightly different but achieves the same goals.

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.

Navigation Menu

Repository navigation

Revisit/Simplify cached image resizing APIs #48885

Use case

Proposal

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Revisit/Simplify cached image resizing APIs #48885

Description

Use case

Proposal

Activity

goderbauer commented on Jan 15, 2020

jamesderlin commented on Apr 30, 2020

nioncode commented on May 2, 2020

iskakaushik commented on May 2, 2020

dnfield commented on Jun 16, 2020

github-actions commented on Aug 13, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions