From beebb0ad5c9fa21422ba3bc6615d7354a8afde01 Mon Sep 17 00:00:00 2001 From: Colin White Date: Wed, 13 Nov 2024 12:01:15 -0800 Subject: [PATCH] Fix memory cache misses in some cases where precision is not set. (#2680) * Fix scale not being properly passed around. * Update the request instead of the defaults. --- .../kotlin/coil3/compose/AsyncImageTest.kt | 4 +- coil-core/api/android/coil-core.api | 4 +- coil-core/api/coil-core.klib.api | 1 + coil-core/api/jvm/coil-core.api | 4 +- .../coil3/RealImageLoaderAndroidTest.kt | 3 +- .../coil3/request/RequestService.android.kt | 71 ++++++++++--------- .../coil3/transition/CrossfadeTransition.kt | 19 +---- .../coil3/request/RequestServiceTest.kt | 62 +++++++--------- .../kotlin/coil3/RealImageLoader.kt | 9 +-- .../kotlin/coil3/decode/DecodeUtils.kt | 2 - .../coil3/intercept/EngineInterceptor.kt | 6 +- .../coil3/intercept/RealInterceptorChain.kt | 3 - .../kotlin/coil3/request/ImageRequest.kt | 36 ++++++++++ .../kotlin/coil3/request/RequestService.kt | 7 +- .../src/commonMain/kotlin/coil3/util/utils.kt | 4 -- .../intercept/RealInterceptorChainTest.kt | 2 - .../request/RequestService.nonAndroid.kt | 25 +++---- 17 files changed, 131 insertions(+), 131 deletions(-) diff --git a/coil-compose-core/src/androidInstrumentedTest/kotlin/coil3/compose/AsyncImageTest.kt b/coil-compose-core/src/androidInstrumentedTest/kotlin/coil3/compose/AsyncImageTest.kt index 3e2b082f03..a05d549880 100644 --- a/coil-compose-core/src/androidInstrumentedTest/kotlin/coil3/compose/AsyncImageTest.kt +++ b/coil-compose-core/src/androidInstrumentedTest/kotlin/coil3/compose/AsyncImageTest.kt @@ -58,13 +58,13 @@ import coil3.size.Scale import coil3.test.utils.ComposeTestActivity import coil3.test.utils.context import java.util.concurrent.atomic.AtomicInteger +import kotlin.coroutines.EmptyCoroutineContext import kotlin.test.assertContains import kotlin.test.assertEquals import kotlin.test.assertIs import kotlin.test.assertTrue import kotlin.test.fail import kotlin.time.Duration.Companion.milliseconds -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.flow.flow import org.junit.After @@ -577,7 +577,7 @@ class AsyncImageTest { model = ImageRequest.Builder(LocalContext.current) .data("https://example.com/image") .memoryCachePolicy(CachePolicy.ENABLED) - .coroutineContext(Dispatchers.Main.immediate) + .coroutineContext(EmptyCoroutineContext) .build(), contentDescription = null, imageLoader = imageLoader, diff --git a/coil-core/api/android/coil-core.api b/coil-core/api/android/coil-core.api index 7cf5d82594..ba35b52612 100644 --- a/coil-core/api/android/coil-core.api +++ b/coil-core/api/android/coil-core.api @@ -665,8 +665,10 @@ public final class coil3/request/ImageRequest$Defaults { public fun ()V public fun (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;)V public synthetic fun (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults; + public final synthetic fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults; + public final fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults; public static synthetic fun copy$default (Lcoil3/request/ImageRequest$Defaults;Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;ILjava/lang/Object;)Lcoil3/request/ImageRequest$Defaults; + public static synthetic fun copy$default (Lcoil3/request/ImageRequest$Defaults;Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;ILjava/lang/Object;)Lcoil3/request/ImageRequest$Defaults; public fun equals (Ljava/lang/Object;)Z public final fun getDecoderCoroutineContext ()Lkotlin/coroutines/CoroutineContext; public final fun getDiskCachePolicy ()Lcoil3/request/CachePolicy; diff --git a/coil-core/api/coil-core.klib.api b/coil-core/api/coil-core.klib.api index e54ec80e19..957820d4a6 100644 --- a/coil-core/api/coil-core.klib.api +++ b/coil-core/api/coil-core.klib.api @@ -659,6 +659,7 @@ final class coil3.request/ImageRequest { // coil3.request/ImageRequest|null[0] final fun (): coil3.size/SizeResolver // coil3.request/ImageRequest.Defaults.sizeResolver.|(){}[0] final fun copy(okio/FileSystem = ..., kotlin.coroutines/CoroutineContext = ..., kotlin.coroutines/CoroutineContext = ..., kotlin.coroutines/CoroutineContext = ..., coil3.request/CachePolicy = ..., coil3.request/CachePolicy = ..., coil3.request/CachePolicy = ..., kotlin/Function1 = ..., kotlin/Function1 = ..., kotlin/Function1 = ..., coil3.size/Precision = ..., coil3/Extras = ...): coil3.request/ImageRequest.Defaults // coil3.request/ImageRequest.Defaults.copy|copy(okio.FileSystem;kotlin.coroutines.CoroutineContext;kotlin.coroutines.CoroutineContext;kotlin.coroutines.CoroutineContext;coil3.request.CachePolicy;coil3.request.CachePolicy;coil3.request.CachePolicy;kotlin.Function1;kotlin.Function1;kotlin.Function1;coil3.size.Precision;coil3.Extras){}[0] + final fun copy(okio/FileSystem = ..., kotlin.coroutines/CoroutineContext = ..., kotlin.coroutines/CoroutineContext = ..., kotlin.coroutines/CoroutineContext = ..., coil3.request/CachePolicy = ..., coil3.request/CachePolicy = ..., coil3.request/CachePolicy = ..., kotlin/Function1 = ..., kotlin/Function1 = ..., kotlin/Function1 = ..., coil3.size/SizeResolver = ..., coil3.size/Scale = ..., coil3.size/Precision = ..., coil3/Extras = ...): coil3.request/ImageRequest.Defaults // coil3.request/ImageRequest.Defaults.copy|copy(okio.FileSystem;kotlin.coroutines.CoroutineContext;kotlin.coroutines.CoroutineContext;kotlin.coroutines.CoroutineContext;coil3.request.CachePolicy;coil3.request.CachePolicy;coil3.request.CachePolicy;kotlin.Function1;kotlin.Function1;kotlin.Function1;coil3.size.SizeResolver;coil3.size.Scale;coil3.size.Precision;coil3.Extras){}[0] final fun equals(kotlin/Any?): kotlin/Boolean // coil3.request/ImageRequest.Defaults.equals|equals(kotlin.Any?){}[0] final fun hashCode(): kotlin/Int // coil3.request/ImageRequest.Defaults.hashCode|hashCode(){}[0] final fun toString(): kotlin/String // coil3.request/ImageRequest.Defaults.toString|toString(){}[0] diff --git a/coil-core/api/jvm/coil-core.api b/coil-core/api/jvm/coil-core.api index 03db460107..049668db8a 100644 --- a/coil-core/api/jvm/coil-core.api +++ b/coil-core/api/jvm/coil-core.api @@ -582,8 +582,10 @@ public final class coil3/request/ImageRequest$Defaults { public fun ()V public fun (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;)V public synthetic fun (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults; + public final synthetic fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults; + public final fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults; public static synthetic fun copy$default (Lcoil3/request/ImageRequest$Defaults;Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;ILjava/lang/Object;)Lcoil3/request/ImageRequest$Defaults; + public static synthetic fun copy$default (Lcoil3/request/ImageRequest$Defaults;Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;ILjava/lang/Object;)Lcoil3/request/ImageRequest$Defaults; public fun equals (Ljava/lang/Object;)Z public final fun getDecoderCoroutineContext ()Lkotlin/coroutines/CoroutineContext; public final fun getDiskCachePolicy ()Lcoil3/request/CachePolicy; diff --git a/coil-core/src/androidInstrumentedTest/kotlin/coil3/RealImageLoaderAndroidTest.kt b/coil-core/src/androidInstrumentedTest/kotlin/coil3/RealImageLoaderAndroidTest.kt index 5989f443fe..d6f8f95475 100644 --- a/coil-core/src/androidInstrumentedTest/kotlin/coil3/RealImageLoaderAndroidTest.kt +++ b/coil-core/src/androidInstrumentedTest/kotlin/coil3/RealImageLoaderAndroidTest.kt @@ -40,6 +40,7 @@ import coil3.util.getDrawableCompat import coil3.util.toDrawable import java.io.File import java.nio.ByteBuffer +import kotlin.coroutines.EmptyCoroutineContext import kotlin.coroutines.resume import kotlin.coroutines.resumeWithException import kotlin.math.roundToInt @@ -268,7 +269,7 @@ class RealImageLoaderAndroidTest { .size(100, 100) .precision(Precision.INEXACT) .allowHardware(true) - .coroutineContext(Dispatchers.Main.immediate) + .coroutineContext(EmptyCoroutineContext) .target( onStart = { // The drawable in the memory cache should be returned here. diff --git a/coil-core/src/androidMain/kotlin/coil3/request/RequestService.android.kt b/coil-core/src/androidMain/kotlin/coil3/request/RequestService.android.kt index 523fb41335..672183dc46 100644 --- a/coil-core/src/androidMain/kotlin/coil3/request/RequestService.android.kt +++ b/coil-core/src/androidMain/kotlin/coil3/request/RequestService.android.kt @@ -9,7 +9,6 @@ import coil3.BitmapImage import coil3.Extras import coil3.ImageLoader import coil3.memory.MemoryCache -import coil3.size.Dimension import coil3.size.Precision import coil3.size.Scale import coil3.size.Size @@ -70,32 +69,33 @@ internal class AndroidRequestService( return context.getLifecycle() } - override fun sizeResolver(request: ImageRequest): SizeResolver { - if (request.defined.sizeResolver != null) { - return request.defined.sizeResolver + override fun updateRequest(request: ImageRequest): ImageRequest { + val builder = request.newBuilder() + .defaults(imageLoader.defaults) + + var sizeResolver = request.defined.sizeResolver + if (sizeResolver == null) { + sizeResolver = request.resolveSizeResolver() + builder.size(sizeResolver) } - val target = request.target - if (target is ViewTarget<*>) { - // CENTER and MATRIX scale types should be decoded at the image's original size. - val view = target.view - if (view is ImageView && view.scaleType.let { it == CENTER || it == MATRIX }) { - return SizeResolver.ORIGINAL - } else { - return ViewSizeResolver(view) - } - } else { - // Fall back to the image's source dimensions. - return SizeResolver.ORIGINAL + if (request.defined.scale == null) { + builder.scale(request.resolveScale()) } + + if (request.defined.precision == null) { + builder.precision(request.resolvePrecision(sizeResolver)) + } + + return builder.build() } - override fun options(request: ImageRequest, sizeResolver: SizeResolver, size: Size): Options { + override fun options(request: ImageRequest, size: Size): Options { return Options( request.context, size, - request.resolveScale(size), - request.resolvePrecision(sizeResolver), + request.scale, + request.precision, request.diskCacheKey, request.fileSystem, request.memoryCachePolicy, @@ -105,16 +105,22 @@ internal class AndroidRequestService( ) } - private fun ImageRequest.resolveScale(size: Size): Scale { - if (defined.scale != null) { - return defined.scale - } - - // Use `Scale.FIT` if either dimension is undefined. - if (size.width == Dimension.Undefined || size.height == Dimension.Undefined) { - return Scale.FIT + private fun ImageRequest.resolveSizeResolver(): SizeResolver { + if (target is ViewTarget<*>) { + // CENTER and MATRIX scale types should be decoded at the image's original size. + val view = target.view + if (view is ImageView && view.scaleType.let { it == CENTER || it == MATRIX }) { + return SizeResolver.ORIGINAL + } else { + return ViewSizeResolver(view) + } + } else { + // Fall back to the image's source dimensions. + return SizeResolver.ORIGINAL } + } + private fun ImageRequest.resolveScale(): Scale { // Autodetect the scale from the ImageView. val imageView = (target as? ViewTarget<*>)?.view as? ImageView if (imageView != null) { @@ -125,10 +131,7 @@ internal class AndroidRequestService( } private fun ImageRequest.resolvePrecision(sizeResolver: SizeResolver): Precision { - if (defined.precision != null) { - return defined.precision - } - + // Use inexact precision if we're falling back to the source dimensions. if (defined.sizeResolver == null && sizeResolver == SizeResolver.ORIGINAL) { return Precision.INEXACT } @@ -173,7 +176,7 @@ internal class AndroidRequestService( return builder.build() } - override fun updateOptionsOnWorkerThread(options: Options): Options { + override fun updateOptions(options: Options): Options { var extras = options.extras var changed = false @@ -233,7 +236,7 @@ internal class AndroidRequestService( } /** Return 'true' if the current bitmap config is valid, else use [Bitmap.Config.ARGB_8888]. */ - fun isBitmapConfigValidMainThread( + private fun isBitmapConfigValidMainThread( request: ImageRequest, size: Size, ): Boolean { @@ -246,7 +249,7 @@ internal class AndroidRequestService( } /** Return 'true' if the current bitmap config is valid, else use [Bitmap.Config.ARGB_8888]. */ - fun isBitmapConfigValidWorkerThread( + private fun isBitmapConfigValidWorkerThread( options: Options, ): Boolean { return !options.bitmapConfig.isHardware || hardwareBitmapService.allowHardwareWorkerThread() diff --git a/coil-core/src/androidMain/kotlin/coil3/transition/CrossfadeTransition.kt b/coil-core/src/androidMain/kotlin/coil3/transition/CrossfadeTransition.kt index c316a199cc..82510e3da3 100644 --- a/coil-core/src/androidMain/kotlin/coil3/transition/CrossfadeTransition.kt +++ b/coil-core/src/androidMain/kotlin/coil3/transition/CrossfadeTransition.kt @@ -1,6 +1,5 @@ package coil3.transition -import android.widget.ImageView import coil3.asDrawable import coil3.asImage import coil3.decode.DataSource @@ -8,8 +7,6 @@ import coil3.request.DEFAULT_CROSSFADE_MILLIS import coil3.request.ErrorResult import coil3.request.ImageResult import coil3.request.SuccessResult -import coil3.size.Scale -import coil3.util.scale /** * A [Transition] that crossfades from the current drawable to a new one. @@ -32,7 +29,7 @@ class CrossfadeTransition @JvmOverloads constructor( val drawable = CrossfadeDrawable( start = target.drawable, end = result.image?.asDrawable(target.view.resources), - scale = scale(), + scale = result.request.scale, durationMillis = durationMillis, fadeStart = result !is SuccessResult || !result.isPlaceholderCached, preferExactIntrinsicSize = preferExactIntrinsicSize, @@ -43,20 +40,6 @@ class CrossfadeTransition @JvmOverloads constructor( } } - private fun scale(): Scale { - val scale = result.request.defined.scale - if (scale != null) { - return scale - } - - val view = target.view - if (view is ImageView) { - return view.scale - } - - return result.request.scale - } - class Factory @JvmOverloads constructor( val durationMillis: Int = DEFAULT_CROSSFADE_MILLIS, val preferExactIntrinsicSize: Boolean = false, diff --git a/coil-core/src/androidUnitTest/kotlin/coil3/request/RequestServiceTest.kt b/coil-core/src/androidUnitTest/kotlin/coil3/request/RequestServiceTest.kt index b4c6fbc574..6ac1735f62 100644 --- a/coil-core/src/androidUnitTest/kotlin/coil3/request/RequestServiceTest.kt +++ b/coil-core/src/androidUnitTest/kotlin/coil3/request/RequestServiceTest.kt @@ -35,9 +35,8 @@ class RequestServiceTest : RobolectricTest() { target(ImageView(context)) precision(Precision.EXACT) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.EXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.EXACT, options.precision) } @Test @@ -45,9 +44,8 @@ class RequestServiceTest : RobolectricTest() { val request = createRequest(context) { precision(Precision.INEXACT) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.INEXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.INEXACT, options.precision) } @Test @@ -55,9 +53,8 @@ class RequestServiceTest : RobolectricTest() { val request = createRequest(context) { target(ImageView(context)) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.INEXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.INEXACT, options.precision) } @Test @@ -66,9 +63,8 @@ class RequestServiceTest : RobolectricTest() { target(ImageView(context)) size(100, 100) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.EXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.EXACT, options.precision) } @Test @@ -78,9 +74,8 @@ class RequestServiceTest : RobolectricTest() { target(imageView) size(ViewSizeResolver(imageView)) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.INEXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.INEXACT, options.precision) } @Test @@ -89,17 +84,15 @@ class RequestServiceTest : RobolectricTest() { target(ImageView(context)) size(ViewSizeResolver(ImageView(context))) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.EXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.EXACT, options.precision) } @Test fun `allowInexactSize - unspecified SizeResolver`() { val request = createRequest(context) - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.INEXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.INEXACT, options.precision) } @Test @@ -108,9 +101,8 @@ class RequestServiceTest : RobolectricTest() { target { /* Empty. */ } size(100, 100) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.EXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.EXACT, options.precision) } @Test @@ -120,9 +112,8 @@ class RequestServiceTest : RobolectricTest() { override val view = View(context) }) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.EXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.EXACT, options.precision) } @Test @@ -130,9 +121,8 @@ class RequestServiceTest : RobolectricTest() { val request = createRequest(context) { size(100, 100) } - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val actual = service.options(request, sizeResolver, Size(100, 100)).precision - assertEquals(Precision.EXACT, actual) + val options = service.options(service.updateRequest(request), Size(100, 100)) + assertEquals(Precision.EXACT, options.precision) } /** Regression test: https://github.com/coil-kt/coil/issues/1768 */ @@ -142,8 +132,7 @@ class RequestServiceTest : RobolectricTest() { val request = ImageRequest.Builder(context) .bitmapConfig(Bitmap.Config.RGB_565) .build() - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val options = service.options(request, sizeResolver, Size(100, 100)) + val options = service.options(service.updateRequest(request), Size(100, 100)) assertEquals(Bitmap.Config.RGB_565, options.bitmapConfig) } @@ -152,12 +141,12 @@ class RequestServiceTest : RobolectricTest() { fun `ImageLoader bitmapConfig is preserved`() { val imageLoader = ImageLoader.Builder(context) .bitmapConfig(Bitmap.Config.RGB_565) - .build() + .build() as RealImageLoader val request = ImageRequest.Builder(context) .defaults(imageLoader.defaults) .build() - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val options = service.options(request, sizeResolver, Size(100, 100)) + val service = RequestService(imageLoader, SystemCallbacks(imageLoader), null) + val options = service.options(service.updateRequest(request), Size(100, 100)) assertEquals(Bitmap.Config.RGB_565, options.bitmapConfig) } @@ -169,8 +158,7 @@ class RequestServiceTest : RobolectricTest() { val request = ImageRequest.Builder(context) .target(imageView) .build() - val sizeResolver = request.defined.sizeResolver ?: service.sizeResolver(request) - val options = service.options(request, sizeResolver, Size(100, 100)) + val options = service.options(service.updateRequest(request), Size(100, 100)) assertEquals(Scale.FILL, options.scale) } } diff --git a/coil-core/src/commonMain/kotlin/coil3/RealImageLoader.kt b/coil-core/src/commonMain/kotlin/coil3/RealImageLoader.kt index 85d3d56670..7a6e157b79 100644 --- a/coil-core/src/commonMain/kotlin/coil3/RealImageLoader.kt +++ b/coil-core/src/commonMain/kotlin/coil3/RealImageLoader.kt @@ -96,10 +96,8 @@ internal class RealImageLoader( findLifecycle = type == REQUEST_TYPE_ENQUEUE, ).apply { assertActive() } - // Apply this image loader's defaults to this request. - val request = initialRequest.newBuilder() - .defaults(defaults) - .build() + // Apply this image loader's defaults and other configuration to this request. + val request = requestService.updateRequest(initialRequest) // Create a new event listener. val eventListener = options.eventListenerFactory.create(request) @@ -125,7 +123,7 @@ internal class RealImageLoader( request.listener?.onStart(request) // Resolve the size. - val sizeResolver = requestService.sizeResolver(request) + val sizeResolver = request.sizeResolver eventListener.resolveSizeStart(request, sizeResolver) val size = sizeResolver.size() eventListener.resolveSizeEnd(request, size) @@ -138,7 +136,6 @@ internal class RealImageLoader( index = 0, request = request, size = size, - sizeResolver = sizeResolver, eventListener = eventListener, isPlaceholderCached = cachedPlaceholder != null, ).proceed() diff --git a/coil-core/src/commonMain/kotlin/coil3/decode/DecodeUtils.kt b/coil-core/src/commonMain/kotlin/coil3/decode/DecodeUtils.kt index dd26228d7d..010a2617b1 100644 --- a/coil-core/src/commonMain/kotlin/coil3/decode/DecodeUtils.kt +++ b/coil-core/src/commonMain/kotlin/coil3/decode/DecodeUtils.kt @@ -1,6 +1,5 @@ package coil3.decode -import coil3.annotation.ExperimentalCoilApi import coil3.size.Dimension import coil3.size.Scale import coil3.size.Size @@ -92,7 +91,6 @@ object DecodeUtils { * scaled into. The returned dimensions can be passed to [computeSizeMultiplier] to get the * final size multiplier. */ - @ExperimentalCoilApi @JvmStatic fun computeDstSize( srcWidth: Int, diff --git a/coil-core/src/commonMain/kotlin/coil3/intercept/EngineInterceptor.kt b/coil-core/src/commonMain/kotlin/coil3/intercept/EngineInterceptor.kt index dcaab5b5dd..984546d96b 100644 --- a/coil-core/src/commonMain/kotlin/coil3/intercept/EngineInterceptor.kt +++ b/coil-core/src/commonMain/kotlin/coil3/intercept/EngineInterceptor.kt @@ -24,7 +24,6 @@ import coil3.util.addFirst import coil3.util.closeQuietly import coil3.util.eventListener import coil3.util.isPlaceholderCached -import coil3.util.sizeResolver import kotlinx.coroutines.CancellationException import kotlinx.coroutines.withContext @@ -42,9 +41,8 @@ internal class EngineInterceptor( val request = chain.request val data = request.data val size = chain.size - val sizeResolver = chain.sizeResolver val eventListener = chain.eventListener - val options = requestService.options(request, sizeResolver, size) + val options = requestService.options(request, size) val scale = options.scale // Perform any data mapping. @@ -103,7 +101,7 @@ internal class EngineInterceptor( var components = imageLoader.components var fetchResult: FetchResult? = null val executeResult = try { - options = requestService.updateOptionsOnWorkerThread(options) + options = requestService.updateOptions(options) if (request.fetcherFactory != null || request.decoderFactory != null) { components = components.newBuilder() diff --git a/coil-core/src/commonMain/kotlin/coil3/intercept/RealInterceptorChain.kt b/coil-core/src/commonMain/kotlin/coil3/intercept/RealInterceptorChain.kt index d064a14ddd..743c4a58ff 100644 --- a/coil-core/src/commonMain/kotlin/coil3/intercept/RealInterceptorChain.kt +++ b/coil-core/src/commonMain/kotlin/coil3/intercept/RealInterceptorChain.kt @@ -5,7 +5,6 @@ import coil3.request.ImageRequest import coil3.request.ImageResult import coil3.request.NullRequestData import coil3.size.Size -import coil3.size.SizeResolver internal class RealInterceptorChain( val initialRequest: ImageRequest, @@ -13,7 +12,6 @@ internal class RealInterceptorChain( val index: Int, override val request: ImageRequest, override val size: Size, - val sizeResolver: SizeResolver, val eventListener: EventListener, val isPlaceholderCached: Boolean, ) : Interceptor.Chain { @@ -61,7 +59,6 @@ internal class RealInterceptorChain( index = index, request = request, size = size, - sizeResolver = sizeResolver, eventListener = eventListener, isPlaceholderCached = isPlaceholderCached, ) diff --git a/coil-core/src/commonMain/kotlin/coil3/request/ImageRequest.kt b/coil-core/src/commonMain/kotlin/coil3/request/ImageRequest.kt index 6c5c7c7963..bb0a3f5f98 100644 --- a/coil-core/src/commonMain/kotlin/coil3/request/ImageRequest.kt +++ b/coil-core/src/commonMain/kotlin/coil3/request/ImageRequest.kt @@ -230,6 +230,42 @@ class ImageRequest private constructor( val precision: Precision = Precision.EXACT, val extras: Extras = Extras.EMPTY, ) { + fun copy( + fileSystem: FileSystem = this.fileSystem, + interceptorCoroutineContext: CoroutineContext = this.interceptorCoroutineContext, + fetcherCoroutineContext: CoroutineContext = this.fetcherCoroutineContext, + decoderCoroutineContext: CoroutineContext = this.decoderCoroutineContext, + memoryCachePolicy: CachePolicy = this.memoryCachePolicy, + diskCachePolicy: CachePolicy = this.diskCachePolicy, + networkCachePolicy: CachePolicy = this.networkCachePolicy, + placeholderFactory: (ImageRequest) -> Image? = this.placeholderFactory, + errorFactory: (ImageRequest) -> Image? = this.errorFactory, + fallbackFactory: (ImageRequest) -> Image? = this.fallbackFactory, + sizeResolver: SizeResolver = this.sizeResolver, + scale: Scale = this.scale, + precision: Precision = this.precision, + extras: Extras = this.extras, + ) = Defaults( + fileSystem = fileSystem, + interceptorCoroutineContext = interceptorCoroutineContext, + fetcherCoroutineContext = fetcherCoroutineContext, + decoderCoroutineContext = decoderCoroutineContext, + memoryCachePolicy = memoryCachePolicy, + diskCachePolicy = diskCachePolicy, + networkCachePolicy = networkCachePolicy, + placeholderFactory = placeholderFactory, + errorFactory = errorFactory, + fallbackFactory = fallbackFactory, + sizeResolver = sizeResolver, + scale = scale, + precision = precision, + extras = extras, + ) + + @Deprecated( + level = DeprecationLevel.HIDDEN, + message = "Kept for binary compatibility." + ) fun copy( fileSystem: FileSystem = this.fileSystem, interceptorCoroutineContext: CoroutineContext = this.interceptorCoroutineContext, diff --git a/coil-core/src/commonMain/kotlin/coil3/request/RequestService.kt b/coil-core/src/commonMain/kotlin/coil3/request/RequestService.kt index 9910a39c69..7ab2591267 100644 --- a/coil-core/src/commonMain/kotlin/coil3/request/RequestService.kt +++ b/coil-core/src/commonMain/kotlin/coil3/request/RequestService.kt @@ -5,7 +5,6 @@ import coil3.annotation.MainThread import coil3.annotation.WorkerThread import coil3.memory.MemoryCache import coil3.size.Size -import coil3.size.SizeResolver import coil3.util.Logger import coil3.util.SystemCallbacks import kotlinx.coroutines.Job @@ -23,13 +22,13 @@ internal interface RequestService { fun requestDelegate(request: ImageRequest, job: Job, findLifecycle: Boolean): RequestDelegate @MainThread - fun sizeResolver(request: ImageRequest): SizeResolver + fun updateRequest(request: ImageRequest): ImageRequest @MainThread - fun options(request: ImageRequest, sizeResolver: SizeResolver, size: Size): Options + fun options(request: ImageRequest, size: Size): Options @WorkerThread - fun updateOptionsOnWorkerThread(options: Options): Options + fun updateOptions(options: Options): Options @WorkerThread fun isCacheValueValidForHardware(request: ImageRequest, cacheValue: MemoryCache.Value): Boolean diff --git a/coil-core/src/commonMain/kotlin/coil3/util/utils.kt b/coil-core/src/commonMain/kotlin/coil3/util/utils.kt index 79f35295ab..92f5afe5d0 100644 --- a/coil-core/src/commonMain/kotlin/coil3/util/utils.kt +++ b/coil-core/src/commonMain/kotlin/coil3/util/utils.kt @@ -12,7 +12,6 @@ import coil3.intercept.RealInterceptorChain import coil3.request.ErrorResult import coil3.request.ImageRequest import coil3.request.NullRequestDataException -import coil3.size.SizeResolver import kotlin.experimental.ExperimentalNativeApi import kotlin.reflect.KClass import okio.Closeable @@ -65,9 +64,6 @@ internal val Interceptor.Chain.isPlaceholderCached: Boolean internal val Interceptor.Chain.eventListener: EventListener get() = if (this is RealInterceptorChain) eventListener else EventListener.NONE -internal val Interceptor.Chain.sizeResolver: SizeResolver - get() = if (this is RealInterceptorChain) sizeResolver else request.sizeResolver - internal fun Int.isMinOrMax() = this == Int.MIN_VALUE || this == Int.MAX_VALUE internal const val SCHEME_FILE = "file" diff --git a/coil-core/src/commonTest/kotlin/coil3/intercept/RealInterceptorChainTest.kt b/coil-core/src/commonTest/kotlin/coil3/intercept/RealInterceptorChainTest.kt index d4a156ccf2..4f5489da28 100644 --- a/coil-core/src/commonTest/kotlin/coil3/intercept/RealInterceptorChainTest.kt +++ b/coil-core/src/commonTest/kotlin/coil3/intercept/RealInterceptorChainTest.kt @@ -5,7 +5,6 @@ import coil3.memory.MemoryCache import coil3.request.ImageRequest import coil3.request.ImageResult import coil3.size.Size -import coil3.size.SizeResolver import coil3.target.Target import coil3.test.utils.RobolectricTest import coil3.test.utils.context @@ -159,7 +158,6 @@ class RealInterceptorChainTests : RobolectricTest() { index = 0, request = request, size = size, - sizeResolver = SizeResolver(size), eventListener = EventListener.NONE, isPlaceholderCached = false ) diff --git a/coil-core/src/nonAndroidMain/kotlin/coil3/request/RequestService.nonAndroid.kt b/coil-core/src/nonAndroidMain/kotlin/coil3/request/RequestService.nonAndroid.kt index ed75860552..1c981d9f11 100644 --- a/coil-core/src/nonAndroidMain/kotlin/coil3/request/RequestService.nonAndroid.kt +++ b/coil-core/src/nonAndroidMain/kotlin/coil3/request/RequestService.nonAndroid.kt @@ -3,7 +3,6 @@ package coil3.request import coil3.ImageLoader import coil3.memory.MemoryCache import coil3.size.Size -import coil3.size.SizeResolver import coil3.util.Logger import coil3.util.SystemCallbacks import kotlinx.coroutines.Job @@ -12,10 +11,12 @@ internal actual fun RequestService( imageLoader: ImageLoader, systemCallbacks: SystemCallbacks, logger: Logger?, -): RequestService = NonAndroidRequestService() +): RequestService = NonAndroidRequestService(imageLoader) /** Handles operations that act on [ImageRequest]s. */ -internal class NonAndroidRequestService : RequestService { +internal class NonAndroidRequestService( + private val imageLoader: ImageLoader, +) : RequestService { override fun requestDelegate( request: ImageRequest, @@ -25,15 +26,13 @@ internal class NonAndroidRequestService : RequestService { return BaseRequestDelegate(job) } - override fun sizeResolver(request: ImageRequest): SizeResolver { - return request.sizeResolver + override fun updateRequest(request: ImageRequest): ImageRequest { + return request.newBuilder() + .defaults(imageLoader.defaults) + .build() } - override fun options( - request: ImageRequest, - sizeResolver: SizeResolver, - size: Size, - ): Options { + override fun options(request: ImageRequest, size: Size): Options { return Options( request.context, size, @@ -48,12 +47,14 @@ internal class NonAndroidRequestService : RequestService { ) } - override fun updateOptionsOnWorkerThread(options: Options): Options { + override fun updateOptions(options: Options): Options { return options } override fun isCacheValueValidForHardware( request: ImageRequest, cacheValue: MemoryCache.Value, - ) = true + ): Boolean { + return true + } }