Refactor static and animated ImageDecoder to share source creation ut…

…ils. (#2460) * Refactor static and animated ImageDecoder to share source creation utils. * Update ci.yml * Update ci.yml * PR feedback. * Spotless. * Tweak logic.
coil-kt · Sep 12, 2024 · 5d4bb2e · 5d4bb2e
1 parent 0822340
commit 5d4bb2e
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 81 deletions.
diff --git a/coil-core/src/androidMain/kotlin/coil3/decode/StaticImageDecoder.kt b/coil-core/src/androidMain/kotlin/coil3/decode/StaticImageDecoder.kt
@@ -1,6 +1,7 @@
 package coil3.decode
 
 import android.graphics.ImageDecoder
+import android.os.Build.VERSION.SDK_INT
 import android.system.ErrnoException
 import android.system.Os
 import android.system.OsConstants.SEEK_SET
@@ -9,6 +10,7 @@ import androidx.core.graphics.decodeBitmap
 import androidx.core.util.component1
 import androidx.core.util.component2
 import coil3.ImageLoader
+import coil3.annotation.InternalCoilApi
 import coil3.asImage
 import coil3.decode.BitmapFactoryDecoder.Companion.DEFAULT_MAX_PARALLELISM
 import coil3.fetch.SourceFetchResult
@@ -19,7 +21,6 @@ import coil3.request.colorSpace
 import coil3.request.maxBitmapSize
 import coil3.request.premultipliedAlpha
 import coil3.size.Precision
-import coil3.toAndroidUri
 import coil3.util.component1
 import coil3.util.component2
 import coil3.util.isHardware
@@ -106,40 +107,43 @@ class StaticImageDecoder(
             options: Options,
             imageLoader: ImageLoader,
         ): Decoder? {
-            val source = result.source.imageDecoderSourceOrNull(options) ?: return null
+            val source = result.source.toImageDecoderSource(options, animated = false) ?: return null
             return StaticImageDecoder(source, result.source, options, parallelismLock)
         }
+    }
+}
 
-        private fun ImageSource.imageDecoderSourceOrNull(options: Options): ImageDecoder.Source? {
-            if (fileSystem === FileSystem.SYSTEM) {
-                val file = fileOrNull()
-                if (file != null) {
-                    return ImageDecoder.createSource(file.toFile())
-                }
-            }
+@InternalCoilApi
+@RequiresApi(28)
+fun ImageSource.toImageDecoderSource(options: Options, animated: Boolean): ImageDecoder.Source? {
+    if (fileSystem === FileSystem.SYSTEM) {
+        val file = fileOrNull()
+        if (file != null) {
+            return ImageDecoder.createSource(file.toFile())
+        }
+    }
 
-            val metadata = metadata
-            return when {
-                metadata is AssetMetadata -> {
-                    ImageDecoder.createSource(options.context.assets, metadata.filePath)
-                }
-                metadata is ContentMetadata -> try {
-                    // Ensure the file descriptor supports lseek.
-                    // https://github.com/coil-kt/coil/issues/2434
-                    val asset = metadata.assetFileDescriptor
-                    Os.lseek(asset.fileDescriptor, asset.startOffset, SEEK_SET)
-                    ImageDecoder.createSource { asset }
-                } catch (_: ErrnoException) {
-                    ImageDecoder.createSource(options.context.contentResolver, metadata.uri.toAndroidUri())
-                }
-                metadata is ResourceMetadata && metadata.packageName == options.context.packageName -> {
-                    ImageDecoder.createSource(options.context.resources, metadata.resId)
-                }
-                metadata is ByteBufferMetadata -> {
-                    ImageDecoder.createSource(metadata.byteBuffer)
-                }
-                else -> null
-            }
+    val metadata = metadata
+    when {
+        metadata is AssetMetadata -> {
+            return ImageDecoder.createSource(options.context.assets, metadata.filePath)
+        }
+        metadata is ContentMetadata && SDK_INT >= 29 -> {
+            try {
+                // Ensure the file descriptor supports lseek.
+                // https://github.com/coil-kt/coil/issues/2434
+                val asset = metadata.assetFileDescriptor
+                Os.lseek(asset.fileDescriptor, asset.startOffset, SEEK_SET)
+                return ImageDecoder.createSource { asset }
+            } catch (_: ErrnoException) {}
+        }
+        metadata is ResourceMetadata && metadata.packageName == options.context.packageName -> {
+            return ImageDecoder.createSource(options.context.resources, metadata.resId)
+        }
+        metadata is ByteBufferMetadata && (SDK_INT >= 30 || !animated || metadata.byteBuffer.isDirect) -> {
+            return ImageDecoder.createSource(metadata.byteBuffer)
         }
     }
+
+    return null
 }
diff --git a/coil-gif/src/androidTest/java/coil3/gif/FallbackSourceTest.kt b/coil-gif/src/androidTest/java/coil3/gif/FallbackSourceTest.kt
@@ -1,5 +1,6 @@
 package coil3.gif
 
+import coil3.gif.internal.squashToDirectByteBuffer
 import java.io.ByteArrayInputStream
 import kotlin.random.Random
 import kotlin.test.assertContentEquals

diff --git a/coil-gif/src/main/java/coil3/gif/AnimatedImageDecoder.kt b/coil-gif/src/main/java/coil3/gif/AnimatedImageDecoder.kt
@@ -10,36 +10,31 @@ import androidx.core.util.component1
 import androidx.core.util.component2
 import coil3.ImageLoader
 import coil3.asImage
-import coil3.decode.AssetMetadata
-import coil3.decode.ByteBufferMetadata
-import coil3.decode.ContentMetadata
 import coil3.decode.DecodeResult
 import coil3.decode.DecodeUtils
 import coil3.decode.Decoder
 import coil3.decode.ImageSource
-import coil3.decode.ResourceMetadata
+import coil3.decode.toImageDecoderSource
 import coil3.fetch.SourceFetchResult
 import coil3.gif.internal.animatable2CallbackOf
 import coil3.gif.internal.asPostProcessor
 import coil3.gif.internal.maybeWrapImageSourceToRewriteFrameDelay
+import coil3.gif.internal.squashToDirectByteBuffer
 import coil3.request.Options
 import coil3.request.allowRgb565
 import coil3.request.bitmapConfig
 import coil3.request.colorSpace
 import coil3.request.maxBitmapSize
 import coil3.size.Precision
 import coil3.size.ScaleDrawable
-import coil3.toAndroidUri
 import coil3.util.component1
 import coil3.util.component2
 import coil3.util.isHardware
-import java.nio.ByteBuffer
 import kotlin.math.roundToInt
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.runInterruptible
 import kotlinx.coroutines.withContext
 import okio.BufferedSource
-import okio.FileSystem
 
 /**
  * A [Decoder] that uses [ImageDecoder] to decode GIFs, animated WebPs, and animated HEIFs.
@@ -61,7 +56,9 @@ class AnimatedImageDecoder(
         var isSampled = false
         val drawable = runInterruptible {
             maybeWrapImageSourceToRewriteFrameDelay(source, enforceMinimumFrameDelay).use { source ->
-                source.toImageDecoderSource().decodeDrawable { info, _ ->
+                val imageSource = source.toImageDecoderSource(options, animated = true)
+                    ?: ImageDecoder.createSource(source.source().use { it.squashToDirectByteBuffer() })
+                imageSource.decodeDrawable { info, _ ->
                     // Configure the output image's size.
                     val (srcWidth, srcHeight) = info.size
                     val (dstWidth, dstHeight) = DecodeUtils.computeDstSize(
@@ -103,38 +100,6 @@ class AnimatedImageDecoder(
         )
     }
 
-    private fun ImageSource.toImageDecoderSource(): ImageDecoder.Source {
-        if (fileSystem === FileSystem.SYSTEM) {
-            val file = fileOrNull()
-            if (file != null && fileSystem === FileSystem.SYSTEM) {
-                return ImageDecoder.createSource(file.toFile())
-            }
-        }
-
-        val metadata = metadata
-        if (metadata is AssetMetadata) {
-            return ImageDecoder.createSource(options.context.assets, metadata.filePath)
-        }
-        if (metadata is ContentMetadata) {
-            return if (SDK_INT >= 29) {
-                // ImageDecoder will seek inner fd to startOffset.
-                ImageDecoder.createSource { metadata.assetFileDescriptor }
-            } else {
-                ImageDecoder.createSource(options.context.contentResolver, metadata.uri.toAndroidUri())
-            }
-        }
-        if (metadata is ResourceMetadata && metadata.packageName == options.context.packageName) {
-            return ImageDecoder.createSource(options.context.resources, metadata.resId)
-        }
-        if (metadata is ByteBufferMetadata) {
-            val isDirect = metadata.byteBuffer.isDirect
-            if (isDirect || SDK_INT >= 30) return ImageDecoder.createSource(metadata.byteBuffer)
-        }
-
-        val bytebuffer = source.source().use { it.squashToDirectByteBuffer() }
-        return ImageDecoder.createSource(bytebuffer)
-    }
-
     private fun ImageDecoder.configureImageDecoderProperties() {
         allocator = if (options.bitmapConfig.isHardware) {
             ImageDecoder.ALLOCATOR_HARDWARE
@@ -194,13 +159,3 @@ class AnimatedImageDecoder(
         }
     }
 }
-
-internal fun BufferedSource.squashToDirectByteBuffer(): ByteBuffer {
-    // Squash bytes to BufferedSource inner buffer then we know total byteCount.
-    request(Long.MAX_VALUE)
-
-    val byteBuffer = ByteBuffer.allocateDirect(buffer.size.toInt())
-    while (!buffer.exhausted()) buffer.read(byteBuffer)
-    byteBuffer.flip()
-    return byteBuffer
-}
diff --git a/coil-gif/src/main/java/coil3/gif/internal/utils.kt b/coil-gif/src/main/java/coil3/gif/internal/utils.kt
@@ -8,6 +8,8 @@ import androidx.annotation.RequiresApi
 import androidx.vectordrawable.graphics.drawable.Animatable2Compat
 import coil3.gif.AnimatedTransformation
 import coil3.gif.PixelOpacity
+import java.nio.ByteBuffer
+import okio.BufferedSource
 
 @RequiresApi(28)
 internal fun AnimatedTransformation.asPostProcessor() =
@@ -36,3 +38,13 @@ internal fun animatable2CompatCallbackOf(
     override fun onAnimationStart(drawable: Drawable) { onStart?.invoke() }
     override fun onAnimationEnd(drawable: Drawable) { onEnd?.invoke() }
 }
+
+internal fun BufferedSource.squashToDirectByteBuffer(): ByteBuffer {
+    // Squash bytes to BufferedSource inner buffer then we know total byteCount.
+    request(Long.MAX_VALUE)
+
+    val byteBuffer = ByteBuffer.allocateDirect(buffer.size.toInt())
+    while (!buffer.exhausted()) buffer.read(byteBuffer)
+    byteBuffer.flip()
+    return byteBuffer
+}