Skip to content

Film strip: two-column negative stats, stale-thumbnail indicator - #1115

Draft
PabloRuizCuevas wants to merge 3 commits into
marcinz606:mainfrom
PabloRuizCuevas:feat/pr-film-strip-stats-polish
Draft

PabloRuizCuevas wants to merge 3 commits into
marcinz606:mainfrom
PabloRuizCuevas:feat/pr-film-strip-stats-polish

Conversation

@PabloRuizCuevas

Copy link
Copy Markdown
Contributor

Summary

  • Negative stats pack two per row: the negative-stats readout uses a two-column layout instead of one long stacked list.
  • Stale-thumbnail indicator: a bulk apply (Apply Settings, a preset, a roll-wide toggle) writes a non-active file's history without rendering it, so the film strip flags that thumbnail — an amber dot on the cell, a tooltip note — until a real render catches up. Discarded once that frame actually renders, or its thumbnail is dropped on unload.

Bug fix included

The indicator as originally written never actually showed: the write side (push_external_history) flags a frame under asset_thumbnail_key(asset) — the same hash-plus-cache-version key used everywhere else in the thumbnail cache, since a trichrome triplet's key differs from its plain hash — but the two read sites (the film strip's dot, its tooltip) checked the bare hash instead, which never matched. Fixed both read sites to key the same way, and added a regression test that exercises the real push_external_history → read path end to end (the original test suite had accidentally masked this by seeding its fixture directly with the bare hash, bypassing asset_thumbnail_key entirely).

Testing

make all (ruff check/format, full pytest) passes with only one known unrelated failure (test_load_tooltip_follows_a_rebinding, a macOS key-symbol rendering difference, pre-existing on main).

🤖 Generated with Claude Code

PabloRuizCuevas and others added 3 commits September 17, 2026 16:50
NegativeStatsWidget laid out one stat per grid row (name|value), which
claimed more vertical height than the right sidebar can spare. Pair
consecutive StatRow entries two at a time (name|value|name|value)
across a 5-column grid, with a thin THEME-colored vertical divider
between the two halves.

Per-stat tooltips, warn/error value styling and the hide-rather-than-
blank behavior for unused slots stay independent per stat; the divider
for a pair hides too when its second slot is empty (an odd row count).
_names/_values/_ROWS keep their existing flat, row-ordered semantics,
so tests/test_repair_coverage.py's two NegativeStatsWidget tests pass
unmodified.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A bulk apply (roll bake, apply-to-roll, apply-to-selection) writes a new
config straight to a non-active file's history (push_external_history) --
no render runs for it, so its cached thumbnail keeps showing the old look
until something else regenerates it. Nothing on the sheet said so.

AppState.stale_thumbnails tracks which cached bitmaps are in that state,
set in push_external_history and cleared wherever a thumbnail actually gets
rebuilt or the session resets. The contact-sheet delegate marks a stale
thumbnail with a small amber dot, top-left -- the one badge corner not
already in use -- with a tooltip line explaining it and pointing at the
fix (open the frame).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… never matches

push_external_history adds asset_thumbnail_key(asset) to stale_thumbnails --
the same hash-plus-cache-version key the add/discard sides everywhere else
in the thumbnail cache use, since a triplet's key differs from its plain
hash. The film strip's stale dot and its tooltip instead checked the bare
hash, so the flag, though correctly set, never matched anything: a bulk
apply (Apply Settings, a preset, a roll-wide toggle) never showed the dot
on a frame it didn't render.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@PabloRuizCuevas
PabloRuizCuevas force-pushed the feat/pr-film-strip-stats-polish branch from c7947f4 to 44b5b94 Compare September 17, 2026 09:57
PabloRuizCuevas added a commit to PabloRuizCuevas/NegPy that referenced this pull request Sep 17, 2026
Ports the stale-thumbnail mechanism from marcinz606#1115 as a prerequisite:
push_external_history (a bulk apply reaching a non-active file) marks
its asset_thumbnail_key stale; the film strip draws a dot and the
tooltip names it until a render clears the flag. Roll-wide defaults
reuse this to show propagation without a batch apply.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PabloRuizCuevas added a commit to PabloRuizCuevas/NegPy that referenced this pull request Sep 17, 2026
…e dot

Completes the marcinz606#1115 port: the delegate-level tests for the amber dot,
the dirty-marker fixtures now carrying stale_thumbnails so painting
doesn't AttributeError, and the corner-badge table's new row.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@thetalkingdrum

Copy link
Copy Markdown
Contributor

Curious how this interacts with #1127. That PR wires frames_edited_offscreen into each bulk-write path (Apply Settings, Apply Preset, Batch Analysis, …) one at a time, and #1118 needs the same treatment for its new reset_roll_settings. If this indicator flags staleness at push_external_history instead, that's a single choke point every bulk writer already goes through — which could catch all of those cases (including future ones) without per-call-site wiring.

Is that the intent here, i.e. should #1127 defer to this instead of adding its own signal? Want to figure out the right order before #1127 gets rebased on top of #1118 and does the wiring by hand.

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