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

Feature: efficient rounding #165

Closed
AlexBlokh opened this issue Oct 2, 2014 · 5 comments
Closed

Feature: efficient rounding #165

AlexBlokh opened this issue Oct 2, 2014 · 5 comments
Labels

Comments

@AlexBlokh
Copy link

Transformation is always a creation and allocation of at least one more bitmap in memory. There is a way to make it way more efficient. RoundedImageView with RoundedDrawable. You just path a bitmap to roundedImageView, then it is passed to a roundedDrawable, transform matrices are calculated, bitmapShader swaps the bitmap and it just draws a circle or a rounded rectangle, or a shape with bitmapShader and it's way more efficient than making a transformation. But when I using this way of drawing rounded image, I'm not able to use a great feature of your library - crossfade.
So if it's possible to make a workaround of it?

@sjudd
Copy link
Collaborator

sjudd commented Oct 3, 2014

Two things.

First, it's possible to write an efficient Transformation with Glide. The BitmapTransformation provides you with a BitmapPool interface which allows you to re-use a Bitmap and avoid the allocation. Generally speaking drawing a Bitmap with some transformation(s) onto a Canvas is very fast, so the primary cost is the garbage collection from the allocation which Glide lets you avoid.

See the CenterCrop transformation for how this can be done. Also worth noting that typically the result of the transformation is cached, so you only incur the cost the first time you try to load the image.

Second, I looked into RoundedImageView a bit in #155. It seemed to work fine except for the cross fade animation, so it looks like the RoundedImageView library doesn't handle TransitionDrawables. The next step would be to create a simple sample app that just tries to do a cross fade using a TransitionDrawable and a RoundedImageView (without using Glide). If you'd like to create that sample app I'm sure the author of that project would appreciate it. It's also possible I'm incorrect about the cause so definitely let me know if you can't reproduce the problem with just a sample app.

@AlexBlokh
Copy link
Author

Thanks a lot, gonna do this asap!

@AlexBlokh
Copy link
Author

the only one thing, that prevents me to do all the stuff, is that

public class GlideBitmapDrawable extends Drawable

how can I get the bitmap from the GlideBitmapDrawable in library, where there is no dependency on Glide? Reflection or something else? Can you please help?

@sjudd
Copy link
Collaborator

sjudd commented Feb 24, 2015

You should be able to just pass in a BitmapTransformation to transform()? In theory you shouldn't need to mutate the drawable at all?

Keep in mind that you can also just request a Bitmap by using asBitmap():

    Glide.with(fragment)
        .load("imageUrl")
        .asBitmap()
        .into(new SimpleTarget<Bitmap>(width, height) {
             ...
        });

If you really need to mutate the Drawable, the only real solution is to use instanceof and cast the GlideDrawable to GlideBitmapDrawable, keeping in mind that it may also be a GifDrawable.

@AlexBlokh
Copy link
Author

Hi, guys! It's been a while

so basically I'll just copy my issue from here vinc3m1/RoundedImageView#196

Hi!
I'm not really sure it has to be here, but there're an issue with Glide library producing Transition Drawable and this library not being able to round it.

There're mainly two pitfalls:
1 - Transition drawable leaves a hidden layer visible after crossfading(fading) drawable in (if you're familiar with overdrawing - you know what I mean)

2 - Transition drawable extends from LayerDrawable(which has horrible api as for me) and LayerDrawable doesn't set drawable ids by default and you can only set drawables by there indexes and index is only obtained by their ids

so, here are some screencasts. My implementation is on the left:

overdrawing
overdraw

rounding
rounded_rorners

There's basically an issue with TransitionDrawable, background layer remains visible after transition, causing overdrawing.
And another issue with transition drawable is that by default transition drawable does not set drawable ids. So it has to be done manually, or RoundedImageView(anybody else) won't be able to change those drawables

Here's my implementation, you can examine it. Maybe we'll leave it as a gist, but I'd still recommend to put that stuff into doc. Would save a lot of times for devs. And might have a performance boost also

https://gist.github.com/AlexBlokh/c4294d355048c50cba137895be7a6a8d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants