diff --git a/integration/compose/api/compose.api b/integration/compose/api/compose.api index f4f4f6d59d..9ceb583721 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 18bb29c5d9..83c6688850 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 @@ -143,6 +144,25 @@ class GlideImageErrorTest { .assert(expectDisplayedResource(failureResourceId)) } + @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" 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 5e3b36f2b6..c8db05ba31 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 @@ -180,6 +181,26 @@ class GlideImagePlaceholderTest { .onNodeWithContentDescription(description) .assert(expectDisplayedDrawable(placeholderDrawable)) } + @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() { 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 0895394473..32a991f9c7 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 c963961f79..d7188eedf0 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 = remember(model, requestManager, requestBuilderTransform) { + val painter: MutableState = 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, - val drawable: MutableState + val painter: MutableState ) : 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,20 +310,21 @@ 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, ) } - /** * Used to specify a [Drawable] to use in conjunction with [GlideImage]'s `loading` or `failure` * parameters. @@ -348,6 +348,14 @@ public fun placeholder(drawable: Drawable?): Placeholder = Placeholder.OfDrawabl 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 +388,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 +397,7 @@ public sealed class Placeholder { is OfDrawable -> true is OfResourceId -> true is OfComposable -> false + is OfPainter -> false } internal fun maybeComposable(): (@Composable () -> Unit)? = @@ -395,6 +406,12 @@ public sealed class Placeholder { else -> null } + internal fun maybePainter() = + when (this) { + is OfPainter -> this.painter + else -> null + } + internal fun apply( resource: (Int) -> RequestBuilder, drawable: (Drawable?) -> RequestBuilder 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 12c5232e17..90a8e1722b 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() { override fun create(): GlideNode { val result = GlideNode() @@ -127,6 +133,8 @@ internal data class GlideNodeElement constructor( requestListener, draw, transitionFactory, + loadingPlaceholder, + errorPlaceholder, ) } @@ -167,12 +175,14 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi private var requestListener: RequestListener? = null private var currentJob: Job? = null - private var painter: Painter? = null + private var primary: GlideNode.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 currentPlaceholder: Painter? = null private var isFirstResource = true // Avoid allocating Point/PointFs during draw @@ -213,6 +223,8 @@ 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 = @@ -227,6 +239,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,13 +248,18 @@ 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) { launchRequest(requestBuilder) } } else { + when(state) { + is RequestState.Loading -> currentPlaceholder = loadingPlaceholder + is RequestState.Failure -> currentPlaceholder = errorPlaceholder + is RequestState.Success -> {} // Do nothing + } invalidateDraw() } } @@ -313,7 +332,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi override fun ContentDrawScope.draw() { if (draw) { val drawPlaceholder = transition.drawPlaceholder ?: DoNotTransition.drawPlaceholder - placeholder?.let { painter -> + currentPlaceholder?.let { painter -> drawContext.canvas.withSave { placeholderPositionAndSize = drawOne(painter, placeholderPositionAndSize) { size -> drawPlaceholder.invoke(this, painter, size, alpha, colorFilter) @@ -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) @@ -384,31 +403,40 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi } Preconditions.checkArgument(currentJob == null) currentJob = (coroutineScope + Dispatchers.Main.immediate).launch { - placeholder = null + currentPlaceholder = loadingPlaceholder 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) + } 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 +447,39 @@ 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 unset() { + drawable?.callback = null + drawable?.setVisible(false, false) + (drawable as? Animatable)?.stop() + } - this.drawable?.callback = null - this.drawable?.setVisible(false, false) - (this.drawable as? Animatable)?.stop() + override fun set(callback: Drawable.Callback) { + drawable?.callback = callback + drawable?.setVisible(true, true) + (drawable as? Animatable)?.start() + } + } + class PrimaryPainter(override val painter: Painter?) : Primary() { + override val drawable = null + override fun unset() {} + override fun set(callback: Drawable.Callback) {} + } + + abstract val painter: Painter? + abstract val drawable: Drawable? + + abstract fun unset() - painter = drawable?.toPainter() - drawable?.callback = callback - drawable?.setVisible(true, true) - (drawable as? Animatable)?.start() + abstract fun set(callback: Drawable.Callback) + } + + private fun updatePrimary(newPrimary: Primary?) { + this.primary?.unset() + this.primary = newPrimary; + newPrimary?.set(callback) drawablePositionAndSize = null } @@ -448,7 +498,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi currentJob?.cancel() currentJob = null state = RequestState.Loading - updateDrawable(null) + updatePrimary(null) } @OptIn(InternalGlideApi::class) @@ -484,7 +534,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) { @@ -522,7 +572,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 +586,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 +601,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 +610,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