Move away from using WithConstraints

buildSrc/src/main/java/dev/chrisbanes/accompanist/buildsrc/dependencies.kt

@@ -73,7 +73,7 @@ object Libs {
         const val coreKtx = "androidx.core:core-ktx:1.2.0"
     }
 
-    const val coil = "io.coil-kt:coil:0.10.1"
+    const val coil = "io.coil-kt:coil:0.11.0"
 
     const val mdc = "com.google.android.material:material:1.1.0"
 

coil/src/main/java/dev/chrisbanes/accompanist/coil/Coil.kt

@@ -19,41 +19,27 @@ package dev.chrisbanes.accompanist.coil
 import android.graphics.drawable.Drawable
 import androidx.compose.Composable
 import androidx.compose.getValue
-import androidx.compose.onCommit
+import androidx.compose.launchInComposition
 import androidx.compose.remember
 import androidx.compose.setValue
-import androidx.compose.stateFor
+import androidx.compose.state
 import androidx.core.graphics.drawable.toBitmap
 import androidx.ui.core.Alignment
-import androidx.ui.core.Constraints
 import androidx.ui.core.ContentScale
 import androidx.ui.core.ContextAmbient
 import androidx.ui.core.Modifier
-import androidx.ui.core.WithConstraints
-import androidx.ui.core.hasBoundedHeight
-import androidx.ui.core.hasBoundedWidth
-import androidx.ui.core.hasFixedHeight
-import androidx.ui.core.hasFixedWidth
 import androidx.ui.foundation.Box
 import androidx.ui.foundation.Image
 import androidx.ui.graphics.ColorFilter
 import androidx.ui.graphics.ImageAsset
 import androidx.ui.graphics.asImageAsset
 import androidx.ui.graphics.painter.ImagePainter
 import androidx.ui.graphics.painter.Painter
+import androidx.ui.unit.IntPxSize
 import coil.Coil
 import coil.decode.DataSource
 import coil.request.GetRequest
 import coil.request.GetRequestBuilder
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.launch
-
-private val Constraints.requestWidth
-    get() = if (hasFixedWidth || hasBoundedWidth) maxWidth else minWidth
-
-private val Constraints.requestHeight
-    get() = if (hasFixedHeight || hasBoundedHeight) maxHeight else minHeight
 
 /**
  * Creates a composable that will attempt to load the given [data] using [Coil], and then
@@ -91,7 +77,9 @@ fun CoilImage(
             // pass the request through
             is GetRequest -> data
             // Otherwise we construct a GetRequest using the data parameter
-            else -> GetRequest.Builder(ContextAmbient.current).data(data).build()
+            else -> remember(data) {
+                GetRequest.Builder(ContextAmbient.current).data(data).build()
+            }
         },
         alignment = alignment,
         contentScale = contentScale,
@@ -105,7 +93,7 @@ fun CoilImage(
 }
 
 /**
- * Creates a composable that will attempt to load the given [data] using [Coil], and then
+ * Creates a composable that will attempt to load the given [request] using [Coil], and then
  * display the result in an [Image].
  *
  * @param request The request to execute. If the request does not have a [GetRequest.sizeResolver]
@@ -135,65 +123,106 @@ fun CoilImage(
     loading: @Composable (() -> Unit)? = null,
     onRequestCompleted: (RequestResult) -> Unit = emptySuccessLambda
 ) {
-    WithConstraints(modifier) {
-        val requestWidth = constraints.requestWidth.value
-        val requestHeight = constraints.requestHeight.value
+    var result by state<RequestResult?> { null }
 
-        // Execute the request using executeAsComposable(), which guards the actual execution
-        // so that the request is only run if the request changes.
-        val result = when {
-            request.sizeResolver != null -> {
-                // If the request has a sizeResolver set, we just execute the request as-is
-                request.executeAsComposable()
-            }
-            requestWidth > 0 && requestHeight > 0 -> {
-                // If we have a non-zero size, we can modify the request to include the size
-                request.newBuilder()
-                    .size(requestWidth, requestHeight)
-                    .build()
-                    .executeAsComposable()
-            }
-            else -> {
-                // Otherwise we have a zero size, so no point executing a request
-                null
+    // This may look a little weird, but allows the launchInComposition callback to always
+    // invoke the last provided [onRequestCompleted].
+    //
+    // If a composition happens *after* launchInComposition has launched, the given
+    // [onRequestCompleted] might have changed. If the actor lambda below directly referenced
+    // [onRequestCompleted] it would have captured access to the initial onRequestCompleted
+    // value, not the latest.
+    //
+    // This `callback` state enables the actor lambda to only capture the remembered state
+    // reference, which we can update on each composition.
+    val callback = state { onRequestCompleted }
+    callback.value = onRequestCompleted
+
+    // GetRequest does not support object equality (as of Coil v0.10.1) so we can not key the
+    // remember() using the request itself. For now we just use the [data] field, but
+    // ideally this should use [request] to track changes in size, transformations, etc too.
+    // See: https://github.com/coil-kt/coil/issues/405
+    val requestActor = remember(request.data) { CoilRequestActor(request) }
+
+    launchInComposition(requestActor) {
+        // Launch the Actor
+        requestActor.run { _, actorResult ->
+            // Update the result state
+            result = actorResult
+
+            if (actorResult != null) {
+                // Execute the onRequestCompleted callback if we have a new result
+                callback.value(actorResult)
             }
         }
+    }
 
-        onCommit(result) {
-            if (result != null) {
-                onRequestCompleted(result)
+    val painter = when (val r = result) {
+        is SuccessResult -> {
+            if (getSuccessPainter != null) {
+                getSuccessPainter(r)
+            } else {
+                defaultSuccessPainterGetter(r)
             }
         }
-
-        val painter = when (result) {
-            is SuccessResult -> {
-                if (getSuccessPainter != null) {
-                    getSuccessPainter(result)
-                } else {
-                    defaultSuccessPainterGetter(result)
-                }
+        is ErrorResult -> {
+            if (getFailurePainter != null) {
+                getFailurePainter(r)
+            } else {
+                defaultFailurePainterGetter(r)
             }
-            is ErrorResult -> {
-                if (getFailurePainter != null) {
-                    getFailurePainter(result)
-                } else {
-                    defaultFailurePainterGetter(result)
-                }
+        }
+        else -> null
+    }
+
+    val mod = modifier.onSizeChanged { size ->
+        // When the size changes, send it to the request actor
+        requestActor.send(size)
+    }
+
+    if (painter == null) {
+        // If we don't have a result painter, we add a Box with our modifier
+        Box(mod) {
+            // If we don't have a result yet, we can show the loading content
+            // (if not null)
+            if (result == null && loading != null) {
+                loading()
             }
-            else -> null
         }
+    } else {
+        Image(
+            painter = painter,
+            contentScale = contentScale,
+            alignment = alignment,
+            colorFilter = colorFilter,
+            modifier = mod
+        )
+    }
+}
 
-        if (result == null && loading != null) {
-            Box(modifier, children = loading)
-        } else if (painter != null) {
-            Image(
-                painter = painter,
-                contentScale = contentScale,
-                alignment = alignment,
-                colorFilter = colorFilter,
-                modifier = modifier
-            )
+private fun CoilRequestActor(
+    request: GetRequest
+) = RequestActor<IntPxSize, RequestResult?> { size ->
+    when {
+        request.sizeResolver != null -> {
+            // If the request has a sizeResolver set, we just execute the request as-is
+            request
+        }
+        size != IntPxSize.Zero -> {
+            // If we have a non-zero size, we can modify the request to include the size
+            request.newBuilder()
+                .size(size.width.value, size.height.value)
+                .build()
         }
+        else -> {
+            // Otherwise we have a zero size, so no point executing a request
+            null
+        }
+    }?.let { transformedRequest ->
+        // Now execute the request in Coil...
+        Coil.imageLoader(transformedRequest.context)
+            .execute(transformedRequest)
+            .toResult()
     }
 }
 
@@ -236,41 +265,11 @@ data class ErrorResult(
     )
 }
 
-/**
- * This will execute the [GetRequest] within a composable, ensuring that the request is only
- * execute once and storing the result, and cancelling requests as required.
- *
- * @return the result from the request execution, or `null` if the request has not finished yet.
- */
-@Composable
-fun GetRequest.executeAsComposable(): RequestResult? {
-    // GetRequest does not support object equality (as of v0.10.1) so we can not key off the
-    // request itself. For now we can just use the `data` parameter, but ideally this should use
-    // `this` to track changes in size, transformations, etc too.
-    // See https://github.com/coil-kt/coil/issues/405
-    val key = data
-
-    var result by stateFor<RequestResult?>(key) { null }
-
-    // Launch and execute a new request when it changes
-    onCommit(key) {
-        val job = CoroutineScope(Dispatchers.Main).launch {
-            // Start loading the image and await the result
-            result = Coil.imageLoader(context).execute(this@executeAsComposable).let {
-                // We map to our internal result entities
-                when (it) {
-                    is coil.request.SuccessResult -> SuccessResult(it)
-                    is coil.request.ErrorResult -> ErrorResult(it)
-                }
-            }
-        }
-
-        // Cancel the request if the input to onCommit changes or
-        // the Composition is removed from the composition tree.
-        onDispose { job.cancel() }
+private fun coil.request.RequestResult.toResult(): RequestResult {
+    return when (this) {
+        is coil.request.SuccessResult -> SuccessResult(this)
+        is coil.request.ErrorResult -> ErrorResult(this)
     }
-
-    return result
 }
 
 @Composable

coil/src/main/java/dev/chrisbanes/accompanist/coil/Modifier.kt

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package dev.chrisbanes.accompanist.coil
+
+import androidx.compose.getValue
+import androidx.compose.setValue
+import androidx.compose.state
+import androidx.ui.core.LayoutCoordinates
+import androidx.ui.core.Modifier
+import androidx.ui.core.OnPositionedModifier
+import androidx.ui.core.composed
+import androidx.ui.unit.IntPxSize
+
+/**
+ * [Modifier] which will invoke [onSizeChanged] whenever the size of the element changes. This
+ * will be called after positioning, similar to `Modifier.onPositioned`.
+ */
+internal fun Modifier.onSizeChanged(
+    onSizeChanged: (IntPxSize) -> Unit
+) = composed {
+    var lastSize by state<IntPxSize?> { null }
+
+    object : OnPositionedModifier {
+        override fun onPositioned(coordinates: LayoutCoordinates) {
+            if (coordinates.size != lastSize) {
+                lastSize = coordinates.size
+                onSizeChanged(coordinates.size)
+            }
+        }
+    }
+}