landscapist icon indicating copy to clipboard operation
landscapist copied to clipboard

Correct image height sizes for the use case of unspecified item height inside of LazyColumn

Open enxy0 opened this issue 1 year ago • 4 comments

Please complete the following information:

  • Library Version: v2.1.11
  • Affected Device(s): Not a device specific

Describe the Bug:

When using Landscapist (CoilImage) there is a significant decrease in performance compared to using just Coil (AsyncImage).

https://user-images.githubusercontent.com/34074019/232067275-068b7e04-3f2b-4813-912a-0f24e270a84f.mp4

Expected Behavior:

Should be the same as in Coil (AsynImage) - smooth scroll.

Project from video https://github.com/enxy0/CoilImage

enxy0 avatar Apr 14 '23 14:04 enxy0

This looks super interesting. It seems to have an issue with caching or requesting scaled image sizes (when the image size is so large) Let me look into the details when I have a moment. Thank you so much for reporting this issue!

skydoves avatar Apr 14 '23 15:04 skydoves

I took a bit here. It seems to work well by giving a key value to the LazyColumn to your sample app, but still digging to improve the performance here.

2023-05-03 00 32 19

skydoves avatar May 02 '23 15:05 skydoves

Alrighty, I found the problem here. In your sample project does not specify the height size of images in the LazyColumn, which means you’re loading 100 to 1000 images at once because of the unclear image size, and all items are loaded at the same time.

You can fix this issue very simply by giving the height size to your CoilImage like the one below.

      CoilImage(
        imageModel = { item.url },
        modifier = Modifier
          .fillMaxWidth()
          .height(300.dp)
      )

I’m not sure how many use cases you need to use, unspecified the height size of the entire item in LazyColumn or list, but let me look into the details when I have a moment. I will change the name of this issue properly.

skydoves avatar May 03 '23 05:05 skydoves

@enxy0 Additionally, if you don't want to specify the height size, you can set the range to the CoilImage at least like the below:

      CoilImage(
        imageModel = { item.url },
        modifier = Modifier
          .fillMaxWidth()
          .heightIn(min = 100.dp, max = 300.dp)
      )

To be honest, I don't want to see this as an issue because if you never specify the height size, the library must measure the proper size of your item's height and coordinate each item layout under the hood whether you use Glide, Coil, Fresco, or whatever, and yes, that's an expensive cost.

skydoves avatar May 03 '23 06:05 skydoves