Lazily allocate drawSizeFlow in AsyncImagePainter. (#2810)

* Lazily allocate drawSizeFlow in AsyncImagePainter. * Fix function names. * Don't run instrumentation. * Fix tests. * Try fix. * Give up on Compose multiplatform test. * Fix tests. * Try fix. * Add idling resource.
coil-kt · Jan 26, 2025 · 009abba · 009abba
1 parent 27b5482
commit 009abba
Show file tree

Hide file tree

Showing 6 changed files with 127 additions and 24 deletions.
diff --git a/coil-compose-core/build.gradle.kts b/coil-compose-core/build.gradle.kts
@@ -12,7 +12,12 @@ plugins {
 }
 
 addAllMultiplatformTargets(libs.versions.skiko)
-androidLibrary(name = "coil3.compose.core")
+androidLibrary(name = "coil3.compose.core") {
+    dependencies {
+        // https://www.jetbrains.com/help/kotlin-multiplatform-dev/compose-test.html
+        debugImplementation(libs.androidx.compose.ui.test.manifest)
+    }
+}
 
 kotlin {
     sourceSets {

diff --git a/...ompose-core/src/androidInstrumentedTest/kotlin/coil3/compose/DrawScopeSizeResolverTest.kt b/...ompose-core/src/androidInstrumentedTest/kotlin/coil3/compose/DrawScopeSizeResolverTest.kt
@@ -0,0 +1,82 @@
+package coil3.compose
+
+import androidx.compose.foundation.Image
+import androidx.compose.foundation.layout.size
+import androidx.compose.runtime.remember
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.geometry.Size
+import androidx.compose.ui.platform.LocalDensity
+import androidx.compose.ui.test.ExperimentalTestApi
+import androidx.compose.ui.test.runComposeUiTest
+import androidx.compose.ui.unit.dp
+import coil3.ColorImage
+import coil3.ImageLoader
+import coil3.request.ImageRequest
+import coil3.request.SuccessResult
+import coil3.size.Dimension
+import coil3.size.Size as CoilSize
+import kotlin.coroutines.EmptyCoroutineContext
+import kotlin.math.roundToInt
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
+
+@OptIn(ExperimentalTestApi::class)
+class DrawScopeSizeResolverTest {
+    private val idlingResource = ImageLoaderIdlingResource()
+
+    @Test
+    fun imageIsLoadedWithDrawScopeSize() = runComposeUiTest {
+        registerIdlingResource(idlingResource)
+
+        val expectedSizeDp = 100.dp
+        var expectedSizePx = Size.Unspecified
+        var actualSizePx = Size.Unspecified
+
+        setContent {
+            expectedSizePx = with(LocalDensity.current) {
+                expectedSizeDp.toPx().roundToInt().toFloat().let { Size(it, it) }
+            }
+
+            val context = LocalPlatformContext.current
+            val imageLoader = remember(context) {
+                ImageLoader.Builder(context)
+                    .components {
+                        add { chain ->
+                            actualSizePx = chain.size.toComposeSize()
+                            SuccessResult(
+                                image = ColorImage(),
+                                request = chain.request,
+                            )
+                        }
+                    }
+                    .coroutineContext(EmptyCoroutineContext)
+                    .eventListener(idlingResource)
+                    .build()
+            }
+            val request = remember(context) {
+                ImageRequest.Builder(context)
+                    .data(Unit)
+                    .size(DrawScopeSizeResolver())
+                    .build()
+            }
+            val painter = rememberAsyncImagePainter(request, imageLoader)
+
+            Image(
+                painter = painter,
+                contentDescription = null,
+                modifier = Modifier.size(expectedSizeDp),
+            )
+        }
+
+        waitForIdle()
+
+        assertNotEquals(Size.Unspecified, actualSizePx)
+        assertEquals(expectedSizePx, actualSizePx)
+    }
+
+    private fun CoilSize.toComposeSize() = Size(
+        width = (width as Dimension.Pixels).px.toFloat(),
+        height = (height as Dimension.Pixels).px.toFloat(),
+    )
+}
diff --git a/coil-compose-core/src/commonMain/kotlin/coil3/compose/AsyncImagePainter.kt b/coil-compose-core/src/commonMain/kotlin/coil3/compose/AsyncImagePainter.kt
@@ -44,6 +44,7 @@ import kotlin.coroutines.EmptyCoroutineContext
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.channels.BufferOverflow.DROP_OLDEST
+import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
@@ -148,27 +149,32 @@ private fun rememberAsyncImagePainter(
 class AsyncImagePainter internal constructor(
     input: Input,
 ) : Painter(), RememberObserver {
-    private val drawSize = MutableSharedFlow<Size>(
-        replay = 1,
-        onBufferOverflow = DROP_OLDEST,
-    )
     private var painter: Painter? by mutableStateOf(null)
     private var alpha: Float = DefaultAlpha
     private var colorFilter: ColorFilter? = null
 
+    private var isRemembered = false
     private var rememberJob: Job? = null
         set(value) {
             field?.cancel()
             field = value
         }
 
+    private var drawSizeFlow: MutableSharedFlow<Size>? = null
+    private var drawSize = Size.Unspecified
+        set(value) {
+            if (field != value) {
+                field = value
+                drawSizeFlow?.tryEmit(value)
+            }
+        }
+
     internal lateinit var scope: CoroutineScope
     internal var transform = DefaultTransform
     internal var onState: ((State) -> Unit)? = null
     internal var contentScale = ContentScale.Fit
     internal var filterQuality = DefaultFilterQuality
     internal var previewHandler: AsyncImagePreviewHandler? = null
-    private var isRemembered = false
 
     internal var _input: Input? = input
         set(value) {
@@ -181,22 +187,17 @@ class AsyncImagePainter internal constructor(
             }
         }
 
-    /** The latest [AsyncImagePainter.Input]. */
     private val inputFlow: MutableStateFlow<Input> = MutableStateFlow(input)
     val input: StateFlow<Input> = inputFlow.asStateFlow()
 
-    /** The latest [AsyncImagePainter.State]. */
     private val _state: MutableStateFlow<State> = MutableStateFlow(State.Empty)
     val state: StateFlow<State> = _state.asStateFlow()
 
     override val intrinsicSize: Size
         get() = painter?.intrinsicSize ?: Size.Unspecified
 
     override fun DrawScope.onDraw() {
-        // Cache the draw scope's current size.
-        drawSize.tryEmit(size)
-
-        // Draw the current painter.
+        drawSize = size
         painter?.apply { draw(size, alpha, colorFilter) }
     }
 
@@ -218,7 +219,7 @@ class AsyncImagePainter internal constructor(
 
     private fun launchJob() {
         val input = _input ?: return
-        // Observe the latest request and execute any emissions.
+
         rememberJob = DeferredDispatchCoroutineScope(scope.coroutineContext).launchUndispatched {
             val previewHandler = previewHandler
             val state = if (previewHandler != null) {
@@ -264,7 +265,7 @@ class AsyncImagePainter internal constructor(
         // Connect the size resolver to the draw scope if necessary.
         val sizeResolver = request.sizeResolver
         if (sizeResolver is DrawScopeSizeResolver) {
-            sizeResolver.connect(drawSize)
+            sizeResolver.connect(lazyDrawSizeFlow())
         }
 
         return request.newBuilder()
@@ -322,6 +323,22 @@ class AsyncImagePainter internal constructor(
         )
     }
 
+    private fun lazyDrawSizeFlow(): Flow<Size> {
+        var drawSizeFlow = drawSizeFlow
+        if (drawSizeFlow == null) {
+            drawSizeFlow = MutableSharedFlow(
+                replay = 1,
+                onBufferOverflow = DROP_OLDEST,
+            )
+            val drawSize = drawSize
+            if (drawSize != Size.Unspecified) {
+                drawSizeFlow.tryEmit(drawSize)
+            }
+            this.drawSizeFlow = drawSizeFlow
+        }
+        return drawSizeFlow
+    }
+
     /**
      * The latest arguments passed to [AsyncImagePainter].
      */

diff --git a/coil-compose-core/src/commonTest/kotlin/coil3/compose/internal/DeferredDispatchTest.kt b/coil-compose-core/src/commonTest/kotlin/coil3/compose/internal/DeferredDispatchTest.kt
@@ -36,7 +36,7 @@ class DeferredDispatchTest : RobolectricTest() {
     private val deferredDispatcher = DeferredDispatchCoroutineDispatcher(testDispatcher)
 
     @Test
-    fun `does not dispatch when unconfined=true`() = runTest {
+    fun doesNotDispatchWhen_unconfined_true() = runTest {
         deferredDispatcher.unconfined = true
 
         withContext(deferredDispatcher) {
@@ -46,7 +46,7 @@ class DeferredDispatchTest : RobolectricTest() {
     }
 
     @Test
-    fun `does dispatch when unconfined=false`() = runTest {
+    fun doesDispatchWhen_unconfined_false() = runTest {
         deferredDispatcher.unconfined = false
 
         withContext(deferredDispatcher) {
@@ -57,7 +57,7 @@ class DeferredDispatchTest : RobolectricTest() {
 
     /** This test emulates the context that [AsyncImagePainter] launches its request into. */
     @Test
-    fun `imageLoader does not dispatch if context does not change`() = runTest {
+    fun imageLoaderDoesNotDispatchIfContextDoesNotChange() = runTest {
         deferredDispatcher.unconfined = true
 
         DeferredDispatchCoroutineScope(coroutineContext + deferredDispatcher).launch {
@@ -77,7 +77,7 @@ class DeferredDispatchTest : RobolectricTest() {
 
     /** This test emulates the context that [AsyncImagePainter] launches its request into. */
     @Test
-    fun `imageLoader does dispatch if context changes`() = runTest {
+    fun imageLoaderDoesDispatchIfContextChanges() = runTest {
         deferredDispatcher.unconfined = true
 
         DeferredDispatchCoroutineScope(coroutineContext + deferredDispatcher).launch {

diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -5,7 +5,7 @@ androidx-lifecycle = "2.8.7"
 androidx-tracing-perfetto = "1.0.0"
 atomicfu = "0.26.1"
 coroutines = "1.10.1"
-jetbrains-compose = "1.8.0-alpha02"
+jetbrains-compose = "1.7.3"
 kotlin = "2.1.0"
 ktlint = "1.4.1"
 ktor2 = "2.3.13"
@@ -14,7 +14,7 @@ okhttp = "4.12.0"
 okio = "3.10.2"
 paparazzi = "1.3.5"
 roborazzi = "1.40.1"
-skiko = "0.8.19"
+skiko = "0.8.18"
 
 [plugins]
 baselineProfile = { id = "androidx.baselineprofile", version.ref = "androidx-benchmark" }
@@ -40,6 +40,7 @@ androidx-appcompat-resources = "androidx.appcompat:appcompat-resources:1.7.0"
 androidx-annotation = "androidx.annotation:annotation:1.9.1"
 androidx-benchmark-macro = "androidx.benchmark:benchmark-macro-junit4:1.3.3"
 androidx-compose-ui-tooling = { group = "androidx.compose.ui", name = "ui-tooling" }
+androidx-compose-ui-test-manifest = { group = "androidx.compose.ui", name = "ui-test-manifest" }
 androidx-constraintlayout = "androidx.constraintlayout:constraintlayout:2.2.0"
 androidx-core = "androidx.core:core-ktx:1.15.0"
 androidx-exifinterface = "androidx.exifinterface:exifinterface:1.3.7"

diff --git a/internal/test-roborazzi/build.gradle.kts b/internal/test-roborazzi/build.gradle.kts
@@ -1,6 +1,5 @@
 import coil3.addAllMultiplatformTargets
 import coil3.androidLibrary
-import org.jetbrains.kotlin.gradle.ExperimentalKotlinGradlePluginApi
 
 plugins {
     id("com.android.library")
@@ -15,7 +14,6 @@ androidLibrary(name = "coil3.test.roborazzi")
 
 kotlin {
     jvm {
-        @OptIn(ExperimentalKotlinGradlePluginApi::class)
         compilerOptions {
             freeCompilerArgs.add("-Xcontext-receivers")
         }
@@ -34,12 +32,12 @@ kotlin {
             implementation(libs.roborazzi.junit)
         }
         named("jvmCommonTest").dependencies {
-            implementation(compose.desktop.uiTestJUnit4)
             implementation(libs.bundles.test.jvm)
+            implementation(compose.desktop.uiTestJUnit4)
         }
         jvmTest.dependencies {
-            implementation(compose.desktop.currentOs)
             implementation(libs.roborazzi.compose.desktop)
+            implementation(compose.desktop.currentOs)
         }
     }
 }