address the review: keep the stored seed on a failed decode, one cache, static bar colour

- resolve() no longer discards a stored seed when the bitmap cannot be decoded (it used to
  return null, dropping the theme for pre-v12 rows); it falls back to the stored pair
- fold the busy flag into the seed cache value so eviction is atomic (a busy entry could be
  evicted from the separate flag LRU while its seed stayed, silently disabling the scrim)
- the artist top bar samples heroBarColor at scroll 0 to match the static HeroGradient, and
  drops the wrong scroll estimate and per-pixel recomposition
- rememberOverlaidNameBusy keys its state so the cached flag is read on key change, not only
  the first composition; bottomBandBusy bails out on an empty bitmap
This commit is contained in:
austinried
2026-10-04 00:29:58 +00:00
parent f4ca0fd6d4
commit a356226e48
5 changed files with 68 additions and 31 deletions
@@ -151,14 +151,7 @@ fun ArtistDetailScreen(
BoxWithConstraints(modifier.fillMaxSize().background(Color.Black)) {
val screenHeightPx = with(density) { maxHeight.toPx() }
val barHeightPx = with(density) { barHeight.toPx() }
val scrollPx by remember {
derivedStateOf {
val index = listState.firstVisibleItemIndex
val before = if (index <= 0) 0f else with(density) { ART_HEIGHT.toPx() } * index
before + listState.firstVisibleItemScrollOffset
}
}
val barColor = heroBarColor(artwork, scrollPx, barHeightPx, screenHeightPx)
val barColor = heroBarColor(artwork, 0f, barHeightPx, screenHeightPx)
HeroGradient(
colors = artwork,
scrollPx = { 0f },
@@ -6,7 +6,10 @@ import androidx.annotation.VisibleForTesting
import androidx.collection.LruCache
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.produceState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.graphics.luminance
import androidx.palette.graphics.Palette
@@ -39,28 +42,33 @@ private const val BUSY_BRIGHT_FRACTION = 0.05f
private const val BUSY_EDGE_LUMINANCE = 0.2f
private const val BUSY_EDGE_FRACTION = 0.1f
private data class Seed(
val primary: Int,
val secondary: Int?,
val busy: Boolean?,
)
object ArtworkSeedCache {
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
private val jobs = ConcurrentHashMap<String, Deferred<Pair<Int, Int?>?>>()
private val cache = LruCache<String, Pair<Int, Int?>>(MAX_CACHED_SEEDS)
private val busy = LruCache<String, Boolean>(MAX_CACHED_SEEDS)
private val cache = LruCache<String, Seed>(MAX_CACHED_SEEDS)
@Volatile
private var store: ArtworkSeedStore? = null
fun install(store: ArtworkSeedStore) {
fun install(store: ArtworkSeedStore?) {
this.store = store
}
fun cached(cacheKey: String): Pair<Int, Int?>? = cache.get(cacheKey)
fun cached(cacheKey: String): Pair<Int, Int?>? = cache.get(cacheKey)?.let { it.primary to it.secondary }
fun cachedBusy(cacheKey: String): Boolean? = busy.get(cacheKey)
fun cachedBusy(cacheKey: String): Boolean? = cache.get(cacheKey)?.busy
suspend fun seeds(
context: Context,
ref: CoverArtRef,
): Pair<Int, Int?>? {
cache.get(ref.cacheKey)?.let { return it }
cache.get(ref.cacheKey)?.let { return it.primary to it.secondary }
val job = jobs.computeIfAbsent(ref.cacheKey) { scope.async { resolve(context, ref) } }
job.invokeOnCompletion { jobs.remove(ref.cacheKey, job) }
return job.await()
@@ -77,32 +85,31 @@ object ArtworkSeedCache {
context: Context,
ref: CoverArtRef,
): Boolean {
busy.get(ref.cacheKey)?.let { return it }
cache.get(ref.cacheKey)?.busy?.let { return it }
seeds(context, ref)
return busy.get(ref.cacheKey) ?: false
return cache.get(ref.cacheKey)?.busy ?: false
}
@VisibleForTesting
fun clear() {
jobs.clear()
cache.evictAll()
busy.evictAll()
}
private suspend fun resolve(
context: Context,
ref: CoverArtRef,
): Pair<Int, Int?>? {
ignoreFailure { store?.seed(ref.cacheKey) }?.let { stored ->
cache.put(ref.cacheKey, stored.primary to stored.secondary)
stored.nameBusy?.let { busy.put(ref.cacheKey, it) }
if (stored.nameBusy != null) return stored.primary to stored.secondary
val stored = ignoreFailure { store?.seed(ref.cacheKey) }
val storedSeeds = stored?.let { it.primary to it.secondary }
if (stored != null) {
cache.put(ref.cacheKey, Seed(stored.primary, stored.secondary, stored.nameBusy))
if (stored.nameBusy != null) return storedSeeds
}
val bitmap = decode(context, ref) ?: return null
val seeds = seedsFrom(bitmap) ?: return null
val bitmap = decode(context, ref) ?: return storedSeeds
val seeds = seedsFrom(bitmap) ?: return storedSeeds
val nameBusy = bottomBandBusy(bitmap)
cache.put(ref.cacheKey, seeds)
busy.put(ref.cacheKey, nameBusy)
cache.put(ref.cacheKey, Seed(seeds.first, seeds.second, nameBusy))
ignoreFailure { store?.save(ArtworkSeed(ref.cacheKey, seeds.first, seeds.second, nameBusy)) }
return seeds
}
@@ -139,6 +146,7 @@ object ArtworkSeedCache {
@VisibleForTesting
internal fun bottomBandBusy(bitmap: Bitmap): Boolean {
if (bitmap.width == 0 || bitmap.height == 0) return false
val firstRow = (bitmap.height * BUSY_BAND_TOP).toInt().coerceIn(0, bitmap.height - 1)
val width = bitmap.width
var previousRow: FloatArray? = null
@@ -220,7 +228,9 @@ fun PrefetchArtworkSeeds(ref: CoverArtRef?) {
fun rememberOverlaidNameBusy(ref: CoverArtRef?): Boolean {
val context = LocalPlatformContext.current
val key = ref?.cacheKey
return produceState(key?.let(ArtworkSeedCache::cachedBusy) ?: false, key) {
value = ref?.let { ArtworkSeedCache.overlaidNameBusy(context, it) } ?: false
}.value
var busy by remember(key) { mutableStateOf(key?.let(ArtworkSeedCache::cachedBusy) ?: false) }
LaunchedEffect(key) {
busy = ref?.let { ArtworkSeedCache.overlaidNameBusy(context, it) } ?: false
}
return busy
}
@@ -17,7 +17,10 @@ import coil3.SingletonImageLoader
import coil3.annotation.DelicateCoilApi
import coil3.asImage
import coil3.test.FakeImageLoaderEngine
import com.subtracks.data.model.ArtworkSeed
import com.subtracks.data.model.CoverArtRef
import com.subtracks.data.repo.ArtworkSeedStore
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
@@ -55,6 +58,7 @@ class ArtworkExtractionTest {
@After
fun resetImageLoader() {
ArtworkSeedCache.clear()
ArtworkSeedCache.install(null)
SingletonImageLoader.reset()
}
@@ -87,4 +91,20 @@ class ArtworkExtractionTest {
composeRule.waitForIdle()
assertEquals(42, state.value)
}
@Test
fun keepsTheStoredSeedWhenTheArtCannotBeDecoded() {
val key = "missing-${System.nanoTime()}"
val stored = ArtworkSeed(key, 0xFF3A7BD5.toInt(), 0xFFD53A3A.toInt(), null)
ArtworkSeedCache.install(
object : ArtworkSeedStore {
override suspend fun seed(cacheKey: String): ArtworkSeed? = stored.takeIf { it.cacheKey == cacheKey }
override suspend fun save(seed: ArtworkSeed) = Unit
},
)
val ref = CoverArtRef(url = key, cacheKey = key)
val seeds = runBlocking { ArtworkSeedCache.seeds(ApplicationProvider.getApplicationContext(), ref) }
assertEquals(stored.primary, seeds?.first)
}
}