Parallelize CI steps. Run instrumentation tests against multiple API levels.

.github/workflows/ci.yml

@@ -5,25 +5,66 @@ on:
             - master
     pull_request:
 jobs:
-    ci:
-        name: Build + Test
-        runs-on: macOS-latest
-        timeout-minutes: 30
+    check:
+        name: Check
+        runs-on: ubuntu-latest
+        timeout-minutes: 10
         steps:
             - uses: actions/checkout@v2
+            - uses: gradle/wrapper-validation-action@v1
+            - uses: actions/setup-java@v1
+              with:
+                  java-version: 11
+            - uses: actions/cache@v1
+              with:
+                  path: ~/.gradle/caches
+                  key: gradle-${{ runner.os }}-${{ hashFiles('**/build.gradle.kts') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}-${{ hashFiles('**/buildSrc/src/main/kotlin/Library.kt') }}
+
+            - name: Check style
+              run: ./gradlew ktlintCheck
 
-            - name: Set Up Java
-              uses: actions/setup-java@v1
+            # Check if there has been a binary incompatible change to the API.
+            # If this change is intentional, run `./gradlew apiDump` and commit the new API files.
+            - name: Check binary compatibility
+              run: ./gradlew apiCheck
+
+    unit-tests:
+        name: Unit tests
+        runs-on: ubuntu-latest
+        timeout-minutes: 10
+        steps:
+            - uses: actions/checkout@v2
+            - uses: gradle/wrapper-validation-action@v1
+            - uses: actions/setup-java@v1
               with:
                   java-version: 11
+            - uses: actions/cache@v1
+              with:
+                  path: ~/.gradle/caches
+                  key: gradle-${{ runner.os }}-${{ hashFiles('**/build.gradle.kts') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}-${{ hashFiles('**/buildSrc/src/main/kotlin/Library.kt') }}
+
+            - name: Unit tests
+              run: ./gradlew testDebugUnitTest
 
-            # Validate the Gradle wrapper JAR files.
+    instrumentation-tests:
+        name: Instrumentation tests
+        runs-on: macOS-latest
+        timeout-minutes: 20
+        strategy:
+            matrix:
+                api-level: [15, 19, 21, 25, 29]
+        steps:
+            - uses: actions/checkout@v2
             - uses: gradle/wrapper-validation-action@v1
+            - uses: actions/setup-java@v1
+              with:
+                  java-version: 11
 
             # Ensure .gradle/caches is empty before writing to it.
             # This helps us stay within Github's cache size limits.
-            - name: Clean Cache
-              run: rm -rf ~/.gradle/caches
+            # Rename the folder instead of deleting it as it's faster.
+            - name: Clean cache
+              run: mv ~/.gradle/caches ~/.gradle/.invalid_caches
 
             # Restore the cache.
             # Intentionally don't set 'restore-keys' so the cache never contains redundant dependencies.
@@ -32,26 +73,31 @@ jobs:
                   path: ~/.gradle/caches
                   key: gradle-${{ runner.os }}-${{ hashFiles('**/build.gradle.kts') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}-${{ hashFiles('**/buildSrc/src/main/kotlin/Library.kt') }}
 
-            - name: Ktlint
-              run: ./gradlew ktlintCheck
-
-            # Check if there has been a binary incompatible change to the API.
-            # If this change is intentional, run `./gradlew apiDump` and commit the new API files.
-            - name: Check Binary Compatibility
-              run: ./gradlew apiCheck
-
-            - name: Unit Tests
-              run: ./gradlew testDebugUnitTest
-
-            - name: Instrumentation Tests
+            - name: Instrumentation tests
               uses: reactivecircus/android-emulator-runner@v2
               with:
-                  api-level: 25
+                  api-level: ${{ matrix.api-level }}
                   arch: x86
                   script: ./gradlew connectedDebugAndroidTest
 
+    deploy-snapshot:
+        name: Deploy snapshot
+        runs-on: ubuntu-latest
+        timeout-minutes: 10
+        if: github.ref == 'refs/heads/master'
+        needs: [check, unit-tests, instrumentation-tests]
+        steps:
+            - uses: actions/checkout@v2
+            - uses: gradle/wrapper-validation-action@v1
+            - uses: actions/setup-java@v1
+              with:
+                  java-version: 11
+            - uses: actions/cache@v1
+              with:
+                  path: ~/.gradle/caches
+                  key: gradle-${{ runner.os }}-${{ hashFiles('**/build.gradle.kts') }}-${{ hashFiles('**/gradle/wrapper/gradle-wrapper.properties') }}-${{ hashFiles('**/buildSrc/src/main/kotlin/Library.kt') }}
+
             - name: Deploy Snapshot
-              if: github.ref == 'refs/heads/master'
               env:
                   SONATYPE_NEXUS_USERNAME: ${{ secrets.SONATYPE_NEXUS_USERNAME }}
                   SONATYPE_NEXUS_PASSWORD: ${{ secrets.SONATYPE_NEXUS_PASSWORD }}

coil-base/src/androidTest/java/coil/decode/BitmapFactoryDecoderTest.kt

@@ -3,6 +3,7 @@ package coil.decode
 import android.content.Context
 import android.graphics.Bitmap
 import android.graphics.drawable.BitmapDrawable
+import android.os.Build.VERSION.SDK_INT
 import androidx.test.core.app.ApplicationProvider
 import coil.bitmappool.BitmapPool
 import coil.size.OriginalSize
@@ -30,7 +31,7 @@ class BitmapFactoryDecoderTest {
     @Before
     fun before() {
         context = ApplicationProvider.getApplicationContext()
-        pool = BitmapPool(0)
+        pool = BitmapPool(Long.MAX_VALUE)
         service = BitmapFactoryDecoder(context)
     }
 
@@ -87,7 +88,7 @@ class BitmapFactoryDecoderTest {
     fun originalSizeDimensionsAreResolvedCorrectly() {
         val size = OriginalSize
         val normal = decode("normal.jpg", size)
-        assertEquals(PixelSize(1080, 1350), normal.run { PixelSize(width, height) })
+        assertEquals(PixelSize(1080, 1350), normal.size)
     }
 
     @Test
@@ -116,7 +117,7 @@ class BitmapFactoryDecoderTest {
             size = PixelSize(1500, 1500),
             options = { createOptions(scale = Scale.FIT, allowInexactSize = true) }
         )
-        assertEquals(PixelSize(1080, 1350), result.run { PixelSize(width, height) })
+        assertEquals(PixelSize(1080, 1350), result.size)
     }
 
     @Test
@@ -126,7 +127,48 @@ class BitmapFactoryDecoderTest {
             size = PixelSize(1500, 1500),
             options = { createOptions(scale = Scale.FIT, allowInexactSize = false) }
         )
-        assertEquals(PixelSize(1200, 1500), result.run { PixelSize(width, height) })
+        assertEquals(PixelSize(1200, 1500), result.size)
+    }
+
+    @Test
+    fun pooledBitmap_exactSize() {
+        val pooledBitmap = Bitmap.createBitmap(1080, 1350, Bitmap.Config.ARGB_8888)
+        pool.put(pooledBitmap)
+
+        val result = decode(
+            fileName = "normal.jpg",
+            size = PixelSize(1080, 1350),
+            options = {
+                createOptions(
+                    config = Bitmap.Config.ARGB_8888,
+                    scale = Scale.FIT,
+                    allowInexactSize = false
+                )
+            }
+        )
+        assertEquals(PixelSize(1080, 1350), result.size)
+        assertEquals(pooledBitmap, result)
+    }
+
+    @Test
+    fun pooledBitmap_inexactSize() {
+        val pooledBitmap = Bitmap.createBitmap(1080, 1350, Bitmap.Config.ARGB_8888)
+        pool.put(pooledBitmap)
+
+        val result = decode(
+            fileName = "normal.jpg",
+            size = PixelSize(500, 500),
+            options = {
+                createOptions(
+                    config = Bitmap.Config.ARGB_8888,
+                    scale = Scale.FILL,
+                    allowInexactSize = false
+                )
+            }
+        )
+        assertEquals(PixelSize(500, 625), result.size)
+        // The bitmap should not be re-used on pre-API 19.
+        assertEquals(pooledBitmap === result, SDK_INT >= 19)
     }
 
     private fun decode(

coil-base/src/androidTest/java/coil/fetch/ContentUriFetcherTest.kt

@@ -102,10 +102,16 @@ class ContentUriFetcherTest {
 
         val contentUri = ContentUris.withAppendedId(RawContacts.CONTENT_URI, id)
         val photoUri = Uri.withAppendedPath(contentUri, RawContacts.DisplayPhoto.CONTENT_DIRECTORY)
-        checkNotNull(context.contentResolver.openAssetFileDescriptor(photoUri, "rw")).use { fd ->
+        val fd = checkNotNull(context.contentResolver.openAssetFileDescriptor(photoUri, "rw"))
+
+        // AssetFileDescriptor does not implement Closable on pre-API 19 so we can't use `use`.
+        @Suppress("ConvertTryFinallyToUseCall")
+        try {
             val source = context.assets.open("normal.jpg").source()
             val sink = fd.createOutputStream().sink().buffer()
             source.use { sink.use { sink.writeAll(source) } }
+        } finally {
+            fd.close()
         }
 
         // Wait for the display image to be parsed by the system.

coil-base/src/androidTest/java/coil/fetch/ResourceUriFetcherTest.kt

@@ -3,6 +3,7 @@ package coil.fetch
 import android.content.ContentResolver.SCHEME_ANDROID_RESOURCE
 import android.content.Context
 import android.graphics.drawable.BitmapDrawable
+import android.os.Build.VERSION.SDK_INT
 import androidx.core.net.toUri
 import androidx.test.core.app.ApplicationProvider
 import coil.base.test.R
@@ -12,6 +13,7 @@ import coil.map.ResourceUriMapper
 import coil.size.PixelSize
 import coil.util.createOptions
 import kotlinx.coroutines.runBlocking
+import org.junit.Assume.assumeTrue
 import org.junit.Before
 import org.junit.Test
 import kotlin.test.assertEquals
@@ -65,9 +67,8 @@ class ResourceUriFetcherTest {
 
     @Test
     fun externalPackageRasterDrawable() {
-        // https://android.googlesource.com/platform/packages/apps/Settings/+/master/res/drawable/regulatory_info.png
-        val packageName = "com.android.settings"
-        val rawUri = "$SCHEME_ANDROID_RESOURCE://$packageName/drawable/regulatory_info".toUri()
+        // https://android.googlesource.com/platform/packages/apps/Settings/+/master/res/drawable-xhdpi/ic_power_system.png
+        val rawUri = "$SCHEME_ANDROID_RESOURCE://com.android.settings/drawable/ic_power_system".toUri()
         val uri = ResourceUriMapper(context).map(rawUri)
 
         assertTrue(fetcher.handles(uri))
@@ -83,9 +84,11 @@ class ResourceUriFetcherTest {
 
     @Test
     fun externalPackageVectorDrawable() {
+        // com.android.settings/drawable/ic_cancel was added in API 23.
+        assumeTrue(SDK_INT >= 23)
+
         // https://android.googlesource.com/platform/packages/apps/Settings/+/master/res/drawable/ic_cancel.xml
-        val packageName = "com.android.settings"
-        val rawUri = "$SCHEME_ANDROID_RESOURCE://$packageName/drawable/ic_cancel".toUri()
+        val rawUri = "$SCHEME_ANDROID_RESOURCE://com.android.settings/drawable/ic_cancel".toUri()
         val uri = ResourceUriMapper(context).map(rawUri)
 
         assertTrue(fetcher.handles(uri))

coil-base/src/androidTest/java/coil/map/ResourceUriMapperTest.kt

@@ -36,9 +36,9 @@ class ResourceUriMapperTest {
 
     @Test
     fun externalResourceNameUri() {
-        // https://android.googlesource.com/platform/packages/apps/Settings/+/master/res/drawable/regulatory_info.png
+        // https://android.googlesource.com/platform/packages/apps/Settings/+/master/res/drawable-xhdpi/ic_power_system.png
         val packageName = "com.android.settings"
-        val input = "$SCHEME_ANDROID_RESOURCE://$packageName/drawable/regulatory_info".toUri()
+        val input = "$SCHEME_ANDROID_RESOURCE://$packageName/drawable/ic_power_system".toUri()
 
         assertTrue(mapper.handles(input))
 

coil-base/src/androidTest/java/coil/transform/GrayscaleTransformationTest.kt

@@ -1,6 +1,8 @@
 package coil.transform
 
 import android.content.Context
+import android.graphics.Bitmap
+import android.graphics.BitmapFactory
 import androidx.test.core.app.ApplicationProvider
 import coil.bitmappool.BitmapPool
 import coil.size.OriginalSize
@@ -27,8 +29,12 @@ class GrayscaleTransformationTest {
 
     @Test
     fun basic() {
-        val input = context.decodeBitmapAsset("normal.jpg")
-        val expected = context.decodeBitmapAsset("normal_grayscale.jpg")
+        val options = BitmapFactory.Options().apply {
+            inPreferredConfig = Bitmap.Config.ARGB_8888
+            inSampleSize = 2 // Downsample the image to avoid running out of memory on pre-API 21.
+        }
+        val input = context.decodeBitmapAsset("normal.jpg", options)
+        val expected = context.decodeBitmapAsset("normal_grayscale.jpg", options)
 
         val actual = runBlocking {
             transformation.transform(pool, input, OriginalSize)

coil-base/src/main/java/coil/decode/BitmapFactoryDecoder.kt

@@ -75,17 +75,19 @@ internal class BitmapFactoryDecoder(private val context: Context) : Decoder {
             outWidth <= 0 || outHeight <= 0 -> {
                 // This occurs if there was an error decoding the image's size.
                 inSampleSize = 1
+                inScaled = false
                 inBitmap = null
             }
             size !is PixelSize -> {
                 // This occurs if size is OriginalSize.
                 inSampleSize = 1
+                inScaled = false
 
                 if (inMutable) {
                     inBitmap = pool.getDirtyOrNull(outWidth, outHeight, inPreferredConfig)
                 }
             }
-            SDK_INT >= 19 -> {
+            else -> {
                 val (width, height) = size
                 inSampleSize = DecodeUtils.calculateInSampleSize(srcWidth, srcHeight, width, height, options.scale)
 
@@ -115,28 +117,26 @@ internal class BitmapFactoryDecoder(private val context: Context) : Decoder {
                 }
 
                 if (inMutable) {
-                    // Allocate a slightly larger bitmap than necessary as the output bitmap's dimensions may not match the
-                    // requested dimensions exactly. This is due to intricacies in Android's downsampling algorithm.
-                    val sampledOutWidth = outWidth / inSampleSize.toDouble()
-                    val sampledOutHeight = outHeight / inSampleSize.toDouble()
-                    inBitmap = pool.getDirtyOrNull(
-                        width = ceil(scale * sampledOutWidth + 0.5).toInt(),
-                        height = ceil(scale * sampledOutHeight + 0.5).toInt(),
-                        config = inPreferredConfig
-                    )
-                }
-            }
-            else -> {
-                // We can only re-use bitmaps that exactly match the size of the image.
-                if (inMutable) {
-                    inBitmap = pool.getDirtyOrNull(outWidth, outHeight, inPreferredConfig)
-                }
-
-                // Sample size must be 1 if we are re-using a bitmap.
-                inSampleSize = if (inBitmap != null) {
-                    1
-                } else {
-                    DecodeUtils.calculateInSampleSize(srcWidth, srcHeight, size.width, size.height, options.scale)
+                    inBitmap = when {
+                        // If we're not scaling the image, use the image's source dimensions.
+                        inSampleSize == 1 && !inScaled -> {
+                            pool.getDirtyOrNull(outWidth, outHeight, inPreferredConfig)
+                        }
+                        // We can only re-use bitmaps that don't match the image's source dimensions on API 19 and above.
+                        SDK_INT >= 19 -> {
+                            // Request a slightly larger bitmap than necessary as the output bitmap's dimensions
+                            // may not match the requested dimensions exactly. This is due to intricacies in Android's
+                            // downsampling algorithm across different API levels.
+                            val sampledOutWidth = outWidth / inSampleSize.toDouble()
+                            val sampledOutHeight = outHeight / inSampleSize.toDouble()
+                            pool.getDirtyOrNull(
+                                width = ceil(scale * sampledOutWidth + 0.5).toInt(),
+                                height = ceil(scale * sampledOutHeight + 0.5).toInt(),
+                                config = inPreferredConfig
+                            )
+                        }
+                        else -> null
+                    }
                 }
             }
         }

coil-base/src/main/java/coil/util/Utils.kt

@@ -56,9 +56,11 @@ internal object Utils {
     }
 
     fun getDefaultBitmapPoolPercentage(): Double {
+        // Allocate less memory for bitmap pooling on API 18 and below as the requirements
+        // for bitmap reuse are quite strict.
         // Allocate less memory for bitmap pooling on API 26 and above since we default to
-        // hardware bitmaps, which cannot be added to the pool.
-        return if (SDK_INT >= 26) 0.25 else 0.5
+        // hardware bitmaps, which cannot be reused.
+        return if (SDK_INT in 19..25) 0.5 else 0.25
     }
 
     fun getDefaultBitmapConfig(): Bitmap.Config {