From 6452fcef553707dbed0a022eea547d0d53db8c3d Mon Sep 17 00:00:00 2001 From: Sagar Viradiya Date: Sun, 21 Feb 2021 13:01:19 +0530 Subject: [PATCH 1/6] Support for start and end callbacks for animated drawables - Add getter and setter extension for start and end callbacks on ImageRequest.Builder. - Apply start and end callbacks through Animatable2.AnimationCallback and Animatable2Compat.AnimationCallback in the ImageDecoderDecoder.kt and GifDecoder.kt respectively. --- .../src/main/java/coil/decode/GifDecoder.kt | 21 ++++++++++++++++ .../java/coil/decode/ImageDecoderDecoder.kt | 22 +++++++++++++++++ coil-gif/src/main/java/coil/request/Gifs.kt | 24 +++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/coil-gif/src/main/java/coil/decode/GifDecoder.kt b/coil-gif/src/main/java/coil/decode/GifDecoder.kt index eb0b90451e..fc944028ef 100644 --- a/coil-gif/src/main/java/coil/decode/GifDecoder.kt +++ b/coil-gif/src/main/java/coil/decode/GifDecoder.kt @@ -4,10 +4,14 @@ package coil.decode import android.graphics.Bitmap import android.graphics.Movie +import android.graphics.drawable.Drawable import android.os.Build.VERSION.SDK_INT +import androidx.vectordrawable.graphics.drawable.Animatable2Compat import coil.bitmap.BitmapPool import coil.drawable.MovieDrawable import coil.request.animatedTransformation +import coil.request.animationEndCallback +import coil.request.animationStartCallback import coil.request.repeatCount import coil.size.Size import okio.BufferedSource @@ -54,6 +58,21 @@ class GifDecoder : Decoder { drawable.setRepeatCount(options.parameters.repeatCount() ?: MovieDrawable.REPEAT_INFINITE) + // Set the start and end animation callbacks if any one is supplied through the request. + if (options.parameters.animationStartCallback() != null || options.parameters.animationEndCallback() != null) { + drawable.registerAnimationCallback(object : Animatable2Compat.AnimationCallback() { + override fun onAnimationStart(drawable: Drawable?) { + super.onAnimationStart(drawable) + options.parameters.animationStartCallback()?.invoke() + } + + override fun onAnimationEnd(drawable: Drawable?) { + super.onAnimationEnd(drawable) + options.parameters.animationEndCallback()?.invoke() + } + }) + } + // Set the animated transformation to be applied on each frame. drawable.setAnimatedTransformation(options.parameters.animatedTransformation()) @@ -66,5 +85,7 @@ class GifDecoder : Decoder { companion object { const val REPEAT_COUNT_KEY = "coil#repeat_count" const val ANIMATED_TRANSFORMATION_KEY = "coil#animated_transformation" + const val ANIMATION_START_CALLBACK_KEY = "coil#aimation_start_callback" + const val ANIMATION_END_CALLBACK_KEY = "coil#aimation_end_callback" } } diff --git a/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt b/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt index 74aa146244..d98e9470d7 100644 --- a/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt +++ b/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt @@ -4,7 +4,9 @@ package coil.decode import android.graphics.Bitmap import android.graphics.ImageDecoder +import android.graphics.drawable.Animatable2 import android.graphics.drawable.AnimatedImageDrawable +import android.graphics.drawable.Drawable import android.os.Build.VERSION.SDK_INT import androidx.annotation.RequiresApi import androidx.core.graphics.decodeDrawable @@ -13,6 +15,8 @@ import androidx.core.util.component2 import coil.bitmap.BitmapPool import coil.drawable.ScaleDrawable import coil.request.animatedTransformation +import coil.request.animationEndCallback +import coil.request.animationStartCallback import coil.request.repeatCount import coil.size.PixelSize import coil.size.Size @@ -108,6 +112,22 @@ class ImageDecoderDecoder : Decoder { val drawable = if (baseDrawable is AnimatedImageDrawable) { baseDrawable.repeatCount = options.parameters.repeatCount() ?: AnimatedImageDrawable.REPEAT_INFINITE + // Set the start and end animation callbacks if any one is supplied through the request. + if (options.parameters.animationStartCallback() != null + || options.parameters.animationEndCallback() != null) { + baseDrawable.registerAnimationCallback(object : Animatable2.AnimationCallback() { + override fun onAnimationStart(drawable: Drawable?) { + super.onAnimationStart(drawable) + options.parameters.animationStartCallback()?.invoke() + } + + override fun onAnimationEnd(drawable: Drawable?) { + super.onAnimationEnd(drawable) + options.parameters.animationEndCallback()?.invoke() + } + }) + } + // Wrap AnimatedImageDrawable in a ScaleDrawable so it always scales to fill its bounds. ScaleDrawable(baseDrawable, options.scale) } else { @@ -126,5 +146,7 @@ class ImageDecoderDecoder : Decoder { companion object { const val REPEAT_COUNT_KEY = GifDecoder.REPEAT_COUNT_KEY const val ANIMATED_TRANSFORMATION_KEY = GifDecoder.ANIMATED_TRANSFORMATION_KEY + const val ANIMATION_START_CALLBACK_KEY = GifDecoder.ANIMATION_START_CALLBACK_KEY + const val ANIMATION_END_CALLBACK_KEY = GifDecoder.ANIMATION_END_CALLBACK_KEY } } diff --git a/coil-gif/src/main/java/coil/request/Gifs.kt b/coil-gif/src/main/java/coil/request/Gifs.kt index 8a32a19faf..867f743675 100644 --- a/coil-gif/src/main/java/coil/request/Gifs.kt +++ b/coil-gif/src/main/java/coil/request/Gifs.kt @@ -5,7 +5,9 @@ package coil.request import android.graphics.drawable.AnimatedImageDrawable import android.graphics.drawable.Drawable +import coil.decode.GifDecoder.Companion.ANIMATION_START_CALLBACK_KEY import coil.decode.GifDecoder.Companion.ANIMATED_TRANSFORMATION_KEY +import coil.decode.GifDecoder.Companion.ANIMATION_END_CALLBACK_KEY import coil.decode.GifDecoder.Companion.REPEAT_COUNT_KEY import coil.drawable.MovieDrawable import coil.transform.AnimatedTransformation @@ -35,3 +37,25 @@ fun ImageRequest.Builder.animatedTransformation(animatedTransformation: Animated fun Parameters.animatedTransformation(): AnimatedTransformation? { return value(ANIMATED_TRANSFORMATION_KEY) as AnimatedTransformation? } + +/** Set the callback to be invoked at the start of the animation if the result is an animated [Drawable]. */ +fun ImageRequest.Builder.onAnimationStart(callback: (() -> Unit)?): ImageRequest.Builder { + return setParameter(ANIMATION_START_CALLBACK_KEY, callback) +} + +/** Get the callback to be invoked at the start of the animation if the result is an animated [Drawable]. */ +@Suppress("UNCHECKED_CAST") +fun Parameters.animationStartCallback(): (() -> Unit)? { + return value(ANIMATION_START_CALLBACK_KEY) as (() -> Unit)? +} + +/** Set the callback to be invoked at the end of the animation if the result is an animated [Drawable]. */ +fun ImageRequest.Builder.onAnimationEnd(callback: (() -> Unit)?): ImageRequest.Builder { + return setParameter(ANIMATION_END_CALLBACK_KEY, callback) +} + +/** Get the callback to be invoked at the end of the animation if the result is an animated [Drawable]. */ +@Suppress("UNCHECKED_CAST") +fun Parameters.animationEndCallback(): (() -> Unit)? { + return value(ANIMATION_END_CALLBACK_KEY) as (() -> Unit)? +} From 8310d83228cfe6ea264bce4ec1fc1426bf24b318 Mon Sep 17 00:00:00 2001 From: Sagar Viradiya Date: Sun, 21 Feb 2021 13:59:52 +0530 Subject: [PATCH 2/6] Minor code refactoring - Fix import order - Fix code formatting issue --- coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt | 4 ++-- coil-gif/src/main/java/coil/request/Gifs.kt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt b/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt index d98e9470d7..fd1fb4a595 100644 --- a/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt +++ b/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt @@ -113,8 +113,8 @@ class ImageDecoderDecoder : Decoder { baseDrawable.repeatCount = options.parameters.repeatCount() ?: AnimatedImageDrawable.REPEAT_INFINITE // Set the start and end animation callbacks if any one is supplied through the request. - if (options.parameters.animationStartCallback() != null - || options.parameters.animationEndCallback() != null) { + if (options.parameters.animationStartCallback() != null || + options.parameters.animationEndCallback() != null) { baseDrawable.registerAnimationCallback(object : Animatable2.AnimationCallback() { override fun onAnimationStart(drawable: Drawable?) { super.onAnimationStart(drawable) diff --git a/coil-gif/src/main/java/coil/request/Gifs.kt b/coil-gif/src/main/java/coil/request/Gifs.kt index 867f743675..c51077324b 100644 --- a/coil-gif/src/main/java/coil/request/Gifs.kt +++ b/coil-gif/src/main/java/coil/request/Gifs.kt @@ -5,9 +5,9 @@ package coil.request import android.graphics.drawable.AnimatedImageDrawable import android.graphics.drawable.Drawable -import coil.decode.GifDecoder.Companion.ANIMATION_START_CALLBACK_KEY import coil.decode.GifDecoder.Companion.ANIMATED_TRANSFORMATION_KEY import coil.decode.GifDecoder.Companion.ANIMATION_END_CALLBACK_KEY +import coil.decode.GifDecoder.Companion.ANIMATION_START_CALLBACK_KEY import coil.decode.GifDecoder.Companion.REPEAT_COUNT_KEY import coil.drawable.MovieDrawable import coil.transform.AnimatedTransformation From 638dd44556b4e67bb35e8412b6ac7dca287fc916 Mon Sep 17 00:00:00 2001 From: Sagar Viradiya Date: Sun, 21 Feb 2021 23:04:44 +0530 Subject: [PATCH 3/6] API dump. --- coil-gif/api/coil-gif.api | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/coil-gif/api/coil-gif.api b/coil-gif/api/coil-gif.api index aa3746caa4..c47defec73 100644 --- a/coil-gif/api/coil-gif.api +++ b/coil-gif/api/coil-gif.api @@ -1,5 +1,7 @@ public final class coil/decode/GifDecoder : coil/decode/Decoder { public static final field ANIMATED_TRANSFORMATION_KEY Ljava/lang/String; + public static final field ANIMATION_END_CALLBACK_KEY Ljava/lang/String; + public static final field ANIMATION_START_CALLBACK_KEY Ljava/lang/String; public static final field Companion Lcoil/decode/GifDecoder$Companion; public static final field REPEAT_COUNT_KEY Ljava/lang/String; public fun ()V @@ -12,6 +14,8 @@ public final class coil/decode/GifDecoder$Companion { public final class coil/decode/ImageDecoderDecoder : coil/decode/Decoder { public static final field ANIMATED_TRANSFORMATION_KEY Ljava/lang/String; + public static final field ANIMATION_END_CALLBACK_KEY Ljava/lang/String; + public static final field ANIMATION_START_CALLBACK_KEY Ljava/lang/String; public static final field Companion Lcoil/decode/ImageDecoderDecoder$Companion; public static final field REPEAT_COUNT_KEY Ljava/lang/String; public fun ()V @@ -81,6 +85,10 @@ public final class coil/drawable/ScaleDrawable : android/graphics/drawable/Drawa public final class coil/request/Gifs { public static final fun animatedTransformation (Lcoil/request/ImageRequest$Builder;Lcoil/transform/AnimatedTransformation;)Lcoil/request/ImageRequest$Builder; public static final fun animatedTransformation (Lcoil/request/Parameters;)Lcoil/transform/AnimatedTransformation; + public static final fun animationEndCallback (Lcoil/request/Parameters;)Lkotlin/jvm/functions/Function0; + public static final fun animationStartCallback (Lcoil/request/Parameters;)Lkotlin/jvm/functions/Function0; + public static final fun onAnimationEnd (Lcoil/request/ImageRequest$Builder;Lkotlin/jvm/functions/Function0;)Lcoil/request/ImageRequest$Builder; + public static final fun onAnimationStart (Lcoil/request/ImageRequest$Builder;Lkotlin/jvm/functions/Function0;)Lcoil/request/ImageRequest$Builder; public static final fun repeatCount (Lcoil/request/ImageRequest$Builder;I)Lcoil/request/ImageRequest$Builder; public static final fun repeatCount (Lcoil/request/Parameters;)Ljava/lang/Integer; } From 7f54ca464a1a0a8ecebddd5c78e304c6a37f68eb Mon Sep 17 00:00:00 2001 From: Sagar Viradiya Date: Mon, 22 Feb 2021 23:40:10 +0530 Subject: [PATCH 4/6] PR review changes. - Remove super call from Animation callbacks. - Minor code enhancements. --- coil-gif/src/main/java/coil/decode/GifDecoder.kt | 2 -- coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt | 2 -- coil-gif/src/main/java/coil/request/Gifs.kt | 4 +--- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/coil-gif/src/main/java/coil/decode/GifDecoder.kt b/coil-gif/src/main/java/coil/decode/GifDecoder.kt index fc944028ef..68d111c572 100644 --- a/coil-gif/src/main/java/coil/decode/GifDecoder.kt +++ b/coil-gif/src/main/java/coil/decode/GifDecoder.kt @@ -62,12 +62,10 @@ class GifDecoder : Decoder { if (options.parameters.animationStartCallback() != null || options.parameters.animationEndCallback() != null) { drawable.registerAnimationCallback(object : Animatable2Compat.AnimationCallback() { override fun onAnimationStart(drawable: Drawable?) { - super.onAnimationStart(drawable) options.parameters.animationStartCallback()?.invoke() } override fun onAnimationEnd(drawable: Drawable?) { - super.onAnimationEnd(drawable) options.parameters.animationEndCallback()?.invoke() } }) diff --git a/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt b/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt index fd1fb4a595..8fffca39b1 100644 --- a/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt +++ b/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt @@ -117,12 +117,10 @@ class ImageDecoderDecoder : Decoder { options.parameters.animationEndCallback() != null) { baseDrawable.registerAnimationCallback(object : Animatable2.AnimationCallback() { override fun onAnimationStart(drawable: Drawable?) { - super.onAnimationStart(drawable) options.parameters.animationStartCallback()?.invoke() } override fun onAnimationEnd(drawable: Drawable?) { - super.onAnimationEnd(drawable) options.parameters.animationEndCallback()?.invoke() } }) diff --git a/coil-gif/src/main/java/coil/request/Gifs.kt b/coil-gif/src/main/java/coil/request/Gifs.kt index c51077324b..67cf7aba3e 100644 --- a/coil-gif/src/main/java/coil/request/Gifs.kt +++ b/coil-gif/src/main/java/coil/request/Gifs.kt @@ -1,4 +1,4 @@ -@file:Suppress("unused") +@file:Suppress("unused", "UNCHECKED_CAST") @file:JvmName("Gifs") package coil.request @@ -44,7 +44,6 @@ fun ImageRequest.Builder.onAnimationStart(callback: (() -> Unit)?): ImageRequest } /** Get the callback to be invoked at the start of the animation if the result is an animated [Drawable]. */ -@Suppress("UNCHECKED_CAST") fun Parameters.animationStartCallback(): (() -> Unit)? { return value(ANIMATION_START_CALLBACK_KEY) as (() -> Unit)? } @@ -55,7 +54,6 @@ fun ImageRequest.Builder.onAnimationEnd(callback: (() -> Unit)?): ImageRequest.B } /** Get the callback to be invoked at the end of the animation if the result is an animated [Drawable]. */ -@Suppress("UNCHECKED_CAST") fun Parameters.animationEndCallback(): (() -> Unit)? { return value(ANIMATION_END_CALLBACK_KEY) as (() -> Unit)? } From e4c81a5556f3dfeaac7959fbbfac6c631a8c84aa Mon Sep 17 00:00:00 2001 From: Sagar Viradiya Date: Fri, 26 Feb 2021 01:07:46 +0530 Subject: [PATCH 5/6] Testing and decoder changes - Instrumentation testing on callback for animated drawables - Moved only decoding part to interruptible source in ImageDecoderDecoder.kt --- .../coil/callback/AnimationCallbacksTest.kt | 79 ++++++++++ .../java/coil/decode/ImageDecoderDecoder.kt | 142 +++++++++--------- 2 files changed, 152 insertions(+), 69 deletions(-) create mode 100644 coil-gif/src/androidTest/java/coil/callback/AnimationCallbacksTest.kt diff --git a/coil-gif/src/androidTest/java/coil/callback/AnimationCallbacksTest.kt b/coil-gif/src/androidTest/java/coil/callback/AnimationCallbacksTest.kt new file mode 100644 index 0000000000..b1dd339c64 --- /dev/null +++ b/coil-gif/src/androidTest/java/coil/callback/AnimationCallbacksTest.kt @@ -0,0 +1,79 @@ +package coil.callback + +import android.content.ContentResolver +import android.content.Context +import android.os.Build +import androidx.lifecycle.Lifecycle +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.rules.activityScenarioRule +import coil.ImageLoader +import coil.decode.GifDecoder +import coil.decode.ImageDecoderDecoder +import coil.request.CachePolicy +import coil.request.ImageRequest +import coil.request.onAnimationEnd +import coil.request.onAnimationStart +import coil.request.repeatCount +import coil.util.TestActivity +import coil.util.activity +import coil.util.runBlockingTest +import kotlinx.coroutines.delay +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import kotlin.test.assertTrue + +class AnimationCallbacksTest { + + private lateinit var context: Context + private lateinit var imageLoader: ImageLoader + + @get:Rule + val activityRule = activityScenarioRule() + + @Before + fun before() { + context = ApplicationProvider.getApplicationContext() + imageLoader = ImageLoader.Builder(context) + .crossfade(false) + .memoryCachePolicy(CachePolicy.DISABLED) + .diskCachePolicy(CachePolicy.DISABLED) + .build() + activityRule.scenario.moveToState(Lifecycle.State.RESUMED) + } + + @After + fun after() { + imageLoader.shutdown() + } + + @Test + fun callbacksTest() = runBlockingTest { + val imageView = activityRule.scenario.activity.imageView + var isStartCalled = false + var isEndCalled = false + val decoder = if (Build.VERSION.SDK_INT >= 28) { + ImageDecoderDecoder() + } else { + GifDecoder() + } + + val imageRequest = ImageRequest.Builder(context) + .repeatCount(0) + .onAnimationStart { + isStartCalled = true + } + .onAnimationEnd { + isEndCalled = true + } + .target(imageView) + .decoder(decoder) + .data("${ContentResolver.SCHEME_FILE}:///android_asset/animated.gif") + .build() + imageLoader.enqueue(imageRequest) + delay(5000) + assertTrue(isStartCalled) + assertTrue(isEndCalled) + } +} diff --git a/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt b/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt index 8fffca39b1..fb684a76a9 100644 --- a/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt +++ b/coil-gif/src/main/java/coil/decode/ImageDecoderDecoder.kt @@ -21,6 +21,8 @@ import coil.request.repeatCount import coil.size.PixelSize import coil.size.Size import coil.util.asPostProcessor +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import okio.BufferedSource import okio.buffer import okio.sink @@ -47,74 +49,78 @@ class ImageDecoderDecoder : Decoder { source: BufferedSource, size: Size, options: Options - ): DecodeResult = withInterruptibleSource(source) { interruptibleSource -> - var tempFile: File? = null - - try { - var isSampled = false - - val bufferedSource = interruptibleSource.buffer() - val decoderSource = if (SDK_INT >= 30) { - // Buffer the source into memory. - ImageDecoder.createSource(ByteBuffer.wrap(bufferedSource.use { it.readByteArray() })) - } else { - // Work around https://issuetracker.google.com/issues/139371066 by copying the source to a temp file. - tempFile = File.createTempFile("tmp", null, null) - bufferedSource.use { tempFile.sink().use(it::readAll) } - ImageDecoder.createSource(tempFile) - } - - val baseDrawable = decoderSource.decodeDrawable { info, _ -> - // It's safe to delete the temp file here. - tempFile?.delete() + ): DecodeResult { + var isSampled = false + val baseDrawable = withInterruptibleSource(source) { interruptibleSource -> + var tempFile: File? = null + try { + val bufferedSource = interruptibleSource.buffer() + val decoderSource = if (SDK_INT >= 30) { + // Buffer the source into memory. + ImageDecoder.createSource(ByteBuffer.wrap(bufferedSource.use { it.readByteArray() })) + } else { + // Work around https://issuetracker.google.com/issues/139371066 by copying the source to a temp file. + tempFile = File.createTempFile("tmp", null, null) + bufferedSource.use { tempFile.sink().use(it::readAll) } + ImageDecoder.createSource(tempFile) + } - if (size is PixelSize) { - val (srcWidth, srcHeight) = info.size - val multiplier = DecodeUtils.computeSizeMultiplier( - srcWidth = srcWidth, - srcHeight = srcHeight, - dstWidth = size.width, - dstHeight = size.height, - scale = options.scale - ) - - // Set the target size if the image is larger than the requested dimensions - // or the request requires exact dimensions. - isSampled = multiplier < 1 - if (isSampled || !options.allowInexactSize) { - val targetWidth = (multiplier * srcWidth).roundToInt() - val targetHeight = (multiplier * srcHeight).roundToInt() - setTargetSize(targetWidth, targetHeight) + return@withInterruptibleSource decoderSource.decodeDrawable { info, _ -> + // It's safe to delete the temp file here. + tempFile?.delete() + + if (size is PixelSize) { + val (srcWidth, srcHeight) = info.size + val multiplier = DecodeUtils.computeSizeMultiplier( + srcWidth = srcWidth, + srcHeight = srcHeight, + dstWidth = size.width, + dstHeight = size.height, + scale = options.scale + ) + + // Set the target size if the image is larger than the requested dimensions + // or the request requires exact dimensions. + isSampled = multiplier < 1 + if (isSampled || !options.allowInexactSize) { + val targetWidth = (multiplier * srcWidth).roundToInt() + val targetHeight = (multiplier * srcHeight).roundToInt() + setTargetSize(targetWidth, targetHeight) + } } - } - allocator = if (options.config == Bitmap.Config.HARDWARE) { - ImageDecoder.ALLOCATOR_HARDWARE - } else { - ImageDecoder.ALLOCATOR_SOFTWARE - } + allocator = if (options.config == Bitmap.Config.HARDWARE) { + ImageDecoder.ALLOCATOR_HARDWARE + } else { + ImageDecoder.ALLOCATOR_SOFTWARE + } - memorySizePolicy = if (options.allowRgb565) { - ImageDecoder.MEMORY_POLICY_LOW_RAM - } else { - ImageDecoder.MEMORY_POLICY_DEFAULT - } + memorySizePolicy = if (options.allowRgb565) { + ImageDecoder.MEMORY_POLICY_LOW_RAM + } else { + ImageDecoder.MEMORY_POLICY_DEFAULT + } - if (options.colorSpace != null) { - setTargetColorSpace(options.colorSpace) - } + if (options.colorSpace != null) { + setTargetColorSpace(options.colorSpace) + } - isUnpremultipliedRequired = !options.premultipliedAlpha + isUnpremultipliedRequired = !options.premultipliedAlpha - postProcessor = options.parameters.animatedTransformation()?.asPostProcessor() + postProcessor = options.parameters.animatedTransformation()?.asPostProcessor() + } + } finally { + tempFile?.delete() } + } - val drawable = if (baseDrawable is AnimatedImageDrawable) { - baseDrawable.repeatCount = options.parameters.repeatCount() ?: AnimatedImageDrawable.REPEAT_INFINITE + val drawable = if (baseDrawable is AnimatedImageDrawable) { + baseDrawable.repeatCount = options.parameters.repeatCount() ?: AnimatedImageDrawable.REPEAT_INFINITE - // Set the start and end animation callbacks if any one is supplied through the request. - if (options.parameters.animationStartCallback() != null || - options.parameters.animationEndCallback() != null) { + // Set the start and end animation callbacks if any one is supplied through the request. + if (options.parameters.animationStartCallback() != null || + options.parameters.animationEndCallback() != null) { + withContext(Dispatchers.Main.immediate) { baseDrawable.registerAnimationCallback(object : Animatable2.AnimationCallback() { override fun onAnimationStart(drawable: Drawable?) { options.parameters.animationStartCallback()?.invoke() @@ -125,20 +131,18 @@ class ImageDecoderDecoder : Decoder { } }) } - - // Wrap AnimatedImageDrawable in a ScaleDrawable so it always scales to fill its bounds. - ScaleDrawable(baseDrawable, options.scale) - } else { - baseDrawable } - DecodeResult( - drawable = drawable, - isSampled = isSampled - ) - } finally { - tempFile?.delete() + // Wrap AnimatedImageDrawable in a ScaleDrawable so it always scales to fill its bounds. + ScaleDrawable(baseDrawable, options.scale) + } else { + baseDrawable } + + return DecodeResult( + drawable = drawable, + isSampled = isSampled + ) } companion object { From 2caf46a8b9cf15006b730fcb393be688a8874840 Mon Sep 17 00:00:00 2001 From: Sagar Viradiya Date: Fri, 26 Feb 2021 22:57:50 +0530 Subject: [PATCH 6/6] PR review changes - Replace delay in the AnimationCallbacksTest.kt with StateFlow to avoid flaky test. - Fix typo in the GifDecoder.kt --- .../java/coil/callback/AnimationCallbacksTest.kt | 16 ++++++++-------- coil-gif/src/main/java/coil/decode/GifDecoder.kt | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/coil-gif/src/androidTest/java/coil/callback/AnimationCallbacksTest.kt b/coil-gif/src/androidTest/java/coil/callback/AnimationCallbacksTest.kt index b1dd339c64..cfed01d117 100644 --- a/coil-gif/src/androidTest/java/coil/callback/AnimationCallbacksTest.kt +++ b/coil-gif/src/androidTest/java/coil/callback/AnimationCallbacksTest.kt @@ -17,7 +17,8 @@ import coil.request.repeatCount import coil.util.TestActivity import coil.util.activity import coil.util.runBlockingTest -import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first import org.junit.After import org.junit.Before import org.junit.Rule @@ -51,8 +52,8 @@ class AnimationCallbacksTest { @Test fun callbacksTest() = runBlockingTest { val imageView = activityRule.scenario.activity.imageView - var isStartCalled = false - var isEndCalled = false + var isStartCalled = MutableStateFlow(false) + var isEndCalled = MutableStateFlow(false) val decoder = if (Build.VERSION.SDK_INT >= 28) { ImageDecoderDecoder() } else { @@ -62,18 +63,17 @@ class AnimationCallbacksTest { val imageRequest = ImageRequest.Builder(context) .repeatCount(0) .onAnimationStart { - isStartCalled = true + isStartCalled.value = true } .onAnimationEnd { - isEndCalled = true + isEndCalled.value = true } .target(imageView) .decoder(decoder) .data("${ContentResolver.SCHEME_FILE}:///android_asset/animated.gif") .build() imageLoader.enqueue(imageRequest) - delay(5000) - assertTrue(isStartCalled) - assertTrue(isEndCalled) + assertTrue(isStartCalled.first { it }) + assertTrue(isEndCalled.first { it }) } } diff --git a/coil-gif/src/main/java/coil/decode/GifDecoder.kt b/coil-gif/src/main/java/coil/decode/GifDecoder.kt index 68d111c572..857db808a6 100644 --- a/coil-gif/src/main/java/coil/decode/GifDecoder.kt +++ b/coil-gif/src/main/java/coil/decode/GifDecoder.kt @@ -83,7 +83,7 @@ class GifDecoder : Decoder { companion object { const val REPEAT_COUNT_KEY = "coil#repeat_count" const val ANIMATED_TRANSFORMATION_KEY = "coil#animated_transformation" - const val ANIMATION_START_CALLBACK_KEY = "coil#aimation_start_callback" - const val ANIMATION_END_CALLBACK_KEY = "coil#aimation_end_callback" + const val ANIMATION_START_CALLBACK_KEY = "coil#animation_start_callback" + const val ANIMATION_END_CALLBACK_KEY = "coil#animation_end_callback" } }