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

Added target from cache #48

Closed

Conversation

csobrinho
Copy link
Contributor

Allow the target to know if the passed bitmap came from a local cache or a network request. Very useful in cases you need to start or not a custom animation with a TransitionDrawable for instance.

Carlos Sobrinho added 3 commits November 4, 2013 12:54
configs, which can't generally be reused.


Signed-off-by: Carlos Sobrinho <[email protected]>
… or a network request. Very useful in cases you need to start or not a custom animation with a TransitionDrawable.

Signed-off-by: Carlos Sobrinho <[email protected]>
@sjudd
Copy link
Collaborator

sjudd commented Nov 14, 2013

Looks good to me but did you mean to include the null check in the bitmap pool? If so did that or the recent change to only recycle for pngs and jpegs solve the exception?

I'm thinking of creating a 3.0 branch and merging this into that since it's definitely not backwards compatible.

@csobrinho
Copy link
Contributor Author

Didn't saw it until now. It still have the check but I still have (although very rarely) the dreadful exception:

java.lang.IllegalArgumentException: Problem decoding into existing bitmap
    at android.graphics.BitmapFactory.decodeStream(BitmapFactory.java:540)
    at com.bumptech.glide.resize.load.Downsampler.decodeStream(Downsampler.java:211)

even after the changes in the recycling and config. I think I would prefer to be safe than sorry. Can't afford to have random crashes in the app because a tiny 50x50 px icon couldn't be decoded. I will add again the try/catch in the method and sleep in peace 👍

@lalith-b
Copy link

+1

@@ -20,9 +20,10 @@

/**
* The method that will be called when the image load has finished
* @param fromCache True if the bitmap comes from a local cache (memory or disk).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually only true if the image comes from the in memory cache. Really it's an indication that onImageReady was called synchronously with the Glide.load call.

@sjudd
Copy link
Collaborator

sjudd commented Nov 24, 2013

Added a couple of quick comments, still LGTM.

@sjudd
Copy link
Collaborator

sjudd commented Jun 28, 2014

For closure I've thought about this a lot and ultimately I'm wary of putting this kind of logic in Target. Target ideally is a dumb receptacle for data. However to address this class of issues, I've updated the animation logic to be more robust and I've also expanded the RequestListener interface so that it is always called whenever the request interacts with the target and so that it has the ability to override the default behavior as you're doing here.

Hopefully that's something of a compromise, you can see these changes in the 3.0a branch.

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.

3 participants