address review: keep one unbounded cover axis undefined instead of forcing a square

This commit is contained in:
austinried
2026-10-04 00:29:59 +00:00
parent 43f39300ce
commit 90171468cc
2 changed files with 34 additions and 18 deletions
@@ -36,13 +36,13 @@ import androidx.compose.ui.layout.ContentScale
import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.dp
import coil3.compose.AsyncImage import coil3.compose.AsyncImage
import coil3.compose.LocalPlatformContext import coil3.compose.LocalPlatformContext
import coil3.compose.rememberAsyncImagePainter import coil3.compose.rememberAsyncImagePainter
import coil3.request.ImageRequest import coil3.request.ImageRequest
import coil3.request.crossfade import coil3.request.crossfade
import coil3.size.Dimension
import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.CoverArtRef
private const val MAX_CACHED_RATIOS = 256 private const val MAX_CACHED_RATIOS = 256
@@ -112,22 +112,25 @@ fun CoverArt(
} else { } else {
BoxWithConstraints(modifier, contentAlignment = Alignment.Center) { BoxWithConstraints(modifier, contentAlignment = Alignment.Center) {
val density = LocalDensity.current val density = LocalDensity.current
val targetSize = val target =
remember(maxWidth, maxHeight, density) { remember(maxWidth, maxHeight, density) {
val width = with(density) { if (maxWidth.value.isFinite()) maxWidth.roundToPx() else 0 } val width = with(density) { if (maxWidth.value.isFinite()) maxWidth.roundToPx() else 0 }.takeIf { it > 0 }
val height = with(density) { if (maxHeight.value.isFinite()) maxHeight.roundToPx() else 0 } val height = with(density) { if (maxHeight.value.isFinite()) maxHeight.roundToPx() else 0 }.takeIf { it > 0 }
when { width to height
width > 0 && height > 0 -> IntSize(width, height)
width > 0 -> IntSize(width, width)
height > 0 -> IntSize(height, height)
else -> null
}
} }
val painter = val painter =
rememberAsyncImagePainter( rememberAsyncImagePainter(
model = model =
remember(ref, thumbnailRef, targetSize) { remember(ref, thumbnailRef, target) {
ref?.let { imageRequest(context, it, crossfade = thumbnailRef != null, size = targetSize) } ref?.let {
imageRequest(
context,
it,
crossfade = thumbnailRef != null,
width = target.first,
height = target.second,
)
}
}, },
onError = { failed = true }, onError = { failed = true },
contentScale = ContentScale.Fit, contentScale = ContentScale.Fit,
@@ -211,7 +214,8 @@ internal fun imageRequest(
context: Context, context: Context,
ref: CoverArtRef, ref: CoverArtRef,
crossfade: Boolean, crossfade: Boolean,
size: IntSize? = null, width: Int? = null,
height: Int? = null,
): ImageRequest { ): ImageRequest {
val builder = val builder =
ImageRequest ImageRequest
@@ -219,12 +223,14 @@ internal fun imageRequest(
.data(ref.url) .data(ref.url)
.diskCacheKey(ref.cacheKey) .diskCacheKey(ref.cacheKey)
.crossfade(crossfade) .crossfade(crossfade)
if (size == null) { if (width == null && height == null) {
builder.memoryCacheKey(ref.cacheKey) builder.memoryCacheKey(ref.cacheKey)
} else { } else {
builder builder
.size(size.width, size.height) .size(
.memoryCacheKey("${ref.cacheKey}:${size.width}x${size.height}") width?.let { Dimension.Pixels(it) } ?: Dimension.Undefined,
height?.let { Dimension.Pixels(it) } ?: Dimension.Undefined,
).memoryCacheKey("${ref.cacheKey}:${width ?: "u"}x${height ?: "u"}")
} }
return builder.build() return builder.build()
} }
@@ -1,7 +1,6 @@
package com.subtracks.ui.components package com.subtracks.ui.components
import android.content.Context import android.content.Context
import androidx.compose.ui.unit.IntSize
import androidx.test.core.app.ApplicationProvider import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.ext.junit.runners.AndroidJUnit4
import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.CoverArtRef
@@ -16,12 +15,23 @@ class CoverArtRequestTest {
val context = ApplicationProvider.getApplicationContext<Context>() val context = ApplicationProvider.getApplicationContext<Context>()
val ref = CoverArtRef(url = "http://example.test/art", cacheKey = "1:art:false") val ref = CoverArtRef(url = "http://example.test/art", cacheKey = "1:art:false")
val request = imageRequest(context, ref, crossfade = false, size = IntSize(600, 800)) val request = imageRequest(context, ref, crossfade = false, width = 600, height = 800)
assertEquals("1:art:false:600x800", request.memoryCacheKey) assertEquals("1:art:false:600x800", request.memoryCacheKey)
assertEquals(ref.cacheKey, request.diskCacheKey) assertEquals(ref.cacheKey, request.diskCacheKey)
} }
@Test
fun oneUnboundedAxisIsNotForcedSquare() {
val context = ApplicationProvider.getApplicationContext<Context>()
val ref = CoverArtRef(url = "http://example.test/art", cacheKey = "1:art:false")
val request = imageRequest(context, ref, crossfade = false, width = 600)
assertEquals("1:art:false:600xu", request.memoryCacheKey)
assertEquals(ref.cacheKey, request.diskCacheKey)
}
@Test @Test
fun unsizedRequestKeepsTheIdentityMemoryKey() { fun unsizedRequestKeepsTheIdentityMemoryKey() {
val context = ApplicationProvider.getApplicationContext<Context>() val context = ApplicationProvider.getApplicationContext<Context>()