From c37a5deafcc4916aa3ec65ba204f97af247fe679 Mon Sep 17 00:00:00 2001 From: Tobby Dallnett Date: Tue, 8 Sep 2026 02:47:12 +0800 Subject: [PATCH 1/5] feat(plustek): add Multi-Pass same-exposure repeat stacking pyopticfilm's Multi-Pass feature (PR #65) repeats a single exposure n_passes times and stacks the aligned repeats for an SNR gain, as an axis orthogonal to the existing ME (short+long) fusion rather than an alternative to it. Replaces the earlier N-brackets design this branch was originally built against (never shipped upstream) with a plain "Passes" control (1-9) independent of the Multi-exposure mode combo (Off/Adaptive/Fixed), matching pyopticfilm's actual API: Scanner.scan(multi_exposure, me_exposure_mode, n_passes, align_passes). IR and Multi-Pass cannot combine yet (pyopticfilm rejects it), so the sidebar disables/resets whichever control was set second, and the backend fails fast with a RuntimeError before touching hardware if both reach it anyway. Claude-Session: https://claude.ai/code/session_016bn5DFZyR6BS8ApqhPXUoY --- negpy/desktop/view/sidebar/scan.py | 270 +++++++++++++----- negpy/infrastructure/scanners/base.py | 4 + negpy/infrastructure/scanners/params.py | 33 ++- .../scanners/plustek_backend.py | 51 +++- negpy/infrastructure/scanners/settings.py | 16 +- tests/scanners/test_plustek_backend.py | 150 +++++++++- tests/scanners/test_scanner_settings.py | 31 ++ tests/test_scan_sidebar.py | 35 ++- 8 files changed, 493 insertions(+), 97 deletions(-) diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index 8570ac58..65c8c1fb 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -21,10 +21,31 @@ from negpy.desktop.view.styles.templates import StatusStrip, hint_label, icon_button as _icon_button, section_subheader from negpy.desktop.view.styles.theme import THEME from negpy.infrastructure.scanners.base import ScannerCapabilities, ScannerDevice -from negpy.infrastructure.scanners.params import FILM_TYPES, FilmType, film_passes_infrared +from negpy.infrastructure.scanners.params import ( + FILM_TYPES, + MIN_N_PASSES, + FilmType, + MultiExposureMode, + film_passes_infrared, +) from negpy.infrastructure.scanners.registry import DEFAULT_BACKEND_ID, backend_choices from negpy.infrastructure.scanners.settings import ScannerSettings +#: Label + explanatory tooltip for each multi-exposure mode, in display order. +_ME_MODE_LABELS: tuple[tuple[MultiExposureMode, str, str], ...] = ( + (MultiExposureMode.OFF, "Off", "One exposure per frame. Fastest."), + ( + MultiExposureMode.ADAPTIVE, + "Adaptive", + "Short and long exposures merged; the long exposure is picked automatically per frame from image content.", + ), + ( + MultiExposureMode.FIXED, + "Fixed", + "Short and long exposures merged; the long exposure is pinned to a fixed, validated value instead of chosen per frame.", + ), +) + _SAMPLE_COUNTS = (1, 2, 4, 8, 16) @@ -71,6 +92,7 @@ def __init__(self, controller) -> None: self._caps_clean = False self._caps_superfine = False self._caps_max_samples = 1 + self._caps_max_n_passes = 1 self._caps_film_formats: tuple[str, ...] = () self._caps_film_types: tuple[str, ...] = () self._device_ir = False @@ -152,8 +174,16 @@ def _init_ui(self) -> None: layout.addLayout(device_form) # ── CAPS INFO ─────────────────────────────────────── + # Crop info sits to the right of Frame info, and only appears once there is a crop + # to report — Prescan's own crop, when the connected backend uses Prescan at all. + frame_info_row = QHBoxLayout() + frame_info_row.setContentsMargins(0, 0, 0, 0) self.frame_label = hint_label("") - layout.addWidget(self.frame_label) + self.crop_label = hint_label("") + self.crop_label.setVisible(False) + frame_info_row.addWidget(self.frame_label) + frame_info_row.addWidget(self.crop_label, 1) + layout.addLayout(frame_info_row) # ── SETTINGS ──────────────────────────────────────── # Four labelled groups in one form, in the order the operator decides them: what is @@ -211,9 +241,29 @@ def _init_ui(self) -> None: self.form.addRow(self.clean_check) self.clean_check.setVisible(False) - self.me_check = QCheckBox("Multi-exposure") - self.me_check.setToolTip("Merge short and long color passes for more highlight and shadow detail. Takes longer.") - self.form.addRow(self.me_check) + self.me_combo = QComboBox() + for mode, label, tooltip in _ME_MODE_LABELS: + self.me_combo.addItem(label, mode.value) + self.me_combo.setItemData(self.me_combo.count() - 1, tooltip, Qt.ItemDataRole.ToolTipRole) + self.me_combo.setToolTip( + "Merge a short and long exposure per frame for extra shadow/highlight detail. " + "Slower than a single exposure." + ) + self.me_label = QLabel("Multi-exposure") + self.form.addRow(self.me_label, self.me_combo) + + # Independent of Multi-exposure above: repeats whichever exposure(s) are chosen (one, + # or the short+long pair) and stacks them for lower noise — "Multi-Pass" alone with + # Multi-exposure Off, or "Adaptive/Fixed Multi-Pass" combined with it. + self.passes_combo = QComboBox() + self.passes_combo.setToolTip( + "Repeat and stack each exposure for lower noise. Each extra pass adds roughly " + "one more scan pass per exposure." + ) + self.passes_label = QLabel("Passes") + self.form.addRow(self.passes_label, self.passes_combo) + self.passes_label.setVisible(False) + self.passes_combo.setVisible(False) self.superfine_check = QCheckBox("Superfine") self.superfine_check.setToolTip("Read one line per pass: slower, and free of line registration") @@ -257,78 +307,89 @@ def _init_ui(self) -> None: self.exposure_label.setVisible(False) self.exposure_row_widget.setVisible(False) + self.output_header = section_subheader("Output") + self.form.addRow(self.output_header) + + self.fmt_combo = QComboBox() + self.fmt_combo.addItems(["TIFF", "DNG"]) + self.fmt_combo.setToolTip("Output file format") + self.form.addRow("Format", self.fmt_combo) + + folder_row = QHBoxLayout() + self.folder_edit = QLineEdit() + self.folder_edit.setPlaceholderText("Output folder…") + self.folder_edit.setToolTip("Directory for scanned files") + self.browse_btn = _icon_button("fa5s.folder-open", "Browse for output folder") + folder_row.addWidget(self.folder_edit) + folder_row.addWidget(self.browse_btn) + self.form.addRow("Folder", folder_row) + + self.pattern_edit = QLineEdit() + self.pattern_edit.setToolTip('Jinja2 template. Variables: {{ date }}, {{ seq }}.\nExample: {{ date }}_{{ "%03d" % seq }}') + self.form.addRow("Filename", self.pattern_edit) + + layout.addLayout(self.form) + + # ── FRAMING (bottom, right above Scan) ─────────────── + # These are the last decisions before pressing Scan, not a settings-form field among + # Film/Quality/Output — full width, not squeezed into the form's shared label column. self.framing_header = section_subheader("Framing") - self.form.addRow(self.framing_header) + layout.addWidget(self.framing_header) # Which frames the batch scans, for roll and strip feeders only. + frame_spec_row = QHBoxLayout() + frame_spec_row.setContentsMargins(0, 0, 0, 0) + frame_spec_row.setSpacing(6) + self.frame_spec_label = QLabel("Frames") self.frame_spec_edit = QLineEdit() self.frame_spec_edit.setPlaceholderText("All frames") self.frame_spec_edit.setToolTip("Frames to scan: 1-6 or 1,2,5. Empty scans every frame.") - self.frame_spec_label = QLabel("Frames") - self.form.addRow(self.frame_spec_label, self.frame_spec_edit) + frame_spec_row.addWidget(self.frame_spec_label) + frame_spec_row.addWidget(self.frame_spec_edit, 1) + layout.addLayout(frame_spec_row) self.frame_spec_label.setVisible(False) self.frame_spec_edit.setVisible(False) # Scan window (strip/roll feeders): set once from a preview, reused per frame. - self.scan_window_widget = QWidget() - scan_window_row = QHBoxLayout(self.scan_window_widget) + scan_window_row = QHBoxLayout() scan_window_row.setContentsMargins(0, 0, 0, 0) + scan_window_row.setSpacing(6) + self.scan_window_row_label = QLabel("Batch") + self.scan_window_widget = QWidget() + scan_window_btn_row = QHBoxLayout(self.scan_window_widget) + scan_window_btn_row.setContentsMargins(0, 0, 0, 0) self.scan_window_btn = QPushButton("Set scan window…") self.scan_window_btn.setToolTip("Preview a frame and set the scan window reused for every frame") self.scan_window_clear_btn = QPushButton("Clear") self.scan_window_clear_btn.setFixedWidth(56) self.scan_window_clear_btn.setToolTip("Scan the whole default frame instead") - scan_window_row.addWidget(self.scan_window_btn, 1) - scan_window_row.addWidget(self.scan_window_clear_btn) - self.scan_window_row_label = QLabel("Batch") - self.form.addRow(self.scan_window_row_label, self.scan_window_widget) + scan_window_btn_row.addWidget(self.scan_window_btn, 1) + scan_window_btn_row.addWidget(self.scan_window_clear_btn) + scan_window_row.addWidget(self.scan_window_row_label) + scan_window_row.addWidget(self.scan_window_widget, 1) + layout.addLayout(scan_window_row) self.scan_window_status = hint_label("") - self.form.addRow("", self.scan_window_status) + layout.addWidget(self.scan_window_status) self.scan_window_row_label.setVisible(False) self.scan_window_widget.setVisible(False) self.scan_window_status.setVisible(False) # Prescan + crop (Plustek SE): low-DPI full window → interactive crop → scan_window. + # Full width, all the way to the left edge — no row label, this is the primary action + # in the group, not a field next to a caption. The crop it sets is reported next to + # Frame info above (self.crop_label), not here. self.prescan_widget = QWidget() prescan_row = QHBoxLayout(self.prescan_widget) prescan_row.setContentsMargins(0, 0, 0, 0) - self.prescan_btn = QPushButton("Prescan…") + self.prescan_btn = QPushButton("Prescan") self.prescan_btn.setToolTip("Scan a low-DPI preview and set the crop for the next scan") self.prescan_clear_btn = QPushButton("Clear") self.prescan_clear_btn.setFixedWidth(56) self.prescan_clear_btn.setToolTip("Scan the full window instead of a crop") prescan_row.addWidget(self.prescan_btn, 1) prescan_row.addWidget(self.prescan_clear_btn) - self.prescan_label = QLabel("Prescan") - self.form.addRow(self.prescan_label, self.prescan_widget) - self.prescan_status = hint_label("") - self.form.addRow("", self.prescan_status) - self.prescan_label.setVisible(False) + layout.addWidget(self.prescan_widget) self.prescan_widget.setVisible(False) - self.prescan_status.setVisible(False) - - self.output_header = section_subheader("Output") - self.form.addRow(self.output_header) - - self.fmt_combo = QComboBox() - self.fmt_combo.addItems(["TIFF", "DNG"]) - self.fmt_combo.setToolTip("Output file format") - self.form.addRow("Format", self.fmt_combo) - - folder_row = QHBoxLayout() - self.folder_edit = QLineEdit() - self.folder_edit.setPlaceholderText("Output folder…") - self.folder_edit.setToolTip("Directory for scanned files") - self.browse_btn = _icon_button("fa5s.folder-open", "Browse for output folder") - folder_row.addWidget(self.folder_edit) - folder_row.addWidget(self.browse_btn) - self.form.addRow("Folder", folder_row) - - self.pattern_edit = QLineEdit() - self.pattern_edit.setToolTip('Jinja2 template. Variables: {{ date }}, {{ seq }}.\nExample: {{ date }}_{{ "%03d" % seq }}') - self.form.addRow("Filename", self.pattern_edit) - - layout.addLayout(self.form) # ── STATUS + SCAN BUTTON ──────────────────────────── # One reserved row for all three: the pass that is running, the message it left, and @@ -367,7 +428,8 @@ def _connect_signals(self) -> None: self.dpi_combo.currentTextChanged.connect(lambda: self._update_settings_from_ui()) self.depth_combo.currentTextChanged.connect(lambda: self._update_settings_from_ui()) self.ir_check.toggled.connect(lambda on: self._on_ir_pass_toggled(self.clean_check, on)) - self.me_check.toggled.connect(lambda: self._update_settings_from_ui()) + self.me_combo.currentIndexChanged.connect(lambda: self._update_settings_from_ui()) + self.passes_combo.currentIndexChanged.connect(self._on_passes_changed) self.autofocus_check.toggled.connect(lambda: self._update_settings_from_ui()) self.ae_check.toggled.connect(lambda: self._on_ae_toggled()) self.clean_check.toggled.connect(lambda on: self._on_ir_pass_toggled(self.ir_check, on)) @@ -482,9 +544,12 @@ def _update_device_caps(self) -> None: self.depth_combo.setVisible(False) self.depth_label.setVisible(False) self.ir_check.setVisible(False) - self.me_check.setVisible(False) + self.me_label.setVisible(False) + self.me_combo.setVisible(False) + self.passes_label.setVisible(False) + self.passes_combo.setVisible(False) self.ir_check.setEnabled(False) - self.me_check.setEnabled(False) + self.me_combo.setEnabled(False) self.eject_btn.setVisible(False) self.frame_spec_label.setVisible(False) self.frame_spec_edit.setVisible(False) @@ -495,9 +560,8 @@ def _update_device_caps(self) -> None: self.exposure_row_widget.setVisible(False) self.autofocus_check.setVisible(False) self.ae_check.setVisible(False) - self.prescan_label.setVisible(False) self.prescan_widget.setVisible(False) - self.prescan_status.setVisible(False) + self.crop_label.setVisible(False) self.clean_check.setVisible(False) self.superfine_check.setVisible(False) self.samples_label.setVisible(False) @@ -511,6 +575,7 @@ def _update_device_caps(self) -> None: self._caps_clean = False self._caps_superfine = False self._caps_max_samples = 1 + self._caps_max_n_passes = 1 self._caps_film_formats = () self._caps_film_types = () self._device_ir = False @@ -522,7 +587,7 @@ def _update_device_caps(self) -> None: self.dpi_combo.setEnabled(True) self.depth_combo.setEnabled(True) self.ir_check.setEnabled(True) - self.me_check.setEnabled(True) + self.me_combo.setEnabled(True) self.eject_btn.setVisible(caps.can_eject) self.eject_btn.setEnabled(caps.can_eject and not self._scanning) self.frame_label.setText(f"Frame: {caps.max_area_mm[0]:.0f} × {caps.max_area_mm[1]:.0f} mm") @@ -543,7 +608,8 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: self.dpi_combo.blockSignals(True) self.depth_combo.blockSignals(True) self.ir_check.blockSignals(True) - self.me_check.blockSignals(True) + self.me_combo.blockSignals(True) + self.passes_combo.blockSignals(True) self.ae_check.blockSignals(True) self.frame_spec_edit.blockSignals(True) @@ -595,17 +661,28 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: self.ir_check.setToolTip("IR scanning not supported by this device") # Multi-exposure (Plustek GL128 scan-ready models) - self.me_check.setVisible(bool(caps.multi_exposure)) - self.me_check.setEnabled(caps.multi_exposure) + self.me_label.setVisible(bool(caps.multi_exposure)) + self.me_combo.setVisible(bool(caps.multi_exposure)) + self.me_combo.setEnabled(caps.multi_exposure) if caps.multi_exposure: - self.me_check.setChecked(self._settings.multi_exposure) - self.me_check.setToolTip( - "Merge short and long colour passes for more highlight and shadow detail. " - "The long pass exposure is chosen per frame. Takes longer." - ) + idx = self.me_combo.findData(self._settings.multi_exposure_mode) + self.me_combo.setCurrentIndex(idx if idx >= 0 else 0) else: - self.me_check.setChecked(False) - self.me_check.setToolTip("Multi-exposure not supported by this device") + self.me_combo.setCurrentIndex(0) + + # Passes (same-exposure repeat stacking) — independent of Multi-exposure above, so its + # own row, gated on max_n_passes alone. + self._caps_max_n_passes = int(caps.max_n_passes) + self.passes_combo.clear() + show_passes = caps.max_n_passes > 1 + if show_passes: + for n in range(MIN_N_PASSES, caps.max_n_passes + 1): + self.passes_combo.addItem(str(n), n) + idx = self.passes_combo.findData(self._settings.n_passes) + self.passes_combo.setCurrentIndex(max(idx, 0)) + self.passes_label.setVisible(show_passes) + self.passes_combo.setVisible(show_passes) + self._refresh_ir_passes_conflict() # Autofocus and auto-exposure, shown only when the device reports them. self._caps_autofocus = bool(caps.autofocus) @@ -730,16 +807,17 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: self._update_scan_window_status() show_prescan = bool(caps.prescan) - self.prescan_label.setVisible(show_prescan) self.prescan_widget.setVisible(show_prescan) - self.prescan_status.setVisible(show_prescan) if show_prescan: - self._update_prescan_status() + self._update_crop_label() + else: + self.crop_label.setVisible(False) self.dpi_combo.blockSignals(False) self.depth_combo.blockSignals(False) self.ir_check.blockSignals(False) - self.me_check.blockSignals(False) + self.me_combo.blockSignals(False) + self.passes_combo.blockSignals(False) self.ae_check.blockSignals(False) self.frame_spec_edit.blockSignals(False) @@ -773,6 +851,35 @@ def _on_ir_pass_toggled(self, other: QCheckBox, checked: bool) -> None: other.blockSignals(True) other.setChecked(False) other.blockSignals(False) + self._refresh_ir_passes_conflict() + self._update_settings_from_ui() + + def _refresh_ir_passes_conflict(self) -> None: + """IR and Multi-Pass (Passes > 1) cannot be combined yet — pyopticfilm rejects the + combination outright (each repeat is its own motor cycle; IR stacking is unvalidated). + Reset whichever control was already set rather than merely disabling one, matching the + IR/ICE exclusivity above.""" + if self.ir_check.isChecked() and self._passes() > 1: + self.passes_combo.blockSignals(True) + idx = self.passes_combo.findData(MIN_N_PASSES) + self.passes_combo.setCurrentIndex(idx if idx >= 0 else 0) + self.passes_combo.blockSignals(False) + + def _me_mode(self) -> MultiExposureMode: + if not self.me_combo.isEnabled(): + return MultiExposureMode.OFF + return MultiExposureMode(self.me_combo.currentData() or MultiExposureMode.OFF.value) + + def _passes(self) -> int: + if self._caps_max_n_passes <= 1: + return MIN_N_PASSES + return int(self.passes_combo.currentData() or MIN_N_PASSES) + + def _on_passes_changed(self, _index: int) -> None: + if self._passes() > 1 and self.ir_check.isChecked(): + self.ir_check.blockSignals(True) + self.ir_check.setChecked(False) + self.ir_check.blockSignals(False) self._update_settings_from_ui() def _samples(self) -> int: @@ -893,7 +1000,7 @@ def _on_prescan(self) -> None: ) if dialog.exec(): self.settings = replace(self._settings, scan_window=dialog.scan_window()) - self._update_prescan_status() + self._update_crop_label() self._save_settings() if dialog.scan_requested(): self._on_scan() @@ -902,10 +1009,12 @@ def _on_clear_prescan_crop(self) -> None: from dataclasses import replace self.settings = replace(self._settings, scan_window=None) - self._update_prescan_status() + self._update_crop_label() self._save_settings() - def _update_prescan_status(self) -> None: + def _update_crop_label(self) -> None: + """Next to Frame info, and only shown when there is an actual crop to report — + a full-window scan says nothing here rather than stating the obvious.""" from negpy.infrastructure.scanners.params import scan_window_to_area device = self._current_device() @@ -915,10 +1024,11 @@ def _update_prescan_status(self) -> None: else None ) if area is None: - self.prescan_status.setText("Full window") + self.crop_label.setVisible(False) else: tl_x, tl_y, br_x, br_y = area - self.prescan_status.setText(f"Crop {br_x - tl_x:.1f} × {br_y - tl_y:.1f} mm") + self.crop_label.setText(f"Crop: {br_x - tl_x:.1f} × {br_y - tl_y:.1f} mm") + self.crop_label.setVisible(True) def _update_scan_window_status(self) -> None: from negpy.infrastructure.scanners.params import scan_window_to_area @@ -998,8 +1108,15 @@ def _update_summary(self) -> None: passes.append("Superfine") if self._samples() > 1: passes.append(f"{self._samples()}× sampled") - if self.me_check.isEnabled() and self.me_check.isChecked(): - passes.append("Multi-exposure") + me_mode = self._me_mode() + n_passes = self._passes() + me_label = next((label for mode, label, _ in _ME_MODE_LABELS if mode == me_mode), None) + if me_mode != MultiExposureMode.OFF and n_passes > 1: + passes.append(f"{me_label} ×{n_passes} passes") + elif me_mode != MultiExposureMode.OFF: + passes.append(me_label) + elif n_passes > 1: + passes.append(f"Multi-Pass ×{n_passes}") # The count and the size are what the operator checks before committing, so they carry # primary weight; the rest of the line stays secondary. strong = f'{{}}' @@ -1042,7 +1159,8 @@ def _on_scan(self) -> None: dpi = self._dpi() depth = int(self.depth_combo.currentData() or 16) capture_ir = self.ir_check.isEnabled() and self.ir_check.isChecked() - multi_exposure = self.me_check.isEnabled() and self.me_check.isChecked() + me_mode = self._me_mode() + n_passes = self._passes() autofocus = self._caps_autofocus and self.autofocus_check.isChecked() auto_exposure = self._caps_auto_exposure and self.ae_check.isChecked() pattern = self.pattern_edit.text().strip() or '{{ date }}_{{ "%03d" % seq }}' @@ -1065,7 +1183,8 @@ def _on_scan(self) -> None: dpi=dpi, depth=depth, capture_ir=capture_ir, - multi_exposure=multi_exposure, + multi_exposure_mode=me_mode, + n_passes=n_passes, autofocus=autofocus, auto_exposure=auto_exposure, exposure_time_us=exposure_time_us, @@ -1219,7 +1338,8 @@ def _update_settings_from_ui(self) -> None: dpi=dpi, depth=depth, capture_ir=self.ir_check.isChecked() and self.ir_check.isEnabled(), - multi_exposure=self.me_check.isChecked() and self.me_check.isEnabled(), + multi_exposure_mode=self._me_mode().value, + n_passes=self._passes(), autofocus=self._caps_autofocus and self.autofocus_check.isChecked(), auto_exposure=self._caps_auto_exposure and self.ae_check.isChecked(), exposure_time_us=(self.exposure_slider.value() if self.exposure_row_widget.isVisible() else None), diff --git a/negpy/infrastructure/scanners/base.py b/negpy/infrastructure/scanners/base.py index af9ac74d..b81dd6ea 100644 --- a/negpy/infrastructure/scanners/base.py +++ b/negpy/infrastructure/scanners/base.py @@ -37,6 +37,10 @@ class ScannerCapabilities: prescan_mirror_x: bool = False prescan_default_crop: tuple[float, float, float, float] | None = None multi_exposure: bool = False + #: Highest n_passes the device accepts; 1 means Multi-Pass (same-exposure repeat stacking) + #: is unavailable. Independent of `multi_exposure` — repeating a single exposure needs no + #: long-exposure capability, so this is not gated on the same condition. + max_n_passes: int = 1 adapter_frame_capacity: int | None = None # transport capacity bound, not an exposure count adapter_frame_control: bool = False can_eject: bool = False diff --git a/negpy/infrastructure/scanners/params.py b/negpy/infrastructure/scanners/params.py index ab176169..365a30f8 100644 --- a/negpy/infrastructure/scanners/params.py +++ b/negpy/infrastructure/scanners/params.py @@ -8,12 +8,43 @@ class ScanMode(StrEnum): TRANSPARENCY = "Transparency" +class MultiExposureMode(StrEnum): + """How the top exposure is chosen when merging short+long colour passes. + + OFF: one exposure, the fast path. ADAPTIVE: short+long merged, long exposure picked per + frame from image content (today's only multi-exposure behavior). FIXED: short+long merged, + long exposure pinned to a fixed, per-model-validated value instead of chosen per frame. + + Orthogonal to ``ScanParams.n_passes``: this picks *which* exposure(s) to merge, while + ``n_passes`` repeats whichever exposure(s) are chosen for a same-exposure SNR stack — the + two compose (pyopticfilm's "Adaptive/Fixed Multi-Pass") rather than being alternatives. + """ + + OFF = "off" + ADAPTIVE = "adaptive" + FIXED = "fixed" + + +#: Repeats of the same exposure to stack for an SNR gain (pyopticfilm's `n_passes`); 1 = no +#: stacking. This module must not import pyopticfilm directly (only plustek_backend.py may, +#: see test_only_adapter_imports_plustek_driver), so this mirrors — rather than imports — +#: pyopticfilm's own `Scanner.scan()` bound; plustek_backend.py's own MAX_N_PASSES import +#: keeps the two in sync at the one place that already reaches into pyopticfilm. +MIN_N_PASSES = 1 +MAX_N_PASSES = 9 +#: Starting point when a user first turns Passes above the off position — comfortably past +#: the floor without defaulting to the slow end. +DEFAULT_N_PASSES = 3 + + @dataclass(frozen=True) class ScanParams: dpi: int depth: int capture_ir: bool - multi_exposure: bool = False + multi_exposure_mode: MultiExposureMode = MultiExposureMode.OFF + # Same-exposure repeats to stack for an SNR gain (1-9); independent of multi_exposure_mode. + n_passes: int = MIN_N_PASSES # Normalized (x1,y1,x2,y2) window 0..1; backend maps to device units (coolscan3 int px). window: tuple[float, float, float, float] | None = None # coolscan3 `subframe` (mm), applied to every frame. 0 = scanner default. diff --git a/negpy/infrastructure/scanners/plustek_backend.py b/negpy/infrastructure/scanners/plustek_backend.py index 7a753276..aebf4b1c 100644 --- a/negpy/infrastructure/scanners/plustek_backend.py +++ b/negpy/infrastructure/scanners/plustek_backend.py @@ -19,7 +19,13 @@ ScannerUnavailable, TransientScanError, ) -from negpy.infrastructure.scanners.params import ScanMode, ScanParams +from negpy.infrastructure.scanners.params import ( + MAX_N_PASSES, + MIN_N_PASSES, + MultiExposureMode, + ScanMode, + ScanParams, +) from pyopticfilm.asic.gl128 import DEFAULT_IMAGE_USB_PACE_S from pyopticfilm.device.select import model_for_device, model_is_scan_ready from pyopticfilm.exceptions import ( @@ -60,6 +66,9 @@ def _caps_for(model: Any) -> ScannerCapabilities: prescan_mirror_x=bool(getattr(model, "mirror_x", False)) if prescan_ready else False, prescan_default_crop=default_frame_crop_norm(model) if prescan_ready else None, multi_exposure=bool(getattr(model, "scan_ready", False) and getattr(model, "exposure_long", None)), + # Multi-Pass (repeating the existing single exposure) needs no long-exposure register — + # every scan-ready GL128 model supports it, independent of ME's exposure_long gating. + max_n_passes=MAX_N_PASSES if getattr(model, "scan_ready", False) else MIN_N_PASSES, adapter_frame_capacity=None, adapter_frame_control=False, can_eject=False, @@ -89,11 +98,18 @@ def _safe_progress( progress(max(0.0, min(1.0, float(value))), phase) -def _gl128_me_pass_layout(*, capture_ir: bool, multi_exposure: bool) -> tuple[int, int] | None: +def _gl128_me_pass_layout(*, capture_ir: bool, multi_exposure: bool, n_passes: int = 1) -> tuple[int, int] | None: + """(n_early, n_pass): physical passes before, and total physical passes across, the single + short→long exposure-change boundary. Every repeat within a slot shares one exposure — no + pyopticfilm-side "preparing" moment between repeats — so there is exactly one boundary + regardless of ``n_passes``: all short-slot repeats (+ the optional IR pass) happen first, + then all long-slot repeats. Collapses to the pre-Multi-Pass ``(2,3)``/``(1,2)`` layout at + ``n_passes=1``.""" if not multi_exposure: return None - n_early = 2 if capture_ir else 1 - return n_early, n_early + 1 + ir_extra = 1 if capture_ir else 0 + n_early = ir_extra + n_passes + return n_early, n_early + n_passes def _make_scan_progress( @@ -101,8 +117,9 @@ def _make_scan_progress( *, multi_exposure: bool, capture_ir: bool, + n_passes: int = 1, ) -> Callable[[float], None]: - layout = _gl128_me_pass_layout(capture_ir=capture_ir, multi_exposure=multi_exposure) + layout = _gl128_me_pass_layout(capture_ir=capture_ir, multi_exposure=multi_exposure, n_passes=n_passes) if layout is None: def scan_progress(p: float) -> None: @@ -154,8 +171,18 @@ def _validate_params(params: ScanParams, *, model: Any | None = None) -> None: raise RuntimeError("Autofocus requested but the device has no autofocus option") if params.capture_ir and model is not None and getattr(model, "supports_infrared", None) is False: raise RuntimeError(f"{getattr(model, 'model', 'device')} does not support infrared") - if params.multi_exposure and model is not None and not getattr(model, "exposure_long", None): + mode = params.multi_exposure_mode + if mode != MultiExposureMode.OFF and model is not None and not getattr(model, "exposure_long", None): raise RuntimeError(f"{getattr(model, 'model', 'device')} does not support multi-exposure") + if params.n_passes > 1: + if not (MIN_N_PASSES <= params.n_passes <= MAX_N_PASSES): + raise RuntimeError(f"n_passes={params.n_passes} out of range ({MIN_N_PASSES}-{MAX_N_PASSES})") + if model is not None and not getattr(model, "scan_ready", False): + raise RuntimeError(f"{getattr(model, 'model', 'device')} does not support Multi-Pass") + if params.capture_ir: + raise RuntimeError( + "IR and Multi-Pass cannot be combined yet — scan IR separately, or set Passes to 1." + ) class PlustekSession: @@ -317,7 +344,12 @@ def _scan_on_scanner( _validate_params(params, model=scanner.model) dpi = int(params.dpi) capture_ir = bool(params.capture_ir) - multi_exposure = bool(params.multi_exposure) + me_mode = params.multi_exposure_mode + multi_exposure = me_mode != MultiExposureMode.OFF + # Irrelevant to pyopticfilm when multi_exposure is False; pass its own default rather + # than a value that would misleadingly suggest ME is fixed-mode when it's simply off. + me_exposure_mode = "fixed" if me_mode == MultiExposureMode.FIXED else "adaptive" + n_passes = int(params.n_passes) window = params.window geometry = self._default_scan_geometry(scanner, dpi=dpi, window=window) @@ -339,6 +371,7 @@ def _scan_on_scanner( progress, multi_exposure=multi_exposure, capture_ir=capture_ir, + n_passes=n_passes, ) def on_status(status: str) -> None: @@ -359,7 +392,9 @@ def on_status(status: str) -> None: on_status=on_status, multi_exposure=multi_exposure, infrared=capture_ir, - me_exposure_mode="adaptive", + me_exposure_mode=me_exposure_mode, + align_passes=True, + n_passes=n_passes, ) ir_plane = np.asarray(rgb_image.ir) if capture_ir and rgb_image.ir is not None else None except ScanCancelled as exc: diff --git a/negpy/infrastructure/scanners/settings.py b/negpy/infrastructure/scanners/settings.py index 7eb1bf2a..75a163b7 100644 --- a/negpy/infrastructure/scanners/settings.py +++ b/negpy/infrastructure/scanners/settings.py @@ -1,6 +1,7 @@ from collections.abc import Iterable from dataclasses import dataclass, field, fields +from negpy.infrastructure.scanners.params import MAX_N_PASSES, MIN_N_PASSES, MultiExposureMode from negpy.infrastructure.scanners.registry import DEFAULT_BACKEND_ID Rect = tuple[float, float, float, float] @@ -16,7 +17,9 @@ class ScannerSettings: dpi: int = 3600 depth: int = 16 capture_ir: bool = False - multi_exposure: bool = False + multi_exposure_mode: str = MultiExposureMode.OFF.value + # Same-exposure repeats to stack for an SNR gain (1-9); independent of multi_exposure_mode. + n_passes: int = MIN_N_PASSES autofocus: bool = True auto_exposure: bool = False # Hardware scan exposure time in microseconds (SANE `scan-exposure-time`). None is the @@ -68,6 +71,17 @@ def from_dict(cls, data: dict) -> "ScannerSettings": every unrelated preference with it. """ data = dict(data) + # Pre-mode blobs only ever had one multi-exposure behavior (today's "adaptive"): a + # checked box meant exactly that, unchecked meant none. + if "multi_exposure_mode" not in data and "multi_exposure" in data: + data["multi_exposure_mode"] = ( + MultiExposureMode.ADAPTIVE.value if data.pop("multi_exposure") else MultiExposureMode.OFF.value + ) + if data.get("multi_exposure_mode") not in set(MultiExposureMode): + data["multi_exposure_mode"] = MultiExposureMode.OFF.value + n_passes = data.get("n_passes") + if isinstance(n_passes, int) and not (MIN_N_PASSES <= n_passes <= MAX_N_PASSES): + data["n_passes"] = min(max(n_passes, MIN_N_PASSES), MAX_N_PASSES) first, last = data.pop("frame_from", None), data.pop("frame_to", None) if not data.get("selected_frames") and isinstance(first, int) and isinstance(last, int) and (first, last) != (1, 1): data["selected_frames"] = tuple(range(first, last + 1)) diff --git a/tests/scanners/test_plustek_backend.py b/tests/scanners/test_plustek_backend.py index b7922aa0..54a55df4 100644 --- a/tests/scanners/test_plustek_backend.py +++ b/tests/scanners/test_plustek_backend.py @@ -14,7 +14,7 @@ import pytest from negpy.infrastructure.scanners.base import ScannerUnavailable, TransientScanError -from negpy.infrastructure.scanners.params import ScanParams +from negpy.infrastructure.scanners.params import MultiExposureMode, ScanParams from negpy.infrastructure.scanners.result import ScanResult pytest.importorskip("pyopticfilm") @@ -194,6 +194,7 @@ def test_8100_v2_caps_match_pyopticfilm_model(monkeypatch): caps = dev.capabilities assert caps.ir_channel is False assert caps.multi_exposure is True + assert caps.max_n_passes > 1 assert caps.prescan is True assert caps.prescan_mirror_x is True # 8100 V2 inherits mirror_x from 8200i SE assert 1200 in caps.supported_dpi @@ -215,6 +216,9 @@ def test_backend_list_devices_maps_caps(monkeypatch): assert dev.capabilities.prescan is True assert dev.capabilities.prescan_dpi == 1200 assert dev.capabilities.multi_exposure is True + from negpy.infrastructure.scanners.params import MAX_N_PASSES + + assert dev.capabilities.max_n_passes == MAX_N_PASSES assert dev.capabilities.prescan_default_crop is not None @@ -341,23 +345,114 @@ def test_multi_exposure_passthrough(monkeypatch): monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) PlustekBackend().scan( _DEVICE_ID, - _params(multi_exposure=True), + _params(multi_exposure_mode=MultiExposureMode.ADAPTIVE), lambda *_: None, threading.Event(), ) assert scanner.scan.call_args.kwargs.get("multi_exposure") is True assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" + assert scanner.scan.call_args.kwargs.get("align_passes") is True + assert scanner.scan.call_args.kwargs.get("n_passes") == 1 def test_colour_scan_passes_adaptive_me_mode(monkeypatch): _patch_enum(monkeypatch) scanner = _fake_scanner() monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) - PlustekBackend().scan(_DEVICE_ID, _params(), lambda *_: None, threading.Event()) + PlustekBackend().scan( + _DEVICE_ID, + _params(multi_exposure_mode=MultiExposureMode.ADAPTIVE), + lambda *_: None, + threading.Event(), + ) assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" assert scanner.scan.call_args.kwargs.get("on_status") is not None +def test_off_mode_defaults_exposure_mode_to_adaptive(monkeypatch): + """OFF is a no-op path — me_exposure_mode is irrelevant to pyopticfilm when + multi_exposure is False, so the value passed just needs to be pyopticfilm's own default.""" + _patch_enum(monkeypatch) + scanner = _fake_scanner() + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + PlustekBackend().scan(_DEVICE_ID, _params(), lambda *_: None, threading.Event()) + assert scanner.scan.call_args.kwargs.get("multi_exposure") is False + assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" + + +def test_fixed_mode_pins_exposure_mode(monkeypatch): + _patch_enum(monkeypatch) + scanner = _fake_scanner() + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + PlustekBackend().scan( + _DEVICE_ID, + _params(multi_exposure_mode=MultiExposureMode.FIXED), + lambda *_: None, + threading.Event(), + ) + assert scanner.scan.call_args.kwargs.get("multi_exposure") is True + assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "fixed" + + +def test_n_passes_flows_through_to_scanner_scan(monkeypatch): + _patch_enum(monkeypatch) + scanner = _fake_scanner() + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + PlustekBackend().scan( + _DEVICE_ID, + _params(n_passes=5), + lambda *_: None, + threading.Event(), + ) + assert scanner.scan.call_args.kwargs.get("multi_exposure") is False + assert scanner.scan.call_args.kwargs.get("n_passes") == 5 + assert scanner.scan.call_args.kwargs.get("align_passes") is True + + +def test_adaptive_multi_pass_passes_both_axes(monkeypatch): + """multi_exposure_mode and n_passes are independent — both reach scanner.scan together.""" + _patch_enum(monkeypatch) + scanner = _fake_scanner() + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + PlustekBackend().scan( + _DEVICE_ID, + _params(multi_exposure_mode=MultiExposureMode.ADAPTIVE, n_passes=4), + lambda *_: None, + threading.Event(), + ) + assert scanner.scan.call_args.kwargs.get("multi_exposure") is True + assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" + assert scanner.scan.call_args.kwargs.get("n_passes") == 4 + + +def test_n_passes_rejects_out_of_range_value(monkeypatch): + _patch_enum(monkeypatch) + scanner = _fake_scanner() + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + with pytest.raises(RuntimeError, match="n_passes"): + PlustekBackend().scan( + _DEVICE_ID, + _params(n_passes=10), + lambda *_: None, + threading.Event(), + ) + scanner.scan.assert_not_called() + + +def test_ir_and_multi_pass_together_is_rejected(monkeypatch): + _patch_enum(monkeypatch) + scanner = _fake_scanner() + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + with pytest.raises(RuntimeError, match="Multi-Pass"): + PlustekBackend().scan( + _DEVICE_ID, + _params(capture_ir=True, n_passes=3), + lambda *_: None, + threading.Event(), + ) + scanner.scan.assert_not_called() + + def test_on_status_reports_priming_then_scanning(monkeypatch): _patch_enum(monkeypatch) scanner = _fake_scanner() @@ -385,7 +480,7 @@ def progress(fraction: float, phase: str = "Scanning") -> None: PlustekBackend().scan( _DEVICE_ID, - _params(multi_exposure=True), + _params(multi_exposure_mode=MultiExposureMode.ADAPTIVE), progress, threading.Event(), ) @@ -406,7 +501,7 @@ def progress(_fraction: float, phase: str = "Scanning") -> None: PlustekBackend().scan( _DEVICE_ID, - _params(multi_exposure=True), + _params(multi_exposure_mode=MultiExposureMode.ADAPTIVE), progress, threading.Event(), ) @@ -414,6 +509,51 @@ def progress(_fraction: float, phase: str = "Scanning") -> None: assert "Preparing long exposure" not in phases +@pytest.mark.parametrize("n_passes", [1, 3, 9]) +def test_adaptive_multi_pass_reports_single_boundary_regardless_of_n_passes(monkeypatch, n_passes): + """Every repeat within a slot shares one exposure, so there is exactly one + short→long boundary no matter how many passes are stacked on each side of it.""" + _patch_enum(monkeypatch) + fractions = [i / (2 * n_passes) for i in range(1, 2 * n_passes)] + [1.0] + scanner = _fake_scanner(me_fractions=fractions) + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + seen: list[tuple[float, str]] = [] + + def progress(fraction: float, phase: str = "Scanning") -> None: + seen.append((fraction, phase)) + + PlustekBackend().scan( + _DEVICE_ID, + _params(multi_exposure_mode=MultiExposureMode.ADAPTIVE, n_passes=n_passes), + progress, + threading.Event(), + ) + phases = [phase for _, phase in seen] + assert phases.count("Preparing long exposure") == 1 + assert phases.count("Merging exposures") == 1 + assert "Preparing next exposure" not in phases + + +def test_multi_pass_without_me_has_no_preparing_or_merging_phases(monkeypatch): + """Passes alone (no ME) has no exposure-change boundary and no fusion step.""" + _patch_enum(monkeypatch) + scanner = _fake_scanner(progress_steps=5) + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + phases: list[str] = [] + + def progress(fraction: float, phase: str = "Scanning") -> None: + phases.append(phase) + + PlustekBackend().scan( + _DEVICE_ID, + _params(n_passes=5), + progress, + threading.Event(), + ) + assert "Preparing long exposure" not in phases + assert "Merging exposures" not in phases + + def test_non_me_scan_skips_me_progress_phases(monkeypatch): _patch_enum(monkeypatch) scanner = _fake_scanner(progress_steps=4) diff --git a/tests/scanners/test_scanner_settings.py b/tests/scanners/test_scanner_settings.py index a2c30df4..9d8c49b9 100644 --- a/tests/scanners/test_scanner_settings.py +++ b/tests/scanners/test_scanner_settings.py @@ -139,3 +139,34 @@ def test_an_unset_saved_frame_range_selects_nothing(): def test_a_key_this_version_dropped_keeps_the_rest_of_the_blob(): restored = ScannerSettings.from_dict({"gone_in_this_version": True, "output_folder": "/scans"}) assert restored.output_folder == "/scans" + + +def test_legacy_multi_exposure_true_migrates_to_adaptive_mode(): + restored = ScannerSettings.from_dict({"multi_exposure": True}) + assert restored.multi_exposure_mode == "adaptive" + + +def test_legacy_multi_exposure_false_migrates_to_off_mode(): + restored = ScannerSettings.from_dict({"multi_exposure": False}) + assert restored.multi_exposure_mode == "off" + + +def test_a_blob_with_multi_exposure_mode_already_set_is_not_double_migrated(): + restored = ScannerSettings.from_dict({"multi_exposure": True, "multi_exposure_mode": "fixed"}) + assert restored.multi_exposure_mode == "fixed" + + +def test_an_unknown_multi_exposure_mode_falls_back_to_off(): + restored = ScannerSettings.from_dict({"multi_exposure_mode": "some_future_mode"}) + assert restored.multi_exposure_mode == "off" + + +def test_fresh_install_defaults_n_passes_to_one(): + assert ScannerSettings.defaults().n_passes == 1 + + +def test_out_of_range_n_passes_degrades_into_bounds(): + restored = ScannerSettings.from_dict({"n_passes": 99}) + assert restored.n_passes == 9 + restored = ScannerSettings.from_dict({"n_passes": 0}) + assert restored.n_passes == 1 diff --git a/tests/test_scan_sidebar.py b/tests/test_scan_sidebar.py index 4c1d738e..03c815a4 100644 --- a/tests/test_scan_sidebar.py +++ b/tests/test_scan_sidebar.py @@ -27,7 +27,7 @@ from negpy.desktop.view.sidebar.scan import ScanSidebar, estimated_frame_bytes from negpy.desktop.view.styles.theme import THEME from negpy.infrastructure.scanners.base import ScannerCapabilities, ScannerDevice -from negpy.infrastructure.scanners.params import FILM_TYPES, ScanMode +from negpy.infrastructure.scanners.params import FILM_TYPES, MultiExposureMode, ScanMode if not QApplication.instance(): _app = QApplication(sys.argv) @@ -82,6 +82,7 @@ prescan=True, prescan_dpi=1200, multi_exposure=True, + max_n_passes=9, prescan_default_crop=(0.0, 0.35, 1.0, 0.65), ) SE_DEVICE = ScannerDevice( @@ -222,10 +223,9 @@ def test_minimal_device_hides_coolscan_controls() -> None: def test_se_device_shows_prescan() -> None: sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek"}) assert sidebar.prescan_widget.isVisibleTo(sidebar) is True - assert sidebar.prescan_label.isVisibleTo(sidebar) is True assert sidebar.scan_window_widget.isVisibleTo(sidebar) is False assert sidebar.ir_check.isEnabled() is True - assert sidebar.me_check.isEnabled() is True + assert sidebar.me_combo.isEnabled() is True assert sidebar.frame_spec_edit.isVisibleTo(sidebar) is False @@ -246,8 +246,29 @@ def test_sane_backend_keeps_single_holder_window_control(monkeypatch) -> None: def test_minimal_device_disables_multi_exposure() -> None: sidebar, _ = _sidebar(MINIMAL_DEVICE) - assert sidebar.me_check.isEnabled() is False - assert sidebar.me_check.isChecked() is False + assert sidebar.me_combo.isEnabled() is False + assert sidebar._me_mode() == MultiExposureMode.OFF + + +def test_minimal_device_hides_passes_control() -> None: + sidebar, _ = _sidebar(MINIMAL_DEVICE) + assert sidebar.passes_combo.isVisibleTo(sidebar) is False + assert sidebar._passes() == 1 + + +def test_passes_above_one_disables_and_unchecks_ir() -> None: + sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek", "capture_ir": True}) + idx = sidebar.passes_combo.findData(3) + assert idx >= 0 + sidebar.passes_combo.setCurrentIndex(idx) + assert sidebar.ir_check.isChecked() is False + + +def test_checking_ir_resets_passes_to_one() -> None: + sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek", "n_passes": 3}) + assert sidebar._passes() == 3 + sidebar.ir_check.setChecked(True) + assert sidebar._passes() == 1 def test_scan_params_include_prescan_crop() -> None: @@ -985,11 +1006,11 @@ def test_a_pass_the_device_cannot_run_is_not_shown_at_all() -> None: # Disabled-with-a-reason is for a pass the film blocks; one the transport lacks goes away. sidebar, _ = _sidebar(MINIMAL_DEVICE) assert sidebar.ir_check.isVisibleTo(sidebar) is False - assert sidebar.me_check.isVisibleTo(sidebar) is False + assert sidebar.me_combo.isVisibleTo(sidebar) is False sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek"}) assert sidebar.ir_check.isVisibleTo(sidebar) is True - assert sidebar.me_check.isVisibleTo(sidebar) is True + assert sidebar.me_combo.isVisibleTo(sidebar) is True def test_a_film_that_blocks_infrared_leaves_the_control_visible_to_explain_itself() -> None: From dbf7d01f237e6b95aefc5da98c1e1c6154ddff2b Mon Sep 17 00:00:00 2001 From: Tobby Dallnett Date: Tue, 8 Sep 2026 03:23:59 +0800 Subject: [PATCH 2/5] feat(plustek): collapse scan mode UI to 4 named options + passes slider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplifies the two independent ME-mode/Passes controls into one "Scan mode" combo (Single-Pass, Multi-Pass, Adaptive Multi-Exposure, Adaptive Multi-Pass) with a 2-9 slider that only appears for the two stacking modes. Drops "Fixed" long-exposure mode from NegPy's surface entirely — it's a pyopticfilm lab/debug-only concept now (see Scan Lab, which keeps full unrestricted access to it and the manual exposure overrides). - MultiExposureMode loses FIXED; ScannerSettings' existing unknown-value-degrades-to-off migration path already handles a persisted "fixed" blob safely, no new migration code needed. - New ScanCaptureMode (UI-only, never persisted) with translation helpers to/from the real (multi_exposure_mode, n_passes) fields, keeping the domain model as pyopticfilm's actual orthogonal axes. - Checking IR while a stacking mode is selected drops the mode to its non-stacking equivalent (Multi-Pass -> Single-Pass, Adaptive Multi-Pass -> Adaptive Multi-Exposure) rather than silently resetting a hidden value; selecting a stacking mode while IR is checked unchecks IR. - Per-item capability gating on the mode combo, with graceful fallback (drop just the unavailable axis) if the current selection becomes invalid on a capability change. Claude-Session: https://claude.ai/code/session_016bn5DFZyR6BS8ApqhPXUoY --- negpy/desktop/view/sidebar/scan.py | 301 ++++++++++++------ negpy/infrastructure/scanners/params.py | 13 +- .../scanners/plustek_backend.py | 8 +- tests/scanners/test_plustek_backend.py | 16 +- tests/scanners/test_scanner_settings.py | 13 +- tests/test_scan_sidebar.py | 60 +++- 6 files changed, 270 insertions(+), 141 deletions(-) diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index 65c8c1fb..5e399f25 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -1,3 +1,5 @@ +from enum import StrEnum + import qtawesome as qta from PyQt6.QtCore import Qt, pyqtSlot from PyQt6.QtGui import QIntValidator @@ -23,7 +25,6 @@ from negpy.infrastructure.scanners.base import ScannerCapabilities, ScannerDevice from negpy.infrastructure.scanners.params import ( FILM_TYPES, - MIN_N_PASSES, FilmType, MultiExposureMode, film_passes_infrared, @@ -31,21 +32,84 @@ from negpy.infrastructure.scanners.registry import DEFAULT_BACKEND_ID, backend_choices from negpy.infrastructure.scanners.settings import ScannerSettings -#: Label + explanatory tooltip for each multi-exposure mode, in display order. -_ME_MODE_LABELS: tuple[tuple[MultiExposureMode, str, str], ...] = ( - (MultiExposureMode.OFF, "Off", "One exposure per frame. Fastest."), + +class ScanCaptureMode(StrEnum): + """The 4 scan modes this app exposes — a UI-only presentation of pyopticfilm's two real, + orthogonal axes (``multi_exposure_mode`` and ``n_passes``). Never persisted or sent to the + backend directly; ``_capture_mode_from_params``/``_params_from_capture_mode`` translate to + and from the real ``ScanParams``/``ScannerSettings`` fields. pyopticfilm's fixed-long-exposure + ME mode and its manual exposure overrides are lab/debug-only (see Scan Lab) and have no + equivalent here.""" + + SINGLE_PASS = "single_pass" + MULTI_PASS = "multi_pass" + ADAPTIVE_ME = "adaptive_me" + ADAPTIVE_MULTI_PASS = "adaptive_multi_pass" + + +#: Label + explanatory tooltip for each capture mode, in display order. +_CAPTURE_MODE_LABELS: tuple[tuple[ScanCaptureMode, str, str], ...] = ( + (ScanCaptureMode.SINGLE_PASS, "Single-Pass", "One exposure per scan. Fastest, standard quality."), + ( + ScanCaptureMode.MULTI_PASS, + "Multi-Pass", + "Repeats the same exposure and stacks the results to reduce noise. Slower; best for a " + "single, well-exposed frame that just needs less noise.", + ), ( - MultiExposureMode.ADAPTIVE, - "Adaptive", - "Short and long exposures merged; the long exposure is picked automatically per frame from image content.", + ScanCaptureMode.ADAPTIVE_ME, + "Adaptive Multi-Exposure", + "Automatically captures a short and long exposure and fuses them for extended dynamic " + "range. No stacking.", ), ( - MultiExposureMode.FIXED, - "Fixed", - "Short and long exposures merged; the long exposure is pinned to a fixed, validated value instead of chosen per frame.", + ScanCaptureMode.ADAPTIVE_MULTI_PASS, + "Adaptive Multi-Pass", + "Combines adaptive dual-exposure fusion with multi-pass stacking for maximum dynamic " + "range and noise reduction. Slowest option.", ), ) +_ME_CAPTURE_MODES = (ScanCaptureMode.ADAPTIVE_ME, ScanCaptureMode.ADAPTIVE_MULTI_PASS) +_STACKING_CAPTURE_MODES = (ScanCaptureMode.MULTI_PASS, ScanCaptureMode.ADAPTIVE_MULTI_PASS) + +#: Passes slider bounds. Mirrored (not imported) from pyopticfilm's own bound, matching this +#: file's existing convention for the same reason (see params.py's MAX_N_PASSES comment) — the +#: floor is 2 here specifically because 1 pass is "not stacking", represented by mode choice. +MIN_PASSES_UI = 2 +MAX_PASSES_UI = 9 + + +def _capture_mode_from_params(mode: MultiExposureMode, n_passes: int) -> ScanCaptureMode: + me = mode == MultiExposureMode.ADAPTIVE + stacking = n_passes > 1 + if me: + return ScanCaptureMode.ADAPTIVE_MULTI_PASS if stacking else ScanCaptureMode.ADAPTIVE_ME + return ScanCaptureMode.MULTI_PASS if stacking else ScanCaptureMode.SINGLE_PASS + + +def _params_from_capture_mode( + capture_mode: ScanCaptureMode, slider_value: int +) -> tuple[MultiExposureMode, int]: + mode = MultiExposureMode.ADAPTIVE if capture_mode in _ME_CAPTURE_MODES else MultiExposureMode.OFF + n_passes = slider_value if capture_mode in _STACKING_CAPTURE_MODES else 1 + return mode, n_passes + + +def _valid_capture_mode(desired: ScanCaptureMode, *, has_me: bool, has_stack: bool) -> ScanCaptureMode: + """``desired`` masked against what the device actually supports — drops just the axis + (ME or stacking) the device lacks, rather than falling all the way back to Single-Pass + unless neither axis is available.""" + want_me = desired in _ME_CAPTURE_MODES and has_me + want_stack = desired in _STACKING_CAPTURE_MODES and has_stack + if want_me and want_stack: + return ScanCaptureMode.ADAPTIVE_MULTI_PASS + if want_me: + return ScanCaptureMode.ADAPTIVE_ME + if want_stack: + return ScanCaptureMode.MULTI_PASS + return ScanCaptureMode.SINGLE_PASS + _SAMPLE_COUNTS = (1, 2, 4, 8, 16) @@ -241,29 +305,41 @@ def _init_ui(self) -> None: self.form.addRow(self.clean_check) self.clean_check.setVisible(False) - self.me_combo = QComboBox() - for mode, label, tooltip in _ME_MODE_LABELS: - self.me_combo.addItem(label, mode.value) - self.me_combo.setItemData(self.me_combo.count() - 1, tooltip, Qt.ItemDataRole.ToolTipRole) - self.me_combo.setToolTip( - "Merge a short and long exposure per frame for extra shadow/highlight detail. " - "Slower than a single exposure." + self.mode_combo = QComboBox() + for mode, label, tooltip in _CAPTURE_MODE_LABELS: + self.mode_combo.addItem(label, mode.value) + self.mode_combo.setItemData(self.mode_combo.count() - 1, tooltip, Qt.ItemDataRole.ToolTipRole) + self.mode_combo.setToolTip( + "How this scan captures exposure: a single pass, repeated passes stacked for lower " + "noise, an adaptive short+long fusion, or both combined." ) - self.me_label = QLabel("Multi-exposure") - self.form.addRow(self.me_label, self.me_combo) - - # Independent of Multi-exposure above: repeats whichever exposure(s) are chosen (one, - # or the short+long pair) and stacks them for lower noise — "Multi-Pass" alone with - # Multi-exposure Off, or "Adaptive/Fixed Multi-Pass" combined with it. - self.passes_combo = QComboBox() - self.passes_combo.setToolTip( - "Repeat and stack each exposure for lower noise. Each extra pass adds roughly " - "one more scan pass per exposure." + self.mode_label = QLabel("Scan mode") + self.form.addRow(self.mode_label, self.mode_combo) + + # Independent axis folded into the combo above: only meaningful (and only shown) for + # the two stacking modes (Multi-Pass / Adaptive Multi-Pass). + self.passes_row_widget = QWidget() + passes_row = QHBoxLayout(self.passes_row_widget) + passes_row.setContentsMargins(0, 0, 0, 0) + passes_row.setSpacing(6) + self.passes_slider = QSlider(Qt.Orientation.Horizontal) + self.passes_slider.setRange(MIN_PASSES_UI, MAX_PASSES_UI) + self.passes_slider.setSingleStep(1) + self.passes_slider.setPageStep(1) + self.passes_slider.setTickPosition(QSlider.TickPosition.TicksBelow) + self.passes_slider.setTickInterval(1) + self.passes_slider.setToolTip( + f"Number of exposures to stack ({MIN_PASSES_UI}-{MAX_PASSES_UI}). Each extra pass " + "adds roughly one more scan pass per exposure." ) + self.passes_value_label = QLabel(str(MIN_PASSES_UI)) + self.passes_value_label.setMinimumWidth(20) + passes_row.addWidget(self.passes_slider, 1) + passes_row.addWidget(self.passes_value_label) self.passes_label = QLabel("Passes") - self.form.addRow(self.passes_label, self.passes_combo) + self.form.addRow(self.passes_label, self.passes_row_widget) self.passes_label.setVisible(False) - self.passes_combo.setVisible(False) + self.passes_row_widget.setVisible(False) self.superfine_check = QCheckBox("Superfine") self.superfine_check.setToolTip("Read one line per pass: slower, and free of line registration") @@ -427,9 +503,9 @@ def _connect_signals(self) -> None: self.fmt_combo.currentTextChanged.connect(lambda: self._update_settings_from_ui()) self.dpi_combo.currentTextChanged.connect(lambda: self._update_settings_from_ui()) self.depth_combo.currentTextChanged.connect(lambda: self._update_settings_from_ui()) - self.ir_check.toggled.connect(lambda on: self._on_ir_pass_toggled(self.clean_check, on)) - self.me_combo.currentIndexChanged.connect(lambda: self._update_settings_from_ui()) - self.passes_combo.currentIndexChanged.connect(self._on_passes_changed) + self.ir_check.toggled.connect(self._on_ir_toggled) + self.mode_combo.currentIndexChanged.connect(self._on_mode_changed) + self.passes_slider.valueChanged.connect(self._on_passes_slider_changed) self.autofocus_check.toggled.connect(lambda: self._update_settings_from_ui()) self.ae_check.toggled.connect(lambda: self._on_ae_toggled()) self.clean_check.toggled.connect(lambda on: self._on_ir_pass_toggled(self.ir_check, on)) @@ -544,12 +620,12 @@ def _update_device_caps(self) -> None: self.depth_combo.setVisible(False) self.depth_label.setVisible(False) self.ir_check.setVisible(False) - self.me_label.setVisible(False) - self.me_combo.setVisible(False) + self.mode_label.setVisible(False) + self.mode_combo.setVisible(False) self.passes_label.setVisible(False) - self.passes_combo.setVisible(False) + self.passes_row_widget.setVisible(False) self.ir_check.setEnabled(False) - self.me_combo.setEnabled(False) + self.mode_combo.setEnabled(False) self.eject_btn.setVisible(False) self.frame_spec_label.setVisible(False) self.frame_spec_edit.setVisible(False) @@ -587,7 +663,7 @@ def _update_device_caps(self) -> None: self.dpi_combo.setEnabled(True) self.depth_combo.setEnabled(True) self.ir_check.setEnabled(True) - self.me_combo.setEnabled(True) + self.mode_combo.setEnabled(True) self.eject_btn.setVisible(caps.can_eject) self.eject_btn.setEnabled(caps.can_eject and not self._scanning) self.frame_label.setText(f"Frame: {caps.max_area_mm[0]:.0f} × {caps.max_area_mm[1]:.0f} mm") @@ -608,8 +684,8 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: self.dpi_combo.blockSignals(True) self.depth_combo.blockSignals(True) self.ir_check.blockSignals(True) - self.me_combo.blockSignals(True) - self.passes_combo.blockSignals(True) + self.mode_combo.blockSignals(True) + self.passes_slider.blockSignals(True) self.ae_check.blockSignals(True) self.frame_spec_edit.blockSignals(True) @@ -660,29 +736,42 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: self.ir_check.setChecked(False) self.ir_check.setToolTip("IR scanning not supported by this device") - # Multi-exposure (Plustek GL128 scan-ready models) - self.me_label.setVisible(bool(caps.multi_exposure)) - self.me_combo.setVisible(bool(caps.multi_exposure)) - self.me_combo.setEnabled(caps.multi_exposure) - if caps.multi_exposure: - idx = self.me_combo.findData(self._settings.multi_exposure_mode) - self.me_combo.setCurrentIndex(idx if idx >= 0 else 0) - else: - self.me_combo.setCurrentIndex(0) - - # Passes (same-exposure repeat stacking) — independent of Multi-exposure above, so its - # own row, gated on max_n_passes alone. + # Scan mode (Plustek GL128 scan-ready models): one combo presenting pyopticfilm's two + # real orthogonal axes (multi_exposure_mode, n_passes) as 4 named options. Per-item + # capability gating below; the whole row hides when neither ME nor Multi-Pass applies — + # a device with neither has nothing to choose, mode is implicitly Single-Pass. self._caps_max_n_passes = int(caps.max_n_passes) - self.passes_combo.clear() - show_passes = caps.max_n_passes > 1 - if show_passes: - for n in range(MIN_N_PASSES, caps.max_n_passes + 1): - self.passes_combo.addItem(str(n), n) - idx = self.passes_combo.findData(self._settings.n_passes) - self.passes_combo.setCurrentIndex(max(idx, 0)) - self.passes_label.setVisible(show_passes) - self.passes_combo.setVisible(show_passes) - self._refresh_ir_passes_conflict() + show_mode = bool(caps.multi_exposure) or caps.max_n_passes > 1 + self.mode_label.setVisible(show_mode) + self.mode_combo.setVisible(show_mode) + self.mode_combo.setEnabled(show_mode) + for capture_mode, _label, _tooltip in _CAPTURE_MODE_LABELS: + idx = self.mode_combo.findData(capture_mode.value) + enabled = ( + True + if capture_mode == ScanCaptureMode.SINGLE_PASS + else bool(caps.multi_exposure) if capture_mode == ScanCaptureMode.ADAPTIVE_ME + else caps.max_n_passes > 1 if capture_mode == ScanCaptureMode.MULTI_PASS + else bool(caps.multi_exposure) and caps.max_n_passes > 1 # ADAPTIVE_MULTI_PASS + ) + item = self.mode_combo.model().item(idx) + if item is not None: + item.setEnabled(enabled) + saved_mode = _capture_mode_from_params( + MultiExposureMode(self._settings.multi_exposure_mode) + if self._settings.multi_exposure_mode in set(MultiExposureMode) + else MultiExposureMode.OFF, + self._settings.n_passes, + ) + self._set_capture_mode( + _valid_capture_mode(saved_mode, has_me=bool(caps.multi_exposure), has_stack=caps.max_n_passes > 1) + ) + self.passes_slider.setRange(MIN_PASSES_UI, max(MIN_PASSES_UI, caps.max_n_passes)) + self.passes_slider.setValue( + min(max(self._settings.n_passes, MIN_PASSES_UI), max(MIN_PASSES_UI, caps.max_n_passes)) + ) + self.passes_value_label.setText(str(self.passes_slider.value())) + self._sync_passes_visibility() # Autofocus and auto-exposure, shown only when the device reports them. self._caps_autofocus = bool(caps.autofocus) @@ -816,8 +905,8 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: self.dpi_combo.blockSignals(False) self.depth_combo.blockSignals(False) self.ir_check.blockSignals(False) - self.me_combo.blockSignals(False) - self.passes_combo.blockSignals(False) + self.mode_combo.blockSignals(False) + self.passes_slider.blockSignals(False) self.ae_check.blockSignals(False) self.frame_spec_edit.blockSignals(False) @@ -851,37 +940,52 @@ def _on_ir_pass_toggled(self, other: QCheckBox, checked: bool) -> None: other.blockSignals(True) other.setChecked(False) other.blockSignals(False) - self._refresh_ir_passes_conflict() self._update_settings_from_ui() - def _refresh_ir_passes_conflict(self) -> None: - """IR and Multi-Pass (Passes > 1) cannot be combined yet — pyopticfilm rejects the - combination outright (each repeat is its own motor cycle; IR stacking is unvalidated). - Reset whichever control was already set rather than merely disabling one, matching the - IR/ICE exclusivity above.""" - if self.ir_check.isChecked() and self._passes() > 1: - self.passes_combo.blockSignals(True) - idx = self.passes_combo.findData(MIN_N_PASSES) - self.passes_combo.setCurrentIndex(idx if idx >= 0 else 0) - self.passes_combo.blockSignals(False) - - def _me_mode(self) -> MultiExposureMode: - if not self.me_combo.isEnabled(): - return MultiExposureMode.OFF - return MultiExposureMode(self.me_combo.currentData() or MultiExposureMode.OFF.value) - - def _passes(self) -> int: - if self._caps_max_n_passes <= 1: - return MIN_N_PASSES - return int(self.passes_combo.currentData() or MIN_N_PASSES) - - def _on_passes_changed(self, _index: int) -> None: - if self._passes() > 1 and self.ir_check.isChecked(): + def _on_ir_toggled(self, checked: bool) -> None: + self._on_ir_pass_toggled(self.clean_check, checked) + if not checked: + return + # IR and Multi-Pass stacking cannot combine yet — pyopticfilm rejects the combination + # outright (each repeat is its own motor cycle; IR stacking is unvalidated). Drop the + # mode to its non-stacking equivalent rather than silently discarding a hidden slider + # value, so the visible mode reflects what actually happens. + mode = self._capture_mode() + if mode == ScanCaptureMode.MULTI_PASS: + self._set_capture_mode(ScanCaptureMode.SINGLE_PASS) + elif mode == ScanCaptureMode.ADAPTIVE_MULTI_PASS: + self._set_capture_mode(ScanCaptureMode.ADAPTIVE_ME) + + def _on_mode_changed(self) -> None: + self._sync_passes_visibility() + if self._capture_mode() in _STACKING_CAPTURE_MODES and self.ir_check.isChecked(): self.ir_check.blockSignals(True) self.ir_check.setChecked(False) self.ir_check.blockSignals(False) self._update_settings_from_ui() + def _sync_passes_visibility(self) -> None: + show = self.mode_combo.isEnabled() and self._capture_mode() in _STACKING_CAPTURE_MODES + self.passes_label.setVisible(show) + self.passes_row_widget.setVisible(show) + + def _set_capture_mode(self, mode: ScanCaptureMode) -> None: + idx = self.mode_combo.findData(mode.value) + if idx >= 0: + self.mode_combo.setCurrentIndex(idx) + + def _capture_mode(self) -> ScanCaptureMode: + if not self.mode_combo.isEnabled(): + return ScanCaptureMode.SINGLE_PASS + return ScanCaptureMode(self.mode_combo.currentData() or ScanCaptureMode.SINGLE_PASS.value) + + def _passes_slider_value(self) -> int: + return self.passes_slider.value() + + def _on_passes_slider_changed(self, value: int) -> None: + self.passes_value_label.setText(str(value)) + self._update_settings_from_ui() + def _samples(self) -> int: if self._caps_max_samples <= 1: return 1 @@ -1108,15 +1212,12 @@ def _update_summary(self) -> None: passes.append("Superfine") if self._samples() > 1: passes.append(f"{self._samples()}× sampled") - me_mode = self._me_mode() - n_passes = self._passes() - me_label = next((label for mode, label, _ in _ME_MODE_LABELS if mode == me_mode), None) - if me_mode != MultiExposureMode.OFF and n_passes > 1: - passes.append(f"{me_label} ×{n_passes} passes") - elif me_mode != MultiExposureMode.OFF: - passes.append(me_label) - elif n_passes > 1: - passes.append(f"Multi-Pass ×{n_passes}") + capture_mode = self._capture_mode() + if capture_mode != ScanCaptureMode.SINGLE_PASS: + mode_label = next(label for mode, label, _ in _CAPTURE_MODE_LABELS if mode == capture_mode) + if capture_mode in _STACKING_CAPTURE_MODES: + mode_label = f"{mode_label} ({self._passes_slider_value()} passes)" + passes.append(mode_label) # The count and the size are what the operator checks before committing, so they carry # primary weight; the rest of the line stays secondary. strong = f'{{}}' @@ -1159,8 +1260,7 @@ def _on_scan(self) -> None: dpi = self._dpi() depth = int(self.depth_combo.currentData() or 16) capture_ir = self.ir_check.isEnabled() and self.ir_check.isChecked() - me_mode = self._me_mode() - n_passes = self._passes() + me_mode, n_passes = _params_from_capture_mode(self._capture_mode(), self._passes_slider_value()) autofocus = self._caps_autofocus and self.autofocus_check.isChecked() auto_exposure = self._caps_auto_exposure and self.ae_check.isChecked() pattern = self.pattern_edit.text().strip() or '{{ date }}_{{ "%03d" % seq }}' @@ -1329,6 +1429,7 @@ def _update_settings_from_ui(self) -> None: from dataclasses import replace device = self._current_device() + me_mode, n_passes = _params_from_capture_mode(self._capture_mode(), self._passes_slider_value()) # replace(), never a fresh ScannerSettings: fields with no sidebar control must survive # UI edits, and reconstruction silently resets any field missing from this list. self.settings = replace( @@ -1338,8 +1439,8 @@ def _update_settings_from_ui(self) -> None: dpi=dpi, depth=depth, capture_ir=self.ir_check.isChecked() and self.ir_check.isEnabled(), - multi_exposure_mode=self._me_mode().value, - n_passes=self._passes(), + multi_exposure_mode=me_mode.value, + n_passes=n_passes, autofocus=self._caps_autofocus and self.autofocus_check.isChecked(), auto_exposure=self._caps_auto_exposure and self.ae_check.isChecked(), exposure_time_us=(self.exposure_slider.value() if self.exposure_row_widget.isVisible() else None), diff --git a/negpy/infrastructure/scanners/params.py b/negpy/infrastructure/scanners/params.py index 365a30f8..4c65153d 100644 --- a/negpy/infrastructure/scanners/params.py +++ b/negpy/infrastructure/scanners/params.py @@ -9,20 +9,19 @@ class ScanMode(StrEnum): class MultiExposureMode(StrEnum): - """How the top exposure is chosen when merging short+long colour passes. + """Whether a short+long colour pass pair is merged for extended dynamic range. OFF: one exposure, the fast path. ADAPTIVE: short+long merged, long exposure picked per - frame from image content (today's only multi-exposure behavior). FIXED: short+long merged, - long exposure pinned to a fixed, per-model-validated value instead of chosen per frame. + frame from image content — today's only multi-exposure behavior in this app (pyopticfilm's + fixed-long-exposure mode is a lab/debug-only option, not exposed here). - Orthogonal to ``ScanParams.n_passes``: this picks *which* exposure(s) to merge, while - ``n_passes`` repeats whichever exposure(s) are chosen for a same-exposure SNR stack — the - two compose (pyopticfilm's "Adaptive/Fixed Multi-Pass") rather than being alternatives. + Orthogonal to ``ScanParams.n_passes``: this picks *whether* to merge a second exposure, + while ``n_passes`` repeats whichever exposure(s) are chosen for a same-exposure SNR stack — + the two compose (pyopticfilm's "Adaptive Multi-Pass") rather than being alternatives. """ OFF = "off" ADAPTIVE = "adaptive" - FIXED = "fixed" #: Repeats of the same exposure to stack for an SNR gain (pyopticfilm's `n_passes`); 1 = no diff --git a/negpy/infrastructure/scanners/plustek_backend.py b/negpy/infrastructure/scanners/plustek_backend.py index aebf4b1c..d93ac579 100644 --- a/negpy/infrastructure/scanners/plustek_backend.py +++ b/negpy/infrastructure/scanners/plustek_backend.py @@ -346,9 +346,11 @@ def _scan_on_scanner( capture_ir = bool(params.capture_ir) me_mode = params.multi_exposure_mode multi_exposure = me_mode != MultiExposureMode.OFF - # Irrelevant to pyopticfilm when multi_exposure is False; pass its own default rather - # than a value that would misleadingly suggest ME is fixed-mode when it's simply off. - me_exposure_mode = "fixed" if me_mode == MultiExposureMode.FIXED else "adaptive" + # "adaptive" is pyopticfilm's own default and the only mode this app exposes — its + # fixed-long-exposure mode is lab/debug-only (see pyopticfilm's Scan Lab). Irrelevant to + # pyopticfilm when multi_exposure is False; sending its default rather than omitting it + # keeps this call shape uniform regardless of mode. + me_exposure_mode = "adaptive" n_passes = int(params.n_passes) window = params.window geometry = self._default_scan_geometry(scanner, dpi=dpi, window=window) diff --git a/tests/scanners/test_plustek_backend.py b/tests/scanners/test_plustek_backend.py index 54a55df4..5675bda9 100644 --- a/tests/scanners/test_plustek_backend.py +++ b/tests/scanners/test_plustek_backend.py @@ -380,18 +380,10 @@ def test_off_mode_defaults_exposure_mode_to_adaptive(monkeypatch): assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" -def test_fixed_mode_pins_exposure_mode(monkeypatch): - _patch_enum(monkeypatch) - scanner = _fake_scanner() - monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) - PlustekBackend().scan( - _DEVICE_ID, - _params(multi_exposure_mode=MultiExposureMode.FIXED), - lambda *_: None, - threading.Event(), - ) - assert scanner.scan.call_args.kwargs.get("multi_exposure") is True - assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "fixed" +def test_multi_exposure_mode_has_no_fixed_option(): + """Fixed-long-exposure mode is a pyopticfilm lab/debug-only concept — NegPy's simplified + surface never exposes it (see Scan Lab for unrestricted access).""" + assert set(MultiExposureMode) == {MultiExposureMode.OFF, MultiExposureMode.ADAPTIVE} def test_n_passes_flows_through_to_scanner_scan(monkeypatch): diff --git a/tests/scanners/test_scanner_settings.py b/tests/scanners/test_scanner_settings.py index 9d8c49b9..fd695be7 100644 --- a/tests/scanners/test_scanner_settings.py +++ b/tests/scanners/test_scanner_settings.py @@ -152,8 +152,8 @@ def test_legacy_multi_exposure_false_migrates_to_off_mode(): def test_a_blob_with_multi_exposure_mode_already_set_is_not_double_migrated(): - restored = ScannerSettings.from_dict({"multi_exposure": True, "multi_exposure_mode": "fixed"}) - assert restored.multi_exposure_mode == "fixed" + restored = ScannerSettings.from_dict({"multi_exposure": True, "multi_exposure_mode": "adaptive"}) + assert restored.multi_exposure_mode == "adaptive" def test_an_unknown_multi_exposure_mode_falls_back_to_off(): @@ -161,6 +161,15 @@ def test_an_unknown_multi_exposure_mode_falls_back_to_off(): assert restored.multi_exposure_mode == "off" +def test_legacy_fixed_mode_degrades_to_off(): + """Fixed-long-exposure mode was dropped from NegPy's surface (lab/debug-only in + pyopticfilm now) — a persisted "fixed" blob from before this change must degrade safely, + not crash, and must not touch the independent n_passes value.""" + restored = ScannerSettings.from_dict({"multi_exposure_mode": "fixed", "n_passes": 3}) + assert restored.multi_exposure_mode == "off" + assert restored.n_passes == 3 + + def test_fresh_install_defaults_n_passes_to_one(): assert ScannerSettings.defaults().n_passes == 1 diff --git a/tests/test_scan_sidebar.py b/tests/test_scan_sidebar.py index 03c815a4..c4fe52ac 100644 --- a/tests/test_scan_sidebar.py +++ b/tests/test_scan_sidebar.py @@ -20,14 +20,14 @@ from types import SimpleNamespace import pytest -from PyQt6.QtCore import QObject, pyqtSignal +from PyQt6.QtCore import QObject, Qt, pyqtSignal from PyQt6.QtGui import QValidator from PyQt6.QtWidgets import QApplication -from negpy.desktop.view.sidebar.scan import ScanSidebar, estimated_frame_bytes +from negpy.desktop.view.sidebar.scan import ScanCaptureMode, ScanSidebar, estimated_frame_bytes from negpy.desktop.view.styles.theme import THEME from negpy.infrastructure.scanners.base import ScannerCapabilities, ScannerDevice -from negpy.infrastructure.scanners.params import FILM_TYPES, MultiExposureMode, ScanMode +from negpy.infrastructure.scanners.params import FILM_TYPES, ScanMode if not QApplication.instance(): _app = QApplication(sys.argv) @@ -225,7 +225,7 @@ def test_se_device_shows_prescan() -> None: assert sidebar.prescan_widget.isVisibleTo(sidebar) is True assert sidebar.scan_window_widget.isVisibleTo(sidebar) is False assert sidebar.ir_check.isEnabled() is True - assert sidebar.me_combo.isEnabled() is True + assert sidebar.mode_combo.isEnabled() is True assert sidebar.frame_spec_edit.isVisibleTo(sidebar) is False @@ -246,29 +246,55 @@ def test_sane_backend_keeps_single_holder_window_control(monkeypatch) -> None: def test_minimal_device_disables_multi_exposure() -> None: sidebar, _ = _sidebar(MINIMAL_DEVICE) - assert sidebar.me_combo.isEnabled() is False - assert sidebar._me_mode() == MultiExposureMode.OFF + assert sidebar.mode_combo.isEnabled() is False + assert sidebar._capture_mode() == ScanCaptureMode.SINGLE_PASS def test_minimal_device_hides_passes_control() -> None: sidebar, _ = _sidebar(MINIMAL_DEVICE) - assert sidebar.passes_combo.isVisibleTo(sidebar) is False - assert sidebar._passes() == 1 + assert sidebar.passes_row_widget.isVisibleTo(sidebar) is False -def test_passes_above_one_disables_and_unchecks_ir() -> None: +def test_only_single_pass_enabled_when_device_has_neither_capability() -> None: + sidebar, _ = _sidebar(MINIMAL_DEVICE) + enabled = { + ScanCaptureMode(sidebar.mode_combo.itemData(i)): sidebar.mode_combo.model().item(i).isEnabled() + for i in range(sidebar.mode_combo.count()) + } + assert enabled[ScanCaptureMode.SINGLE_PASS] is True + assert enabled[ScanCaptureMode.MULTI_PASS] is False + assert enabled[ScanCaptureMode.ADAPTIVE_ME] is False + assert enabled[ScanCaptureMode.ADAPTIVE_MULTI_PASS] is False + + +def test_mode_combo_tooltips_present() -> None: + sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek"}) + for i in range(sidebar.mode_combo.count()): + tooltip = sidebar.mode_combo.itemData(i, Qt.ItemDataRole.ToolTipRole) + assert isinstance(tooltip, str) and tooltip.strip() + + +def test_selecting_multi_pass_reveals_passes_slider_and_unchecks_ir() -> None: sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek", "capture_ir": True}) - idx = sidebar.passes_combo.findData(3) - assert idx >= 0 - sidebar.passes_combo.setCurrentIndex(idx) + sidebar._set_capture_mode(ScanCaptureMode.MULTI_PASS) + assert sidebar.passes_row_widget.isVisibleTo(sidebar) is True assert sidebar.ir_check.isChecked() is False -def test_checking_ir_resets_passes_to_one() -> None: +def test_ir_toggle_drops_multi_pass_to_single_pass() -> None: + sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek", "n_passes": 3}) + sidebar._set_capture_mode(ScanCaptureMode.MULTI_PASS) + sidebar.ir_check.setChecked(True) + assert sidebar._capture_mode() == ScanCaptureMode.SINGLE_PASS + assert sidebar.ir_check.isChecked() is True + + +def test_ir_toggle_drops_adaptive_multi_pass_to_adaptive_me() -> None: sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek", "n_passes": 3}) - assert sidebar._passes() == 3 + sidebar._set_capture_mode(ScanCaptureMode.ADAPTIVE_MULTI_PASS) sidebar.ir_check.setChecked(True) - assert sidebar._passes() == 1 + assert sidebar._capture_mode() == ScanCaptureMode.ADAPTIVE_ME + assert sidebar.ir_check.isChecked() is True def test_scan_params_include_prescan_crop() -> None: @@ -1006,11 +1032,11 @@ def test_a_pass_the_device_cannot_run_is_not_shown_at_all() -> None: # Disabled-with-a-reason is for a pass the film blocks; one the transport lacks goes away. sidebar, _ = _sidebar(MINIMAL_DEVICE) assert sidebar.ir_check.isVisibleTo(sidebar) is False - assert sidebar.me_combo.isVisibleTo(sidebar) is False + assert sidebar.mode_combo.isVisibleTo(sidebar) is False sidebar, _ = _sidebar(SE_DEVICE, settings={"backend": "plustek"}) assert sidebar.ir_check.isVisibleTo(sidebar) is True - assert sidebar.me_combo.isVisibleTo(sidebar) is True + assert sidebar.mode_combo.isVisibleTo(sidebar) is True def test_a_film_that_blocks_infrared_leaves_the_control_visible_to_explain_itself() -> None: From b322534d2e769ff6bd0868c396cc8a22ef5d5121 Mon Sep 17 00:00:00 2001 From: Tobby Dallnett Date: Wed, 9 Sep 2026 00:36:12 +0800 Subject: [PATCH 3/5] fix(plustek): drop removed me_exposure_mode kwarg, self-heal IR/stacking conflict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pyopticfilm dropped its "fixed" ME exposure mode (never exposed in this app's UI) and the me_exposure_mode parameter along with it — remove the now-nonexistent kwarg from the Scanner.scan() call. Also self-heal an IR + Multi-Pass stacking conflict at settings-load time the same way the live toggle handlers already do, instead of only catching it when the user touches a control (a settings blob saved with both set would otherwise reach Scan and fail there). Replace the locally mirrored MAX_PASSES_UI constant with an import of pyopticfilm's own MAX_N_PASSES, and add a test for the n_passes=9 upper boundary. Claude-Session: https://claude.ai/code/session_01KThZJVYa894isy2k8aZXtK --- negpy/desktop/view/sidebar/scan.py | 51 +++++++++---------- negpy/infrastructure/scanners/params.py | 3 +- .../scanners/plustek_backend.py | 6 --- tests/scanners/test_plustek_backend.py | 30 ++++++----- 4 files changed, 42 insertions(+), 48 deletions(-) diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index 5e399f25..eef5355c 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -26,6 +26,7 @@ from negpy.infrastructure.scanners.params import ( FILM_TYPES, FilmType, + MAX_N_PASSES, MultiExposureMode, film_passes_infrared, ) @@ -35,11 +36,10 @@ class ScanCaptureMode(StrEnum): """The 4 scan modes this app exposes — a UI-only presentation of pyopticfilm's two real, - orthogonal axes (``multi_exposure_mode`` and ``n_passes``). Never persisted or sent to the + orthogonal axes (``multi_exposure`` and ``n_passes``). Never persisted or sent to the backend directly; ``_capture_mode_from_params``/``_params_from_capture_mode`` translate to - and from the real ``ScanParams``/``ScannerSettings`` fields. pyopticfilm's fixed-long-exposure - ME mode and its manual exposure overrides are lab/debug-only (see Scan Lab) and have no - equivalent here.""" + and from the real ``ScanParams``/``ScannerSettings`` fields. pyopticfilm's manual exposure + overrides are lab/debug-only (see Scan Lab) and have no equivalent here.""" SINGLE_PASS = "single_pass" MULTI_PASS = "multi_pass" @@ -59,25 +59,23 @@ class ScanCaptureMode(StrEnum): ( ScanCaptureMode.ADAPTIVE_ME, "Adaptive Multi-Exposure", - "Automatically captures a short and long exposure and fuses them for extended dynamic " - "range. No stacking.", + "Automatically captures a short and long exposure and fuses them for extended dynamic range. No stacking.", ), ( ScanCaptureMode.ADAPTIVE_MULTI_PASS, "Adaptive Multi-Pass", - "Combines adaptive dual-exposure fusion with multi-pass stacking for maximum dynamic " - "range and noise reduction. Slowest option.", + "Combines adaptive dual-exposure fusion with multi-pass stacking for maximum dynamic range and noise reduction. Slowest option.", ), ) _ME_CAPTURE_MODES = (ScanCaptureMode.ADAPTIVE_ME, ScanCaptureMode.ADAPTIVE_MULTI_PASS) _STACKING_CAPTURE_MODES = (ScanCaptureMode.MULTI_PASS, ScanCaptureMode.ADAPTIVE_MULTI_PASS) -#: Passes slider bounds. Mirrored (not imported) from pyopticfilm's own bound, matching this -#: file's existing convention for the same reason (see params.py's MAX_N_PASSES comment) — the -#: floor is 2 here specifically because 1 pass is "not stacking", represented by mode choice. +#: Passes slider floor. UI-only — 1 pass is "not stacking", represented by mode choice, so the +#: slider (shown only for a stacking mode) never needs to reach it. The ceiling is +#: pyopticfilm's own ``MAX_N_PASSES``, imported directly since this file already reaches into +#: params.py for other names. MIN_PASSES_UI = 2 -MAX_PASSES_UI = 9 def _capture_mode_from_params(mode: MultiExposureMode, n_passes: int) -> ScanCaptureMode: @@ -88,9 +86,7 @@ def _capture_mode_from_params(mode: MultiExposureMode, n_passes: int) -> ScanCap return ScanCaptureMode.MULTI_PASS if stacking else ScanCaptureMode.SINGLE_PASS -def _params_from_capture_mode( - capture_mode: ScanCaptureMode, slider_value: int -) -> tuple[MultiExposureMode, int]: +def _params_from_capture_mode(capture_mode: ScanCaptureMode, slider_value: int) -> tuple[MultiExposureMode, int]: mode = MultiExposureMode.ADAPTIVE if capture_mode in _ME_CAPTURE_MODES else MultiExposureMode.OFF n_passes = slider_value if capture_mode in _STACKING_CAPTURE_MODES else 1 return mode, n_passes @@ -323,14 +319,13 @@ def _init_ui(self) -> None: passes_row.setContentsMargins(0, 0, 0, 0) passes_row.setSpacing(6) self.passes_slider = QSlider(Qt.Orientation.Horizontal) - self.passes_slider.setRange(MIN_PASSES_UI, MAX_PASSES_UI) + self.passes_slider.setRange(MIN_PASSES_UI, MAX_N_PASSES) self.passes_slider.setSingleStep(1) self.passes_slider.setPageStep(1) self.passes_slider.setTickPosition(QSlider.TickPosition.TicksBelow) self.passes_slider.setTickInterval(1) self.passes_slider.setToolTip( - f"Number of exposures to stack ({MIN_PASSES_UI}-{MAX_PASSES_UI}). Each extra pass " - "adds roughly one more scan pass per exposure." + f"Number of exposures to stack ({MIN_PASSES_UI}-{MAX_N_PASSES}). Each extra pass adds roughly one more scan pass per exposure." ) self.passes_value_label = QLabel(str(MIN_PASSES_UI)) self.passes_value_label.setMinimumWidth(20) @@ -750,8 +745,10 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: enabled = ( True if capture_mode == ScanCaptureMode.SINGLE_PASS - else bool(caps.multi_exposure) if capture_mode == ScanCaptureMode.ADAPTIVE_ME - else caps.max_n_passes > 1 if capture_mode == ScanCaptureMode.MULTI_PASS + else bool(caps.multi_exposure) + if capture_mode == ScanCaptureMode.ADAPTIVE_ME + else caps.max_n_passes > 1 + if capture_mode == ScanCaptureMode.MULTI_PASS else bool(caps.multi_exposure) and caps.max_n_passes > 1 # ADAPTIVE_MULTI_PASS ) item = self.mode_combo.model().item(idx) @@ -763,13 +760,15 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: else MultiExposureMode.OFF, self._settings.n_passes, ) - self._set_capture_mode( - _valid_capture_mode(saved_mode, has_me=bool(caps.multi_exposure), has_stack=caps.max_n_passes > 1) - ) + self._set_capture_mode(_valid_capture_mode(saved_mode, has_me=bool(caps.multi_exposure), has_stack=caps.max_n_passes > 1)) + # IR and Multi-Pass stacking cannot combine (see _on_ir_toggled) — a settings blob + # saved with both set (signals are blocked through this whole method, so the toggle + # handlers that normally resolve this never fire) must self-heal here the same way, + # rather than reaching Scan and failing there. + if self._capture_mode() in _STACKING_CAPTURE_MODES and self.ir_check.isChecked(): + self.ir_check.setChecked(False) self.passes_slider.setRange(MIN_PASSES_UI, max(MIN_PASSES_UI, caps.max_n_passes)) - self.passes_slider.setValue( - min(max(self._settings.n_passes, MIN_PASSES_UI), max(MIN_PASSES_UI, caps.max_n_passes)) - ) + self.passes_slider.setValue(min(max(self._settings.n_passes, MIN_PASSES_UI), max(MIN_PASSES_UI, caps.max_n_passes))) self.passes_value_label.setText(str(self.passes_slider.value())) self._sync_passes_visibility() diff --git a/negpy/infrastructure/scanners/params.py b/negpy/infrastructure/scanners/params.py index 4c65153d..fa353485 100644 --- a/negpy/infrastructure/scanners/params.py +++ b/negpy/infrastructure/scanners/params.py @@ -12,8 +12,7 @@ class MultiExposureMode(StrEnum): """Whether a short+long colour pass pair is merged for extended dynamic range. OFF: one exposure, the fast path. ADAPTIVE: short+long merged, long exposure picked per - frame from image content — today's only multi-exposure behavior in this app (pyopticfilm's - fixed-long-exposure mode is a lab/debug-only option, not exposed here). + frame from image content — pyopticfilm's only multi-exposure behavior. Orthogonal to ``ScanParams.n_passes``: this picks *whether* to merge a second exposure, while ``n_passes`` repeats whichever exposure(s) are chosen for a same-exposure SNR stack — diff --git a/negpy/infrastructure/scanners/plustek_backend.py b/negpy/infrastructure/scanners/plustek_backend.py index d93ac579..30d1e013 100644 --- a/negpy/infrastructure/scanners/plustek_backend.py +++ b/negpy/infrastructure/scanners/plustek_backend.py @@ -346,11 +346,6 @@ def _scan_on_scanner( capture_ir = bool(params.capture_ir) me_mode = params.multi_exposure_mode multi_exposure = me_mode != MultiExposureMode.OFF - # "adaptive" is pyopticfilm's own default and the only mode this app exposes — its - # fixed-long-exposure mode is lab/debug-only (see pyopticfilm's Scan Lab). Irrelevant to - # pyopticfilm when multi_exposure is False; sending its default rather than omitting it - # keeps this call shape uniform regardless of mode. - me_exposure_mode = "adaptive" n_passes = int(params.n_passes) window = params.window geometry = self._default_scan_geometry(scanner, dpi=dpi, window=window) @@ -394,7 +389,6 @@ def on_status(status: str) -> None: on_status=on_status, multi_exposure=multi_exposure, infrared=capture_ir, - me_exposure_mode=me_exposure_mode, align_passes=True, n_passes=n_passes, ) diff --git a/tests/scanners/test_plustek_backend.py b/tests/scanners/test_plustek_backend.py index 5675bda9..fe7815db 100644 --- a/tests/scanners/test_plustek_backend.py +++ b/tests/scanners/test_plustek_backend.py @@ -350,7 +350,6 @@ def test_multi_exposure_passthrough(monkeypatch): threading.Event(), ) assert scanner.scan.call_args.kwargs.get("multi_exposure") is True - assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" assert scanner.scan.call_args.kwargs.get("align_passes") is True assert scanner.scan.call_args.kwargs.get("n_passes") == 1 @@ -365,21 +364,10 @@ def test_colour_scan_passes_adaptive_me_mode(monkeypatch): lambda *_: None, threading.Event(), ) - assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" + assert scanner.scan.call_args.kwargs.get("multi_exposure") is True assert scanner.scan.call_args.kwargs.get("on_status") is not None -def test_off_mode_defaults_exposure_mode_to_adaptive(monkeypatch): - """OFF is a no-op path — me_exposure_mode is irrelevant to pyopticfilm when - multi_exposure is False, so the value passed just needs to be pyopticfilm's own default.""" - _patch_enum(monkeypatch) - scanner = _fake_scanner() - monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) - PlustekBackend().scan(_DEVICE_ID, _params(), lambda *_: None, threading.Event()) - assert scanner.scan.call_args.kwargs.get("multi_exposure") is False - assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" - - def test_multi_exposure_mode_has_no_fixed_option(): """Fixed-long-exposure mode is a pyopticfilm lab/debug-only concept — NegPy's simplified surface never exposes it (see Scan Lab for unrestricted access).""" @@ -401,6 +389,21 @@ def test_n_passes_flows_through_to_scanner_scan(monkeypatch): assert scanner.scan.call_args.kwargs.get("align_passes") is True +def test_n_passes_upper_boundary_accepted(monkeypatch): + """9 is the top of the valid range (10 is rejected — see + test_n_passes_rejects_out_of_range_value) and must actually reach scanner.scan.""" + _patch_enum(monkeypatch) + scanner = _fake_scanner() + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + PlustekBackend().scan( + _DEVICE_ID, + _params(n_passes=9), + lambda *_: None, + threading.Event(), + ) + assert scanner.scan.call_args.kwargs.get("n_passes") == 9 + + def test_adaptive_multi_pass_passes_both_axes(monkeypatch): """multi_exposure_mode and n_passes are independent — both reach scanner.scan together.""" _patch_enum(monkeypatch) @@ -413,7 +416,6 @@ def test_adaptive_multi_pass_passes_both_axes(monkeypatch): threading.Event(), ) assert scanner.scan.call_args.kwargs.get("multi_exposure") is True - assert scanner.scan.call_args.kwargs.get("me_exposure_mode") == "adaptive" assert scanner.scan.call_args.kwargs.get("n_passes") == 4 From 05c6f3be0c3fcda494277fa38f2451c95cef449c Mon Sep 17 00:00:00 2001 From: Tobby Dallnett Date: Thu, 10 Sep 2026 09:38:01 +0800 Subject: [PATCH 4/5] fix(plustek): drop dead caps_max_n_passes store, wire DEFAULT_N_PASSES self._caps_max_n_passes was written in three places but never read; every live site already passes caps.max_n_passes straight through. First-time Multi-Pass selection was landing users on the slider floor (2) instead of the intended default (3). Claude-Session: https://claude.ai/code/session_01GTVvLh5UfczCAeTKFHzny6 --- negpy/desktop/view/sidebar/scan.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index eef5355c..c36a94f2 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -24,9 +24,11 @@ from negpy.desktop.view.styles.theme import THEME from negpy.infrastructure.scanners.base import ScannerCapabilities, ScannerDevice from negpy.infrastructure.scanners.params import ( + DEFAULT_N_PASSES, FILM_TYPES, FilmType, MAX_N_PASSES, + MIN_N_PASSES, MultiExposureMode, film_passes_infrared, ) @@ -152,7 +154,6 @@ def __init__(self, controller) -> None: self._caps_clean = False self._caps_superfine = False self._caps_max_samples = 1 - self._caps_max_n_passes = 1 self._caps_film_formats: tuple[str, ...] = () self._caps_film_types: tuple[str, ...] = () self._device_ir = False @@ -646,7 +647,6 @@ def _update_device_caps(self) -> None: self._caps_clean = False self._caps_superfine = False self._caps_max_samples = 1 - self._caps_max_n_passes = 1 self._caps_film_formats = () self._caps_film_types = () self._device_ir = False @@ -735,7 +735,6 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: # real orthogonal axes (multi_exposure_mode, n_passes) as 4 named options. Per-item # capability gating below; the whole row hides when neither ME nor Multi-Pass applies — # a device with neither has nothing to choose, mode is implicitly Single-Pass. - self._caps_max_n_passes = int(caps.max_n_passes) show_mode = bool(caps.multi_exposure) or caps.max_n_passes > 1 self.mode_label.setVisible(show_mode) self.mode_combo.setVisible(show_mode) @@ -767,8 +766,10 @@ def _populate_form(self, caps: ScannerCapabilities) -> None: # rather than reaching Scan and failing there. if self._capture_mode() in _STACKING_CAPTURE_MODES and self.ir_check.isChecked(): self.ir_check.setChecked(False) - self.passes_slider.setRange(MIN_PASSES_UI, max(MIN_PASSES_UI, caps.max_n_passes)) - self.passes_slider.setValue(min(max(self._settings.n_passes, MIN_PASSES_UI), max(MIN_PASSES_UI, caps.max_n_passes))) + ceiling = max(MIN_PASSES_UI, caps.max_n_passes) + starting_passes = DEFAULT_N_PASSES if self._settings.n_passes <= MIN_N_PASSES else self._settings.n_passes + self.passes_slider.setRange(MIN_PASSES_UI, ceiling) + self.passes_slider.setValue(min(max(starting_passes, MIN_PASSES_UI), ceiling)) self.passes_value_label.setText(str(self.passes_slider.value())) self._sync_passes_visibility() From 8c7f25792e1d62a851f4daa812322557dab380a5 Mon Sep 17 00:00:00 2001 From: Tobby Dallnett Date: Sun, 13 Sep 2026 09:34:48 +0800 Subject: [PATCH 5/5] fix: address code review findings from the merge resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - scan.py: the merge silently kept two "Framing" sections — the one this branch already relocated to a full-width block at the bottom of the panel, and upstream's re-styled but still in-form original, since the two edits didn't textually overlap. Deletes the reintroduced in-form duplicate and ports upstream's styling (uppercase header, capitalized placeholder, labeled_action buttons) onto the surviving bottom-of-panel block. - plustek_backend.py: _validate_params only range-checked n_passes when n_passes > 1, so 0 or a negative value skipped validation entirely and reached _gl128_me_pass_layout's division, raising ZeroDivisionError instead of a clean RuntimeError. Range-checks n_passes unconditionally. - docs/USER_GUIDE.md: replaces the retired Multi-exposure checkbox description with the Scan mode combo (Single-Pass/Multi-Pass/ Adaptive Multi-Exposure/Adaptive Multi-Pass) and Passes slider. Verified: pytest tests/scanners/ tests/test_scan_sidebar.py -q — 556 passed, 4 skipped; ruff check clean on the touched files. Claude-Session: https://claude.ai/code/session_01X9n1QurLDU6LrimuSTYa5H --- docs/USER_GUIDE.md | 2 +- negpy/desktop/view/sidebar/scan.py | 62 ++----------------- .../scanners/plustek_backend.py | 4 +- tests/scanners/test_plustek_backend.py | 14 +++++ 4 files changed, 23 insertions(+), 59 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 2d414216..2d9826bb 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -964,7 +964,7 @@ Capture film directly into NegPy. Two collapsible sections. Drive a film scanner. Choose a **Backend**: **SANE** (Linux/macOS; Coolscans and other SANE devices), **Nikon Coolscan (nkscan)** (a direct driver for Nikon Coolscans on Linux, Windows and macOS) or **pyOpticfilm (Plustek)** (OpticFilm 8200i SE and 8100 V2; Windows, macOS and Linux). Controls are grouped in the order you decide them: **Film** (what is on the film), **Quality** (resolution, depth, extra passes), **Framing** (which frames, and the window) and and **Output** (format, folder, filename template). A group's header disappears with the whole group when the device has nothing in it. **Frames** takes the frames to scan as a list: `1-6`, `1,2,5`, or empty for every frame on the film. The strip preview writes its picks there, so a selection can be changed without previewing again. The line above **Scan** says what pressing it will do: how many frames, at what resolution, which extra passes and roughly how much disk it takes. **Depth** appears only when the device offers more than one bit depth, so it is hidden for the OpticFilm 8200i SE, which is 16-bit only. **Autofocus** and hardware **Auto-exposure** appear only when the connected device reports them, so typically on Coolscans and not on the OpticFilm 8200i SE. **Prescan** appears for devices that support a low-DPI full-window preview, such as the OpticFilm 8200i SE: run the preview, drag a crop rectangle, and the next Scan uses that hardware ROI. When the scanner exposes a `scan-exposure-time` option, as some genesys devices do, an **Exposure** slider appears; set it to override the scanner's default exposure time, and the value shows in µs, ms or s as appropriate. A device without the option hides the slider, so a saved value never breaks a different scanner. -**pyOpticfilm (Plustek)** notes: the **OpticFilm 8200i SE** (`07b3:1825`) and the **8100 V2** (`07b3:1824`) are scan-ready. Other OpticFilm models may appear in the device list but cannot scan until pyopticfilm marks them ready; on Linux and macOS, switch Backend to **SANE** if that backend lists the scanner. Use **Prescan** to grab a 1200 dpi full-window preview, set a crop, then leave with **Apply Crop** or **Scan Frame**. Either way the next scan reads that hardware ROI at the chosen DPI, not a software crop. **Multi-exposure** (8200i SE, 8100 V2; off by default) merges short and long color passes for more highlight and shadow detail; the long pass exposure is chosen per frame, and the scan takes longer than a normal pass. Scans from pyopticfilm 1.1.2 onward match SilverFast orientation; rescans older files if left-right matters. +**pyOpticfilm (Plustek)** notes: the **OpticFilm 8200i SE** (`07b3:1825`) and the **8100 V2** (`07b3:1824`) are scan-ready. Other OpticFilm models may appear in the device list but cannot scan until pyopticfilm marks them ready; on Linux and macOS, switch Backend to **SANE** if that backend lists the scanner. Use **Prescan** to grab a 1200 dpi full-window preview, set a crop, then leave with **Apply Crop** or **Scan Frame**. Either way the next scan reads that hardware ROI at the chosen DPI, not a software crop. **Scan mode** (8200i SE, 8100 V2; Single-Pass by default) chooses among **Single-Pass** (one exposure), **Multi-Pass** (repeats the exposure and stacks the results to reduce noise, with a **Passes** slider from 2 to 9), **Adaptive Multi-Exposure** (fuses a short and long exposure for extended dynamic range) and **Adaptive Multi-Pass** (both together); every mode past Single-Pass takes longer, and Multi-Pass cannot combine with IR. Scans from pyopticfilm 1.1.2 onward match SilverFast orientation; rescans older files if left-right matters. With **IR** checked, color and infrared come back in one scan pass; pyopticfilm aligns the IR plane to the color frame. Color scans apply ASIC shading measured at home before the film feed, the same order as SilverFast, so the strip may stay loaded. The table is cached per DPI, so later scans only re-upload it. diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index 852b7541..a3a18600 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -386,50 +386,6 @@ def _init_ui(self) -> None: self.exposure_label.setVisible(False) self.exposure_row_widget.setVisible(False) - self.framing_header = section_subheader("FRAMING") - self.form.addRow(self.framing_header) - - # Which frames the batch scans, for roll and strip feeders only. - self.frame_spec_edit = QLineEdit() - self.frame_spec_edit.setPlaceholderText("All Frames") - self.frame_spec_edit.setToolTip("Frames to scan: 1-6 or 1,2,5. Empty scans every frame.") - self.frame_spec_label = QLabel("Frames") - self.form.addRow(self.frame_spec_label, self.frame_spec_edit) - self.frame_spec_label.setVisible(False) - self.frame_spec_edit.setVisible(False) - - # Scan window (strip/roll feeders): set once from a preview, reused per frame. - self.scan_window_widget = QWidget() - scan_window_row = QHBoxLayout(self.scan_window_widget) - scan_window_row.setContentsMargins(0, 0, 0, 0) - self.scan_window_btn = labeled_action("", "Set Scan Window…", "Preview a frame and set the scan window reused for every frame") - self.scan_window_clear_btn = labeled_action("", "Clear", "Scan the whole default frame instead") - scan_window_row.addWidget(self.scan_window_btn, 1) - scan_window_row.addWidget(self.scan_window_clear_btn) - self.scan_window_row_label = QLabel("Batch") - self.form.addRow(self.scan_window_row_label, self.scan_window_widget) - self.scan_window_status = hint_label("") - self.form.addRow("", self.scan_window_status) - self.scan_window_row_label.setVisible(False) - self.scan_window_widget.setVisible(False) - self.scan_window_status.setVisible(False) - - # Prescan + crop (Plustek SE): low-DPI full window → interactive crop → scan_window. - self.prescan_widget = QWidget() - prescan_row = QHBoxLayout(self.prescan_widget) - prescan_row.setContentsMargins(0, 0, 0, 0) - self.prescan_btn = labeled_action("", "Prescan…", "Scan a low-DPI preview and set the crop for the next scan") - self.prescan_clear_btn = labeled_action("", "Clear", "Scan the full window instead of a crop") - prescan_row.addWidget(self.prescan_btn, 1) - prescan_row.addWidget(self.prescan_clear_btn) - self.prescan_label = QLabel("Prescan") - self.form.addRow(self.prescan_label, self.prescan_widget) - self.prescan_status = hint_label("") - self.form.addRow("", self.prescan_status) - self.prescan_label.setVisible(False) - self.prescan_widget.setVisible(False) - self.prescan_status.setVisible(False) - self.output_header = section_subheader("OUTPUT") self.form.addRow(self.output_header) @@ -456,7 +412,7 @@ def _init_ui(self) -> None: # ── FRAMING (bottom, right above Scan) ─────────────── # These are the last decisions before pressing Scan, not a settings-form field among # Film/Quality/Output — full width, not squeezed into the form's shared label column. - self.framing_header = section_subheader("Framing") + self.framing_header = section_subheader("FRAMING") layout.addWidget(self.framing_header) # Which frames the batch scans, for roll and strip feeders only. @@ -465,7 +421,7 @@ def _init_ui(self) -> None: frame_spec_row.setSpacing(6) self.frame_spec_label = QLabel("Frames") self.frame_spec_edit = QLineEdit() - self.frame_spec_edit.setPlaceholderText("All frames") + self.frame_spec_edit.setPlaceholderText("All Frames") self.frame_spec_edit.setToolTip("Frames to scan: 1-6 or 1,2,5. Empty scans every frame.") frame_spec_row.addWidget(self.frame_spec_label) frame_spec_row.addWidget(self.frame_spec_edit, 1) @@ -481,11 +437,8 @@ def _init_ui(self) -> None: self.scan_window_widget = QWidget() scan_window_btn_row = QHBoxLayout(self.scan_window_widget) scan_window_btn_row.setContentsMargins(0, 0, 0, 0) - self.scan_window_btn = QPushButton("Set scan window…") - self.scan_window_btn.setToolTip("Preview a frame and set the scan window reused for every frame") - self.scan_window_clear_btn = QPushButton("Clear") - self.scan_window_clear_btn.setFixedWidth(56) - self.scan_window_clear_btn.setToolTip("Scan the whole default frame instead") + self.scan_window_btn = labeled_action("", "Set Scan Window…", "Preview a frame and set the scan window reused for every frame") + self.scan_window_clear_btn = labeled_action("", "Clear", "Scan the whole default frame instead") scan_window_btn_row.addWidget(self.scan_window_btn, 1) scan_window_btn_row.addWidget(self.scan_window_clear_btn) scan_window_row.addWidget(self.scan_window_row_label) @@ -504,11 +457,8 @@ def _init_ui(self) -> None: self.prescan_widget = QWidget() prescan_row = QHBoxLayout(self.prescan_widget) prescan_row.setContentsMargins(0, 0, 0, 0) - self.prescan_btn = QPushButton("Prescan") - self.prescan_btn.setToolTip("Scan a low-DPI preview and set the crop for the next scan") - self.prescan_clear_btn = QPushButton("Clear") - self.prescan_clear_btn.setFixedWidth(56) - self.prescan_clear_btn.setToolTip("Scan the full window instead of a crop") + self.prescan_btn = labeled_action("", "Prescan…", "Scan a low-DPI preview and set the crop for the next scan") + self.prescan_clear_btn = labeled_action("", "Clear", "Scan the full window instead of a crop") prescan_row.addWidget(self.prescan_btn, 1) prescan_row.addWidget(self.prescan_clear_btn) layout.addWidget(self.prescan_widget) diff --git a/negpy/infrastructure/scanners/plustek_backend.py b/negpy/infrastructure/scanners/plustek_backend.py index 30d1e013..df618902 100644 --- a/negpy/infrastructure/scanners/plustek_backend.py +++ b/negpy/infrastructure/scanners/plustek_backend.py @@ -174,9 +174,9 @@ def _validate_params(params: ScanParams, *, model: Any | None = None) -> None: mode = params.multi_exposure_mode if mode != MultiExposureMode.OFF and model is not None and not getattr(model, "exposure_long", None): raise RuntimeError(f"{getattr(model, 'model', 'device')} does not support multi-exposure") + if not (MIN_N_PASSES <= params.n_passes <= MAX_N_PASSES): + raise RuntimeError(f"n_passes={params.n_passes} out of range ({MIN_N_PASSES}-{MAX_N_PASSES})") if params.n_passes > 1: - if not (MIN_N_PASSES <= params.n_passes <= MAX_N_PASSES): - raise RuntimeError(f"n_passes={params.n_passes} out of range ({MIN_N_PASSES}-{MAX_N_PASSES})") if model is not None and not getattr(model, "scan_ready", False): raise RuntimeError(f"{getattr(model, 'model', 'device')} does not support Multi-Pass") if params.capture_ir: diff --git a/tests/scanners/test_plustek_backend.py b/tests/scanners/test_plustek_backend.py index fe7815db..0e5fcc08 100644 --- a/tests/scanners/test_plustek_backend.py +++ b/tests/scanners/test_plustek_backend.py @@ -433,6 +433,20 @@ def test_n_passes_rejects_out_of_range_value(monkeypatch): scanner.scan.assert_not_called() +def test_n_passes_rejects_below_minimum(monkeypatch): + _patch_enum(monkeypatch) + scanner = _fake_scanner() + monkeypatch.setattr(f"{_BACKEND}.Scanner.open", _FakeOpen(scanner)) + with pytest.raises(RuntimeError, match="n_passes"): + PlustekBackend().scan( + _DEVICE_ID, + _params(n_passes=0), + lambda *_: None, + threading.Event(), + ) + scanner.scan.assert_not_called() + + def test_ir_and_multi_pass_together_is_rejected(monkeypatch): _patch_enum(monkeypatch) scanner = _fake_scanner()