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

Correctly handle indexed pngs #25

Conversation

csobrinho
Copy link
Contributor

No description provided.

Carlos Sobrinho added 13 commits August 30, 2013 23:57
Signed-off-by: Carlos Sobrinho <[email protected]>
… and then the DiskCacheAdapter overrides the instance.

Signed-off-by: Carlos Sobrinho <[email protected]>
…ntException when ever the opts.inBitmap is not null and the native part of the decoder returns a null. My impression is that either there is a rare problem with the bitmap pool (wrong dimensions) or the image might have a different config (png vs gif for instance).

Signed-off-by: Carlos Sobrinho <[email protected]>
Conflicts:
	library/src/com/bumptech/glide/resize/load/Downsampler.java
	library/src/com/bumptech/glide/util/Log.java

Signed-off-by: Carlos Sobrinho <[email protected]>
Signed-off-by: Carlos Sobrinho <[email protected]>
Signed-off-by: Carlos Sobrinho <[email protected]>
- Added support for indexed PNG's (PNG-8) that can have an optional transparency.

Signed-off-by: Carlos Sobrinho <[email protected]>
@csobrinho
Copy link
Contributor Author

I'm back! Was having some problems with indexed PNG's with transparency like this one. Didn't noticed until now that the .gitignore was also changed...

bitmap.compress(bitmap.getConfig() == Bitmap.Config.ARGB_8888 ? Bitmap.CompressFormat.PNG : Bitmap.CompressFormat.JPEG, bitmapCompressQuality, os);
Bitmap.Config config = bitmap.getConfig();
bitmap.compress(config == null || config == Bitmap.Config.ARGB_8888 || config == Bitmap.Config.ARGB_4444
? Bitmap.CompressFormat.PNG : Bitmap.CompressFormat.JPEG, bitmapCompressQuality, os);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make this an if/else? This is rather complicated for a ternary (I shouldn't have used one to begin with).

@sjudd
Copy link
Collaborator

sjudd commented Oct 1, 2013

This looks great to me, thanks for catching and fixing those issues. The history is a little complicated though, would you mind squashing these down into a commit or two?

@csobrinho
Copy link
Contributor Author

Hi @sjudd . Sorry for the delay. I already merged in my code so how can I merge the lines now?

@sjudd
Copy link
Collaborator

sjudd commented Oct 10, 2013

Don't worry about it, it's not a big deal. You can just commit the small fix I mentioned about the ternary, push your branch to your remote again, and I'll merge it.

@sjudd
Copy link
Collaborator

sjudd commented Oct 10, 2013

Wait I think I see what happened. You merged your pull request into your remote. It hasn't actually been merged in to the main Glide repo. So you are still free to make changes to it and push them to your remote (which in turn will update this pull request).

@sjudd
Copy link
Collaborator

sjudd commented Oct 20, 2013

@deathlord87 Can you be more specific? Nothing in this pull request relates directly to exceptions?

@sjudd
Copy link
Collaborator

sjudd commented Oct 20, 2013

@deathlord87 I just saw what you are referring to. That commit was reverted when he merged with master again. You can click on 'Files Changed' tab on github to see the final diff between his branch and master (which doesn't include those changes).

@sjudd
Copy link
Collaborator

sjudd commented Oct 20, 2013

I cherry picked out the relevant commits and pushed them separately to master. Thanks for the contribution!

@sjudd sjudd closed this Oct 20, 2013
@csobrinho
Copy link
Contributor Author

Hi sorry for the delay. Have a release to do this week and I'm all tied up...

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