coil-kt / coil

Image loading for Android and Compose Multiplatform.
https://coil-kt.github.io/coil/
Apache License 2.0
10.74k stars 653 forks source link

Image flickers when url changes for rememberImagePainter #928

Closed cj1098 closed 2 years ago

cj1098 commented 3 years ago

Currently I'm using a library to reorder items in a LazyColumn. The library works and functions as expected. The issue I'm running into is that when an item is reordered in the list, it causes a recomposition because my state has changed since the position of the item has changed. This is fine, but when items in my list recompose, they might have different urls so it tries to load the new image for both items that had their positions change. This causes a flicker as it tries to load new images. I've tried to use caching to prevent rememberImagePainter from having to go fetch the image again but I don't think that's how that works. I've tried to prevent the request from going through by providing an ExecuteCallback that returned false if the state wasn't empty and that didn't work either.

I would expect to be able to configure coil to not re-load an image when a url changes but instead use a cached version if it exists.

I'm using version 1.3.2 and it's not device specific.

colinrtwhite commented 3 years ago

Do you have a project or code sample I can use to reproduce this? Coil should re-use images cached in-memory or on-disk automatically.

cj1098 commented 3 years ago

I'll try and put something together tomorrow! Thanks for the quick reply

cj1098 commented 3 years ago

I'm using this library https://github.com/aclassen/ComposeReorderable/ and my setup looks like this:

LazyColumn(
    state = reorderState.listState,
    modifier = Modifier.reorderable(
    state = reorderState,
    onMove = uiActions::swapItemPositions
)
) {
    itemsIndexed(uiState.items) { idx, item ->
        ReorderableListItem(
            modifier = Modifier
                             .draggedItem(reorderState.offsetByIndex(idx))
                             .detectReorderAfterLongPress(reorderState),
                             item
                        )
      }
}

Inside that ReorderableListItem is

Image(painter = rememberImagePainter(data = uiState.url), contentDescription = null, modifier = Modifier
            .padding(top = 8.dp, start = 8.dp)
            .size(40.dp)
            .constrainAs(logo) {
        start.linkTo(parent.start)
       top.linkTo(parent.top)
})

And inside my viewModel this swapItemPositions function does the switching of the positions and then updates the uiState for my top level composable to recompose:

val swapList = uiModel.items.toMutableList()
Collections.swap(swapList, from, to)
// update the uiState here by passing in the new SwapList as the list for the LazyColumn.
colinrtwhite commented 3 years ago

@cj1098 Do you have a sample project that reproduces the issue? I can't reproduce the issue based on the code snippets - it may be related to a detail of your project.

cj1098 commented 3 years ago

Sure! I'll setup a sample project for this and post it sometime this weekend for you guys next week!

cj1098 commented 3 years ago

@colinrtwhite In setting up the external project I did sort of solve the issue... although I'm not sure what's causing it entirely. Currently we have a Flow setup for our uiStates that we update on the viewmodel and observe on our top level composable. Updating our uiState when item positions changed during drag and drop was causing the flickering. If I keep the list of items in the viewmodel and do operations on the list without causing recomposition every time it does work. Although it's nice to have a solution It's frustrating not knowing why. I can still post the sample project if you'd like to take a look?

colinrtwhite commented 3 years ago

Sure, if the sample reproduces the issue I can take a look.

cj1098 commented 3 years ago

It seems like it's 29 mb so just over the limit. Is there an email address or somewhere else I can send it to you?

colinrtwhite commented 3 years ago

Can you post it as a public repo on Github?

cj1098 commented 3 years ago

@colinrtwhite here you go! https://github.com/cj1098/sampleDragnDrop

colinrtwhite commented 2 years ago

@cj1098 This is fixed in 2.0.0-rc01.

cj1098 commented 2 years ago

@colinrtwhite Thank you for looking at this and fixing it! Someone on our team upgraded and I believe it fixed it. I am curious if you could share what the actual fix was? Looked through the changelog but couldn't discern what would've fixed it. Disk caching done by Coil instead of okhttp?

Swirastlynn commented 2 years ago

I can still observe flickering on 2.1.0. It doesn't always happen, but quite often. I haven't set any caching, all setup is default.

Code:

 val imageRatio = 9f / 16f

 LazyColumn(
            modifier = Modifier
                    .fillMaxSize()
                    .background(AppTheme.colors.itemBackground)
    ) {
        item {
            val screenWidth = LocalConfiguration.current.screenWidthDp.dp
            AsyncImage(
                    model =  detailsInfo?.landscapeImageUrl ?: "", // this change between recompositions is problematic
                    placeholder = ColorPainter(colorResource(id = R.color.details_header_bg)),
                    contentDescription = detailsInfo?.title ?: "placeholder",
                    modifier = Modifier.size(screenWidth, screenWidth * imageRatio)
            )
        }
   ...
   }
iamutkarshtiwari commented 2 years ago

Facing the same issue as mentioned by @Swirastlynn with 2.1.0 even when disk and memory caching is applied. I happen to be using Coil in LazyRow.

volkansahin45 commented 1 year ago

I feel like your problem might be you are not using key. https://developer.android.com/jetpack/compose/lists#item-keys