[FR] Expand return type on imperative APIs

[chrisbanes]

Currently all of the imperative APIs return a Drawable, which is great for drawing but it would be handy to know more information about the load itself. Things like: where did the image come from? (cache? network?)

I imagine something like the transition API:

enum class Source { DISK, MEMORY_CACHE, NETWORK }

sealed class LoadResult {
    class Success(
        val drawable: Drawable,
        val source: Source
    ) : LoadResult()

    class Error(
        val drawable: Drawable?
    ) : LoadResult()
}
    
fun Coil.getResult(data: Any): LoadResult

This would be super useful for when Coil is being used in outside of View environments 🚀

WDYT?

[colinrtwhite]

Oooh Coil in Compose 🤩.

Definitely in my radar to support Compose. Have mostly been waiting for Coroutines to work with the new Kotlin compiler and also to see how they plan to support animated images. This will probably change, but my current plan is to have a coil-compose extension module with two main functions:

• fun image(loader: ImageLoader, request: GetRequest) = effectOf<Image?>, which is the ImageLoader.get equivalent that exposes the raw Image to the caller.
• @Composable fun LoadImage(loader: ImageLoader, request: LoadRequest), which auto-resolves the target size for the image and supports bitmap pooling since it has a well-defined lifecycle destroy event via CommitScope.onDispose.

^ If you had something like that would you still need the Source from Coil.get? You can also get the source by adding a listener to your Coil.get request.

Currently, I like the simplicity of Coil.get returning the result Drawable. Also it's similar to the imperative APIs of Glide, Picasso, etc.

[chrisbanes]

Just a quick catch up: effects are no longer, you just use normal @Composable functions.

@Composable fun LoadImage(loader: ImageLoader, request: LoadRequest), which auto-resolves the target size for the image and supports bitmap pooling since it has a well-defined lifecycle destroy event via CommitScope.onDispose.

I was actually going to tackle target size next. I'll see if I can get pooling in there too. I'm happy to contribute this to Coil eventually, or just leave it in Tivi as a proof of concept.

Currently, I like the simplicity of Coil.get returning the result Drawable. Also it's similar to the imperative APIs of Glide, Picasso, etc.

Sure, totally agree. Are you opposed to adding an 'advanced' overload supports more information?

[colinrtwhite]

Just a quick catch up: effects are no longer, you just use normal @composable functions.

Ah TIL, I guess we only need LoadImage.

I was actually going to tackle target size next. I'll see if I can get pooling in there too. I'm happy to contribute this to Coil eventually, or just leave it in Tivi as a proof of concept.

👍 If you want to contribute the coil-compose module that'd be awesome! Here's what I was thinking for the integration, but feel free to deviate:

• For target sizing, I think you might be able to create a custom SizeResolver and suspend until the first pass of @Composable Draw is called with parentSize.

• For bitmap pooling, it might require some work in the coil-base module. Currently, Coil listens for a Lifecycle.onDestroy event and/or a View.onDetach event if the target is a ViewTarget. You might need to add something like a DetachableTarget:

interface DetachableTarget : Target {
    val detachable: Detachable
}

interface Detachable {

    // This would call `onDispose(callback)` inside `LoadImage`.
    fun addOnDetachListener(callback: () -> Unit))
}

This way coil-base can set an onDispose callback without depending on any of the "androidx.ui:ui-* libraries explicitly.

Sure, totally agree. Are you opposed to adding an 'advanced' overload supports more information?

I think so. What's the use case for getting the DataSource from Coil.get?

[chrisbanes]

For target sizing, I think you might be able to create a custom SizeResolver and suspend until the first pass of @composable Draw is called with parentSize.

It's probably simpler than that. See here for a quick impl.

What's the use case for getting the DataSource from Coil.get

Basically replicating what you did for Transition but for things which don't use Drawables

[colinrtwhite]

It's probably simpler than that. See here for a quick impl.

Oh neat I didn't know about OnChildPositioned.

Basically replicating what you did for Transition but for things which don't use Drawables

Ah I see what you mean. My preference would be to integrate Coil's Transition with the Compose Transition class if that's possible. However, that doesn't look likely at the moment... Going to play around with Compose more to improve my understanding.

[colinrtwhite]

@chrisbanes Ended up changing my mind on this 🙂. Implemented here: #349 and will be part of the next release.