Skip to content

Commit 9809d98

Browse files
sjuddsjudd
committedSep 26, 2023
Add a Painter variant of the placeholder APIs
·
v5.0.0-rc01
1 parent e8d10fc commit 9809d98

File tree

6 files changed

+171
-46
lines changed

6 files changed

+171
-46
lines changed
 

‎integration/compose/api/compose.api

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ public final class com/bumptech/glide/integration/compose/GlideImageKt {
1919
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
2020
public static final fun placeholder (I)Lcom/bumptech/glide/integration/compose/Placeholder;
2121
public static final fun placeholder (Landroid/graphics/drawable/Drawable;)Lcom/bumptech/glide/integration/compose/Placeholder;
22+
public static final fun placeholder (Landroidx/compose/ui/graphics/painter/Painter;)Lcom/bumptech/glide/integration/compose/Placeholder;
2223
public static final fun placeholder (Lkotlin/jvm/functions/Function2;)Lcom/bumptech/glide/integration/compose/Placeholder;
2324
}
2425

‎integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageErrorTest.kt

+20
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import androidx.compose.ui.test.onNodeWithContentDescription
77
import androidx.test.core.app.ApplicationProvider
88
import com.bumptech.glide.integration.compose.test.GlideComposeRule
99
import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
10+
import com.bumptech.glide.integration.compose.test.expectDisplayedPainter
1011
import com.bumptech.glide.integration.compose.test.expectDisplayedResource
1112
import com.bumptech.glide.integration.compose.test.expectNoDrawable
1213
import org.junit.Rule
@@ -143,6 +144,25 @@ class GlideImageErrorTest {
143144
.assert(expectDisplayedResource(failureResourceId))
144145
}
145146

147+
@Test
148+
fun failure_setViaFailureParameterWithPainter_andRequestBuilderTransform_prefersFailurePainter() {
149+
val description = "test"
150+
val failurePainter = context.getDrawable(android.R.drawable.star_big_off).toPainter()
151+
glideComposeRule.setContent {
152+
GlideImage(
153+
model = null,
154+
contentDescription = description,
155+
failure = placeholder(failurePainter),
156+
) {
157+
it.error(android.R.drawable.btn_star)
158+
}
159+
}
160+
161+
glideComposeRule
162+
.onNodeWithContentDescription(description)
163+
.assert(expectDisplayedPainter(failurePainter))
164+
}
165+
146166
@Test
147167
fun failure_setViaFailureParameterWithDrawable_andRequestBuilderTransform_prefersFailureParameter() {
148168
val description = "test"

‎integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImagePlaceholderTest.kt

+22
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import androidx.compose.ui.test.junit4.createComposeRule
77
import androidx.compose.ui.test.onNodeWithContentDescription
88
import androidx.test.core.app.ApplicationProvider
99
import com.bumptech.glide.integration.compose.test.expectDisplayedDrawable
10+
import com.bumptech.glide.integration.compose.test.expectDisplayedPainter
1011
import com.bumptech.glide.integration.compose.test.expectDisplayedResource
1112
import com.bumptech.glide.integration.compose.test.expectNoDrawable
1213
import com.bumptech.glide.testutil.TearDownGlide
@@ -181,6 +182,27 @@ class GlideImagePlaceholderTest {
181182
.assert(expectDisplayedDrawable(placeholderDrawable))
182183
}
183184

185+
@Test
186+
fun loading_setViaLoadingParameterWithPainter_andRequestBuilderTransform_prefersLoadingParameter() {
187+
val description = "test"
188+
val waitModel = waitModelLoaderRule.waitOn(android.R.drawable.star_big_on)
189+
val placeholderDrawable = context.getDrawable(android.R.drawable.star_big_off)
190+
val placeholderPainter = placeholderDrawable.toPainter()
191+
composeRule.setContent {
192+
GlideImage(
193+
model = waitModel,
194+
contentDescription = description,
195+
loading = placeholder(placeholderPainter),
196+
) {
197+
it.placeholder(android.R.drawable.btn_star)
198+
}
199+
}
200+
201+
composeRule
202+
.onNodeWithContentDescription(description)
203+
.assert(expectDisplayedPainter(placeholderPainter))
204+
}
205+
184206
@Test
185207
fun loading_setViaLoadingParameterWithNullDrawable_andRequestBuilderTransform_showsNoResource() {
186208
val description = "test"

‎integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/test/expectations.kt

+6-4
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import android.graphics.drawable.BitmapDrawable
1010
import android.graphics.drawable.Drawable
1111
import android.util.TypedValue
1212
import androidx.compose.runtime.MutableState
13+
import androidx.compose.ui.graphics.painter.Painter
1314
import androidx.compose.ui.semantics.SemanticsPropertyKey
1415
import androidx.compose.ui.test.SemanticsMatcher
1516
import androidx.test.core.app.ApplicationProvider
1617
import com.bumptech.glide.integration.compose.DisplayedDrawableKey
18+
import com.bumptech.glide.integration.compose.DisplayedPainterKey
1719
import com.bumptech.glide.integration.ktx.InternalGlideApi
1820
import com.bumptech.glide.integration.ktx.Size
1921
import kotlin.math.roundToInt
@@ -43,10 +45,10 @@ fun expectDisplayedDrawableSize(expectedSize: Size): SemanticsMatcher =
4345
fun expectDisplayedDrawable(expectedValue: Drawable?): SemanticsMatcher =
4446
expectDisplayedDrawable(expectedValue.bitmapOrThrow(), ::compareBitmaps) { it.bitmapOrThrow() }
4547

46-
fun expectAnimatingDrawable(): SemanticsMatcher =
47-
expectDisplayedDrawable(true) {
48-
(it as Animatable).isRunning
49-
}
48+
fun expectDisplayedPainter(expectedValue: Painter?): SemanticsMatcher =
49+
expectStateValue(
50+
DisplayedPainterKey, expectedValue, {first, second -> first == second}, {value -> value}
51+
)
5052

5153
fun expectNoDrawable(): SemanticsMatcher = expectDisplayedDrawable(null)
5254

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

+34-15
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,8 @@ public fun GlideImage(
145145
alpha,
146146
colorFilter,
147147
transition,
148+
loadingPlaceholder = loading?.maybePainter(),
149+
errorPlaceholder = failure?.maybePainter(),
148150
)
149151
)
150152
}
@@ -167,13 +169,10 @@ public interface GlideSubcompositionScope {
167169

168170
@ExperimentalGlideComposeApi
169171
internal class GlideSubcompositionScopeImpl(
170-
private val drawable: Drawable?,
172+
maybePainter: Painter?,
171173
override val state: RequestState
172174
) : GlideSubcompositionScope {
173-
174-
override val painter: Painter
175-
get() = drawable?.toPainter() ?: ColorPainter(Color.Transparent)
176-
175+
override val painter: Painter = maybePainter ?: ColorPainter(Color.Transparent)
177176
}
178177

179178
/**
@@ -266,19 +265,19 @@ public fun GlideSubcomposition(
266265
remember(model, requestManager, requestBuilderTransform) {
267266
mutableStateOf(RequestState.Loading)
268267
}
269-
val drawable: MutableState<Drawable?> = remember(model, requestManager, requestBuilderTransform) {
268+
val painter: MutableState<Painter?> = remember(model, requestManager, requestBuilderTransform) {
270269
mutableStateOf(null)
271270
}
272271

273272
val requestListener: StateTrackingListener =
274273
remember(model, requestManager, requestBuilderTransform) {
275274
StateTrackingListener(
276275
requestState,
277-
drawable
276+
painter
278277
)
279278
}
280279

281-
val scope = GlideSubcompositionScopeImpl(drawable.value, requestState.value)
280+
val scope = GlideSubcompositionScopeImpl(painter.value, requestState.value)
282281

283282
Box(
284283
modifier
@@ -295,12 +294,12 @@ public fun GlideSubcomposition(
295294
@ExperimentalGlideComposeApi
296295
private class StateTrackingListener(
297296
val state: MutableState<RequestState>,
298-
val drawable: MutableState<Drawable?>
297+
val painter: MutableState<Painter?>
299298
) : RequestListener {
300299

301-
override fun onStateChanged(model: Any?, drawable: Drawable?, requestState: RequestState) {
300+
override fun onStateChanged(model: Any?, painter: Painter?, requestState: RequestState) {
302301
state.value = requestState
303-
this.drawable.value = drawable
302+
this.painter.value = painter
304303
}
305304
}
306305

@@ -311,15 +310,18 @@ private fun PreviewResourceOrDrawable(
311310
contentDescription: String?,
312311
modifier: Modifier,
313312
) {
314-
val drawable =
313+
val painter =
315314
when (loading) {
316-
is Placeholder.OfDrawable -> loading.drawable
317-
is Placeholder.OfResourceId -> LocalContext.current.getDrawable(loading.resourceId)
315+
is Placeholder.OfDrawable -> loading.drawable.toPainter()
316+
is Placeholder.OfResourceId ->
317+
LocalContext.current.getDrawable(loading.resourceId).toPainter()
318+
319+
is Placeholder.OfPainter -> loading.painter
318320
is Placeholder.OfComposable ->
319321
throw IllegalArgumentException("Composables should go through the production codepath")
320322
}
321323
Image(
322-
painter = remember(drawable) { drawable.toPainter() },
324+
painter = painter,
323325
modifier = modifier,
324326
contentDescription = contentDescription,
325327
)
@@ -348,6 +350,14 @@ public fun placeholder(drawable: Drawable?): Placeholder = Placeholder.OfDrawabl
348350
public fun placeholder(@DrawableRes resourceId: Int): Placeholder =
349351
Placeholder.OfResourceId(resourceId)
350352

353+
/**
354+
* Used to specify a [Painter] to use in conjunction with [GlideImage]'s `loading` or `failure`
355+
* parameters.
356+
*/
357+
@ExperimentalGlideComposeApi
358+
public fun placeholder(painter: Painter?): Placeholder =
359+
Placeholder.OfPainter(painter ?: ColorPainter(Color.Transparent))
360+
351361
/**
352362
* Used to specify a [Composable] function to use in conjunction with [GlideImage]'s `loading` or
353363
* `failure` parameter.
@@ -380,13 +390,16 @@ public fun placeholder(composable: @Composable () -> Unit): Placeholder =
380390
public sealed class Placeholder {
381391
internal class OfDrawable(internal val drawable: Drawable?) : Placeholder()
382392
internal class OfResourceId(@DrawableRes internal val resourceId: Int) : Placeholder()
393+
394+
internal class OfPainter(internal val painter: Painter) : Placeholder()
383395
internal class OfComposable(internal val composable: @Composable () -> Unit) : Placeholder()
384396

385397
internal fun isResourceOrDrawable() =
386398
when (this) {
387399
is OfDrawable -> true
388400
is OfResourceId -> true
389401
is OfComposable -> false
402+
is OfPainter -> false
390403
}
391404

392405
internal fun maybeComposable(): (@Composable () -> Unit)? =
@@ -395,6 +408,12 @@ public sealed class Placeholder {
395408
else -> null
396409
}
397410

411+
internal fun maybePainter() =
412+
when (this) {
413+
is OfPainter -> this.painter
414+
else -> null
415+
}
416+
398417
internal fun <T> apply(
399418
resource: (Int) -> RequestBuilder<T>,
400419
drawable: (Drawable?) -> RequestBuilder<T>

‎integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideModifier.kt

+88-27
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ import kotlin.math.roundToInt
6565

6666
@ExperimentalGlideComposeApi
6767
internal interface RequestListener {
68-
fun onStateChanged(model: Any?, drawable: Drawable?, requestState: RequestState)
68+
fun onStateChanged(model: Any?, painter: Painter?, requestState: RequestState)
6969
}
7070

7171
@ExperimentalGlideComposeApi
@@ -79,6 +79,8 @@ internal fun Modifier.glideNode(
7979
transitionFactory: Transition.Factory? = null,
8080
requestListener: RequestListener? = null,
8181
draw: Boolean? = null,
82+
loadingPlaceholder: Painter? = null,
83+
errorPlaceholder: Painter? = null,
8284
): Modifier {
8385
return this then GlideNodeElement(
8486
requestBuilder,
@@ -89,6 +91,8 @@ internal fun Modifier.glideNode(
8991
requestListener,
9092
draw,
9193
transitionFactory,
94+
loadingPlaceholder,
95+
errorPlaceholder,
9296
)
9397
.clipToBounds()
9498
.semantics {
@@ -110,6 +114,8 @@ internal data class GlideNodeElement constructor(
110114
private val requestListener: RequestListener?,
111115
private val draw: Boolean?,
112116
private val transitionFactory: Transition.Factory?,
117+
private val loadingPlaceholder: Painter?,
118+
private val errorPlaceholder: Painter?,
113119
) : ModifierNodeElement<GlideNode>() {
114120
override fun create(): GlideNode {
115121
val result = GlideNode()
@@ -127,6 +133,8 @@ internal data class GlideNodeElement constructor(
127133
requestListener,
128134
draw,
129135
transitionFactory,
136+
loadingPlaceholder,
137+
errorPlaceholder,
130138
)
131139
}
132140

@@ -167,10 +175,12 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
167175
private var requestListener: RequestListener? = null
168176

169177
private var currentJob: Job? = null
170-
private var painter: Painter? = null
178+
private var primary: Primary? = null
179+
180+
private var loadingPlaceholder: Painter? = null
181+
private var errorPlaceholder: Painter? = null
171182

172183
// Only used for debugging
173-
private var drawable: Drawable? = null
174184
private var state: RequestState = RequestState.Loading
175185
private var placeholder: Painter? = null
176186
private var isFirstResource = true
@@ -213,11 +223,18 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
213223
requestListener: RequestListener?,
214224
draw: Boolean?,
215225
transitionFactory: Transition.Factory?,
226+
loadingPlaceholder: Painter?,
227+
errorPlaceholder: Painter?,
216228
) {
217229
// Other attributes can be refreshed by re-drawing rather than restarting a request
218230
val restartLoad =
219231
!this::requestBuilder.isInitialized ||
220232
requestBuilder != this.requestBuilder
233+
// TODO(sam): Avoid restarting the entire load if we just change the placeholder. State
234+
// management makes this complicated and this might not be a common use case, so we
235+
// haven't yet done so.
236+
|| loadingPlaceholder != this.loadingPlaceholder
237+
|| errorPlaceholder != this.errorPlaceholder
221238

222239
this.requestBuilder = requestBuilder
223240
this.contentScale = contentScale
@@ -227,14 +244,16 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
227244
this.requestListener = requestListener
228245
this.draw = draw ?: true
229246
this.transitionFactory = transitionFactory ?: DoNotTransition.Factory
247+
this.loadingPlaceholder = loadingPlaceholder
248+
this.errorPlaceholder = errorPlaceholder
230249
this.resolvableGlideSize =
231250
requestBuilder.maybeImmediateSize()
232251
?: inferredGlideSize?.let { ImmediateGlideSize(it) }
233252
?: AsyncGlideSize()
234253

235254
if (restartLoad) {
236255
clear()
237-
updateDrawable(null)
256+
updatePrimary(null)
238257

239258
// If we're not attached, we'll be measured when we eventually are attached.
240259
if (isAttached) {
@@ -321,7 +340,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
321340
}
322341
}
323342

324-
painter?.let { painter ->
343+
primary?.painter?.let { painter ->
325344
drawContext.canvas.withSave {
326345
drawablePositionAndSize = drawOne(painter, drawablePositionAndSize) { size ->
327346
transition.drawCurrent.invoke(this, painter, size, alpha, colorFilter)
@@ -347,7 +366,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
347366
override fun onReset() {
348367
super.onReset()
349368
clear()
350-
updateDrawable(null)
369+
updatePrimary(null)
351370
}
352371

353372
@OptIn(ExperimentGlideFlows::class)
@@ -388,27 +407,37 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
388407
placeholderPositionAndSize = null
389408

390409
requestBuilder.flowResolvable(resolvableGlideSize).collect {
391-
val (state, drawable) =
410+
val (state, primary) =
392411
when (it) {
393412
is Resource -> {
394413
maybeAnimate(it)
395-
Pair(RequestState.Success(it.dataSource), it.resource)
414+
Pair(RequestState.Success(it.dataSource), Primary.PrimaryDrawable(it.resource))
396415
}
397416

398417
is Placeholder -> {
399-
val drawable = it.placeholder
400418
val state = when (it.status) {
401419
Status.RUNNING, Status.CLEARED -> RequestState.Loading
402420
Status.FAILED -> RequestState.Failure
403421
Status.SUCCEEDED -> throw IllegalStateException()
404422
}
405-
placeholder = drawable?.toPainter()
423+
// Prefer the override Painters if set.
424+
val painter = when (state) {
425+
is RequestState.Loading -> loadingPlaceholder
426+
is RequestState.Failure -> errorPlaceholder
427+
is RequestState.Success -> throw IllegalStateException()
428+
}
429+
val primary = if (painter != null) {
430+
Primary.PrimaryPainter(painter)
431+
} else {
432+
Primary.PrimaryDrawable(it.placeholder)
433+
}
434+
placeholder = primary.painter
406435
placeholderPositionAndSize = null
407-
Pair(state, drawable)
436+
Pair(state, primary)
408437
}
409438
}
410-
updateDrawable(drawable)
411-
requestListener?.onStateChanged(requestBuilder.internalModel, drawable, state)
439+
updatePrimary(primary)
440+
requestListener?.onStateChanged(requestBuilder.internalModel, primary.painter, state)
412441
this@GlideNode.state = state
413442
if (hasFixedSize) {
414443
invalidateDraw()
@@ -419,17 +448,40 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
419448
}
420449
}
421450

422-
private fun updateDrawable(drawable: Drawable?) {
423-
this.drawable = drawable
451+
private sealed class Primary {
452+
class PrimaryDrawable(override val drawable: Drawable?) : Primary() {
453+
override val painter = drawable?.toPainter()
454+
override fun onUnset() {
455+
drawable?.callback = null
456+
drawable?.setVisible(false, false)
457+
(drawable as? Animatable)?.stop()
458+
}
459+
460+
override fun onSet(callback: Drawable.Callback) {
461+
drawable?.callback = callback
462+
drawable?.setVisible(true, true)
463+
(drawable as? Animatable)?.start()
464+
}
465+
}
424466

425-
this.drawable?.callback = null
426-
this.drawable?.setVisible(false, false)
427-
(this.drawable as? Animatable)?.stop()
467+
class PrimaryPainter(override val painter: Painter?) : Primary() {
468+
override val drawable = null
469+
override fun onUnset() {}
470+
override fun onSet(callback: Drawable.Callback) {}
471+
}
428472

429-
painter = drawable?.toPainter()
430-
drawable?.callback = callback
431-
drawable?.setVisible(true, true)
432-
(drawable as? Animatable)?.start()
473+
abstract val painter: Painter?
474+
abstract val drawable: Drawable?
475+
476+
abstract fun onUnset()
477+
478+
abstract fun onSet(callback: Drawable.Callback)
479+
}
480+
481+
private fun updatePrimary(newPrimary: Primary?) {
482+
this.primary?.onUnset()
483+
this.primary = newPrimary
484+
newPrimary?.onSet(callback)
433485
drawablePositionAndSize = null
434486
}
435487

@@ -448,7 +500,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
448500
currentJob?.cancel()
449501
currentJob = null
450502
state = RequestState.Loading
451-
updateDrawable(null)
503+
updatePrimary(null)
452504
}
453505

454506
@OptIn(InternalGlideApi::class)
@@ -484,7 +536,7 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
484536
)
485537
}
486538

487-
val intrinsicSize = painter?.intrinsicSize ?: return constraints
539+
val intrinsicSize = primary?.painter?.intrinsicSize ?: return constraints
488540

489541
val intrinsicWidth =
490542
if (constraints.hasFixedWidth) {
@@ -509,8 +561,8 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
509561

510562
val srcSize = Size(intrinsicWidth.toFloat(), intrinsicHeight.toFloat())
511563
val scaleFactor = contentScale.computeScaleFactor(
512-
srcSize, Size(constrainedWidth.toFloat(), constrainedHeight.toFloat())
513-
)
564+
srcSize, Size(constrainedWidth.toFloat(), constrainedHeight.toFloat())
565+
)
514566
if (scaleFactor == ScaleFactor.Unspecified) {
515567
return constraints
516568
}
@@ -522,7 +574,8 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
522574
}
523575

524576
override fun SemanticsPropertyReceiver.applySemantics() {
525-
displayedDrawable = { drawable }
577+
displayedDrawable = { primary?.drawable }
578+
displayedPainter = { primary?.painter }
526579
}
527580

528581
override fun equals(other: Any?): Boolean {
@@ -535,6 +588,8 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
535588
&& draw == other.draw
536589
&& transitionFactory == other.transitionFactory
537590
&& alpha == other.alpha
591+
&& loadingPlaceholder == other.loadingPlaceholder
592+
&& errorPlaceholder == other.errorPlaceholder
538593
}
539594
return false
540595
}
@@ -548,10 +603,16 @@ internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifi
548603
result = 31 * result + requestListener.hashCode()
549604
result = 31 * result + transitionFactory.hashCode()
550605
result = 31 * result + alpha.hashCode()
606+
result = 31 * result + loadingPlaceholder.hashCode()
607+
result = 31 * result + errorPlaceholder.hashCode()
551608
return result
552609
}
553610
}
554611

555612
internal val DisplayedDrawableKey =
556613
SemanticsPropertyKey<() -> Drawable?>("DisplayedDrawable")
557614
internal var SemanticsPropertyReceiver.displayedDrawable by DisplayedDrawableKey
615+
616+
internal val DisplayedPainterKey =
617+
SemanticsPropertyKey<() -> Painter?>("DisplayedPainter")
618+
internal var SemanticsPropertyReceiver.displayedPainter by DisplayedPainterKey

0 commit comments

Comments
 (0)
Please sign in to comment.