Android: Rewrite image loading with Kotlin and Coil

[t895]

When working on implementing the GameTDB fallbacks for PAL covers through glide, our code was getting bloated with boilerplate. Additionally, since we're starting to migrate to Kotlin, I thought this would be a good time to get all of the image loading code rewritten. Glide starts to feel outdated next to other Kotlin-native code so Coil was a natural choice. It is written for Kotlin first and we save a big volume of code taking this route. So now all image caching and fallbacks are handled gracefully by Coil. I've also taken the time to rewrite all of the files that work with image loading in Kotlin so we could take full advantage of coroutines and completely remove the messy set of Executors and Loopers from before. Everything responds nicely to the activity's lifecycle and we maintain the same performance.

Note - This also fixes behavior with the grid options where previously if you disabled downloading covers, the covers would just stay there. Now they are disabled outright and unless you have a custom cover, the no_cover image is displayed.

[JosJuice]

Would it be possible to first have a commit that translates to Kotlin and then a commit that actually makes the changes to how we load images? The PR would be a lot easier to review if so. Or would the changes be difficult to split retroactively?

[t895]

Would it be possible to first have a commit that translates to Kotlin and then a commit that actually makes the changes to how we load images? The PR would be a lot easier to review if so. Or would the changes be difficult to split retroactively?

It would be difficult retroactively. Using Coil depends on using Kotlin in all of the classes that use it. So if I were to go back I'd have to rewrite it without coroutines and then with them.

At least to explain things better, instead of having GlideUtils that manages the threads and call each load like this
GlideUtils.loadGameCover(holder, holder.binding.imageGameScreen, gameFile, mActivity);

We now bind the image loading to the lifecycle of the activity that spawns it. This means that when the activity is destroyed, all loading within this block is cancelled safely. The first portion of the block starts a load on the system-provided IO thread to find the custom cover and once that work is finished, we pass the result into the Coil load that works very similarly to how the Glide one would go.
mActivity.lifecycleScope.launchWhenStarted {
withContext(Dispatchers.IO) {
val customCoverUri = CoilUtils.findCustomCover(gameFile)
withContext(Dispatchers.Main) {
CoilUtils.loadGameCover(
holder,
holder.binding.imageGameScreen,
gameFile,
customCoverUri
)
}
}
}

[JosJuice]

It would be difficult retroactively. Using Coil depends on using Kotlin in all of the classes that use it. So if I were to go back I'd have to rewrite it without coroutines and then with them.

I'm not asking for first a commit that makes us use Coil and then a commit that makes us use Kotlin, I'm asking for first a commit that makes us use Kotlin (while keeping Glide) and then a commit that makes us use Coil.

[t895]

It would be difficult retroactively. Using Coil depends on using Kotlin in all of the classes that use it. So if I were to go back I'd have to rewrite it without coroutines and then with them.

I'm not asking for first a commit that makes us use Coil and then a commit that makes us use Kotlin, I'm asking for first a commit that makes us use Kotlin (while keeping Glide) and then a commit that makes us use Coil.

Let me see what I can do

[JosJuice]

How is the caching handled now? I couldn't see any explicit mention of it.

[t895]

How is the caching handled now? I couldn't see any explicit mention of it.

It's handled internally by Coil. They maintain a memory and disk cache. It's customizable if we need later down the line too.

[JosJuice]

It's handled internally by Coil. They maintain a memory and disk cache. It's customizable if we need later down the line too.

Does this mean that the refetching problem is back?

[t895]

It's handled internally by Coil. They maintain a memory and disk cache. It's customizable if we need later down the line too.

Does this mean that the refetching problem is back?

It is, but we talked about how preventing this issue isn't really reasonable to implement in IRC. Unless I misunderstood, I thought it was more reasonable that we deal with extra network requests.

Right now, if someone has a game that doesn't have the right cover for their language in PAL, it will default to the generic region and download that cover. But at least on subsequent tries, it will have one unnecessary network request and then load the cached fallback image.

[JosJuice]

It is, but we talked about how preventing this issue isn't really reasonable to implement in IRC. Unless I misunderstood, I thought it was more reasonable that we deal with extra network requests.

I don't recall this.

Right now, if someone has a game that doesn't have the right cover for their language in PAL, it will default to the generic region and download that cover. But at least on subsequent tries, it will have one unnecessary network request and then load the cached fallback image.

Yes, and that means that every time you open Dolphin from then on, Dolphin will do as many unnecessary network requests as you have affected games. I don't think that's reasonable.

[JosJuice]

For reference for others reading this: We agreed in PMs that dropping support for fallbacks would be fine for now.