From 361ef6798ea4f9cf4fe8113fec4995f530f3f7bb Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Apr 2026 15:36:34 +0000 Subject: [PATCH] refactor: simplify thumbnail cache API and remove unused code - ThumbnailDiskCache: combine get()+decodeThumbnail() into single load() method. Move bitmap resize logic into generateFromFile() so callers just pass a source file and the cache handles decode+resize+save. - ProfilePictureFetcher: remove unused `options` field, remove @Stable annotation, delegate all resize logic to ThumbnailDiskCache. - UserAvatar: fix stale doc comment. https://claude.ai/code/session_01PBJS3HDrurLP3i5n4tMsCv --- .../service/images/ProfilePictureFetcher.kt | 117 ++++-------------- .../service/images/ThumbnailDiskCache.kt | 77 +++++++++--- .../commons/ui/components/UserAvatar.kt | 2 +- 3 files changed, 79 insertions(+), 117 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ProfilePictureFetcher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ProfilePictureFetcher.kt index cc7c4c3d3..0f85b5926 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ProfilePictureFetcher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ProfilePictureFetcher.kt @@ -20,9 +20,6 @@ */ package com.vitorpamplona.amethyst.service.images -import android.graphics.Bitmap -import android.graphics.BitmapFactory -import androidx.compose.runtime.Stable import coil3.ImageLoader import coil3.annotation.ExperimentalCoilApi import coil3.asImage @@ -45,48 +42,35 @@ import okhttp3.Call import kotlin.coroutines.cancellation.CancellationException /** - * Coil Fetcher that serves pre-resized profile picture thumbnails from a dedicated disk cache. + * Coil Fetcher for profile picture thumbnails. * - * Composables pass [ProfilePictureUrl] as the model to AsyncImage. Coil routes to this - * fetcher by type — no string concatenation or scheme parsing needed. + * Composables pass [ProfilePictureUrl] as the AsyncImage model. Coil routes here by type. * - * Flow: - * - Thumbnail cache hit: returns the tiny ~5KB JPEG directly. No network, no large decode. - * - Thumbnail cache miss (first load): passes the result straight through to Coil's normal - * pipeline (zero overhead). In the background, reads the file from Coil's disk cache - * after download and generates a thumbnail for next time. + * - Cache hit: returns a tiny ~5KB JPEG from the thumbnail disk cache. + * - Cache miss: passes the network result straight through to Coil (zero overhead), + * then generates the thumbnail in the background from Coil's disk cache for next time. * - * The zoomable full-screen dialog uses the raw URL string (not wrapped in ProfilePictureUrl), - * which goes through the normal Coil pipeline for full-resolution display. + * Full-size display (zoomable dialog) uses the raw URL string, bypassing this fetcher. */ -@Stable class ProfilePictureFetcher( - private val originalUrl: String, - private val options: Options, + private val url: String, private val thumbnailCache: ThumbnailDiskCache, private val networkFetcher: Fetcher, private val diskCacheLazy: Lazy, private val backgroundScope: CoroutineScope, ) : Fetcher { override suspend fun fetch(): FetchResult? { - // Fast path: return pre-resized thumbnail from disk (~5KB read) - val cached = thumbnailCache.get(originalUrl) - if (cached != null) { - val bitmap = thumbnailCache.decodeThumbnail(cached) - if (bitmap != null) { - return ImageFetchResult( - image = bitmap.asImage(true), - isSampled = true, - dataSource = DataSource.DISK, - ) - } + // Fast path: return pre-resized thumbnail (~5KB read + decode) + val bitmap = thumbnailCache.load(url) + if (bitmap != null) { + return ImageFetchResult( + image = bitmap.asImage(true), + isSampled = true, + dataSource = DataSource.DISK, + ) } - // Cache miss: download via normal network fetcher. - // NetworkFetcher writes the original to Coil's disk cache and returns - // a source pointing to it. We pass this through unchanged — Coil's - // decoder pipeline handles streaming decode with inSampleSize, no - // large byte array allocation. + // Cache miss: let Coil's normal pipeline handle download + decode val result = try { networkFetcher.fetch() ?: return null @@ -95,75 +79,17 @@ class ProfilePictureFetcher( return null } - // Fire-and-forget: generate thumbnail from Coil's disk cache in background. - // The file is already there because NetworkFetcher just wrote it. + // Generate thumbnail in background from Coil's disk cache for next time backgroundScope.launch { - generateThumbnailFromDiskCache() + val diskCache = diskCacheLazy.value ?: return@launch + diskCache.openSnapshot(url)?.use { snapshot -> + thumbnailCache.generateFromFile(url, snapshot.data.toFile()) + } } return result } - /** - * Reads the original image from Coil's disk cache and generates a small - * thumbnail for our dedicated thumbnail cache. Uses file-based BitmapFactory - * decode which is seekable (supports two-pass bounds+decode) and never loads - * the entire file into a byte array. - */ - private fun generateThumbnailFromDiskCache() { - val diskCache = diskCacheLazy.value ?: return - - diskCache.openSnapshot(originalUrl)?.use { snapshot -> - val file = snapshot.data.toFile() - val targetSize = ThumbnailDiskCache.THUMBNAIL_SIZE_PX - - // First pass: decode bounds only (no memory allocation for pixels) - val boundsOptions = - BitmapFactory.Options().apply { - inJustDecodeBounds = true - } - BitmapFactory.decodeFile(file.absolutePath, boundsOptions) - - if (boundsOptions.outWidth <= 0 || boundsOptions.outHeight <= 0) return - - // Calculate inSampleSize for efficient memory use during decode - val sampleSize = calculateInSampleSize(boundsOptions, targetSize, targetSize) - - // Second pass: decode at reduced size (streams from file, not byte array) - val decodeOptions = - BitmapFactory.Options().apply { - inSampleSize = sampleSize - } - val decoded = BitmapFactory.decodeFile(file.absolutePath, decodeOptions) ?: return - - // Scale to exact target size - val scaled = Bitmap.createScaledBitmap(decoded, targetSize, targetSize, true) - if (scaled !== decoded) { - decoded.recycle() - } - - thumbnailCache.save(originalUrl, scaled) - scaled.recycle() - } - } - - private fun calculateInSampleSize( - options: BitmapFactory.Options, - reqWidth: Int, - reqHeight: Int, - ): Int { - val (height, width) = options.outHeight to options.outWidth - var inSampleSize = 1 - if (height > reqHeight || width > reqWidth) { - val halfHeight = height / 2 - val halfWidth = width / 2 - while (halfHeight / inSampleSize >= reqHeight && halfWidth / inSampleSize >= reqWidth) { - inSampleSize *= 2 - } - } - return inSampleSize - } - @OptIn(ExperimentalCoilApi::class) class Factory( private val thumbnailCache: ThumbnailDiskCache, @@ -192,7 +118,6 @@ class ProfilePictureFetcher( return ProfilePictureFetcher( data.url, - options, thumbnailCache, netFetcher, diskCacheLazy, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt index e1f7f8f30..9e3d6789c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/images/ThumbnailDiskCache.kt @@ -45,10 +45,9 @@ class ThumbnailDiskCache( ) { companion object { const val THUMBNAIL_SIZE_PX = 256 - const val JPEG_QUALITY = 80 + private const val JPEG_QUALITY = 80 } - // Tracks URLs currently being written to prevent duplicate saves private val inFlight = ConcurrentHashMap.newKeySet() init { @@ -57,36 +56,66 @@ class ThumbnailDiskCache( private fun keyFor(url: String): String = sha256(url.toByteArray()).toHexKey() - fun get(url: String): File? { + /** + * Loads a cached thumbnail bitmap for the given URL, or null if not cached. + */ + fun load(url: String): Bitmap? { val file = File(cacheDir, keyFor(url)) - return if (file.exists()) file else null + if (!file.exists()) return null + return try { + BitmapFactory.decodeFile(file.absolutePath) + } catch (e: Exception) { + file.delete() + null + } } /** - * Saves a bitmap as a JPEG thumbnail. Uses atomic write (temp file + rename) - * to prevent readers from seeing a partially written file. + * Generates a thumbnail from a source file and saves it to the cache. + * Uses two-pass BitmapFactory decode (bounds then pixels) so the full + * file is never loaded into a byte array. * - * Returns true if saved, false if already in flight or on error. + * Returns true if saved, false if already in flight, already cached, or on error. */ - fun save( + fun generateFromFile( url: String, - bitmap: Bitmap, + sourceFile: File, ): Boolean { - // Skip if another coroutine is already saving this URL if (!inFlight.add(url)) return false try { val key = keyFor(url) val finalFile = File(cacheDir, key) - - // Already saved by another coroutine that finished before us if (finalFile.exists()) return true - // Write to temp file, then atomically rename + val path = sourceFile.absolutePath + val targetSize = THUMBNAIL_SIZE_PX + + // First pass: decode bounds only + val boundsOptions = + BitmapFactory.Options().apply { + inJustDecodeBounds = true + } + BitmapFactory.decodeFile(path, boundsOptions) + if (boundsOptions.outWidth <= 0 || boundsOptions.outHeight <= 0) return false + + // Second pass: decode at reduced size + val sampleSize = calculateInSampleSize(boundsOptions, targetSize) + val decodeOptions = + BitmapFactory.Options().apply { + inSampleSize = sampleSize + } + val decoded = BitmapFactory.decodeFile(path, decodeOptions) ?: return false + + // Scale to exact target size and write atomically + val scaled = Bitmap.createScaledBitmap(decoded, targetSize, targetSize, true) + if (scaled !== decoded) decoded.recycle() + val tempFile = File(cacheDir, "$key.tmp") tempFile.outputStream().use { out -> - bitmap.compress(Bitmap.CompressFormat.JPEG, JPEG_QUALITY, out) + scaled.compress(Bitmap.CompressFormat.JPEG, JPEG_QUALITY, out) } + scaled.recycle() tempFile.renameTo(finalFile) evictIfNeeded() @@ -98,13 +127,21 @@ class ThumbnailDiskCache( } } - fun decodeThumbnail(file: File): Bitmap? = - try { - BitmapFactory.decodeFile(file.absolutePath) - } catch (e: Exception) { - file.delete() - null + private fun calculateInSampleSize( + options: BitmapFactory.Options, + targetSize: Int, + ): Int { + val (height, width) = options.outHeight to options.outWidth + var inSampleSize = 1 + if (height > targetSize || width > targetSize) { + val halfHeight = height / 2 + val halfWidth = width / 2 + while (halfHeight / inSampleSize >= targetSize && halfWidth / inSampleSize >= targetSize) { + inSampleSize *= 2 + } } + return inSampleSize + } private fun evictIfNeeded() { val files = cacheDir.listFiles() ?: return diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UserAvatar.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UserAvatar.kt index e48d70b3a..bb71afaee 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UserAvatar.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/ui/components/UserAvatar.kt @@ -61,7 +61,7 @@ data class ProfilePictureUrl( * @param contentDescription Accessibility description * @param loadProfilePicture Whether to load the profile picture (false = show robohash only) * @param loadRobohash Whether to generate robohash (false = show generic icon) - * @param useThumbnailCache Whether to wrap the URL with profilepic:// scheme for thumbnail caching + * @param useThumbnailCache Whether to use the thumbnail disk cache for faster repeated loads */ @Composable fun UserAvatar(