Update the request instead of the defaults.

coil-kt · Nov 13, 2024 · 935cf8f · 935cf8f
1 parent 39879d0
commit 935cf8f
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 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
@@ -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
@@ -35,29 +35,26 @@ 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
     fun `allowInexactSize - inexact precision`() {
         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
     fun `allowInexactSize - ImageViewTarget`() {
         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,19 +112,17 @@ 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
     fun `allowInexactSize - SizeResolver explicit`() {
         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
@@ -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
@@ -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
@@ -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 {