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
This commit is contained in:
+21
-96
@@ -20,9 +20,6 @@
|
|||||||
*/
|
*/
|
||||||
package com.vitorpamplona.amethyst.service.images
|
package com.vitorpamplona.amethyst.service.images
|
||||||
|
|
||||||
import android.graphics.Bitmap
|
|
||||||
import android.graphics.BitmapFactory
|
|
||||||
import androidx.compose.runtime.Stable
|
|
||||||
import coil3.ImageLoader
|
import coil3.ImageLoader
|
||||||
import coil3.annotation.ExperimentalCoilApi
|
import coil3.annotation.ExperimentalCoilApi
|
||||||
import coil3.asImage
|
import coil3.asImage
|
||||||
@@ -45,48 +42,35 @@ import okhttp3.Call
|
|||||||
import kotlin.coroutines.cancellation.CancellationException
|
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
|
* Composables pass [ProfilePictureUrl] as the AsyncImage model. Coil routes here by type.
|
||||||
* fetcher by type — no string concatenation or scheme parsing needed.
|
|
||||||
*
|
*
|
||||||
* Flow:
|
* - Cache hit: returns a tiny ~5KB JPEG from the thumbnail disk cache.
|
||||||
* - Thumbnail cache hit: returns the tiny ~5KB JPEG directly. No network, no large decode.
|
* - Cache miss: passes the network result straight through to Coil (zero overhead),
|
||||||
* - Thumbnail cache miss (first load): passes the result straight through to Coil's normal
|
* then generates the thumbnail in the background from Coil's disk cache for next time.
|
||||||
* pipeline (zero overhead). In the background, reads the file from Coil's disk cache
|
|
||||||
* after download and generates a thumbnail for next time.
|
|
||||||
*
|
*
|
||||||
* The zoomable full-screen dialog uses the raw URL string (not wrapped in ProfilePictureUrl),
|
* Full-size display (zoomable dialog) uses the raw URL string, bypassing this fetcher.
|
||||||
* which goes through the normal Coil pipeline for full-resolution display.
|
|
||||||
*/
|
*/
|
||||||
@Stable
|
|
||||||
class ProfilePictureFetcher(
|
class ProfilePictureFetcher(
|
||||||
private val originalUrl: String,
|
private val url: String,
|
||||||
private val options: Options,
|
|
||||||
private val thumbnailCache: ThumbnailDiskCache,
|
private val thumbnailCache: ThumbnailDiskCache,
|
||||||
private val networkFetcher: Fetcher,
|
private val networkFetcher: Fetcher,
|
||||||
private val diskCacheLazy: Lazy<DiskCache?>,
|
private val diskCacheLazy: Lazy<DiskCache?>,
|
||||||
private val backgroundScope: CoroutineScope,
|
private val backgroundScope: CoroutineScope,
|
||||||
) : Fetcher {
|
) : Fetcher {
|
||||||
override suspend fun fetch(): FetchResult? {
|
override suspend fun fetch(): FetchResult? {
|
||||||
// Fast path: return pre-resized thumbnail from disk (~5KB read)
|
// Fast path: return pre-resized thumbnail (~5KB read + decode)
|
||||||
val cached = thumbnailCache.get(originalUrl)
|
val bitmap = thumbnailCache.load(url)
|
||||||
if (cached != null) {
|
if (bitmap != null) {
|
||||||
val bitmap = thumbnailCache.decodeThumbnail(cached)
|
return ImageFetchResult(
|
||||||
if (bitmap != null) {
|
image = bitmap.asImage(true),
|
||||||
return ImageFetchResult(
|
isSampled = true,
|
||||||
image = bitmap.asImage(true),
|
dataSource = DataSource.DISK,
|
||||||
isSampled = true,
|
)
|
||||||
dataSource = DataSource.DISK,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Cache miss: download via normal network fetcher.
|
// Cache miss: let Coil's normal pipeline handle download + decode
|
||||||
// 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.
|
|
||||||
val result =
|
val result =
|
||||||
try {
|
try {
|
||||||
networkFetcher.fetch() ?: return null
|
networkFetcher.fetch() ?: return null
|
||||||
@@ -95,75 +79,17 @@ class ProfilePictureFetcher(
|
|||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fire-and-forget: generate thumbnail from Coil's disk cache in background.
|
// Generate thumbnail in background from Coil's disk cache for next time
|
||||||
// The file is already there because NetworkFetcher just wrote it.
|
|
||||||
backgroundScope.launch {
|
backgroundScope.launch {
|
||||||
generateThumbnailFromDiskCache()
|
val diskCache = diskCacheLazy.value ?: return@launch
|
||||||
|
diskCache.openSnapshot(url)?.use { snapshot ->
|
||||||
|
thumbnailCache.generateFromFile(url, snapshot.data.toFile())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return result
|
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)
|
@OptIn(ExperimentalCoilApi::class)
|
||||||
class Factory(
|
class Factory(
|
||||||
private val thumbnailCache: ThumbnailDiskCache,
|
private val thumbnailCache: ThumbnailDiskCache,
|
||||||
@@ -192,7 +118,6 @@ class ProfilePictureFetcher(
|
|||||||
|
|
||||||
return ProfilePictureFetcher(
|
return ProfilePictureFetcher(
|
||||||
data.url,
|
data.url,
|
||||||
options,
|
|
||||||
thumbnailCache,
|
thumbnailCache,
|
||||||
netFetcher,
|
netFetcher,
|
||||||
diskCacheLazy,
|
diskCacheLazy,
|
||||||
|
|||||||
+57
-20
@@ -45,10 +45,9 @@ class ThumbnailDiskCache(
|
|||||||
) {
|
) {
|
||||||
companion object {
|
companion object {
|
||||||
const val THUMBNAIL_SIZE_PX = 256
|
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<String>()
|
private val inFlight = ConcurrentHashMap.newKeySet<String>()
|
||||||
|
|
||||||
init {
|
init {
|
||||||
@@ -57,36 +56,66 @@ class ThumbnailDiskCache(
|
|||||||
|
|
||||||
private fun keyFor(url: String): String = sha256(url.toByteArray()).toHexKey()
|
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))
|
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)
|
* Generates a thumbnail from a source file and saves it to the cache.
|
||||||
* to prevent readers from seeing a partially written file.
|
* 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,
|
url: String,
|
||||||
bitmap: Bitmap,
|
sourceFile: File,
|
||||||
): Boolean {
|
): Boolean {
|
||||||
// Skip if another coroutine is already saving this URL
|
|
||||||
if (!inFlight.add(url)) return false
|
if (!inFlight.add(url)) return false
|
||||||
|
|
||||||
try {
|
try {
|
||||||
val key = keyFor(url)
|
val key = keyFor(url)
|
||||||
val finalFile = File(cacheDir, key)
|
val finalFile = File(cacheDir, key)
|
||||||
|
|
||||||
// Already saved by another coroutine that finished before us
|
|
||||||
if (finalFile.exists()) return true
|
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")
|
val tempFile = File(cacheDir, "$key.tmp")
|
||||||
tempFile.outputStream().use { out ->
|
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)
|
tempFile.renameTo(finalFile)
|
||||||
|
|
||||||
evictIfNeeded()
|
evictIfNeeded()
|
||||||
@@ -98,13 +127,21 @@ class ThumbnailDiskCache(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fun decodeThumbnail(file: File): Bitmap? =
|
private fun calculateInSampleSize(
|
||||||
try {
|
options: BitmapFactory.Options,
|
||||||
BitmapFactory.decodeFile(file.absolutePath)
|
targetSize: Int,
|
||||||
} catch (e: Exception) {
|
): Int {
|
||||||
file.delete()
|
val (height, width) = options.outHeight to options.outWidth
|
||||||
null
|
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() {
|
private fun evictIfNeeded() {
|
||||||
val files = cacheDir.listFiles() ?: return
|
val files = cacheDir.listFiles() ?: return
|
||||||
|
|||||||
+1
-1
@@ -61,7 +61,7 @@ data class ProfilePictureUrl(
|
|||||||
* @param contentDescription Accessibility description
|
* @param contentDescription Accessibility description
|
||||||
* @param loadProfilePicture Whether to load the profile picture (false = show robohash only)
|
* @param loadProfilePicture Whether to load the profile picture (false = show robohash only)
|
||||||
* @param loadRobohash Whether to generate robohash (false = show generic icon)
|
* @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
|
@Composable
|
||||||
fun UserAvatar(
|
fun UserAvatar(
|
||||||
|
|||||||
Reference in New Issue
Block a user