feat(assets): per-file half-frame split, auto-detect or manual override - #1065
Merged
marcinz606 merged 8 commits intoSep 12, 2026
Merged
Conversation
Half Frame's crop/split/gutter has been one shared profile for the whole roll since it shipped: set once in the rectangle editor, then applied identically to every scan, whatever their real gutter position. A roll with a bit of irregular frame spacing gets some frames' splits wrong with no way to fix them short of abandoning the shared profile. The per-file pieces already existed and were simply unused: each expanded half already carries its own split_x/crop_rect/gutter_thickness, and detect_split_x_for_file already finds a scan's actual gutter -- it just stopped running the moment a profile was saved. Two independent, backward-compatible additions (existing behaviour is unchanged when neither is used): - Auto-detect split per frame: a checkbox in the roll-wide editor. On, each scan's own gutter wins over the fixed split_x; crop_rect/gutter_thickness stay the profile's (they describe the scanner rig, not the film). A frame whose gutter can't be found falls back to the profile's tuned value rather than a blind re-center. - Per-frame override: right-click a half-frame asset -> "Adjust split for this frame..." opens the same editor scoped to that file alone, saved separately from the roll-wide profile. "Reset split to roll default" clears it. Wins over both the profile and auto-detection, for the odd frame that still gets it wrong. Resolution order in _expand_half_frames: per-file override > per-file auto-detect (if the profile's auto_split is on, or no profile exists yet) > the profile's fixed split_x. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…split Heal strokes, dust spots, scratch lines and dodge/burn masks are stored in half-local normalized coordinates. Changing a file's split or crop redefines that local space, so anything painted before the change was left pointing at the wrong spot after it -- a heal circle drifting into the sky is the visible form. remap_point/remap_workspace_config re-anchor a point through full-scan space (the crop/split's old geometry -> full scan -> new geometry), so every position-based manual edit stays on the same physical film location. Wired into every path that changes a file's effective geometry: the per-frame editor, the roll-wide profile, and a new batch auto-detect. Also replaces the "Auto-detect split per frame" checkbox with the roll-wide editor's own Apply dropdown -- current frame / selected frames / all frames -- matching the Export button's split-button pattern, since the live per-file auto-detect toggle it replaces never had a way to fix the one frame it got wrong without turning it off for the whole roll. Auto-detect all splits batches the per-file gutter search directly into overrides instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
auto_detect_all_half_frame_splits ran detect_split_x_for_file (a decode plus a gutter scan) synchronously on the GUI thread for every loaded frame, with no feedback, so a roll of any size froze the UI for the whole batch. Moved it onto the asset-discovery worker's own thread via a new AutoDetectAllSplitsTask, reusing _map_files' thread pool and the existing progress -> status bar wiring, so it now reports a running count like every other batch operation. The controller applies the results (save + remap) once the worker reports back. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tton The current/selected/all scope picker was a menu on the toolbar's Adjust button, opened before the split & crop editor -- so it had to be decided before seeing the frame, and it didn't compose with the editor's own Auto-detect (clicking Auto-detect there only ever previewed the current scan's gutter; the menu had already fixed what Apply would do with it). Moved to a split-button Apply inside the dialog itself, mirroring Export's exact pattern: a main Apply action plus a ▾ that picks the scope, chosen after drawing the rectangle and running Auto-detect, not before. The choice is remembered as the next default (half_frame_apply_scope), except the per-frame context menu, which always starts at "current". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…profile _diptych_task rebuilt a whole-frame diptych's crop/split from the roll-wide profile alone. A file with its own per-file override (set via "Adjust split for this frame...") was cut into its two halves with that override, but turning Half Frame off reconstructed the diptych with the profile's geometry instead -- a mismatch that misaligns the decode each half's saved edit expects, visible as a badly broken render. Reuses _half_frame_geometry_for, the same override > profile > auto-detect resolution already used everywhere else, so this is also the first time the "no profile was ever saved" case reconstructs a diptych at all (previously silent 0.5/no-crop). Also adds an Unsplit diptych button beside Half Frame, enabled only for the active frame's diptych state -- the right-click context menu was the only way in, easy to miss when the panel just looks locked with no clue why. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
remap_workspace_config re-anchored heal strokes, dust spots, scratch lines and masks across a split/crop change, but left the regular Crop tool's geometry.crop_rect untouched with a note that it can't be remapped (it lives in post-rotation transformed-image space, not raw space). Untouched meant stale: a crop drawn against the old frame boundary keeps applying after the boundary moves, silently clipping the wrong region -- confusing on its own, and worse layered on top of a half-frame crop that looks generous enough. Clears it instead of remapping it. Only the crop_rect field: crop_from_auto and crop_detect_key are left alone, so an auto-detected crop re-arms and re-detects against the new boundary on the next render rather than vanishing, while a manual one drops back to none rather than staying wrong. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… own _current_file() matched the active file by path, then returned that entry's own hash verbatim. Both halves of a half-frame asset share one path, so the match always landed on whichever half _expand_half_frames lists first (half 1) and returned its marcinz606#1/marcinz606#2-suffixed hash -- never the base hash save_half_frame_override and half_frame_overrides() key by. Every Apply from the toolbar's Adjust Half Frame button (current, selected or all) saved its override under that wrong, half-suffixed key. The next discovery's overrides.get(base_hash(a["hash"])) lookup never found it, so the edit silently never took effect -- indistinguishable from Apply doing nothing. The right-click "Adjust split for this frame..." path was already correct, since it computes base_hash itself before calling in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
marcinz606
reviewed
Sep 10, 2026
| ) | ||
| self.half_frame_adjust_btn.clicked.connect(self._on_half_frame_adjust) | ||
|
|
||
| self.half_frame_auto_all_btn = QToolButton() |
Owner
There was a problem hiding this comment.
I don't like so many half-frame specific buttons put in here, maybe instead have one half-frame specific button that brings up the menu from which you can select all the related options?
Contributor
Author
There was a problem hiding this comment.
Fair — collapsed Adjust split, Auto-detect all splits and Unsplit diptych into one button with a menu (dfbe763), so it's back to one half-frame-specific icon plus the mode toggle.
Adjust split, Auto-detect all splits and Unsplit diptych each got their own toolbar icon as they were added, ending up as three buttons alongside the Half Frame mode toggle. Collapsed into one button with a menu, synced on aboutToShow so Unsplit diptych enables for whichever frame is active when it actually opens. Addresses review feedback on marcinz606#1065. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
|
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
2 participants
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.
Problem
Half Frame's crop/split/gutter has been one shared profile for the whole roll since it shipped: set once in the rectangle editor, then applied identically to every scan regardless of that scan's actual gutter position. A roll with a bit of irregular frame spacing gets some frames' splits wrong, with no way to fix them short of abandoning the shared profile entirely.
The per-file pieces already existed and were simply unused: each expanded half already carries its own
split_x/crop_rect/gutter_thickness, anddetect_split_x_for_filealready finds a scan's real gutter — it just stops running the moment a profile is saved (_expand_half_framesonly calls it whenprofile is None).Fix — two independent, backward-compatible additions
Existing behavior is unchanged when neither is used (no profile → still auto-detects everything, same as today; a profile with the new option left off → same fixed split for every scan, same as today).
1. Auto-detect split per frame — a checkbox in the roll-wide rectangle editor. On, each scan's own gutter wins over the fixed
split_x;crop_rect/gutter_thicknessstill come from the shared profile (they describe the scanner rig, not the film). A frame whose gutter can't be found (detect_split_x's own 0.5 "nothing found" sentinel) falls back to the profile's tuned value rather than a blind re-center.2. Per-frame override — right-click a half-frame asset → "Adjust split for this frame…" opens the same editor scoped to just that file, saved separately from the roll-wide profile (the roll-wide auto-detect checkbox is hidden — meaningless for a single fixed frame). "Reset split to roll default" appears once an override exists, to drop back to the shared setting. Wins over both the profile and auto-detection, for the odd frame that still gets it wrong even with #1 on.
Resolution order in
_expand_half_frames: per-file override > per-file auto-detect (profile'sauto_spliton, or no profile saved yet) > the profile's fixedsplit_x.Tests
tests/test_half_frame.py— new cases for auto_split on/off and override precedence, alongside the existing_expand_half_framescoveragetests/test_half_frame_dialog.py— new file: the auto-split checkbox shown/hidden/checked correctly, dialog titletests/test_controller.py— profile/override storage round-trips,open_half_frame_dialog's per-frame vs roll-wide branchingtests/test_file_browser_widget.py— context menu shows the new actions only when relevantdocs/USER_GUIDE.mdupdated (Half Frame section).Full suite: 5457 passed (up from 5438), one pre-existing unrelated failure (macOS Qt tooltip-rendering quirk, present identically on
main).🤖 Generated with Claude Code