fix(session): let a roll-wide reset undo an unwanted setting (#1047) - #1118
Merged
marcinz606 merged 3 commits intoSep 19, 2026
Merged
Conversation
…606#1047) Once a boolean like flip H/V or Normalize was applied across a roll, there was no way to push the unchecked/default state back out: Apply Settings' picker only pre-selects rows that differ from default, so a field already at its default on every source frame could never be chosen. Adds a roll-scoped "Reset Roll to Defaults..." action that resets every visible frame to its own asset defaults (reusing _asset_defaults, so composites and merges keep what they are), plus closes a catalog gap where Normalize, Use Luma Average and Use Color Average had no SettingRow at all and so were invisible to Apply/Paste/Sticky in either direction.
reset_roll_settings mirrors apply_preset_fields's render=False convention for the active frame, which requires the caller to trigger the render — _reset_roll skipped that, so the canvas kept showing the pre-reset image until the active frame was reselected.
thetalkingdrum
force-pushed
the
fix/reset-roll-to-defaults
branch
from
September 16, 2026 20:49
726fa60 to
37bed39
Compare
thetalkingdrum
marked this pull request as draft
September 17, 2026 16:55
With several frames selected in the file list, the context menu's Reset Settings action now reads Reset N Frames and resets the whole selection instead of only the active frame, reusing the existing reset_roll_settings(scope="selection") and gated behind a confirm dialog like Reset Roll to Defaults. Single-frame Reset Settings is unchanged. Also fixes two findings from reviewing this change: confirm_reset_frames now uses count_of() for correct pluralization instead of a hardcoded "frames", and e6_normalize (Normalize) is added to _BOUNDS_INPUT_FIELDS so applying or pasting it alone invalidates a target E6 frame's cached bounds, matching its sibling rows (Use Luma/Color Average) added in the same PR.
thetalkingdrum
marked this pull request as ready for review
September 17, 2026 17:28
This was referenced Sep 17, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1047: once a boolean setting (flip H/V, Normalize, Use Luma/Color Average) got
applied across a roll, there was no way to push the unchecked state back out. A misclick
meant fixing every frame by hand.
Root cause:
GranularSettingsDialog(behind Apply/Paste/Sticky) pre-selects andsurfaces rows by comparing each field to
WorkspaceConfig()defaults. A field a user setand then reverted to its default is indistinguishable from "never touched," so it's hidden
and unticked by default — there's no live frame left to use as a "pristine" source once
every frame in the roll already holds the unwanted value.
Fix:
which resets every visible frame to its own asset defaults in one step — reusing
_asset_defaults, the same per-asset logicReset Settingsalready uses, so acomposite's inherited film process, a merge's seeded shadow lift, and stitch/triplet
wiring survive the reset rather than being wiped by a blind
WorkspaceConfig()overlay.Each frame's reset is still an ordinary undo step, and the active frame re-renders
immediately.
resets the whole selection instead of only the active frame — the same confirm-gated
reset_roll_settings, just scoped tostate.selected_indicesinstead of the wholevisible roll. Single-frame Reset Settings is unchanged (still unconfirmed, one click).
Normalize(e6_normalize),Use Luma Average, andUse Color Averagehad noSettingRowat all, so they were invisible to Apply/Paste/Sticky in either direction, independent of the reset gap above.
e6_normalizeis alsoa metering input — it decides whether an E-6 frame's highlight point comes from sampling
the image or a fixed offset from the shadow — so it's registered in
_BOUNDS_INPUT_FIELDSalongside its two siblings, or applying it alone would leave atarget frame's cached bounds stale.
What this doesn't fix
The default-diff heuristic behind
GranularSettingsDialog/_row_editedis unchanged: ApplySettings still can't tell "this field is at its default because the user put it there" apart
from "never touched," so a source frame holding an unwanted-but-default value still won't
pre-select or propagate through the normal Apply Settings picker. This PR is a blunt escape
hatch for that case, not a fix to the picker itself. The root fix would track which fields a
user has actually interacted with, independent of the resulting value — no such tracking
exists anywhere in
WorkspaceConfig/session today — which is a bigger redesign than thisissue's reported symptom calls for. Flagging it in case it's worth its own issue.
Known limitation (pre-existing, not introduced here)
A reset frame that isn't the active one keeps a stale film-strip thumbnail until you
select it — selecting it renders it correctly right away, so the underlying setting is
correct, only the thumbnail lags. This is not new: Apply Settings and Apply Preset
have the same gap today, since thumbnails only regenerate as a side effect of actually
rendering a frame, and both write straight to the DB for every frame but the active one.
Worth its own follow-up covering all three actions rather than a partial fix here.
Heads up on overlapping open PRs
Normalizecatalog row (same field, samesticky=True, one lineapart). Real but trivial overlap — whichever of us merges second will hit a one-line
conflict on that row and should just drop the now-redundant addition.
only) via a tab icon's context menu. No code overlap with this PR, but the two "Reset X
to Defaults" names landing around the same time might be worth reconciling for clarity.
it inserts a
frames_edited_offscreen.emit(...)block at the tail ofapply_preset_fields, the exact pointreset_roll_settingsis inserted right after inthis PR — same hunk in
session.py. It also edits the identicaldocs/USER_GUIDE.mdright-click-menu sentence this PR does. Both are trivial to resolve by keeping both
additions; whichever of us merges second does it by hand. Separately: feat(thumbnails): background thumbnail refresh + manual Update Thumbnails control #1127's background
thumbnail refresh wires
frames_edited_offscreeninto Apply Settings/Apply Preset/BatchAnalysis/etc., but not into this PR's
reset_roll_settings— so the "Known limitation"thumbnail-staleness note above stays open for the roll/selection reset paths even after
feat(thumbnails): background thumbnail refresh + manual Update Thumbnails control #1127 lands, unless
reset_roll_settingsis later wired to emit it too (or Film strip: two-column negative stats, stale-thumbnail indicator #1115'sstale-dot approach lands and catches it structurally instead).
Testing
make all(lint + type + full pytest) passes, with the one known pre-existing failure(
test_load_tooltip_follows_a_rebinding, a macOS key-symbol rendering difference — failsthe same way on
mainwith this branch stashed, and CI runs Ubuntu).Also verified end-to-end against the real
FileBrowserwidget and a real SQLite-backedStorageRepository, and manually in the running app: the menu item appears correctlypositioned, the confirm dialog reports the right frame count, the active frame's canvas
updates immediately, and every frame's setting reverts to default with no crash. The
multi-frame-selection path got the same treatment, plus a driver run against real scans
from a live roll confirming a selected-but-inactive frame resets and an unselected frame is
left untouched.