fix(plustek): restore Prescan crop x-mirroring for mirrored sensors - #2
Open
TobbyTravel wants to merge 1 commit into
Open
fix(plustek): restore Prescan crop x-mirroring for mirrored sensors#2TobbyTravel wants to merge 1 commit into
TobbyTravel wants to merge 1 commit into
Conversation
PR marcinz606#958 ("feat(plustek): upgrade to pyopticfilm 1.1.2") accidentally flattened the Prescan crop↔window conversion to a plain clamp, dropping the x-reflection that had been correct since the Plustek backend first shipped (marcinz606#783). pyopticfilm flips the image left–right in assemble() (mirror_x, 8200i SE / 8100 V2), so the Prescan the user frames on is the mirror-corrected image while the hardware crops by sensor x. Passing the display crop straight into TA space produced correct dimensions but an x-displaced ROI — "correct crop size, offset one way". Restore the self-inverse mirror-aware conversion: - params.py: crop_to_scan_window(crop, *, mirror_x) — reflect x to (1-x2, y1, 1-x1, y2) when mirrored, passthrough otherwise. - base.py / plustek_backend.py: re-add prescan_mirror_x capability, wired from model.mirror_x. - prescan_dialog.py: apply the conversion at all three crop↔window points. Tests: x-reflection, passthrough, self-inverse for crop_to_scan_window, and prescan_mirror_x surfaced on 8100 V2 caps. Verified on hardware (OpticFilm 8100 V2): a Prescan crop now lands exactly where drawn.
TobbyTravel
pushed a commit
that referenced
this pull request
Sep 13, 2026
…de (marcinz606#1065) * feat(assets): per-file half-frame split, auto-detect or manual override 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> * fix(assets): keep manual edits anchored across a half-frame recrop/resplit 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> * perf(assets): run auto-detect-all-splits off the GUI thread 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> * fix(ui): move Half Frame's Apply scope onto the editor's own Apply button 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> * fix(assets): use a diptych's own split geometry, not always the roll 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> * fix(assets): clear a frame's crop when its half geometry changes 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> * fix(ui): key Adjust Half Frame's Apply on the base hash, not a half's 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 #1/#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> * refactor(ui): consolidate Half Frame's action buttons into one menu 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> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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 the Plustek Prescan crop being horizontally displaced ("correct crop size, offset one way") on mirror_x devices (OpticFilm 8200i SE / 8100 V2).
Root cause — a regression from marcinz606#958
assemble()(mirror_x) because the sensor reads right-to-left. The Prescan the user frames on is therefore the mirror-corrected image.windowunchanged into TA/hardware space. The hardware crops by sensor x (pre-mirror) while the user drew by display x (post-mirror), so the returned ROI was reflected → right dimensions, displaced x.The mirror-aware conversion existed and was correct since the Plustek backend shipped (marcinz606#783) —
crop_to_scan_window(..., mirror_x=...)with the x-reflection(x1,y1,x2,y2) → (1-x2, y1, 1-x1, y2). PR marcinz606#958 ("feat(plustek): upgrade to pyopticfilm 1.1.2") silently replaced it with a plainclamp_scan_areaat all three call sites and deleted the capability plumbing. This PR restores that behavior.Changes
negpy/infrastructure/scanners/params.py: restorecrop_to_scan_window(crop, *, mirror_x)— self-inverse x-reflection.negpy/infrastructure/scanners/base.py+plustek_backend.py: restoreprescan_mirror_xcapability, wired frommodel.mirror_x.negpy/desktop/view/widgets/prescan_dialog.py: apply the conversion at all three crop↔window points (default-crop display, window change, accept).prescan_mirror_xsurfaced on 8100 V2 caps.Validation
Scope
Plustek-backend only.
PrescanCropDialogis gated oncapabilities.prescan, which only Plustek sets; SANE/PieUSB use their own window handling and are unaffected. (Confirmed during investigation.)