Skip to content

Commit

Permalink
Re-add soft dependency on Dispatchers.Main.immediate. (#2356)
Browse files Browse the repository at this point in the history
* Use EmptyCoroutineContext instead of Dispatchers.Unconfined.

* Update painter on main thread.

* Fix.

* Re-add.

* Re-add.

* Docs.

* Clean up implementation.

* Use collectLatest.

* Revert.
  • Loading branch information
colinrtwhite authored Jul 8, 2024
1 parent 29e0726 commit 92d9342
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import coil3.compose.internal.AsyncImageState
import coil3.compose.internal.CrossfadePainter
import coil3.compose.internal.onStateOf
import coil3.compose.internal.requestOf
import coil3.compose.internal.safeImmediateMainDispatcher
import coil3.compose.internal.toScale
import coil3.compose.internal.transformOf
import coil3.request.ErrorResult
Expand All @@ -45,6 +46,7 @@ import coil3.request.ImageResult
import coil3.request.SuccessResult
import coil3.size.Precision
import coil3.size.SizeResolver
import kotlin.coroutines.EmptyCoroutineContext
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -165,7 +167,7 @@ private fun rememberAsyncImagePainter(

val input = Input(state.imageLoader, request)
val painter = remember { AsyncImagePainter(input) }
painter.scope = rememberCoroutineScope { Dispatchers.Unconfined }
painter.scope = rememberCoroutineScope()
painter.transform = transform
painter.onState = onState
painter.contentScale = contentScale
Expand All @@ -191,13 +193,13 @@ class AsyncImagePainter internal constructor(
private var alpha: Float by mutableFloatStateOf(DefaultAlpha)
private var colorFilter: ColorFilter? by mutableStateOf(null)

internal lateinit var scope: CoroutineScope
private var rememberJob: Job? = null
set(value) {
field?.cancel()
field = value
}

internal lateinit var scope: CoroutineScope
internal var transform = DefaultTransform
internal var onState: ((State) -> Unit)? = null
internal var contentScale = ContentScale.Fit
Expand Down Expand Up @@ -238,19 +240,23 @@ class AsyncImagePainter internal constructor(
(painter as? RememberObserver)?.onRemembered()

// Observe the latest request and execute any emissions.
rememberJob = scope.launch {
val previewHandler = previewHandler
if (previewHandler != null) {
// If we're in inspection mode use the preview renderer.
val previewHandler = previewHandler
if (previewHandler != null) {
// If we're in inspection mode use the preview renderer.
rememberJob = scope.launch(Dispatchers.Unconfined) {
_input.mapLatest {
previewHandler.handle(it.imageLoader, updateRequest(it.request, isPreview = true))
}
} else {
// Else, execute the request as normal.
val request = updateRequest(it.request, isPreview = true)
previewHandler.handle(it.imageLoader, request)
}.collect(::updateState)
}
} else {
// Else, execute the request as normal.
rememberJob = scope.launch(safeImmediateMainDispatcher) {
_input.mapLatest {
it.imageLoader.execute(updateRequest(it.request, isPreview = false)).toState()
}
}.collect(::updateState)
val request = updateRequest(it.request, isPreview = false)
it.imageLoader.execute(request).toState()
}.collect(::updateState)
}
}
}

Expand Down Expand Up @@ -293,21 +299,22 @@ class AsyncImagePainter internal constructor(
precision(Precision.INEXACT)
}
if (isPreview) {
coroutineContext(Dispatchers.Unconfined)
// The request must be executed synchronously in the preview environment.
coroutineContext(EmptyCoroutineContext)
}
applyGlobalLifecycle()
}
.build()
}

private fun updateState(input: State) {
private fun updateState(state: State) {
val previous = _state.value
val current = transform(input)
val current = transform(state)
_state.value = current
painter = maybeNewCrossfadePainter(previous, current, contentScale) ?: current.painter

// Manually forget and remember the old/new painters if we're already remembered.
if (rememberJob != null && previous.painter !== current.painter) {
// Manually forget and remember the old/new painters.
if (previous.painter !== current.painter) {
(previous.painter as? RememberObserver)?.onForgotten()
(current.painter as? RememberObserver)?.onRemembered()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ import coil3.size.Dimension
import coil3.size.Scale
import coil3.size.Size as CoilSize
import coil3.size.SizeResolver
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext
import kotlin.math.roundToInt
import kotlinx.coroutines.Dispatchers

/** Create an [ImageRequest] from the [model]. */
@Composable
Expand Down Expand Up @@ -196,3 +199,17 @@ internal inline fun Float.takeOrElse(block: () -> Float) = if (isFinite()) this
internal fun Size.toIntSize() = IntSize(width.roundToInt(), height.roundToInt())

internal val Size.isPositive get() = width >= 0.5 && height >= 0.5

// We need `Dispatchers.Main.immediate` to be able to execute immediately on the main thread so we
// can reach the loading state, set the placeholder, and maybe resolve from the memory cache.
// The default main dispatcher provided with Compose always dispatches, which will often cause one
// frame of delay. In the cases where we don't have the main dispatcher implicitly fall back to
// Compose's built in main dispatcher.
internal val safeImmediateMainDispatcher: CoroutineContext = try {
Dispatchers.Main.immediate.also {
// This will throw if the implementation is missing.
it.isDispatchNeeded(EmptyCoroutineContext)
}
} catch (_: Throwable) {
EmptyCoroutineContext
}
1 change: 1 addition & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ androidx-vectordrawable-animated = "androidx.vectordrawable:vectordrawable-anima

coroutines-android = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-android", version.ref = "coroutines" }
coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "coroutines" }
coroutines-swing = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-swing", version.ref = "coroutines" }
coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "coroutines" }

google-material = "com.google.android.material:material:1.12.0"
Expand Down
1 change: 1 addition & 0 deletions internal/test-roborazzi/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ kotlin {
named("jvmCommonTest").dependencies {
implementation(compose.desktop.uiTestJUnit4)
implementation(libs.bundles.test.jvm)
implementation(libs.coroutines.swing)
}
jvmTest.dependencies {
implementation(compose.desktop.currentOs)
Expand Down
1 change: 1 addition & 0 deletions samples/shared/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ kotlin {
jvmMain {
dependencies {
api(projects.coilNetworkOkhttp)
api(libs.coroutines.swing)
}
}
named("wasmJsMain") {
Expand Down
2 changes: 1 addition & 1 deletion test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ set -e

# Run separately to work around https://github.com/diffplug/spotless/issues/1572.
./gradlew apiCheck spotlessCheck
./gradlew allTests testDebugUnitTest connectedDebugAndroidTest verifyPaparazziDebug verifyRoborazziDebug verifyRoborazziJvm
./gradlew allTests testDebugUnitTest connectedDebugAndroidTest validateDebugScreenshotTest verifyPaparazziDebug verifyRoborazziDebug verifyRoborazziJvm

0 comments on commit 92d9342

Please # to comment.