Skip to content

Commit 60b567e

Browse files
sjuddsjudd
committed
Remove GlidePainter in favor of Modifier nodes / Flows
Using a custom Modifier node instead of a Painter follows a recommendation from the Compose team. It will allow us or library users to compose Glide's modifier in the future for animations of other effects. For now we do not actually expose the Modifier directly. This change adds a GlideSubcomposition that uses' Glide's flows integration to allow subcomposition based on image load state. Subcomposition allows us to deprecate the expensive Composable placeholder variants and also allows users to implement complex layouts or animations. In a subsequent change we'll explore adding a default crossfade and/or exposing the Glide modifier node so that users of the library can compose modifiers themselves.
1 parent 8f6d645 commit 60b567e

File tree

31 files changed

+1071
-360
lines changed

31 files changed

+1071
-360
lines changed

‎benchmark/src/androidTest/java/com/bumptech/glide/benchmark/BenchmarkFromCache.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import android.app.Application;
44
import android.graphics.Bitmap;
5+
import androidx.annotation.NonNull;
56
import androidx.annotation.Nullable;
67
import androidx.annotation.RawRes;
78
import androidx.benchmark.BenchmarkState;
@@ -111,17 +112,17 @@ private void loadImageWithExpectedDataSource(
111112
public boolean onLoadFailed(
112113
@Nullable GlideException e,
113114
Object model,
114-
Target<Bitmap> target,
115+
@NonNull Target<Bitmap> target,
115116
boolean isFirstResource) {
116117
return false;
117118
}
118119

119120
@Override
120121
public boolean onResourceReady(
121-
Bitmap resource,
122-
Object model,
122+
@NonNull Bitmap resource,
123+
@NonNull Object model,
123124
Target<Bitmap> target,
124-
DataSource dataSource,
125+
@NonNull DataSource dataSource,
125126
boolean isFirstResource) {
126127
dataSourceRef.set(dataSource);
127128
return false;

‎instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,17 @@ public void thumbnail_onResourceReady_forPrimary_isComplete_whenRequestListenerI
6868
public boolean onLoadFailed(
6969
@Nullable GlideException e,
7070
Object model,
71-
Target<Drawable> target,
71+
@NonNull Target<Drawable> target,
7272
boolean isFirstResource) {
7373
return false;
7474
}
7575

7676
@Override
7777
public boolean onResourceReady(
78-
Drawable resource,
79-
Object model,
78+
@NonNull Drawable resource,
79+
@NonNull Object model,
8080
Target<Drawable> target,
81-
DataSource dataSource,
81+
@NonNull DataSource dataSource,
8282
boolean isFirstResource) {
8383
isPrimaryRequestComplete.set(target.getRequest().isComplete());
8484
countDownLatch.countDown();
@@ -115,7 +115,7 @@ public void thumbnail_onLoadFailed_forPrimary_isNotRunningOrComplete_whenRequest
115115
public boolean onLoadFailed(
116116
@Nullable GlideException e,
117117
Object model,
118-
Target<Drawable> target,
118+
@NonNull Target<Drawable> target,
119119
boolean isFirstResource) {
120120
Request request = target.getRequest();
121121
isNeitherRunningNorComplete.set(!request.isComplete() && !request.isRunning());
@@ -125,10 +125,10 @@ public boolean onLoadFailed(
125125

126126
@Override
127127
public boolean onResourceReady(
128-
Drawable resource,
129-
Object model,
128+
@NonNull Drawable resource,
129+
@NonNull Object model,
130130
Target<Drawable> target,
131-
DataSource dataSource,
131+
@NonNull DataSource dataSource,
132132
boolean isFirstResource) {
133133
return false;
134134
}

‎integration/compose/api/compose.api

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ public abstract interface annotation class com/bumptech/glide/integration/compos
33

44
public final class com/bumptech/glide/integration/compose/GlideImageKt {
55
public static final fun GlideImage (Ljava/lang/Object;Ljava/lang/String;Landroidx/compose/ui/Modifier;Landroidx/compose/ui/Alignment;Landroidx/compose/ui/layout/ContentScale;FLandroidx/compose/ui/graphics/ColorFilter;Lcom/bumptech/glide/integration/compose/Placeholder;Lcom/bumptech/glide/integration/compose/Placeholder;Lkotlin/jvm/functions/Function1;Landroidx/compose/runtime/Composer;II)V
6+
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
67
public static final fun placeholder (I)Lcom/bumptech/glide/integration/compose/Placeholder;
78
public static final fun placeholder (Landroid/graphics/drawable/Drawable;)Lcom/bumptech/glide/integration/compose/Placeholder;
89
public static final fun placeholder (Lkotlin/jvm/functions/Function2;)Lcom/bumptech/glide/integration/compose/Placeholder;
@@ -13,6 +14,11 @@ public abstract interface class com/bumptech/glide/integration/compose/GlidePrel
1314
public abstract fun getSize ()I
1415
}
1516

17+
public abstract interface class com/bumptech/glide/integration/compose/GlideSubcompositionScope {
18+
public abstract fun getPainter ()Landroidx/compose/ui/graphics/painter/Painter;
19+
public abstract fun getState ()Lcom/bumptech/glide/integration/compose/RequestState;
20+
}
21+
1622
public abstract class com/bumptech/glide/integration/compose/Placeholder {
1723
public static final field $stable I
1824
}
@@ -22,3 +28,29 @@ public final class com/bumptech/glide/integration/compose/PreloadKt {
2228
public static final fun rememberGlidePreloadingData-u6VnWhU (ILkotlin/jvm/functions/Function1;JILjava/lang/Integer;Lkotlin/jvm/functions/Function2;Landroidx/compose/runtime/Composer;II)Lcom/bumptech/glide/integration/compose/GlidePreloadingData;
2329
}
2430

31+
public abstract class com/bumptech/glide/integration/compose/RequestState {
32+
public static final field $stable I
33+
}
34+
35+
public final class com/bumptech/glide/integration/compose/RequestState$Failure : com/bumptech/glide/integration/compose/RequestState {
36+
public static final field $stable I
37+
public static final field INSTANCE Lcom/bumptech/glide/integration/compose/RequestState$Failure;
38+
}
39+
40+
public final class com/bumptech/glide/integration/compose/RequestState$Loading : com/bumptech/glide/integration/compose/RequestState {
41+
public static final field $stable I
42+
public static final field INSTANCE Lcom/bumptech/glide/integration/compose/RequestState$Loading;
43+
}
44+
45+
public final class com/bumptech/glide/integration/compose/RequestState$Success : com/bumptech/glide/integration/compose/RequestState {
46+
public static final field $stable I
47+
public fun <init> (Lcom/bumptech/glide/load/DataSource;)V
48+
public final fun component1 ()Lcom/bumptech/glide/load/DataSource;
49+
public final fun copy (Lcom/bumptech/glide/load/DataSource;)Lcom/bumptech/glide/integration/compose/RequestState$Success;
50+
public static synthetic fun copy$default (Lcom/bumptech/glide/integration/compose/RequestState$Success;Lcom/bumptech/glide/load/DataSource;ILjava/lang/Object;)Lcom/bumptech/glide/integration/compose/RequestState$Success;
51+
public fun equals (Ljava/lang/Object;)Z
52+
public final fun getDataSource ()Lcom/bumptech/glide/load/DataSource;
53+
public fun hashCode ()I
54+
public fun toString ()Ljava/lang/String;
55+
}
56+

‎integration/compose/build.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ plugins {
55

66
android {
77
namespace 'com.bumptech.glide.integration.compose'
8-
compileSdk 33
8+
compileSdk 34
99

1010
defaultConfig {
1111
minSdk 21
12-
targetSdk 33
12+
targetSdk 34
1313

1414
testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
1515
}

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
@file:OptIn(ExperimentalGlideComposeApi::class, ExperimentalCoroutinesApi::class)
2-
31
package com.bumptech.glide.integration.compose
42

53
import android.graphics.Canvas
@@ -16,7 +14,6 @@ import com.bumptech.glide.integration.compose.test.Constants
1614
import com.bumptech.glide.integration.compose.test.GlideComposeRule
1715
import com.bumptech.glide.integration.compose.test.assertDisplaysInstance
1816
import com.bumptech.glide.integration.compose.test.onNodeWithDefaultContentDescription
19-
import kotlinx.coroutines.ExperimentalCoroutinesApi
2017
import kotlinx.coroutines.test.runTest
2118
import org.junit.Rule
2219
import org.junit.Test
@@ -28,6 +25,7 @@ import org.junit.runners.Parameterized
2825
*
2926
* Transformable types are tested in [GlideImageDefaultTransformationTest].
3027
*/
28+
@OptIn(ExperimentalGlideComposeApi::class)
3129
@RunWith(Parameterized::class)
3230
class GlideImageCustomDrawableTransformationTest(
3331
private val contentScale: ContentScale,
@@ -98,8 +96,8 @@ class GlideImageCustomDrawableTransformationTest(
9896
@Suppress("DeprecatedCallableAddReplaceWith")
9997
private open class FakeDrawable : Drawable() {
10098
override fun draw(p0: Canvas) {}
101-
override fun setAlpha(p0: Int) = throw UnsupportedOperationException()
102-
override fun setColorFilter(p0: ColorFilter?) = throw UnsupportedOperationException()
99+
override fun setAlpha(p0: Int) {}
100+
override fun setColorFilter(p0: ColorFilter?) {}
103101
@Deprecated("Deprecated in Java")
104102
override fun getOpacity(): Int = throw UnsupportedOperationException()
105103
}

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
package com.bumptech.glide.integration.compose
88

99
import android.content.Context
10-
import android.content.res.Resources
1110
import android.graphics.drawable.Drawable
12-
import android.util.TypedValue
1311
import androidx.annotation.DrawableRes
1412
import androidx.compose.foundation.layout.size
1513
import androidx.compose.runtime.Composable
@@ -28,7 +26,6 @@ import com.bumptech.glide.integration.ktx.ExperimentGlideFlows
2826
import com.bumptech.glide.integration.ktx.Resource
2927
import com.bumptech.glide.integration.ktx.Status
3028
import com.bumptech.glide.integration.ktx.flow
31-
import kotlin.math.roundToInt
3229
import kotlinx.coroutines.ExperimentalCoroutinesApi
3330
import kotlinx.coroutines.flow.first
3431
import kotlinx.coroutines.test.runTest

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

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
@file:OptIn(ExperimentalGlideComposeApi::class)
2-
31
package com.bumptech.glide.integration.compose
42

53
import android.content.Context
@@ -18,9 +16,12 @@ import org.junit.Test
1816
* Avoids [com.bumptech.glide.load.engine.executor.GlideIdlingResourceInit] because we want to make
1917
* assertions about loads that have not yet completed.
2018
*/
19+
@OptIn(ExperimentalGlideComposeApi::class)
20+
@Suppress("DEPRECATION") // Tests for a deprecated method
2121
class GlideImageErrorTest {
2222
private val context: Context = ApplicationProvider.getApplicationContext()
23-
@get:Rule val glideComposeRule = GlideComposeRule()
23+
@get:Rule
24+
val glideComposeRule = GlideComposeRule()
2425

2526
@Test
2627
fun requestBuilderTransform_withErrorResourceId_displaysError() {
@@ -105,16 +106,16 @@ class GlideImageErrorTest {
105106
model = null,
106107
contentDescription = "none",
107108
failure =
108-
placeholder {
109-
// Nesting GlideImage is not really a good idea, but it's convenient for this test
110-
// because
111-
// we can use our helpers to assert on its contents.
112-
GlideImage(
113-
model = null,
114-
contentDescription = description,
115-
failure = placeholder(failureResourceId),
116-
)
117-
}
109+
placeholder {
110+
// Nesting GlideImage is not really a good idea, but it's convenient for this test
111+
// because
112+
// we can use our helpers to assert on its contents.
113+
GlideImage(
114+
model = null,
115+
contentDescription = description,
116+
failure = placeholder(failureResourceId),
117+
)
118+
}
118119
)
119120
}
120121

@@ -186,13 +187,13 @@ class GlideImageErrorTest {
186187
model = null,
187188
contentDescription = "other",
188189
failure =
189-
placeholder {
190-
GlideImage(
191-
model = null,
192-
contentDescription = description,
193-
failure = placeholder(failureResourceId),
194-
)
195-
},
190+
placeholder {
191+
GlideImage(
192+
model = null,
193+
contentDescription = description,
194+
failure = placeholder(failureResourceId),
195+
)
196+
},
196197
) {
197198
it.error(android.R.drawable.btn_star)
198199
}

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

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
@file:OptIn(ExperimentalGlideComposeApi::class)
2-
31
package com.bumptech.glide.integration.compose
42

53
import android.content.Context
@@ -21,11 +19,16 @@ import org.junit.Test
2119
* [com.bumptech.glide.integration.compose.test.GlideComposeRule] because we want to make assertions
2220
* about loads that have not yet completed.
2321
*/
22+
@Suppress("DEPRECATION") // Tests for a deprecated method
23+
@OptIn(ExperimentalGlideComposeApi::class)
2424
class GlideImagePlaceholderTest {
2525
private val context: Context = ApplicationProvider.getApplicationContext()
26-
@get:Rule(order = 1) val composeRule = createComposeRule()
27-
@get:Rule(order = 2) val waitModelLoaderRule = WaitModelLoaderRule()
28-
@get:Rule(order = 3) val tearDownGlide = TearDownGlide()
26+
@get:Rule(order = 1)
27+
val composeRule = createComposeRule()
28+
@get:Rule(order = 2)
29+
val waitModelLoaderRule = WaitModelLoaderRule()
30+
@get:Rule(order = 3)
31+
val tearDownGlide = TearDownGlide()
2932

3033
@Test
3134
fun requestBuilderTransform_withPlaceholderResourceId_displaysPlaceholder() {
@@ -120,16 +123,16 @@ class GlideImagePlaceholderTest {
120123
model = waitModel,
121124
contentDescription = "none",
122125
loading =
123-
placeholder {
124-
// Nesting GlideImage is not really a good idea, but it's convenient for this test
125-
// because
126-
// we can use our helpers to assert on its contents.
127-
GlideImage(
128-
model = waitModel,
129-
contentDescription = description,
130-
loading = placeholder(placeholderResourceId),
131-
)
132-
}
126+
placeholder {
127+
// Nesting GlideImage is not really a good idea, but it's convenient for this test
128+
// because
129+
// we can use our helpers to assert on its contents.
130+
GlideImage(
131+
model = waitModel,
132+
contentDescription = description,
133+
loading = placeholder(placeholderResourceId),
134+
)
135+
}
133136
)
134137
}
135138

@@ -205,13 +208,13 @@ class GlideImagePlaceholderTest {
205208
model = waitModel,
206209
contentDescription = "other",
207210
loading =
208-
placeholder {
209-
GlideImage(
210-
model = waitModel,
211-
contentDescription = description,
212-
loading = placeholder(placeholderResourceId),
213-
)
214-
},
211+
placeholder {
212+
GlideImage(
213+
model = waitModel,
214+
contentDescription = description,
215+
loading = placeholder(placeholderResourceId),
216+
)
217+
},
215218
) {
216219
it.placeholder(android.R.drawable.btn_star)
217220
}

0 commit comments

Comments
 (0)