Skip to content

Commit

Permalink
Fix scale not being properly passed around.
Browse files Browse the repository at this point in the history
  • Loading branch information
colinrtwhite committed Nov 13, 2024
1 parent 69e1395 commit 39879d0
Show file tree
Hide file tree
Showing 16 changed files with 109 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,13 @@ import coil3.size.Scale
import coil3.test.utils.ComposeTestActivity
import coil3.test.utils.context
import java.util.concurrent.atomic.AtomicInteger
import kotlin.coroutines.EmptyCoroutineContext
import kotlin.test.assertContains
import kotlin.test.assertEquals
import kotlin.test.assertIs
import kotlin.test.assertTrue
import kotlin.test.fail
import kotlin.time.Duration.Companion.milliseconds
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.flow
import org.junit.After
Expand Down Expand Up @@ -577,7 +577,7 @@ class AsyncImageTest {
model = ImageRequest.Builder(LocalContext.current)
.data("https://example.com/image")
.memoryCachePolicy(CachePolicy.ENABLED)
.coroutineContext(Dispatchers.Main.immediate)
.coroutineContext(EmptyCoroutineContext)
.build(),
contentDescription = null,
imageLoader = imageLoader,
Expand Down
4 changes: 3 additions & 1 deletion coil-core/api/android/coil-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -665,8 +665,10 @@ public final class coil3/request/ImageRequest$Defaults {
public fun <init> ()V
public fun <init> (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;)V
public synthetic fun <init> (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults;
public final synthetic fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults;
public final fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults;
public static synthetic fun copy$default (Lcoil3/request/ImageRequest$Defaults;Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;ILjava/lang/Object;)Lcoil3/request/ImageRequest$Defaults;
public static synthetic fun copy$default (Lcoil3/request/ImageRequest$Defaults;Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;ILjava/lang/Object;)Lcoil3/request/ImageRequest$Defaults;
public fun equals (Ljava/lang/Object;)Z
public final fun getDecoderCoroutineContext ()Lkotlin/coroutines/CoroutineContext;
public final fun getDiskCachePolicy ()Lcoil3/request/CachePolicy;
Expand Down
1 change: 1 addition & 0 deletions coil-core/api/coil-core.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ final class coil3.request/ImageRequest { // coil3.request/ImageRequest|null[0]
final fun <get-sizeResolver>(): coil3.size/SizeResolver // coil3.request/ImageRequest.Defaults.sizeResolver.<get-sizeResolver>|<get-sizeResolver>(){}[0]

final fun copy(okio/FileSystem = ..., kotlin.coroutines/CoroutineContext = ..., kotlin.coroutines/CoroutineContext = ..., kotlin.coroutines/CoroutineContext = ..., coil3.request/CachePolicy = ..., coil3.request/CachePolicy = ..., coil3.request/CachePolicy = ..., kotlin/Function1<coil3.request/ImageRequest, coil3/Image?> = ..., kotlin/Function1<coil3.request/ImageRequest, coil3/Image?> = ..., kotlin/Function1<coil3.request/ImageRequest, coil3/Image?> = ..., coil3.size/Precision = ..., coil3/Extras = ...): coil3.request/ImageRequest.Defaults // coil3.request/ImageRequest.Defaults.copy|copy(okio.FileSystem;kotlin.coroutines.CoroutineContext;kotlin.coroutines.CoroutineContext;kotlin.coroutines.CoroutineContext;coil3.request.CachePolicy;coil3.request.CachePolicy;coil3.request.CachePolicy;kotlin.Function1<coil3.request.ImageRequest,coil3.Image?>;kotlin.Function1<coil3.request.ImageRequest,coil3.Image?>;kotlin.Function1<coil3.request.ImageRequest,coil3.Image?>;coil3.size.Precision;coil3.Extras){}[0]
final fun copy(okio/FileSystem = ..., kotlin.coroutines/CoroutineContext = ..., kotlin.coroutines/CoroutineContext = ..., kotlin.coroutines/CoroutineContext = ..., coil3.request/CachePolicy = ..., coil3.request/CachePolicy = ..., coil3.request/CachePolicy = ..., kotlin/Function1<coil3.request/ImageRequest, coil3/Image?> = ..., kotlin/Function1<coil3.request/ImageRequest, coil3/Image?> = ..., kotlin/Function1<coil3.request/ImageRequest, coil3/Image?> = ..., coil3.size/SizeResolver = ..., coil3.size/Scale = ..., coil3.size/Precision = ..., coil3/Extras = ...): coil3.request/ImageRequest.Defaults // coil3.request/ImageRequest.Defaults.copy|copy(okio.FileSystem;kotlin.coroutines.CoroutineContext;kotlin.coroutines.CoroutineContext;kotlin.coroutines.CoroutineContext;coil3.request.CachePolicy;coil3.request.CachePolicy;coil3.request.CachePolicy;kotlin.Function1<coil3.request.ImageRequest,coil3.Image?>;kotlin.Function1<coil3.request.ImageRequest,coil3.Image?>;kotlin.Function1<coil3.request.ImageRequest,coil3.Image?>;coil3.size.SizeResolver;coil3.size.Scale;coil3.size.Precision;coil3.Extras){}[0]
final fun equals(kotlin/Any?): kotlin/Boolean // coil3.request/ImageRequest.Defaults.equals|equals(kotlin.Any?){}[0]
final fun hashCode(): kotlin/Int // coil3.request/ImageRequest.Defaults.hashCode|hashCode(){}[0]
final fun toString(): kotlin/String // coil3.request/ImageRequest.Defaults.toString|toString(){}[0]
Expand Down
4 changes: 3 additions & 1 deletion coil-core/api/jvm/coil-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,10 @@ public final class coil3/request/ImageRequest$Defaults {
public fun <init> ()V
public fun <init> (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;)V
public synthetic fun <init> (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults;
public final synthetic fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults;
public final fun copy (Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;)Lcoil3/request/ImageRequest$Defaults;
public static synthetic fun copy$default (Lcoil3/request/ImageRequest$Defaults;Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/Precision;Lcoil3/Extras;ILjava/lang/Object;)Lcoil3/request/ImageRequest$Defaults;
public static synthetic fun copy$default (Lcoil3/request/ImageRequest$Defaults;Lokio/FileSystem;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lkotlin/coroutines/CoroutineContext;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lcoil3/request/CachePolicy;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lkotlin/jvm/functions/Function1;Lcoil3/size/SizeResolver;Lcoil3/size/Scale;Lcoil3/size/Precision;Lcoil3/Extras;ILjava/lang/Object;)Lcoil3/request/ImageRequest$Defaults;
public fun equals (Ljava/lang/Object;)Z
public final fun getDecoderCoroutineContext ()Lkotlin/coroutines/CoroutineContext;
public final fun getDiskCachePolicy ()Lcoil3/request/CachePolicy;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import coil3.util.getDrawableCompat
import coil3.util.toDrawable
import java.io.File
import java.nio.ByteBuffer
import kotlin.coroutines.EmptyCoroutineContext
import kotlin.coroutines.resume
import kotlin.coroutines.resumeWithException
import kotlin.math.roundToInt
Expand Down Expand Up @@ -268,7 +269,7 @@ class RealImageLoaderAndroidTest {
.size(100, 100)
.precision(Precision.INEXACT)
.allowHardware(true)
.coroutineContext(Dispatchers.Main.immediate)
.coroutineContext(EmptyCoroutineContext)
.target(
onStart = {
// The drawable in the memory cache should be returned here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import coil3.BitmapImage
import coil3.Extras
import coil3.ImageLoader
import coil3.memory.MemoryCache
import coil3.size.Dimension
import coil3.size.Precision
import coil3.size.Scale
import coil3.size.Size
Expand Down Expand Up @@ -70,32 +69,32 @@ internal class AndroidRequestService(
return context.getLifecycle()
}

override fun sizeResolver(request: ImageRequest): SizeResolver {
if (request.defined.sizeResolver != null) {
return request.defined.sizeResolver
}
override fun defaults(request: ImageRequest): ImageRequest.Defaults {
val sizeResolver = request.resolveSizeResolver()
val scale = request.resolveScale()
val precision = request.resolvePrecision()
var defaults = imageLoader.defaults

val target = request.target
if (target is ViewTarget<*>) {
// CENTER and MATRIX scale types should be decoded at the image's original size.
val view = target.view
if (view is ImageView && view.scaleType.let { it == CENTER || it == MATRIX }) {
return SizeResolver.ORIGINAL
} else {
return ViewSizeResolver(view)
}
} else {
// Fall back to the image's source dimensions.
return SizeResolver.ORIGINAL
if (sizeResolver != request.defaults.sizeResolver ||
scale != request.defaults.scale ||
precision != request.defaults.precision
) {
defaults = defaults.copy(
sizeResolver = sizeResolver,
scale = scale,
precision = precision,
)
}

return defaults
}

override fun options(request: ImageRequest, sizeResolver: SizeResolver, size: Size): Options {
override fun options(request: ImageRequest, size: Size): Options {
return Options(
request.context,
size,
request.resolveScale(size),
request.resolvePrecision(sizeResolver),
request.scale,
request.precision,
request.diskCacheKey,
request.fileSystem,
request.memoryCachePolicy,
Expand All @@ -105,14 +104,28 @@ internal class AndroidRequestService(
)
}

private fun ImageRequest.resolveScale(size: Size): Scale {
if (defined.scale != null) {
return defined.scale
private fun ImageRequest.resolveSizeResolver(): SizeResolver {
if (defined.sizeResolver != null) {
return defined.sizeResolver
}

// Use `Scale.FIT` if either dimension is undefined.
if (size.width == Dimension.Undefined || size.height == Dimension.Undefined) {
return Scale.FIT
if (target is ViewTarget<*>) {
// CENTER and MATRIX scale types should be decoded at the image's original size.
val view = target.view
if (view is ImageView && view.scaleType.let { it == CENTER || it == MATRIX }) {
return SizeResolver.ORIGINAL
} else {
return ViewSizeResolver(view)
}
} else {
// Fall back to the image's source dimensions.
return SizeResolver.ORIGINAL
}
}

private fun ImageRequest.resolveScale(): Scale {
if (defined.scale != null) {
return defined.scale
}

// Autodetect the scale from the ImageView.
Expand All @@ -124,7 +137,7 @@ internal class AndroidRequestService(
return scale
}

private fun ImageRequest.resolvePrecision(sizeResolver: SizeResolver): Precision {
private fun ImageRequest.resolvePrecision(): Precision {
if (defined.precision != null) {
return defined.precision
}
Expand Down Expand Up @@ -173,7 +186,7 @@ internal class AndroidRequestService(
return builder.build()
}

override fun updateOptionsOnWorkerThread(options: Options): Options {
override fun updateOptions(options: Options): Options {
var extras = options.extras
var changed = false

Expand Down Expand Up @@ -233,7 +246,7 @@ internal class AndroidRequestService(
}

/** Return 'true' if the current bitmap config is valid, else use [Bitmap.Config.ARGB_8888]. */
fun isBitmapConfigValidMainThread(
private fun isBitmapConfigValidMainThread(
request: ImageRequest,
size: Size,
): Boolean {
Expand All @@ -246,7 +259,7 @@ internal class AndroidRequestService(
}

/** Return 'true' if the current bitmap config is valid, else use [Bitmap.Config.ARGB_8888]. */
fun isBitmapConfigValidWorkerThread(
private fun isBitmapConfigValidWorkerThread(
options: Options,
): Boolean {
return !options.bitmapConfig.isHardware || hardwareBitmapService.allowHardwareWorkerThread()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
package coil3.transition

import android.widget.ImageView
import coil3.asDrawable
import coil3.asImage
import coil3.decode.DataSource
import coil3.request.DEFAULT_CROSSFADE_MILLIS
import coil3.request.ErrorResult
import coil3.request.ImageResult
import coil3.request.SuccessResult
import coil3.size.Scale
import coil3.util.scale

/**
* A [Transition] that crossfades from the current drawable to a new one.
Expand All @@ -32,7 +29,7 @@ class CrossfadeTransition @JvmOverloads constructor(
val drawable = CrossfadeDrawable(
start = target.drawable,
end = result.image?.asDrawable(target.view.resources),
scale = scale(),
scale = result.request.scale,
durationMillis = durationMillis,
fadeStart = result !is SuccessResult || !result.isPlaceholderCached,
preferExactIntrinsicSize = preferExactIntrinsicSize,
Expand All @@ -43,20 +40,6 @@ class CrossfadeTransition @JvmOverloads constructor(
}
}

private fun scale(): Scale {
val scale = result.request.defined.scale
if (scale != null) {
return scale
}

val view = target.view
if (view is ImageView) {
return view.scale
}

return result.request.scale
}

class Factory @JvmOverloads constructor(
val durationMillis: Int = DEFAULT_CROSSFADE_MILLIS,
val preferExactIntrinsicSize: Boolean = false,
Expand Down
5 changes: 2 additions & 3 deletions coil-core/src/commonMain/kotlin/coil3/RealImageLoader.kt
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ internal class RealImageLoader(

// Apply this image loader's defaults to this request.
val request = initialRequest.newBuilder()
.defaults(defaults)
.defaults(requestService.defaults(initialRequest))
.build()

// Create a new event listener.
Expand All @@ -125,7 +125,7 @@ internal class RealImageLoader(
request.listener?.onStart(request)

// Resolve the size.
val sizeResolver = requestService.sizeResolver(request)
val sizeResolver = request.sizeResolver
eventListener.resolveSizeStart(request, sizeResolver)
val size = sizeResolver.size()
eventListener.resolveSizeEnd(request, size)
Expand All @@ -138,7 +138,6 @@ internal class RealImageLoader(
index = 0,
request = request,
size = size,
sizeResolver = sizeResolver,
eventListener = eventListener,
isPlaceholderCached = cachedPlaceholder != null,
).proceed()
Expand Down
2 changes: 0 additions & 2 deletions coil-core/src/commonMain/kotlin/coil3/decode/DecodeUtils.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package coil3.decode

import coil3.annotation.ExperimentalCoilApi
import coil3.size.Dimension
import coil3.size.Scale
import coil3.size.Size
Expand Down Expand Up @@ -92,7 +91,6 @@ object DecodeUtils {
* scaled into. The returned dimensions can be passed to [computeSizeMultiplier] to get the
* final size multiplier.
*/
@ExperimentalCoilApi
@JvmStatic
fun computeDstSize(
srcWidth: Int,
Expand Down
Loading

0 comments on commit 39879d0

Please sign in to comment.