bumptech / glide

An image loading and caching library for Android focused on smooth scrolling
https://bumptech.github.io/glide/
Other
34.59k stars 6.12k forks source link

Correct image layout parameters with 4.7.1 #3046

Closed swftvsn closed 6 years ago

swftvsn commented 6 years ago

Glide Version:

implementation 'com.github.bumptech.glide:glide:4.7.1'
annotationProcessor 'com.github.bumptech.glide:compiler:4.7.1'

Integration libraries:

Device/Android Version: All

Issue details / Repro steps / Use case background: So I have my recycler view that only displays images that should be as wide as the display is and the height should be whatever the image height happens to be proportionally. I use this in my RecyclerView.Adapter:

AppCompatImageView imageView = new AppCompatImageView(parent.getContext());
RecyclerView.LayoutParams params = new RecyclerView.LayoutParams(
    ViewGroup.LayoutParams.MATCH_PARENT, 
    ViewGroup.LayoutParams.WRAP_CONTENT
);
imageView.setLayoutParams(params);
imageView.setScaleType(ImageView.ScaleType.FIT_CENTER);
imageView.setAdjustViewBounds(true);
return new DetailViewHolder(imageView, false);

It functions correctly, but displays this warning / error message in console: I/ViewTarget: Glide treats LayoutParams.WRAP_CONTENT as a request for an image the size of this device's screen dimensions. If you want to load the original image and are ok with the corresponding memory cost and OOMs (depending on the input size), use .override(Target.SIZE_ORIGINAL). Otherwise, use LayoutParams.MATCH_PARENT, set layout_width and layout_height to fixed dimension, or use .override() with fixed dimensions.

The question is, how to properly use the layout parameters in this case to enable optimal performance?

sjudd commented 6 years ago

The last sentences in the warning messages provide some suggestions:

If you want to load the original image and are ok with the corresponding memory cost and OOMs (depending on the input size), use .override(Target.SIZE_ORIGINAL). Otherwise, use LayoutParams.MATCH_PARENT, set layout_width and layout_height to fixed dimension, or use .override() with fixed dimensions.

It really depends on what you want your UI to look like. If images are loading the way you expect and you're not seeing performance problems you can ignore the log message. It's there because frequently these layout parameters produce unexpected results and we wanted to give developers some hint as to why that might be happening.

In this case Target.SIZE_ORIGINAL might work the way you expect, but again I wouldn't worry too much about changing it if you're seeing what you expect to see.

swftvsn commented 6 years ago

I'd like to have recycler view that only displays images that should be as wide as the display is and the height should be whatever it needs to be to maintain aspect ratio.

The performance is not what I'd like it to be, but I'm a bit at loss how to approach this. As said before, the image should be as wide as parent, but I don't know what to set to height? I'd like Glide to cache the images at that size and use those later. Does this warning indicate, that cache is not used, and the whole image is always loaded?

sjudd commented 6 years ago

What it means is that Glide is using (width_of_parent, height_of_screen) as the bounding dimensions for your image. height_of_screen is a guess on Glide's part because it's not really sure what you meant by wrap_content. I think in this case it ends up being what you want or roughly want you want. You could compare the loaded image size to the size you expect on a particular device to confirm.

As long as the parameters are the same and the width of the parent doesn't change, caching will work normally. You'll probably end up using different cached images in portrait vs landscape mode though (unless your device is square :)).

The .thumbnail() API might be able to help if you just think that images are loading slowly. You could pick a fixed lower resolution size and use .override() with that particular size for all images when loading the thumbnail so that the cache key is consistent and the images are smaller and therefore faster to load. If you're relying on the image size to determine the size of your view, you'd need to find a way to set a fixed width/height based on the image size in the view for this to work without a bunch of jank.

If you have a jank problem I'd recommend starting with the profiler in AS. Systrace can also help point out what parts of each frame are particularly slow.

swftvsn commented 6 years ago

Thank you @sjudd for the detailed answer and for your time! I'll profile the view that is problematic and hope that it's my code (as that's easiest to optimize 😀). If it turns out to be Glide and I need help understanding something I'll open a new ticket.

Thanks for the awesome library btw 😉!