Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on Dispatchers.Main.immediate for immediate dispatching in Compose. #2725

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
042751b
Remove dependency on Dispatchers.Main.immediate for immediate dispatc…
colinrtwhite Nov 29, 2024
faeb7be
Fix API.
colinrtwhite Nov 29, 2024
a906110
Docs.
colinrtwhite Nov 29, 2024
f70aed3
Add tests.
colinrtwhite Dec 3, 2024
e586ac5
Change strategy.
colinrtwhite Dec 19, 2024
0a872e7
Rename dispatcher.
colinrtwhite Dec 19, 2024
eb00783
Slight tweak.
colinrtwhite Dec 19, 2024
d167d51
Rename and change implementation.
colinrtwhite Dec 20, 2024
8146de6
Remove Android test.
colinrtwhite Dec 20, 2024
10169b3
Fix tests.
colinrtwhite Dec 20, 2024
080198a
Docs.
colinrtwhite Dec 20, 2024
3ff04e5
Improve test coverage.
colinrtwhite Dec 20, 2024
94661bb
Fix test.
colinrtwhite Dec 20, 2024
6d4a8e3
Remove note.
colinrtwhite Dec 20, 2024
5378aba
Improve tests.
colinrtwhite Dec 20, 2024
7b2fd2b
Fix tests.
colinrtwhite Dec 20, 2024
2346e27
Fix mutating shared variable across contexts.
colinrtwhite Dec 20, 2024
829d7f9
Fix tests.
colinrtwhite Dec 20, 2024
dbded59
Fix tests.
colinrtwhite Dec 20, 2024
394b22d
Tweak solution.
colinrtwhite Dec 20, 2024
553e527
Fix style.
colinrtwhite Dec 20, 2024
8356177
Guard against race conditions.
colinrtwhite Dec 20, 2024
f193279
Fix build.
colinrtwhite Dec 20, 2024
2b11ee5
Improve solution.
colinrtwhite Dec 21, 2024
9d3381c
Docs.
colinrtwhite Dec 21, 2024
0e7aa84
Remove unnecessary optimization.
colinrtwhite Dec 21, 2024
8ac95cc
Pass context through.
colinrtwhite Dec 22, 2024
ebddb12
Docs.
colinrtwhite Dec 22, 2024
63c3f6c
Fix missing join.
colinrtwhite Dec 23, 2024
5e53a1b
Don't pay for dispatch.
colinrtwhite Dec 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions coil-compose-core/api/coil-compose-core.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ final class coil3.compose/ImagePainter : androidx.compose.ui.graphics.painter/Pa
final val coil3.compose.internal/coil3_compose_internal_AsyncImageState$stableprop // coil3.compose.internal/coil3_compose_internal_AsyncImageState$stableprop|#static{}coil3_compose_internal_AsyncImageState$stableprop[0]
final val coil3.compose.internal/coil3_compose_internal_ContentPainterElement$stableprop // coil3.compose.internal/coil3_compose_internal_ContentPainterElement$stableprop|#static{}coil3_compose_internal_ContentPainterElement$stableprop[0]
final val coil3.compose.internal/coil3_compose_internal_ContentPainterNode$stableprop // coil3.compose.internal/coil3_compose_internal_ContentPainterNode$stableprop|#static{}coil3_compose_internal_ContentPainterNode$stableprop[0]
final val coil3.compose.internal/coil3_compose_internal_ForwardingCoroutineContext$stableprop // coil3.compose.internal/coil3_compose_internal_ForwardingCoroutineContext$stableprop|#static{}coil3_compose_internal_ForwardingCoroutineContext$stableprop[0]
final val coil3.compose.internal/coil3_compose_internal_ForwardingUnconfinedCoroutineDispatcher$stableprop // coil3.compose.internal/coil3_compose_internal_ForwardingUnconfinedCoroutineDispatcher$stableprop|#static{}coil3_compose_internal_ForwardingUnconfinedCoroutineDispatcher$stableprop[0]
final val coil3.compose/LocalAsyncImageModelEqualityDelegate // coil3.compose/LocalAsyncImageModelEqualityDelegate|{}LocalAsyncImageModelEqualityDelegate[0]
final fun <get-LocalAsyncImageModelEqualityDelegate>(): androidx.compose.runtime/ProvidableCompositionLocal<coil3.compose/AsyncImageModelEqualityDelegate> // coil3.compose/LocalAsyncImageModelEqualityDelegate.<get-LocalAsyncImageModelEqualityDelegate>|<get-LocalAsyncImageModelEqualityDelegate>(){}[0]
final val coil3.compose/LocalAsyncImagePreviewHandler // coil3.compose/LocalAsyncImagePreviewHandler|{}LocalAsyncImagePreviewHandler[0]
Expand All @@ -204,6 +206,8 @@ final fun (coil3/Image).coil3.compose/asPainter(coil3/PlatformContext, androidx.
final fun coil3.compose.internal/coil3_compose_internal_AsyncImageState$stableprop_getter(): kotlin/Int // coil3.compose.internal/coil3_compose_internal_AsyncImageState$stableprop_getter|coil3_compose_internal_AsyncImageState$stableprop_getter(){}[0]
final fun coil3.compose.internal/coil3_compose_internal_ContentPainterElement$stableprop_getter(): kotlin/Int // coil3.compose.internal/coil3_compose_internal_ContentPainterElement$stableprop_getter|coil3_compose_internal_ContentPainterElement$stableprop_getter(){}[0]
final fun coil3.compose.internal/coil3_compose_internal_ContentPainterNode$stableprop_getter(): kotlin/Int // coil3.compose.internal/coil3_compose_internal_ContentPainterNode$stableprop_getter|coil3_compose_internal_ContentPainterNode$stableprop_getter(){}[0]
final fun coil3.compose.internal/coil3_compose_internal_ForwardingCoroutineContext$stableprop_getter(): kotlin/Int // coil3.compose.internal/coil3_compose_internal_ForwardingCoroutineContext$stableprop_getter|coil3_compose_internal_ForwardingCoroutineContext$stableprop_getter(){}[0]
final fun coil3.compose.internal/coil3_compose_internal_ForwardingUnconfinedCoroutineDispatcher$stableprop_getter(): kotlin/Int // coil3.compose.internal/coil3_compose_internal_ForwardingUnconfinedCoroutineDispatcher$stableprop_getter|coil3_compose_internal_ForwardingUnconfinedCoroutineDispatcher$stableprop_getter(){}[0]
final fun coil3.compose/AsyncImage(kotlin/Any?, kotlin/String?, coil3/ImageLoader, androidx.compose.ui/Modifier?, androidx.compose.ui.graphics.painter/Painter?, androidx.compose.ui.graphics.painter/Painter?, androidx.compose.ui.graphics.painter/Painter?, kotlin/Function1<coil3.compose/AsyncImagePainter.State.Loading, kotlin/Unit>?, kotlin/Function1<coil3.compose/AsyncImagePainter.State.Success, kotlin/Unit>?, kotlin/Function1<coil3.compose/AsyncImagePainter.State.Error, kotlin/Unit>?, androidx.compose.ui/Alignment?, androidx.compose.ui.layout/ContentScale?, kotlin/Float, androidx.compose.ui.graphics/ColorFilter?, androidx.compose.ui.graphics/FilterQuality, kotlin/Boolean, androidx.compose.runtime/Composer?, kotlin/Int, kotlin/Int, kotlin/Int) // coil3.compose/AsyncImage|AsyncImage(kotlin.Any?;kotlin.String?;coil3.ImageLoader;androidx.compose.ui.Modifier?;androidx.compose.ui.graphics.painter.Painter?;androidx.compose.ui.graphics.painter.Painter?;androidx.compose.ui.graphics.painter.Painter?;kotlin.Function1<coil3.compose.AsyncImagePainter.State.Loading,kotlin.Unit>?;kotlin.Function1<coil3.compose.AsyncImagePainter.State.Success,kotlin.Unit>?;kotlin.Function1<coil3.compose.AsyncImagePainter.State.Error,kotlin.Unit>?;androidx.compose.ui.Alignment?;androidx.compose.ui.layout.ContentScale?;kotlin.Float;androidx.compose.ui.graphics.ColorFilter?;androidx.compose.ui.graphics.FilterQuality;kotlin.Boolean;androidx.compose.runtime.Composer?;kotlin.Int;kotlin.Int;kotlin.Int){}[0]
final fun coil3.compose/AsyncImage(kotlin/Any?, kotlin/String?, coil3/ImageLoader, androidx.compose.ui/Modifier?, kotlin/Function1<coil3.compose/AsyncImagePainter.State, coil3.compose/AsyncImagePainter.State>?, kotlin/Function1<coil3.compose/AsyncImagePainter.State, kotlin/Unit>?, androidx.compose.ui/Alignment?, androidx.compose.ui.layout/ContentScale?, kotlin/Float, androidx.compose.ui.graphics/ColorFilter?, androidx.compose.ui.graphics/FilterQuality, kotlin/Boolean, androidx.compose.runtime/Composer?, kotlin/Int, kotlin/Int, kotlin/Int) // coil3.compose/AsyncImage|AsyncImage(kotlin.Any?;kotlin.String?;coil3.ImageLoader;androidx.compose.ui.Modifier?;kotlin.Function1<coil3.compose.AsyncImagePainter.State,coil3.compose.AsyncImagePainter.State>?;kotlin.Function1<coil3.compose.AsyncImagePainter.State,kotlin.Unit>?;androidx.compose.ui.Alignment?;androidx.compose.ui.layout.ContentScale?;kotlin.Float;androidx.compose.ui.graphics.ColorFilter?;androidx.compose.ui.graphics.FilterQuality;kotlin.Boolean;androidx.compose.runtime.Composer?;kotlin.Int;kotlin.Int;kotlin.Int){}[0]
final fun coil3.compose/DrawScopeSizeResolver(): coil3.compose/DrawScopeSizeResolver // coil3.compose/DrawScopeSizeResolver|DrawScopeSizeResolver(){}[0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableFloatStateOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import androidx.compose.ui.geometry.Size
import androidx.compose.ui.graphics.ColorFilter
Expand All @@ -30,8 +31,10 @@ import coil3.compose.AsyncImagePainter.Companion.DefaultTransform
import coil3.compose.AsyncImagePainter.Input
import coil3.compose.AsyncImagePainter.State
import coil3.compose.internal.AsyncImageState
import coil3.compose.internal.ForwardingUnconfinedCoroutineDispatcher
import coil3.compose.internal.ForwardingUnconfinedCoroutineScope
import coil3.compose.internal.dispatcher
import coil3.compose.internal.onStateOf
import coil3.compose.internal.rememberImmediateCoroutineScope
import coil3.compose.internal.requestOf
import coil3.compose.internal.toScale
import coil3.compose.internal.transformOf
Expand All @@ -43,16 +46,19 @@ import coil3.size.Precision
import coil3.size.SizeResolver
import kotlin.coroutines.EmptyCoroutineContext
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.Job
import kotlinx.coroutines.channels.BufferOverflow.DROP_OLDEST
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.mapLatest
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.transformLatest
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext

/**
* Return an [AsyncImagePainter] that executes an [ImageRequest] asynchronously and renders the result.
Expand Down Expand Up @@ -136,7 +142,7 @@ private fun rememberAsyncImagePainter(

val input = Input(state.imageLoader, request, state.modelEqualityDelegate)
val painter = remember { AsyncImagePainter(input) }
painter.scope = rememberImmediateCoroutineScope()
painter.scope = rememberCoroutineScope()
painter.transform = transform
painter.onState = onState
painter.contentScale = contentScale
Expand Down Expand Up @@ -213,22 +219,26 @@ class AsyncImagePainter internal constructor(
(painter as? RememberObserver)?.onRemembered()

// Observe the latest request and execute any emissions.
rememberJob = scope.launch {
restartSignal
.flatMapLatest { _input }
.mapLatest { input ->
val previewHandler = previewHandler
if (previewHandler != null) {
// If we're in inspection mode use the preview renderer.
val request = updateRequest(input.request, isPreview = true)
previewHandler.handle(input.imageLoader, request)
} else {
// Else, execute the request as normal.
val request = updateRequest(input.request, isPreview = false)
input.imageLoader.execute(request).toState()
val originalDispatcher = scope.coroutineContext.dispatcher ?: Dispatchers.Unconfined
val scope = ForwardingUnconfinedCoroutineScope(scope.coroutineContext)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a while to understand how ForwardingUnconfinedCoroutineScope and ForwardingUnconfinedCoroutineDispatcher interact. Could the code that wires them up with the launch be extracted into a helper function? That logic seems mostly unrelated to the other logic in this function, which is more high-level, and pulling it out might make this a bit more readable.

Eg. (names are arbitrary):

// Observe the latest request and execute any emissions.
rememberJob = scope.launchTemporarilyUnconfined {
  restartSignal.transformLatest<Unit, Nothing> {
    _input.collect { input ->
      withNormalDispatch {
        val previewHandler = previewHandler
        // ...
      }
    }
  }
}

fun CoroutineScope.launchTemporarilyUnconfined(
  block: suspend DeferringDispatchScope.() -> Unit
): Job {
  val originalDispatcher = coroutineContext.dispatcher ?: Dispatchers.Unconfined
  val forwardingScope = ForwardingUnconfinedCoroutineScope(scope.coroutineContext)
  forwardingScope.launch(Dispatchers.Unconfined, CoroutineStart.UNDISPATCHED) {
    DeferringDispatchScope(this, originalDispatcher).block()
  }
}

class DeferringDispatchScope internal constructor(
  scope: CoroutineScope,
  private val originalDispatcher: CoroutineDispatcher
): CoroutineScope by scope {
  // This could also just be a function passed as a normal param to launchDeferringDispatch's
  // block lambda if you don't want to bother with a separate class. 
  suspend fun <T> withNormalDispatch(block: suspend CoroutineScope.() -> T) =
    withContext(ForwardingUnconfinedCoroutineDispatcher(originalDispatcher), block)
}

but that might be a bit over-engineered, just an idea.

rememberJob = scope.launch(Dispatchers.Unconfined, CoroutineStart.UNDISPATCHED) {
restartSignal.transformLatest<Unit, Nothing> {
_input.collect { input ->
withContext(ForwardingUnconfinedCoroutineDispatcher(originalDispatcher)) {
val previewHandler = previewHandler
val state = if (previewHandler != null) {
// If we're in inspection mode use the preview renderer.
val request = updateRequest(input.request, isPreview = true)
previewHandler.handle(input.imageLoader, request)
} else {
// Else, execute the request as normal.
val request = updateRequest(input.request, isPreview = false)
input.imageLoader.execute(request).toState()
}
updateState(state)
}
}
.collect(::updateState)
}.collect()
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package coil3.compose.internal

import kotlin.coroutines.CoroutineContext

/**
* A special [CoroutineContext] implementation that lets us observe changes to its elements.
*/
internal class ForwardingCoroutineContext(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oo this is a neat trick.

private val delegate: CoroutineContext,
private val onNewContext: (old: CoroutineContext, new: CoroutineContext) -> Unit,
) : CoroutineContext by delegate {

override fun minusKey(key: CoroutineContext.Key<*>): CoroutineContext {
val new = delegate.minusKey(key)
onNewContext(this, new)
return ForwardingCoroutineContext(new, onNewContext)
}

override operator fun plus(context: CoroutineContext): CoroutineContext {
val new = delegate + context
onNewContext(this, new)
return ForwardingCoroutineContext(new, onNewContext)
}

override fun equals(other: Any?): Boolean {
return delegate == other
}

override fun hashCode(): Int {
return delegate.hashCode()
}

override fun toString(): String {
return delegate.toString()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package coil3.compose.internal

import kotlin.coroutines.CoroutineContext
import kotlinx.atomicfu.atomic
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.InternalCoroutinesApi
import kotlinx.coroutines.Runnable

/**
* A [CoroutineScope] with a special [CoroutineContext] that enables [ForwardingUnconfinedCoroutineDispatcher]
* to enable dispatching after it is replaced in the context.
*/
internal fun ForwardingUnconfinedCoroutineScope(
context: CoroutineContext,
) = CoroutineScope(
context = ForwardingCoroutineContext(context) { old, new ->
val oldDispatcher = old.dispatcher
val newDispatcher = new.dispatcher
if (oldDispatcher is ForwardingUnconfinedCoroutineDispatcher && oldDispatcher != newDispatcher) {
oldDispatcher.unconfined = oldDispatcher.unconfined &&
(newDispatcher == null || newDispatcher == Dispatchers.Unconfined)
}
}
)

/**
* A [CoroutineDispatcher] that delegates to [Dispatchers.Unconfined] while [unconfined] is true
* and [delegate] when [unconfined] is false.
*/
internal class ForwardingUnconfinedCoroutineDispatcher(
private val delegate: CoroutineDispatcher,
) : CoroutineDispatcher() {
private val _unconfined = atomic(true)
var unconfined by _unconfined

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider documenting that after setting this to false, it's ignored even if set to true again.


private val currentDispatcher: CoroutineDispatcher
get() = if (_unconfined.value) Dispatchers.Unconfined else delegate

override fun isDispatchNeeded(context: CoroutineContext): Boolean {
return currentDispatcher.isDispatchNeeded(context)
}

override fun limitedParallelism(parallelism: Int, name: String?): CoroutineDispatcher {
return currentDispatcher.limitedParallelism(parallelism, name)
}

override fun dispatch(context: CoroutineContext, block: Runnable) {
currentDispatcher.dispatch(context, block)
}

@InternalCoroutinesApi
override fun dispatchYield(context: CoroutineContext, block: Runnable) {
currentDispatcher.dispatchYield(context, block)
}

override fun toString(): String {
return "ForwardingUnconfinedCoroutineDispatcher(delegate=$delegate)"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ import androidx.compose.runtime.NonRestartableComposable
import androidx.compose.runtime.ReadOnlyComposable
import androidx.compose.runtime.Stable
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Size
import androidx.compose.ui.geometry.isUnspecified
import androidx.compose.ui.graphics.painter.Painter
import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.platform.LocalInspectionMode
import androidx.compose.ui.semantics.Role
import androidx.compose.ui.semantics.contentDescription
import androidx.compose.ui.semantics.role
Expand All @@ -35,12 +33,8 @@ 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.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.MainCoroutineDispatcher

/** Create an [ImageRequest] from the [model]. */
@Composable
Expand Down Expand Up @@ -219,43 +213,6 @@ 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. If `Dispatchers.Main.immediate` isn't available, fall back to
// `Dispatchers.Unconfined`, which will execute immediately even if we're not on the main
// thread. This will typically only occur in preview/test environments where image loading
// should execute synchronously.
private val immediateDispatcher: CoroutineDispatcher = try {
Dispatchers.Main.immediate.also {
// This will throw if the implementation is missing.
it.isDispatchNeeded(EmptyCoroutineContext)
}
} catch (_: Throwable) {
Dispatchers.Unconfined
}

@OptIn(ExperimentalStdlibApi::class)
private fun CoroutineContext.resolveImmediateDispatcher(): CoroutineDispatcher {
val dispatcher = get(CoroutineDispatcher)
if (dispatcher is MainCoroutineDispatcher) {
try {
return dispatcher.immediate
} catch (_: UnsupportedOperationException) {}
}
return immediateDispatcher
}

@Composable
internal fun rememberImmediateCoroutineScope(): CoroutineScope {
val scope = rememberCoroutineScope()
val isPreview = LocalInspectionMode.current
return remember(scope, isPreview) {
val context = if (isPreview) {
scope.coroutineContext + Dispatchers.Unconfined
} else {
scope.coroutineContext.run { this + resolveImmediateDispatcher() }
}
CoroutineScope(context)
}
}
internal val CoroutineContext.dispatcher: CoroutineDispatcher?
get() = get(CoroutineDispatcher)
Loading
Loading