square / picasso

A powerful image downloading and caching library for Android
https://square.github.io/picasso/
Apache License 2.0
18.71k stars 3.97k forks source link

Allow transformed Bitmaps to be (optionally) cached at interim transformation steps #205

Open jhansche opened 11 years ago

jhansche commented 11 years ago

An example would be if we want to have a regular thumbnail image, plus a slightly transformed (pixelated/blurred, masked, cropped, etc) version of that thumbnail. Right now, if we request the original thumbnail, it gets cached using the original bitmap key; but if we request that same original thumbnail followed by a MaskTransformation (e.g., clip it to some alpha mask), it must make a separate request to fetch the original Bitmap, even though we already have that Bitmap in the cache.

I would expect that if we request a mask-transformed version of the original thumbnail, we should be able to fetch the original from cache, then transform it, then cache the transformed version.

Or, the other way around: request a masked version of the thumbnail, it will download the thumbnail, then apply the mask, and then cache that. If we then request the unmasked Bitmap, it will re-request the original thumbnail, and then cache that. But the first request for the masked version should have already had a cacheable version of that thumbnail that could have been returned immediately.

I understand the read-through HTTP disk cache will be able to account for the 2nd use case^, but that's only true for HTTP requests, not for something like a ContentProvider request. And if the original image is actually a full-size image, then requesting a thumbnail version, followed by a masked thumbnail of the same image, actually ends up reading from the ContentProvider FIVE times (twice for each thumbnail type, because it needs to compute the scale factor based on the resize dimensions, and that causes it to reopen the file descriptor twice, plus once for the original size). That's 5 times we have to load the full-size image data into a file descriptor that Picasso can use for decoding.

One possible issue with allowing those to be cached intermediately, would be the fact that some transformations end up modifying the source Bitmap that is given to it, instead of creating a new Bitmap. That could result in altering the in-cache Bitmap unexpectedly. But if we make that an explicit desire, the Transformation object can take that into account and decide whether it wants to reuse the source, or if it should create a copy (so as not to modify the original source). E.g.:

    Picasso.with(this).load("content://com.example/image").resize(100, 100).cropCenter().into(imgThumb);
    Picasso.with(this).load("content://com.example/image").into(imgFull);
    Picasso.with(this).load("content://com.example/image").resize(100, 100).cropCenter().transform(new MaskTransformation()).into(imgMasked);

imgFull will decode the full file descriptor handed to it from the ContentProvider and cache that in memory. imgThumb will decode the full file descriptor from the ContentProvider twice (once for inDecodeBounds in order to set up the scaling factor, and a second time to do the actual decoding with a scaling inSampleSize), then it will cache to memory. And imgMasked will do all the same work that imgThumb did, and then it will perform the transformation and cache to memory.

Ideally, the imgFull read would be able to cache that original bitmap; imgThumb will be able to start from a copy of the imgFull cached Bitmap and then perform its resize and crop; and imgMasked should be able to start from a copy of imgThumb's cache and only apply the mask transformation from that.

I believe one way to at least get a quick performance boost would be to attempt to rewind() the InputStream that is obtained from the ContentProvider. If that is able to rewind, then there's no need to re-open the same URI from the ContentProvider (if it can't rewind, then so be it...). In my case, I'm using ParcelFileDescriptor.createPipe() to do my reading from a separate thread.

JakeWharton commented 11 years ago

We talked about this very same thing (but in a different context) in the planning stages for v2. I think there is room for something like this in a post-2.0 world but it'd require a significant refactor and potential change in key structure (or maybe the cache interface).

I certainly don't think something like this is the common case. Like you said, the HTTP cache will mitigate duplicate requests for the same image and the use of a content provider should imply that the file is on the device somewhere.

I always like optimizing for speed and efficiency so we should definitely keep this in mind one 2.0 is released.

jhansche commented 11 years ago

@JakeWharton, yes, in our case the ContentProvider obtains the full bitmap from a BLOB record in a SQLite database, so it is "on the device". But it does still take anywhere from 300-700ms to decode that image, which can be problematic when the request is for a resized thumbnail, because it does have to read the stream twice (due to inDecodeBounds and the fact that it has to re-open the InputStream for the 2nd down-sampled decode). Perhaps a more immediate improvement could be made in that case, where it can attempt to rewind back to the 0-offset mark after the inDecodeBounds pass, to avoid having to requery the ContentProvider when doing the 2nd pass?

dnkoutso commented 11 years ago

We will be discussing this feature for the next version of Picasso.

stephanenicolas commented 10 years ago

If the implementation is based on DiskLruCache, then https://github.com/JakeWharton/DiskLruCache/issues/68 is linked.

JakeWharton commented 10 years ago

@stephanenicolas we wouldn't need to mock it for testing. It can write to a temp directory.

raylee4204 commented 9 years ago

I'm experiencing the same issue with the latest version of Picasso. Is there anyway to cache the masked images separately?

JakeWharton commented 9 years ago

No