Harden the thumbnail path: versioned cache keys, the icon as the placeholder, and a decided cache
Three things, each a failure the thumbnail path could have caused. First, correctness: Coil keys caches by the request, and a request built from a File is keyed by its path, so a picture replaced or edited outside the app kept showing its previous self - in a file manager that reads as data loss. core/ThumbnailKey now carries path, size and modification time, is built from the numbers the listing already has (no stat, so it stays off the main thread's disk work), and has four tests that name the two ways a file changes. The request carries it as the memory and disk cache key. Second, the placeholder: the row drew an empty square while a thumbnail decoded, which a thousand-row folder makes very visible. The kind icon is now drawn first and the picture paints over it, so every row shows what it is immediately and the thumbnail replaces it without a flicker. Third, the cache is decided rather than inherited: memory only, at 15% of the heap, and no disk cache at all - the sources are local files that decode cheaply, and a disk copy is a second copy that survives the file changing. Coil's default would have kept up to 2% of free space of stale thumbnails. Measured on the phone and recorded in EDGE_CASE_TESTING: a 6200-file folder holding 300 real JPEGs opened and scrolled five times with no crash, no OOM and text rows keeping their icons (PSS 86.6 MB), and a folder of 300 real JPEGs where every row drew its own picture and eight swipes down and one back left PSS at 149.6 MB with the cache full and stable. Stated as not measured: jank on this screen (this device reports no frame durations) and a 20000-image folder.
This commit is contained in:
@@ -6,6 +6,7 @@ import android.content.pm.ApplicationInfo
|
||||
import android.os.StrictMode
|
||||
import coil3.ImageLoader
|
||||
import coil3.SingletonImageLoader
|
||||
import coil3.memory.MemoryCache
|
||||
import coil3.video.VideoFrameDecoder
|
||||
import de.softwareapp_hb.filemanager.data.ApkInspector
|
||||
import de.softwareapp_hb.filemanager.data.BookmarkStore
|
||||
@@ -122,5 +123,29 @@ class FileManagerApplication : Application(), SingletonImageLoader.Factory {
|
||||
override fun newImageLoader(context: Context): ImageLoader =
|
||||
ImageLoader.Builder(context)
|
||||
.components { add(VideoFrameDecoder.Factory()) }
|
||||
// Memory only, and a size decided rather than inherited.
|
||||
//
|
||||
// A disk cache would hold a second copy of pictures that are already on this device: the
|
||||
// sources are local files, the decode is cheap, and the copy is a liability — it survives the
|
||||
// file being edited or replaced and would then serve the old picture from an entry nothing
|
||||
// invalidates. The memory cache is what makes scrolling back up a thousand rows instant, and
|
||||
// it is bounded explicitly so a big folder cannot grow it into the app's whole heap.
|
||||
.diskCache(null)
|
||||
.memoryCache {
|
||||
MemoryCache.Builder()
|
||||
.maxSizePercent(context, MEMORY_CACHE_PERCENT)
|
||||
.build()
|
||||
}
|
||||
.build()
|
||||
private companion object {
|
||||
/**
|
||||
* How much of the app's heap the thumbnail memory cache may use.
|
||||
*
|
||||
* Coil's own default is a quarter. A file manager's thumbnails are small and numerous, and the
|
||||
* rows are also holding on to their own state, so this leaves the rest of the heap to the listing
|
||||
* and to Compose — a size chosen with the screen in mind rather than inherited.
|
||||
*/
|
||||
const val MEMORY_CACHE_PERCENT = 0.15
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -0,0 +1,22 @@
|
||||
package de.softwareapp_hb.filemanager.core
|
||||
|
||||
/**
|
||||
* The identity of a thumbnail: which file it came from, and which *version* of that file.
|
||||
*
|
||||
* Coil keys its caches by whatever a request says, and by default that is the `File` — its path. Keyed
|
||||
* by path alone, a file that is replaced, edited or renamed-over outside the app keeps showing the
|
||||
* picture the cache already has: nothing tells Coil the pixels changed. In a file manager that is not a
|
||||
* cosmetic bug — the app exists to show what is on disk — so the key carries the size and the
|
||||
* modification time, and a file that changes becomes a different entry.
|
||||
*
|
||||
* It is a pure function over data the listing already carries, deliberately: computing the key must not
|
||||
* `stat` the file. The listing has just read those numbers, and asking the filesystem again for every
|
||||
* visible row would put disk reads back on the thread that draws them — the exact cost that was taken
|
||||
* off the startup path in [docs/PERFORMANCE.md].
|
||||
*/
|
||||
object ThumbnailKey {
|
||||
|
||||
/** A cache key that changes when the file's path, size or modification time changes. */
|
||||
fun of(path: String, sizeBytes: Long, lastModifiedMillis: Long): String =
|
||||
"$path|$sizeBytes|$lastModifiedMillis"
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import android.content.Context
|
||||
import androidx.compose.foundation.ExperimentalFoundationApi
|
||||
import androidx.compose.foundation.combinedClickable
|
||||
import androidx.compose.foundation.layout.Arrangement
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.Spacer
|
||||
import androidx.compose.foundation.layout.fillMaxWidth
|
||||
@@ -53,6 +54,7 @@ import de.softwareapp_hb.filemanager.core.FileCategory
|
||||
import de.softwareapp_hb.filemanager.core.FileEntry
|
||||
import de.softwareapp_hb.filemanager.core.FileKind
|
||||
import de.softwareapp_hb.filemanager.core.FileSize
|
||||
import de.softwareapp_hb.filemanager.core.ThumbnailKey
|
||||
import java.io.File
|
||||
|
||||
/** The thumbnail an entry's leading visual gets in a list row. */
|
||||
@@ -89,21 +91,10 @@ fun EntryIcon(entry: FileEntry, size: Dp) {
|
||||
// what it settled on, which is the icon.
|
||||
var failed by remember(entry.path.value) { mutableStateOf(false) }
|
||||
|
||||
if (entry.hasThumbnail() && !failed) {
|
||||
val context = LocalContext.current
|
||||
// Remembered, because Coil keys its caches by the request: a new request object on every
|
||||
// recomposition would be a new cache entry on every recomposition.
|
||||
val model = remember(entry.path.value, entry.kind) { entry.thumbnailRequest(context) }
|
||||
AsyncImage(
|
||||
model = model,
|
||||
contentDescription = null,
|
||||
contentScale = ContentScale.Crop,
|
||||
onError = { failed = true },
|
||||
modifier = Modifier
|
||||
.size(size)
|
||||
.clip(RoundedCornerShape(6.dp)),
|
||||
)
|
||||
} else {
|
||||
// The icon is drawn first and the thumbnail paints over it. That is the placeholder: a row scrolled
|
||||
// into view while a thousand others are still decoding shows the file's kind immediately instead of
|
||||
// an empty square, and the picture replaces it without a flicker.
|
||||
Box(modifier = Modifier.size(size), contentAlignment = Alignment.Center) {
|
||||
Icon(
|
||||
imageVector = iconFor(entry.kind),
|
||||
contentDescription = null,
|
||||
@@ -114,6 +105,25 @@ fun EntryIcon(entry: FileEntry, size: Dp) {
|
||||
MaterialTheme.colorScheme.onSurfaceVariant
|
||||
},
|
||||
)
|
||||
|
||||
if (entry.hasThumbnail() && !failed) {
|
||||
val context = LocalContext.current
|
||||
// Remembered, because Coil keys its caches by the request: a new request object on every
|
||||
// recomposition would be a new cache entry on every recomposition. The size and timestamp are
|
||||
// part of the key (and of this `remember`), so a file that changes is decoded again.
|
||||
val model = remember(entry.path.value, entry.kind, entry.sizeBytes, entry.lastModifiedMillis) {
|
||||
entry.thumbnailRequest(context)
|
||||
}
|
||||
AsyncImage(
|
||||
model = model,
|
||||
contentDescription = null,
|
||||
contentScale = ContentScale.Crop,
|
||||
onError = { failed = true },
|
||||
modifier = Modifier
|
||||
.size(size)
|
||||
.clip(RoundedCornerShape(6.dp)),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -132,14 +142,20 @@ private fun FileEntry.hasThumbnail(): Boolean =
|
||||
* The frame is taken [VIDEO_FRAME_MILLIS] in rather than at zero, because the first frame of a
|
||||
* recording is very often black or the middle of a fade-in, and a thumbnail of a black rectangle is
|
||||
* no more useful than the icon it replaced.
|
||||
*
|
||||
* Both carry an explicit cache key built from the listing's own numbers, so a file that has changed since
|
||||
* it was cached decodes again instead of showing the previous picture; see [ThumbnailKey] for why the key
|
||||
* carries more than the path, and why it does not `stat` the file.
|
||||
*/
|
||||
private fun FileEntry.thumbnailRequest(context: Context): Any {
|
||||
val file = File(path.value)
|
||||
if (kind != FileKind.VIDEO) return file
|
||||
return ImageRequest.Builder(context)
|
||||
val key = ThumbnailKey.of(path.value, sizeBytes, lastModifiedMillis)
|
||||
val request = ImageRequest.Builder(context)
|
||||
.data(file)
|
||||
.videoFrameMillis(VIDEO_FRAME_MILLIS)
|
||||
.build()
|
||||
.memoryCacheKey(key)
|
||||
.diskCacheKey(key)
|
||||
if (kind == FileKind.VIDEO) request.videoFrameMillis(VIDEO_FRAME_MILLIS)
|
||||
return request.build()
|
||||
}
|
||||
|
||||
/** Where in a video its thumbnail is taken: far enough in to be past the usual fade-in. */
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
package de.softwareapp_hb.filemanager.core
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNotEquals
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* What a thumbnail's cache entry is allowed to remember.
|
||||
*
|
||||
* These are the two ways a file manager shows a wrong picture, and the key is what prevents both: a file
|
||||
* whose contents were replaced (same path, new size and timestamp) and a file that was edited in place
|
||||
* (same size, new timestamp).
|
||||
*/
|
||||
class ThumbnailKeyTest {
|
||||
|
||||
@Test
|
||||
fun `the same file with the same size and timestamp is the same cache entry`() {
|
||||
assertEquals(
|
||||
ThumbnailKey.of("/storage/emulated/0/DCIM/a.jpg", 4096, 1_700_000_000_000),
|
||||
ThumbnailKey.of("/storage/emulated/0/DCIM/a.jpg", 4096, 1_700_000_000_000),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a replaced file is a different cache entry even though its path is unchanged`() {
|
||||
assertNotEquals(
|
||||
ThumbnailKey.of("/storage/emulated/0/DCIM/a.jpg", 4096, 1_700_000_000_000),
|
||||
ThumbnailKey.of("/storage/emulated/0/DCIM/a.jpg", 8192, 1_700_000_000_000),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a file edited in place is a different cache entry even though its size is unchanged`() {
|
||||
assertNotEquals(
|
||||
ThumbnailKey.of("/storage/emulated/0/DCIM/a.jpg", 4096, 1_700_000_000_000),
|
||||
ThumbnailKey.of("/storage/emulated/0/DCIM/a.jpg", 4096, 1_700_000_060_000),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `two files with the same size and timestamp do not share an entry`() {
|
||||
assertNotEquals(
|
||||
ThumbnailKey.of("/storage/emulated/0/DCIM/a.jpg", 4096, 1_700_000_000_000),
|
||||
ThumbnailKey.of("/storage/emulated/0/DCIM/b.jpg", 4096, 1_700_000_000_000),
|
||||
)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user