Fix always setting bitmap config to ARGB_8888 if hardware bitmaps are…

… disabled. (#1770)

* Fix always setting bitmap config to ARGB_8888 if hardware bitmaps are disabled.

* Increase timeout.

coil-base/src/main/java/coil/request/RequestService.kt

@@ -67,19 +67,20 @@ internal class RequestService(
             CachePolicy.DISABLED
         }
 
-        // Disable allowRgb565 if there are transformations or the requested config is ALPHA_8.
-        // ALPHA_8 is a mask config where each pixel is 1 byte so it wouldn't make sense to use
-        // RGB_565 as an optimization in that case.
-        val allowRgb565 = request.allowRgb565 && request.transformations.isEmpty() &&
-            config != Bitmap.Config.ALPHA_8
-
         // Use `Scale.FIT` if either dimension is undefined.
         val scale = if (size.width == Dimension.Undefined || size.height == Dimension.Undefined) {
             Scale.FIT
         } else {
             request.scale
         }
 
+        // Disable allowRgb565 if there are transformations or the requested config is ALPHA_8.
+        // ALPHA_8 is a mask config where each pixel is 1 byte so it wouldn't make sense to use
+        // RGB_565 as an optimization in that case.
+        val allowRgb565 = request.allowRgb565 &&
+            request.transformations.isEmpty() &&
+            config != Bitmap.Config.ALPHA_8
+
         return Options(
             context = request.context,
             config = config,
@@ -100,15 +101,19 @@ internal class RequestService(
     }
 
     /**
-     * Return 'true' if [requestedConfig] is a valid (i.e. can be returned to its [Target]) config
-     * for [request].
+     * Return 'true' if [requestedConfig] is a valid (i.e. can be returned to its [Target])
+     * config for [request].
      */
     fun isConfigValidForHardware(request: ImageRequest, requestedConfig: Bitmap.Config): Boolean {
         // Short circuit if the requested bitmap config is software.
-        if (!requestedConfig.isHardware) return true
+        if (!requestedConfig.isHardware) {
+            return true
+        }
 
         // Ensure the request allows hardware bitmaps.
-        if (!request.allowHardware) return false
+        if (!request.allowHardware) {
+            return false
+        }
 
         // Prevent hardware bitmaps for non-hardware accelerated targets.
         val target = request.target
@@ -126,13 +131,18 @@ internal class RequestService(
     }
 
     /**
-     * Return 'true' if [request]'s requested bitmap config is valid (i.e. can be returned to its
-     * [Target]).
+     * Return 'true' if [request]'s requested bitmap config is valid (i.e. can be returned
+     * to its [Target]).
      *
      * This check is similar to [isConfigValidForHardware] except this method also checks
      * that we are able to allocate a new hardware bitmap.
      */
     private fun isConfigValidForHardwareAllocation(request: ImageRequest, size: Size): Boolean {
+        // Short circuit if the requested bitmap config is software.
+        if (!request.bitmapConfig.isHardware) {
+            return true
+        }
+
         return isConfigValidForHardware(request, request.bitmapConfig) &&
             hardwareBitmapService.allowHardwareMainThread(size)
     }

coil-base/src/test/java/coil/request/RequestServiceTest.kt

@@ -1,23 +1,27 @@
 package coil.request
 
 import android.content.Context
+import android.graphics.Bitmap
 import android.view.View
 import android.widget.ImageView
 import androidx.test.core.app.ApplicationProvider
 import coil.ImageLoader
 import coil.RealImageLoader
 import coil.size.Precision
+import coil.size.Size
 import coil.size.ViewSizeResolver
 import coil.target.ViewTarget
 import coil.util.SystemCallbacks
 import coil.util.allowInexactSize
 import coil.util.createRequest
+import kotlin.test.assertEquals
 import kotlin.test.assertFalse
 import kotlin.test.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
 
 @RunWith(RobolectricTestRunner::class)
 class RequestServiceTest {
@@ -118,4 +122,16 @@ class RequestServiceTest {
         }
         assertFalse(request.allowInexactSize)
     }
+
+    /** Regression test: https://github.com/coil-kt/coil/issues/1768 */
+    @Test
+    @Config(sdk = [23])
+    fun `RGB_565 is preserved if hardware bitmaps are disabled`() {
+        val request = ImageRequest.Builder(context)
+            .data(Unit)
+            .bitmapConfig(Bitmap.Config.RGB_565)
+            .build()
+        val options = service.options(request, Size(100, 100))
+        assertEquals(Bitmap.Config.RGB_565, options.config)
+    }
 }

coil-video/src/androidTest/java/coil/decode/VideoFrameDecoderTest.kt

@@ -15,6 +15,7 @@ import coil.util.decodeBitmapAsset
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
+import kotlin.time.Duration.Companion.minutes
 import kotlinx.coroutines.test.runTest
 import okio.buffer
 import okio.source
@@ -31,7 +32,7 @@ class VideoFrameDecoderTest {
     }
 
     @Test
-    fun noSetFrameTime() = runTest {
+    fun noSetFrameTime() = runTest(timeout = 1.minutes) {
         // MediaMetadataRetriever does not work on the emulator pre-API 23.
         assumeTrue(SDK_INT >= 23)
 
@@ -52,7 +53,7 @@ class VideoFrameDecoderTest {
     }
 
     @Test
-    fun specificFrameTime() = runTest {
+    fun specificFrameTime() = runTest(timeout = 1.minutes) {
         // MediaMetadataRetriever does not work on the emulator pre-API 23.
         assumeTrue(SDK_INT >= 23)
 
@@ -78,7 +79,7 @@ class VideoFrameDecoderTest {
     }
 
     @Test
-    fun specificFramePercent() = runTest {
+    fun specificFramePercent() = runTest(timeout = 1.minutes) {
         // MediaMetadataRetriever does not work on the emulator pre-API 23.
         assumeTrue(SDK_INT >= 23)
 
@@ -104,7 +105,7 @@ class VideoFrameDecoderTest {
     }
 
     @Test
-    fun rotation() = runTest {
+    fun rotation() = runTest(timeout = 1.minutes) {
         // MediaMetadataRetriever does not work on the emulator pre-API 23.
         assumeTrue(SDK_INT >= 23)
 
@@ -126,7 +127,7 @@ class VideoFrameDecoderTest {
 
     /** Regression test: https://github.com/coil-kt/coil/issues/1482 */
     @Test
-    fun nestedAsset() = runTest {
+    fun nestedAsset() = runTest(timeout = 1.minutes) {
         // MediaMetadataRetriever does not work on the emulator pre-API 23.
         assumeTrue(SDK_INT >= 23)