From e63c5d216520baecaaad9273699765b8c02b28be Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Sun, 13 Nov 2022 16:11:13 -0800 Subject: [PATCH] Change Glide's preloading API to be data based By data based we mean that we preload based on access to the data, rather than the position of the user in the view. Data based access matches how the Compose paging API works and is much simpler to implement in safe / reasonable way. The primary compromise is that we cannot begin preloading until the data is accessed. If there are multiple disjoint data sets or multiple header rows, we may not preload data while scrolling until we reach the first item in each data set. Users can work around this in each application by manually accessing the prealoding data set, but it's not super convenient. Another option would be to wrap LazyListState and track each insertion via item / items. That would let our preloader track exactly which positions are preloadable and which are not. While the functionality is somewhat better and it's safe, the implementation is much larger and it's more complex for callers to call correctly. We can add it in later if we find that the data based API is painful to use. Yet another option would be to more faithfully implement the ListPreloader interface and rely on users to paper over more gaps, just as we do for non-Compose code. This is significantly more complex for users to implement. We've discussed a few options for this API here: https://chat.google.com/room/AAAAYRnp4-Y/Q61ILNtb8hu --- gradle.properties | 4 +- integration/compose/api/compose.api | 8 +- integration/compose/build.gradle | 8 +- .../integration/compose/GlidePreloaderTest.kt | 188 ---------- .../RememberGlidePreloadingDataTest.kt | 231 ++++++++++++ .../glide/integration/compose/Preload.kt | 331 +++++++++++------- .../com/bumptech/glide/ListPreloader.java | 8 +- samples/gallery/build.gradle | 6 +- .../gallery/HorizontalGalleryFragment.kt | 68 ++-- 9 files changed, 483 insertions(+), 369 deletions(-) delete mode 100644 integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlidePreloaderTest.kt create mode 100644 integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/RememberGlidePreloadingDataTest.kt diff --git a/gradle.properties b/gradle.properties index fa0fa84343..b601fcd72e 100644 --- a/gradle.properties +++ b/gradle.properties @@ -56,7 +56,9 @@ ANDROID_X_ANNOTATION_VERSION=1.3.0 ANDROID_X_APPCOMPAT_VERSION=1.3.1 ANDROID_X_BENCHMARK_VERSION=1.1.0 ANDROID_X_CARDVIEW_VERSION=1.0.0 -ANDROID_X_COMPOSE_VERSION=1.2.1 +ANDROID_X_COMPOSE_VERSION=1.3.2 +ANDROID_X_COMPOSE_FOUNDATION_VERSION=1.3.1 +ANDROID_X_COMPOSE_MATERIAL_VERSION=1.3.1 ANDROID_X_CONCURRENT_FUTURES_VERSION=1.1.0 ANDROID_X_CORE_VERSION=1.6.0 ANDROID_X_EXIF_INTERFACE_VERSION=1.3.3 diff --git a/integration/compose/api/compose.api b/integration/compose/api/compose.api index 08ec1029b1..5185e1f008 100644 --- a/integration/compose/api/compose.api +++ b/integration/compose/api/compose.api @@ -8,11 +8,17 @@ public final class com/bumptech/glide/integration/compose/GlideImageKt { public static final fun placeholder (Lkotlin/jvm/functions/Function2;)Lcom/bumptech/glide/integration/compose/Placeholder; } +public abstract interface class com/bumptech/glide/integration/compose/GlidePreloadingData { + public abstract fun get (ILandroidx/compose/runtime/Composer;I)Lkotlin/Pair; + public abstract fun getSize ()I +} + public abstract class com/bumptech/glide/integration/compose/Placeholder { public static final field $stable I } public final class com/bumptech/glide/integration/compose/PreloadKt { - public static final fun GlideLazyListPreloader-aVSU5A8 (Landroidx/compose/foundation/lazy/LazyListState;Ljava/util/List;Lkotlin/jvm/functions/Function1;JILjava/lang/Integer;Lkotlin/jvm/functions/Function2;Landroidx/compose/runtime/Composer;II)V + public static final fun rememberGlidePreloadingData-Z8o_i8w (Ljava/util/List;JILjava/lang/Integer;Lkotlin/jvm/functions/Function2;Landroidx/compose/runtime/Composer;II)Lcom/bumptech/glide/integration/compose/GlidePreloadingData; + 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; } diff --git a/integration/compose/build.gradle b/integration/compose/build.gradle index c71ce3767d..89480bfd7f 100644 --- a/integration/compose/build.gradle +++ b/integration/compose/build.gradle @@ -4,11 +4,11 @@ plugins { } android { - compileSdk 32 + compileSdk 33 defaultConfig { minSdk 21 - targetSdk 32 + targetSdk 33 testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } @@ -50,7 +50,7 @@ dependencies { implementation(project(':integration:recyclerview')) { transitive = false } - implementation "androidx.compose.foundation:foundation:$ANDROID_X_COMPOSE_VERSION" + implementation "androidx.compose.foundation:foundation:$ANDROID_X_COMPOSE_FOUNDATION_VERSION" implementation "androidx.compose.ui:ui:$ANDROID_X_COMPOSE_VERSION" implementation "com.google.accompanist:accompanist-drawablepainter:$ACCOMPANIEST_VERSION" implementation "androidx.core:core-ktx:$ANDROID_X_CORE_KTX_VERSION" @@ -60,7 +60,7 @@ dependencies { androidTestImplementation "androidx.test.espresso:espresso-core:$ANDROID_X_TEST_ESPRESSO_VERSION" androidTestImplementation "androidx.test.espresso.idling:idling-concurrent:$ANDROID_X_TEST_ESPRESSO_VERSION" androidTestImplementation "androidx.test.ext:junit:$ANDROID_X_TEST_JUNIT_VERSION" - androidTestImplementation "androidx.compose.material:material:$ANDROID_X_COMPOSE_VERSION" + androidTestImplementation "androidx.compose.material:material:$ANDROID_X_COMPOSE_MATERIAL_VERSION" androidTestImplementation project(':testutil') androidTestImplementation "com.google.truth:truth:${TRUTH_VERSION}" } diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlidePreloaderTest.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlidePreloaderTest.kt deleted file mode 100644 index f8603f4615..0000000000 --- a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlidePreloaderTest.kt +++ /dev/null @@ -1,188 +0,0 @@ -@file:OptIn(ExperimentalGlideComposeApi::class) - -package com.bumptech.glide.integration.compose - -import android.content.Context -import android.graphics.drawable.Drawable -import androidx.annotation.DrawableRes -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.lazy.LazyListState -import androidx.compose.foundation.lazy.LazyRow -import androidx.compose.foundation.lazy.itemsIndexed -import androidx.compose.foundation.lazy.rememberLazyListState -import androidx.compose.material.Text -import androidx.compose.material.TextButton -import androidx.compose.runtime.Composable -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.ui.Modifier -import androidx.compose.ui.geometry.Size -import androidx.compose.ui.platform.testTag -import androidx.compose.ui.test.hasTestTag -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 -import com.bumptech.glide.integration.compose.test.GlideComposeRule -import com.bumptech.glide.request.target.Target -import com.google.common.truth.Truth.assertThat -import org.junit.Rule -import org.junit.Test - -class GlidePreloaderTest { - private val context: Context = ApplicationProvider.getApplicationContext() - @get:Rule val glideComposeRule = GlideComposeRule() - - @Test - fun glideLazyListPreloader_withoutScroll_preloadsNextItem() { - glideComposeRule.setContent { - val state = rememberLazyListState() - LazyRow(state = state, modifier = Modifier.testTag(listTestTag)) { - itemsIndexed(models) { index, drawableResourceId -> - GlideImage( - model = drawableResourceId, - contentDescription = imageContentDescription(index), - Modifier.fillParentMaxWidth(), - ) - } - } - - PreloadOneItemGlideLazyListPreloader(state) - } - - assertThatModelIsInMemoryCache(preloadModels[1]) - } - - fun assertThatModelIsInMemoryCache(@DrawableRes model: Int){ - // Wait for previous aysnc image loads to finish - glideComposeRule.waitForIdle() - val nextPreloadModel: Drawable = - Glide.with(context).load(model).onlyRetrieveFromCache(true).submit().get() - assertThat(nextPreloadModel).isNotNull() - } - - @Test - fun glideLazyListPreloader_onScroll_preloadsAheadInDirectionOfScroll() { - glideComposeRule.setContent { - val state = rememberLazyListState() - LazyRow(state = state, modifier = Modifier.testTag(listTestTag)) { - itemsIndexed(models) { index, drawableResourceId -> - GlideImage( - model = drawableResourceId, - contentDescription = imageContentDescription(index), - Modifier.fillParentMaxWidth(), - ) - } - } - - PreloadOneItemGlideLazyListPreloader(state) - } - - val scrollToIndex = 1 - glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex) - - assertThatModelIsInMemoryCache(preloadModels[2]) - } - - @Test - fun glideLazyListPreloader_withHeaderItem_onScroll_doesNotCrash() { - glideComposeRule.setContent { - val state = rememberLazyListState() - LazyRow(state = state, modifier = Modifier.testTag(listTestTag)) { - item { Text(text = "Header") } - itemsIndexed(models) { index, drawableResourceId -> - GlideImage( - model = drawableResourceId, - contentDescription = imageContentDescription(index), - Modifier.fillParentMaxWidth(), - ) - } - } - - PreloadOneItemGlideLazyListPreloader(state) - } - - val scrollToIndex = 1 - glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex) - - assertThatModelIsInMemoryCache(preloadModels[2]) - } - - @Test - fun glideLazyListPreloader_whenDataChanges_onScroll_preloadsUpdatedData() { - glideComposeRule.setContent { - // Swap both to avoid confusing the preloader. The preloader doesn't notice or take into - // account data set changes (this is a bug in the Java preloading API)... - val currentPreloadModels = remember { mutableStateOf(listOf()) } - val currentModels = remember { mutableStateOf(listOf()) } - // Use a button to swap data because we can't mutate state in setContent easily from outside - // the method, nor can you call setContent multiple times. - fun swapData() { - currentPreloadModels.value = preloadModels - currentModels.value = models - } - val state = rememberLazyListState() - Column { - TextButton(onClick = ::swapData) { Text(text = "Swap") } - LazyRow(state = state, - modifier = Modifier.testTag(listTestTag), - horizontalArrangement = Arrangement.spacedBy(10.dp)) { - item { Text(text = "Header") } - itemsIndexed(currentModels.value) { index, drawableResourceId -> - GlideImage( - model = drawableResourceId, - contentDescription = imageContentDescription(index), - Modifier.fillParentMaxWidth(), - ) - } - } - } - - PreloadOneItemGlideLazyListPreloader(state, data = currentPreloadModels.value) - } - - glideComposeRule.onNodeWithText("Swap").performClick() - val scrollToIndex = 1 - glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex) - - assertThatModelIsInMemoryCache(preloadModels[2]) - } - - @Suppress("TestFunctionName") // Not a Test... - @Composable - private fun PreloadOneItemGlideLazyListPreloader( - state: LazyListState, - data: List = preloadModels, - ) = - GlideLazyListPreloader( - state = state, - data = data, - size = Size(Target.SIZE_ORIGINAL.toFloat(), Target.SIZE_ORIGINAL.toFloat()), - numberOfItemsToPreload = 1, - requestBuilderTransform = { resourceId, requestBuilder -> requestBuilder.load(resourceId) }) - - companion object { - val models = - listOf( - android.R.drawable.star_big_on, - android.R.drawable.star_big_off, - android.R.drawable.btn_plus, - ) - // Use different preload and non-preload models so that we can assert on which items are - // preloaded and not loaded by the list. This is bad practice in production code and would waste - // resources while doing nothing useful in a real app. - val preloadModels = - listOf( - android.R.drawable.btn_minus, - android.R.drawable.btn_radio, - android.R.drawable.btn_star, - ) - - val listTestTag = "listTestTag" - fun imageContentDescription(index: Int) = "Image $index" - } -} - diff --git a/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/RememberGlidePreloadingDataTest.kt b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/RememberGlidePreloadingDataTest.kt new file mode 100644 index 0000000000..8ab6693df6 --- /dev/null +++ b/integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/RememberGlidePreloadingDataTest.kt @@ -0,0 +1,231 @@ +@file:OptIn(ExperimentalGlideComposeApi::class, ExperimentalGlideComposeApi::class) + +package com.bumptech.glide.integration.compose + +import android.content.Context +import android.graphics.drawable.Drawable +import androidx.annotation.DrawableRes +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.lazy.LazyRow +import androidx.compose.material.Text +import androidx.compose.material.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.runtime.mutableStateListOf +import androidx.compose.runtime.remember +import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Size +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.test.hasTestTag +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 +import com.bumptech.glide.RequestBuilder +import com.bumptech.glide.integration.compose.test.GlideComposeRule +import com.bumptech.glide.request.target.Target +import com.google.common.truth.Truth.assertThat +import org.junit.Rule +import org.junit.Test + +class RememberGlidePreloadingDataTest { + private val context: Context = ApplicationProvider.getApplicationContext() + @get:Rule val glideComposeRule = GlideComposeRule() + + @Test + fun rememberGlidePreloadingData_withoutScroll_preloadsNextItem() { + glideComposeRule.setContent { + val preloadingData = rememberOneItemAtATimePreloadingData() + + LazyRow(modifier = Modifier.testTag(listTestTag)) { + items(preloadingData.size) { index -> + preloadingData.triggerPreload(index) + GlideImage( + model = model, + contentDescription = imageContentDescription(index), + Modifier.fillParentMaxWidth(), + ) + } + } + } + + assertThatModelIsInMemoryCache(preloadModels[1]) + } + + @Test + fun glideLazyListPreloader_onScroll_preloadsAheadInDirectionOfScroll() { + glideComposeRule.setContent { + val preloadingData = rememberOneItemAtATimePreloadingData() + LazyRow(modifier = Modifier.testTag(listTestTag)) { + items(preloadingData.size) { index -> + preloadingData.triggerPreload(index) + GlideImage( + model = model, + contentDescription = imageContentDescription(index), + Modifier.fillParentMaxWidth(), + ) + } + } + } + + val scrollToIndex = 1 + glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex) + + assertThatModelIsInMemoryCache(preloadModels[2]) + } + + @Test + fun glideLazyListPreloader_withHeaderItem_onScroll_doesNotCrash() { + glideComposeRule.setContent { + val preloadingData = rememberOneItemAtATimePreloadingData() + + LazyRow(modifier = Modifier.testTag(listTestTag)) { + item { Text(text = "Header") } + items(preloadingData.size) { index -> + preloadingData.triggerPreload(index) + GlideImage( + model = model, + contentDescription = imageContentDescription(index), + Modifier.fillParentMaxWidth(), + ) + } + } + } + + // Scroll to the 0th image, accounting for the first header item. + val scrollToIndex = 1 + glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex) + // Make sure the next image, the 1th, is in memory due to preloading. + assertThatModelIsInMemoryCache(preloadModels[1]) + } + + @Test + fun glideLazyListPreloader_whenDataChanges_onScroll_preloadsUpdatedData() { + glideComposeRule.setContent { + // Swap both to avoid confusing the preloader. The preloader doesn't notice or take into + // account data set changes (this is a bug in the Java preloading API)... + val currentPreloadModels = remember { mutableStateListOf() } + val currentModels = remember { mutableStateListOf() } + // Use a button to swap data because we can't mutate state in setContent easily from outside + // the method, nor can you call setContent multiple times. + fun swapData() { + currentPreloadModels.addAll(preloadModels) + currentModels.addAll(preloadModels) + } + val preloadData = + rememberGlidePreloadingData( + data = currentPreloadModels, + preloadImageSize = Target.SIZE_ORIGINAL.toSize(), + numberOfItemsToPreload = 1, + fixedVisibleItemCount = 1, + ) { data: Int, requestBuilder: RequestBuilder -> + requestBuilder.load(data) + } + + TextButton(onClick = ::swapData) { Text(text = "Swap") } + + Column { + LazyRow( + modifier = Modifier.testTag(listTestTag), + horizontalArrangement = Arrangement.spacedBy(10.dp), + ) { + items(currentModels.size) { index -> + // This mismatch between currentModels and preloadData may lead to errors in the future + // because items may be recomposed before the setContent method's function is + // recomposed. See https://chat.google.com/room/AAAAYRnp4-Y/AvFrBgb_peU for a bunch of + // detailed discussion. + preloadData.triggerPreload(index) + GlideImage( + model = currentModels[index], + contentDescription = imageContentDescription(index), + Modifier.fillParentMaxWidth(), + ) + } + } + } + } + + glideComposeRule.onNodeWithText("Swap").performClick() + glideComposeRule.waitForIdle() + val scrollToIndex = 1 + glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex) + + assertThatModelIsInMemoryCache(preloadModels[scrollToIndex + 1]) + } + + @Test + fun glideLazyListPreloader_withHeaderItems_andPositionFunction_onScroll_preloadsTheFirstItem() { + val numHeaderItems = 3 + glideComposeRule.setContent { + val data = rememberOneItemAtATimePreloadingData() + LazyRow(modifier = Modifier.testTag(listTestTag)) { + repeat(numHeaderItems) { + item { Text(text = "Header$it") } + } + items(data.size) { index -> + data.triggerPreload(index) + GlideImage( + model = model, + contentDescription = imageContentDescription(index), + Modifier.fillParentMaxWidth(), + ) + } + } + } + + val imageIndex = 1 + val scrollToIndex = numHeaderItems + imageIndex + glideComposeRule.onNode(hasTestTag(listTestTag)).performScrollToIndex(scrollToIndex) + + assertThatModelIsInMemoryCache(preloadModels[imageIndex + 1]) + } + + // Ignore the preload request because we want to test that the preloader loaded a model + // and not be confused by our UI loading a model. Do not ignore the preload request + // builder in real code! + @Composable + private fun GlidePreloadingData.triggerPreload(index: Int) = this[index].first + + @Composable + private fun rememberOneItemAtATimePreloadingData(): GlidePreloadingData { + return rememberGlidePreloadingData( + data = preloadModels, + preloadImageSize = Target.SIZE_ORIGINAL.toSize(), + numberOfItemsToPreload = 1, + fixedVisibleItemCount = 1, + ) { model, requestBuilder -> + requestBuilder.load(model) + } + } + + private fun assertThatModelIsInMemoryCache(@DrawableRes model: Int){ + // Wait for previous async image loads to finish + glideComposeRule.waitForIdle() + val nextPreloadModel: Drawable = + Glide.with(context).load(model).onlyRetrieveFromCache(true).submit().get() + assertThat(nextPreloadModel).isNotNull() + } + + private companion object { + const val model = android.R.drawable.star_big_on + + // Use different preload and non-preload models so that we can assert on which items are + // preloaded and not loaded by the list. This is bad practice in production code and would waste + // resources while doing nothing useful in a real app. + val preloadModels = + listOf( + android.R.drawable.btn_minus, + android.R.drawable.btn_radio, + android.R.drawable.btn_star, + ) + + const val listTestTag = "listTestTag" + + fun imageContentDescription(index: Int) = "Image $index" + } +} + +private fun Int.toSize() = this.toFloat().let { Size(it, it) } + diff --git a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/Preload.kt b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/Preload.kt index f6594d7e4e..ef5069ec63 100644 --- a/integration/compose/src/main/java/com/bumptech/glide/integration/compose/Preload.kt +++ b/integration/compose/src/main/java/com/bumptech/glide/integration/compose/Preload.kt @@ -1,15 +1,9 @@ package com.bumptech.glide.integration.compose -import android.util.Log -import androidx.compose.foundation.lazy.LazyListState -import androidx.compose.foundation.lazy.LazyRow +import android.graphics.drawable.Drawable import androidx.compose.runtime.Composable -import androidx.compose.runtime.Immutable import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.State import androidx.compose.runtime.remember -import androidx.compose.runtime.rememberUpdatedState -import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.geometry.Size import androidx.compose.ui.platform.LocalContext import com.bumptech.glide.Glide @@ -17,18 +11,58 @@ import com.bumptech.glide.ListPreloader import com.bumptech.glide.RequestBuilder import com.bumptech.glide.RequestManager +private const val DEFAULT_ITEMS_TO_PRELOAD = 10 + /** - * Preloads ahead of the users current scroll position for [LazyRow] and - * [androidx.compose.foundation.lazy.LazyColumn], similar to [ListPreloader] and - * [com.bumptech.glide.integration.recyclerview.RecyclerViewPreloader]. + * Preloads ahead of the data access position on the returned [GlidePreloadingData], similar to + * [ListPreloader] and [com.bumptech.glide.integration.recyclerview.RecyclerViewPreloader]. * * The only time this API is useful is when your UI also loads an item with exactly the same - * options, model and size. Be careful to make sure that your requests are identical in the - * preloader and in the UI, or you might end up hurting performance instead of improving it. + * options, model and size. You can ensure you're doing so by using the [RequestBuilder] returned + * by [GlidePreloadingData.get] + * + * Typical usage will look something like this: + * ``` + * val glidePreloadingData = + * rememberGlidePreloadingData(myDataList, THUMBNAIL_SIZE) { myDataItem, requestBuilder -> + * // THUMBNAIL_SIZE is applied for you, but .load() is not because determining the model from + * // the underlying data isn't trivial. Don't forget to call .load()! + * requestBuilder.load(myDataItem.url) + * } + * + * LazyRow(...) { + * item { Text(text = "Header") } + * items(glidePreloadingData.size) { index -> + * val (myDataItem, preloadRequest) = glidePreloadingData[index] + * GlideImage(model = item.url, contentDescription = item.description, ...) { primaryRequest -> + * primaryRequest.thumbnail(preloadRequest) + * } + * } + * } + * ``` + * + * Note that preloading will not occur until the first access of `glidePreloadingData`. If you + * have multiple disjoint data sets that you'd like to preload, or have some number of preceding + * header rows prior to your first image, you can optionally add a few manual calls to make + * preloading continue smoothly across data sets. One way you might do so is to call the next data + * set toward the end of the previous data set, e.g.: + * + * ``` + * val itemsToPreload = 15 + * items(firstDataSet.size) { index -> + * ... // Do something with first data set. * - * @param state The [LazyListState] provided to the `LazyRow` or `LazyColumn` - * @param data The backing list of metadata that we're going to preload images for. - * @param size The override size we'll pass to [RequestBuilder.override] . + * // Then as you get to the end of the first data set, start preloading the next data set + * manually + * if (index >= firstDataSet.size - itemsToPreload) { + * nextDataSet[itemsToPreload - (firstDataSet.size - index)] + * } + * } + * ``` + * + * @param dataSize The total number of items to display and preload. + * @param dataGetter A getter for the item at the given index (ie [List.get]. + * @param preloadImageSize The override size we'll pass to [RequestBuilder.override] . * @param numberOfItemsToPreload The number of items to preload ahead of the user's current * position. This should be tested for each application. If the total memory size of the preloaded * images exceeds the memory cache size, preloading for a lazy list is not effective. However if you @@ -38,136 +72,179 @@ import com.bumptech.glide.RequestManager * @param fixedVisibleItemCount The number of visible items. In some cases this can vary widely in * which case you can leave this value `null`. If the number of visible items is always one or two, * it might make sense to just set this to the larger of the two to reduce churn in the preloader. - * @param viewToDataPosition A function that can be used to map a view position to a position in - * [data]. If your `LazyRow` or `LazyColumn` contains only images so there's a 1:1 correspondence - * between the position in the view and the position in [data], you do not need to provide this - * function. Otherwise you need to provide a function that offsets the view position into [data] or - * returns `null` if the position corresponds to a view that isn't showing an image from [data]. - * TODO(judds): like the TODO below, we could handle this automatically with some more wrapping. - * @param requestBuilderTransform See [ListPreloader.PreloadModelProvider.getPreloadRequestBuilder] + * @param requestBuilderTransform See [ListPreloader.PreloadModelProvider.getPreloadRequestBuilder]. + * You should call [RequestBuilder.load] on the given `item` so that any type specific options + * applied the matching [RequestManager.load] method are applied identically to the preload request. + * Remember that the request produced by this transform must exactly match the request made in your + * non-preload request for preloading to be useful. */ -// TODO(judds): Consider wrapping a LazyRow / LazyColumn and providing state instead of a separate -// function. Wrapping might also make it easier to pass through the size and request builder -// modifications so that it's easier to make sure the preload size matches a size on the -// GlideImage @Composable -@ExperimentalGlideComposeApi -public fun GlideLazyListPreloader( - state: LazyListState, - data: List, - viewToDataPosition: ((Int) -> Int?)? = null, - size: Size, - numberOfItemsToPreload: Int, +public fun rememberGlidePreloadingData( + dataSize: Int, + dataGetter: (Int) -> DataT, + preloadImageSize: Size, + numberOfItemsToPreload: Int = DEFAULT_ITEMS_TO_PRELOAD, fixedVisibleItemCount: Int? = null, - requestBuilderTransform: PreloadRequestBuilderTransform, -) { - val preloader = - rememberGlidePreloader( - data = data, - viewToDataPosition = viewToDataPosition, - size = size, - numberOfItemsToPreload = numberOfItemsToPreload, - requestBuilderTransform = requestBuilderTransform, + requestBuilderTransform: PreloadRequestBuilderTransform, +): GlidePreloadingData { + val requestManager = LocalContext.current.let { remember(it) { Glide.with(it) } } + return remember( + requestManager, + dataSize, + dataGetter, + preloadImageSize, + numberOfItemsToPreload, + fixedVisibleItemCount, + requestBuilderTransform, + ) { + val preloaderData = + PreloaderData(dataSize, dataGetter, requestBuilderTransform, preloadImageSize) + val preloader = + ListPreloader( + requestManager, + PreloadModelProvider( + requestManager, + preloaderData, + ), + PreloadDimensionsProvider(preloaderData), + numberOfItemsToPreload, + ) + PreloadDataImpl( + dataSize, + dataGetter, + requestManager, + preloadImageSize, + fixedVisibleItemCount, + preloader, + requestBuilderTransform, ) - LaunchPreload(preloader = preloader, state = state, fixedVisibleItemCount = fixedVisibleItemCount) + } } +/** + * A helper for [rememberGlidePreloadingData] that accepts a [List]. See the more general equivalent + * for details. + */ @Composable -private fun LaunchPreload( - preloader: ListPreloader, - state: LazyListState, - fixedVisibleItemCount: Int? -) = - LaunchedEffect(preloader, state, fixedVisibleItemCount) { - snapshotFlow { state.lazyListVisibleInfo(fixedVisibleItemCount) } - .collect { lazyListVisibleInfo -> - preloader.onScroll( - /* absListView = */ null, - lazyListVisibleInfo.firstVisibleItemIndex, - lazyListVisibleInfo.visibleItemCount, - lazyListVisibleInfo.totalItemCount, - ) - } - } +public fun rememberGlidePreloadingData( + data: List, + preloadImageSize: Size, + numberOfItemsToPreload: Int = DEFAULT_ITEMS_TO_PRELOAD, + fixedVisibleItemCount: Int? = null, + requestBuilderTransform: PreloadRequestBuilderTransform, +): GlidePreloadingData { + return rememberGlidePreloadingData( + dataSize = data.size, + dataGetter = data::get, + preloadImageSize = preloadImageSize, + numberOfItemsToPreload = numberOfItemsToPreload, + fixedVisibleItemCount = fixedVisibleItemCount, + requestBuilderTransform = requestBuilderTransform, + ) +} -@Composable -private fun rememberGlidePreloader( - data: List, - viewToDataPosition: ((Int) -> Int?)?, - size: Size, - numberOfItemsToPreload: Int, - requestBuilderTransform: PreloadRequestBuilderTransform, -): ListPreloader { - val context = LocalContext.current - val requestManager = remember(context) { Glide.with(context) } - - val updatedData = rememberUpdatedState(data) - val updatedSize = rememberUpdatedState(size) - val actualViewToDataPosition = - viewToDataPosition - ?: actualViewToDataPosition@{ - if (it < updatedData.value.size) { - return@actualViewToDataPosition it - } - if (Log.isLoggable(Constants.TAG, Log.WARN)) { - Log.w( - Constants.TAG, - "Mismatch between view size ($it) and data size (${updatedData.value.size}), provide a" + - " viewToDataPosition to GlideLazyListPreloader", - ) - } - null - } - - return remember(requestManager, requestBuilderTransform, numberOfItemsToPreload) { - ListPreloader( - requestManager, - PreloadModelProvider( - requestManager, - requestBuilderTransform, - updatedData, - actualViewToDataPosition, - ), - { _, _, _ -> intArrayOf(updatedSize.value.width.toInt(), updatedSize.value.height.toInt()) }, - numberOfItemsToPreload, - ) +private data class PreloaderData( + val dataSize: Int, + val dataAccessor: (Int) -> DataT, + val requestBuilderTransform: PreloadRequestBuilderTransform, + val size: Size, +) { + fun preloadRequests( + requestManager: RequestManager, + item: DataT, + ): RequestBuilder { + return requestBuilderTransform(item, requestManager.asDrawable()) } } +/** + * Wraps a set of data, triggers image preloads based on the positions provided to [get] and exposes + * the data and the preload [RequestBuilder]. + */ +public interface GlidePreloadingData { + /** The total number of items in the data set. */ + public val size: Int + + /** + * Returns the [DataT] at a given index in the data and a [RequestBuilder] that will trigger a + * request that exactly matches the preload request for this index. + * + * The returned [RequestBuilder] should always be used to display the item at the given index. + * Otherwise the preload request triggered by this call is likely useless work. The + * [RequestBuilder] can either be used as the primary request, or more likely, passed as the + * [RequestBuilder.thumbnail] to a higher resolution request. + * + * This method has side affects! Calling it will trigger preloads based on the given [index]. + * Preloading assumes sequential access in a manner that matches what the user will see. If you + * need to look up data at indices for other reasons, use the underlying data source directly so + * that you do not confuse the preloader. Only use this method when obtaining data to display to + * the user. + */ + @Composable + public operator fun get(index: Int): Pair> +} -private class PreloadModelProvider( +private class PreloadDataImpl( + override val size: Int, + private val indexToData: (Int) -> DataT, private val requestManager: RequestManager, - private val requestBuilderTransform: PreloadRequestBuilderTransform, - private val data: State>, - private val viewToDataPosition: (Int) -> Int?, -) : ListPreloader.PreloadModelProvider { - override fun getPreloadItems(viewPosition: Int): List { - val dataPosition = viewToDataPosition(viewPosition) - return dataPosition?.let { listOf(this.data.value[it]) } ?: listOf() - } + private val preloadImageSize: Size, + private val fixedVisibleItemCount: Int?, + private val preloader: ListPreloader, + private val requestBuilderTransform: PreloadRequestBuilderTransform, +) : GlidePreloadingData { + + @Composable + override fun get(index: Int): Pair> { + val item = indexToData(index) + val requestBuilder = + requestBuilderTransform( + item, + requestManager.asDrawable() + .override(preloadImageSize.width.toInt(), preloadImageSize.height.toInt()), + ) - override fun getPreloadRequestBuilder(item: DataTypeT): RequestBuilder<*> { - return requestBuilderTransform(item, requestManager.asDrawable().load(item)) + LaunchedEffect(preloader, preloadImageSize, requestBuilderTransform, indexToData, index) { + preloader.onScroll( + /* absListView = */ null, + index, + fixedVisibleItemCount ?: 1, + size, + ) + } + return item to requestBuilder } } -private fun LazyListState.lazyListVisibleInfo(fixedVisibleItemCount: Int?) = - LazyListVisibleInfo( - firstVisibleItemIndex = firstVisibleItemIndex, - visibleItemCount = fixedVisibleItemCount ?: layoutInfo.visibleItemsInfo.size, - totalItemCount = layoutInfo.totalItemsCount - ) +private class PreloadDimensionsProvider( + private val updatedData: PreloaderData, +) : ListPreloader.PreloadSizeProvider { + override fun getPreloadSize(item: DataT, adapterPosition: Int, perItemPosition: Int): IntArray = + updatedData.size.toIntArray() +} -@Immutable -private data class LazyListVisibleInfo( - val firstVisibleItemIndex: Int, - val visibleItemCount: Int, - val totalItemCount: Int, -) +private fun Size.toIntArray() = intArrayOf(width.toInt(), height.toInt()) -private typealias PreloadRequestBuilderTransform = - (item: DataTypeT, requestBuilder: RequestBuilder<*>) -> RequestBuilder<*> +private class PreloadModelProvider( + private val requestManager: RequestManager, + private val data: PreloaderData, +) : ListPreloader.PreloadModelProvider { + + override fun getPreloadItems(position: Int): MutableList { + return mutableListOf(data.dataAccessor(position)) + } -private object Constants { - const val TAG = "GlidePreloader" + override fun getPreloadRequestBuilder(item: DataT): RequestBuilder<*> { + return data.preloadRequests(requestManager, item) + } } + +/** + * Provides the data to load and a [RequestBuilder] to load it with. + * + * You must at least call [RequestBuilder.load] with the appropriate model extracted from `item` on + * the given `requestBuilder`. You can also optionally call any other methods available on + * `requestBuilder` to customize your load. + */ +public typealias PreloadRequestBuilderTransform = + (item: DataTypeT, requestBuilder: RequestBuilder) -> RequestBuilder \ No newline at end of file diff --git a/library/src/main/java/com/bumptech/glide/ListPreloader.java b/library/src/main/java/com/bumptech/glide/ListPreloader.java index 46fd4a188d..380d32c28a 100644 --- a/library/src/main/java/com/bumptech/glide/ListPreloader.java +++ b/library/src/main/java/com/bumptech/glide/ListPreloader.java @@ -181,12 +181,16 @@ private void preload(int from, int to) { if (from < to) { // Increasing for (int i = start; i < end; i++) { - preloadAdapterPosition(preloadModelProvider.getPreloadItems(i), i, true); + preloadAdapterPosition( + preloadModelProvider.getPreloadItems(i), /* position= */ i, /* isIncreasing= */ true); } } else { // Decreasing for (int i = end - 1; i >= start; i--) { - preloadAdapterPosition(preloadModelProvider.getPreloadItems(i), i, false); + preloadAdapterPosition( + preloadModelProvider.getPreloadItems(i), + /* position= */ i, + /* isIncreasing= */ false); } } diff --git a/samples/gallery/build.gradle b/samples/gallery/build.gradle index 9025bd7f55..875578ead9 100644 --- a/samples/gallery/build.gradle +++ b/samples/gallery/build.gradle @@ -16,7 +16,7 @@ dependencies { implementation "org.jetbrains.kotlinx:kotlinx-coroutines-core:$JETBRAINS_KOTLINX_COROUTINES_VERSION" implementation "org.jetbrains.kotlinx:kotlinx-coroutines-android:$JETBRAINS_KOTLINX_COROUTINES_VERSION" implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$JETBRAINS_KOTLIN_VERSION" - implementation "androidx.compose.foundation:foundation:$ANDROID_X_COMPOSE_VERSION" + implementation "androidx.compose.foundation:foundation:$ANDROID_X_COMPOSE_FOUNDATION_VERSION" implementation "androidx.compose.ui:ui:$ANDROID_X_COMPOSE_VERSION" ksp project(':annotation:ksp') @@ -29,12 +29,12 @@ kotlin { } android { - compileSdk 32 + compileSdk 33 defaultConfig { applicationId 'com.bumptech.glide.samples.gallery' minSdk 29 - targetSdk 32 + targetSdk 33 versionCode 1 versionName '1.0' } 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 962f44c935..7b9a823f6f 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 @@ -1,5 +1,6 @@ package com.bumptech.glide.samples.gallery +import android.graphics.drawable.Drawable import android.os.Bundle import androidx.fragment.app.Fragment import android.view.LayoutInflater @@ -7,22 +8,17 @@ import android.view.View import android.view.ViewGroup import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.lazy.LazyRow -import androidx.compose.foundation.lazy.items -import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState -import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.geometry.Size import androidx.compose.ui.platform.ComposeView -import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.unit.dp import androidx.fragment.app.viewModels -import com.bumptech.glide.Glide -import com.bumptech.glide.RequestManager +import com.bumptech.glide.RequestBuilder import com.bumptech.glide.integration.compose.ExperimentalGlideComposeApi import com.bumptech.glide.integration.compose.GlideImage -import com.bumptech.glide.integration.compose.GlideLazyListPreloader +import com.bumptech.glide.integration.compose.rememberGlidePreloadingData import com.bumptech.glide.signature.MediaStoreSignature /** Displays media store data in a recycler view. */ @@ -48,51 +44,37 @@ class HorizontalGalleryFragment : Fragment() { @Composable fun DeviceMedia(mediaStoreData: List) { - val state = rememberLazyListState() - val requestManager = rememberRequestManager() - LazyRow(horizontalArrangement = Arrangement.spacedBy(10.dp), state = state) { - items(mediaStoreData) { mediaStoreItem -> - MediaStoreView(mediaStoreItem, requestManager, Modifier.fillParentMaxSize()) + val requestBuilderTransform = + { item: MediaStoreData, requestBuilder: RequestBuilder -> + requestBuilder.load(item.uri).signature(item.signature()) } - } - GlideLazyListPreloader( - state = state, - data = mediaStoreData, - size = THUMBNAIL_SIZE, - numberOfItemsToPreload = 15, - fixedVisibleItemCount = 2, - ) { item, requestBuilder -> - requestBuilder.load(item.uri).signature(item.signature()) + val preloadingData = + rememberGlidePreloadingData( + mediaStoreData, + THUMBNAIL_SIZE, + requestBuilderTransform = requestBuilderTransform, + ) + + LazyRow(horizontalArrangement = Arrangement.spacedBy(10.dp)) { + items(preloadingData.size) { index -> + val (mediaStoreItem, preloadRequestBuilder) = preloadingData[index] + MediaStoreView(mediaStoreItem, preloadRequestBuilder, Modifier.fillParentMaxSize()) + } } } - @Composable - private fun rememberRequestManager() = - LocalContext.current.let { remember(it) { Glide.with(it) } } - private fun MediaStoreData.signature() = MediaStoreSignature(mimeType, dateModified, orientation) @Composable - fun MediaStoreView(item: MediaStoreData, requestManager: RequestManager, modifier: Modifier) { - val signature = item.signature() - - GlideImage( - model = item.uri, - contentDescription = item.displayName, - modifier = modifier, - ) { - it - .thumbnail( - requestManager - .asDrawable() - .load(item.uri) - .signature(signature) - .override(THUMBNAIL_DIMENSION) - ) - .signature(signature) + fun MediaStoreView( + item: MediaStoreData, + preloadRequestBuilder: RequestBuilder, + modifier: Modifier, + ) = + GlideImage(model = item.uri, contentDescription = item.displayName, modifier = modifier) { + it.thumbnail(preloadRequestBuilder).signature(item.signature()) } - } companion object { private const val THUMBNAIL_DIMENSION = 50