From 60b567e5feec70ba810a663e56a10d837f26c56c Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Sun, 2 Jul 2023 09:18:12 -0700 Subject: [PATCH] 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. --- .../glide/benchmark/BenchmarkFromCache.java | 9 +- .../com/bumptech/glide/MultiRequestTest.java | 16 +- integration/compose/api/compose.api | 32 ++ integration/compose/build.gradle | 4 +- ...deImageCustomDrawableTransformationTest.kt | 8 +- .../GlideImageDefaultTransformationTest.kt | 3 - .../compose/GlideImageErrorTest.kt | 41 +- .../compose/GlideImagePlaceholderTest.kt | 47 +-- .../integration/compose/GlideImageTest.kt | 67 +++- .../compose/GlideSubcompositionTest.kt | 166 ++++++++ .../integration/compose/test/expectations.kt | 6 +- .../java/com/bumptech/glide/ModelExtractor.kt | 4 + .../glide/integration/compose/GlideImage.kt | 336 ++++++++++++---- .../integration/compose/GlideModifier.kt | 362 ++++++++++++++++++ .../glide/integration/compose/GlidePainter.kt | 132 ------- .../glide/integration/compose/Sizes.kt | 20 + .../integration/concurrent/GlideFutures.java | 4 +- integration/ktx/api/ktx.api | 18 +- .../bumptech/glide/integration/ktx/Flows.kt | 46 ++- .../glide/integration/ktx/FlowsTest.kt | 16 +- .../com/bumptech/glide/RequestBuilder.java | 4 + .../glide/request/RequestFutureTarget.java | 4 +- .../glide/request/RequestListener.java | 20 +- .../java/com/bumptech/glide/GlideTest.java | 8 +- .../bumptech/glide/RequestBuilderTest.java | 17 +- .../glide/request/SingleRequestTest.java | 16 +- samples/gallery/build.gradle | 1 + .../gallery/HorizontalGalleryFragment.kt | 2 +- .../samples/giphy/FullscreenActivity.java | 9 +- .../samples/svg/SvgSoftwareLayerSetter.java | 9 +- settings.gradle | 4 +- 31 files changed, 1071 insertions(+), 360 deletions(-) create mode 100644 integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideSubcompositionTest.kt create mode 100644 integration/compose/src/main/java/com/bumptech/glide/ModelExtractor.kt create mode 100644 integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideModifier.kt delete mode 100644 integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlidePainter.kt diff --git a/benchmark/src/androidTest/java/com/bumptech/glide/benchmark/BenchmarkFromCache.java b/benchmark/src/androidTest/java/com/bumptech/glide/benchmark/BenchmarkFromCache.java index 9b33ebd158..e6b5b94dfe 100644 --- a/benchmark/src/androidTest/java/com/bumptech/glide/benchmark/BenchmarkFromCache.java +++ b/benchmark/src/androidTest/java/com/bumptech/glide/benchmark/BenchmarkFromCache.java @@ -2,6 +2,7 @@ import android.app.Application; import android.graphics.Bitmap; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.RawRes; import androidx.benchmark.BenchmarkState; @@ -111,17 +112,17 @@ private void loadImageWithExpectedDataSource( public boolean onLoadFailed( @Nullable GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { return false; } @Override public boolean onResourceReady( - Bitmap resource, - Object model, + @NonNull Bitmap resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { dataSourceRef.set(dataSource); return false; diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java index b04cc220fe..cd5948b8ad 100644 --- a/instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/MultiRequestTest.java @@ -68,17 +68,17 @@ public void thumbnail_onResourceReady_forPrimary_isComplete_whenRequestListenerI public boolean onLoadFailed( @Nullable GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { return false; } @Override public boolean onResourceReady( - Drawable resource, - Object model, + @NonNull Drawable resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { isPrimaryRequestComplete.set(target.getRequest().isComplete()); countDownLatch.countDown(); @@ -115,7 +115,7 @@ public void thumbnail_onLoadFailed_forPrimary_isNotRunningOrComplete_whenRequest public boolean onLoadFailed( @Nullable GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { Request request = target.getRequest(); isNeitherRunningNorComplete.set(!request.isComplete() && !request.isRunning()); @@ -125,10 +125,10 @@ public boolean onLoadFailed( @Override public boolean onResourceReady( - Drawable resource, - Object model, + @NonNull Drawable resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { return false; } diff --git a/integration/compose/api/compose.api b/integration/compose/api/compose.api index 5185e1f008..ff27488b34 100644 --- a/integration/compose/api/compose.api +++ b/integration/compose/api/compose.api @@ -3,6 +3,7 @@ public abstract interface annotation class com/bumptech/glide/integration/compos public final class com/bumptech/glide/integration/compose/GlideImageKt { 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 + 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 (Lkotlin/jvm/functions/Function2;)Lcom/bumptech/glide/integration/compose/Placeholder; @@ -13,6 +14,11 @@ public abstract interface class com/bumptech/glide/integration/compose/GlidePrel public abstract fun getSize ()I } +public abstract interface class com/bumptech/glide/integration/compose/GlideSubcompositionScope { + public abstract fun getPainter ()Landroidx/compose/ui/graphics/painter/Painter; + public abstract fun getState ()Lcom/bumptech/glide/integration/compose/RequestState; +} + public abstract class com/bumptech/glide/integration/compose/Placeholder { public static final field $stable I } @@ -22,3 +28,29 @@ public final class com/bumptech/glide/integration/compose/PreloadKt { 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; } +public abstract class com/bumptech/glide/integration/compose/RequestState { + public static final field $stable I +} + +public final class com/bumptech/glide/integration/compose/RequestState$Failure : com/bumptech/glide/integration/compose/RequestState { + public static final field $stable I + public static final field INSTANCE Lcom/bumptech/glide/integration/compose/RequestState$Failure; +} + +public final class com/bumptech/glide/integration/compose/RequestState$Loading : com/bumptech/glide/integration/compose/RequestState { + public static final field $stable I + public static final field INSTANCE Lcom/bumptech/glide/integration/compose/RequestState$Loading; +} + +public final class com/bumptech/glide/integration/compose/RequestState$Success : com/bumptech/glide/integration/compose/RequestState { + public static final field $stable I + public fun (Lcom/bumptech/glide/load/DataSource;)V + public final fun component1 ()Lcom/bumptech/glide/load/DataSource; + public final fun copy (Lcom/bumptech/glide/load/DataSource;)Lcom/bumptech/glide/integration/compose/RequestState$Success; + 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; + public fun equals (Ljava/lang/Object;)Z + public final fun getDataSource ()Lcom/bumptech/glide/load/DataSource; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + diff --git a/integration/compose/build.gradle b/integration/compose/build.gradle index 9c6b6ebaff..e879c9216e 100644 --- a/integration/compose/build.gradle +++ b/integration/compose/build.gradle @@ -5,11 +5,11 @@ plugins { android { namespace 'com.bumptech.glide.integration.compose' - compileSdk 33 + compileSdk 34 defaultConfig { minSdk 21 - targetSdk 33 + targetSdk 34 testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageCustomDrawableTransformationTest.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageCustomDrawableTransformationTest.kt index a12312f2aa..e51dbec7cd 100644 --- a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageCustomDrawableTransformationTest.kt +++ b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageCustomDrawableTransformationTest.kt @@ -1,5 +1,3 @@ -@file:OptIn(ExperimentalGlideComposeApi::class, ExperimentalCoroutinesApi::class) - package com.bumptech.glide.integration.compose import android.graphics.Canvas @@ -16,7 +14,6 @@ import com.bumptech.glide.integration.compose.test.Constants import com.bumptech.glide.integration.compose.test.GlideComposeRule import com.bumptech.glide.integration.compose.test.assertDisplaysInstance import com.bumptech.glide.integration.compose.test.onNodeWithDefaultContentDescription -import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test @@ -28,6 +25,7 @@ import org.junit.runners.Parameterized * * Transformable types are tested in [GlideImageDefaultTransformationTest]. */ +@OptIn(ExperimentalGlideComposeApi::class) @RunWith(Parameterized::class) class GlideImageCustomDrawableTransformationTest( private val contentScale: ContentScale, @@ -98,8 +96,8 @@ class GlideImageCustomDrawableTransformationTest( @Suppress("DeprecatedCallableAddReplaceWith") private open class FakeDrawable : Drawable() { override fun draw(p0: Canvas) {} - override fun setAlpha(p0: Int) = throw UnsupportedOperationException() - override fun setColorFilter(p0: ColorFilter?) = throw UnsupportedOperationException() + override fun setAlpha(p0: Int) {} + override fun setColorFilter(p0: ColorFilter?) {} @Deprecated("Deprecated in Java") override fun getOpacity(): Int = throw UnsupportedOperationException() } diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageDefaultTransformationTest.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageDefaultTransformationTest.kt index d97e4ade03..03cebd061e 100644 --- a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageDefaultTransformationTest.kt +++ b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageDefaultTransformationTest.kt @@ -7,9 +7,7 @@ package com.bumptech.glide.integration.compose import android.content.Context -import android.content.res.Resources import android.graphics.drawable.Drawable -import android.util.TypedValue import androidx.annotation.DrawableRes import androidx.compose.foundation.layout.size import androidx.compose.runtime.Composable @@ -28,7 +26,6 @@ import com.bumptech.glide.integration.ktx.ExperimentGlideFlows import com.bumptech.glide.integration.ktx.Resource import com.bumptech.glide.integration.ktx.Status import com.bumptech.glide.integration.ktx.flow -import kotlin.math.roundToInt import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest 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 939edf7aea..18bb29c5d9 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 @@ -1,5 +1,3 @@ -@file:OptIn(ExperimentalGlideComposeApi::class) - package com.bumptech.glide.integration.compose import android.content.Context @@ -18,9 +16,12 @@ import org.junit.Test * Avoids [com.bumptech.glide.load.engine.executor.GlideIdlingResourceInit] because we want to make * assertions about loads that have not yet completed. */ +@OptIn(ExperimentalGlideComposeApi::class) +@Suppress("DEPRECATION") // Tests for a deprecated method class GlideImageErrorTest { private val context: Context = ApplicationProvider.getApplicationContext() - @get:Rule val glideComposeRule = GlideComposeRule() + @get:Rule + val glideComposeRule = GlideComposeRule() @Test fun requestBuilderTransform_withErrorResourceId_displaysError() { @@ -105,16 +106,16 @@ class GlideImageErrorTest { model = null, contentDescription = "none", failure = - placeholder { - // Nesting GlideImage is not really a good idea, but it's convenient for this test - // because - // we can use our helpers to assert on its contents. - GlideImage( - model = null, - contentDescription = description, - failure = placeholder(failureResourceId), - ) - } + placeholder { + // Nesting GlideImage is not really a good idea, but it's convenient for this test + // because + // we can use our helpers to assert on its contents. + GlideImage( + model = null, + contentDescription = description, + failure = placeholder(failureResourceId), + ) + } ) } @@ -186,13 +187,13 @@ class GlideImageErrorTest { model = null, contentDescription = "other", failure = - placeholder { - GlideImage( - model = null, - contentDescription = description, - failure = placeholder(failureResourceId), - ) - }, + placeholder { + GlideImage( + model = null, + contentDescription = description, + failure = placeholder(failureResourceId), + ) + }, ) { it.error(android.R.drawable.btn_star) } 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 eac66cef0e..5e3b36f2b6 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 @@ -1,5 +1,3 @@ -@file:OptIn(ExperimentalGlideComposeApi::class) - package com.bumptech.glide.integration.compose import android.content.Context @@ -21,11 +19,16 @@ import org.junit.Test * [com.bumptech.glide.integration.compose.test.GlideComposeRule] because we want to make assertions * about loads that have not yet completed. */ +@Suppress("DEPRECATION") // Tests for a deprecated method +@OptIn(ExperimentalGlideComposeApi::class) class GlideImagePlaceholderTest { private val context: Context = ApplicationProvider.getApplicationContext() - @get:Rule(order = 1) val composeRule = createComposeRule() - @get:Rule(order = 2) val waitModelLoaderRule = WaitModelLoaderRule() - @get:Rule(order = 3) val tearDownGlide = TearDownGlide() + @get:Rule(order = 1) + val composeRule = createComposeRule() + @get:Rule(order = 2) + val waitModelLoaderRule = WaitModelLoaderRule() + @get:Rule(order = 3) + val tearDownGlide = TearDownGlide() @Test fun requestBuilderTransform_withPlaceholderResourceId_displaysPlaceholder() { @@ -120,16 +123,16 @@ class GlideImagePlaceholderTest { model = waitModel, contentDescription = "none", loading = - placeholder { - // Nesting GlideImage is not really a good idea, but it's convenient for this test - // because - // we can use our helpers to assert on its contents. - GlideImage( - model = waitModel, - contentDescription = description, - loading = placeholder(placeholderResourceId), - ) - } + placeholder { + // Nesting GlideImage is not really a good idea, but it's convenient for this test + // because + // we can use our helpers to assert on its contents. + GlideImage( + model = waitModel, + contentDescription = description, + loading = placeholder(placeholderResourceId), + ) + } ) } @@ -205,13 +208,13 @@ class GlideImagePlaceholderTest { model = waitModel, contentDescription = "other", loading = - placeholder { - GlideImage( - model = waitModel, - contentDescription = description, - loading = placeholder(placeholderResourceId), - ) - }, + placeholder { + GlideImage( + model = waitModel, + contentDescription = description, + loading = placeholder(placeholderResourceId), + ) + }, ) { it.placeholder(android.R.drawable.btn_star) } diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageTest.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageTest.kt index 2080fa7cfa..7873de4e23 100644 --- a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageTest.kt +++ b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageTest.kt @@ -1,22 +1,25 @@ -@file:OptIn(ExperimentalGlideComposeApi::class, InternalGlideApi::class) - package com.bumptech.glide.integration.compose import android.content.Context import android.graphics.drawable.Drawable +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.size +import androidx.compose.foundation.lazy.LazyRow import androidx.compose.material.Text import androidx.compose.material.TextButton import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.platform.testTag import androidx.compose.ui.test.assert +import androidx.compose.ui.test.hasTestTag import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performScrollToIndex import androidx.compose.ui.unit.dp import androidx.test.core.app.ApplicationProvider import com.bumptech.glide.Glide @@ -38,9 +41,12 @@ import java.util.concurrent.atomic.AtomicReference import org.junit.Rule import org.junit.Test +@OptIn(ExperimentalGlideComposeApi::class, InternalGlideApi::class) class GlideImageTest { private val context: Context = ApplicationProvider.getApplicationContext() - @get:Rule val glideComposeRule = GlideComposeRule() + + @get:Rule + val glideComposeRule = GlideComposeRule() @Test fun glideImage_noModifierSize_resourceDrawable_displaysDrawable() { @@ -100,6 +106,11 @@ class GlideImageTest { } } + // Precondition - ensure we loaded the first drawable. + glideComposeRule + .onNodeWithContentDescription(description) + .assert(expectDisplayedDrawable(firstDrawable)) + glideComposeRule.waitForIdle() glideComposeRule.onNodeWithText("Swap").performClick() glideComposeRule.waitForIdle() @@ -173,7 +184,6 @@ class GlideImageTest { .assertDisplays(null) } - @Test fun glideImage_withNegativeSize_doesNotStartLoad() { val description = "test" @@ -295,17 +305,17 @@ class GlideImageTest { override fun onLoadFailed( e: GlideException?, model: Any?, - target: Target?, + target: Target, isFirstResource: Boolean, ): Boolean { throw UnsupportedOperationException() } override fun onResourceReady( - resource: Drawable?, - model: Any?, - target: Target?, - dataSource: DataSource?, + resource: Drawable, + model: Any, + target: Target, + dataSource: DataSource, isFirstResource: Boolean, ): Boolean { onResourceReadyCounter.incrementAndGet() @@ -323,4 +333,43 @@ class GlideImageTest { assertThat(onResourceReadyCounter.get()).isEqualTo(1) } + + @Test + fun glideImage_whenDetachedAndReattached_rendersImage() { + val description = "test" + val testTag = "testTag" + glideComposeRule.setContent { + LazyRow( + horizontalArrangement = Arrangement.spacedBy(10.dp), + modifier = Modifier.testTag(testTag) + ) { + items(3) { + GlideImage( + model = android.R.drawable.star_big_on, + contentDescription = description + it, + modifier = Modifier.fillParentMaxSize() + ) + } + } + } + + // Scroll back and forth to trigger re-use of the GlideImages with the same + // parameters. + for (i in 0..2) { + glideComposeRule.onNode(hasTestTag(testTag)).performScrollToIndex(i) + glideComposeRule.waitForIdle() + } + glideComposeRule.onNode(hasTestTag(testTag)).performScrollToIndex(0) + glideComposeRule.waitForIdle() + + val drawable = context.getDrawable(android.R.drawable.star_big_on) + // Make sure that all images are rendered + for (i in 0..2) { + glideComposeRule.onNode(hasTestTag(testTag)).performScrollToIndex(i) + glideComposeRule.waitForIdle() + glideComposeRule + .onNodeWithContentDescription(description + i) + .assert(expectDisplayedDrawable(drawable)) + } + } } diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideSubcompositionTest.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideSubcompositionTest.kt new file mode 100644 index 0000000000..9084194c6c --- /dev/null +++ b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideSubcompositionTest.kt @@ -0,0 +1,166 @@ +package com.bumptech.glide.integration.compose + +import android.content.Context +import androidx.compose.foundation.Image +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.size +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp +import androidx.test.core.app.ApplicationProvider +import com.bumptech.glide.Glide +import com.bumptech.glide.integration.compose.test.GlideComposeRule +import com.bumptech.glide.load.DataSource +import com.google.common.truth.Truth.assertThat +import org.junit.Rule +import org.junit.Test + +@OptIn(ExperimentalGlideComposeApi::class) +class GlideSubcompositionTest { + val context: Context = ApplicationProvider.getApplicationContext() + + @get:Rule + val glideComposeRule = GlideComposeRule() + + @Test + fun glideSubcomposition_withoutSize_startsWithStateLoading() { + var currentState: RequestState? = null + glideComposeRule.setContent { + GlideSubcomposition(model = android.R.drawable.star_big_on) { + if (currentState == null) { + currentState = state + } + } + } + assertThat(currentState).isEqualTo(RequestState.Loading) + } + + @Test + fun glideSubcomposition_withOverrideSize_loadsImage() { + var currentState: RequestState? = null + glideComposeRule.setContent { + GlideSubcomposition( + android.R.drawable.star_big_on, + requestBuilderTransform = { it.override(50) } + ) { + currentState = state + } + } + glideComposeRule.waitForIdle() + assertThat(currentState).isInstanceOf(RequestState.Success::class.java) + } + + @Test + fun glideSubcomposition_whenDrawnWithSize_loadsImage() { + var currentState: RequestState? = null + glideComposeRule.setContent { + GlideSubcomposition(model = android.R.drawable.star_big_on) { + currentState = state + Image( + painter = painter, + contentDescription = "", + ) + } + } + glideComposeRule.waitForIdle() + assertThat(currentState).isInstanceOf(RequestState.Success::class.java) + } + + @Test + fun glideSubcomposition_withLayoutSize_startsWithStateLoading() { + var currentState: RequestState? = null + glideComposeRule.setContent { + GlideSubcomposition(model = android.R.drawable.star_big_on, Modifier.size(10.dp)) { + if (currentState == null) { + currentState = state + } + Image( + painter = painter, + contentDescription = "", + ) + } + } + assertThat(currentState).isEqualTo(RequestState.Loading) + } + + @Test + fun glideSubcomposition_withLayoutSize_appliedToBox_loadsImage() { + var currentState: RequestState? = null + glideComposeRule.setContent { + GlideSubcomposition(model = android.R.drawable.star_big_on, Modifier.size(10.dp)) { + currentState = state + Box(Modifier.size(10.dp)) + } + } + glideComposeRule.waitForIdle() + assertThat(currentState).isInstanceOf(RequestState.Success::class.java) + } + + @Test + fun glideSubcomposition_withOverrideSize_andInvalidImage_setsStateToFailed() { + var currentState: RequestState? = null + glideComposeRule.setContent { + GlideSubcomposition(model = 1234, requestBuilderTransform = { it.override(50) }) { + currentState = state + } + } + glideComposeRule.waitForIdle() + assertThat(currentState).isEqualTo(RequestState.Failure) + } + + @Test + fun glideSubcomposition_withLayoutSize_andInvalidImage_setsStateToFailed() { + var currentState: RequestState? = null + glideComposeRule.setContent { + GlideSubcomposition(model = 1234, Modifier.size(10.dp)) { + currentState = state + } + } + glideComposeRule.waitForIdle() + assertThat(currentState).isEqualTo(RequestState.Failure) + } + + @Test + fun glideSubcomposition_onLoadFromSource_setsDataSourceToSource() { + var dataSource: DataSource? = null + glideComposeRule.setContent { + GlideSubcomposition( + model = android.R.drawable.star_big_on, + requestBuilderTransform = { it.override(50) } + ) { + val currentState = state + if (currentState is RequestState.Success) { + dataSource = currentState.dataSource + } + } + } + glideComposeRule.waitForIdle() + assertThat(dataSource).isEqualTo(DataSource.LOCAL) + } + + @Test + fun glideSubcomposition_onLoadFromMemory_setsDataSourceToMemory() { + var dataSource: DataSource? = null + val resourceId = android.R.drawable.star_big_on + val overrideSize = 50 + // TODO: Compose always uses the generic paths to load models, so it skips options that are + // set by default by Glide's various class specific .load() method overrides. + val future = Glide.with(context).load(resourceId as Any).override(overrideSize).submit() + glideComposeRule.waitForIdle() + future.get() + glideComposeRule.setContent { + GlideSubcomposition( + model = resourceId, + requestBuilderTransform = { it.override(overrideSize) } + ) { + val currentState = state + if (currentState is RequestState.Success) { + dataSource = currentState.dataSource + } + } + } + + glideComposeRule.waitForIdle() + assertThat(dataSource).isEqualTo(DataSource.MEMORY_CACHE) + } +} + 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 d409f21288..620502eeaf 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 @@ -62,13 +62,13 @@ private fun expectDisplayedDrawable( expectStateValue(DisplayedDrawableKey, expectedValue, compare) { transform(it) } private fun expectStateValue( - key: SemanticsPropertyKey>, + key: SemanticsPropertyKey<() -> ValueT?>, expectedValue: TransformedValueT, compare: (TransformedValueT?, TransformedValueT?) -> Boolean, transform: (ValueT?) -> TransformedValueT?, ): SemanticsMatcher = SemanticsMatcher("${key.name} = '$expectedValue'") { - val value = transform(it.config.getOrElseNullable(key) { null }?.value) + val value = transform(it.config.getOrElseNullable(key) { null }?.invoke()) if (!compare(value, expectedValue)) { throw AssertionError("Expected: $expectedValue, but was: $value") } @@ -77,7 +77,7 @@ private fun expectStateValue( fun expectSameInstance(expectedDrawable: Drawable) = SemanticsMatcher("${DisplayedDrawableKey.name} = '$expectedDrawable'") { - val actualValue: Drawable? = it.config.getOrElseNullable(DisplayedDrawableKey) { null }?.value + val actualValue: Drawable? = it.config.getOrElseNullable(DisplayedDrawableKey) { null }?.invoke() if (actualValue !== expectedDrawable) { throw AssertionError("Expected: $expectedDrawable, but was: $actualValue") } diff --git a/integration/compose/src/main/java/com/bumptech/glide/ModelExtractor.kt b/integration/compose/src/main/java/com/bumptech/glide/ModelExtractor.kt new file mode 100644 index 0000000000..504b760edd --- /dev/null +++ b/integration/compose/src/main/java/com/bumptech/glide/ModelExtractor.kt @@ -0,0 +1,4 @@ +package com.bumptech.glide + +internal val RequestBuilder<*>.internalModel + get() = model \ No newline at end of file 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 d6c9a6726c..67b00706ed 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 @@ -1,33 +1,42 @@ package com.bumptech.glide.integration.compose +import android.graphics.drawable.BitmapDrawable +import android.graphics.drawable.ColorDrawable import android.graphics.drawable.Drawable import androidx.annotation.DrawableRes import androidx.compose.foundation.Image import androidx.compose.foundation.layout.Box import androidx.compose.runtime.Composable -import androidx.compose.runtime.MutableState +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.ColorFilter import androidx.compose.ui.graphics.DefaultAlpha +import androidx.compose.ui.graphics.asImageBitmap +import androidx.compose.ui.graphics.painter.BitmapPainter +import androidx.compose.ui.graphics.painter.ColorPainter +import androidx.compose.ui.graphics.painter.Painter import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.layout.Layout +import androidx.compose.ui.layout.layout import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalInspectionMode -import androidx.compose.ui.semantics.SemanticsPropertyKey -import androidx.compose.ui.semantics.SemanticsPropertyReceiver -import androidx.compose.ui.semantics.semantics import com.bumptech.glide.Glide import com.bumptech.glide.RequestBuilder import com.bumptech.glide.RequestManager import com.bumptech.glide.integration.ktx.AsyncGlideSize import com.bumptech.glide.integration.ktx.ExperimentGlideFlows +import com.bumptech.glide.integration.ktx.GlideFlowInstant import com.bumptech.glide.integration.ktx.ImmediateGlideSize import com.bumptech.glide.integration.ktx.InternalGlideApi import com.bumptech.glide.integration.ktx.ResolvableGlideSize -import com.bumptech.glide.integration.ktx.Size +import com.bumptech.glide.integration.ktx.Resource import com.bumptech.glide.integration.ktx.Status +import com.bumptech.glide.integration.ktx.flowResolvable +import com.bumptech.glide.load.DataSource +import com.google.accompanist.drawablepainter.DrawablePainter import com.google.accompanist.drawablepainter.rememberDrawablePainter /** Mutates and returns the given [RequestBuilder] to apply relevant options. */ @@ -104,8 +113,6 @@ public fun GlideImage( .let { loading?.apply(it::placeholder, it::placeholder) ?: it } .let { failure?.apply(it::error, it::error) ?: it } - val overrideSize: Size? = requestBuilder.overrideSize() - 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 @@ -115,18 +122,216 @@ public fun GlideImage( return } - SizedGlideImage( - requestBuilder = requestBuilder, - size = size, - modifier = modifier, - contentDescription = contentDescription, - alignment = alignment, - contentScale = contentScale, - alpha = alpha, - colorFilter = colorFilter, - placeholder = loading?.maybeComposable(), - failure = failure?.maybeComposable(), - ) + // TODO(sam): Remove this branch when GlideSubcomposition has been out for a bit. + val loadingComposable = loading?.maybeComposable() + val failureComposable = failure?.maybeComposable() + if (loadingComposable != null || failureComposable != null) { + GlideSubcomposition(model, modifier, { requestBuilder }) { + if (state == RequestState.Loading && loadingComposable != null) { + loadingComposable() + } else if (state == RequestState.Failure && failureComposable != null) { + failureComposable() + } else { + Image( + painter, + contentDescription, + modifier, + alignment, + contentScale, + alpha, + colorFilter + ) + } + } + } else { + val size: ImmediateGlideSize? = requestBuilder.overrideSize()?.let { ImmediateGlideSize(it) } + ModifierGlideImage( + requestBuilder, + size, + contentDescription, + modifier, + alignment, + contentScale, + alpha, + colorFilter, + ) + } +} + +/** + * Provides the current state of the request and a [Painter] to draw it. + */ +@ExperimentalGlideComposeApi +public interface GlideSubcompositionScope { + /** The current state of the request, slightly simplified over Glide's standard request state. */ + public val state: RequestState + + /** + * A painter that will draw the placeholder or resource matching the current request state. If no + * placeholder or resource is available currently, the painter will draw transparent. + */ + public val painter: Painter +} + +@OptIn(ExperimentGlideFlows::class) +@ExperimentalGlideComposeApi +internal class GlideSubcompositionScopeImpl( + private val value: GlideFlowInstant?, +) : GlideSubcompositionScope { + + override val painter: Painter + get() = value?.drawable()?.toPainter() ?: ColorPainter(Color.Transparent) + + override val state: RequestState + get() = when (val current = value) { + is com.bumptech.glide.integration.ktx.Placeholder -> { + when (current.status) { + Status.CLEARED -> RequestState.Loading + Status.RUNNING -> RequestState.Loading + Status.FAILED -> RequestState.Failure + Status.SUCCEEDED -> throw IllegalStateException() + } + } + + is Resource -> RequestState.Success(current.dataSource) + null -> RequestState.Loading + } + + private fun GlideFlowInstant.drawable(): Drawable? = when (this) { + is com.bumptech.glide.integration.ktx.Placeholder -> placeholder + is Resource -> resource + } + + private fun Drawable.toPainter(): Painter = + when (this) { + is BitmapDrawable -> BitmapPainter(bitmap.asImageBitmap()) + is ColorDrawable -> ColorPainter(Color(color)) + else -> DrawablePainter(mutate()) + } +} + +@OptIn(InternalGlideApi::class) +private fun Modifier.sizeObservingModifier(size: ResolvableGlideSize): Modifier = + this.layout { measurable, constraints -> + if (size is AsyncGlideSize) { + val inferredSize = constraints.inferredGlideSize() + if (inferredSize != null) { + size.setSize(inferredSize) + } + } + val placeable = measurable.measure(constraints) + layout(placeable.width, placeable.height) { placeable.place(0, 0) } + } + +/** + * The current state of a request associated with a Glide painter. + * + * This state is a bit of a simplification over Glide's real state. In particular [Success] is + * used in any case where we have an image, even if that image is the thumbnail of a full request + * where the full request has failed. From the point of view of the UI this is usually reasonable + * and a significant simplification of this API. + */ +@ExperimentalGlideComposeApi +public sealed class RequestState { + + @ExperimentalGlideComposeApi + public object Loading : RequestState() + + /** + * Indicates the load finished successfully (or at least one thumbnail was loaded, see the details + * on [RequestState]). + * + * @param dataSource The data source the latest image was loaded from. If your request uses one + * or more thumbnails this value may change as each successive thumbnail is loaded. + */ + @ExperimentalGlideComposeApi + public data class Success( + val dataSource: DataSource, + ) : RequestState() + + @ExperimentalGlideComposeApi + public object Failure : RequestState() +} + +/** + * Starts an image load with Glide, exposing the state of the load via [GlideSubcompositionScope] + * to allow complex subcompositions or animations that depend on the load's state. + * + * [GlideImage] is significantly more efficient and easier to use than this method. GlideImage + * should be preferred over GlideSubcomposition whenever possible. Using GlideSubcomposition in a + * scrolling list will cause multiple recompositions per image, significantly degrading performance. + * The use case for this method is as a fallback for cases where you cannot animate or compose your + * layout without knowing the status of the image load request. + * + * All that said, you can use this class to display custom placeholders and/or animations. For + * example to start an animation when a load completes, you might do something like: + * + * ``` + * GlideSubcomposition(model = uri, modifier) { + * when (state) { + * RequestState.Loading -> ShowLoadingUi() + * RequestState.Failure -> ShowFailureUi() + * is RequestState.Success -> { + * if (state.dataSource != DataSource.MEMORY_CACHE) { + * ShowSomeComplexAnimation() + * } else { + * DoSomethingNormal() + * } + * } + * } + * } + * ``` + * + * [RequestState.Success] contains the [DataSource] where the image was loaded from so that you can + * avoid animating or otherwise change your composition if the image was loaded from the memory + * cache. Typically you do not want to animate loads from the memory cache. + * + * If your [requestBuilderTransform] does not have an [overrideSize] set, this method will wrap your + * subcomposition in [Box] and use the size of that `Box` to determine the + * size to use when loading the image. The box's modifier will be set to the [modifier] you provide. + * As with [GlideImage] try to ensure that you either set a reasonable [RequestBuilder.override] + * size using [requestBuilderTransform] or that you provide a [modifier] that will cause this + * composition to go through layout with a reasonable size. Failing to do so may result in the image + * load never starting, or in an unreasonably large amount of memory being used. Loading overly + * large images in memory can also impact scrolling performance. + */ +@OptIn(InternalGlideApi::class, ExperimentGlideFlows::class) +@ExperimentalGlideComposeApi +@Composable +public fun GlideSubcomposition( + model: Any?, + modifier: Modifier = Modifier, + requestBuilderTransform: RequestBuilderTransform = { it }, + content: @Composable GlideSubcompositionScope.() -> Unit +) { + val requestManager: RequestManager = LocalContext.current.let { remember(it) { Glide.with(it) } } + val requestBuilder = + remember(model, requestManager, requestBuilderTransform) { + requestBuilderTransform(requestManager.load(model)) + } + + val overrideSize = requestBuilder.overrideSize() + val size = remember(overrideSize) { + if (overrideSize != null) { + ImmediateGlideSize(overrideSize) + } else { + AsyncGlideSize() + } + } + + val result = remember(requestBuilder, size) { + requestBuilder.flowResolvable(size) + }.collectAsState(initial = null) + + val scope = GlideSubcompositionScopeImpl(result.value) + + if (overrideSize != null) { + scope.content() + } else { + Box(modifier = modifier.sizeObservingModifier(size)) { + scope.content() + } + } } @OptIn(ExperimentalGlideComposeApi::class) @@ -137,7 +342,7 @@ private fun PreviewResourceOrDrawable( modifier: Modifier, ) { val drawable = - when(loading) { + when (loading) { is Placeholder.OfDrawable -> loading.drawable is Placeholder.OfResourceId -> LocalContext.current.getDrawable(loading.resourceId) is Placeholder.OfComposable -> @@ -180,6 +385,11 @@ public fun placeholder(@DrawableRes resourceId: Int): Placeholder = * Providing a nested [GlideImage] is not recommended. Use [RequestBuilder.thumbnail] or * [RequestBuilder.error] as an alternative. */ +@Deprecated( + "Using this method forces recomposition when the image load state changes." + + " If you require this behavior use GlideSubcomposition instead", + level = DeprecationLevel.WARNING +) @ExperimentalGlideComposeApi public fun placeholder(composable: @Composable () -> Unit): Placeholder = Placeholder.OfComposable(composable) @@ -227,15 +437,6 @@ public sealed class Placeholder { } } -@OptIn(InternalGlideApi::class) -@Composable -private fun rememberResolvableSize( - overrideSize: Size?, -) = - remember(overrideSize) { - overrideSize?.let { ImmediateGlideSize(it) } ?: AsyncGlideSize() - } - @Composable private fun rememberRequestBuilderWithDefaults( model: Any?, @@ -254,6 +455,7 @@ private fun RequestBuilder.contentScaleTransform( ContentScale.Crop -> { optionalCenterCrop() } + ContentScale.Inside, ContentScale.Fit -> { // Outside compose, glide would use fitCenter() for FIT. But that's probably not a good @@ -262,6 +464,7 @@ private fun RequestBuilder.contentScaleTransform( // centerInside(). The UI can still scale the view even if the Bitmap is smaller. optionalCenterInside() } + else -> { this } @@ -269,66 +472,31 @@ private fun RequestBuilder.contentScaleTransform( // TODO(judds): Think about how to handle the various fills } -@OptIn(InternalGlideApi::class, ExperimentGlideFlows::class) +@OptIn(InternalGlideApi::class, ExperimentalGlideComposeApi::class) @Composable -private fun SizedGlideImage( +private fun ModifierGlideImage( requestBuilder: RequestBuilder, - size: ResolvableGlideSize, - modifier: Modifier, + size: ImmediateGlideSize?, contentDescription: String?, + modifier: Modifier, alignment: Alignment, contentScale: ContentScale, alpha: Float, - colorFilter: ColorFilter?, - placeholder: @Composable (() -> Unit)?, - failure: @Composable (() -> Unit)?, + colorFilter: ColorFilter? ) { - // Use a Box so we can infer the size if the request doesn't have an explicit size. - @Composable fun @Composable () -> Unit.boxed() = Box(modifier = modifier) { this@boxed() } - - val painter = - rememberGlidePainter( - requestBuilder = requestBuilder, - size = size, - ) - if (placeholder != null && painter.status.showPlaceholder()) { - placeholder.boxed() - } else if (failure != null && painter.status == Status.FAILED) { - failure.boxed() - } else { - Image( - painter = painter, - contentDescription = contentDescription, - alignment = alignment, - contentScale = contentScale, - alpha = alpha, - colorFilter = colorFilter, - modifier = modifier.then(Modifier.semantics { displayedDrawable = painter.currentDrawable }) - ) + Layout( + {}, + modifier + .glideNode( + requestBuilder, + size, + contentDescription, + alignment, + contentScale, + alpha, + colorFilter, + ) + ) { _, constraints -> + layout(constraints.minWidth, constraints.minHeight) {} } } - -@OptIn(ExperimentGlideFlows::class) -private fun Status.showPlaceholder(): Boolean = - when (this) { - Status.RUNNING -> true - Status.CLEARED -> true - else -> false - } - -@OptIn(InternalGlideApi::class) -@Composable -private fun rememberGlidePainter( - requestBuilder: RequestBuilder, - size: ResolvableGlideSize, -): GlidePainter { - val scope = rememberCoroutineScope() - // TODO(judds): Calling onRemembered here manually might make a minor improvement in how quickly - // the image load is started, but it also triggers a recomposition. I can't figure out why it - // triggers a recomposition - return remember(requestBuilder, size) { GlidePainter(requestBuilder, size, scope) } -} - -internal val DisplayedDrawableKey = - SemanticsPropertyKey>("DisplayedDrawable") -internal var SemanticsPropertyReceiver.displayedDrawable by DisplayedDrawableKey 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 new file mode 100644 index 0000000000..e1ed7c6ecc --- /dev/null +++ b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideModifier.kt @@ -0,0 +1,362 @@ +package com.bumptech.glide.integration.compose + +import android.graphics.PointF +import android.graphics.drawable.Drawable +import androidx.compose.ui.Alignment +import androidx.compose.ui.ExperimentalComposeUiApi +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.alpha +import androidx.compose.ui.draw.clipToBounds +import androidx.compose.ui.geometry.Size +import androidx.compose.ui.graphics.ColorFilter +import androidx.compose.ui.graphics.DefaultAlpha +import androidx.compose.ui.graphics.asAndroidColorFilter +import androidx.compose.ui.graphics.drawscope.ContentDrawScope +import androidx.compose.ui.graphics.drawscope.translate +import androidx.compose.ui.graphics.nativeCanvas +import androidx.compose.ui.layout.ContentScale +import androidx.compose.ui.layout.Measurable +import androidx.compose.ui.layout.MeasureResult +import androidx.compose.ui.layout.MeasureScope +import androidx.compose.ui.layout.times +import androidx.compose.ui.node.DrawModifierNode +import androidx.compose.ui.node.LayoutModifierNode +import androidx.compose.ui.node.ModifierNodeElement +import androidx.compose.ui.node.SemanticsModifierNode +import androidx.compose.ui.node.invalidateDraw +import androidx.compose.ui.node.invalidateMeasurement +import androidx.compose.ui.platform.InspectorInfo +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.SemanticsPropertyKey +import androidx.compose.ui.semantics.SemanticsPropertyReceiver +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.role +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.unit.Constraints +import androidx.compose.ui.unit.IntOffset +import androidx.compose.ui.unit.IntSize +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 +import com.bumptech.glide.integration.ktx.Resource +import com.bumptech.glide.integration.ktx.Status +import com.bumptech.glide.integration.ktx.flowResolvable +import com.bumptech.glide.internalModel +import com.bumptech.glide.util.Preconditions +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import kotlinx.coroutines.plus +import kotlin.math.roundToInt + +@ExperimentalGlideComposeApi +@OptIn(InternalGlideApi::class) +internal fun Modifier.glideNode( + requestBuilder: RequestBuilder, + size: ImmediateGlideSize?, + contentDescription: String?, + alignment: Alignment, + contentScale: ContentScale, + alpha: Float = DefaultAlpha, + colorFilter: ColorFilter?, +): Modifier = + this then GlideNodeElement( + requestBuilder, + size, + contentScale, + alignment, + colorFilter, + ) then + clipToBounds() then + alpha(alpha) then + if (contentDescription != null) { + semantics { + this@semantics.contentDescription = contentDescription + role = Role.Image + } + } else { + Modifier + } + +@ExperimentalGlideComposeApi +@OptIn(InternalGlideApi::class) +internal data class GlideNodeElement constructor( + private val requestBuilder: RequestBuilder, + private val fixedSize: ImmediateGlideSize?, + private val contentScale: ContentScale, + private val alignment: Alignment, + private val colorFilter: ColorFilter?, +) : ModifierNodeElement() { + override fun create(): GlideNode { + val result = GlideNode() + result.onNewRequest( + requestBuilder, + fixedSize, + contentScale, + alignment, + colorFilter, + ) + return result + } + + override fun update(node: GlideNode) { + node.onNewRequest( + requestBuilder, + fixedSize, + contentScale, + alignment, + colorFilter, + ) + } + + override fun InspectorInfo.inspectableProperties() { + name = "GlideNode" + properties["model"] = requestBuilder.internalModel + properties["size"] = fixedSize ?: "LayoutBased" + properties["alignment"] = alignment + properties["contentScale"] = contentScale + properties["colorFilter"] = colorFilter + } +} + +@ExperimentalGlideComposeApi +@OptIn(InternalGlideApi::class) +internal class GlideNode : DrawModifierNode, LayoutModifierNode, SemanticsModifierNode, + Modifier.Node() { + private lateinit var requestBuilder: RequestBuilder + private lateinit var contentScale: ContentScale + private lateinit var alignment: Alignment + private var fixedSize: ImmediateGlideSize? = null + private var colorFilter: ColorFilter? = null + + private var currentJob: Job? = null + private var drawable: Drawable? = null + private var state: RequestState = RequestState.Loading + private var resolvableGlideSize: ResolvableGlideSize? = null + + fun onNewRequest( + requestBuilder: RequestBuilder, + fixedSize: ImmediateGlideSize?, + contentScale: ContentScale, + alignment: Alignment, + colorFilter: ColorFilter?, + ) { + // Other attributes can be refreshed by re-drawing rather than restarting a request + val restartLoad = + !this::requestBuilder.isInitialized || + requestBuilder != this.requestBuilder + || fixedSize != this.fixedSize + + this.requestBuilder = requestBuilder + this.fixedSize = fixedSize + this.contentScale = contentScale + this.alignment = alignment + this.colorFilter = colorFilter + + if (restartLoad) { + clear() + updateDrawable(null) + + // If we're not attached, we'll be measured when we eventually are attached. + if (isAttached) { + // If we don't have a fixed size, we need a new layout pass to figure out how large the + // image should be. Ideally we'd retain the old resolved glide size unless some other + // modifier node had already requested measurement. Since we can't tell if measurement is + // requested, we can either re-use the old resolvableGlideSize, which will be incorrect if + // measurement was requested. Or we can invalidate resolvableGlideSize and ensure that it's + // resolved by requesting measurement ourselves. Requesting is less efficient, but more + // correct. + // TODO(sam): See if we can find a reasonable way to remove this behavior, or be more + // targeted. + if (fixedSize == null) { + invalidateMeasurement() + } + launchRequest(requestBuilder) + } + } else { + drawable?.colorFilter = colorFilter?.asAndroidColorFilter() + invalidateDraw() + } + } + + private val Int.isValidDimension + get() = this > 0 + + private val Float.isValidDimension + get() = this > 0f + + private val Size.isValid + get() = width.isValidDimension && height.isValidDimension + + private fun Size.roundToInt() = IntSize(width.roundToInt(), height.roundToInt()) + + private fun IntOffset.toPointF() = PointF(x.toFloat(), y.toFloat()) + + override fun ContentDrawScope.draw() { + val drawable = drawable ?: return + val srcWidth = if (drawable.intrinsicWidth.isValidDimension) { + drawable.intrinsicWidth.toFloat() + } else { + size.width + } + val srcHeight = if (drawable.intrinsicHeight.isValidDimension) { + drawable.intrinsicHeight.toFloat() + } else { + size.height + } + val srcSize = Size(srcWidth, srcHeight) + + val scaledSize = if (size.isValid) { + contentScale.computeScaleFactor(srcSize, size).times(srcSize).roundToInt() + } else { + Size.Zero.roundToInt() + } + + drawable.setBounds(0, 0, scaledSize.width, scaledSize.height) + val alignedPosition: PointF = alignment.align( + IntSize(scaledSize.width, scaledSize.height), + IntSize(size.width.roundToInt(), size.height.roundToInt()), + layoutDirection + ).toPointF() + + translate(alignedPosition.x, alignedPosition.y) { + drawable.draw(drawContext.canvas.nativeCanvas) + } + + // Allow chaining of DrawModifiers + drawContent() + } + + override val shouldAutoInvalidate: Boolean + get() = false + + override fun onAttach() { + super.onAttach() + if (currentJob == null) { + launchRequest(requestBuilder) + } + } + + override fun onReset() { + super.onReset() + clear() + updateDrawable(null) + } + + @OptIn(ExperimentGlideFlows::class, InternalGlideApi::class, ExperimentalComposeUiApi::class) + private fun launchRequest(requestBuilder: RequestBuilder) = + // Launch via sideEffect because onAttach is called before onNewRequest and onNewRequest is not + // always called. That means in onAttach if we launch the request, we might restart an old + // request only to have it immediately replaced by a new request, causing jank. Or if we don't + // launch the new requests in onAttach, then onNewRequest might not be called and we won't show + // the old image. + // sideEffect is called after all changes in the tree, so we can always queue a new request, but + // drop any for old requests by comparing requests builders. + sideEffect { + if (this.requestBuilder != requestBuilder) { + return@sideEffect + } + Preconditions.checkArgument(currentJob == null) + currentJob = (coroutineScope + Dispatchers.Main.immediate).launch { + this@GlideNode.resolvableGlideSize = fixedSize ?: AsyncGlideSize() + + requestBuilder.flowResolvable(resolvableGlideSize!!).collect { + val (state, drawable) = + when (it) { + is Resource -> Pair(RequestState.Success(it.dataSource), it.resource) + is Placeholder -> { + val drawable = it.placeholder + val state = when (it.status) { + Status.CLEARED, Status.RUNNING -> RequestState.Loading + Status.FAILED -> RequestState.Failure + Status.SUCCEEDED -> throw IllegalStateException() + } + Pair(state, drawable) + } + } + updateDrawable(drawable) + this@GlideNode.state = state + invalidateDraw() + } + } + } + + private fun updateDrawable(drawable: Drawable?) { + this.drawable?.callback = null + this.drawable = drawable + drawable?.colorFilter = colorFilter?.asAndroidColorFilter() + drawable?.callback = object : Drawable.Callback { + override fun invalidateDrawable(who: Drawable) { + invalidateDraw() + } + + override fun scheduleDrawable(who: Drawable, what: Runnable, `when`: Long) {} + override fun unscheduleDrawable(who: Drawable, what: Runnable) {} + } + } + + override fun onDetach() { + super.onDetach() + clear() + } + + private fun clear() { + resolvableGlideSize = null + currentJob?.cancel() + currentJob = null + state = RequestState.Loading + updateDrawable(null) + } + + @OptIn(InternalGlideApi::class) + override fun MeasureScope.measure( + measurable: Measurable, + constraints: Constraints + ): MeasureResult { + when (val currentSize = resolvableGlideSize) { + is AsyncGlideSize -> { + val inferredSize = constraints.inferredGlideSize() + if (inferredSize != null) { + currentSize.setSize(inferredSize) + } + } + // Do nothing. + is ImmediateGlideSize -> {} + null -> {} + } + val placeable = measurable.measure(constraints) + return layout(constraints.maxWidth, constraints.maxHeight) { placeable.placeRelative(0, 0) } + } + + override fun SemanticsPropertyReceiver.applySemantics() { + displayedDrawable = { drawable } + } + + override fun equals(other: Any?): Boolean { + if (other is GlideNode) { + return requestBuilder == other.requestBuilder + && fixedSize == other.fixedSize + && contentScale == other.contentScale + && alignment == other.alignment + && colorFilter == other.colorFilter + } + return false + } + + override fun hashCode(): Int { + var result = requestBuilder.hashCode() + result = 31 * result + fixedSize.hashCode() + result = 31 * result + contentScale.hashCode() + result = 31 * result + alignment.hashCode() + result = 31 * result + colorFilter.hashCode() + return result + } +} + +internal val DisplayedDrawableKey = + SemanticsPropertyKey<() -> Drawable?>("DisplayedDrawable") +internal var SemanticsPropertyReceiver.displayedDrawable by DisplayedDrawableKey diff --git a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlidePainter.kt b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlidePainter.kt deleted file mode 100644 index adb81d1568..0000000000 --- a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlidePainter.kt +++ /dev/null @@ -1,132 +0,0 @@ -package com.bumptech.glide.integration.compose - -import android.graphics.drawable.BitmapDrawable -import android.graphics.drawable.ColorDrawable -import android.graphics.drawable.Drawable -import androidx.compose.runtime.MutableState -import androidx.compose.runtime.RememberObserver -import androidx.compose.runtime.Stable -import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.setValue -import androidx.compose.ui.geometry.Size -import androidx.compose.ui.graphics.Color -import androidx.compose.ui.graphics.ColorFilter -import androidx.compose.ui.graphics.DefaultAlpha -import androidx.compose.ui.graphics.asImageBitmap -import androidx.compose.ui.graphics.drawscope.DrawScope -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 -import com.bumptech.glide.integration.ktx.Resource -import com.bumptech.glide.integration.ktx.Status -import com.bumptech.glide.integration.ktx.flowResolvable -import com.google.accompanist.drawablepainter.DrawablePainter -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Job -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.job -import kotlinx.coroutines.launch -import kotlinx.coroutines.plus - -// This class is inspired by a similar implementation in the excellent Coil library -// (https://github.com/coil-kt/coil), specifically: -// https://github.com/coil-kt/coil/blob/main/coil-compose-base/src/main/java/coil/compose/AsyncImagePainter.kt -@Stable -internal class GlidePainter -@OptIn(InternalGlideApi::class) -constructor( - private val requestBuilder: RequestBuilder, - private val resolvableSize: ResolvableGlideSize, - scope: CoroutineScope, -) : Painter(), RememberObserver { - @OptIn(ExperimentGlideFlows::class) internal var status: Status by mutableStateOf(Status.CLEARED) - internal val currentDrawable: MutableState = mutableStateOf(null) - private var alpha: Float by mutableStateOf(DefaultAlpha) - private var colorFilter: ColorFilter? by mutableStateOf(null) - private var delegate: Painter? by mutableStateOf(null) - private val scope = - scope + SupervisorJob(parent = scope.coroutineContext.job) + Dispatchers.Main.immediate - private var currentJob: Job? = null - - 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) } - } - - override fun onAbandoned() { - (delegate as? RememberObserver)?.onAbandoned() - } - - override fun onForgotten() { - (delegate as? RememberObserver)?.onForgotten() - currentJob?.cancel() - currentJob = null - } - - override fun onRemembered() { - (delegate as? RememberObserver)?.onRemembered() - if (currentJob == null) { - currentJob = launchRequest() - } - } - - @OptIn(ExperimentGlideFlows::class, InternalGlideApi::class) - private fun launchRequest() = this.scope.launch { - requestBuilder.flowResolvable(resolvableSize).collect { - updateDelegate( - when (it) { - is Resource -> it.resource - is Placeholder -> it.placeholder - } - ) - status = it.status - } - } - - private fun Drawable.toPainter() = - when (this) { - is BitmapDrawable -> BitmapPainter(bitmap.asImageBitmap()) - is ColorDrawable -> ColorPainter(Color(color)) - else -> DrawablePainter(mutate()) - } - - private fun updateDelegate(drawable: Drawable?) { - val newDelegate = drawable?.toPainter() - val oldDelegate = delegate - if (newDelegate !== oldDelegate) { - (oldDelegate as? RememberObserver)?.onForgotten() - (newDelegate as? RememberObserver)?.onRemembered() - currentDrawable.value = drawable - delegate = newDelegate - } - } - - override fun applyAlpha(alpha: Float): Boolean { - this.alpha = alpha - return true - } - - override fun applyColorFilter(colorFilter: ColorFilter?): Boolean { - this.colorFilter = colorFilter - return true - } -} diff --git a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/Sizes.kt b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/Sizes.kt index c59550f865..0a37c1b3f1 100644 --- a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/Sizes.kt +++ b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/Sizes.kt @@ -2,6 +2,7 @@ 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 @@ -39,3 +40,22 @@ internal fun androidx.compose.ui.geometry.Size.toGlideSize(): Size? { } return Size(width, height); } + +internal fun Constraints.inferredGlideSize(): Size? { + val width = + if (hasBoundedWidth) { + maxWidth + } else { + com.bumptech.glide.request.target.Target.SIZE_ORIGINAL + } + val height = + if (hasBoundedHeight) { + maxHeight + } else { + com.bumptech.glide.request.target.Target.SIZE_ORIGINAL + } + if (!width.isValidGlideDimension() || !height.isValidGlideDimension()) { + return null + } + return Size(width, height) +} diff --git a/integration/concurrent/src/main/java/com/bumptech/glide/integration/concurrent/GlideFutures.java b/integration/concurrent/src/main/java/com/bumptech/glide/integration/concurrent/GlideFutures.java index 2557330b94..bec2bf37e7 100644 --- a/integration/concurrent/src/main/java/com/bumptech/glide/integration/concurrent/GlideFutures.java +++ b/integration/concurrent/src/main/java/com/bumptech/glide/integration/concurrent/GlideFutures.java @@ -181,14 +181,14 @@ private static final class GlideLoadingListener implements RequestListener @Override public boolean onLoadFailed( - @Nullable GlideException e, Object model, Target target, boolean isFirst) { + @Nullable GlideException e, Object model, @NonNull Target target, boolean isFirst) { completer.setException(e != null ? e : new RuntimeException("Unknown error")); return true; } @Override public boolean onResourceReady( - T resource, Object model, Target target, DataSource dataSource, boolean isFirst) { + @NonNull T resource, @NonNull Object model, Target target, @NonNull DataSource dataSource, boolean isFirst) { try { completer.set(new TargetAndResult<>(target, resource)); } catch (Throwable t) { diff --git a/integration/ktx/api/ktx.api b/integration/ktx/api/ktx.api index 4e3d08762d..9d0ce00aad 100644 --- a/integration/ktx/api/ktx.api +++ b/integration/ktx/api/ktx.api @@ -1,3 +1,9 @@ +public final class com/bumptech/glide/integration/ktx/AsyncGlideSize : com/bumptech/glide/integration/ktx/ResolvableGlideSize { + public fun ()V + public final fun getSize (Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun setSize (Lcom/bumptech/glide/integration/ktx/Size;)V +} + public abstract interface annotation class com/bumptech/glide/integration/ktx/ExperimentGlideFlows : java/lang/annotation/Annotation { } @@ -5,6 +11,7 @@ public final class com/bumptech/glide/integration/ktx/FlowsKt { public static final fun flow (Lcom/bumptech/glide/RequestBuilder;)Lkotlinx/coroutines/flow/Flow; public static final fun flow (Lcom/bumptech/glide/RequestBuilder;I)Lkotlinx/coroutines/flow/Flow; public static final fun flow (Lcom/bumptech/glide/RequestBuilder;II)Lkotlinx/coroutines/flow/Flow; + public static final fun flow (Lcom/bumptech/glide/RequestBuilder;Lcom/bumptech/glide/integration/ktx/AsyncGlideSize;)Lkotlinx/coroutines/flow/Flow; } public abstract class com/bumptech/glide/integration/ktx/GlideFlowInstant { @@ -28,15 +35,20 @@ public final class com/bumptech/glide/integration/ktx/Placeholder : com/bumptech } public final class com/bumptech/glide/integration/ktx/Resource : com/bumptech/glide/integration/ktx/GlideFlowInstant { - public fun (Lcom/bumptech/glide/integration/ktx/Status;Ljava/lang/Object;)V + public fun (Lcom/bumptech/glide/integration/ktx/Status;Ljava/lang/Object;ZLcom/bumptech/glide/load/DataSource;)V + public final fun asFailure ()Lcom/bumptech/glide/integration/ktx/Resource; public final fun component1 ()Lcom/bumptech/glide/integration/ktx/Status; public final fun component2 ()Ljava/lang/Object; - public final fun copy (Lcom/bumptech/glide/integration/ktx/Status;Ljava/lang/Object;)Lcom/bumptech/glide/integration/ktx/Resource; - public static synthetic fun copy$default (Lcom/bumptech/glide/integration/ktx/Resource;Lcom/bumptech/glide/integration/ktx/Status;Ljava/lang/Object;ILjava/lang/Object;)Lcom/bumptech/glide/integration/ktx/Resource; + public final fun component3 ()Z + public final fun component4 ()Lcom/bumptech/glide/load/DataSource; + public final fun copy (Lcom/bumptech/glide/integration/ktx/Status;Ljava/lang/Object;ZLcom/bumptech/glide/load/DataSource;)Lcom/bumptech/glide/integration/ktx/Resource; + public static synthetic fun copy$default (Lcom/bumptech/glide/integration/ktx/Resource;Lcom/bumptech/glide/integration/ktx/Status;Ljava/lang/Object;ZLcom/bumptech/glide/load/DataSource;ILjava/lang/Object;)Lcom/bumptech/glide/integration/ktx/Resource; public fun equals (Ljava/lang/Object;)Z + public final fun getDataSource ()Lcom/bumptech/glide/load/DataSource; public final fun getResource ()Ljava/lang/Object; public fun getStatus ()Lcom/bumptech/glide/integration/ktx/Status; public fun hashCode ()I + public final fun isFirstResource ()Z public fun toString ()Ljava/lang/String; } diff --git a/integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt b/integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt index a0ce7afa76..768462f438 100644 --- a/integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt +++ b/integration/ktx/src/main/java/com/bumptech/glide/integration/ktx/Flows.kt @@ -19,6 +19,7 @@ import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.launch +import java.lang.UnsupportedOperationException @RequiresOptIn( level = RequiresOptIn.Level.ERROR, @@ -100,7 +101,7 @@ public fun RequestBuilder.flow( * functionality for traditional Views. We could consider expanding the visibility if there are use * cases for asynchronous size resolution outside of Glide's Compose integration. */ -@InternalGlideApi +@OptIn(InternalGlideApi::class) @ExperimentGlideFlows public fun RequestBuilder.flow( size: AsyncGlideSize, @@ -201,6 +202,8 @@ public data class Placeholder( public data class Resource( public override val status: Status, public val resource: ResourceT, + public val isFirstResource: Boolean, + public val dataSource: DataSource, ) : GlideFlowInstant() { init { require( @@ -215,6 +218,9 @@ public data class Resource( } ) } + + public fun asFailure():Resource = + Resource(Status.FAILED, resource, isFirstResource, dataSource) } @InternalGlideApi @@ -264,7 +270,7 @@ private class FlowTarget( ) : Target, RequestListener { @Volatile private var resolvedSize: Size? = null @Volatile private var currentRequest: Request? = null - @Volatile private var lastResource: ResourceT? = null + @Volatile private var lastResource: Resource? = null @GuardedBy("this") private val sizeReadyCallbacks = mutableListOf() @@ -306,15 +312,8 @@ private class FlowTarget( } override fun onResourceReady(resource: ResourceT, transition: Transition?) { - lastResource = resource - scope.trySend( - Resource( - // currentRequest is the entire request state, so we can use it to figure out if this - // resource is from a thumbnail request (isComplete is false) or the primary request. - if (currentRequest?.isComplete == true) Status.SUCCEEDED else Status.RUNNING, - resource - ) - ) + throw UnsupportedOperationException() + } override fun onLoadCleared(placeholder: Drawable?) { @@ -354,25 +353,36 @@ private class FlowTarget( override fun onLoadFailed( e: GlideException?, model: Any?, - target: Target?, + target: Target, isFirstResource: Boolean, ): Boolean { val localLastResource = lastResource val localRequest = currentRequest if (localLastResource != null && localRequest?.isComplete == false && !localRequest.isRunning) { - scope.channel.trySend(Resource(Status.FAILED, localLastResource)) + scope.channel.trySend(localLastResource.asFailure()) } return false } override fun onResourceReady( resource: ResourceT, - model: Any?, - target: Target?, - dataSource: DataSource?, + model: Any, + target: Target, + dataSource: DataSource, isFirstResource: Boolean, ): Boolean { - return false + val result = + Resource( + // currentRequest is the entire request state, so we can use it to figure out if this + // resource is from a thumbnail request (isComplete is false) or the primary request. + if (currentRequest?.isComplete == true) Status.SUCCEEDED else Status.RUNNING, + resource, + isFirstResource, + dataSource + ) + lastResource = result + scope.trySend(result) + return true } } @@ -388,7 +398,7 @@ public data class Size(val width: Int, val height: Int) { @InternalGlideApi public data class ImmediateGlideSize(val size: Size) : ResolvableGlideSize() -@InternalGlideApi +@OptIn(InternalGlideApi::class) public class AsyncGlideSize : ResolvableGlideSize() { private val size = CompletableDeferred() diff --git a/integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt b/integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt index acee8028e4..d59d48ac77 100644 --- a/integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt +++ b/integration/ktx/src/test/java/com/bumptech/glide/integration/ktx/FlowsTest.kt @@ -646,22 +646,22 @@ private fun atMostOnce(function: () -> Unit): () -> Unit { } } -private fun onSuccess(onSuccess: () -> Unit) = +private fun onSuccess(onSuccess: () -> Unit) = simpleRequestListener(onSuccess) {} -private fun onFailure(onFailure: () -> Unit) = +private fun onFailure(onFailure: () -> Unit) = simpleRequestListener({}, onFailure) -private fun simpleRequestListener( +private fun simpleRequestListener( onSuccess: () -> Unit, onFailure: () -> Unit ): RequestListener = object : RequestListener { override fun onResourceReady( - resource: ResourceT?, - model: Any?, - target: Target?, - dataSource: DataSource?, + resource: ResourceT, + model: Any, + target: Target, + dataSource: DataSource, isFirstResource: Boolean, ): Boolean { onSuccess() @@ -671,7 +671,7 @@ private fun simpleRequestListener( override fun onLoadFailed( e: GlideException?, model: Any?, - target: Target?, + target: Target, isFirstResource: Boolean, ): Boolean { onFailure() diff --git a/library/src/main/java/com/bumptech/glide/RequestBuilder.java b/library/src/main/java/com/bumptech/glide/RequestBuilder.java index 819a9f3de7..488b4e5177 100644 --- a/library/src/main/java/com/bumptech/glide/RequestBuilder.java +++ b/library/src/main/java/com/bumptech/glide/RequestBuilder.java @@ -1295,6 +1295,10 @@ private Request obtainRequest( callbackExecutor); } + Object getModel() { + return model; + } + @Override public boolean equals(Object o) { if (o instanceof RequestBuilder) { diff --git a/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java b/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java index 8d3e825199..b6eba857c8 100644 --- a/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java +++ b/library/src/main/java/com/bumptech/glide/request/RequestFutureTarget.java @@ -241,7 +241,7 @@ public void onDestroy() { @Override public synchronized boolean onLoadFailed( - @Nullable GlideException e, Object model, Target target, boolean isFirstResource) { + @Nullable GlideException e, Object model, @NonNull Target target, boolean isFirstResource) { loadFailed = true; exception = e; waiter.notifyAll(this); @@ -250,7 +250,7 @@ public synchronized boolean onLoadFailed( @Override public synchronized boolean onResourceReady( - R resource, Object model, Target target, DataSource dataSource, boolean isFirstResource) { + @NonNull R resource, @NonNull Object model, Target target, @NonNull DataSource dataSource, boolean isFirstResource) { // We might get a null result. resultReceived = true; this.resource = resource; diff --git a/library/src/main/java/com/bumptech/glide/request/RequestListener.java b/library/src/main/java/com/bumptech/glide/request/RequestListener.java index f8da91dc25..bb97e843fa 100644 --- a/library/src/main/java/com/bumptech/glide/request/RequestListener.java +++ b/library/src/main/java/com/bumptech/glide/request/RequestListener.java @@ -2,6 +2,7 @@ import android.graphics.drawable.Drawable; import android.widget.ImageView; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.bumptech.glide.RequestBuilder; import com.bumptech.glide.load.DataSource; @@ -60,7 +61,10 @@ public interface RequestListener { * Target#onLoadFailed(Drawable)} to be called on {@code target}. */ boolean onLoadFailed( - @Nullable GlideException e, Object model, Target target, boolean isFirstResource); + @Nullable GlideException e, + @Nullable Object model, + @NonNull Target target, + boolean isFirstResource); /** * Called when a load completes successfully, immediately before {@link @@ -68,8 +72,12 @@ boolean onLoadFailed( * *

For threading guarantees, see the class comment. * - * @param resource The resource that was loaded for the target. - * @param model The specific model that was used to load the image. + * @param resource The resource that was loaded for the target. Non-null because a null resource + * will result in a call to {@link #onLoadFailed(GlideException, Object, Target, boolean)} + * instead of this method. + * @param model The specific model that was used to load the image. Non-null because a null model + * will result in a call to {@link #onLoadFailed(GlideException, Object, Target, boolean)} + * instead of this method. * @param target The target the model was loaded into. * @param dataSource The {@link DataSource} the resource was loaded from. * @param isFirstResource {@code true} if this is the first resource to in this load to be loaded @@ -81,5 +89,9 @@ boolean onLoadFailed( * Target#onResourceReady(Object, Transition)} to be called on {@code target}. */ boolean onResourceReady( - R resource, Object model, Target target, DataSource dataSource, boolean isFirstResource); + @NonNull R resource, + @NonNull Object model, + Target target, + @NonNull DataSource dataSource, + boolean isFirstResource); } diff --git a/library/test/src/test/java/com/bumptech/glide/GlideTest.java b/library/test/src/test/java/com/bumptech/glide/GlideTest.java index e174df7e7e..f7a8d1b720 100644 --- a/library/test/src/test/java/com/bumptech/glide/GlideTest.java +++ b/library/test/src/test/java/com/bumptech/glide/GlideTest.java @@ -424,17 +424,17 @@ private void runTestStringDefaultLoader(String string) { public boolean onLoadFailed( GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { throw new RuntimeException("Load failed"); } @Override public boolean onResourceReady( - Drawable resource, - Object model, + @NonNull Drawable resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { return false; } diff --git a/library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java b/library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java index 7c2e0a5446..ebac2453ca 100644 --- a/library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java +++ b/library/test/src/test/java/com/bumptech/glide/RequestBuilderTest.java @@ -13,6 +13,7 @@ import android.app.Application; import android.net.Uri; import android.widget.ImageView; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.test.core.app.ApplicationProvider; import com.bumptech.glide.load.DataSource; @@ -174,17 +175,17 @@ public void testEquals() { public boolean onLoadFailed( @Nullable GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { return false; } @Override public boolean onResourceReady( - Object resource, - Object model, + @NonNull Object resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { return false; } @@ -195,17 +196,17 @@ public boolean onResourceReady( public boolean onLoadFailed( @Nullable GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { return false; } @Override public boolean onResourceReady( - Object resource, - Object model, + @NonNull Object resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { return false; } diff --git a/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java b/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java index 2376cc13f1..6f6a4a6765 100644 --- a/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java +++ b/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java @@ -696,17 +696,17 @@ public void onResourceReady_notifiesRequestCoordinator_beforeCallingRequestListe public boolean onLoadFailed( @Nullable GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { return false; } @Override public boolean onResourceReady( - List resource, - Object model, + @NonNull List resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { verify(builder.requestCoordinator).onRequestSuccess(target.getRequest()); isRequestCoordinatorVerified.set(true); @@ -733,7 +733,7 @@ public void onLoadFailed_notifiesRequestCoordinator_beforeCallingRequestListener public boolean onLoadFailed( @Nullable GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { verify(builder.requestCoordinator).onRequestFailed(target.getRequest()); isRequestCoordinatorVerified.set(true); @@ -742,10 +742,10 @@ public boolean onLoadFailed( @Override public boolean onResourceReady( - List resource, - Object model, + @NonNull List resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { return false; } diff --git a/samples/gallery/build.gradle b/samples/gallery/build.gradle index 51a1609db9..71d0823d85 100644 --- a/samples/gallery/build.gradle +++ b/samples/gallery/build.gradle @@ -6,6 +6,7 @@ apply plugin: 'com.google.devtools.ksp' dependencies { implementation project(':library') implementation project(':integration:compose') + implementation project(':integration:ktx') implementation(project(':integration:recyclerview')) { transitive = false } diff --git a/samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/HorizontalGalleryFragment.kt b/samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/HorizontalGalleryFragment.kt index 7b9a823f6f..80e0d4f497 100644 --- a/samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/HorizontalGalleryFragment.kt +++ b/samples/gallery/src/main/java/com/bumptech/glide/samples/gallery/HorizontalGalleryFragment.kt @@ -72,7 +72,7 @@ class HorizontalGalleryFragment : Fragment() { preloadRequestBuilder: RequestBuilder, modifier: Modifier, ) = - GlideImage(model = item.uri, contentDescription = item.displayName, modifier = modifier) { + GlideImage(model = item.uri, contentDescription = null, modifier = modifier) { it.thumbnail(preloadRequestBuilder).signature(item.signature()) } diff --git a/samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/FullscreenActivity.java b/samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/FullscreenActivity.java index 05996088b6..55920fd151 100644 --- a/samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/FullscreenActivity.java +++ b/samples/giphy/src/main/java/com/bumptech/glide/samples/giphy/FullscreenActivity.java @@ -10,6 +10,7 @@ import android.os.Bundle; import android.view.View; import android.widget.ImageView; +import androidx.annotation.NonNull; import com.bumptech.glide.RequestBuilder; import com.bumptech.glide.load.DataSource; import com.bumptech.glide.load.engine.GlideException; @@ -70,17 +71,17 @@ public void onClick(View view) { public boolean onLoadFailed( GlideException e, Object model, - Target target, + @NonNull Target target, boolean isFirstResource) { return false; } @Override public boolean onResourceReady( - Drawable resource, - Object model, + @NonNull Drawable resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { if (resource instanceof GifDrawable) { gifDrawable = (GifDrawable) resource; diff --git a/samples/svg/src/main/java/com/bumptech/glide/samples/svg/SvgSoftwareLayerSetter.java b/samples/svg/src/main/java/com/bumptech/glide/samples/svg/SvgSoftwareLayerSetter.java index 411b50d34b..f2ff2c380d 100644 --- a/samples/svg/src/main/java/com/bumptech/glide/samples/svg/SvgSoftwareLayerSetter.java +++ b/samples/svg/src/main/java/com/bumptech/glide/samples/svg/SvgSoftwareLayerSetter.java @@ -2,6 +2,7 @@ import android.graphics.drawable.PictureDrawable; import android.widget.ImageView; +import androidx.annotation.NonNull; import com.bumptech.glide.load.DataSource; import com.bumptech.glide.load.engine.GlideException; import com.bumptech.glide.request.RequestListener; @@ -17,7 +18,7 @@ public class SvgSoftwareLayerSetter implements RequestListener @Override public boolean onLoadFailed( - GlideException e, Object model, Target target, boolean isFirstResource) { + GlideException e, Object model, @NonNull Target target, boolean isFirstResource) { ImageView view = ((ImageViewTarget) target).getView(); view.setLayerType(ImageView.LAYER_TYPE_NONE, null); return false; @@ -25,10 +26,10 @@ public boolean onLoadFailed( @Override public boolean onResourceReady( - PictureDrawable resource, - Object model, + @NonNull PictureDrawable resource, + @NonNull Object model, Target target, - DataSource dataSource, + @NonNull DataSource dataSource, boolean isFirstResource) { ImageView view = ((ImageViewTarget) target).getView(); view.setLayerType(ImageView.LAYER_TYPE_SOFTWARE, null); diff --git a/settings.gradle b/settings.gradle index f3855c65bc..53af2945cc 100644 --- a/settings.gradle +++ b/settings.gradle @@ -54,7 +54,7 @@ dependencyResolutionManagement { version('kotlin-compiler-extension', '1.2.0') // Versions for dependencies - version('compose', '1.4.3') + version('compose', '1.5.0') version('coroutines', '1.7.2') version('dagger', '2.46.1') version('errorprone', '2.18.0') @@ -99,7 +99,7 @@ dependencyResolutionManagement { library('androidx-tracing', 'androidx.tracing:tracing:1.0.0') library('androidx.vectordrawable', 'androidx.vectordrawable:vectordrawable-animated:1.1.0') library('proguard-gradle', 'com.guardsquare:proguard-gradle:7.1.0') - library('compose-foundation', 'androidx.compose.foundation:foundation:1.3.1') + library('compose-foundation', 'androidx.compose.foundation', 'foundation').versionRef('compose') library('compose-material', 'androidx.compose.material:material:1.3.1') library('compose-ui', 'androidx.compose.ui', 'ui').versionRef('compose') library('compose-ui.testmanifest', 'androidx.compose.ui', 'ui-test-manifest').versionRef('compose')