feat(scan): single-shot preview window for devices with no frame adapter - #779
Open
cymbal221 wants to merge 2 commits into
Open
feat(scan): single-shot preview window for devices with no frame adapter#779cymbal221 wants to merge 2 commits into
cymbal221 wants to merge 2 commits into
Conversation
…scanners A device whose only IR mechanism is a `source` string (Plustek, and genesys's "Transparency Adapter Infrared" on Epson-class flatbeds) could never actually use it — three separate bugs, all in the same code path. 1. Capability detection never checked `source` at all _detect_ir() only recognised RGBI mode, a dedicated `ir` option, or coolscan3's `infrared` boolean — never the source-string mechanism `_ir_strategy()` already used at scan time. So the "Capture IR" checkbox stayed permanently disabled for any device relying on it, even though the scan-time code path already worked. 2. The source-name heuristic didn't match "infrared" Once wired up, the substring check `"ir" in name` doesn't actually match the word "infrared" — there's no `i` immediately followed by `r` in that word. This silently broke source-string IR detection for any device spelling it out in full (genesys's "Transparency Adapter Infrared") rather than using a short "IR" token. Fixed by also checking for "infrared" explicitly. 3. No registration between the two passes The 'source' strategy runs two full, independent scans (RGB, then IR after switching `source`). Unlike inline RGBI/coolscan3 IR — one photosite read, inherently aligned — a two-pass flatbed's carriage does not re-home with pixel-perfect repeatability, so the IR plane can land a few pixels off. Added _align_ir_to_rgb(): phase-correlates the IR plane against RGB and applies the estimated offset via whole-pixel array indexing only — no sub-pixel warp. A dust defect is a *minimum* in the IR ratio, and downsample_ir a few functions away already documents that interpolating this plane softens exactly that minimum (a thin hair's dip shallowed 0.22 -> 0.31 and shattered under INTER_AREA). More importantly, _ir_normalize_ratio in retouch/logic.py (marcinz606#659, marcinz606#715) calibrates its clean-film pivot and dip-depth stretch against this plane's own per-frame MAD sigma — interpolating it here would bias exactly that measurement, worst on the low-native-sigma hardware (Plustek/SilverFast, σ ~0.005) this strategy exists for. A carriage-repeatability offset is whole pixels anyway, so nothing is lost by not going sub-pixel. Also: the second (IR) arr_snap() had no progress callback wired to it at all, so the bar hit 100% after the RGB pass and sat still through the whole IR pass. Both passes now report their own independent 0->1 (explicit reset to 0.0 before the second pass) rather than sharing one range — no conditionality needed on whether IR was requested. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The scan-window preview (StripPreviewDialog) was gated entirely on caps.adapter_frame_capacity — a SANE `frame` option only a motorized strip/roll feeder (Coolscan's SF-210, autoloader) exposes. Plustek and similar scanners have one manual film holder with no such option, so the whole scan-window row was hidden for them: no way to preview a frame and crop before the real (full-resolution) scan. QuickScanPreviewDialog is a lighter sibling of StripPreviewDialog for this case — one preview, one crop window, no offset/drift sliders or tile grid (those correct feed-axis drift along a strip, meaningless for a single manual holder). Extracted the shared preview_positive() negative->positive display helper out of strip_preview_dialog.py so both dialogs use it. PerFrameRollSession.preview() previously always requested `frame=slot`, which fails loud on a device with no `frame` option (frame 1 is not "the first frame" there, it's simply not a settable option). Now omits `frame` when the device has no adapter capacity, making the existing roll-preview worker/signal plumbing (RollPreviewRequest -> ScanWorker.run_roll_preview -> roll_preview_ready) reusable for single-slot devices with no other backend changes. Building this surfaced a real bug in the 'source' IR strategy: the scan window (tl_x/tl_y/br_x/br_y) was only ever applied once, before the RGB pass's dev.start(). The IR pass's own dev.start() (after switching `source`) never re-applied it — and a source switch can reset or re-range those geometry options on real hardware. Left alone, that means a windowed IR-enabled scan on this class of device gets RGB and IR passes back at *different pixel sizes*, and preview_manager's loader-side guard silently drops the "mismatched" IR plane — which surfaces as the IR Removal toggle just being greyed out after loading, no error. Fixed by extracting _apply_scan_window() and calling it before both dev.start() calls, not just the first. Verified with a fake device that reproduces the reset (test fails without the fix — ir-pass snapshot shows the window wiped — and passes with it restored). 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
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.
Stacked on #778 — this diff will show both commits until that one merges; review only the second (
feat(scan): single-shot preview window...) commit.The scan-window preview (StripPreviewDialog) was gated entirely oncaps.adapter_frame_capacity — a SANE
frameoption only a motorizedstrip/roll feeder (Coolscan's SF-210, autoloader) exposes. Plustek and
similar scanners have one manual film holder with no such option, so the
whole scan-window row was hidden for them: no way to preview a frame and
crop before the real (full-resolution) scan.
QuickScanPreviewDialog is a lighter sibling of StripPreviewDialog for this
case — one preview, one crop window, no offset/drift sliders or tile grid
(those correct feed-axis drift along a strip, meaningless for a single
manual holder). Extracted the shared preview_positive() negative->positive
display helper out of strip_preview_dialog.py so both dialogs use it.
PerFrameRollSession.preview() previously always requested
frame=slot,which fails loud on a device with no
frameoption (frame 1 is not "thefirst frame" there, it's simply not a settable option). Now omits
framewhen the device has no adapter capacity, making the existing roll-preview
worker/signal plumbing (RollPreviewRequest -> ScanWorker.run_roll_preview
-> roll_preview_ready) reusable for single-slot devices with no other
backend changes.
Building this surfaced a real bug in the 'source' IR strategy: the scan
window (tl_x/tl_y/br_x/br_y) was only ever applied once, before the RGB
pass's dev.start(). The IR pass's own dev.start() (after switching
source) never re-applied it — and a source switch can reset or re-rangethose geometry options on real hardware. Left alone, that means a windowed
IR-enabled scan on this class of device gets RGB and IR passes back at
different pixel sizes, and preview_manager's loader-side guard silently
drops the "mismatched" IR plane — which surfaces as the IR Removal toggle
just being greyed out after loading, no error. Fixed by extracting
_apply_scan_window() and calling it before both dev.start() calls, not
just the first. Verified with a fake device that reproduces the reset
(test fails without the fix — ir-pass snapshot shows the window wiped —
and passes with it restored).
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com