From 935cf8f1b5e8974eeba78ae6d441ef27076cfacc Mon Sep 17 00:00:00 2001 From: Colin White Date: Wed, 13 Nov 2024 00:09:44 -0800 Subject: [PATCH] Update the request instead of the defaults. --- .../coil3/request/RequestService.android.kt | 48 ++++++-------- .../coil3/request/RequestServiceTest.kt | 62 ++++++++----------- .../kotlin/coil3/RealImageLoader.kt | 6 +- .../kotlin/coil3/request/RequestService.kt | 2 +- .../request/RequestService.nonAndroid.kt | 6 +- 5 files changed, 51 insertions(+), 73 deletions(-) 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 d2a2f7fe6..672183dc4 100644 --- a/coil-core/src/androidMain/kotlin/coil3/request/RequestService.android.kt +++ b/coil-core/src/androidMain/kotlin/coil3/request/RequestService.android.kt @@ -69,24 +69,25 @@ internal class AndroidRequestService( return context.getLifecycle() } - override fun defaults(request: ImageRequest): ImageRequest.Defaults { - val sizeResolver = request.resolveSizeResolver() - val scale = request.resolveScale() - val precision = request.resolvePrecision() - var defaults = imageLoader.defaults - - if (sizeResolver != request.defaults.sizeResolver || - scale != request.defaults.scale || - precision != request.defaults.precision - ) { - defaults = defaults.copy( - sizeResolver = sizeResolver, - scale = scale, - precision = precision, - ) + 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) + } + + if (request.defined.scale == null) { + builder.scale(request.resolveScale()) } - return defaults + if (request.defined.precision == null) { + builder.precision(request.resolvePrecision(sizeResolver)) + } + + return builder.build() } override fun options(request: ImageRequest, size: Size): Options { @@ -105,10 +106,6 @@ internal class AndroidRequestService( } private fun ImageRequest.resolveSizeResolver(): SizeResolver { - if (defined.sizeResolver != null) { - return defined.sizeResolver - } - if (target is ViewTarget<*>) { // CENTER and MATRIX scale types should be decoded at the image's original size. val view = target.view @@ -124,10 +121,6 @@ internal class AndroidRequestService( } private fun ImageRequest.resolveScale(): Scale { - if (defined.scale != null) { - return defined.scale - } - // Autodetect the scale from the ImageView. val imageView = (target as? ViewTarget<*>)?.view as? ImageView if (imageView != null) { @@ -137,11 +130,8 @@ internal class AndroidRequestService( return scale } - private fun ImageRequest.resolvePrecision(): Precision { - if (defined.precision != null) { - return defined.precision - } - + private fun ImageRequest.resolvePrecision(sizeResolver: SizeResolver): Precision { + // Use inexact precision if we're falling back to the source dimensions. if (defined.sizeResolver == null && sizeResolver == SizeResolver.ORIGINAL) { return Precision.INEXACT } diff --git a/coil-core/src/androidUnitTest/kotlin/coil3/request/RequestServiceTest.kt b/coil-core/src/androidUnitTest/kotlin/coil3/request/RequestServiceTest.kt index b4c6fbc57..6ac1735f6 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 7c76e1bf2..7a6e157b7 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(requestService.defaults(initialRequest)) - .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) diff --git a/coil-core/src/commonMain/kotlin/coil3/request/RequestService.kt b/coil-core/src/commonMain/kotlin/coil3/request/RequestService.kt index 17785c47f..7ab259126 100644 --- a/coil-core/src/commonMain/kotlin/coil3/request/RequestService.kt +++ b/coil-core/src/commonMain/kotlin/coil3/request/RequestService.kt @@ -22,7 +22,7 @@ internal interface RequestService { fun requestDelegate(request: ImageRequest, job: Job, findLifecycle: Boolean): RequestDelegate @MainThread - fun defaults(request: ImageRequest): ImageRequest.Defaults + fun updateRequest(request: ImageRequest): ImageRequest @MainThread fun options(request: ImageRequest, size: Size): Options 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 224dd3c57..1c981d9f1 100644 --- a/coil-core/src/nonAndroidMain/kotlin/coil3/request/RequestService.nonAndroid.kt +++ b/coil-core/src/nonAndroidMain/kotlin/coil3/request/RequestService.nonAndroid.kt @@ -26,8 +26,10 @@ internal class NonAndroidRequestService( return BaseRequestDelegate(job) } - override fun defaults(request: ImageRequest): ImageRequest.Defaults { - return imageLoader.defaults + override fun updateRequest(request: ImageRequest): ImageRequest { + return request.newBuilder() + .defaults(imageLoader.defaults) + .build() } override fun options(request: ImageRequest, size: Size): Options {