Move size resolution from GlideImage to GlidePainter

integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideImage.kt

@@ -13,7 +13,6 @@ import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.ColorFilter
 import androidx.compose.ui.graphics.DefaultAlpha
 import androidx.compose.ui.layout.ContentScale
-import androidx.compose.ui.layout.layout
 import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.platform.LocalInspectionMode
 import androidx.compose.ui.semantics.SemanticsPropertyKey
@@ -35,9 +34,8 @@ import com.google.accompanist.drawablepainter.rememberDrawablePainter
 public typealias RequestBuilderTransform<T> = (RequestBuilder<T>) -> RequestBuilder<T>
 
 /**
- * Start a request by passing [model] to [RequestBuilder.load] using the given [requestManager] and
- * then applying the [requestBuilderTransform] function to add options or apply mutations if the
- * caller desires.
+ * Start a request by passing [model] to [RequestBuilder.load] and then applying the
+ * [requestBuilderTransform] function to add options or apply mutations if the caller desires.
  *
  * [alignment], [contentScale], [alpha], [colorFilter] and [contentDescription] have the same
  * defaults (if any) and function identically to the parameters in [Image].
@@ -51,10 +49,6 @@ public typealias RequestBuilderTransform<T> = (RequestBuilder<T>) -> RequestBuil
  * explicitly if you can. You may end up loading a substantially larger image than you need, which
  * will increase memory usage and may also increase latency.
  *
- * If you provide your own [requestManager] rather than using this method's default, consider using
- * [remember] at a higher level to avoid some amount of overhead of retrieving it each
- * re-composition.
- *
  * This method will inspect [contentScale] and apply a matching transformation if one exists. Any
  * automatically applied transformation can be overridden using [requestBuilderTransform]. Either
  * apply a specific transformation instead, or use [RequestBuilder.dontTransform]]
@@ -111,7 +105,7 @@ public fun GlideImage(
  .let { failure?.apply(it::error, it::error) ?: it }
 
  val overrideSize: Size? = requestBuilder.overrideSize()
- val (size, finalModifier) = rememberSizeAndModifier(overrideSize, modifier)
+ val size = rememberResolvableSize(overrideSize)
 
  // TODO(judds): It seems like we should be able to use the production paths for
  // resource / drawables as well as Composables. It's not totally clear what part of the prod code
@@ -124,7 +118,7 @@ public fun GlideImage(
  SizedGlideImage(
  requestBuilder = requestBuilder,
  size = size,
- modifier = finalModifier,
+ modifier = modifier,
  contentDescription = contentDescription,
  alignment = alignment,
  contentScale = contentScale,
@@ -233,25 +227,13 @@ public sealed class Placeholder {
  }
 }
 
-@OptIn(InternalGlideApi::class)
-private data class SizeAndModifier(val size: ResolvableGlideSize, val modifier: Modifier)
-
 @OptIn(InternalGlideApi::class)
 @Composable
-private fun rememberSizeAndModifier(
+private fun rememberResolvableSize(
  overrideSize: Size?,
- modifier: Modifier,
 ) =
- remember(overrideSize, modifier) {
- if (overrideSize != null) {
- SizeAndModifier(ImmediateGlideSize(overrideSize), modifier)
- } else {
- val sizeObserver = SizeObserver()
- SizeAndModifier(
- AsyncGlideSize(sizeObserver::getSize),
- modifier.sizeObservingModifier(sizeObserver)
- )
- }
+ remember(overrideSize) {
+ overrideSize?.let { ImmediateGlideSize(it) } ?: AsyncGlideSize()
  }
 
 @Composable
@@ -347,17 +329,6 @@ private fun rememberGlidePainter(
  return remember(requestBuilder, size) { GlidePainter(requestBuilder, size, scope) }
 }
 
-@OptIn(InternalGlideApi::class)
-private fun Modifier.sizeObservingModifier(sizeObserver: SizeObserver): Modifier =
- this.layout { measurable, constraints ->
- val inferredSize = constraints.inferredGlideSize()
- if (inferredSize != null) {
- sizeObserver.setSize(inferredSize)
- }
- val placeable = measurable.measure(constraints)
- layout(placeable.width, placeable.height) { placeable.place(0, 0) }
- }
-
 internal val DisplayedDrawableKey =
  SemanticsPropertyKey<MutableState<Drawable?>>("DisplayedDrawable")
 internal var SemanticsPropertyReceiver.displayedDrawable by DisplayedDrawableKey

integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlidePainter.kt

@@ -3,7 +3,6 @@ package com.bumptech.glide.integration.compose
 import android.graphics.drawable.BitmapDrawable
 import android.graphics.drawable.ColorDrawable
 import android.graphics.drawable.Drawable
-import android.util.Log
 import androidx.compose.runtime.MutableState
 import androidx.compose.runtime.RememberObserver
 import androidx.compose.runtime.Stable
@@ -20,7 +19,9 @@ import androidx.compose.ui.graphics.painter.BitmapPainter
 import androidx.compose.ui.graphics.painter.ColorPainter
 import androidx.compose.ui.graphics.painter.Painter
 import com.bumptech.glide.RequestBuilder
+import com.bumptech.glide.integration.ktx.AsyncGlideSize
 import com.bumptech.glide.integration.ktx.ExperimentGlideFlows
+import com.bumptech.glide.integration.ktx.ImmediateGlideSize
 import com.bumptech.glide.integration.ktx.InternalGlideApi
 import com.bumptech.glide.integration.ktx.Placeholder
 import com.bumptech.glide.integration.ktx.ResolvableGlideSize
@@ -44,7 +45,7 @@ internal class GlidePainter
 @OptIn(InternalGlideApi::class)
 constructor(
  private val requestBuilder: RequestBuilder<Drawable>,
- private val size: ResolvableGlideSize,
+ private val resolvableSize: ResolvableGlideSize,
  scope: CoroutineScope,
 ) : Painter(), RememberObserver {
  @OptIn(ExperimentGlideFlows::class) internal var status: Status by mutableStateOf(Status.CLEARED)
@@ -59,7 +60,15 @@ constructor(
  override val intrinsicSize: Size
  get() = delegate?.intrinsicSize ?: Size.Unspecified
 
+ @OptIn(InternalGlideApi::class)
  override fun DrawScope.onDraw() {
+ when (resolvableSize) {
+ is AsyncGlideSize -> {
+ size.toGlideSize()?.let { resolvableSize.setSize(it) }
+ }
+ // Do nothing.
+ is ImmediateGlideSize -> {}
+ }
  delegate?.apply { draw(size, alpha, colorFilter) }
  }
 
@@ -82,7 +91,7 @@ constructor(
 
  @OptIn(ExperimentGlideFlows::class, InternalGlideApi::class)
  private fun launchRequest() = this.scope.launch {
- requestBuilder.flowResolvable(size).collect {
+ requestBuilder.flowResolvable(resolvableSize).collect {
  updateDelegate(
  when (it) {
  is Resource -> it.resource

integration/compose/src/main/java/com/bumptech/glide/integration/compose/Sizes.kt

@@ -2,13 +2,12 @@
 
 package com.bumptech.glide.integration.compose
 
-import androidx.compose.ui.unit.Constraints
 import com.bumptech.glide.RequestBuilder
 import com.bumptech.glide.integration.ktx.InternalGlideApi
 import com.bumptech.glide.integration.ktx.Size
 import com.bumptech.glide.integration.ktx.isValidGlideDimension
-import com.bumptech.glide.request.target.Target
 import kotlinx.coroutines.CompletableDeferred
+import kotlin.math.roundToInt
 
 internal class SizeObserver {
  private val size = CompletableDeferred<Size>()
@@ -32,12 +31,11 @@ internal fun RequestBuilder<out Any?>.overrideSize(): Size? =
 internal fun RequestBuilder<out Any?>.isOverrideSizeSet(): Boolean =
  overrideWidth.isValidGlideDimension() && overrideHeight.isValidGlideDimension()
 
-internal fun Constraints.inferredGlideSize(): Size? {
- val width = if (hasBoundedWidth) maxWidth else Target.SIZE_ORIGINAL
- val height = if (hasBoundedHeight) maxHeight else Target.SIZE_ORIGINAL
+internal fun androidx.compose.ui.geometry.Size.toGlideSize(): Size? {
+ val width = width.roundToInt();
+ val height = height.roundToInt();
  if (!width.isValidGlideDimension() || !height.isValidGlideDimension()) {
- return null
+ return null;
  }
- return Size(width, height)
+ return Size(width, height);
 }
-

integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt

@@ -13,6 +13,7 @@ import com.bumptech.glide.request.target.Target
 import com.bumptech.glide.request.transition.Transition
 import com.bumptech.glide.requestManager
 import com.bumptech.glide.util.Util
+import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.channels.ProducerScope
 import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
@@ -82,17 +83,18 @@ public fun <ResourceT : Any> RequestBuilder<ResourceT>.flow(
 
 /**
  * Identical to [flow] with dimensions, except that the size is resolved asynchronously using
- * [waitForSize].
+ * [size].
  *
  * If an override size has been set using [RequestBuilder.override], that size will be used instead
- * and [waitForSize] may never be called.
+ * and [size] may never be called.
  *
- * [Placeholder] values may be emitted prior to [waitForSize] returning. Similarly if
+ * [Placeholder] values may be emitted prior to [size] returning. Similarly if
  * [RequestBuilder.thumbnail] requests are present and have overridden sizes, [Resource] values for
- * those thumbnails may also be emitted. [waitForSize] will only be used for requests where no
+ * those thumbnails may also be emitted. [size] will only be used for requests where no
  * [RequestBuilder.override] size is available.
  *
- * If [waitForSize] does not return, this flow may never return values other than placeholders.
+ * If [size] never has [AsyncGlideSize.setSize] called, this flow may never return values other than
+ * placeholders.
  *
  * This function is internal only, intended primarily for Compose. The Target API provides similar
  * functionality for traditional Views. We could consider expanding the visibility if there are use
@@ -101,8 +103,8 @@ public fun <ResourceT : Any> RequestBuilder<ResourceT>.flow(
 @InternalGlideApi
 @ExperimentGlideFlows
 public fun <ResourceT : Any> RequestBuilder<ResourceT>.flow(
- waitForSize: suspend () -> Size,
-): Flow<GlideFlowInstant<ResourceT>> = flow(AsyncGlideSize(waitForSize))
+ size: AsyncGlideSize,
+): Flow<GlideFlowInstant<ResourceT>> = flowResolvable(size)
 
 /**
  * Convert a load in Glide into a flow that emits placeholders and resources in the order they'd be
@@ -276,7 +278,7 @@ private class FlowTarget<ResourceT : Any>(
  // begin immediately, shaving off some small amount of time.
  is AsyncGlideSize ->
  scope.launch {
- val localResolvedSize = size.asyncSize()
+ val localResolvedSize = size.getSize()
  val callbacksToNotify: List<SizeReadyCallback>
  synchronized(this) {
  resolvedSize = localResolvedSize
@@ -387,6 +389,16 @@ public data class Size(val width: Int, val height: Int) {
 @InternalGlideApi public data class ImmediateGlideSize(val size: Size) : ResolvableGlideSize()
 
 @InternalGlideApi
-public data class AsyncGlideSize(val asyncSize: suspend () -> Size) : ResolvableGlideSize()
+public class AsyncGlideSize : ResolvableGlideSize() {
+ private val size = CompletableDeferred<Size>()
+
+ public fun setSize(size: Size) {
+ this.size.complete(size)
+ }
+
+ public suspend fun getSize(): Size {
+ return size.await()
+ }
+}
 
 @InternalGlideApi public fun Int.isValidGlideDimension(): Boolean = Util.isValidDimension(this)

integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt

@@ -31,14 +31,11 @@ import com.google.common.truth.Correspondence
 import com.google.common.truth.IterableSubject
 import com.google.common.truth.Truth.assertThat
 import java.io.File
-import java.lang.RuntimeException
 import java.util.concurrent.atomic.AtomicReference
 import kotlin.reflect.KClass
 import kotlin.test.assertFailsWith
 import kotlinx.coroutines.DelicateCoroutinesApi
 import kotlinx.coroutines.ExperimentalCoroutinesApi
-import kotlinx.coroutines.channels.Channel
-import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.flow
@@ -494,11 +491,13 @@ class FlowsTest {
  @Test
  fun flow_withAsyncSize_andOverrideSize_usesOverrideSize() = runTest {
  val requestedSizeReference = registerSizeCapturingFakeModelLoader()
+ val asyncSize = AsyncGlideSize()
+ asyncSize.setSize(Size(1, 2))
 
  Glide.with(context)
  .load(FakeModel())
  .override(200, 100)
- .flow { Size(1, 2) }
+ .flow(asyncSize)
  .firstLoad()
  .toList()
 
@@ -525,11 +524,7 @@ class FlowsTest {
  @Test
  fun flow_withAsyncSize_concreteSizeForThumbnail_startsMainRequestWhenAsyncSizeIsAvailable() =
  runTest {
- val waitForThumbnailToFinishChannel = Channel<Boolean>()
- val waitForThumbnailToFinishSize: suspend () -> Size = {
- waitForThumbnailToFinishChannel.receive()
- Size(100, 200)
- }
+ val size = AsyncGlideSize()
 
  val result =
  Glide.with(context)
@@ -538,9 +533,9 @@ class FlowsTest {
  Glide.with(context)
  .load(newImageFile())
  .override(75, 50)
- .listener(onSuccess { launch { waitForThumbnailToFinishChannel.send(true) } })
+ .listener(onSuccess { launch { size.setSize(Size(100, 200)) } })
  )
- .flow(waitForThumbnailToFinishSize)
+ .flow(size)
  .firstLoad()
  .toList()
 
@@ -573,10 +568,7 @@ class FlowsTest {
  assertFailsWith<IllegalArgumentException> { requestBuilder.flow() }
  }
 
- private val delayForever: suspend () -> Size = {
- delay(kotlin.time.Duration.INFINITE)
- throw RuntimeException()
- }
+ private val delayForever = AsyncGlideSize()
 
  private fun registerSizeCapturingFakeModelLoader(): AtomicReference<Size> {
  val result = AtomicReference<Size>()