Add a Painter variant of the placeholder APIs

6 files changed, 171 insertions, 46 deletions

diff --git a/integration/compose/api/compose.api b/integration/compose/api/compose.api
index f4f4f6d5..9ceb5837 100644
--- a/integration/compose/api/compose.api
+++ b/integration/compose/api/compose.api
@@ -19,6 +19,7 @@ public final class com/bumptech/glide/integration/compose/GlideImageKt {
 	public static final fun GlideSubcomposition (Ljava/lang/Object;Landroidx/compose/ui/Modifier;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V
 	public static final fun placeholder (I)Lcom/bumptech/glide/integration/compose/Placeholder;
 	public static final fun placeholder (Landroid/graphics/drawable/Drawable;)Lcom/bumptech/glide/integration/compose/Placeholder;
+	public static final fun placeholder (Landroidx/compose/ui/graphics/painter/Painter;)Lcom/bumptech/glide/integration/compose/Placeholder;
 	public static final fun placeholder (Lkotlin/jvm/functions/Function2;)Lcom/bumptech/glide/integration/compose/Placeholder;
 }
 
diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageErrorTest.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageErrorTest.kt
index 18bb29c5..83c66888 100644
--- a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageErrorTest.kt
+++ b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageErrorTest.kt
@@ -7,6 +7,7 @@ import androidx.compose.ui.test.onNodeWithContentDescription
 import androidx.test.core.app.ApplicationProvider
 import com.bumptech.glide.integration.compose.test.GlideComposeRule
 import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
+import com.bumptech.glide.integration.compose.test.expectDisplayedPainter
 import com.bumptech.glide.integration.compose.test.expectDisplayedResource
 import com.bumptech.glide.integration.compose.test.expectNoDrawable
 import org.junit.Rule
@@ -144,6 +145,25 @@ class GlideImageErrorTest {
   }
 
   @Test
+  fun failure_setViaFailureParameterWithPainter_andRequestBuilderTransform_prefersFailurePainter() {
+    val description = "test"
+    val failurePainter = context.getDrawable(android.R.drawable.star_big_off).toPainter()
+    glideComposeRule.setContent {
+      GlideImage(
+        model = null,
+        contentDescription = description,
+        failure = placeholder(failurePainter),
+      ) {
+        it.error(android.R.drawable.btn_star)
+      }
+    }
+
+    glideComposeRule
+      .onNodeWithContentDescription(description)
+      .assert(expectDisplayedPainter(failurePainter))
+  }
+
+  @Test
   fun failure_setViaFailureParameterWithDrawable_andRequestBuilderTransform_prefersFailureParameter() {
     val description = "test"
     val failureDrawable = context.getDrawable(android.R.drawable.star_big_off)
diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImagePlaceholderTest.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImagePlaceholderTest.kt
index 5e3b36f2..d6c5a3b0 100644
--- a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImagePlaceholderTest.kt
+++ b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImagePlaceholderTest.kt
@@ -7,6 +7,7 @@ import androidx.compose.ui.test.junit4.createComposeRule
 import androidx.compose.ui.test.onNodeWithContentDescription
 import androidx.test.core.app.ApplicationProvider
 import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
+import com.bumptech.glide.integration.compose.test.expectDisplayedPainter
 import com.bumptech.glide.integration.compose.test.expectDisplayedResource
 import com.bumptech.glide.integration.compose.test.expectNoDrawable
 import com.bumptech.glide.testutil.TearDownGlide
@@ -182,6 +183,27 @@ class GlideImagePlaceholderTest {
   }
 
   @Test
+  fun loading_setViaLoadingParameterWithPainter_andRequestBuilderTransform_prefersLoadingParameter() {
+    val description = "test"
+    val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
+    val placeholderDrawable = context.getDrawable(android.R.drawable.star_big_off)
+    val placeholderPainter = placeholderDrawable.toPainter()
+    composeRule.setContent {
+      GlideImage(
+        model = waitModel,
+        contentDescription = description,
+        loading = placeholder(placeholderPainter),
+      ) {
+        it.placeholder(android.R.drawable.btn_star)
+      }
+    }
+
+    composeRule
+      .onNodeWithContentDescription(description)
+      .assert(expectDisplayedPainter(placeholderPainter))
+  }
+
+  @Test
   fun loading_setViaLoadingParameterWithNullDrawable_andRequestBuilderTransform_showsNoResource() {
     val description = "test"
     val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/test/expectations.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/test/expectations.kt
index 08953944..32a991f9 100644
--- a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/test/expectations.kt
+++ b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/test/expectations.kt
@@ -10,10 +10,12 @@ import android.graphics.drawable.BitmapDrawable
 import android.graphics.drawable.Drawable
 import android.util.TypedValue
 import androidx.compose.runtime.MutableState
+import androidx.compose.ui.graphics.painter.Painter
 import androidx.compose.ui.semantics.SemanticsPropertyKey
 import androidx.compose.ui.test.SemanticsMatcher
 import androidx.test.core.app.ApplicationProvider
 import com.bumptech.glide.integration.compose.DisplayedDrawableKey
+import com.bumptech.glide.integration.compose.DisplayedPainterKey
 import com.bumptech.glide.integration.ktx.InternalGlideApi
 import com.bumptech.glide.integration.ktx.Size
 import kotlin.math.roundToInt
@@ -43,10 +45,10 @@ fun expectDisplayedDrawableSize(expectedSize: Size): SemanticsMatcher =
 fun expectDisplayedDrawable(expectedValue: Drawable?): SemanticsMatcher =
   expectDisplayedDrawable(expectedValue.bitmapOrThrow(), ::compareBitmaps) { it.bitmapOrThrow() }
 
-fun expectAnimatingDrawable(): SemanticsMatcher =
-  expectDisplayedDrawable(true) {
-    (it as Animatable).isRunning
-  }
+fun expectDisplayedPainter(expectedValue: Painter?): SemanticsMatcher =
+  expectStateValue(
+    DisplayedPainterKey, expectedValue, {first, second -> first == second}, {value -> value}
+  )
 
 fun expectNoDrawable(): SemanticsMatcher = expectDisplayedDrawable(null)
 
diff --git a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideImage.kt b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideImage.kt
index c963961f..e88d4094 100644
--- a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideImage.kt
+++ b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideImage.kt
@@ -145,6 +145,8 @@ public fun GlideImage(
           alpha,
           colorFilter,
           transition,
+          loadingPlaceholder = loading?.maybePainter(),
+          errorPlaceholder = failure?.maybePainter(),
         )
     )
   }
@@ -167,13 +169,10 @@ public interface GlideSubcompositionScope {
 
 @ExperimentalGlideComposeApi
 internal class GlideSubcompositionScopeImpl(
-  private val drawable: Drawable?,
+  maybePainter: Painter?,
   override val state: RequestState
 ) : GlideSubcompositionScope {
-
-  override val painter: Painter
-    get() = drawable?.toPainter() ?: ColorPainter(Color.Transparent)
-
+  override val painter: Painter = maybePainter ?: ColorPainter(Color.Transparent)
 }
 
 /**
@@ -266,7 +265,7 @@ public fun GlideSubcomposition(
     remember(model, requestManager, requestBuilderTransform) {
       mutableStateOf(RequestState.Loading)
     }
-  val drawable: MutableState<Drawable?> = remember(model, requestManager, requestBuilderTransform) {
+  val painter: MutableState<Painter?> = remember(model, requestManager, requestBuilderTransform) {
     mutableStateOf(null)
   }
 
@@ -274,11 +273,11 @@ public fun GlideSubcomposition(
     remember(model, requestManager, requestBuilderTransform) {
       StateTrackingListener(
         requestState,
-        drawable
+        painter
       )
     }
 
-  val scope = GlideSubcompositionScopeImpl(drawable.value, requestState.value)
+  val scope = GlideSubcompositionScopeImpl(painter.value, requestState.value)
 
   Box(
     modifier
@@ -295,12 +294,12 @@ public fun GlideSubcomposition(
 @ExperimentalGlideComposeApi
 private class StateTrackingListener(
   val state: MutableState<RequestState>,
-  val drawable: MutableState<Drawable?>
+  val painter: MutableState<Painter?>
 ) : RequestListener {
 
-  override fun onStateChanged(model: Any?, drawable: Drawable?, requestState: RequestState) {
+  override fun onStateChanged(model: Any?, painter: Painter?, requestState: RequestState) {
     state.value = requestState
-    this.drawable.value = drawable
+    this.painter.value = painter
   }
 }
 
@@ -311,15 +310,18 @@ private fun PreviewResourceOrDrawable(
   contentDescription: String?,
   modifier: Modifier,
 ) {
-  val drawable =
+  val painter =
     when (loading) {
-      is Placeholder.OfDrawable -> loading.drawable
-      is Placeholder.OfResourceId -> LocalContext.current.getDrawable(loading.resourceId)
+      is Placeholder.OfDrawable -> loading.drawable.toPainter()
+      is Placeholder.OfResourceId ->
+        LocalContext.current.getDrawable(loading.resourceId).toPainter()
+
+      is Placeholder.OfPainter -> loading.painter
       is Placeholder.OfComposable ->
         throw IllegalArgumentException("Composables should go through the production codepath")
     }
   Image(
-    painter = remember(drawable) { drawable.toPainter() },
+    painter = painter,
     modifier = modifier,
     contentDescription = contentDescription,
   )
@@ -349,6 +351,14 @@ public fun placeholder(@DrawableRes resourceId: Int): Placeholder =
   Placeholder.OfResourceId(resourceId)
 
 /**
+ * Used to specify a [Painter] to use in conjunction with [GlideImage]'s `loading` or `failure`
+ * parameters.
+ */
+@ExperimentalGlideComposeApi
+public fun placeholder(painter: Painter?): Placeholder =
+  Placeholder.OfPainter(painter ?: ColorPainter(Color.Transparent))
+
+/**
  * Used to specify a [Composable] function to use in conjunction with [GlideImage]'s `loading` or
  * `failure` parameter.
  *
@@ -380,6 +390,8 @@ public fun placeholder(composable: @Composable () -> Unit): Placeholder =
 public sealed class Placeholder {
   internal class OfDrawable(internal val drawable: Drawable?) : Placeholder()
   internal class OfResourceId(@DrawableRes internal val resourceId: Int) : Placeholder()
+
+  internal class OfPainter(internal val painter: Painter) : Placeholder()
   internal class OfComposable(internal val composable: @Composable () -> Unit) : Placeholder()
 
   internal fun isResourceOrDrawable() =
@@ -387,6 +399,7 @@ public sealed class Placeholder {
       is OfDrawable -> true
       is OfResourceId -> true
       is OfComposable -> false
+      is OfPainter -> false
     }
 
   internal fun maybeComposable(): (@Composable () -> Unit)? =
@@ -395,6 +408,12 @@ public sealed class Placeholder {
       else -> null
     }
 
+  internal fun maybePainter() =
+    when (this) {
+      is OfPainter -> this.painter
+      else -> null
+    }
+
   internal fun <T> apply(
     resource: (Int) -> RequestBuilder<T>,
     drawable: (Drawable?) -> RequestBuilder<T>
diff --git a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideModifier.kt b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideModifier.kt
index 12c5232e..a193f844 100644
--- a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideModifier.kt
+++ b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideModifier.kt
@@ -65,7 +65,7 @@ import kotlin.math.roundToInt
 
 @ExperimentalGlideComposeApi
 internal interface RequestListener {
-  fun onStateChanged(model: Any?, drawable: Drawable?, requestState: RequestState)
+  fun onStateChanged(model: Any?, painter: Painter?, requestState: RequestState)
 }
 
 @ExperimentalGlideComposeApi
@@ -79,6 +79,8 @@ internal fun Modifier.glideNode(
   transitionFactory: Transition.Factory? = null,
   requestListener: RequestListener? = null,
   draw: Boolean? = null,
+  loadingPlaceholder: Painter? = null,
+  errorPlaceholder: Painter? = null,
 ): Modifier {
   return this then GlideNodeElement(
     requestBuilder,
@@ -89,6 +91,8 @@ internal fun Modifier.glideNode(
     requestListener,
     draw,
     transitionFactory,
+    loadingPlaceholder,
+    errorPlaceholder,
   )
     .clipToBounds()
     .semantics {
@@ -110,6 +114,8 @@ internal data class GlideNodeElement constructor(
   private val requestListener: RequestListener?,
   private val draw: Boolean?,
   private val transitionFactory: Transition.Factory?,
+  private val loadingPlaceholder: Painter?,
+  private val errorPlaceholder: Painter?,
 ) : ModifierNodeElement<GlideNode>() {
   override fun create(): GlideNode {
     val result = GlideNode()
@@ -127,6 +133,8 @@ internal data class GlideNodeElement constructor(
       requestListener,
       draw,
       transitionFactory,
+      loadingPlaceholder,
+      errorPlaceholder,
     )
   }
 
@@ -167,10 +175,12 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
   private var requestListener: RequestListener? = null
 
   private var currentJob: Job? = null
-  private var painter: Painter? = null
+  private var primary: Primary? = null
+
+  private var loadingPlaceholder: Painter? = null
+  private var errorPlaceholder: Painter? = null
 
   // Only used for debugging
-  private var drawable: Drawable? = null
   private var state: RequestState = RequestState.Loading
   private var placeholder: Painter? = null
   private var isFirstResource = true
@@ -213,11 +223,18 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
     requestListener: RequestListener?,
     draw: Boolean?,
     transitionFactory: Transition.Factory?,
+    loadingPlaceholder: Painter?,
+    errorPlaceholder: Painter?,
   ) {
     // Other attributes can be refreshed by re-drawing rather than restarting a request
     val restartLoad =
       !this::requestBuilder.isInitialized ||
           requestBuilder != this.requestBuilder
+          // TODO(sam): Avoid restarting the entire load if we just change the placeholder. State
+          // management makes this complicated and this might not be a common use case, so we
+          // haven't yet done so.
+          || loadingPlaceholder != this.loadingPlaceholder
+          || errorPlaceholder != this.errorPlaceholder
 
     this.requestBuilder = requestBuilder
     this.contentScale = contentScale
@@ -227,6 +244,8 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
     this.requestListener = requestListener
     this.draw = draw ?: true
     this.transitionFactory = transitionFactory ?: DoNotTransition.Factory
+    this.loadingPlaceholder = loadingPlaceholder
+    this.errorPlaceholder = errorPlaceholder
     this.resolvableGlideSize =
       requestBuilder.maybeImmediateSize()
         ?: inferredGlideSize?.let { ImmediateGlideSize(it) }
@@ -234,7 +253,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
 
     if (restartLoad) {
       clear()
-      updateDrawable(null)
+      updatePrimary(null)
 
       // If we're not attached, we'll be measured when we eventually are attached.
       if (isAttached) {
@@ -321,7 +340,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
         }
       }
 
-      painter?.let { painter ->
+      primary?.painter?.let { painter ->
         drawContext.canvas.withSave {
           drawablePositionAndSize = drawOne(painter, drawablePositionAndSize) { size ->
             transition.drawCurrent.invoke(this, painter, size, alpha, colorFilter)
@@ -347,7 +366,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
   override fun onReset() {
     super.onReset()
     clear()
-    updateDrawable(null)
+    updatePrimary(null)
   }
 
   @OptIn(ExperimentGlideFlows::class)
@@ -388,27 +407,37 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
         placeholderPositionAndSize = null
 
         requestBuilder.flowResolvable(resolvableGlideSize).collect {
-          val (state, drawable) =
+          val (state, primary) =
             when (it) {
               is Resource -> {
                 maybeAnimate(it)
-                Pair(RequestState.Success(it.dataSource), it.resource)
+                Pair(RequestState.Success(it.dataSource), Primary.PrimaryDrawable(it.resource))
               }
 
               is Placeholder -> {
-                val drawable = it.placeholder
                 val state = when (it.status) {
                   Status.RUNNING, Status.CLEARED -> RequestState.Loading
                   Status.FAILED -> RequestState.Failure
                   Status.SUCCEEDED -> throw IllegalStateException()
                 }
-                placeholder = drawable?.toPainter()
+                // Prefer the override Painters if set.
+                val painter = when (state) {
+                  is RequestState.Loading -> loadingPlaceholder
+                  is RequestState.Failure -> errorPlaceholder
+                  is RequestState.Success -> throw IllegalStateException()
+                }
+                val primary = if (painter != null) {
+                  Primary.PrimaryPainter(painter)
+                } else {
+                  Primary.PrimaryDrawable(it.placeholder)
+                }
+                placeholder = primary.painter
                 placeholderPositionAndSize = null
-                Pair(state, drawable)
+                Pair(state, primary)
               }
             }
-          updateDrawable(drawable)
-          requestListener?.onStateChanged(requestBuilder.internalModel, drawable, state)
+          updatePrimary(primary)
+          requestListener?.onStateChanged(requestBuilder.internalModel, primary.painter, state)
           this@GlideNode.state = state
           if (hasFixedSize) {
             invalidateDraw()
@@ -419,17 +448,40 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
       }
     }
 
-  private fun updateDrawable(drawable: Drawable?) {
-    this.drawable = drawable
+  private sealed class Primary {
+    class PrimaryDrawable(override val drawable: Drawable?) : Primary() {
+      override val painter = drawable?.toPainter()
+      override fun onUnset() {
+        drawable?.callback = null
+        drawable?.setVisible(false, false)
+        (drawable as? Animatable)?.stop()
+      }
+
+      override fun onSet(callback: Drawable.Callback) {
+        drawable?.callback = callback
+        drawable?.setVisible(true, true)
+        (drawable as? Animatable)?.start()
+      }
+    }
 
-    this.drawable?.callback = null
-    this.drawable?.setVisible(false, false)
-    (this.drawable as? Animatable)?.stop()
+    class PrimaryPainter(override val painter: Painter?) : Primary() {
+      override val drawable = null
+      override fun onUnset() {}
+      override fun onSet(callback: Drawable.Callback) {}
+    }
 
-    painter = drawable?.toPainter()
-    drawable?.callback = callback
-    drawable?.setVisible(true, true)
-    (drawable as? Animatable)?.start()
+    abstract val painter: Painter?
+    abstract val drawable: Drawable?
+
+    abstract fun onUnset()
+
+    abstract fun onSet(callback: Drawable.Callback)
+  }
+
+  private fun updatePrimary(newPrimary: Primary?) {
+    this.primary?.onUnset()
+    this.primary = newPrimary
+    newPrimary?.onSet(callback)
     drawablePositionAndSize = null
   }
 
@@ -448,7 +500,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
     currentJob?.cancel()
     currentJob = null
     state = RequestState.Loading
-    updateDrawable(null)
+    updatePrimary(null)
   }
 
   @OptIn(InternalGlideApi::class)
@@ -484,7 +536,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
       )
     }
 
-    val intrinsicSize = painter?.intrinsicSize ?: return constraints
+    val intrinsicSize = primary?.painter?.intrinsicSize ?: return constraints
 
     val intrinsicWidth =
       if (constraints.hasFixedWidth) {
@@ -509,8 +561,8 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
 
     val srcSize = Size(intrinsicWidth.toFloat(), intrinsicHeight.toFloat())
     val scaleFactor = contentScale.computeScaleFactor(
-        srcSize, Size(constrainedWidth.toFloat(), constrainedHeight.toFloat())
-      )
+      srcSize, Size(constrainedWidth.toFloat(), constrainedHeight.toFloat())
+    )
     if (scaleFactor == ScaleFactor.Unspecified) {
       return constraints
     }
@@ -522,7 +574,8 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
   }
 
   override fun SemanticsPropertyReceiver.applySemantics() {
-    displayedDrawable = { drawable }
+    displayedDrawable = { primary?.drawable }
+    displayedPainter = { primary?.painter }
   }
 
   override fun equals(other: Any?): Boolean {
@@ -535,6 +588,8 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
           && draw == other.draw
           && transitionFactory == other.transitionFactory
           && alpha == other.alpha
+          && loadingPlaceholder == other.loadingPlaceholder
+          && errorPlaceholder == other.errorPlaceholder
     }
     return false
   }
@@ -548,6 +603,8 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
     result = 31 * result + requestListener.hashCode()
     result = 31 * result + transitionFactory.hashCode()
     result = 31 * result + alpha.hashCode()
+    result = 31 * result + loadingPlaceholder.hashCode()
+    result = 31 * result + errorPlaceholder.hashCode()
     return result
   }
 }
@@ -555,3 +612,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
 internal val DisplayedDrawableKey =
   SemanticsPropertyKey<() -> Drawable?>("DisplayedDrawable")
 internal var SemanticsPropertyReceiver.displayedDrawable by DisplayedDrawableKey
+
+internal val DisplayedPainterKey =
+  SemanticsPropertyKey<() -> Painter?>("DisplayedPainter")
+internal var SemanticsPropertyReceiver.displayedPainter by DisplayedPainterKey