mipmap cache: the eviction write is about the entry's state, and it lets go of a file before unlinking it by aurelienpierre · Pull Request #1495 · aurelienpierreeng/ansel · GitHub

Second PR of the thumbnail/cache cluster (#1472–#1479). Closes #1477 and #1478 — one PR because both
live in dt_mipmap_cache_deallocate_dynamic(), the single place a thumbnail reaches the disk cache,
and splitting them would mean two PRs editing the same twenty lines.

Four defects, one function

1. The write predicate was dsc->width > 8 && dsc->height > 8 and nothing else (#1478). That
catches the skull and misses the other two states a buffer can be in:

Never generated. A fresh entry carries DT_MIPMAP_BUFFER_DSC_FLAG_GENERATE over an
uninitialised dt_alloc_align() block at the mip's full dimensions — so the size test passed and
the JPEG encoder was handed whatever the allocator returned. This is not a corner case:
_generate_blocking() deliberately leaves such an entry behind whenever the shutdown atomic is
raised mid-render, and dt_view_image_get_surface_async() raises it on every widget resize or image
change. Scrolling a filmstrip produces them; eviction or dt_cache_cleanup() at exit wrote each one
out as a thumbnail of uninitialised heap, which the next session loaded as valid. (A minor
information disclosure as well as a visible defect.)
Invalidated. The INVALIDATE branch unlinked the file and then, with no else, immediately
wrote back the very buffer just declared stale. dt_mipmap_cache_remove_at_size() survived that
only by accident — it unlinks a second time after dt_cache_remove() returns — so with
flush_disk FALSE nothing did, and dropping an entry the caller wanted gone persisted it instead.

2. g_unlink() ran while a handle on the file was still open — both in the read path and in the
write path's error branch (#1477). On Windows DeleteFileW refuses a file any handle holds open
without FILE_SHARE_DELETE, which the CRT's fopen never sets, so a bad disk-cache entry could
never be cleared on Windows: re-read, re-rejected and re-logged on every start, permanently.
Invisible on POSIX, where unlinking an open file is ordinary.

3. The output file was opened twice. dt_imageio_jpeg_write() opens, writes and closes the path
itself and returns non-zero on failure; the caller also opened it "wb" purely to test writability,
which truncated it before the free-space check ran and held a second handle across the whole write.
That outer open is gone.

4. A free-space probe that could not answer vetoed the write. It ran statvfs() on the output
file — hence the redundant open, the file had to exist first — and treated an unanswerable probe as
"no room". src/win/statvfs.c built a drive root out of path[0]: right for C:\..., wrong for a UNC
share, a redirected %LOCALAPPDATA% or a drive mounted as a folder, and there every thumbnail write
failed silently. The probe now runs on the directory before anything is created and answers TRUE when
it cannot tell; the shim uses GetDiskFreeSpaceExW, which takes any path.

The write block moved into _write_thumbnail_to_disk_cache(), which retires a goto write_error that
jumped into the middle of an if/else across two scopes.

What was measured

Staged build, isolated --configdir/--cachedir/--library, one NEF, --disable-opencl:

	result
ansel-generate-cache --min-mip 0 --max-mip 2	3 thumbnails written (16164 / 61057 / 201976 B), 0 withheld by the two new guards
second run on the same cache	all three found valid, left byte-identical
truncate one to 0 bytes, re-run	reported Reason: empty file, unlinked, regenerated and rewritten
A/B against origin/master, same image, fresh cache each side	bit-identical on all three (sha256)

That last row is the one that matters for a rewrite this size: the restructured writer produces
byte-for-byte what master produces.

Build clean with 0 warnings in both build and build-nofeatures; pragma_once_to_guards.py --verify passes; include_graph.py --summary reports cycles 0.

What is not measured, and what I did about it

The never-generated case needs the GUI's abort path and the Windows unlink/statvfs behaviour needs
Windows, so neither is reachable from this machine. Rather than assert them, the skip is now logged
under -d cache:

[mipmap_cache] image 42 at mip size 2 is not written to the disk cache: never generated | a skull

so a real session can answer how often an aborted render leaves an entry behind — a question this
cluster has repeatedly had to guess at. On the measured path that line appears 0 times, which is the
evidence that the guards reject nothing a real render produced.

One thing noticed in passing, not fixed here

dt_util_test_image_file() (src/common/utility.c) checks readable + regular + size > 0, and nothing
about the content. ansel-generate-cache uses it to decide "a valid thumbnail is already on disc — do
nothing", so a non-empty garbage cache file is skipped forever by that tool. Measured: a 28-byte text
file planted as a cached thumbnail was never repaired, while the 0-byte one was. The GUI read path
does reject and regenerate it (that is the path fixed above), so this is a wart in the offline tool
rather than a user-visible defect. Say if you want it filed.

🤖 Generated with Claude Code

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

…ets go of a file before unlinking it Four defects in the one place a thumbnail reaches the disk cache, each of them a way to leave a cached file that should not exist or that cannot be removed. The write predicate was `dsc->width > 8 && dsc->height > 8` and nothing else. That catches the skull and misses the other two states a buffer can be in: - An entry that was NEVER GENERATED carries DT_MIPMAP_BUFFER_DSC_FLAG_GENERATE over an uninitialised dt_alloc_align() block at the mip's full dimensions, so the size test passed and the JPEG encoder was handed whatever the allocator returned. Not a corner case: _generate_blocking() deliberately leaves such an entry behind whenever the shutdown atomic is raised mid-render, and dt_view_image_get_surface_async() raises it on every widget resize or image change -- so scrolling a filmstrip produced them, and eviction or dt_cache_cleanup() at exit wrote each one out as a thumbnail of uninitialised heap, which the next session then loaded as valid (#1478). - An INVALIDATED entry unlinked its file and then, with no `else` between the two branches, immediately wrote back the very buffer that had just been declared stale. dt_mipmap_cache_remove_at_size() survived that only by accident, unlinking a second time after dt_cache_remove() returns; with flush_disk FALSE nothing did, so dropping an entry the caller wanted gone persisted it instead. g_unlink() now comes after fclose(), in both halves. On Windows DeleteFileW refuses a file any handle still holds open without FILE_SHARE_DELETE, which the CRT's fopen never sets. The read path unlinked a file it had judged corrupt while its own read handle was open, and the write path's error branch unlinked while holding one of its own -- so on Windows a bad disk-cache entry could never be cleared: re-read, re-rejected and re-logged on every start, for good (#1477). Invisible on POSIX, where unlinking an open file is ordinary. The output file is opened once. dt_imageio_jpeg_write() opens, writes and closes the path itself and returns non-zero on failure, which is the only answer the caller needs; the caller also opened it "wb" purely to test writability, which truncated it before the free-space check ran and held a second handle across the whole write. A free-space probe that cannot answer no longer vetoes the write. It ran statvfs() on the output file -- hence the redundant open, since the file had to exist first -- and treated an unanswerable probe as "no room". src/win/statvfs.c built a drive root out of path[0], right for C:\... and wrong for a UNC share, a redirected %LOCALAPPDATA% or a drive mounted as a folder, and there EVERY thumbnail write failed silently. The probe now runs on the directory before anything is created and answers TRUE when it cannot tell; the shim uses GetDiskFreeSpaceExW, which takes any path, and reports bytes with f_frsize == 1 -- every consumer computes f_frsize * f_bavail, which that satisfies exactly, at the cost of the cluster geometry nothing here reads. The write block moved into _write_thumbnail_to_disk_cache(), which is what retires the `goto write_error` that jumped into the middle of an if/else across two scopes. Measured with a staged build, an isolated --configdir/--cachedir/--library and one NEF: ansel-generate-cache --min-mip 0 --max-mip 2 writes 3 thumbnails (16164 / 61057 / 201976 bytes) and skips none; a second run finds all three valid and leaves them byte-identical; truncating one to 0 bytes gets it reported ("Reason: empty file"), unlinked and rewritten. The new skip branch reports 0 entries withheld on that path, i.e. the two added guards reject nothing a real render produced. Not exercised locally: the never-generated case needs the GUI's abort path, and the Windows unlink and statvfs behaviour needs Windows. The skip is therefore logged under `-d cache` rather than silent, so a real session can answer how often an aborted render leaves an entry behind. Findings recorded in doc/image-mipmap-cache.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…oc says what the missing else cost The INVALIDATE flag is set on a thumbnail only by dt_mipmap_cache_remove_at_size() with flush_disk TRUE, which unlinks a second time once dt_cache_remove() has returned. The missing `else` therefore never left a stale file behind, and with flush_disk FALSE the flag is never set at all, as the code comment and doc/image-mipmap-cache.md had it. What it cost was a JPEG encode and write per removed size, and a window in which a get of the same key, whose allocate callback reads the disk cache under the cache lock that dt_cache_remove() has already released, loads the stale file back. The doc says so, as read from src/caches/cache.c and not measured, and records the earlier claim. The doc no longer says GetDiskFreeSpaceExW takes any path: it takes a directory, and its documentation asks a UNC path to end in a backslash, which the cache directory does not. On a UNC share the probe may still fail, now without consequence since the write goes ahead. Not tried on Windows. The code comments are cut to the rule, the account staying in doc/, as AGENTS.md asks. _entry_holds_a_thumbnail(), a two-line predicate with one caller, is inlined there. The messages the rewrite added print the image id with PRId32. src/win/statvfs.c names both of its 2026 authors. Re-measured on the rebase onto 2fadf8a and recorded in the doc: ansel-generate-cache on one NEF, mips 0-2, writes three thumbnails with 0 skip lines, a second run leaves them byte-identical, a truncated one is reported, unlinked and rewritten identically, and the three files are bit-identical (sha256) to those written by master's src/caches/mipmap_cache.c. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…OCK get either A render aborted by the shutdown atomic leaves its entry in the cache flagged GENERATE, over a buffer that holds nothing. The eviction write now keeps such an entry off the disk, but a DT_MIPMAP_TESTLOCK get still handed its buffer back at the box's full dimensions, and _init_8() builds a size from the next larger one it finds that way: the smaller size was downsampled from nothing and became an ordinary thumbnail, shown and written to disk. A TESTLOCK get now releases such an entry and answers as if it were not there, which is what its header says ("would have to be loaded first") and what both callers mean by it: _init_8() wants a source to downsample, and the darkroom's cache_ready test wants a frame. Measured with a program linked against libansel that requests size 2 of one NEF with the shutdown atomic already raised, then size 1, with lighttable/embedded_jpg=0 so that _init_8() looks for a larger size. On 2fadf8a, TESTLOCK returns size 2 as 1440x900 of zeros, size 1 is downsampled from it to 720x450 of zeros, and both are written to the disk cache black. With the eviction write fixed only, size 2 is withheld and the black size 1 is still written. With this change, TESTLOCK returns no buffer, size 1 is rendered from scratch (300x450, mean byte value 127), and that is what is written. With ansel-generate-cache and lighttable/embedded_jpg=0, one size rendered from scratch and two downsampled from a larger one, the three files are bit-identical with and without this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Rebased onto 2fadf8a366, with two commits on top. The only conflict was in doc/image-mipmap-cache.md, where master added the section on where the disk cache lives at the same place; both sections are kept. The first commit's code is unchanged.

Two corrections to the description above

Invalidated entries. The description says that with flush_disk FALSE the stale thumbnail was persisted. With flush_disk FALSE the flag is never set: INVALIDATE is set on a thumbnail only by dt_mipmap_cache_remove_at_size() with flush_disk TRUE, which unlinks a second time once dt_cache_remove() has returned, so no stale file outlived the call. What the missing else cost was a JPEG encode and write per removed size, and a window: dt_cache_remove() releases the cache lock before that second unlink, and a get of the same key in between runs the allocate callback, which reads the stale file back. Read from src/caches/cache.c, not measured. The fix itself stands.
GetDiskFreeSpaceExW does not take any path. It takes a directory, and its documentation asks a UNC path to end in a backslash, which the cache directory does not. On a UNC share the probe may still fail, now without consequence since the write goes ahead. Not tried on Windows.

2d147f7eda: comments, doc, formats

The code comment and doc/image-mipmap-cache.md carry the two corrections above.
Code comments are cut to the rule, the account staying in doc/, as AGENTS.md asks; _entry_holds_a_thumbnail(), a two-line predicate with one caller, is inlined there.
The messages the rewrite added print the image id with PRId32.
src/win/statvfs.c names both of its 2026 authors.

29bfc796f7: an aborted entry is not there for a TESTLOCK get either

Keeping a never-generated entry off the disk was not enough. Unless the embedded JPEG is used (the default for an unedited raw), _init_8() builds a size from the next larger one a DT_MIPMAP_TESTLOCK get finds, and that get handed back an aborted entry's buffer at the box's full dimensions: the smaller size was downsampled from nothing, then shown and written to disk as an ordinary thumbnail. A TESTLOCK get now releases an entry still flagged GENERATE and answers as if it were not there, which is what its header already says ("would have to be loaded first") and what both callers mean by it (_init_8()'s source, the darkroom's cache_ready).

Measured with a program linked against libansel: request size 2 of one NEF with the shutdown atomic already raised, then size 1 (lighttable/embedded_jpg=0, --disable-opencl, fresh --configdir, --cachedir and --library):

build	TESTLOCK of size 2	size 1	on disk
master 2fadf8a366	1440×900, all zeros	downsampled from it, 720×450, all zeros	both, black
+ the eviction-write commit	1440×900, all zeros	downsampled from it, 720×450, all zeros	size 1, black
+ 29bfc796f7	no buffer	rendered from scratch, 300×450, mean 127	size 1, real

This is also the first measurement of #1478 itself: on master, the aborted size 2 is written to the disk cache as a black 1440×900 JPEG. The buffer read as zeros here, a fresh allocation; nothing guarantees that in a long session. How often the GUI's abort path produces such entries in a real session is still not measured.

Re-measured on the rebased branch

clang-19, RelWithDebInfo, one NEF, --disable-opencl, fresh --configdir/--cachedir/--library:

	result
ansel-generate-cache --min-mip 0 --max-mip 2	3 thumbnails written (5792 / 15531 / 44178 B), skip line 0 times
second run	byte-identical
truncate one to 0 bytes, re-run	Reason: empty file, unlinked, rewritten identically
A/B against master's src/caches/mipmap_cache.c	bit-identical on all three (sha256)
same with lighttable/embedded_jpg=0 (1 size from scratch, 2 downsampled), with and without 29bfc796f7	bit-identical on all three

All eight blocking CI gates pass locally (check_layering, check_module_boundaries, check_return_types, check_list_order, pragma_once_to_guards --verify, check_conditional_includes, check_statelessness, check_unused_includes --changed), include_graph.py --summary reports cycles 0, ctest passes 30/30 under a window-manager-less X server, and check_it_runs.sh passes.

Noticed in passing, not fixed here

A thumbnail loaded from the disk cache is re-encoded and rewritten at every eviction and at exit, losing JPEG quality each session; the FIXME that said so went with the old write block.

…n wrote with and without the eviction write fixed The account of never-generated entries called them the common case and said scrolling a filmstrip produced them. Only the async surface fetcher passes a shutdown flag (preview window, slideshow, map, print, studio capture); lighttable and filmstrip thumbnails go through dt_view_image_get_surface(), which passes none, and never abort. The next get of the same key renders over an aborted entry, so one survives to eviction only when nothing asks that size of that image again, as when a preview window is enlarged and closed before its render ends. Measured in the GUI on twelve hardlinks of one NEF with fresh config, cache and library: 27 minutes of lighttable and filmstrip scrolling aborted nothing; enlarging and closing the preview window on three images wrote five all-black JPEGs at the box's full dimensions with master's mipmap_cache.c, and none with this branch, where the four entries left behind are logged "never generated". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

#1478 reproduced in the GUI, and one more correction to the description.

Which renders abort

The description says scrolling a filmstrip produces never-generated entries. It does not: only the async surface fetcher passes a shutdown flag (dt_view_image_get_surface_async(): preview window, slideshow, map, print, studio capture). Lighttable and filmstrip thumbnails go through dt_view_image_get_surface(), which passes none. The next get of the same key renders over an aborted entry, so one survives to eviction only when nothing asks that size of that image again, as when a preview window is enlarged and then closed before its render ends.

Measured

Twelve hardlinks of one NEF, lighttable/embedded_jpg=0, fresh --configdir/--cachedir/--library, -d cache; aborts counted by their DT_IMAGEIO_ABORTED ("error 11") line.

session	aborts	entries left never generated	black JPEGs written at exit
this branch, 27 min of lighttable and filmstrip scrolling, 5 per row	0	0	0
master's mipmap_cache.c, preview window enlarged then closed within a second, 3 images	14	5	5: size 2 of images 2, 6, 8 at 1440×900 (a real render is 600×900), size 1 of images 6, 8 at 720×450 (real: 300×450)
this branch, same gestures	11	4, each logged never generated	0 (13 files written, all real images)

On the same branch, the darkroom refresh path that uses TESTLOCK (_preview_pipe_finished()'s cache_ready) still updates the lighttable and filmstrip thumbnail after an exposure change. No GUI session built a size from an aborted larger one, so the TESTLOCK half stays measured only by the program in the previous comment.

562836a6d2 corrects the account in doc/image-mipmap-cache.md and records these measurements.

…k to it The read path left no trace on the entry, so eviction and exit could not tell a thumbnail read from its file from a fresh render, and re-encoded it as JPEG at every session; the comment over the write said existing files were not written, and the FIXME under it that they were. Measured over 150 sessions on one cache: sizes 0 and 1 settle after 8 and 12 rewrites, but on size 2 one 8x8 block gains one level per session until it is near white, after about 100 sessions. The entry now carries DT_MIPMAP_BUFFER_DSC_FLAG_ON_DISK from the read until new pixels are written into it, and the eviction write skips it while it does; dt_mipmap_cache_swap_at_size(), the only path that writes new pixels into an entry read from disk, clears it. With it the files are byte-identical over 30 sessions, the first one byte-identical to before, and a disk-read entry overwritten by the darkroom is written back. On top of the eviction rewrite of this PR, checked again with two GUI sessions on one cache, quit cleanly from gdb: the second session loaded 13 thumbnails from disk, wrote none of them back (`not written to the disk cache: read from it`), and left all 13 files byte-identical; its 16 writes were thumbnails rendered for the first time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Added one commit on top, 5634022: a thumbnail read from the disk cache is not written back to it at eviction.

It was #1597 (closed as a duplicate of the eviction rewrite here); ported onto this branch since both touch dt_mipmap_cache_deallocate_dynamic(), so they cannot merge separately without a conflict.

The descriptor gets DT_MIPMAP_BUFFER_DSC_FLAG_ON_DISK (1 << 2) when the pixels come from the disk-cache file; the eviction write skips such an entry; dt_mipmap_cache_swap_at_size() clears it, so an entry the darkroom overwrites is written as before.
doc/image-mipmap-cache.md gets a section.

Measured on this branch, two GUI sessions on one cache, quit from gdb with dt_control_quit(): the second session loaded 13 thumbnails from disk, wrote none of them back (not written to the disk cache: read from it × 13), and the 13 files are byte-identical before and after; its 16 writes were all thumbnails rendered for the first time. Gates: conditional/unused includes, pragma verify, ctest 30/30.

🤖 Generated with Claude Code

Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

…r a source _init_8() and _init_f() report a failure as a zero-sized image, and _generate_blocking() then cleared DT_MIPMAP_BUFFER_DSC_FLAG_GENERATE unconditionally. So the failure was latched on the spot: every later request took the "already in cache, no I/O needed" path and got the 8x8 skull back, and dt_view_image_get_surface() reports an 8x8 buffer as DT_VIEW_SURFACE_OK, so the widget committed it and never asked again. One transient fault -- a momentary database lock, a network share reconnecting -- therefore cost an image its thumbnail for the whole session, clearable only through "Purge selected thumbnails from cache", which is what the reporter found by trial and error (#1474). One retry is granted instead, recorded in a new RETRIED flag. Bounded on purpose: an image that is genuinely unreadable must not re-run a full pipeline on every redraw, so it costs at most two attempts per cache entry. Read with GENERATE the flag gives the three states a failed thumbnail can be in -- GENERATE|RETRIED is "failed once, try again", RETRIED alone is "failed twice, we have stopped", neither is a thumbnail that generated normally -- and `-d cache` now says which. The retry needs the descriptor put back before it runs, and this is the part that is easy to get wrong: the failed attempt left the skull's 8x8 in dsc->width/height, and _init_8() opens with `if(size >= DT_MIPMAP_F || *width < 16 || *height < 16) return;`. Re-entering with those dimensions does nothing at all, silently, so the retry would have been a guaranteed second failure and the flag would have bought nothing. _generate_blocking() restores what dt_mipmap_cache_update_buffer_addresses() asks for before trying again. And a skull is no longer accepted as a downsample source. _init_8() fills a size from the first larger size present in RAM and tested only for a non-NULL buffer -- which a failed entry has: the dead image. The smaller mips were therefore filled from it and marked as successfully generated WITHOUT EVER ATTEMPTING their own generation, which also defeats the retry above for them. One claim in #1474 was wrong and is corrected here rather than quietly dropped. The issue said the propagated skull, "being larger than 8x8, then cleared the 'don't write skulls' guard on eviction and reached the disk cache, where it survived restarts". It does not: dt_iop_flip_and_zoom_8() never upscales, so the copy comes out 8x8 too and the guard holds. Measured on master with the repro below -- `generate mip size 1 for image 1 from mip size 2 (8x8->8x8)`, 2 mips filled from the skull, 0 jpegs written. The harm is the latching of the smaller sizes, not disk contamination. Measured with a staged build, an isolated --configdir/--cachedir/--library, and a copy of a NEF whose contents were replaced with 200 kB of random bytes after import -- which fails generation AFTER the downsample loop, the only way to reach the propagation path. With lighttable/embedded_jpg=0 so that loop is not skipped, `ansel-generate-cache --min-mip 0 --max-mip 2`: this branch master could not process thumbnail: 3 1 is a skull, not using it: 3 - will try again: 4 - generated from a larger mip: 0 2 jpegs on disk: 0 0 Master attempts ONE generation and fills the two smaller mips from its skull; this branch attempts each of the three honestly and refuses the skull source each time. The retry itself was observed, because "does it re-enter _init_8() or silently no-op on the 8x8" is the whole question and the answer is not visible from the source. ansel-generate-cache asks once per mip, so it was temporarily instrumented to ask twice for the same key (reverted; not part of this commit): [mipmap_cache] image 1 at mip size 2 failed to generate; the next request will try again [harness] second request for image 1 mip 2 [mipmap_cache] could not process thumbnail! <-- _init_8() really ran again [mipmap_cache] image 1 at mip size 2 failed to generate twice; keeping the skull A SECOND "could not process thumbnail!" is the evidence: without the descriptor restore, _init_8() would have returned at its `*width < 16` guard and printed nothing. Not measured: the retry SUCCEEDING, which needs a fault that clears between two requests. The recovery path is reachable in the GUI (a scroll away and back, a resize, a refresh, an eviction) and not from this tool. Ported onto #1495 (eviction rewrite and ON_DISK) and #1595 (embedded-JPEG mode snapshot), against 7426c0c. RETRIED is 1 << 3, since ON_DISK took 1 << 2. The restore comment quotes the current _init_8() guard, which returns DT_IMAGEIO_OK, so without the restore the skull would be kept as a thumbnail that generated rather than fail a second time. The skull check's comment no longer says the propagated skull reached the disk cache. On this stack, a size that failed once keeps GENERATE and is already skipped by the DT_MIPMAP_TESTLOCK get of the downsample loop; the skull check is there for a size that failed twice. Re-measured with the same repro on 7426c0c and on this commit: could not process thumbnail 1 / 3, will try again - / 4, generated from a larger mip 2 / 0, jpegs on disk 0 / 0. The skull check did not fire in this run (0), for the reason above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…r a source _init_8() and _init_f() report a failure as a zero-sized image, and _generate_blocking() then cleared DT_MIPMAP_BUFFER_DSC_FLAG_GENERATE unconditionally. So the failure was latched on the spot: every later request took the "already in cache, no I/O needed" path and got the 8x8 skull back, and dt_view_image_get_surface() reports an 8x8 buffer as DT_VIEW_SURFACE_OK, so the widget committed it and never asked again. One transient fault -- a momentary database lock, a network share reconnecting -- therefore cost an image its thumbnail for the whole session, clearable only through "Purge selected thumbnails from cache", which is what the reporter found by trial and error (#1474). One retry is granted instead, recorded in a new RETRIED flag. Bounded on purpose: an image that is genuinely unreadable must not re-run a full pipeline on every redraw, so it costs at most two attempts per cache entry. Read with GENERATE the flag gives the three states a failed thumbnail can be in -- GENERATE|RETRIED is "failed once, try again", RETRIED alone is "failed twice, we have stopped", neither is a thumbnail that generated normally -- and `-d cache` now says which. The retry needs the descriptor put back before it runs, and this is the part that is easy to get wrong: the failed attempt left the skull's 8x8 in dsc->width/height, and _init_8() opens with `if(size >= DT_MIPMAP_F || *width < 16 || *height < 16) return;`. Re-entering with those dimensions does nothing at all, silently, so the retry would have been a guaranteed second failure and the flag would have bought nothing. _generate_blocking() restores what dt_mipmap_cache_update_buffer_addresses() asks for before trying again. And a skull is no longer accepted as a downsample source. _init_8() fills a size from the first larger size present in RAM and tested only for a non-NULL buffer -- which a failed entry has: the dead image. The smaller mips were therefore filled from it and marked as successfully generated WITHOUT EVER ATTEMPTING their own generation, which also defeats the retry above for them. One claim in #1474 was wrong and is corrected here rather than quietly dropped. The issue said the propagated skull, "being larger than 8x8, then cleared the 'don't write skulls' guard on eviction and reached the disk cache, where it survived restarts". It does not: dt_iop_flip_and_zoom_8() never upscales, so the copy comes out 8x8 too and the guard holds. Measured on master with the repro below -- `generate mip size 1 for image 1 from mip size 2 (8x8->8x8)`, 2 mips filled from the skull, 0 jpegs written. The harm is the latching of the smaller sizes, not disk contamination. Measured with a staged build, an isolated --configdir/--cachedir/--library, and a copy of a NEF whose contents were replaced with 200 kB of random bytes after import -- which fails generation AFTER the downsample loop, the only way to reach the propagation path. With lighttable/embedded_jpg=0 so that loop is not skipped, `ansel-generate-cache --min-mip 0 --max-mip 2`: this branch master could not process thumbnail: 3 1 is a skull, not using it: 3 - will try again: 4 - generated from a larger mip: 0 2 jpegs on disk: 0 0 Master attempts ONE generation and fills the two smaller mips from its skull; this branch attempts each of the three honestly and refuses the skull source each time. The retry itself was observed, because "does it re-enter _init_8() or silently no-op on the 8x8" is the whole question and the answer is not visible from the source. ansel-generate-cache asks once per mip, so it was temporarily instrumented to ask twice for the same key (reverted; not part of this commit): [mipmap_cache] image 1 at mip size 2 failed to generate; the next request will try again [harness] second request for image 1 mip 2 [mipmap_cache] could not process thumbnail! <-- _init_8() really ran again [mipmap_cache] image 1 at mip size 2 failed to generate twice; keeping the skull A SECOND "could not process thumbnail!" is the evidence: without the descriptor restore, _init_8() would have returned at its `*width < 16` guard and printed nothing. Not measured: the retry SUCCEEDING, which needs a fault that clears between two requests. The recovery path is reachable in the GUI (a scroll away and back, a resize, a refresh, an eviction) and not from this tool. Ported onto #1495 (eviction rewrite and ON_DISK) and #1595 (embedded-JPEG mode snapshot, with the decision back in _write_mipmap_to_disk()), against 1cf2a67. RETRIED is 1 << 3, since ON_DISK took 1 << 2. The restore comment quotes the current _init_8() guard, which returns DT_IMAGEIO_OK, so without the restore the skull would be kept as a thumbnail that generated rather than fail a second time. The skull check's comment no longer says the propagated skull reached the disk cache. On this stack, a size that failed once keeps GENERATE and is already skipped by the DT_MIPMAP_TESTLOCK get of the downsample loop; the skull check is there for a size that failed twice. Re-measured with the same repro on 1cf2a67 and on this commit: could not process thumbnail 1 / 3, will try again - / 4, generated from a larger mip 2 / 0, jpegs on disk 0 / 0. The skull check did not fire in this run (0), for the reason above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…an unedited one in modes 1 and 2 The previous commit moved `mode == 2 || (mode == 1 && !altered)` into _init_8(), whose cognitive complexity then went from 24 to 26, over the 25 allowed. The same decision written on `altered` first, `altered ? mode == 2 : mode >= 1`, costs one point instead of two, and reads as what the three modes mean. The three flags _init_8() receives from _write_mipmap_to_disk() are initialised to FALSE. No behaviour change: for modes 0, 1 and 2 both expressions give the same result, edited or not. The signature of _write_mipmap_to_disk() is left as is, so this branch still merges independently of #1495, which moves one of its calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…r a source _init_8() and _init_f() report a failure as a zero-sized image, and _generate_blocking() then cleared DT_MIPMAP_BUFFER_DSC_FLAG_GENERATE unconditionally. So the failure was latched on the spot: every later request took the "already in cache, no I/O needed" path and got the 8x8 skull back, and dt_view_image_get_surface() reports an 8x8 buffer as DT_VIEW_SURFACE_OK, so the widget committed it and never asked again. One transient fault -- a momentary database lock, a network share reconnecting -- therefore cost an image its thumbnail for the whole session, clearable only through "Purge selected thumbnails from cache", which is what the reporter found by trial and error (#1474). One retry is granted instead, recorded in a new RETRIED flag. Bounded on purpose: an image that is genuinely unreadable must not re-run a full pipeline on every redraw, so it costs at most two attempts per cache entry. Read with GENERATE the flag gives the three states a failed thumbnail can be in -- GENERATE|RETRIED is "failed once, try again", RETRIED alone is "failed twice, we have stopped", neither is a thumbnail that generated normally -- and `-d cache` now says which. The retry needs the descriptor put back before it runs, and this is the part that is easy to get wrong: the failed attempt left the skull's 8x8 in dsc->width/height, and _init_8() opens with `if(size >= DT_MIPMAP_F || *width < 16 || *height < 16) return;`. Re-entering with those dimensions does nothing at all, silently, so the retry would have been a guaranteed second failure and the flag would have bought nothing. _generate_blocking() restores what dt_mipmap_cache_update_buffer_addresses() asks for before trying again. And a skull is no longer accepted as a downsample source. _init_8() fills a size from the first larger size present in RAM and tested only for a non-NULL buffer -- which a failed entry has: the dead image. The smaller mips were therefore filled from it and marked as successfully generated WITHOUT EVER ATTEMPTING their own generation, which also defeats the retry above for them. One claim in #1474 was wrong and is corrected here rather than quietly dropped. The issue said the propagated skull, "being larger than 8x8, then cleared the 'don't write skulls' guard on eviction and reached the disk cache, where it survived restarts". It does not: dt_iop_flip_and_zoom_8() never upscales, so the copy comes out 8x8 too and the guard holds. Measured on master with the repro below -- `generate mip size 1 for image 1 from mip size 2 (8x8->8x8)`, 2 mips filled from the skull, 0 jpegs written. The harm is the latching of the smaller sizes, not disk contamination. Measured with a staged build, an isolated --configdir/--cachedir/--library, and a copy of a NEF whose contents were replaced with 200 kB of random bytes after import -- which fails generation AFTER the downsample loop, the only way to reach the propagation path. With lighttable/embedded_jpg=0 so that loop is not skipped, `ansel-generate-cache --min-mip 0 --max-mip 2`: this branch master could not process thumbnail: 3 1 is a skull, not using it: 3 - will try again: 4 - generated from a larger mip: 0 2 jpegs on disk: 0 0 Master attempts ONE generation and fills the two smaller mips from its skull; this branch attempts each of the three honestly and refuses the skull source each time. The retry itself was observed, because "does it re-enter _init_8() or silently no-op on the 8x8" is the whole question and the answer is not visible from the source. ansel-generate-cache asks once per mip, so it was temporarily instrumented to ask twice for the same key (reverted; not part of this commit): [mipmap_cache] image 1 at mip size 2 failed to generate; the next request will try again [harness] second request for image 1 mip 2 [mipmap_cache] could not process thumbnail! <-- _init_8() really ran again [mipmap_cache] image 1 at mip size 2 failed to generate twice; keeping the skull A SECOND "could not process thumbnail!" is the evidence: without the descriptor restore, _init_8() would have returned at its `*width < 16` guard and printed nothing. Not measured: the retry SUCCEEDING, which needs a fault that clears between two requests. The recovery path is reachable in the GUI (a scroll away and back, a resize, a refresh, an eviction) and not from this tool. Ported onto #1495 (eviction rewrite and ON_DISK) and #1595 (embedded-JPEG mode snapshot), against 2d3162d. RETRIED is 1 << 3, since ON_DISK took 1 << 2. The restore comment quotes the current _init_8() guard, which returns DT_IMAGEIO_OK, so without the restore the skull would be kept as a thumbnail that generated rather than fail a second time. The skull check's comment no longer says the propagated skull reached the disk cache. On this stack, a size that failed once keeps GENERATE and is already skipped by the DT_MIPMAP_TESTLOCK get of the downsample loop; the skull check is there for a size that failed twice. Re-measured with the same repro on 2d3162d and on this commit: could not process thumbnail 1 / 3, will try again - / 4, generated from a larger mip 2 / 0, jpegs on disk 0 / 0. The skull check did not fire in this run (0), for the reason above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…an unedited one in modes 1 and 2 The previous commit moved `mode == 2 || (mode == 1 && !altered)` into _init_8(), whose cognitive complexity then went from 24 to 26, over the 25 allowed. The same decision written on `altered` first, `altered ? mode == 2 : mode >= 1`, costs one point instead of two, and reads as what the three modes mean. The three flags _init_8() receives from _write_mipmap_to_disk() are initialised to FALSE. No behaviour change: for modes 0, 1 and 2 both expressions give the same result, edited or not. The signature of _write_mipmap_to_disk() is left as is, so this branch still merges independently of #1495, which moves one of its calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…r a source _init_8() and _init_f() report a failure as a zero-sized image, and _generate_blocking() then cleared DT_MIPMAP_BUFFER_DSC_FLAG_GENERATE unconditionally. So the failure was latched on the spot: every later request took the "already in cache, no I/O needed" path and got the 8x8 skull back, and dt_view_image_get_surface() reports an 8x8 buffer as DT_VIEW_SURFACE_OK, so the widget committed it and never asked again. One transient fault -- a momentary database lock, a network share reconnecting -- therefore cost an image its thumbnail for the whole session, clearable only through "Purge selected thumbnails from cache", which is what the reporter found by trial and error (#1474). One retry is granted instead, recorded in a new RETRIED flag. Bounded on purpose: an image that is genuinely unreadable must not re-run a full pipeline on every redraw, so it costs at most two attempts per cache entry. Read with GENERATE the flag gives the three states a failed thumbnail can be in -- GENERATE|RETRIED is "failed once, try again", RETRIED alone is "failed twice, we have stopped", neither is a thumbnail that generated normally -- and `-d cache` now says which. The retry needs the descriptor put back before it runs, and this is the part that is easy to get wrong: the failed attempt left the skull's 8x8 in dsc->width/height, and _init_8() opens with `if(size >= DT_MIPMAP_F || *width < 16 || *height < 16) return;`. Re-entering with those dimensions does nothing at all, silently, so the retry would have been a guaranteed second failure and the flag would have bought nothing. _generate_blocking() restores what dt_mipmap_cache_update_buffer_addresses() asks for before trying again. And a skull is no longer accepted as a downsample source. _init_8() fills a size from the first larger size present in RAM and tested only for a non-NULL buffer -- which a failed entry has: the dead image. The smaller mips were therefore filled from it and marked as successfully generated WITHOUT EVER ATTEMPTING their own generation, which also defeats the retry above for them. One claim in #1474 was wrong and is corrected here rather than quietly dropped. The issue said the propagated skull, "being larger than 8x8, then cleared the 'don't write skulls' guard on eviction and reached the disk cache, where it survived restarts". It does not: dt_iop_flip_and_zoom_8() never upscales, so the copy comes out 8x8 too and the guard holds. Measured on master with the repro below -- `generate mip size 1 for image 1 from mip size 2 (8x8->8x8)`, 2 mips filled from the skull, 0 jpegs written. The harm is the latching of the smaller sizes, not disk contamination. Measured with a staged build, an isolated --configdir/--cachedir/--library, and a copy of a NEF whose contents were replaced with 200 kB of random bytes after import -- which fails generation AFTER the downsample loop, the only way to reach the propagation path. With lighttable/embedded_jpg=0 so that loop is not skipped, `ansel-generate-cache --min-mip 0 --max-mip 2`: this branch master could not process thumbnail: 3 1 is a skull, not using it: 3 - will try again: 4 - generated from a larger mip: 0 2 jpegs on disk: 0 0 Master attempts ONE generation and fills the two smaller mips from its skull; this branch attempts each of the three honestly and refuses the skull source each time. The retry itself was observed, because "does it re-enter _init_8() or silently no-op on the 8x8" is the whole question and the answer is not visible from the source. ansel-generate-cache asks once per mip, so it was temporarily instrumented to ask twice for the same key (reverted; not part of this commit): [mipmap_cache] image 1 at mip size 2 failed to generate; the next request will try again [harness] second request for image 1 mip 2 [mipmap_cache] could not process thumbnail! <-- _init_8() really ran again [mipmap_cache] image 1 at mip size 2 failed to generate twice; keeping the skull A SECOND "could not process thumbnail!" is the evidence: without the descriptor restore, _init_8() would have returned at its `*width < 16` guard and printed nothing. Not measured: the retry SUCCEEDING, which needs a fault that clears between two requests. The recovery path is reachable in the GUI (a scroll away and back, a resize, a refresh, an eviction) and not from this tool. Rebased onto master 8f8b83d, after the eviction rewrite and ON_DISK (#1495) and the embedded-JPEG mode snapshot (#1595). RETRIED is 1 << 3, since ON_DISK took 1 << 2. The restore comment quotes the current _init_8() guard, which returns DT_IMAGEIO_OK, so without the restore the skull would be kept as a thumbnail that generated rather than fail a second time. The skull check's comment no longer says the propagated skull reached the disk cache. On this master, a size that failed once keeps GENERATE and is already skipped by the DT_MIPMAP_TESTLOCK get of the downsample loop; the skull check is there for a size that failed twice. Re-measured with the same repro on 8f8b83d and on this commit: could not process thumbnail 1 / 3, will try again - / 4, generated from a larger mip 2 / 0, jpegs on disk 0 / 0. The skull check did not fire in this run (0), for the reason above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-project-automation Bot added this to Ansel development management Sep 30, 2026

aurelienpierre and others added 2 commits October 23, 2026 15:02

Jiyone force-pushed the mipmap-disk-write-path branch from a6d9197 to 2d147f7 Compare October 23, 2026 13:34

Jiyone self-assigned this Oct 23, 2026

This was referenced Oct 23, 2026

[Task] mipmap disk cache: a thumbnail read from its file is re-encoded and written back at every session #1589

Open

mipmap cache: a thumbnail removed with flush_disk is not encoded back to disk #1597

Closed

Jiyone mentioned this pull request Oct 25, 2026

lighttable: the thumbnail mode menu tells the mipmap cache, not only conf #1595

Merged

Jiyone merged commit 8f8b83d into master Oct 25, 2026
22 checks passed

github-project-automation Bot moved this to Done in Ansel development management Oct 25, 2026

mipmap cache: the eviction write is about the entry's state, and it lets go of a file before unlinking it by aurelienpierre · Pull Request #1495 · aurelienpierreeng/ansel · GitHub

Repository navigation

mipmap cache: the eviction write is about the entry's state, and it lets go of a file before unlinking it - #1495

mipmap cache: the eviction write is about the entry's state, and it lets go of a file before unlinking it#1495
Jiyone merged 5 commits into
masterfrom
mipmap-disk-write-path

aurelienpierre commented Sep 30, 2026 •
edited by Jiyone
Loading

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026

Uh oh!

Jiyone commented Oct 23, 2026

Uh oh!

Jiyone commented Oct 23, 2026

Uh oh!

Jiyone commented Oct 25, 2026

Uh oh!

sonarqubecloud Bot commented Oct 25, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mipmap cache: the eviction write is about the entry's state, and it lets go of a file before unlinking it by aurelienpierre · Pull Request #1495 · aurelienpierreeng/ansel · GitHub

Repository navigation

mipmap cache: the eviction write is about the entry's state, and it lets go of a file before unlinking it - #1495

mipmap cache: the eviction write is about the entry's state, and it lets go of a file before unlinking it#1495Jiyone merged 5 commits intomasteraurelienpierreeng/ansel:masterfrom mipmap-disk-write-pathaurelienpierreeng/ansel:mipmap-disk-write-pathCopy head branch name to clipboard

Conversation

aurelienpierre commented Sep 30, 2026 • edited by Jiyone Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Four defects, one function

What was measured

What is not measured, and what I did about it

One thing noticed in passing, not fixed here

Uh oh!

chatgpt-codex-connector Bot commented Sep 30, 2026

Uh oh!

Jiyone commented Oct 23, 2026

Two corrections to the description above

2d147f7eda: comments, doc, formats

29bfc796f7: an aborted entry is not there for a TESTLOCK get either

Re-measured on the rebased branch

Noticed in passing, not fixed here

Uh oh!

Jiyone commented Oct 23, 2026

Which renders abort

Measured

Uh oh!

Jiyone commented Oct 25, 2026

Uh oh!

sonarqubecloud Bot commented Oct 25, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mipmap cache: the eviction write is about the entry's state, and it lets go of a file before unlinking it#1495
Jiyone merged 5 commits into
masterfrom
mipmap-disk-write-path

aurelienpierre commented Sep 30, 2026 •
edited by Jiyone
Loading