Stop refetching on every size change by default

Currently we invoke Coil on every size change, which is correct but a lot of wasted work most of the time. We no longer re-fetch on every size change. If developers want to customize this, they can use the new `shouldRefetchOnSizeChange` lambda parameter to allow re-fetching as they see fit.
google · Jul 9, 2020 · 17635e3 · 17635e3
1 parent 50c4090
commit 17635e3
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 8 deletions.
diff --git a/coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/CoilTest.kt b/coil/src/androidTest/java/dev/chrisbanes/accompanist/coil/CoilTest.kt
@@ -48,6 +48,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.TestCoroutineDispatcher
 import kotlinx.coroutines.test.runBlockingTest
+import kotlinx.coroutines.withTimeoutOrNull
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -175,6 +176,39 @@ class CoilTest {
         loadCompleteSignal.close()
     }
 
+    @OptIn(ExperimentalCoroutinesApi::class)
+    @Test
+    fun basicLoad_changeSize() {
+        val loadCompleteSignal = Channel<Unit>(Channel.UNLIMITED)
+        val sizeFlow = MutableStateFlow(128.dp)
+
+        composeTestRule.setContent {
+            val size = sizeFlow.collectAsState()
+            CoilImage(
+                data = resourceUri(R.drawable.blue_rectangle),
+                modifier = Modifier.preferredSize(size.value).testTag(CoilTestTags.Image),
+                onRequestCompleted = { loadCompleteSignal.offer(Unit) }
+            )
+        }
+
+        // Await the first load
+        runBlocking {
+            loadCompleteSignal.receive()
+        }
+
+        // Now change the size
+        sizeFlow.value = 256.dp
+
+        // Await the potential second load (which shouldn't come)
+        runBlocking {
+            val result = withTimeoutOrNull(3000) { loadCompleteSignal.receive() }
+            assertThat(result).isNull()
+        }
+
+        // Close the signal channel
+        loadCompleteSignal.close()
+    }
+
     @Test
     @SdkSuppress(minSdkVersion = 26) // captureToBitmap is SDK 26+
     fun customGetPainter() {

diff --git a/coil/src/main/java/dev/chrisbanes/accompanist/coil/Coil.kt b/coil/src/main/java/dev/chrisbanes/accompanist/coil/Coil.kt
@@ -58,6 +58,8 @@ import coil.request.GetRequestBuilder
  * @param getFailurePainter Optional builder for the [Painter] to be used to draw the failure
  * loading result. Passing in `null` will result in falling back to the default [Painter].
  * @param loading Content to be displayed when the request is in progress.
+ * @param shouldRefetchOnSizeChange Lambda which will be invoked when the size changes, allowing
+ * optional re-fetching of the image. Return true to re-fetch the image.
  * @param onRequestCompleted Listener which will be called when the loading request has finished.
  */
 @Composable
@@ -70,6 +72,7 @@ fun CoilImage(
     getSuccessPainter: @Composable ((SuccessResult) -> Painter)? = null,
     getFailurePainter: @Composable ((ErrorResult) -> Painter?)? = null,
     loading: @Composable (() -> Unit)? = null,
+    shouldRefetchOnSizeChange: (currentResult: RequestResult, size: IntSize) -> Boolean = defaultRefetchOnSizeChangeLambda,
     onRequestCompleted: (RequestResult) -> Unit = emptySuccessLambda
 ) {
     CoilImage(
@@ -89,6 +92,7 @@ fun CoilImage(
         getSuccessPainter = getSuccessPainter,
         getFailurePainter = getFailurePainter,
         loading = loading,
+        shouldRefetchOnSizeChange = shouldRefetchOnSizeChange,
         modifier = modifier
     )
 }
@@ -110,6 +114,8 @@ fun CoilImage(
  * @param getFailurePainter Optional builder for the [Painter] to be used to draw the failure
  * loading result. Passing in `null` will result in falling back to the default [Painter].
  * @param loading Content to be displayed when the request is in progress.
+ * @param shouldRefetchOnSizeChange Lambda which will be invoked when the size changes, allowing
+ * optional re-fetching of the image. Return true to re-fetch the image.
  * @param onRequestCompleted Listener which will be called when the loading request has finished.
  */
 @Composable
@@ -122,9 +128,16 @@ fun CoilImage(
     getSuccessPainter: @Composable ((SuccessResult) -> Painter)? = null,
     getFailurePainter: @Composable ((ErrorResult) -> Painter?)? = null,
     loading: @Composable (() -> Unit)? = null,
+    shouldRefetchOnSizeChange: (currentResult: RequestResult, size: IntSize) -> Boolean = defaultRefetchOnSizeChangeLambda,
     onRequestCompleted: (RequestResult) -> Unit = emptySuccessLambda
 ) {
-    var result by stateFor<RequestResult?>(request.data) { null }
+    // GetRequest does not support object equality (as of Coil v0.10.1) so we can not key any
+    // remember calls 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 requestKey = request.data ?: request
+
+    var result by stateFor<RequestResult?>(requestKey) { null }
 
     // This may look a little weird, but allows the launchInComposition callback to always
     // invoke the last provided [onRequestCompleted].
@@ -139,11 +152,7 @@ fun CoilImage(
     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) }
+    val requestActor = remember(requestKey) { CoilRequestActor(request) }
 
     launchInComposition(requestActor) {
         // Launch the Actor
@@ -176,9 +185,14 @@ fun CoilImage(
         else -> null
     }
 
+    // Update the modifier, so that we listen for size changes to invoke the actor
     val mod = modifier.onSizeChanged { size ->
-        // When the size changes, send it to the request actor
-        requestActor.send(size)
+        val r = result
+        if (r == null || shouldRefetchOnSizeChange(r, size)) {
+            // If we don't have a result yet, or the size has changed and we should re-fetch,
+            // send it to the request actor
+            requestActor.send(size)
+        }
     }
 
     if (painter == null) {
@@ -295,6 +309,8 @@ internal fun defaultSuccessPainterGetter(result: SuccessResult): Painter {
 
 internal val emptySuccessLambda: (RequestResult) -> Unit = {}
 
+internal val defaultRefetchOnSizeChangeLambda: (RequestResult, IntSize) -> Boolean = { _, _ -> false }
+
 internal fun Drawable.toImageAsset(fallbackSize: IntSize = IntSize.Zero): ImageAsset {
     return toBitmap(
         width = if (intrinsicWidth > 0) intrinsicWidth else fallbackSize.width,

diff --git a/coil/src/main/java/dev/chrisbanes/accompanist/coil/Crossfade.kt b/coil/src/main/java/dev/chrisbanes/accompanist/coil/Crossfade.kt
@@ -67,6 +67,8 @@ private const val DefaultTransitionDuration = 1000
  * @param crossfadeDuration The duration of the crossfade animation in milliseconds.
  * @param getFailurePainter Optional builder for the [Painter] to be used to draw the failure
  * loading result. Passing in `null` will result in falling back to the default [Painter].
+ * @param shouldRefetchOnSizeChange Lambda which will be invoked when the size changes, allowing
+ * optional re-fetching of the image. Return true to re-fetch the image.
  * @param onRequestCompleted Listener which will be called when the loading request has finished.
  */
 @Composable
@@ -78,6 +80,7 @@ fun CoilImageWithCrossfade(
     crossfadeDuration: Int = DefaultTransitionDuration,
     getFailurePainter: @Composable ((ErrorResult) -> Painter?)? = null,
     loading: @Composable (() -> Unit)? = null,
+    shouldRefetchOnSizeChange: (currentResult: RequestResult, size: IntSize) -> Boolean = defaultRefetchOnSizeChangeLambda,
     onRequestCompleted: (RequestResult) -> Unit = emptySuccessLambda
 ) {
     CoilImage(
@@ -88,6 +91,7 @@ fun CoilImageWithCrossfade(
         getFailurePainter = getFailurePainter,
         loading = loading,
         modifier = modifier,
+        shouldRefetchOnSizeChange = shouldRefetchOnSizeChange,
         onRequestCompleted = onRequestCompleted
     )
 }
@@ -109,6 +113,8 @@ fun CoilImageWithCrossfade(
  * @param crossfadeDuration The duration of the crossfade animation in milliseconds.
  * @param getFailurePainter Optional builder for the [Painter] to be used to draw the failure
  * loading result. Passing in `null` will result in falling back to the default [Painter].
+ * @param shouldRefetchOnSizeChange Lambda which will be invoked when the size changes, allowing
+ * optional re-fetching of the image. Return true to re-fetch the image.
  * @param onRequestCompleted Listener which will be called when the loading request has finished.
  */
 @Composable
@@ -120,6 +126,7 @@ fun CoilImageWithCrossfade(
     crossfadeDuration: Int = DefaultTransitionDuration,
     getFailurePainter: @Composable ((ErrorResult) -> Painter?)? = null,
     loading: @Composable (() -> Unit)? = null,
+    shouldRefetchOnSizeChange: (currentResult: RequestResult, size: IntSize) -> Boolean = defaultRefetchOnSizeChangeLambda,
     onRequestCompleted: (RequestResult) -> Unit = emptySuccessLambda
 ) {
     CoilImage(
@@ -129,6 +136,7 @@ fun CoilImageWithCrossfade(
         getSuccessPainter = { crossfadePainter(it, durationMs = crossfadeDuration) },
         getFailurePainter = getFailurePainter,
         loading = loading,
+        shouldRefetchOnSizeChange = shouldRefetchOnSizeChange,
         modifier = modifier,
         onRequestCompleted = onRequestCompleted
     )