Skip to content

feat(thumbnails): background thumbnail refresh + manual Update Thumbnails control - #1127

Merged
marcinz606 merged 7 commits into
marcinz606:mainfrom
thetalkingdrum:feature/background-thumbnail-rerender
Sep 19, 2026
Merged

marcinz606 merged 7 commits into
marcinz606:mainfrom
thetalkingdrum:feature/background-thumbnail-rerender

Conversation

@thetalkingdrum

@thetalkingdrum thetalkingdrum commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Applying settings, a preset, a scan-setup change, or Batch Analysis to more than the
current frame writes straight to the DB for every other touched frame — but nothing
refreshed those frames' filmstrip thumbnails short of opening them by hand.

  • A background pass re-renders and persists each touched frame's thumbnail without
    opening it.
  • It never blocks Export, Stitch, HDR, or anything else — no progress dialog pops, no
    batch is refused because of it.
  • Auto Crop All and Batch Analysis do share its render thread, so they pre-empt it
    immediately (within about one frame) instead of waiting it out — the frames that got
    cut short are automatically retried once they're done, with a status message so it's
    not silent.
  • Diptych/half-frame rows are intentionally left out of scope for now: a whole-frame
    render can't reproduce how their canvas joins two half configs.
  • A manual Update Thumbnails toolbar button (whole roll) and context-menu entry
    (current selection) let you force this on demand, for staleness an automatic trigger
    missed or predates one. The same control turns into Cancel while it's running —
    a very large accidentally-opened folder needs a way out, and unlike a real batch's
    pre-emption, cancelling discards the backlog outright instead of resuming it.

How it works

  • desktop/workers/render.py: new ThumbnailRenderWorker, its own CPU-only
    ImageProcessor and decode cache, on the existing norm_thread.
  • desktop/session.py / desktop/controller.py: a frames_edited_offscreen signal
    fired by every bulk-write path (Apply Settings, Apply Preset, scan setup, Batch
    Analysis, Apply Normalization Roll), dispatching refresh_thumbnails_for.
  • _begin_batch pre-empts a running refresh only for the two batch types that actually
    share its thread (Auto Crop All, Batch Analysis) — everything else runs alongside it
    untouched.
  • desktop/view/sidebar/files.py: the manual toolbar button and context-menu entry,
    routed through new request_thumbnail_refresh/cancel_thumbnail_refresh controller
    methods — same refresh_thumbnails_for plumbing underneath, no new backend path.

Heads up on overlapping open PRs

Test plan

  • make lint / make type clean
  • New unit tests for the worker, the controller wiring, every bulk-write trigger's
    signal emit, the manual control's dispatch/cancel behavior, and the real widget
    wiring (toolbar button, context menu)
  • Full suite green (one pre-existing, unrelated Norwegian-keyboard tooltip failure,
    also fails on main)
  • Manually verified in the app: Apply Settings to roll, Apply Preset to roll,
    exporting a frame mid-refresh, Batch Analysis pre-empting and resuming, opening a
    different non-active frame mid-refresh, a forced-Slide-mode frame's color
    surviving the background render unchanged, the manual Update Thumbnails button
    and context-menu entry, and cancelling a roll-wide run mid-flight

@thetalkingdrum thetalkingdrum changed the title feat(thumbnails): re-render bulk-edited frames' thumbnails in the background feat(thumbnails): background thumbnail refresh + manual Update Thumbnails control Sep 17, 2026
@thetalkingdrum
thetalkingdrum marked this pull request as ready for review September 17, 2026 16:44
@thetalkingdrum

Copy link
Copy Markdown
Contributor Author

Heads up: merge #1118 before this one. It touches the same hunk in session.py (reset_roll_settings lands right where this PR's frames_edited_offscreen.emit(...) goes, at the tail of apply_preset_fields) and the same docs/USER_GUIDE.md right-click-menu sentence.

When this rebases past #1118, fold reset_roll_settings into the wiring list here too. #1118 flagged it as a known limitation — a roll/selection reset leaves a stale thumbnail on non-active frames, the same gap Apply Settings/Apply Preset had before this PR — but the reset path itself isn't wired to emit the signal yet.

thetalkingdrum and others added 6 commits September 19, 2026 12:34
…kground

Reset/Apply Settings/Apply Preset and the scan-setup wizard write a new
WorkspaceConfig straight to the DB for every non-active target frame, but
nothing refreshed that frame's filmstrip thumbnail short of opening it.

A new ThumbnailRenderWorker, on its own CPU-only ImageProcessor and off the
shared batch lane, re-renders each touched frame and persists its thumbnail
without blocking Export or any other real batch. A batch that actually
shares norm_thread with it (Auto Crop All, roll analysis) pre-empts it
within one frame's processing time instead of waiting out the whole roll.
Auto Crop All and roll analysis correctly cut a running thumbnail refresh
short to get norm_thread back, but nothing retried the frames it left
behind — they stayed stale until the next unrelated bulk edit touched them,
silently. Track which frames a generation didn't get to, fold them into a
resume set on cancellation, and re-request them the moment norm_thread is
free again; Qt's own queuing keeps this from ever racing the real batch
that pre-empted it. Also surfaces a status message when this happens
instead of stopping without a trace.
_on_normalization_finished and apply_normalization_roll write a new
roll-wide luma/color baseline to every loaded frame's saved settings, the
same shape of bulk write Apply Settings/Preset and scan setup already
trigger a background thumbnail refresh from — but neither emitted
frames_edited_offscreen, so every non-active frame's thumbnail went stale
after a batch analysis and stayed that way.
…wire Auto Crop All

A bulk write arriving while a resume generation was already using
norm_thread (most notably Batch Analysis's own completion write, during
the window its own pre-emption opened) was silently dropped instead of
retried, reintroducing the exact staleness this feature exists to fix.
Fold it into the resume backlog instead.

Auto Crop All's own completion handler writes a new crop/rotation to
every non-active frame but never offered them for a thumbnail refresh —
a sixth unwired bulk-write site, the most visible kind since it's a
geometry change. Wired it up the same way as the others.

Also drops a status message that was always overwritten in the same
call stack before it could ever be seen.
A toolbar button (whole roll) and a context-menu entry (current
selection) let the user force the background thumbnail refresh on
demand, for staleness an automatic trigger missed or predates one.
Both reuse refresh_thumbnails_for unchanged.

While one is running, the same control turns into Cancel — a very
large accidentally-opened folder needs a way out. Unlike a real
batch's pre-emption, cancelling discards the backlog outright instead
of resuming it once norm_thread frees up.
…sure

The background thumbnail refresh (marcinz606#1127) grows its own preview cache on
top of whatever the navigation and Auto Crop All caches already hold,
per the coordination note on the marcinz606#1107 thread: it never touches the GPU
texture pool (marcinz606#1121) bounds, since its ImageProcessor is CPU-only, but
its RAM use was uncoordinated with marcinz606#1121's own memory-safety margin.

refresh_thumbnails_for() now checks available system memory against the
same MIN_RAM_RESERVE_BYTES reserve marcinz606#1121's prefetch admission uses
(promoted from prefetch_policy's private constant) before starting a
generation, and retries later instead of proceeding under pressure.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@thetalkingdrum
thetalkingdrum force-pushed the feature/background-thumbnail-rerender branch from 2974e69 to 51af764 Compare September 19, 2026 11:32
@thetalkingdrum

Copy link
Copy Markdown
Contributor Author

Thanks for the note on the #1107 thread about coordinating with #1121's memory work — added in 51af764.

GPU needed no coordination: ThumbnailRenderWorker already uses its own ImageProcessor(use_gpu=False), so it never touches the GPU texture pool #1121 bounds.

RAM was the real gap: this refresh gets its own PreviewManager (following the same pattern the existing Auto Crop All worker already uses), so worst-case combined preview-cache footprint goes from 2x to 3x the fixed per-instance cap — and unlike #1121's prefetch path, nothing here checked memory pressure before starting a generation.

Fix: refresh_thumbnails_for now checks available_system_memory_bytes() against the same MIN_RAM_RESERVE_BYTES reserve #1121's prefetch admission uses (promoted out of prefetch_policy's private constant so both share one number), and defers + retries after 5s instead of starting a refresh under pressure. Falls back to proceeding if the OS can't report available memory, so it doesn't silently stop working on platforms without that introspection.

# Conflicts:
#	docs/USER_GUIDE.md
#	negpy/desktop/view/sidebar/files.py
@marcinz606
marcinz606 merged commit 876dd00 into marcinz606:main Sep 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants