Skip to content

Conversation

@piki
Copy link

@piki piki commented Aug 11, 2014

I have a fairly straightforward use case that I couldn't find a good way to implement with Picasso, so I added a new resizer stage to the decoding pipeline.

The use case is: display images as a column of ImageViews in a ListView, with all images scaled to no-larger-than the ListView's width, while preserving aspect ratio. ImageView heights are sized based on the images Picasso loads into them, rather than the other way around. Think web browser, messaging app, or chat room.

A custom transformation can sort of implement this, but not well enough. Some of the images are pretty big, up to 10 megapixels, which means that merely creating the Bitmap to transform takes 20-40MB of RAM per image. Even if that Bitmap is scaled down and recycled, it's a waste of RAM and CPU to create it full-scale. Doing a whole column of these thrashes the GC, then starts failing image loads with OOM, then eventually crashes the whole app with OOM. I want to scale the images at decode time, not transform time, but without knowing the exact width and height up front.

So what I want is the ability to pick image bounds after the image is retrieved and its inherent (full-size) bounds are known, but before it is actually decoded into a Bitmap.

Rather than build something specific to my fit-to-width use case -- e.g., resize(maxImageWidth, -1) -- I generalized. I added a new function to the fluent builder pipeline, resizer, to specify a dynamic resizing function. Here's how it works:

Resizer fitWidth = new Resizer() {
    @Override
    public Point resize(int width, int height) {
        if (width <= maxImageWidth) return null;  // small enough; don't increase the size
        float scaleRatio = maxImageWidth / (float)width;
        int newWidth = (int)(width * scaleRatio);
        int newHeight = (int)(height * scaleRatio);
        return new Point(newWidth, newHeight);
    }
};
Picasso.with(context).load(imageUrl).resizer(fitWidth).error(R.drawable.img_broken).into(ivBody);

I believe this would be useful for #226.

@JakeWharton
Copy link
Collaborator

I'd rather we just accept 0 as an argument to resize or resizeDimen which meant scale with respect to aspect ratio.

@piki
Copy link
Author

piki commented Aug 12, 2014

The magical zero is definitely simpler, to support and to use. It doesn't quite cover the functionality for my use case, since I'm not scaling small images up to the container width.

@JakeWharton
Copy link
Collaborator

The current implementation of this has a few problems. It's a heavy API for one. This would be a big commitment to take into the API for supporting for the forseeable future. It also looks like it can't honor the contract of keys which need to be deterministic and immutable. By changing the target size dynamically you are going to cause all kinds of really subtle bugs.

@piki
Copy link
Author

piki commented Aug 12, 2014

I'm happy to iterate if you want me to clean up the feature, but I won't be offended if you want to 🔥 the PR. Thanks for taking the time to consider it.

If I'm cleaning it up:

  • I think keys are solvable. "fitWidth(" + maxImageWidth + ")" seems fine for my example. It's cleaner if it's a named class and maxImageWidth is a member, but I think the key contract can be upheld either way. The general case of making key functions for resizers is no harder than for transformers -- the transformation for any particular value of key needs to be deterministic.
  • I make targetWidth and targetHeight final again by introducing two new fields that are only used for decode-time scaling.
  • I'm open to suggestions about the API. A fluent function like scaleToNoMoreThanMaxWidthWhilePreservingRatio would solve my specific case but no others.

@piki piki closed this Feb 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants