Coil: Images inside recyclerview flash when notifyDataSetChanged() is called

[dafi]

I'm migrating my code from Picasso to Coil and discovered a strange behaviour non present with Picasso.

A recycler view contains images but when I call notifyDataSetChanged() on the adapter some images flash as shown in attached GIF

I created a project useful to reproduce the bug, CoilFlashing

Version
Coil 0.9.4
Android 9 and 10 under Android emulator, Xiaomi MiA1, Pixel 3XL

[colinrtwhite]

Thanks for the report + sample project. I just added a fix here: https://github.com/coil-kt/coil/pull/283.

[dafi]

0.12.0 contains a regression and the bug popped up again

The demo project to reproduce the issue is still valid, you can download and use it, with version 0.11.0 everything works fine

[colinrtwhite]

This is likely intended behaviour in 0.12.0 as the memory cache no longer resolves values synchronously on the main thread. This change was made for several reasons:

• Resolving synchronously on the main thread didn't work unexpectedly in some edge cases. For instance, if the request has a Transformation the request likely won't check the memory cache synchronously (since it needs the view to be measured).
• It requires running Mappers on the main thread, which can also be unexpected (since they're part of the image pipeline) and is slower on the main thread.
• Building the memory cache key on the main thread can cause a strict mode violation for Files.
• Interceptors can't run on the main thread without likely causing performance issues.

Both Glide and Picasso also check the memory cache off of the main thread.

[mattlogan]

Hey Colin - I understand your reasoning above for checking the cache asynchronously, but I noticed that Picasso doesn't have this issue despite also checking the memory cache off the main thread. I'm not sure I understand why checking the memory cache asynchronously necessitates clearing & reloading the image as it appears to be doing now. Perhaps you could wait to clear the image until the memory cache check completes?

It would be awesome if you have an idea for another solution here, otherwise I think this will lead to a slightly degraded UX for situations like the one described above. Thanks for the awesome library!

[colinrtwhite]

@mattlogan I agree this behaviour isn't best for UX in certain cases. Also after reviewing Picasso's code, it looks like they do check the memory cache on the main thread. They're also able to avoid waiting for the request's size as their Transformation interface does not include a size param (which limits what you can do with a transformation).

Coil clears the drawable in onClear to support bitmap pooling and also to preemptively clear images attached to detached Fragments and detached views in a RecyclerView's view pool. We could avoid calling onClear if bitmap pooling is disabled, however we still clear the drawable in onStart at the beginning of the next request.

Unfortunately there are two competing use cases with Interceptor support and maximizing UI thread performance on one hand and synchronous memory cache checks on the other. That said, I think it's worthwhile (and possible) to support both use cases with a boolean flag when constructing your image loader - something like ImageLoader.Builder.allowSynchronousMemoryCacheChecks(true/false). allowSynchronousMemoryCacheChecks would need to be disabled to add an Interceptor. What do you think? I'm going to reopen this issue to track.