From d08db7f2eeef3ecf7f4a4ac126a499e64c98b81e Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 2 Sep 2026 20:13:35 +0200 Subject: [PATCH 01/16] feat(scan): correct one frame's registration on its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The strip preview corrected registration with two global sliders: Offset, applied to every frame, and Drift, a linear ramp across frame positions. Neither reaches a single frame that sits off on its own — a splice, a mis-fired advance, a boundary the detector placed a hair late. Each tile now carries its own slider, adding a per-frame correction on top of that ramp: offset(N) = frame_offset_mm + (N-1) * drift + frame_offsets[N] An absent key is 0.0, so nothing existing changes. The delta rides frame_offset_mm, which already slides the nkscan rect and writes the coolscan3 subframe option, so no backend work was needed. A measured strip re-addresses an absolute rect and may go either way; a feeder clamps the total as before, because it cannot back up. The correction registers one strip rather than the transport, so eject drops it alongside the frame selection and crops. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7 --- docs/USER_GUIDE.md | 3 +- negpy/desktop/view/sidebar/scan.py | 7 ++- .../view/widgets/strip_preview_dialog.py | 52 +++++++++++++++-- negpy/desktop/workers/scan_worker.py | 4 +- negpy/infrastructure/scanners/settings.py | 5 ++ tests/scanners/test_scanner_settings.py | 5 ++ tests/test_scan_sidebar.py | 20 +++++++ tests/test_scan_worker.py | 21 +++++++ tests/test_strip_preview_dialog.py | 56 +++++++++++++++++++ 9 files changed, 164 insertions(+), 9 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 2d4142169..f32f1a7b0 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -1005,7 +1005,8 @@ Every preview dialog ends the same way: **Cancel**, then **Apply** (keep the fra * **Cropping**: drag on a previewed frame. A corner resizes, inside moves. Each frame keeps its own window, and **Clear Crops** drops the lot. * **Offset**: slides every frame along the film to clear the inter-frame gap. Frames shift left as it grows, live. The shaded band on the right is film past the frame boundary the transport cannot deliver, so offset past the gap costs frame tail. A feeder cannot back up, so there it only goes one way. * **Drift**: adds progressively more (or less) offset per frame position, for a strip whose gaps creep along its length. Re-preview to refresh the pixels. -* **Which frames**: each tile carries its own tick; **All** and **None** move the lot, and the count says how many will be scanned. On a measured strip the ticks and crops describe the piece of film in the transport, so ejecting clears them; Offset and Drift survive, because they register the transport. +* **Per-frame offset**: the slider under each tile corrects that frame alone, on top of Offset and Drift, for a boundary that sits off on its own. Its reading is in the tooltip; double-click resets it. +* **Which frames**: each tile carries its own tick; **All** and **None** move the lot, and the count says how many will be scanned. On a measured strip the ticks and crops describe the piece of film in the transport, so ejecting clears them, per-frame offsets included; Offset and Drift survive, because they register the transport. --- diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index 14d83dcef..31cc5ade6 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -840,6 +840,7 @@ def _on_set_scan_window(self) -> None: initial_selected=self._settings.selected_frames, initial_offset=self._settings.frame_offset_mm, initial_offset_modifier=self._settings.frame_offset_modifier_mm, + initial_frame_offsets=self._settings.frame_offsets, film_format=self._film_format(), film_type=self._film_type(), parent=self, @@ -851,6 +852,7 @@ def _on_set_scan_window(self) -> None: selected_frames=dialog.selected_frames(), frame_offset_mm=dialog.frame_offset(), frame_offset_modifier_mm=dialog.frame_offset_modifier(), + frame_offsets=dialog.frame_offsets(), ) self._update_scan_window_status() if dialog.scan_requested(): @@ -1095,6 +1097,7 @@ def _on_scan(self) -> None: frames=frames, frame_windows=frame_windows, frame_offset_modifier_mm=self._settings.frame_offset_modifier_mm, + frame_offsets=self._settings.frame_offsets, ) ) else: @@ -1162,9 +1165,9 @@ def _on_ejected(self, triggered: bool) -> None: return # Frames and their crops describe the piece of film that just came out; the next strip # is a different one, and silently reusing them scans the wrong frames. - stale = bool(self._settings.selected_frames or self._settings.frame_windows) + stale = bool(self._settings.selected_frames or self._settings.frame_windows or self._settings.frame_offsets) if stale: - self.settings = replace(self._settings, selected_frames=(), frame_windows={}) + self.settings = replace(self._settings, selected_frames=(), frame_windows={}, frame_offsets={}) self._update_scan_window_status() self._update_summary() self.status_strip.set_message("Film ejected — frame selection cleared" if stale else "Film ejected") diff --git a/negpy/desktop/view/widgets/strip_preview_dialog.py b/negpy/desktop/view/widgets/strip_preview_dialog.py index 2742e1cad..99dfbcb3a 100644 --- a/negpy/desktop/view/widgets/strip_preview_dialog.py +++ b/negpy/desktop/view/widgets/strip_preview_dialog.py @@ -44,6 +44,7 @@ _PREVIEW_FALLBACK_DPI = 500 # only when the device reports no DPI list at all _MAX_MEASURED_OFFSET_TENTHS = 25 # ±2.5 mm, in the slider's tenths of a millimetre _TILE_H = 140 # constant tile height; width follows the device aspect +_TILE_SLIDER_H = 18 # the per-frame offset slider under each tile _TILES_PER_ROW = 6 # one SA-21 strip per row; roll adapters (up to 40 frames) wrap below # A transport that measures the strip reports its frame count only as previews arrive, so ask # for a roll's worth and keep the tiles it answers with. @@ -92,6 +93,7 @@ def _display_to_scan_rect(rect): "offset past the gap costs frame tail." ) _DRIFT_TIP = "Adds progressively more (or less) offset per frame position, for a strip whose gaps creep. Re-preview to refresh the pixels." +_TILE_OFFSET_TIP = "Corrects this frame alone, on top of Offset and Drift. Double-click to reset." class _ResetSlider(QSlider): @@ -108,12 +110,21 @@ def mouseDoubleClickEvent(self, _event) -> None: class _Tile: """One strip position: its preview label and include box.""" - def __init__(self, frame: int, label: ScanWindowLabel, checkbox: QCheckBox, preview_btn: QPushButton, widget: QWidget) -> None: + def __init__( + self, + frame: int, + label: ScanWindowLabel, + checkbox: QCheckBox, + preview_btn: QPushButton, + offset_slider: "_ResetSlider", + widget: QWidget, + ) -> None: self.frame = frame self.previewed_offset: float | None = None # offset the shown preview was scanned at self.label = label self.checkbox = checkbox self.preview_btn = preview_btn + self.offset_slider = offset_slider self.widget = widget @@ -128,6 +139,7 @@ def __init__( initial_selected=None, initial_offset: float = 0.0, initial_offset_modifier: float = 0.0, + initial_frame_offsets: dict[int, float] | None = None, film_format: str | None = None, film_type: str = "negative", parent=None, @@ -152,12 +164,13 @@ def __init__( self._scan_now = False # set when the user chooses "Scan" over "Use" initial_windows = initial_windows or {} initial_selected = tuple(initial_selected or ()) + self._initial_frame_offsets = dict(initial_frame_offsets or {}) self.setWindowTitle("Preview Strip — Set a Window per Frame") self.setModal(True) tile_w, tile_h = self._tile_size() cols = min(self._capacity or _TILES_PER_ROW, _TILES_PER_ROW) rows = max(1, -(-self._capacity // _TILES_PER_ROW)) - self.resize(cols * (tile_w + 4) + 36, min(rows, 3) * (tile_h + 4) + 260) + self.resize(cols * (tile_w + 4) + 36, min(rows, 3) * (tile_h + _TILE_SLIDER_H + 4) + 260) layout = QVBoxLayout(self) @@ -367,7 +380,18 @@ def _build_tile(self, frame: int, initial_window, checked: bool) -> _Tile: oh.addWidget(preview_btn) grid.addWidget(overlay, 0, 0, Qt.AlignmentFlag.AlignTop | Qt.AlignmentFlag.AlignLeft) - return _Tile(frame, label, checkbox, preview_btn, widget) + offset_slider = _ResetSlider() + offset_slider.setRange(-_MAX_MEASURED_OFFSET_TENTHS, _MAX_MEASURED_OFFSET_TENTHS) + offset_slider.setFixedSize(self._tile_size()[0], _TILE_SLIDER_H) + # Seeded before the connection, so building a tile never runs the refresh against a + # dialog that is still assembling itself. + offset_slider.setValue(int(round(self._initial_frame_offsets.get(frame, 0.0) * 10))) + offset_slider.valueChanged.connect(lambda _v, f=frame: self._on_tile_offset_changed(f)) + grid.addWidget(offset_slider, 1, 0) + + tile = _Tile(frame, label, checkbox, preview_btn, offset_slider, widget) + self._set_tile_offset_tooltip(tile) + return tile def _tile_size(self) -> tuple[int, int]: return int(_TILE_H * self._tile_aspect), _TILE_H @@ -386,6 +410,10 @@ def _to_display(self, rect): def _to_scan(self, rect): return _display_to_scan_rect(rect) if self._rotation else rect + def frame_offsets(self) -> dict[int, float]: + """Per-frame corrections, non-zero entries only.""" + return {f: t.offset_slider.value() / 10.0 for f, t in self._tiles.items() if t.offset_slider.value()} + def frame_offset(self) -> float: return self.offset_slider.value() / 10.0 @@ -396,11 +424,16 @@ def _frame_pitch(self) -> float: """Feed-axis frame pitch (mm) — the length a tile represents. 0.0 when unknown.""" return effective_pitch_mm(self._caps) + def _frame_delta(self, frame: int) -> float: + """This frame's own correction. A slot with no tile yet contributes nothing.""" + tile = self._tiles.get(frame) + return tile.offset_slider.value() / 10.0 if tile else self._initial_frame_offsets.get(frame, 0.0) + def _raw_offset_for_frame(self, frame: int) -> float: - return self.frame_offset() + (frame - 1) * self.frame_offset_modifier() + return self.frame_offset() + (frame - 1) * self.frame_offset_modifier() + self._frame_delta(frame) def _offset_for_frame(self, frame: int) -> float: - """Effective offset for a frame position: base + (N-1)·drift. + """Effective offset for a frame position: base + (N-1)·drift + the frame's own correction. A feeder is floored at 0 and held short of one pitch: it cannot back up, and the scan blacks out at the frame boundary. A measured strip re-addresses the frame instead, so @@ -459,6 +492,15 @@ def _set_previewing(self, busy: bool) -> None: self.status_strip.stop_progress() self._update_ok_enabled() + def _set_tile_offset_tooltip(self, tile: _Tile) -> None: + tile.offset_slider.setToolTip(f"Frame {tile.frame}: {tile.offset_slider.value() / 10.0:+.1f} mm. {_TILE_OFFSET_TIP}") + + def _on_tile_offset_changed(self, frame: int) -> None: + tile = self._tiles.get(frame) + if tile is not None: + self._set_tile_offset_tooltip(tile) + self._on_offset_changed(0) + def _on_offset_changed(self, _value: int) -> None: self.offset_label.setText(f"{self.frame_offset():.1f} mm") self.drift_label.setText(f"{self.frame_offset_modifier():+.2f} mm/frame") diff --git a/negpy/desktop/workers/scan_worker.py b/negpy/desktop/workers/scan_worker.py index a36821315..dc63d63fc 100644 --- a/negpy/desktop/workers/scan_worker.py +++ b/negpy/desktop/workers/scan_worker.py @@ -61,6 +61,8 @@ class BatchRequest: # Feed-axis drift (mm/frame): frame N scans at frame_offset_mm + (N-1) * modifier, # floored at 0. frame_offset_modifier_mm: float = 0.0 + # Per-frame correction (mm) on top of that ramp; an absent key means none. + frame_offsets: dict[int, float] = field(default_factory=dict) class ScanWorker(QObject): @@ -223,7 +225,7 @@ def run_batch(self, req: BatchRequest) -> None: window = req.frame_windows.get(frame, req.params.window) # No floor here: a transport that cannot back up clamps in its own backend, and # one that re-addresses an absolute frame may legitimately go negative. - offset = req.params.frame_offset_mm + (frame - 1) * req.frame_offset_modifier_mm + offset = req.params.frame_offset_mm + (frame - 1) * req.frame_offset_modifier_mm + req.frame_offsets.get(frame, 0.0) frame_params = dataclasses.replace(req.params, frame=frame, window=window, frame_offset_mm=offset) base = index / total diff --git a/negpy/infrastructure/scanners/settings.py b/negpy/infrastructure/scanners/settings.py index 7eb1bf2a8..a40ba2c5d 100644 --- a/negpy/infrastructure/scanners/settings.py +++ b/negpy/infrastructure/scanners/settings.py @@ -42,6 +42,9 @@ class ScannerSettings: # switch to a sorted tuple of pairs if that ever changes. frame_windows: dict[int, Rect] = field(default_factory=dict) selected_frames: tuple[int, ...] = () + # Per-frame feed-axis correction (mm), added on top of frame_offset_mm + drift. An absent + # key means no correction for that frame. + frame_offsets: dict[int, float] = field(default_factory=dict) def __post_init__(self) -> None: # JSON round-trips tuples as lists and dict keys as strings; coerce back. @@ -55,6 +58,8 @@ def __post_init__(self) -> None: ) if isinstance(self.selected_frames, list): object.__setattr__(self, "selected_frames", tuple(self.selected_frames)) + if isinstance(self.frame_offsets, dict): + object.__setattr__(self, "frame_offsets", {int(k): float(v) for k, v in self.frame_offsets.items()}) @classmethod def defaults(cls) -> "ScannerSettings": diff --git a/tests/scanners/test_scanner_settings.py b/tests/scanners/test_scanner_settings.py index a2c30df4a..33d0396a7 100644 --- a/tests/scanners/test_scanner_settings.py +++ b/tests/scanners/test_scanner_settings.py @@ -139,3 +139,8 @@ 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_per_frame_offsets_round_trip_through_json_string_keys(): + restored = ScannerSettings.from_dict({"frame_offsets": {"2": 0.4, "5": -0.3}}) + assert restored.frame_offsets == {2: 0.4, 5: -0.3} diff --git a/tests/test_scan_sidebar.py b/tests/test_scan_sidebar.py index d0fb08df1..a5ebf164d 100644 --- a/tests/test_scan_sidebar.py +++ b/tests/test_scan_sidebar.py @@ -483,6 +483,17 @@ def test_scan_carries_offset_and_drift_into_the_batch_request() -> None: assert req.frame_offset_modifier_mm == 0.2 +def test_scan_carries_the_per_frame_corrections_into_the_batch_request() -> None: + sidebar, controller = _sidebar(LS50_DEVICE) + sidebar.folder_edit.setText("/tmp/negpy-scan-out") + sidebar.settings = replace(sidebar._settings, frame_offsets={2: -0.4}) + + sidebar._on_scan() + + _kind, req = controller.started[0] + assert req.frame_offsets == {2: -0.4} + + def test_eject_button_calls_controller() -> None: sidebar, controller = _sidebar(FULL_DEVICE) sidebar._on_eject() @@ -948,6 +959,15 @@ def test_ejecting_drops_the_frame_selection_of_the_film_that_left() -> None: assert "frame selection cleared" in sidebar.status_strip.message() +def test_ejecting_drops_the_per_frame_corrections_of_the_film_that_left() -> None: + # A per-frame correction registers one strip's own boundaries; the next strip has its own. + sidebar, _ = _sidebar(FULL_DEVICE, settings={"frame_offsets": {"2": 0.4}}) + + sidebar._on_ejected(True) + + assert sidebar.settings.frame_offsets == {} + + def test_ejecting_keeps_the_registration_offsets() -> None: # Offset and drift belong to the transport's own registration, not to one strip. sidebar, _ = _sidebar(FULL_DEVICE, settings={"selected_frames": [1], "frame_offset_mm": 1.5, "frame_offset_modifier_mm": 0.2}) diff --git a/tests/test_scan_worker.py b/tests/test_scan_worker.py index 3c0d7ca96..dd21f82b5 100644 --- a/tests/test_scan_worker.py +++ b/tests/test_scan_worker.py @@ -152,6 +152,27 @@ def test_batch_applies_progressive_offset_per_frame_position() -> None: assert service.offsets == pytest.approx([1.2, 1.4, 1.6]) +def test_batch_adds_a_per_frame_correction_on_top_of_the_ramp() -> None: + worker = ScanWorker() + service = _BatchService() + worker._service = service # type: ignore[assignment] + req = BatchRequest( + device_id="coolscan3:test", + params=ScanParams(dpi=4_000, depth=16, capture_ir=False, frame_offset_mm=1.0), + output_folder="/tmp", + filename_pattern='scan-{{ "%03d" % seq }}', + output_format="TIFF", + frames=(2, 3, 4), + frame_offset_modifier_mm=0.2, + frame_offsets={3: -0.5}, + ) + + worker.run_batch(req) + + # Only frame 3 moves; a frame with no entry keeps base + drift. + assert service.offsets == pytest.approx([1.2, 0.9, 1.6]) + + def test_batch_passes_a_negative_drift_through_to_the_backend() -> None: worker = ScanWorker() service = _BatchService() diff --git a/tests/test_strip_preview_dialog.py b/tests/test_strip_preview_dialog.py index 5e4240efa..d94d5651a 100644 --- a/tests/test_strip_preview_dialog.py +++ b/tests/test_strip_preview_dialog.py @@ -889,3 +889,59 @@ def test_a_saved_negative_offset_comes_back_as_it_was() -> None: dialog = StripPreviewDialog(_FakeController(), _discovery_device(), initial_offset=-1.5) assert dialog.frame_offset() == -1.5 assert dialog.offset_label.text() == "-1.5 mm" + + +# ── the per-frame offset slider ─────────────────────────────────────────── + + +def test_a_tile_slider_corrects_its_own_frame_only() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(3), initial_offset=2.0) + + dialog._tiles[2].offset_slider.setValue(-5) # tenths of a mm + + assert dialog._raw_offset_for_frame(1) == pytest.approx(2.0) + assert dialog._raw_offset_for_frame(2) == pytest.approx(1.5) + assert dialog._raw_offset_for_frame(3) == pytest.approx(2.0) + + +def test_a_tile_slider_moves_that_tiles_band_and_no_other() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(3), initial_offset=4.0) + + dialog._tiles[2].offset_slider.setValue(19) # +1.9 mm + + ((first, _),) = dialog._tiles[1].label._offset_indicators + ((second, _),) = dialog._tiles[2].label._offset_indicators + assert first == pytest.approx(4.0 / 38.0, abs=1e-3) + assert second == pytest.approx(5.9 / 38.0, abs=1e-3) + + +def test_frame_offsets_reports_the_corrected_frames_only() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(3)) + + dialog._tiles[3].offset_slider.setValue(7) + + assert dialog.frame_offsets() == {3: 0.7} + + +def test_a_saved_correction_comes_back_on_its_tile() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(3), initial_frame_offsets={2: -0.8}) + + assert dialog._tiles[2].offset_slider.value() == -8 + assert dialog.frame_offsets() == {2: -0.8} + + +def test_double_clicking_a_tile_slider_clears_its_correction() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(3), initial_frame_offsets={1: 1.2}) + + dialog._tiles[1].offset_slider.mouseDoubleClickEvent(None) + + assert dialog.frame_offsets() == {} + + +def test_the_preview_request_carries_the_per_frame_correction() -> None: + controller = _FakeController() + dialog = StripPreviewDialog(controller, _device(3), initial_offset=1.0, initial_frame_offsets={2: 0.9}) + + dialog._on_preview_all() + + assert controller.preview_reqs[0].offsets[2] == pytest.approx(1.9 / 38.0, abs=1e-4) From 6864cf68d3aadef7a353cf3edc0c18ce373205de Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 2 Sep 2026 20:45:06 +0200 Subject: [PATCH 02/16] feat(scan): write TIFF only, in colour or mono MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to what a scan leaves on disk. Film that carries a single record reads the same off one plane as off three, so Format gains "TIFF (mono)": one 16-bit grey plane, minisblack, at a third of the size. The plane is the mean of the three, summed in uint32 so nothing overflows and rounded rather than floored. Mono film meters with its channels locked, so the three are one density read three times and their mean is the least noisy of them. Deriving it host-side rather than asking the transport keeps one code path: nkscan offers no grey mode at all. The loader restacks a 2-plane TIFF to three, so NegPy reads its own mono scans back unchanged. DNG output is retired. write_dng_linear and its encoder are gone, and a saved "DNG" preference lands on TIFF, the other 16-bit master — the same mapping RETIRED_EXPORT_FORMATS already applies on the export side. Reading LinearRaw DNGs from SilverFast and VueScan is untouched; only writing them is dropped. TestScanRoundTripParity went with it: it existed to prove a scan TIFF and its DNG twin decode alike, and there is no twin now. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7 --- docs/USER_GUIDE.md | 2 +- negpy/desktop/view/sidebar/scan.py | 6 +- negpy/desktop/workers/scan_worker.py | 2 +- negpy/infrastructure/loaders/rawpy_loader.py | 5 +- negpy/infrastructure/scanners/settings.py | 9 ++ negpy/services/scanning/__init__.py | 3 +- negpy/services/scanning/service.py | 11 +- negpy/services/scanning/writer.py | 108 ++++--------------- tests/scanners/test_scanner_settings.py | 6 ++ tests/scanners/test_service.py | 19 ++++ tests/scanners/test_writer.py | 95 +++++----------- tests/test_scan_sidebar.py | 17 +++ tests/test_tiff_loader_encoding.py | 32 +----- 13 files changed, 114 insertions(+), 201 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index f32f1a7b0..69419a26c 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -962,7 +962,7 @@ Capture film directly into NegPy. Two collapsible sections. ### Film Scanner -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. +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. **Format** writes `TIFF` or `TIFF (mono)`; the mono form writes one 16-bit grey plane instead of three, at a third of the size, for film that carries a single record such as a black-and-white negative. NegPy reads it back the same way. **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. diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index 31cc5ade6..2b121b68b 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -30,7 +30,7 @@ from negpy.infrastructure.scanners.base import ScannerCapabilities, ScannerDevice from negpy.infrastructure.scanners.params import FILM_TYPES, FilmType, film_passes_infrared from negpy.infrastructure.scanners.registry import DEFAULT_BACKEND_ID, backend_choices -from negpy.infrastructure.scanners.settings import ScannerSettings +from negpy.infrastructure.scanners.settings import OUTPUT_FORMATS, ScannerSettings _SAMPLE_COUNTS = (1, 2, 4, 8, 16) @@ -312,8 +312,8 @@ def _init_ui(self) -> None: self.form.addRow(self.output_header) self.fmt_combo = QComboBox() - self.fmt_combo.addItems(["TIFF", "DNG"]) - self.fmt_combo.setToolTip("Output file format") + self.fmt_combo.addItems(list(OUTPUT_FORMATS)) + self.fmt_combo.setToolTip("Output file format. Mono writes one grey plane instead of three, for film with a single record.") self.form.addRow("Format", self.fmt_combo) folder_row = QHBoxLayout() diff --git a/negpy/desktop/workers/scan_worker.py b/negpy/desktop/workers/scan_worker.py index dc63d63fc..5fce73fac 100644 --- a/negpy/desktop/workers/scan_worker.py +++ b/negpy/desktop/workers/scan_worker.py @@ -19,7 +19,7 @@ class ScanRequest: params: ScanParams output_folder: str filename_pattern: str - output_format: str # "TIFF" or "DNG" + output_format: str # one of settings.OUTPUT_FORMATS @dataclass(frozen=True) diff --git a/negpy/infrastructure/loaders/rawpy_loader.py b/negpy/infrastructure/loaders/rawpy_loader.py index 1a6006213..97311bf85 100644 --- a/negpy/infrastructure/loaders/rawpy_loader.py +++ b/negpy/infrastructure/loaders/rawpy_loader.py @@ -137,9 +137,8 @@ def tag(name: str) -> Optional[Any]: def _peek_linearraw_4ch(file_path: str) -> Optional[Tuple[np.ndarray, np.ndarray]]: """Inspect a DNG. If it carries 4 linear samples (RGB + IR), return (rgb, ir) as float32 [0,1]. - NegPy's own `write_dng_linear` produces a single-IFD DNG; VueScan and Adobe-style DNGs - put the full-res data in a SubIFD behind a reduced-resolution thumbnail IFD0 — both are - checked. Returns None for camera DNGs (Bayer, 3-channel, etc.) so rawpy can handle them. + A single-IFD DNG carries the data in IFD0; VueScan and Adobe-style DNGs put the full-res + data in a SubIFD behind a reduced-resolution thumbnail IFD0 — both are checked. Returns None for camera DNGs (Bayer, 3-channel, etc.) so rawpy can handle them. """ if not _is_dng(file_path): return None diff --git a/negpy/infrastructure/scanners/settings.py b/negpy/infrastructure/scanners/settings.py index a40ba2c5d..d8e3c601d 100644 --- a/negpy/infrastructure/scanners/settings.py +++ b/negpy/infrastructure/scanners/settings.py @@ -5,6 +5,12 @@ Rect = tuple[float, float, float, float] +#: Written as one 16-bit grey plane instead of three. Film that carries a single record +#: (a B&W negative) reads the same off one plane, at a third of the size. +MONO_TIFF = "TIFF (mono)" +#: What the Format combo offers, in order. +OUTPUT_FORMATS = ("TIFF", MONO_TIFF) + @dataclass(frozen=True) class ScannerSettings: @@ -73,6 +79,9 @@ def from_dict(cls, data: dict) -> "ScannerSettings": every unrelated preference with it. """ data = dict(data) + # DNG output is retired; a saved one lands on TIFF, the other 16-bit master. + if str(data.get("output_format", "")).upper() == "DNG": + data["output_format"] = "TIFF" 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/negpy/services/scanning/__init__.py b/negpy/services/scanning/__init__.py index bdc2f09be..881471989 100644 --- a/negpy/services/scanning/__init__.py +++ b/negpy/services/scanning/__init__.py @@ -1,10 +1,9 @@ """Scanner orchestration — no Qt dependencies.""" from negpy.services.scanning.service import ScannerService -from negpy.services.scanning.writer import write_dng_linear, write_tiff_16bit +from negpy.services.scanning.writer import write_tiff_16bit __all__ = [ "ScannerService", - "write_dng_linear", "write_tiff_16bit", ] diff --git a/negpy/services/scanning/service.py b/negpy/services/scanning/service.py index 5c9cd039e..e7ecd544f 100644 --- a/negpy/services/scanning/service.py +++ b/negpy/services/scanning/service.py @@ -134,12 +134,14 @@ def write_result( """ from datetime import date as dt_date - from negpy.services.scanning.writer import write_dng_linear, write_tiff_16bit + from negpy.infrastructure.scanners.settings import MONO_TIFF + from negpy.services.scanning.writer import write_tiff_16bit os.makedirs(output_folder, exist_ok=True) + fmt = output_format.upper() date_str = dt_date.today().strftime("%Y%m%d") - ext = ".dng" if output_format.upper() == "DNG" else ".tif" + ext = ".tif" require_sequence_varying_scan_filename(filename_pattern, date_str) @@ -151,9 +153,6 @@ def write_result( break current += 1 - if output_format.upper() == "DNG": - rgb_path = write_dng_linear(result, rgb_path) - else: - rgb_path = write_tiff_16bit(result, rgb_path) + rgb_path = write_tiff_16bit(result, rgb_path, mono=fmt == MONO_TIFF.upper()) return rgb_path diff --git a/negpy/services/scanning/writer.py b/negpy/services/scanning/writer.py index b9a62f481..271cb6451 100644 --- a/negpy/services/scanning/writer.py +++ b/negpy/services/scanning/writer.py @@ -1,6 +1,4 @@ -import io import os -import struct import tempfile import numpy as np @@ -29,21 +27,37 @@ def _to_uint16(arr: np.ndarray) -> np.ndarray: return arr.astype(np.uint16) -def write_tiff_16bit(result: ScanResult, path: str) -> str: +def _to_grey(rgb: np.ndarray) -> np.ndarray: + """One plane from three, by their mean. + + Film with a single record is metered with its channels locked, so the three planes are one + density read three times and their mean is the least noisy of them. Summing in uint32 keeps + the accumulator off the source dtype; `(s + 1) // 3` rounds rather than floors. + """ + if rgb.ndim == 2: + return rgb + return ((np.sum(rgb, axis=-1, dtype=np.uint32) + 1) // 3).astype(rgb.dtype) + + +def write_tiff_16bit(result: ScanResult, path: str, *, mono: bool = False) -> str: """Write ScanResult to 16-bit TIFF. IR written as sidecar `_IR.tif`. - Uses atomic write (write to a part file, then rename) to avoid partial files. - Returns final RGB path. + `mono` writes one grey plane instead of three. Uses atomic write (write to a part file, + then rename) to avoid partial files. Returns final RGB path. """ if not path.lower().endswith((".tif", ".tiff")): path = path + ".tif" rgb = _to_uint16(result.rgb) + photometric = "rgb" + if mono: + rgb = _to_grey(rgb) + photometric = "minisblack" fd, tmp_path = tempfile.mkstemp(suffix=_PART_SUFFIX, dir=os.path.dirname(path) or ".") os.close(fd) try: - tifffile.imwrite(tmp_path, rgb, photometric="rgb", compression="zlib", predictor=True) + tifffile.imwrite(tmp_path, rgb, photometric=photometric, compression="zlib", predictor=True) os.replace(tmp_path, path) except Exception: if os.path.exists(tmp_path): @@ -81,85 +95,3 @@ def write_tiff_16bit(result: ScanResult, path: str) -> str: raise return path - - -def write_dng_linear(result: ScanResult, path: str) -> str: - """Write ScanResult to an uncompressed 16-bit LinearRaw DNG via tifffile. - - A LinearRaw DNG is a single-IFD TIFF plus a few DNG tags. If result.ir is - present it is stacked as an extra sample. Atomic write; returns final path. - """ - if not path.lower().endswith(".dng"): - path = path + ".dng" - - rgb = _to_uint16(result.rgb) - - if result.ir is not None: - ir = result.ir - if ir.ndim == 2: - ir = ir[:, :, np.newaxis] - ir = _to_uint16(ir) - full_array = np.dstack([rgb, ir]) - else: - full_array = np.ascontiguousarray(rgb) - - model = result.device_model - # (code, dtype, count, value, writeonce); NewSubfileType=0 is required or LibRaw rejects the DNG. - extratags = [ - (254, 4, 1, 0, True), # NewSubfileType - (50706, 1, 4, (1, 4, 0, 0), True), # DNGVersion - (50707, 1, 4, (1, 0, 0, 0), True), # DNGBackwardVersion - (274, 3, 1, 1, True), # Orientation - (271, 2, len(model) + 1, model, True), # Make - (272, 2, len(model) + 1, model, True), # Model - ] - payload = _encode_dng(full_array, extratags) - - fd, tmp_path = tempfile.mkstemp(suffix=_PART_SUFFIX, dir=os.path.dirname(path) or ".") - os.close(fd) - try: - with open(tmp_path, "wb") as fh: - fh.write(payload) - os.replace(tmp_path, path) - except Exception: - if os.path.exists(tmp_path): - os.unlink(tmp_path) - raise - - return path - - -def _encode_dng(full_array: np.ndarray, extratags: list) -> bytes: - """Encode an RGB(+IR) uint16 array as LinearRaw DNG bytes. - - RGB is written with the RGB photometric so tifffile emits a clean 3 *color* - samples with no ExtraSamples (matching pidng); the PhotometricInterpretation - tag is then patched to LinearRaw (34892), which DNG requires. Marking color - planes as ExtraSamples instead makes some raw processors treat the file as a - 1-channel sensor + aux planes and mis-demosaic it. - - The IR (4-sample) case keeps the LINEAR_RAW photometric with the extra planes - declared as extra samples — there the 4th plane genuinely is infrared, and - tifffile has no clean 4-color-sample form. - """ - buf = io.BytesIO() - if full_array.shape[-1] == 3: - tifffile.imwrite(buf, full_array, photometric=tifffile.PHOTOMETRIC.RGB, compression=None, metadata=None, extratags=extratags) - data = bytearray(buf.getvalue()) - with tifffile.TiffFile(io.BytesIO(bytes(data))) as tf: - offset = tf.pages[0].tags["PhotometricInterpretation"].valueoffset - byteorder = tf.byteorder - struct.pack_into(byteorder + "H", data, offset, 34892) # RGB(2) → LinearRaw(34892) - return bytes(data) - - extrasamples = (0,) * (full_array.shape[-1] - 1) - tifffile.imwrite( - buf, - full_array, - photometric=tifffile.PHOTOMETRIC.LINEAR_RAW, - compression=None, - metadata=None, - extrasamples=extrasamples, - extratags=extratags, - ) - return buf.getvalue() diff --git a/tests/scanners/test_scanner_settings.py b/tests/scanners/test_scanner_settings.py index 33d0396a7..9822ce6fe 100644 --- a/tests/scanners/test_scanner_settings.py +++ b/tests/scanners/test_scanner_settings.py @@ -144,3 +144,9 @@ def test_a_key_this_version_dropped_keeps_the_rest_of_the_blob(): def test_per_frame_offsets_round_trip_through_json_string_keys(): restored = ScannerSettings.from_dict({"frame_offsets": {"2": 0.4, "5": -0.3}}) assert restored.frame_offsets == {2: 0.4, 5: -0.3} + + +def test_a_saved_dng_output_format_lands_on_tiff(): + # DNG output is retired; the saved preference must not survive as an unknown format. + assert ScannerSettings.from_dict({"output_format": "DNG"}).output_format == "TIFF" + assert ScannerSettings.from_dict({"output_format": "TIFF (mono)"}).output_format == "TIFF (mono)" diff --git a/tests/scanners/test_service.py b/tests/scanners/test_service.py index e37d5a012..61298a490 100644 --- a/tests/scanners/test_service.py +++ b/tests/scanners/test_service.py @@ -224,6 +224,25 @@ def test_no_overwrite_increments(self) -> None: assert os.path.exists(path2) assert path1 != path2 + def test_the_mono_format_writes_one_plane_and_keeps_the_tif_extension(self) -> None: + import tempfile + + import numpy as np + import tifffile + + from negpy.infrastructure.scanners.result import ScanResult + from negpy.infrastructure.scanners.settings import MONO_TIFF + + result = ScanResult(rgb=np.zeros((8, 8, 3), dtype=np.uint16), ir=None, dpi=300, device_model="Test") + + with tempfile.TemporaryDirectory() as tmpdir: + service = ScannerService() + service._backend = FakeBackend() + path = service.write_result(result, tmpdir, '{{ date }}_{{ "%03d" % seq }}', MONO_TIFF) + + assert path.endswith(".tif") + assert tifffile.imread(path).shape == (8, 8) + def test_write_refuses_a_pattern_that_does_not_vary_with_sequence(self) -> None: import tempfile diff --git a/tests/scanners/test_writer.py b/tests/scanners/test_writer.py index e25513476..1a8515bab 100644 --- a/tests/scanners/test_writer.py +++ b/tests/scanners/test_writer.py @@ -1,4 +1,4 @@ -"""Tests for TIFF and DNG output writers.""" +"""Tests for the TIFF output writer.""" import os import tempfile @@ -7,7 +7,7 @@ import tifffile from negpy.infrastructure.scanners.result import ScanResult -from negpy.services.scanning.writer import write_dng_linear, write_tiff_16bit +from negpy.services.scanning.writer import write_tiff_16bit class TestTiffWriter: @@ -83,55 +83,6 @@ def test_converts_non_uint16(self) -> None: assert readback.dtype == np.uint16 -class TestDngWriter: - def test_writes_linear_dng(self) -> None: - rgb = np.random.randint(0, 65535, (200, 300, 3), dtype=np.uint16) - result = ScanResult(rgb=rgb, ir=None, dpi=3600, device_model="TestScanner") - - with tempfile.TemporaryDirectory() as tmpdir: - path = write_dng_linear(result, os.path.join(tmpdir, "test_scan")) - assert os.path.exists(path) - assert path.endswith(".dng") - - readback = tifffile.imread(path) - assert readback.shape == (200, 300, 3) - assert readback.dtype == np.uint16 - np.testing.assert_array_equal(readback, rgb) - - with tifffile.TiffFile(path) as tf: - tags = tf.pages[0].tags - assert int(tags["PhotometricInterpretation"].value) == 34892 # LinearRaw - assert tuple(tags["DNGVersion"].value) == (1, 4, 0, 0) - assert int(tags["SamplesPerPixel"].value) == 3 - # 3 plain color samples, no ExtraSamples (matches pidng); marking color - # planes as extra makes some raw processors mis-demosaic the file. - assert tags.get("ExtraSamples") is None - - def test_writes_dng_with_ir(self) -> None: - rgb = np.random.randint(0, 65535, (100, 150, 3), dtype=np.uint16) - ir = np.random.randint(0, 65535, (100, 150), dtype=np.uint16) - result = ScanResult(rgb=rgb, ir=ir, dpi=3600, device_model="TestScanner") - - with tempfile.TemporaryDirectory() as tmpdir: - path = write_dng_linear(result, os.path.join(tmpdir, "test_ir")) - assert os.path.exists(path) - - readback = tifffile.imread(path) - assert readback.shape == (100, 150, 4) - np.testing.assert_array_equal(readback[:, :, :3], rgb) - np.testing.assert_array_equal(readback[:, :, 3], ir) - with tifffile.TiffFile(path) as tf: - assert int(tf.pages[0].tags["SamplesPerPixel"].value) == 4 - - def test_adds_dng_extension(self) -> None: - rgb = np.random.randint(0, 65535, (50, 50, 3), dtype=np.uint16) - result = ScanResult(rgb=rgb, ir=None, dpi=300, device_model="T") - - with tempfile.TemporaryDirectory() as tmpdir: - path = write_dng_linear(result, os.path.join(tmpdir, "noext")) - assert path.endswith(".dng") - - class TestHotFolderSeesOnlyFinishedScans: """The output folder may be watched, and it indexes on extension alone.""" @@ -159,25 +110,37 @@ def watching_imwrite(file, data, **kwargs): assert offered # the probe ran, and only ever saw finished scans assert FolderWatchService.scan_for_new_files(tmpdir, set()) == [os.path.abspath(path)] - def test_the_dng_being_written_is_not_offered_to_the_watcher(self, monkeypatch) -> None: - from negpy.infrastructure.filesystem.watcher import FolderWatchService - rgb = np.random.randint(0, 65535, (60, 80, 3), dtype=np.uint16) +class TestMonoTiff: + def test_writes_one_grey_plane(self) -> None: + rgb = np.random.randint(0, 65535, (40, 60, 3), dtype=np.uint16) result = ScanResult(rgb=rgb, ir=None, dpi=3600, device_model="TestScanner") - probes = 0 - real_replace = os.replace with tempfile.TemporaryDirectory() as tmpdir: + path = write_tiff_16bit(result, os.path.join(tmpdir, "mono"), mono=True) - def watching_replace(src, dst): - # The written file is complete here, but still under its part name. - nonlocal probes - probes += 1 - assert FolderWatchService.scan_for_new_files(tmpdir, set()) == [] - real_replace(src, dst) + readback = tifffile.imread(path) + assert readback.shape == (40, 60) + assert readback.dtype == np.uint16 + with tifffile.TiffFile(path) as tf: + assert int(tf.pages[0].tags["PhotometricInterpretation"].value) == 1 # minisblack - monkeypatch.setattr(os, "replace", watching_replace) - path = write_dng_linear(result, os.path.join(tmpdir, "scan_002")) + def test_the_grey_plane_is_the_mean_of_the_three(self) -> None: + rgb = np.array([[[0, 0, 0], [10, 11, 12], [65535, 65535, 65535], [1, 2, 2]]], dtype=np.uint16) + result = ScanResult(rgb=rgb, ir=None, dpi=3600, device_model="TestScanner") - assert probes == 1 - assert FolderWatchService.scan_for_new_files(tmpdir, set()) == [os.path.abspath(path)] + with tempfile.TemporaryDirectory() as tmpdir: + path = write_tiff_16bit(result, os.path.join(tmpdir, "mean"), mono=True) + readback = tifffile.imread(path) + + # Rounded, not floored: 5/3 reads 2, and the top of the range does not wrap. + assert readback.tolist() == [[0, 11, 65535, 2]] + + def test_the_ir_sidecar_still_comes_out_beside_it(self) -> None: + rgb = np.random.randint(0, 65535, (20, 30, 3), dtype=np.uint16) + ir = np.random.randint(0, 65535, (20, 30), dtype=np.uint16) + result = ScanResult(rgb=rgb, ir=ir, dpi=3600, device_model="TestScanner") + + with tempfile.TemporaryDirectory() as tmpdir: + path = write_tiff_16bit(result, os.path.join(tmpdir, "mono_ir"), mono=True) + assert os.path.exists(path.replace(".tif", "_IR.tif")) diff --git a/tests/test_scan_sidebar.py b/tests/test_scan_sidebar.py index a5ebf164d..03ebb5294 100644 --- a/tests/test_scan_sidebar.py +++ b/tests/test_scan_sidebar.py @@ -1018,3 +1018,20 @@ def test_a_film_that_blocks_infrared_leaves_the_control_visible_to_explain_itsel assert sidebar.ir_check.isVisibleTo(sidebar) is True assert sidebar.ir_check.isEnabled() is False + + +def test_the_format_combo_offers_the_mono_tiff() -> None: + sidebar, _ = _sidebar(FULL_DEVICE) + offered = [sidebar.fmt_combo.itemText(i) for i in range(sidebar.fmt_combo.count())] + assert offered == ["TIFF", "TIFF (mono)"] + + +def test_the_chosen_format_reaches_the_batch_request() -> None: + sidebar, controller = _sidebar(LS50_DEVICE) + sidebar.folder_edit.setText("/tmp/negpy-scan-out") + sidebar.fmt_combo.setCurrentText("TIFF (mono)") + + sidebar._on_scan() + + _kind, req = controller.started[0] + assert req.output_format == "TIFF (mono)" diff --git a/tests/test_tiff_loader_encoding.py b/tests/test_tiff_loader_encoding.py index fda5c1ea1..b72311d1d 100644 --- a/tests/test_tiff_loader_encoding.py +++ b/tests/test_tiff_loader_encoding.py @@ -1,4 +1,4 @@ -"""Scanner TIFFs must decode identically to their LinearRaw DNG twins.""" +"""How a scanner TIFF is encoded, and what the loader makes of it.""" import base64 import os @@ -12,11 +12,9 @@ from negpy.infrastructure.display.color_spaces import WORKING_COLOR_SPACE from negpy.infrastructure.loaders.helpers import NonStandardFileWrapper from negpy.infrastructure.loaders.tiff_loader import TiffLoader -from negpy.infrastructure.scanners.result import ScanResult from negpy.kernel.image.logic import srgb_to_linear, working_oetf_decode from negpy.features.process.logic import effective_linear_raw from negpy.features.process.models import ProcessConfig, ProcessMode -from negpy.services.scanning.writer import write_dng_linear, write_tiff_16bit # The standard Adobe RGB (1998) ICC profile — real-world scanner/export software tags # TIFFs with exactly this, so the loader's Adobe RGB branch is tested against what @@ -182,34 +180,6 @@ def test_without_positive_source_the_tag_is_still_ignored(self) -> None: assert metadata["color_space"] is None -class TestScanRoundTripParity: - def test_tiff_and_dng_decode_identically(self) -> None: - from negpy.services.rendering.image_processor import ImageProcessor - - result = ScanResult(rgb=_rgb16(), ir=None, dpi=3600, device_model="TestScanner") - proc = ImageProcessor() - with tempfile.TemporaryDirectory() as tmpdir: - tif_path = write_tiff_16bit(result, os.path.join(tmpdir, "pair")) - dng_path = write_dng_linear(result, os.path.join(tmpdir, "pair")) - tif_rgb, _ = proc._decode_sensor_rgb(tif_path, linear_raw=True) - dng_rgb, _ = proc._decode_sensor_rgb(dng_path, linear_raw=True) - np.testing.assert_array_equal(tif_rgb, dng_rgb) - - def test_tiff_and_dng_agree_on_same_as_source_target(self) -> None: - """The twins must also export alike, not just decode alike.""" - from negpy.services.rendering.image_processor import ImageProcessor - - result = ScanResult(rgb=_rgb16(), ir=None, dpi=3600, device_model="TestScanner") - proc = ImageProcessor() - with tempfile.TemporaryDirectory() as tmpdir: - tif_path = write_tiff_16bit(result, os.path.join(tmpdir, "pair")) - dng_path = write_dng_linear(result, os.path.join(tmpdir, "pair")) - _, tif_meta = proc._decode_sensor_rgb(tif_path, linear_raw=True) - _, dng_meta = proc._decode_sensor_rgb(dng_path, linear_raw=True) - assert tif_meta.get("color_space") is None - assert dng_meta.get("color_space") is None - - class TestUncharacterisedSourceResolvesToWorkingSpace: """A source with no embedded profile is already in the working space: "Same as Source" must export it without a needless conversion into a narrower gamut.""" From 4dcbecf06bda20339d41e5216bbb73bea73a183f Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 2 Sep 2026 21:26:25 +0200 Subject: [PATCH 03/16] fix(scan): keep per-frame corrections a measured strip never redrew MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A per-frame correction reached the scan only while its tile existed. frame_offsets() read the sliders and nothing else, and a measured strip opens with no tiles at all — they appear when the strip is detected. Reopening the strip preview and accepting it without detecting therefore answered with an empty map and wiped every saved correction, so the batch scanned unshifted. A frame with no tile now keeps what was saved; a tile the operator can see overrides it, and zeroing a visible slider still clears it. The arithmetic was right the whole way down — dialog, preview fractions, batch millimetres and the stage rect all agree — so the tests that covered it all passed. They shared one blind spot: every dialog test built a device with an adapter capacity, which is a feeder, whose tiles are built eagerly. Nothing constructed a transport that measures the strip. There is a _discovery_device() helper now, and the end-to-end test drives the real sidebar values rather than a hand-built request. Alongside it, two things the strip preview was missing: Tiles get a size slider (90-340 px, remembered as strip_tile_height). The label letterboxes its kept pixmap at paint time, so resizing costs no scanning. The grid no longer wraps at a fixed six: columns are computed from the width, which is what lets a large tile stay on screen, and the dialog reflows when resized. Every previewed frame gets a thin outline. That box is the boundary the offset is measured from, and without it there was nothing to judge the offset against on a dark scan. It is drawn last so neither the shaded band nor a crop taken to the edge dims it, and it lives in the shared label, so the prescan and quick-scan previews get it too. The clamp warnings now name the frame's own slider, which they could not have caused before it existed. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7 --- docs/USER_GUIDE.md | 2 + negpy/desktop/view/sidebar/scan.py | 2 + .../desktop/view/widgets/scan_window_label.py | 9 + .../view/widgets/strip_preview_dialog.py | 109 ++++++++-- negpy/infrastructure/scanners/settings.py | 3 + tests/scanners/test_nkscan_backend.py | 16 ++ tests/test_scan_sidebar.py | 5 + tests/test_scan_window_label.py | 44 ++++ tests/test_strip_preview_dialog.py | 190 ++++++++++++++++++ 9 files changed, 367 insertions(+), 13 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 69419a26c..50d3091b6 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -1003,9 +1003,11 @@ Camera scanning needs the optional `python-gphoto2` dependency (`pip install gph Every preview dialog ends the same way: **Cancel**, then **Apply** (keep the framing and go back to the panel) and **Scan** (start the scan from here). The Apply button names what it keeps: **Apply Framing** on a strip, **Apply Window** on a single holder, **Apply Crop** after a Prescan. * **Cropping**: drag on a previewed frame. A corner resizes, inside moves. Each frame keeps its own window, and **Clear Crops** drops the lot. +* **Frame outline**: a thin grey box marks the detected frame on every previewed tile. That box is the boundary Offset, Drift and the per-frame slider are measured from, so it stays visible under the shaded band and any crop drawn to the edge. * **Offset**: slides every frame along the film to clear the inter-frame gap. Frames shift left as it grows, live. The shaded band on the right is film past the frame boundary the transport cannot deliver, so offset past the gap costs frame tail. A feeder cannot back up, so there it only goes one way. * **Drift**: adds progressively more (or less) offset per frame position, for a strip whose gaps creep along its length. Re-preview to refresh the pixels. * **Per-frame offset**: the slider under each tile corrects that frame alone, on top of Offset and Drift, for a boundary that sits off on its own. Its reading is in the tooltip; double-click resets it. +* **Size**: how big the tiles are drawn. The grid reflows to whatever fits the dialog, so larger tiles mean fewer per row. It costs no scanning — the tile is redrawn from the pixels already in hand — and the setting is remembered. Double-click resets it. * **Which frames**: each tile carries its own tick; **All** and **None** move the lot, and the count says how many will be scanned. On a measured strip the ticks and crops describe the piece of film in the transport, so ejecting clears them, per-frame offsets included; Offset and Drift survive, because they register the transport. --- diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index 2b121b68b..df92b0c5d 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -841,6 +841,7 @@ def _on_set_scan_window(self) -> None: initial_offset=self._settings.frame_offset_mm, initial_offset_modifier=self._settings.frame_offset_modifier_mm, initial_frame_offsets=self._settings.frame_offsets, + initial_tile_height=self._settings.strip_tile_height, film_format=self._film_format(), film_type=self._film_type(), parent=self, @@ -853,6 +854,7 @@ def _on_set_scan_window(self) -> None: frame_offset_mm=dialog.frame_offset(), frame_offset_modifier_mm=dialog.frame_offset_modifier(), frame_offsets=dialog.frame_offsets(), + strip_tile_height=dialog.tile_height(), ) self._update_scan_window_status() if dialog.scan_requested(): diff --git a/negpy/desktop/view/widgets/scan_window_label.py b/negpy/desktop/view/widgets/scan_window_label.py index d43520003..b77b22531 100644 --- a/negpy/desktop/view/widgets/scan_window_label.py +++ b/negpy/desktop/view/widgets/scan_window_label.py @@ -10,6 +10,7 @@ from PyQt6.QtWidgets import QLabel, QSizePolicy from negpy.desktop.view.styles.theme import THEME +from negpy.desktop.view.styles.theme import THEME from negpy.desktop.view.widgets.scan_window_geometry import ( Rect, hit_corner, @@ -20,6 +21,7 @@ _HANDLE_TOL = 0.03 # corner grab radius, fraction of frame _HANDLE_PX = 5 # drawn handle half-size, widget px +_FRAME_EDGE_ALPHA = 150 # the frame outline reads against the picture without competing with the crop class ScanWindowLabel(QLabel): @@ -207,6 +209,13 @@ def paintEvent(self, _ev) -> None: painter.drawRect(QRect(x, draw_rect.top(), max(0, draw_rect.right() - x), draw_rect.height())) painter.setPen(pen) painter.drawLine(x, draw_rect.top(), x, draw_rect.bottom()) + # Last, so neither the offset band nor a crop drawn to the edge dims it: this is the + # boundary the offset is measured from, and it has to stay readable on any frame. + boundary = QColor(THEME.text_secondary) + boundary.setAlpha(_FRAME_EDGE_ALPHA) + painter.setPen(QPen(boundary, 1)) + painter.setBrush(Qt.BrushStyle.NoBrush) + painter.drawRect(draw_rect.adjusted(0, 0, -1, -1)) else: painter.fillRect(self.rect(), QColor(THEME.bg_dark)) painter.end() diff --git a/negpy/desktop/view/widgets/strip_preview_dialog.py b/negpy/desktop/view/widgets/strip_preview_dialog.py index 99dfbcb3a..9c7c21c4f 100644 --- a/negpy/desktop/view/widgets/strip_preview_dialog.py +++ b/negpy/desktop/view/widgets/strip_preview_dialog.py @@ -43,9 +43,11 @@ _FRAME_LEN_MM = 36.0 _PREVIEW_FALLBACK_DPI = 500 # only when the device reports no DPI list at all _MAX_MEASURED_OFFSET_TENTHS = 25 # ±2.5 mm, in the slider's tenths of a millimetre -_TILE_H = 140 # constant tile height; width follows the device aspect +_TILE_H = 140 # default tile height; width follows the device aspect +_TILE_H_MIN, _TILE_H_MAX = 90, 340 # what the size slider spans +_GRID_MARGIN = 36 # dialog width the strip grid does not get: frame, scrollbar, padding _TILE_SLIDER_H = 18 # the per-frame offset slider under each tile -_TILES_PER_ROW = 6 # one SA-21 strip per row; roll adapters (up to 40 frames) wrap below +_TILES_PER_ROW = 6 # columns assumed before the grid has a width to measure # A transport that measures the strip reports its frame count only as previews arrive, so ask # for a roll's worth and keep the tiles it answers with. _DISCOVERY_SLOTS = 40 @@ -94,6 +96,7 @@ def _display_to_scan_rect(rect): ) _DRIFT_TIP = "Adds progressively more (or less) offset per frame position, for a strip whose gaps creep. Re-preview to refresh the pixels." _TILE_OFFSET_TIP = "Corrects this frame alone, on top of Offset and Drift. Double-click to reset." +_SIZE_TIP = "Tile size. The grid reflows to whatever fits the dialog. Double-click to reset." class _ResetSlider(QSlider): @@ -140,6 +143,7 @@ def __init__( initial_offset: float = 0.0, initial_offset_modifier: float = 0.0, initial_frame_offsets: dict[int, float] | None = None, + initial_tile_height: int = _TILE_H, film_format: str | None = None, film_type: str = "negative", parent=None, @@ -156,7 +160,7 @@ def __init__( self._rotation = 0 if self._discovers else _DISPLAY_ROTATION_DEG self._capacity = 0 if self._discovers else max(1, self._caps.adapter_frame_capacity or 1) # Landscape tile aspect (W/H) from the rotated raster: the feed axis (max_area_mm[1]) - # becomes horizontal. Tiles are constant-size at this aspect. + # becomes horizontal. Every tile keeps this aspect; the size slider drives the height. mm = self._caps.max_area_mm self._tile_aspect = (mm[1] / mm[0]) if (mm and len(mm) > 1 and mm[0]) else 1.5 self._previewing = False @@ -165,12 +169,16 @@ def __init__( initial_windows = initial_windows or {} initial_selected = tuple(initial_selected or ()) self._initial_frame_offsets = dict(initial_frame_offsets or {}) + self._tile_h = max(_TILE_H_MIN, min(_TILE_H_MAX, int(initial_tile_height))) + # Columns the grid is currently laid out in, and the stretch cells pinning it top-left. + self._cols = _TILES_PER_ROW + self._stretch: tuple[int, int] | None = None self.setWindowTitle("Preview Strip — Set a Window per Frame") self.setModal(True) tile_w, tile_h = self._tile_size() cols = min(self._capacity or _TILES_PER_ROW, _TILES_PER_ROW) rows = max(1, -(-self._capacity // _TILES_PER_ROW)) - self.resize(cols * (tile_w + 4) + 36, min(rows, 3) * (tile_h + _TILE_SLIDER_H + 4) + 260) + self.resize(cols * (tile_w + 4) + _GRID_MARGIN, min(rows, 3) * (tile_h + _TILE_SLIDER_H + 4) + 260) layout = QVBoxLayout(self) @@ -269,9 +277,7 @@ def __init__( self._initial_selected = initial_selected for frame in range(1, self._capacity + 1): self._ensure_tile(frame) - # Pin the grid top-left so a partial last row doesn't spread across the viewport. - strip.setColumnStretch(cols, 1) - strip.setRowStretch(rows, 1) + self._relayout(force=True) self._scroll.setWidget(container) layout.addWidget(self._scroll, 1) @@ -301,6 +307,16 @@ def __init__( self.clear_btn.clicked.connect(self._on_clear_all) btns.addWidget(self.clear_btn) btns.addStretch() + self.size_slider = _ResetSlider(_TILE_H) + self.size_slider.setRange(_TILE_H_MIN, _TILE_H_MAX) + self.size_slider.setSingleStep(10) + self.size_slider.setPageStep(40) + self.size_slider.setFixedWidth(120) + self.size_slider.setValue(self._tile_h) + self.size_slider.setToolTip(_SIZE_TIP) + btns.addWidget(QLabel("Size")) + btns.addWidget(self.size_slider) + btns.addSpacing(16) self.cancel_btn = QPushButton("Cancel") self.cancel_btn.clicked.connect(self._on_cancel_clicked) btns.addWidget(self.cancel_btn) @@ -323,6 +339,7 @@ def __init__( self._tiles_wired = True self.offset_slider.valueChanged.connect(self._on_offset_changed) self.drift_slider.valueChanged.connect(self._on_offset_changed) + self.size_slider.valueChanged.connect(self._on_tile_size_changed) self._on_offset_changed(self.offset_slider.value()) self._update_ok_enabled() @@ -338,10 +355,10 @@ def _ensure_tile(self, frame: int) -> _Tile: self._tiles[frame] = tile self._capacity = max(self._capacity, frame) self._empty_hint.setVisible(False) - self._strip.addWidget(tile.widget, (frame - 1) // _TILES_PER_ROW, (frame - 1) % _TILES_PER_ROW) if self._tiles_wired: tile.checkbox.toggled.connect(self._update_ok_enabled) self._update_ok_enabled() + self._relayout(force=True) return tile def _build_tile(self, frame: int, initial_window, checked: bool) -> _Tile: @@ -393,8 +410,48 @@ def _build_tile(self, frame: int, initial_window, checked: bool) -> _Tile: self._set_tile_offset_tooltip(tile) return tile + def _fitting_columns(self) -> int: + """Tiles that fit across the strip area, at least one.""" + tile_w = self._tile_size()[0] + # Until the dialog is shown its viewport carries a Qt default width unrelated to the + # size resize() asked for, so measure the dialog itself while that is the case. + viewport = self._scroll.viewport() + width = viewport.width() if (self.isVisible() and viewport is not None) else self.width() - _GRID_MARGIN + available = width - 4 # the grid's own left/right margins + if available < tile_w: + return 1 + return max(1, (available + 4) // (tile_w + 4)) # 4 = grid spacing + + def _relayout(self, *, force: bool = False) -> None: + """Re-place every tile at the current column count. A no-op while it has not changed.""" + cols = self._fitting_columns() + if cols == self._cols and not force: + return + self._cols = cols + for frame in sorted(self._tiles): + tile = self._tiles[frame] + self._strip.removeWidget(tile.widget) + self._strip.addWidget(tile.widget, (frame - 1) // cols, (frame - 1) % cols) + # Removed before it is re-added: adding a widget the grid already holds leaves the old + # cell behind as a second item. + self._strip.removeWidget(self._empty_hint) + self._strip.addWidget(self._empty_hint, 0, 0, 1, cols) + # Pin the grid top-left so a partial last row does not spread across the viewport. + # The previous pin has to be released or a now-occupied cell keeps stretching. + if self._stretch is not None: + self._strip.setColumnStretch(self._stretch[0], 0) + self._strip.setRowStretch(self._stretch[1], 0) + rows = max(1, -(-len(self._tiles) // cols)) + self._strip.setColumnStretch(cols, 1) + self._strip.setRowStretch(rows, 1) + self._stretch = (cols, rows) + + def resizeEvent(self, event) -> None: + super().resizeEvent(event) + self._relayout() + def _tile_size(self) -> tuple[int, int]: - return int(_TILE_H * self._tile_aspect), _TILE_H + return int(self._tile_h * self._tile_aspect), self._tile_h # ── result getters ──────────────────────────────────────────────── @@ -410,9 +467,24 @@ def _to_display(self, rect): def _to_scan(self, rect): return _display_to_scan_rect(rect) if self._rotation else rect + def tile_height(self) -> int: + return int(self.size_slider.value()) + def frame_offsets(self) -> dict[int, float]: - """Per-frame corrections, non-zero entries only.""" - return {f: t.offset_slider.value() / 10.0 for f, t in self._tiles.items() if t.offset_slider.value()} + """Per-frame corrections, non-zero entries only. + + A frame with no tile keeps whatever was saved for it. A measured strip opens with no + tiles at all, so reading the sliders alone would erase every correction the moment the + dialog was accepted without detecting the strip again. + """ + merged = dict(self._initial_frame_offsets) + for frame, tile in self._tiles.items(): + value = tile.offset_slider.value() / 10.0 + if value: + merged[frame] = value + else: + merged.pop(frame, None) # reset on a tile the operator could see is deliberate + return merged def frame_offset(self) -> float: return self.offset_slider.value() / 10.0 @@ -501,6 +573,15 @@ def _on_tile_offset_changed(self, frame: int) -> None: self._set_tile_offset_tooltip(tile) self._on_offset_changed(0) + def _on_tile_size_changed(self, value: int) -> None: + """Resize every tile in place. The label letterboxes its kept pixmap, so nothing rescans.""" + self._tile_h = int(value) + size = self._tile_size() + for tile in self._tiles.values(): + tile.label.setFixedSize(*size) + tile.offset_slider.setFixedWidth(size[0]) + self._relayout(force=True) + def _on_offset_changed(self, _value: int) -> None: self.offset_label.setText(f"{self.frame_offset():.1f} mm") self.drift_label.setText(f"{self.frame_offset_modifier():+.2f} mm/frame") @@ -561,13 +642,15 @@ def _refresh_offset_indicators(self) -> None: tile.label.set_coverage(self._tile_coverage(tile)) if clamped: frames = ", ".join(str(f) for f in clamped) - self.status_strip.set_message(f"{_CLAMP_NOTICE} on {plural(len(clamped), 'frame')} {frames} — reduce Offset or Drift.") + self.status_strip.set_message( + f"{_CLAMP_NOTICE} on {plural(len(clamped), 'frame')} {frames} — reduce Offset, Drift or that frame's own slider." + ) elif cut: frames = ", ".join(str(f) for f, _ in cut) worst = max(loss for _, loss in cut) self.status_strip.set_message( f"{_CUT_NOTICE} on {plural(len(cut), 'frame')} {frames} — up to {worst:.1f} mm of picture lost off the " - f"frame tail; reduce Offset, or re-feed the strip for a better registration." + f"frame tail; reduce Offset or that frame's own slider, or re-feed the strip for a better registration." ) elif self.status_strip.message().startswith((_CLAMP_NOTICE, _CUT_NOTICE)): self.status_strip.set_message("") diff --git a/negpy/infrastructure/scanners/settings.py b/negpy/infrastructure/scanners/settings.py index d8e3c601d..ea185e74b 100644 --- a/negpy/infrastructure/scanners/settings.py +++ b/negpy/infrastructure/scanners/settings.py @@ -51,6 +51,9 @@ class ScannerSettings: # Per-frame feed-axis correction (mm), added on top of frame_offset_mm + drift. An absent # key means no correction for that frame. frame_offsets: dict[int, float] = field(default_factory=dict) + # Height in px of one tile in the strip preview. Tile width follows the device aspect, and + # the grid reflows to whatever fits the dialog. + strip_tile_height: int = 140 def __post_init__(self) -> None: # JSON round-trips tuples as lists and dict keys as strings; coerce back. diff --git a/tests/scanners/test_nkscan_backend.py b/tests/scanners/test_nkscan_backend.py index d583d162c..043130b3c 100644 --- a/tests/scanners/test_nkscan_backend.py +++ b/tests/scanners/test_nkscan_backend.py @@ -515,3 +515,19 @@ def test_the_films_the_backend_names_are_films_the_extension_knows() -> None: backend, _ = make_backend() for film in FILM_TYPES: assert backend.locks_white_balance(film) == nkscan.Capabilities.locks_white_balance(film) + + +def test_a_per_frame_offset_slides_only_the_feed_axis_of_the_frame_asked_for() -> None: + """The rect handed to nkscan must move by the film distance the operator dialled, on the + feed axis alone, and keep the frame's own extent.""" + shift = round(0.7 * 4000 / 25.4) # 0.7 mm at the fake's optical dpi + for frame, offset_mm, expected in ((3, 0.0, 0), (3, 0.7, shift), (3, -0.7, -shift), (1, 0.7, shift)): + backend, module = make_backend() + _scan(backend, dataclasses.replace(_PARAMS, frame=frame, frame_offset_mm=offset_mm)) + + rect = module.opened[-1].scans[-1]["frame"] + detected = FRAMES[frame - 1] + assert rect[0] - detected[0] == expected + assert rect[2] - detected[2] == expected + assert (rect[1], rect[3]) == (detected[1], detected[3]) # across-film edges untouched + assert rect[2] - rect[0] == detected[2] - detected[0] # the frame keeps its length diff --git a/tests/test_scan_sidebar.py b/tests/test_scan_sidebar.py index 03ebb5294..3964a876b 100644 --- a/tests/test_scan_sidebar.py +++ b/tests/test_scan_sidebar.py @@ -1035,3 +1035,8 @@ def test_the_chosen_format_reaches_the_batch_request() -> None: _kind, req = controller.started[0] assert req.output_format == "TIFF (mono)" + + +def test_the_strip_tile_height_survives_the_dialog() -> None: + sidebar, _ = _sidebar(FULL_DEVICE, settings={"strip_tile_height": 260}) + assert sidebar.settings.strip_tile_height == 260 diff --git a/tests/test_scan_window_label.py b/tests/test_scan_window_label.py index 4b5fb0a4c..0e2762490 100644 --- a/tests/test_scan_window_label.py +++ b/tests/test_scan_window_label.py @@ -110,3 +110,47 @@ def test_crop_rect_round_trips_through_widget_pixels_under_an_offset() -> None: back = label._rect_in_widget((fx, fy, fx, fy), draw) assert abs(back.left() - point.x()) <= 1 + + +def _render(label: ScanWindowLabel): + from PyQt6.QtGui import QImage, QPainter + + shot = QImage(label.width(), label.height(), QImage.Format.Format_RGB32) + shot.fill(0) + painter = QPainter(shot) + label.render(painter) + painter.end() + return shot + + +def test_the_frame_boundary_is_outlined_once_there_is_a_frame() -> None: + """The offset is measured from the frame edge, so that edge has to be visible.""" + from PyQt6.QtGui import QColor + + label = ScanWindowLabel() + label.setFixedSize(120, 80) + black = QPixmap(120, 80) + black.fill(QColor("#000000")) + label.set_frame(black) + + shot = _render(label) + rect = label._display() + assert rect is not None + + on_edge = shot.pixelColor(rect.left(), rect.top() + rect.height() // 2) + inside = shot.pixelColor(rect.left() + rect.width() // 2, rect.top() + rect.height() // 2) + # A neutral line over the black frame, with the picture itself left alone. + assert on_edge.red() == on_edge.green() == on_edge.blue() + assert on_edge.red() > inside.red() + 40 + assert inside.red() == 0 + + +def test_an_empty_tile_is_not_outlined() -> None: + """Nothing was detected there, so there is no boundary to mark.""" + label = ScanWindowLabel() + label.setFixedSize(120, 80) + + shot = _render(label) + + assert label._display() is None + assert shot.pixelColor(0, 40).red() == shot.pixelColor(60, 40).red() diff --git a/tests/test_strip_preview_dialog.py b/tests/test_strip_preview_dialog.py index d94d5651a..968df15d3 100644 --- a/tests/test_strip_preview_dialog.py +++ b/tests/test_strip_preview_dialog.py @@ -21,6 +21,7 @@ from negpy.desktop.view.widgets.scan_preview_common import preview_positive from negpy.desktop.view.widgets.strip_preview_dialog import ( + _TILE_H_MAX, StripPreviewDialog, _display_to_scan_rect, _scan_to_display_rect, @@ -46,6 +47,21 @@ def _device(capacity: int) -> ScannerDevice: return ScannerDevice(id="coolscan3:usb:libusb:001:050", vendor="Nikon", model="LS-50", capabilities=caps) +def _discovery_device() -> ScannerDevice: + """A transport that measures the strip: no capacity, tiles grow from the previews.""" + caps = ScannerCapabilities( + ir_channel=False, + supported_dpi=(4000,), + supported_depths=(16,), + sources=(ScanMode.NEGATIVE,), + max_area_mm=(25.0, 38.0), + adapter_frame_capacity=None, + roll_discovery=True, + can_eject=True, + ) + return ScannerDevice(id="nkscan:usb:001", vendor="Nikon", model="LS-50", capabilities=caps) + + class _FakeController(QObject): scan_roll_preview_ready = pyqtSignal(object) scan_roll_preview_finished = pyqtSignal() @@ -945,3 +961,177 @@ def test_the_preview_request_carries_the_per_frame_correction() -> None: dialog._on_preview_all() assert controller.preview_reqs[0].offsets[2] == pytest.approx(1.9 / 38.0, abs=1e-4) + + +# ── the tile size slider ────────────────────────────────────────────────── + + +def test_tiles_start_at_the_saved_size() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(3), initial_tile_height=220) + + assert dialog.size_slider.value() == 220 + assert dialog._tiles[1].label.height() == 220 + assert dialog.tile_height() == 220 + + +def test_a_saved_size_outside_the_slider_is_clamped() -> None: + assert StripPreviewDialog(_FakeController(), _device(2), initial_tile_height=10_000).tile_height() == 340 + assert StripPreviewDialog(_FakeController(), _device(2), initial_tile_height=1).tile_height() == 90 + + +def test_moving_the_slider_resizes_every_tile_and_its_offset_slider() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(3)) + + dialog.size_slider.setValue(300) + + for tile in dialog._tiles.values(): + assert tile.label.height() == 300 + assert tile.label.width() == tile.offset_slider.width() + + +def _cell(dialog, frame: int) -> tuple[int, int]: + grid = dialog._strip + return grid.getItemPosition(grid.indexOf(dialog._tiles[frame].widget))[:2] + + +def test_the_grid_reflows_to_the_columns_that_fit() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(12)) + tile_w = dialog._tile_size()[0] + + dialog.resize(6 * (tile_w + 4) + 36, 600) + dialog._relayout(force=True) + assert dialog._cols == 6 + assert _cell(dialog, 7) == (1, 0) + + dialog.resize(2 * (tile_w + 4) + 36, 600) + dialog._relayout(force=True) + assert dialog._cols == 2 + assert _cell(dialog, 7) == (3, 0) + + +def test_a_strip_area_narrower_than_one_tile_still_shows_a_column() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(3)) + + dialog.resize(60, 600) + dialog._relayout(force=True) + + assert dialog._cols == 1 + + +def test_bigger_tiles_reflow_into_fewer_columns_at_a_fixed_width() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(12)) + dialog.resize(6 * (dialog._tile_size()[0] + 4) + 36, 600) + dialog._relayout(force=True) + assert dialog._cols == 6 + + dialog.size_slider.setValue(_TILE_H_MAX) + + assert dialog._cols < 6 + + +def test_resizing_the_tiles_does_not_rescan() -> None: + controller = _FakeController() + dialog = StripPreviewDialog(controller, _device(3)) + + dialog.size_slider.setValue(320) + + assert controller.preview_reqs == [] + + +def test_reflowing_does_not_accumulate_layout_items() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(6)) + before = dialog._strip.count() + + for width in (400, 900, 400, 1200): + dialog.resize(width, 600) + dialog._relayout(force=True) + + assert dialog._strip.count() == before + + +# ── the per-frame offset, end to end ────────────────────────────────────── + + +def test_the_preview_and_the_scan_land_on_the_same_millimetres() -> None: + """The tile the operator judges must be the film the batch then scans. + + Preview speaks fractions of a pitch and the scan speaks millimetres, so the two halves + are only equivalent if base, drift and the per-frame correction compose the same way. + """ + + from negpy.desktop.workers.scan_worker import BatchRequest, ScanWorker + from negpy.infrastructure.scanners.params import ScanParams + + controller = _FakeController() + dialog = StripPreviewDialog(controller, _device(5), initial_offset=1.0, initial_offset_modifier=0.2) + dialog._tiles[3].offset_slider.setValue(-7) # -0.7 mm on frame 3 alone + + dialog._on_preview_all() + pitch = 38.0 # _device's max_area_mm[1] + previewed = [controller.preview_reqs[0].offsets[f] * pitch for f in (1, 2, 3, 4, 5)] + + scanned: list[float] = [] + + class _Service: + def run_scan(self, device_id, params, progress, cancel): + scanned.append(params.frame_offset_mm) + return object() + + def write_result(self, **_kwargs): + return "/tmp/frame.tif" + + def eject(self, *_args, **_kwargs): + return True + + worker = ScanWorker() + worker._service = _Service() # type: ignore[assignment] + worker.run_batch( + BatchRequest( + device_id="coolscan3:test", + params=ScanParams(dpi=4000, depth=16, capture_ir=False, frame_offset_mm=dialog.frame_offset()), + output_folder="/tmp", + filename_pattern='{{ date }}_{{ "%03d" % seq }}', + output_format="TIFF", + frames=(1, 2, 3, 4, 5), + frame_offset_modifier_mm=dialog.frame_offset_modifier(), + frame_offsets=dialog.frame_offsets(), + ) + ) + + assert scanned == pytest.approx([1.0, 1.2, 0.7, 1.6, 1.8]) + assert scanned == pytest.approx(previewed) + + +def test_a_feeder_says_which_control_it_clamped() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(6), initial_offset=0.5) + + dialog._tiles[2].offset_slider.setValue(-20) # drives frame 2 below the transport's floor + dialog._refresh_offset_indicators() + + assert dialog._offset_for_frame(2) == 0.0 # the feeder cannot back up + assert "frame 2" in dialog.status_strip.message() + assert "own slider" in dialog.status_strip.message() + + +def test_accepting_without_detecting_keeps_the_saved_corrections() -> None: + """A measured strip opens with no tiles, and must not erase what was saved for them.""" + dialog = StripPreviewDialog(_FakeController(), _discovery_device(), initial_frame_offsets={3: -0.7}) + + assert dialog._tiles == {} + assert dialog.frame_offsets() == {3: -0.7} + + +def test_a_tile_the_operator_reset_drops_its_saved_correction() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(4), initial_frame_offsets={2: 0.5, 3: -0.4}) + + dialog._tiles[2].offset_slider.setValue(0) # deliberately cleared, with the tile in view + + assert dialog.frame_offsets() == {3: -0.4} + + +def test_a_detected_strip_overrides_the_saved_correction() -> None: + dialog = StripPreviewDialog(_FakeController(), _device(4), initial_frame_offsets={2: 0.5}) + + dialog._tiles[2].offset_slider.setValue(-9) + + assert dialog.frame_offsets() == {2: -0.9} From 8b5e90695effe59030743897aa8d8f0a87c6eb66 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 2 Sep 2026 21:36:48 +0200 Subject: [PATCH 04/16] feat(scan): log the offset every batch frame is scanned at Whether a per-frame correction reached the scan was only answerable by reading the written file and the settings blob back. One line per frame says it outright, which is what a report of "the offset did nothing" needs first. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7 --- negpy/desktop/workers/scan_worker.py | 1 + tests/test_scan_worker.py | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/negpy/desktop/workers/scan_worker.py b/negpy/desktop/workers/scan_worker.py index 5fce73fac..d80130187 100644 --- a/negpy/desktop/workers/scan_worker.py +++ b/negpy/desktop/workers/scan_worker.py @@ -227,6 +227,7 @@ def run_batch(self, req: BatchRequest) -> None: # one that re-addresses an absolute frame may legitimately go negative. offset = req.params.frame_offset_mm + (frame - 1) * req.frame_offset_modifier_mm + req.frame_offsets.get(frame, 0.0) frame_params = dataclasses.replace(req.params, frame=frame, window=window, frame_offset_mm=offset) + logger.info("Batch frame %d at %+.2f mm on the feed axis", frame, offset) base = index / total # The frame's position in the run rides on the phase string: a batch's global diff --git a/tests/test_scan_worker.py b/tests/test_scan_worker.py index dd21f82b5..133b1f0ab 100644 --- a/tests/test_scan_worker.py +++ b/tests/test_scan_worker.py @@ -626,3 +626,27 @@ def test_batch_progress_names_the_frame_and_its_position() -> None: "Frame 3 of 3 — Scanning", ] assert [fraction for fraction, _phase in seen] == pytest.approx([1 / 3, 2 / 3, 1.0]) + + +def test_the_batch_logs_the_offset_each_frame_was_scanned_at(caplog) -> None: + """Whether a per-frame correction reached the scan must be answerable from the log.""" + import logging + + worker = ScanWorker() + service = _BatchService() + worker._service = service # type: ignore[assignment] + req = BatchRequest( + device_id="coolscan3:test", + params=ScanParams(dpi=4_000, depth=16, capture_ir=False, frame_offset_mm=1.0), + output_folder="/tmp", + filename_pattern='scan-{{ "%03d" % seq }}', + output_format="TIFF", + frames=(1, 2), + frame_offsets={2: -0.5}, + ) + + with caplog.at_level(logging.INFO): + worker.run_batch(req) + + assert "Batch frame 1 at +1.00 mm" in caplog.text + assert "Batch frame 2 at +0.50 mm" in caplog.text From 3d01fb84593629fbb5bb15d034e2ae0cfe343a3b Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 2 Sep 2026 21:48:00 +0200 Subject: [PATCH 05/16] feat(scan): log the rect each nkscan frame is scanned at The offset reaches the scanner, and the delivered file moves the opposite way from the preview. Separating the two halves needs the rect that was actually sent, next to the one discovery reported, which no file or setting records. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7 --- negpy/infrastructure/scanners/nkscan_backend.py | 2 ++ tests/scanners/test_nkscan_backend.py | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/negpy/infrastructure/scanners/nkscan_backend.py b/negpy/infrastructure/scanners/nkscan_backend.py index 87ffa5707..3e8f3d128 100644 --- a/negpy/infrastructure/scanners/nkscan_backend.py +++ b/negpy/infrastructure/scanners/nkscan_backend.py @@ -361,8 +361,10 @@ def _scan_on_session( report = _progress_bridge(progress, cancel) rect = self._resolve_frame(session, device_id, params, report) optical = int(session.capabilities.optical_dpi) + detected = rect rect = _shift_frame(rect, _offset_units(params.frame_offset_mm, optical)) rect = _crop_frame(rect, params.window) + logger.info("Frame %s detected %s, scanning %s (%+0.2f mm)", params.frame, detected, rect, params.frame_offset_mm) with self._mapped_errors(): result = self.scan_frame( session, diff --git a/tests/scanners/test_nkscan_backend.py b/tests/scanners/test_nkscan_backend.py index 043130b3c..eba9f62ea 100644 --- a/tests/scanners/test_nkscan_backend.py +++ b/tests/scanners/test_nkscan_backend.py @@ -531,3 +531,15 @@ def test_a_per_frame_offset_slides_only_the_feed_axis_of_the_frame_asked_for() - assert rect[2] - detected[2] == expected assert (rect[1], rect[3]) == (detected[1], detected[3]) # across-film edges untouched assert rect[2] - rect[0] == detected[2] - detected[0] # the frame keeps its length + + +def test_the_scan_logs_the_detected_and_the_shifted_rect(caplog) -> None: + """Which rect a frame was actually scanned at has to be readable without the file.""" + import logging + + backend, _module = make_backend() + with caplog.at_level(logging.INFO): + _scan(backend, dataclasses.replace(_PARAMS, frame=2, frame_offset_mm=1.0)) + + assert "detected (10742, 0, 16410, 3945)" in caplog.text + assert "+1.00 mm" in caplog.text From 07d5b94ffdea079cf13da7173b6036dbe9e8eec4 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 2 Sep 2026 21:59:41 +0200 Subject: [PATCH 06/16] feat(scan): find a measured strip's frames as the dialog opens Every per-frame control lives on a tile, and a measured strip had no tiles until the operator pressed Detect frames, so reopening the strip preview to adjust one frame's offset offered nothing to adjust. The search now runs as the dialog opens, once, and only for a transport that measures the strip: previewing a feeder costs a scanner pass per frame, which is not something to start unasked. The rects and the strip pass are cached until the film is ejected and previews are cut from that pass, so only the first open per film costs anything. Detect frames stays, for a film that has moved. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7 --- docs/USER_GUIDE.md | 2 +- .../view/widgets/strip_preview_dialog.py | 19 +++++++++- tests/test_strip_preview_dialog.py | 38 ++++++++++++++++++- 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 50d3091b6..96d1261c6 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -970,7 +970,7 @@ With **IR** checked, color and infrared come back in one scan pass; pyopticfilm The default Full window includes a little holder chrome top and bottom; host-path scans clamp those near-white margins to the film highlight so auto exposure is not skewed. Raise **Analysis Buffer** or crop if a frame still looks off. Autofocus and hardware Auto-exposure controls stay hidden, because the SE does not report those capabilities. On Windows, bind the device to **WinUSB** with Zadig before use, since the stock vendor or SilverFast driver conflicts. The driver is the optional **pyopticfilm** package: install it with `uv sync --group plustek` or `pip install negpy[plustek]`; Windows release builds bundle it. See [PLUSTEK_WINDOWS.md](PLUSTEK_WINDOWS.md). -**Nikon Coolscan (nkscan)** notes: the driver talks to the scanner directly, so it needs no SANE backend. It measures the loaded film instead of counting frames: **Preview strip…** reads the whole strip in one pass, finds every frame on it, and cuts every tile out of that same pass. The tiles appear as the frames turn up, and there is no preview resolution to choose. Check the framing before scanning; a measured boundary can be nudged with **Offset** (±2.5 mm, either way, since the frame is re-addressed rather than fed past) and **Drift**, and because the tile comes out of the strip pass, a nudge re-frames without going back to the scanner. **Scan** with nothing picked scans every frame on the strip, measuring it first if no preview has. To scan a subset, type it in **Frames**, or untick frames in **Preview strip…**. Each tile carries its own tick, **All** and **None** move the lot, and the count next to them says how many will be scanned. Either way the selection shows in **Frames**, and ejecting the film clears it, since the frames and their crops describe the piece of film that just came out. **Offset** and **Drift** survive an eject, because they register the transport rather than one strip. Four controls appear only on this backend: +**Nikon Coolscan (nkscan)** notes: the driver talks to the scanner directly, so it needs no SANE backend. It measures the loaded film instead of counting frames: **Preview strip…** reads the whole strip in one pass, finds every frame on it, and cuts every tile out of that same pass. It starts that search as it opens, so the tiles and their per-frame controls are there without pressing anything; the rects and the strip pass are kept until the film is ejected, so every later open is free. **Detect frames** re-runs the search when the film has moved. The tiles appear as the frames turn up, and there is no preview resolution to choose. Check the framing before scanning; a measured boundary can be nudged with **Offset** (±2.5 mm, either way, since the frame is re-addressed rather than fed past) and **Drift**, and because the tile comes out of the strip pass, a nudge re-frames without going back to the scanner. **Scan** with nothing picked scans every frame on the strip, measuring it first if no preview has. To scan a subset, type it in **Frames**, or untick frames in **Preview strip…**. Each tile carries its own tick, **All** and **None** move the lot, and the count next to them says how many will be scanned. Either way the selection shows in **Frames**, and ejecting the film clears it, since the frames and their crops describe the piece of film that just came out. **Offset** and **Drift** survive an eject, because they register the transport rather than one strip. Four controls appear only on this backend: * **ICE**: remove dust and scratches with the infrared channel while scanning. Permanent, because it is baked into the file, unlike the Retouch panel's IR Restore, which stays editable. Color film only: silver grain blocks infrared, so the mask on a black-and-white negative is the picture again. **ICE** and **IR** exclude each other, because they read the same pass: ticking one unticks the other. Tick **IR** to keep the plane and clean the file later in Retouch, **ICE** to have the scanner do it now. * **Samples**: reads per line the scanner averages (1–16). Higher settings cut shadow noise and cost proportionally more time. diff --git a/negpy/desktop/view/widgets/strip_preview_dialog.py b/negpy/desktop/view/widgets/strip_preview_dialog.py index 9c7c21c4f..fa1a7c1cb 100644 --- a/negpy/desktop/view/widgets/strip_preview_dialog.py +++ b/negpy/desktop/view/widgets/strip_preview_dialog.py @@ -87,7 +87,7 @@ def _display_to_scan_rect(rect): # One line of orientation. Offset and Drift explain themselves on their own sliders, where # the hand already is, and the ⓘ carries the rest. _FEEDER_HELP = "Preview a frame, drag on it to crop, and tick the frames to scan." -_DISCOVERY_HELP = "Detect the frames, untick what you do not want, drag on a tile to crop it." +_DISCOVERY_HELP = "Untick what you do not want, drag on a tile to crop it, slide under a tile to shift it." _OFFSET_TIP = ( "Slides every frame along the film to clear the inter-frame gap. Frames shift left as it " @@ -164,6 +164,7 @@ def __init__( mm = self._caps.max_area_mm self._tile_aspect = (mm[1] / mm[0]) if (mm and len(mm) > 1 and mm[0]) else 1.5 self._previewing = False + self._detected_on_open = False self._failed_frames: list[int] = [] self._scan_now = False # set when the user chooses "Scan" over "Use" initial_windows = initial_windows or {} @@ -269,7 +270,7 @@ def __init__( self._tiles: dict[int, _Tile] = {} self._tiles_wired = False self._strip = strip - self._empty_hint = QLabel("Press Detect frames to measure the strip" if self._discovers else "Preview a frame to set its window") + self._empty_hint = QLabel("Finding the frames on the strip…" if self._discovers else "Preview a frame to set its window") self._empty_hint.setAlignment(Qt.AlignmentFlag.AlignCenter) self._empty_hint.setStyleSheet(f"color: {THEME.text_hint}; font-size: {THEME.font_size_base}px; padding: 48px;") strip.addWidget(self._empty_hint, 0, 0, 1, _TILES_PER_ROW) @@ -450,6 +451,20 @@ def resizeEvent(self, event) -> None: super().resizeEvent(event) self._relayout() + def showEvent(self, event) -> None: + """Find the frames as the dialog opens, once. + + A measured strip has no tiles until it is measured, and every per-frame control lives + on a tile, so an operator reopening this dialog to adjust one had nothing to adjust. + The rects and the strip pass are cached until the film is ejected, so this is free for + every open after the first. + """ + super().showEvent(event) + self._relayout() + if self._discovers and not self._detected_on_open: + self._detected_on_open = True + self._on_preview_all() + def _tile_size(self) -> tuple[int, int]: return int(self._tile_h * self._tile_aspect), self._tile_h diff --git a/tests/test_strip_preview_dialog.py b/tests/test_strip_preview_dialog.py index 968df15d3..03f130305 100644 --- a/tests/test_strip_preview_dialog.py +++ b/tests/test_strip_preview_dialog.py @@ -741,10 +741,10 @@ def test_a_measured_strip_crop_needs_no_axis_swap() -> None: # ── picking the frames to scan ──────────────────────────────────────────── -def test_an_undetected_strip_says_what_to_press() -> None: +def test_an_unmeasured_strip_says_it_is_working_rather_than_what_to_press() -> None: dialog = StripPreviewDialog(_FakeController(), _discovery_device()) assert dialog._empty_hint.isVisibleTo(dialog) is True - assert "Detect frames" in dialog._empty_hint.text() + assert "Finding the frames" in dialog._empty_hint.text() assert dialog.selection_label.text() == "none yet" @@ -1135,3 +1135,37 @@ def test_a_detected_strip_overrides_the_saved_correction() -> None: dialog._tiles[2].offset_slider.setValue(-9) assert dialog.frame_offsets() == {2: -0.9} + + +# ── finding the frames as the dialog opens ──────────────────────────────── + + +def test_a_measured_strip_finds_its_frames_as_it_opens() -> None: + """Every per-frame control lives on a tile, so an empty dialog offers nothing to adjust.""" + controller = _FakeController() + dialog = StripPreviewDialog(controller, _discovery_device()) + assert controller.preview_reqs == [] # nothing before it is on screen + + dialog.show() + + assert len(controller.preview_reqs) == 1 + + +def test_it_only_finds_them_once_however_often_the_dialog_is_shown() -> None: + controller = _FakeController() + dialog = StripPreviewDialog(controller, _discovery_device()) + + dialog.show() + dialog.hide() + dialog.show() + + assert len(controller.preview_reqs) == 1 + + +def test_a_feeder_is_left_alone_because_previewing_it_costs_a_pass_per_frame() -> None: + controller = _FakeController() + dialog = StripPreviewDialog(controller, _device(6)) + + dialog.show() + + assert controller.preview_reqs == [] From eb7ce9dbec5d9f52ae068b2c3363b46be2287bac Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 2 Sep 2026 22:37:49 +0200 Subject: [PATCH 07/16] fix(scan): let a measured strip's offset reach a boundary that sits far out The per-frame correction cut exactly what it was given, and still left a clear strip on two frames of a test roll: their boundaries sat 876 and 1339 stage units off the picture, 5.6 mm and 8.5 mm at 4000 dpi, and the control stopped at 2.5 mm. A feeder already reached 10 mm on the same slider, while a measured strip, the transport that can re-address a rect in either direction, was held to a quarter of that. Both span 10 mm now, still far short of the frame pitch, so the offset stays a boundary correction rather than a way to the next frame. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7 --- docs/USER_GUIDE.md | 2 +- .../view/widgets/strip_preview_dialog.py | 4 ++- tests/test_strip_preview_dialog.py | 26 ++++++++++++++++--- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 96d1261c6..cf1362754 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -970,7 +970,7 @@ With **IR** checked, color and infrared come back in one scan pass; pyopticfilm The default Full window includes a little holder chrome top and bottom; host-path scans clamp those near-white margins to the film highlight so auto exposure is not skewed. Raise **Analysis Buffer** or crop if a frame still looks off. Autofocus and hardware Auto-exposure controls stay hidden, because the SE does not report those capabilities. On Windows, bind the device to **WinUSB** with Zadig before use, since the stock vendor or SilverFast driver conflicts. The driver is the optional **pyopticfilm** package: install it with `uv sync --group plustek` or `pip install negpy[plustek]`; Windows release builds bundle it. See [PLUSTEK_WINDOWS.md](PLUSTEK_WINDOWS.md). -**Nikon Coolscan (nkscan)** notes: the driver talks to the scanner directly, so it needs no SANE backend. It measures the loaded film instead of counting frames: **Preview strip…** reads the whole strip in one pass, finds every frame on it, and cuts every tile out of that same pass. It starts that search as it opens, so the tiles and their per-frame controls are there without pressing anything; the rects and the strip pass are kept until the film is ejected, so every later open is free. **Detect frames** re-runs the search when the film has moved. The tiles appear as the frames turn up, and there is no preview resolution to choose. Check the framing before scanning; a measured boundary can be nudged with **Offset** (±2.5 mm, either way, since the frame is re-addressed rather than fed past) and **Drift**, and because the tile comes out of the strip pass, a nudge re-frames without going back to the scanner. **Scan** with nothing picked scans every frame on the strip, measuring it first if no preview has. To scan a subset, type it in **Frames**, or untick frames in **Preview strip…**. Each tile carries its own tick, **All** and **None** move the lot, and the count next to them says how many will be scanned. Either way the selection shows in **Frames**, and ejecting the film clears it, since the frames and their crops describe the piece of film that just came out. **Offset** and **Drift** survive an eject, because they register the transport rather than one strip. Four controls appear only on this backend: +**Nikon Coolscan (nkscan)** notes: the driver talks to the scanner directly, so it needs no SANE backend. It measures the loaded film instead of counting frames: **Preview strip…** reads the whole strip in one pass, finds every frame on it, and cuts every tile out of that same pass. It starts that search as it opens, so the tiles and their per-frame controls are there without pressing anything; the rects and the strip pass are kept until the film is ejected, so every later open is free. **Detect frames** re-runs the search when the film has moved. The tiles appear as the frames turn up, and there is no preview resolution to choose. Check the framing before scanning; a measured boundary can be nudged with **Offset** (±10 mm, either way, since the frame is re-addressed rather than fed past) and **Drift**, and because the tile comes out of the strip pass, a nudge re-frames without going back to the scanner. **Scan** with nothing picked scans every frame on the strip, measuring it first if no preview has. To scan a subset, type it in **Frames**, or untick frames in **Preview strip…**. Each tile carries its own tick, **All** and **None** move the lot, and the count next to them says how many will be scanned. Either way the selection shows in **Frames**, and ejecting the film clears it, since the frames and their crops describe the piece of film that just came out. **Offset** and **Drift** survive an eject, because they register the transport rather than one strip. Four controls appear only on this backend: * **ICE**: remove dust and scratches with the infrared channel while scanning. Permanent, because it is baked into the file, unlike the Retouch panel's IR Restore, which stays editable. Color film only: silver grain blocks infrared, so the mask on a black-and-white negative is the picture again. **ICE** and **IR** exclude each other, because they read the same pass: ticking one unticks the other. Tick **IR** to keep the plane and clean the file later in Retouch, **ICE** to have the scanner do it now. * **Samples**: reads per line the scanner averages (1–16). Higher settings cut shadow noise and cost proportionally more time. diff --git a/negpy/desktop/view/widgets/strip_preview_dialog.py b/negpy/desktop/view/widgets/strip_preview_dialog.py index fa1a7c1cb..06fe6d616 100644 --- a/negpy/desktop/view/widgets/strip_preview_dialog.py +++ b/negpy/desktop/view/widgets/strip_preview_dialog.py @@ -42,7 +42,9 @@ # (pitch - frame) discards that much picture off the frame tail. _FRAME_LEN_MM = 36.0 _PREVIEW_FALLBACK_DPI = 500 # only when the device reports no DPI list at all -_MAX_MEASURED_OFFSET_TENTHS = 25 # ±2.5 mm, in the slider's tenths of a millimetre +# ±10 mm, in the slider's tenths of a millimetre. A feeder's own range is already 0..10 mm, +# and a measured boundary can sit several millimetres off the picture, so the two match. +_MAX_MEASURED_OFFSET_TENTHS = 100 _TILE_H = 140 # default tile height; width follows the device aspect _TILE_H_MIN, _TILE_H_MAX = 90, 340 # what the size slider spans _GRID_MARGIN = 36 # dialog width the strip grid does not get: frame, scrollbar, padding diff --git a/tests/test_strip_preview_dialog.py b/tests/test_strip_preview_dialog.py index 03f130305..776c9b6a9 100644 --- a/tests/test_strip_preview_dialog.py +++ b/tests/test_strip_preview_dialog.py @@ -894,11 +894,13 @@ def test_a_feeder_still_counts_the_frames_it_previews() -> None: def test_the_offset_is_a_boundary_correction_not_a_way_to_the_next_frame() -> None: + # 10 mm reaches a badly placed boundary and still stops far short of the ~36 mm pitch. measured = StripPreviewDialog(_FakeController(), _discovery_device()) - assert (measured.offset_slider.minimum(), measured.offset_slider.maximum()) == (-25, 25) + assert (measured.offset_slider.minimum(), measured.offset_slider.maximum()) == (-100, 100) - measured.offset_slider.setValue(400) - assert measured.frame_offset() == 2.5 + measured.offset_slider.setValue(4000) + assert measured.frame_offset() == 10.0 + assert measured.frame_offset() < measured._frame_pitch() def test_a_saved_negative_offset_comes_back_as_it_was() -> None: @@ -1169,3 +1171,21 @@ def test_a_feeder_is_left_alone_because_previewing_it_costs_a_pass_per_frame() - dialog.show() assert controller.preview_reqs == [] + + +def test_the_offset_sliders_reach_a_boundary_several_millimetres_out() -> None: + """A measured boundary can sit well off the picture, and the slider has to reach it. + + Observed on a real strip: gaps of 876 and 1339 stage units (5.6 mm and 8.5 mm) at 4000 dpi, + which a +-2.5 mm control could not cut. + """ + dialog = StripPreviewDialog(_FakeController(), _discovery_device()) + ctl = _FakeController() + grown = StripPreviewDialog(ctl, _device(3)) + + assert dialog.offset_slider.minimum() == -100 and dialog.offset_slider.maximum() == 100 + # A feeder cannot back up, and its forward reach is unchanged. + assert grown.offset_slider.minimum() == 0 and grown.offset_slider.maximum() == 100 + tile = grown._tiles[1].offset_slider + tile.setValue(85) + assert grown.frame_offsets() == {1: 8.5} From 1b8716aa8866c5f774466845f34e6aad55a144fa Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Fri, 4 Sep 2026 21:40:08 +0200 Subject: [PATCH 08/16] chore(deps): require nkscan 0.10 0.10 rewrites the USB/SCSI transport's busy handling: it polls a busy unit's phase rather than resending the command, paces the retries, gives a session's first command a warm-up budget, and keeps every READ on a granule boundary. It also fixes low-DPI scans on the LS-5000. The Python API is unchanged. Claude-Session: https://claude.ai/code/session_01XqiQLys9TYeFmJSguFHgmV --- pyproject.toml | 4 ++-- uv.lock | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e5410d300..094ca9588 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,7 +40,7 @@ classifiers = [ ] [project.optional-dependencies] -nkscan = ["nkscan>=0.9"] +nkscan = ["nkscan>=0.10"] plustek = ["pyopticfilm>=1.3.3"] sane = ["python-sane>=2.9"] camera = ["gphoto2>=2.5 ; sys_platform != 'win32'"] @@ -69,7 +69,7 @@ pieusb = [ ] nkscan = [ # Nikon Coolscan over SCSI/USB. A Rust extension, shipped as wheels. - "nkscan>=0.9", + "nkscan>=0.10", ] [project.urls] diff --git a/uv.lock b/uv.lock index 91b19f00f..867f6d3be 100644 --- a/uv.lock +++ b/uv.lock @@ -382,7 +382,7 @@ requires-dist = [ { name = "imagecodecs", specifier = "==2026.6.6" }, { name = "imageio", specifier = "==2.37.3" }, { name = "jinja2", specifier = "==3.1.6" }, - { name = "nkscan", marker = "extra == 'nkscan'", specifier = ">=0.9" }, + { name = "nkscan", marker = "extra == 'nkscan'", specifier = ">=0.10" }, { name = "numba", specifier = "==0.65.1" }, { name = "numpy", specifier = "==2.4.4" }, { name = "opencv-python-headless", specifier = "==4.13.0.92" }, @@ -409,21 +409,21 @@ dev = [ { name = "ruff", specifier = "==0.14.10" }, { name = "ty", specifier = ">=0.0.26" }, ] -nkscan = [{ name = "nkscan", specifier = ">=0.9" }] +nkscan = [{ name = "nkscan", specifier = ">=0.10" }] pieusb = [{ name = "pieusb", specifier = ">=0.3.7" }] plustek = [{ name = "pyopticfilm", specifier = ">=1.3.3" }] sane = [{ name = "python-sane", specifier = ">=2.9" }] [[package]] name = "nkscan" -version = "0.9.0" +version = "0.10.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/33/4d/8b02d8e3a7ce1604f9d3b1ae94e31fac5850b7daaa484a81b2f8f78ab6e9/nkscan-0.9.0.tar.gz", hash = "sha256:b38d89afc0d6bfe3473294db587f58fa9fe8a9c6a9017561105a633d8cd355a8", size = 2804597, upload-time = "2026-08-24T01:28:37.411Z" } +sdist = { url = "https://files.pythonhosted.org/packages/f7/cc/4642272d170d374924e2cc9f94486da83fa4fceec2b5b5911983b805a469/nkscan-0.10.0.tar.gz", hash = "sha256:57079bcf7dd97ae2e0f4f4ea04ea74126103af2f3433b99a09414254bae5ee74", size = 2808736, upload-time = "2026-09-04T19:13:45.82Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/a3/c3/29574ec5e7dea0aac7f9fa65e4573711f0f0099bd8843146973fb09d151b/nkscan-0.9.0-cp313-abi3-macosx_10_12_x86_64.whl", hash = "sha256:c0cf978009af4f0ca2c59aa97926ba8e9d7291bf57fdef0247c9224c5587520d", size = 743459, upload-time = "2026-08-26T00:41:14.069Z" }, - { url = "https://files.pythonhosted.org/packages/b6/50/483d48da107ac514b8fad9ca5caff4f1d20dc048f2a4e4822259d49c5ae2/nkscan-0.9.0-cp313-abi3-macosx_11_0_arm64.whl", hash = "sha256:1f7be405c2b601cdf942f4e87e8b16eb2a6702ae5c10dc145947c4e4cdaf7a49", size = 735425, upload-time = "2026-08-24T01:28:32.919Z" }, - { url = "https://files.pythonhosted.org/packages/43/b7/92008b81db30187f1461d6fbdb8c313713001a40cdad82bbaba0bdbe19e7/nkscan-0.9.0-cp313-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:f4c0550b73e18393461f9073d4ab75ecd9ff8ae2042ebb5c5028271eeeb1f10a", size = 836903, upload-time = "2026-08-24T01:28:34.506Z" }, - { url = "https://files.pythonhosted.org/packages/1c/94/f53841c7168ebea01f16ab758714f70da348ef56b2e0204024babd56d358/nkscan-0.9.0-cp313-abi3-win_amd64.whl", hash = "sha256:dff231a33cbd3164c9d116b5bbf6ca9a7c070f8d386330833f89266a42ed7f08", size = 595336, upload-time = "2026-08-24T01:28:35.973Z" }, + { url = "https://files.pythonhosted.org/packages/12/76/b68a6941616ff9e41343cebc4374bc8721a511ff472ea8c98f711bc49000/nkscan-0.10.0-cp313-abi3-macosx_10_12_x86_64.whl", hash = "sha256:5c6accfb802f9f851aa9a65a15b46be802a49f8fc642d07b8d5a540b6f8e30c2", size = 750584, upload-time = "2026-09-04T19:13:39.924Z" }, + { url = "https://files.pythonhosted.org/packages/a7/21/3c19378359b85a768d57fb2020377a31c39b9ad71d143ed77eb70a3cbbce/nkscan-0.10.0-cp313-abi3-macosx_11_0_arm64.whl", hash = "sha256:1de8491669add3e35e5dc6ed8928193b49bbbb5024f121de55977321dcd93975", size = 733790, upload-time = "2026-09-04T19:13:41.346Z" }, + { url = "https://files.pythonhosted.org/packages/28/6b/7362d7eb671a132923f454e74815457853bf74484774dd7a3f40084b45cd/nkscan-0.10.0-cp313-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:a46bbce25224e57f491ba326a8346f496fb4028262f30adc9f8eb3621b31ae7d", size = 842336, upload-time = "2026-09-04T19:13:42.799Z" }, + { url = "https://files.pythonhosted.org/packages/98/eb/9e25d21a957bee0c1e9c2d0513b935354688fc497d3b75bed2830902eb74/nkscan-0.10.0-cp313-abi3-win_amd64.whl", hash = "sha256:67184353e2a25e2be5d8fd32b335a43006178041e61db20c4baea2528c3da19f", size = 602004, upload-time = "2026-09-04T19:13:44.414Z" }, ] [[package]] From 8617e7ad2607871b151213e781bbd5c265c5cc03 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Sat, 5 Sep 2026 22:31:27 +0200 Subject: [PATCH 09/16] fix(scan): cut a strip tile at the pitch the pass was laid out with A thumbnail column is one line pitch of film, and the pitch is a whole number of stage addresses. Deriving the scale from the rect and the row count gave 41.09 where the pass uses 41, so every tile sat further off the film it names the further down the strip it was, reaching 0.43 mm by the last frame of a six-frame strip. Every frame top a real LS-50 reported divides by 41 exactly. The fake modelled the same wrong formula, so it agreed with the bug; it now lays its strip out the way a unit does. Claude-Session: https://claude.ai/code/session_012UXETEHMjMAqRhQ1sZdNA7 --- negpy/infrastructure/scanners/nkscan_roll.py | 24 ++++++----- tests/scanners/fake_nkscan.py | 9 ++--- tests/scanners/test_nkscan_roll.py | 42 +++++++++++++++++++- 3 files changed, 58 insertions(+), 17 deletions(-) diff --git a/negpy/infrastructure/scanners/nkscan_roll.py b/negpy/infrastructure/scanners/nkscan_roll.py index 80713457b..601afa39f 100644 --- a/negpy/infrastructure/scanners/nkscan_roll.py +++ b/negpy/infrastructure/scanners/nkscan_roll.py @@ -26,15 +26,16 @@ _PREVIEW_DEPTH_DPI = 0 # the strip pass has its own resolution; nothing chooses it -def thumbnail_scale(rect: tuple[int, int, int, int], rows: int) -> float: - """Stage addresses per thumbnail pixel. +def thumbnail_scale(optical_dpi: int, thumbnail_dpi: int) -> float: + """Stage addresses per thumbnail column. - The strip pass covers the adapter's opening across the film and the whole feed axis along - it, at one resolution on both axes, and every measured rect spans that same opening. So the - frame's width over the pass's row count is the scale, and a column is a feed address. + A column is one line pitch of film and the pass starts at the axis origin, so a column is + a feed address. The pitch is a whole number of addresses; the resolution the unit reports + for the pass is that pitch rounded down, so the trip back through it has to round. """ - _top, left, _bottom, right = rect - return (right - left) / rows if rows else 0.0 + if optical_dpi <= 0 or thumbnail_dpi <= 0: + return 0.0 + return float(round(optical_dpi / thumbnail_dpi)) def slice_frame(strip: np.ndarray, rect: tuple[int, int, int, int], scale: float) -> np.ndarray | None: @@ -119,15 +120,16 @@ def _preview_one(self, slot: int, cancel: threading.Event) -> np.ndarray: rect = self._rect(slot) strip = self.thumbnail if strip is not None: - tile = slice_frame(strip, rect, self._scale(strip)) + tile = slice_frame(strip, rect, self._scale()) if tile is not None: return tile logger.info("Slot %s falls outside the strip pass; scanning it instead", slot) return self._scan_preview(rect, cancel) - def _scale(self, strip: np.ndarray) -> float: - frames = self._backend.frames(self._device.id) - return thumbnail_scale(frames[0], strip.shape[0]) if frames else 0.0 + def _scale(self) -> float: + caps = self._session.capabilities + dpi = tuple(caps.thumbnail_dpi) + return thumbnail_scale(int(caps.optical_dpi), int(dpi[0]) if dpi else 0) def _scan_preview(self, rect: tuple[int, int, int, int], cancel: threading.Event) -> np.ndarray: """A pass of one frame, for a mechanism that measured the film without a strip pass.""" diff --git a/tests/scanners/fake_nkscan.py b/tests/scanners/fake_nkscan.py index 352932def..7f8b1ccae 100644 --- a/tests/scanners/fake_nkscan.py +++ b/tests/scanners/fake_nkscan.py @@ -152,14 +152,13 @@ def list_devices(self) -> list[FakeDevice]: def strip_pass(self) -> dict[str, np.ndarray] | None: """The whole-strip pass, laid out the way the unit delivers one. - Columns are feed addresses from the axis start, at the same resolution as the rows, - which span the adapter opening. Each frame's band carries its own slot number, so a - test can tell which part of the strip a tile was cut from. + A column is one line pitch of film, counted from the axis start, the same way the unit + lays one out. Each frame's band carries its own slot number, so a test can tell which + part of the strip a tile was cut from. """ if not self.thumbnail or not self.frames: return None - top, left, _bottom, right = self.frames[0] - scale = (right - left) / self.rows + scale = round(self.caps.optical_dpi / self.caps.thumbnail_dpi[0]) cols = int(max(f[2] for f in self.frames) / scale) + self.strip_slack plane = np.zeros((self.rows, cols), np.uint16) for slot, (top, _l, bottom, _r) in enumerate(self.frames, 1): diff --git a/tests/scanners/test_nkscan_roll.py b/tests/scanners/test_nkscan_roll.py index 2317881d6..14209604e 100644 --- a/tests/scanners/test_nkscan_roll.py +++ b/tests/scanners/test_nkscan_roll.py @@ -4,10 +4,11 @@ import threading +import numpy as np import pytest from negpy.infrastructure.scanners.base import ScannerDevice -from negpy.infrastructure.scanners.nkscan_roll import NkscanRollSession +from negpy.infrastructure.scanners.nkscan_roll import NkscanRollSession, thumbnail_scale from tests.scanners import fake_nkscan from tests.scanners.fake_nkscan import DEVICE_ID, FRAMES, make_backend @@ -230,3 +231,42 @@ def test_ejecting_forgets_the_strip_pass_because_that_film_is_gone() -> None: backend.eject(device.id) assert backend.strip_pass(device.id) is None + + +def test_a_thumbnail_column_is_a_whole_line_pitch() -> None: + """The pitch is a whole number of addresses, and the reported resolution rounds it down. + + An LS-50 answers 97 dpi for a 4000 dpi unit, where the pitch it actually laid the pass out + with is 41: every frame top a real strip reported was an exact multiple of it. Carrying the + unrounded 41.24, or a scale taken across the film instead, walks the tile off the film it + names, further with every frame down the strip. + """ + assert thumbnail_scale(4000, 97) == 41.0 + assert thumbnail_scale(4000, 250) == 16.0 + assert thumbnail_scale(4000, 0) == 0.0 + assert thumbnail_scale(0, 97) == 0.0 + + # Tops an LS-50 measured on a real strip, which the pitch has to divide exactly. + for top in (246, 6109, 12013, 17917, 23821, 29725): + assert top % int(thumbnail_scale(4000, 97)) == 0 + + +def test_a_tile_is_cut_at_the_address_the_scan_will_use() -> None: + """The tile and the fine scan must name the same film, or the operator judges the wrong one.""" + backend, module = make_backend() + device = backend.list_devices()[0] + session = backend.open_roll(device, dpi=500) + try: + previews = _previews(session) + finally: + session.close() + + scale = int(round(module.caps.optical_dpi / module.caps.thumbnail_dpi[0])) + strip = backend.strip_pass(device.id) + assert strip is not None + for preview, (slot, rect) in zip(previews, enumerate(module.frames, 1)): + expected = strip[:, round(rect[0] / scale) : round(rect[2] / scale)] + assert preview.rgb.shape[1] == expected.shape[1] + # Every band carries its own slot, so a tile cut at the wrong column opens on the gap. + assert set(np.unique(preview.rgb[:, 0])) == {slot} + assert set(np.unique(preview.rgb)) <= {0, slot} From f459107ccc03d83804c449288e6b9dacdfe9bf35 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Sat, 5 Sep 2026 22:31:39 +0200 Subject: [PATCH 10/16] feat(scan): draw the detected frame outline in the house red The boundary every offset control is measured from was a grey box at partial alpha, which is hard to pick out against a negative's own low contrast. It is the reference the operator judges the offset against, so it gets the accent colour at full opacity, and stays drawn last so neither the offset band nor a crop taken to the edge dims it. Claude-Session: https://claude.ai/code/session_012UXETEHMjMAqRhQ1sZdNA7 --- docs/USER_GUIDE.md | 2 +- negpy/desktop/view/widgets/scan_window_label.py | 5 +---- tests/test_scan_window_label.py | 7 ++++--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index cf1362754..618d699cb 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -1003,7 +1003,7 @@ Camera scanning needs the optional `python-gphoto2` dependency (`pip install gph Every preview dialog ends the same way: **Cancel**, then **Apply** (keep the framing and go back to the panel) and **Scan** (start the scan from here). The Apply button names what it keeps: **Apply Framing** on a strip, **Apply Window** on a single holder, **Apply Crop** after a Prescan. * **Cropping**: drag on a previewed frame. A corner resizes, inside moves. Each frame keeps its own window, and **Clear Crops** drops the lot. -* **Frame outline**: a thin grey box marks the detected frame on every previewed tile. That box is the boundary Offset, Drift and the per-frame slider are measured from, so it stays visible under the shaded band and any crop drawn to the edge. +* **Frame outline**: a thin red box marks the detected frame on every previewed tile. That box is the boundary Offset, Drift and the per-frame slider are measured from, so it stays visible under the shaded band and any crop drawn to the edge. * **Offset**: slides every frame along the film to clear the inter-frame gap. Frames shift left as it grows, live. The shaded band on the right is film past the frame boundary the transport cannot deliver, so offset past the gap costs frame tail. A feeder cannot back up, so there it only goes one way. * **Drift**: adds progressively more (or less) offset per frame position, for a strip whose gaps creep along its length. Re-preview to refresh the pixels. * **Per-frame offset**: the slider under each tile corrects that frame alone, on top of Offset and Drift, for a boundary that sits off on its own. Its reading is in the tooltip; double-click resets it. diff --git a/negpy/desktop/view/widgets/scan_window_label.py b/negpy/desktop/view/widgets/scan_window_label.py index b77b22531..a6abe7f82 100644 --- a/negpy/desktop/view/widgets/scan_window_label.py +++ b/negpy/desktop/view/widgets/scan_window_label.py @@ -21,7 +21,6 @@ _HANDLE_TOL = 0.03 # corner grab radius, fraction of frame _HANDLE_PX = 5 # drawn handle half-size, widget px -_FRAME_EDGE_ALPHA = 150 # the frame outline reads against the picture without competing with the crop class ScanWindowLabel(QLabel): @@ -211,9 +210,7 @@ def paintEvent(self, _ev) -> None: painter.drawLine(x, draw_rect.top(), x, draw_rect.bottom()) # Last, so neither the offset band nor a crop drawn to the edge dims it: this is the # boundary the offset is measured from, and it has to stay readable on any frame. - boundary = QColor(THEME.text_secondary) - boundary.setAlpha(_FRAME_EDGE_ALPHA) - painter.setPen(QPen(boundary, 1)) + painter.setPen(QPen(QColor(THEME.accent_primary), 1)) painter.setBrush(Qt.BrushStyle.NoBrush) painter.drawRect(draw_rect.adjusted(0, 0, -1, -1)) else: diff --git a/tests/test_scan_window_label.py b/tests/test_scan_window_label.py index 0e2762490..a857a6e22 100644 --- a/tests/test_scan_window_label.py +++ b/tests/test_scan_window_label.py @@ -127,6 +127,8 @@ def test_the_frame_boundary_is_outlined_once_there_is_a_frame() -> None: """The offset is measured from the frame edge, so that edge has to be visible.""" from PyQt6.QtGui import QColor + from negpy.desktop.view.styles.theme import THEME + label = ScanWindowLabel() label.setFixedSize(120, 80) black = QPixmap(120, 80) @@ -139,9 +141,8 @@ def test_the_frame_boundary_is_outlined_once_there_is_a_frame() -> None: on_edge = shot.pixelColor(rect.left(), rect.top() + rect.height() // 2) inside = shot.pixelColor(rect.left() + rect.width() // 2, rect.top() + rect.height() // 2) - # A neutral line over the black frame, with the picture itself left alone. - assert on_edge.red() == on_edge.green() == on_edge.blue() - assert on_edge.red() > inside.red() + 40 + # The house red, undiluted, with the picture itself left alone. + assert on_edge.name() == QColor(THEME.accent_primary).name() assert inside.red() == 0 From 1ad3b150d342a692dac03eda2790694be406147f Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Sat, 5 Sep 2026 22:32:02 +0200 Subject: [PATCH 11/16] feat(scan): re-cut a measured strip's tiles when its offset moves A moved offset slid the pixmap the tile already held under a fixed frame box, so what the operator set the offset against was a simulation of the move rather than the film the scan would take. A measured strip's whole pass is already in memory, so the tile can be re-cut from it instead: 250 ms after any offset slider stops, the frames whose offset no longer matches what is on show are read again at the rect the batch will scan. No scanning, and the debounce keeps a drag to one request. Closing the dialog cancels a pending re-cut, since the timer holds it. Claude-Session: https://claude.ai/code/session_012UXETEHMjMAqRhQ1sZdNA7 --- docs/USER_GUIDE.md | 4 +- .../view/widgets/strip_preview_dialog.py | 51 ++++++++++- tests/test_strip_preview_dialog.py | 91 +++++++++++++++++++ 3 files changed, 139 insertions(+), 7 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 618d699cb..0c4e2e3ac 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -1004,8 +1004,8 @@ Every preview dialog ends the same way: **Cancel**, then **Apply** (keep the fra * **Cropping**: drag on a previewed frame. A corner resizes, inside moves. Each frame keeps its own window, and **Clear Crops** drops the lot. * **Frame outline**: a thin red box marks the detected frame on every previewed tile. That box is the boundary Offset, Drift and the per-frame slider are measured from, so it stays visible under the shaded band and any crop drawn to the edge. -* **Offset**: slides every frame along the film to clear the inter-frame gap. Frames shift left as it grows, live. The shaded band on the right is film past the frame boundary the transport cannot deliver, so offset past the gap costs frame tail. A feeder cannot back up, so there it only goes one way. -* **Drift**: adds progressively more (or less) offset per frame position, for a strip whose gaps creep along its length. Re-preview to refresh the pixels. +* **Offset**: slides every frame along the film to clear the inter-frame gap. Frames shift left as it grows, live. On a measured strip the tiles re-read the film a moment after the slider stops, rather than sliding the pixels already on show; it costs no scanning, because the whole strip is already in hand. The shaded band on the right is film past the frame boundary the transport cannot deliver, so offset past the gap costs frame tail. A feeder cannot back up, so there it only goes one way. +* **Drift**: adds progressively more (or less) offset per frame position, for a strip whose gaps creep along its length. * **Per-frame offset**: the slider under each tile corrects that frame alone, on top of Offset and Drift, for a boundary that sits off on its own. Its reading is in the tooltip; double-click resets it. * **Size**: how big the tiles are drawn. The grid reflows to whatever fits the dialog, so larger tiles mean fewer per row. It costs no scanning — the tile is redrawn from the pixels already in hand — and the setting is remembered. Double-click resets it. * **Which frames**: each tile carries its own tick; **All** and **None** move the lot, and the count says how many will be scanned. On a measured strip the ticks and crops describe the piece of film in the transport, so ejecting clears them, per-frame offsets included; Offset and Drift survive, because they register the transport. diff --git a/negpy/desktop/view/widgets/strip_preview_dialog.py b/negpy/desktop/view/widgets/strip_preview_dialog.py index 06fe6d616..fd7c611f3 100644 --- a/negpy/desktop/view/widgets/strip_preview_dialog.py +++ b/negpy/desktop/view/widgets/strip_preview_dialog.py @@ -6,7 +6,7 @@ """ import qtawesome as qta -from PyQt6.QtCore import Qt, pyqtSlot +from PyQt6.QtCore import Qt, QTimer, pyqtSlot from PyQt6.QtGui import QPixmap, QTransform from PyQt6.QtWidgets import ( QCheckBox, @@ -53,6 +53,9 @@ # A transport that measures the strip reports its frame count only as previews arrive, so ask # for a roll's worth and keep the tiles it answers with. _DISCOVERY_SLOTS = 40 +# Stillness before a moved offset re-cuts its tiles out of the strip pass. Long enough that a +# drag makes one request, short enough that the pixels follow the hand. +_RECUT_DELAY_MS = 250 # A coolscan3 raster is portrait, with the feed axis vertical, so rotate each preview -90° # and the frame reads landscape. QTransform().rotate(-90) maps a scan point (fx, fy) to @@ -96,7 +99,7 @@ def _display_to_scan_rect(rect): "grows; the shaded band is film past the frame boundary the transport cannot deliver, so " "offset past the gap costs frame tail." ) -_DRIFT_TIP = "Adds progressively more (or less) offset per frame position, for a strip whose gaps creep. Re-preview to refresh the pixels." +_DRIFT_TIP = "Adds progressively more (or less) offset per frame position, for a strip whose gaps creep." _TILE_OFFSET_TIP = "Corrects this frame alone, on top of Offset and Drift. Double-click to reset." _SIZE_TIP = "Tile size. The grid reflows to whatever fits the dialog. Double-click to reset." @@ -167,6 +170,13 @@ def __init__( self._tile_aspect = (mm[1] / mm[0]) if (mm and len(mm) > 1 and mm[0]) else 1.5 self._previewing = False self._detected_on_open = False + # A measured strip is already in memory, so a moved offset re-reads the film instead of + # sliding the pixels already on show: the tile is the film the scan takes. + self._recutting = False + self._recut = QTimer(self) + self._recut.setSingleShot(True) + self._recut.setInterval(_RECUT_DELAY_MS) + self._recut.timeout.connect(self._recut_moved_tiles) self._failed_frames: list[int] = [] self._scan_now = False # set when the user chooses "Scan" over "Use" initial_windows = initial_windows or {} @@ -603,6 +613,8 @@ def _on_offset_changed(self, _value: int) -> None: self.offset_label.setText(f"{self.frame_offset():.1f} mm") self.drift_label.setText(f"{self.frame_offset_modifier():+.2f} mm/frame") self._refresh_offset_indicators() + if self._discovers and self._tiles: + self._recut.start() def _tile_coverage(self, tile: _Tile) -> tuple[float, float]: """Span a raster previewed at x occupies when the slider reads y: (x − y, 1 − y). @@ -685,7 +697,32 @@ def _on_preview_all(self) -> None: slots = _DISCOVERY_SLOTS if self._discovers else self._capacity self._start_preview(tuple(range(1, slots + 1))) - def _start_preview(self, slots: tuple[int, ...]) -> None: + def done(self, result: int) -> None: + """A pending re-cut outlives nothing: the timer holds this dialog.""" + self._recut.stop() + super().done(result) + + def _recut_moved_tiles(self) -> None: + """Re-read the frames whose offset no longer matches the pixels on show. + + Cutting a tile out of the strip pass costs no scanning, so a nudged frame shows the film + the scan will take rather than a slide of the film it took. + """ + if self._previewing: + self._recut.start() + return + pitch = self._frame_pitch() + if not pitch: + return + moved = tuple( + frame + for frame, tile in sorted(self._tiles.items()) + if abs(self._raw_offset_for_frame(frame) / pitch - (tile.previewed_offset or 0.0)) > 1e-4 + ) + if moved: + self._start_preview(moved, message=f"Re-cutting {count_of(len(moved), 'frame')}…") + + def _start_preview(self, slots: tuple[int, ...], *, message: str | None = None) -> None: if self._previewing: return self._failed_frames = [] @@ -706,8 +743,11 @@ def _start_preview(self, slots: tuple[int, ...]) -> None: self.status_strip.set_message(f"Scanner busy — {e}") return self._previewing = True + self._recutting = message is not None self._set_previewing(True) - if self._discovers and len(slots) > 1: + if message is not None: + self.status_strip.set_message(message) + elif self._discovers and len(slots) > 1: # The slot count asked for is a roll's worth, not what the strip holds. self.status_strip.set_message("Measuring the strip…") else: @@ -745,7 +785,8 @@ def _on_preview_ready(self, preview) -> None: def _on_preview_finished(self) -> None: self._previewing = False self._set_previewing(False) - if self._discovers and not self._failed_frames: + recut, self._recutting = self._recutting, False + if self._discovers and not self._failed_frames and not recut: found = len(self._tiles) self.status_strip.set_message( f"{count_of(found, 'frame')} detected — check the framing before scanning." diff --git a/tests/test_strip_preview_dialog.py b/tests/test_strip_preview_dialog.py index 776c9b6a9..7390af13a 100644 --- a/tests/test_strip_preview_dialog.py +++ b/tests/test_strip_preview_dialog.py @@ -1189,3 +1189,94 @@ def test_the_offset_sliders_reach_a_boundary_several_millimetres_out() -> None: tile = grown._tiles[1].offset_slider tile.setValue(85) assert grown.frame_offsets() == {1: 8.5} + + +# ── re-cutting a nudged frame out of the strip pass ──────────────────────── + + +def _measured(controller, slots=(1, 2, 3), **kwargs): + """A dialog over a measured strip with its tiles already cut at offset 0.""" + dialog = StripPreviewDialog(controller, _discovery_device(), **kwargs) + controller.deliver_all(slots) + controller.preview_reqs.clear() + return dialog + + +def _dispose(dialog) -> None: + """Close and destroy a dialog: a timer left armed on a collected one crashes teardown.""" + dialog.reject() + dialog.deleteLater() + QApplication.processEvents() + + +def test_a_moved_frame_offset_re_cuts_that_frame_alone() -> None: + controller = _FakeController() + dialog = _measured(controller) + + dialog._tiles[2].offset_slider.setValue(15) + dialog._recut_moved_tiles() + _dispose(dialog) + + assert [r.slots for r in controller.preview_reqs] == [(2,)] + assert controller.preview_reqs[0].offsets[2] == pytest.approx(1.5 / 36.0, abs=1e-4) + + +def test_the_global_offset_re_cuts_every_tile() -> None: + controller = _FakeController() + dialog = _measured(controller) + + dialog.offset_slider.setValue(20) + dialog._recut_moved_tiles() + _dispose(dialog) + + assert [r.slots for r in controller.preview_reqs] == [(1, 2, 3)] + + +def test_an_offset_the_tiles_were_already_cut_at_re_cuts_nothing() -> None: + controller = _FakeController() + dialog = _measured(controller) + + dialog._recut_moved_tiles() + _dispose(dialog) + + assert controller.preview_reqs == [] + + +def test_moving_an_offset_arms_the_re_cut() -> None: + controller = _FakeController() + dialog = _measured(controller) + + dialog._tiles[1].offset_slider.setValue(4) + armed = dialog._recut.isActive() + _dispose(dialog) + + assert armed + assert not dialog._recut.isActive() # closing takes the pending re-cut with it + + +def test_a_feeder_never_re_cuts_its_tiles() -> None: + """Only a measured strip has the pass in memory; a feeder would have to scan again.""" + controller = _FakeController() + dialog = StripPreviewDialog(controller, _device(3)) + controller.deliver_all((1, 2, 3)) + controller.preview_reqs.clear() + + dialog._tiles[1].offset_slider.setValue(6) + armed = dialog._recut.isActive() + _dispose(dialog) + + assert not armed + + +def test_a_re_cut_does_not_report_the_strip_as_freshly_detected() -> None: + controller = _FakeController() + dialog = _measured(controller) + + dialog._tiles[3].offset_slider.setValue(-9) + dialog._recut_moved_tiles() + assert dialog.status_strip.message().startswith("Re-cutting") + controller.deliver_all((3,)) + _dispose(dialog) + + assert dialog.status_strip.message() == "" + From 825c218f97f0315e0660ceacb591a5db22b3c42e Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Sat, 5 Sep 2026 22:32:13 +0200 Subject: [PATCH 12/16] fix(scan): keep a strip's saved crops and selection when nothing was measured selected_frames() and frame_windows() read the live tiles alone, so accepting the dialog before detection had built any returned an empty selection and no crops, wiping what a previous pass had saved. Stopping the preview and pressing Apply was enough to lose the lot. Both now merge against what was saved, the way frame_offsets() already did; a crop cleared on a tile the operator can see still drops. Claude-Session: https://claude.ai/code/session_012UXETEHMjMAqRhQ1sZdNA7 --- .../view/widgets/strip_preview_dialog.py | 18 ++++++++++- tests/test_strip_preview_dialog.py | 31 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/negpy/desktop/view/widgets/strip_preview_dialog.py b/negpy/desktop/view/widgets/strip_preview_dialog.py index fd7c611f3..69b38b073 100644 --- a/negpy/desktop/view/widgets/strip_preview_dialog.py +++ b/negpy/desktop/view/widgets/strip_preview_dialog.py @@ -483,10 +483,26 @@ def _tile_size(self) -> tuple[int, int]: # ── result getters ──────────────────────────────────────────────── def selected_frames(self) -> tuple[int, ...]: + """Ticked frames, or what was saved where the strip was never measured. + + A dialog whose detection was stopped has no tiles to read, and an empty answer there + would drop a selection the operator made on a previous pass. + """ + if not self._tiles: + return self._initial_selected return tuple(sorted(f for f, t in self._tiles.items() if t.checkbox.isChecked())) def frame_windows(self) -> dict: - return {f: self._to_scan(t.label.window()) for f, t in self._tiles.items() if t.label.window() is not None} + """Per-frame crops. A frame with no tile keeps whatever was saved for it, as + ``frame_offsets`` does, so a stopped detection cannot erase the lot.""" + merged = dict(self._initial_windows) + for frame, tile in self._tiles.items(): + window = tile.label.window() + if window is None: + merged.pop(frame, None) # cleared on a tile the operator could see is deliberate + else: + merged[frame] = self._to_scan(window) + return merged def _to_display(self, rect): return _scan_to_display_rect(rect) if self._rotation else rect diff --git a/tests/test_strip_preview_dialog.py b/tests/test_strip_preview_dialog.py index 7390af13a..f90fc0f22 100644 --- a/tests/test_strip_preview_dialog.py +++ b/tests/test_strip_preview_dialog.py @@ -1280,3 +1280,34 @@ def test_a_re_cut_does_not_report_the_strip_as_freshly_detected() -> None: assert dialog.status_strip.message() == "" + +def test_a_stopped_detection_does_not_erase_the_saved_framing() -> None: + """Accepting before any tile exists must keep what a previous pass set, not wipe it.""" + controller = _FakeController() + dialog = StripPreviewDialog( + controller, + _discovery_device(), + initial_windows={2: (0.1, 0.1, 0.9, 0.9)}, + initial_selected=(2, 3), + initial_frame_offsets={2: 0.4}, + ) + assert dialog._tiles == {} + + windows, selected, offsets = dialog.frame_windows(), dialog.selected_frames(), dialog.frame_offsets() + _dispose(dialog) + + assert windows == {2: (0.1, 0.1, 0.9, 0.9)} + assert selected == (2, 3) + assert offsets == {2: 0.4} + + +def test_clearing_a_crop_on_a_tile_drops_the_saved_one() -> None: + controller = _FakeController() + dialog = _measured(controller, initial_windows={2: (0.1, 0.1, 0.9, 0.9)}) + assert dialog.frame_windows() == {2: (0.1, 0.1, 0.9, 0.9)} + + dialog._tiles[2].label.clear_window() + windows = dialog.frame_windows() + _dispose(dialog) + + assert windows == {} From 9fa0444ab60b1bcc4e5acf69faec29c8b27f3ec1 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Sun, 6 Sep 2026 12:32:28 +0200 Subject: [PATCH 13/16] fix(scan): map a measured strip's crop onto the sensor and feed axes A crop drawn on a measured strip's tile went to the backend as drawn, but the backend's window has y along the feed and x along the sensor, whose high addresses are the top of the tile. A crop over a tile's top-left came back as the frame's bottom-left. The rotated feeder tile already applies that transform, so the measured strip takes the same one; the rotation only decides which raster had to be turned. Verified on an LS-50 over nkscan: the crop matches the full scan's bottom-left quadrant at 0.987 correlation and its own quadrant at -0.16. --- negpy/desktop/view/widgets/strip_preview_dialog.py | 7 +++++-- tests/test_strip_preview_dialog.py | 11 +++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/negpy/desktop/view/widgets/strip_preview_dialog.py b/negpy/desktop/view/widgets/strip_preview_dialog.py index 69b38b073..e41e59d7b 100644 --- a/negpy/desktop/view/widgets/strip_preview_dialog.py +++ b/negpy/desktop/view/widgets/strip_preview_dialog.py @@ -504,11 +504,14 @@ def frame_windows(self) -> dict: merged[frame] = self._to_scan(window) return merged + # Both rasters are shown with the feed axis along display x and the sensor's high addresses + # at the top, so one rect transform serves both; the rotation only says which raster had to + # be turned to get there. def _to_display(self, rect): - return _scan_to_display_rect(rect) if self._rotation else rect + return _scan_to_display_rect(rect) def _to_scan(self, rect): - return _display_to_scan_rect(rect) if self._rotation else rect + return _display_to_scan_rect(rect) def tile_height(self) -> int: return int(self.size_slider.value()) diff --git a/tests/test_strip_preview_dialog.py b/tests/test_strip_preview_dialog.py index f90fc0f22..c21729def 100644 --- a/tests/test_strip_preview_dialog.py +++ b/tests/test_strip_preview_dialog.py @@ -728,14 +728,17 @@ def test_a_feeder_raster_turns_landscape() -> None: assert (pixmap.width(), pixmap.height()) == (24, 8) -def test_a_measured_strip_crop_needs_no_axis_swap() -> None: +def test_a_measured_strip_crop_maps_display_x_to_the_feed_and_display_top_to_the_sensor_end() -> None: + # A measured strip's tile arrives landscape, so it is not rotated, but the backend's window + # still has y along the feed and x along the sensor, whose high addresses are the top of the + # tile: the same transform a rotated feeder tile needs. Measured on an LS-50 over nkscan. controller = _FakeController() dialog = StripPreviewDialog(controller, _discovery_device()) dialog._on_preview_all() controller.deliver_all((1,)) - dialog._tiles[1].label.set_window((0.1, 0.2, 0.5, 0.6)) + dialog._tiles[1].label.set_window((0.0, 0.0, 0.5, 0.3)) # top-left: first half of the feed - assert dialog.frame_windows()[1] == (0.1, 0.2, 0.5, 0.6) + assert dialog.frame_windows()[1] == pytest.approx((0.7, 0.0, 1.0, 0.5)) # ── picking the frames to scan ──────────────────────────────────────────── @@ -1304,7 +1307,7 @@ def test_a_stopped_detection_does_not_erase_the_saved_framing() -> None: def test_clearing_a_crop_on_a_tile_drops_the_saved_one() -> None: controller = _FakeController() dialog = _measured(controller, initial_windows={2: (0.1, 0.1, 0.9, 0.9)}) - assert dialog.frame_windows() == {2: (0.1, 0.1, 0.9, 0.9)} + assert dialog.frame_windows()[2] == pytest.approx((0.1, 0.1, 0.9, 0.9)) dialog._tiles[2].label.clear_window() windows = dialog.frame_windows() From e2c7c98f8d4bd524e3643763053b3afc8cdf8022 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Sun, 6 Sep 2026 18:05:45 +0200 Subject: [PATCH 14/16] feat(scan): hand nkscan the detected frame table with every scan A perforation-framed Coolscan positions the film by its frame table and honours that table only as a whole, and the fine scan runs in a session that never measured the strip. nkscan's scan_frame now takes the detected frames back for that, so a shifted or cropped rect lands where the preview shows it instead of being read as an offset into the frame it fell under. Bindings without the argument are called as before. Measured on an LS-50 through the roll session and the batch path: frame 3 at +8 and -8 mm came back whole and aligned with its tile, where before both blacked out past the gate. --- negpy/infrastructure/scanners/nkscan_backend.py | 8 ++++++++ negpy/infrastructure/scanners/nkscan_roll.py | 1 + tests/scanners/fake_nkscan.py | 2 ++ tests/scanners/test_nkscan_backend.py | 8 ++++++++ 4 files changed, 19 insertions(+) diff --git a/negpy/infrastructure/scanners/nkscan_backend.py b/negpy/infrastructure/scanners/nkscan_backend.py index 3e8f3d128..ceb24d412 100644 --- a/negpy/infrastructure/scanners/nkscan_backend.py +++ b/negpy/infrastructure/scanners/nkscan_backend.py @@ -9,6 +9,7 @@ from __future__ import annotations +import inspect import threading from collections.abc import Callable from contextlib import contextmanager, suppress @@ -377,6 +378,7 @@ def _scan_on_session( lock_white_balance=self.locks_white_balance(params.film_type), exposures=exposures, progress=report, + frames=self._frames.get(device_id), ) if cancel.is_set(): raise RuntimeError("Scan cancelled") @@ -390,14 +392,20 @@ def scan_frame( rect: tuple[int, int, int, int], *, superfine: bool = False, + frames: list[tuple[int, int, int, int]] | None = None, **options: Any, ) -> Any: """One pass over `rect`. Every scan goes through here, previews included. A unit whose CCD cannot read its lines at once — the LS-50 cannot — has only the superfine ordering, and asking for the fast one is refused before the stage moves. + `frames` is the detected table: a unit that positions the film by it honours it only + as a whole, so a session that did not measure the strip is handed it back before a + shifted or cropped rect can land where it asks. Older bindings take no such argument. """ want = bool(superfine) or not bool(session.capabilities.multi_line) + if frames and "frames" in inspect.signature(session.scan_frame).parameters: + options["frames"] = [tuple(int(v) for v in rect) for rect in frames] return session.scan_frame(rect, superfine=want, **options) def locks_white_balance(self, film_type: str) -> bool: diff --git a/negpy/infrastructure/scanners/nkscan_roll.py b/negpy/infrastructure/scanners/nkscan_roll.py index 601afa39f..58d6dbe34 100644 --- a/negpy/infrastructure/scanners/nkscan_roll.py +++ b/negpy/infrastructure/scanners/nkscan_roll.py @@ -144,6 +144,7 @@ def _scan_preview(self, rect: tuple[int, int, int, int], cancel: threading.Event lock_white_balance=self._backend.locks_white_balance(self._film_type), exposures=self._exposures, progress=_progress_bridge(None, cancel), + frames=self._backend.frames(self._device.id), ) if self._exposures is None: self._exposures = dict(result.exposures) diff --git a/tests/scanners/fake_nkscan.py b/tests/scanners/fake_nkscan.py index 7f8b1ccae..3a00510a5 100644 --- a/tests/scanners/fake_nkscan.py +++ b/tests/scanners/fake_nkscan.py @@ -239,6 +239,7 @@ def scan_frame( lock_white_balance: bool = True, exposures: dict[str, int] | None = None, progress: Callable[..., Any] | None = None, + frames: list[tuple[int, int, int, int]] | None = None, ) -> FakeScanResult: module = self._module # A unit whose CCD reads one line at a time refuses the fast ordering, in the recipe @@ -259,6 +260,7 @@ def scan_frame( "clean": clean, "lock_white_balance": lock_white_balance, "exposures": exposures, + "frames": frames, } ) for step in range(1, module.progress_steps + 1): diff --git a/tests/scanners/test_nkscan_backend.py b/tests/scanners/test_nkscan_backend.py index eba9f62ea..f2f49c2c2 100644 --- a/tests/scanners/test_nkscan_backend.py +++ b/tests/scanners/test_nkscan_backend.py @@ -176,6 +176,14 @@ def test_the_offset_and_the_window_both_reach_the_scan() -> None: # ── result ──────────────────────────────────────────────────────────────── +def test_the_detected_table_goes_back_to_the_unit_with_every_scan() -> None: + # A perforation-framed unit honours its frame table only as a whole, and the fine scan + # runs in a session that never measured the strip. + backend, module = make_backend() + _scan(backend, dataclasses.replace(_PARAMS, frame=2, frame_offset_mm=3.0)) + assert module.opened[-1].scans[0]["frames"] == list(FRAMES) + + def test_the_planes_come_back_as_one_rgb_array() -> None: backend, _ = make_backend() result = _scan(backend) From 4b0c08b19f2f898d744ad17366b690d04d4b98b6 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Fri, 11 Sep 2026 18:55:59 +0200 Subject: [PATCH 15/16] feat(scan): follow nkscan main's Python API, trim prose nkscan main (0.11.0) drops `positive` from discover_frames, which made our call raise TypeError, and registers a moved or cropped rect with the unit itself. Stop passing `positive` and the `frames=` table (never in an upstream release), and drop the now-unused film_type from detect_frames. Cut strip-preview tiles at Discovery.addresses_per_column instead of optical_dpi / thumbnail_dpi: an LS-50 reports 97 dpi (41 addresses a column) where the film moves about 41.9, so tiles drifted towards the next frame down the strip. Shorten the comments, docstrings and USER_GUIDE prose this branch adds. Claude-Session: https://claude.ai/code/session_01BijGE62D7xF5J3oPwr16FR --- docs/USER_GUIDE.md | 12 ++-- negpy/desktop/view/sidebar/scan.py | 2 +- .../desktop/view/widgets/scan_window_label.py | 4 +- .../view/widgets/strip_preview_dialog.py | 62 +++++-------------- negpy/desktop/workers/scan_worker.py | 2 +- negpy/infrastructure/loaders/rawpy_loader.py | 4 +- .../infrastructure/scanners/nkscan_backend.py | 30 ++++----- negpy/infrastructure/scanners/nkscan_roll.py | 21 +------ negpy/infrastructure/scanners/settings.py | 11 ++-- negpy/services/scanning/service.py | 4 +- negpy/services/scanning/writer.py | 7 +-- tests/scanners/fake_nkscan.py | 28 ++++----- tests/scanners/test_nkscan_backend.py | 24 +------ tests/scanners/test_nkscan_roll.py | 24 +------ tests/scanners/test_service.py | 8 +-- tests/test_scan_worker.py | 6 +- tests/test_strip_preview_dialog.py | 17 ++--- 17 files changed, 77 insertions(+), 189 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 0c4e2e3ac..54a69ef3a 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -962,7 +962,7 @@ Capture film directly into NegPy. Two collapsible sections. ### Film Scanner -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. **Format** writes `TIFF` or `TIFF (mono)`; the mono form writes one 16-bit grey plane instead of three, at a third of the size, for film that carries a single record such as a black-and-white negative. NegPy reads it back the same way. **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. +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. **Format** writes `TIFF` or `TIFF (mono)`, which is one 16-bit grey plane for film with a single record, such as a black-and-white negative. **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. @@ -970,7 +970,7 @@ With **IR** checked, color and infrared come back in one scan pass; pyopticfilm The default Full window includes a little holder chrome top and bottom; host-path scans clamp those near-white margins to the film highlight so auto exposure is not skewed. Raise **Analysis Buffer** or crop if a frame still looks off. Autofocus and hardware Auto-exposure controls stay hidden, because the SE does not report those capabilities. On Windows, bind the device to **WinUSB** with Zadig before use, since the stock vendor or SilverFast driver conflicts. The driver is the optional **pyopticfilm** package: install it with `uv sync --group plustek` or `pip install negpy[plustek]`; Windows release builds bundle it. See [PLUSTEK_WINDOWS.md](PLUSTEK_WINDOWS.md). -**Nikon Coolscan (nkscan)** notes: the driver talks to the scanner directly, so it needs no SANE backend. It measures the loaded film instead of counting frames: **Preview strip…** reads the whole strip in one pass, finds every frame on it, and cuts every tile out of that same pass. It starts that search as it opens, so the tiles and their per-frame controls are there without pressing anything; the rects and the strip pass are kept until the film is ejected, so every later open is free. **Detect frames** re-runs the search when the film has moved. The tiles appear as the frames turn up, and there is no preview resolution to choose. Check the framing before scanning; a measured boundary can be nudged with **Offset** (±10 mm, either way, since the frame is re-addressed rather than fed past) and **Drift**, and because the tile comes out of the strip pass, a nudge re-frames without going back to the scanner. **Scan** with nothing picked scans every frame on the strip, measuring it first if no preview has. To scan a subset, type it in **Frames**, or untick frames in **Preview strip…**. Each tile carries its own tick, **All** and **None** move the lot, and the count next to them says how many will be scanned. Either way the selection shows in **Frames**, and ejecting the film clears it, since the frames and their crops describe the piece of film that just came out. **Offset** and **Drift** survive an eject, because they register the transport rather than one strip. Four controls appear only on this backend: +**Nikon Coolscan (nkscan)** notes: the driver talks to the scanner directly, so it needs no SANE backend. It measures the loaded film instead of counting frames: **Preview strip…** reads the whole strip in one pass, finds every frame on it, and cuts every tile out of that same pass. The search starts when the dialog opens, and its result is kept until the film is ejected. **Detect frames** runs it again when the film has moved. The tiles appear as the frames turn up, and there is no preview resolution to choose. Check the framing before scanning; a measured boundary can be nudged with **Offset** (±10 mm, either way, since the frame is re-addressed rather than fed past) and **Drift**, and because the tile comes out of the strip pass, a nudge re-frames without going back to the scanner. **Scan** with nothing picked scans every frame on the strip, measuring it first if no preview has. To scan a subset, type it in **Frames**, or untick frames in **Preview strip…**. Each tile carries its own tick, **All** and **None** move the lot, and the count next to them says how many will be scanned. Either way the selection shows in **Frames**, and ejecting the film clears it, since the frames and their crops describe the piece of film that just came out. **Offset** and **Drift** survive an eject, because they register the transport rather than one strip. Four controls appear only on this backend: * **ICE**: remove dust and scratches with the infrared channel while scanning. Permanent, because it is baked into the file, unlike the Retouch panel's IR Restore, which stays editable. Color film only: silver grain blocks infrared, so the mask on a black-and-white negative is the picture again. **ICE** and **IR** exclude each other, because they read the same pass: ticking one unticks the other. Tick **IR** to keep the plane and clean the file later in Retouch, **ICE** to have the scanner do it now. * **Samples**: reads per line the scanner averages (1–16). Higher settings cut shadow noise and cost proportionally more time. @@ -1003,11 +1003,11 @@ Camera scanning needs the optional `python-gphoto2` dependency (`pip install gph Every preview dialog ends the same way: **Cancel**, then **Apply** (keep the framing and go back to the panel) and **Scan** (start the scan from here). The Apply button names what it keeps: **Apply Framing** on a strip, **Apply Window** on a single holder, **Apply Crop** after a Prescan. * **Cropping**: drag on a previewed frame. A corner resizes, inside moves. Each frame keeps its own window, and **Clear Crops** drops the lot. -* **Frame outline**: a thin red box marks the detected frame on every previewed tile. That box is the boundary Offset, Drift and the per-frame slider are measured from, so it stays visible under the shaded band and any crop drawn to the edge. -* **Offset**: slides every frame along the film to clear the inter-frame gap. Frames shift left as it grows, live. On a measured strip the tiles re-read the film a moment after the slider stops, rather than sliding the pixels already on show; it costs no scanning, because the whole strip is already in hand. The shaded band on the right is film past the frame boundary the transport cannot deliver, so offset past the gap costs frame tail. A feeder cannot back up, so there it only goes one way. +* **Frame outline**: a red box marks the detected frame on each tile. Offset, Drift and the per-frame slider are measured from it. +* **Offset**: slides every frame along the film to clear the inter-frame gap. Frames shift left as it grows, live. On a measured strip the tiles are cut again from the strip pass when the slider stops, with no new scan. The shaded band on the right is film past the frame boundary the transport cannot deliver, so offset past the gap costs frame tail. A feeder cannot back up, so there it only goes one way. * **Drift**: adds progressively more (or less) offset per frame position, for a strip whose gaps creep along its length. -* **Per-frame offset**: the slider under each tile corrects that frame alone, on top of Offset and Drift, for a boundary that sits off on its own. Its reading is in the tooltip; double-click resets it. -* **Size**: how big the tiles are drawn. The grid reflows to whatever fits the dialog, so larger tiles mean fewer per row. It costs no scanning — the tile is redrawn from the pixels already in hand — and the setting is remembered. Double-click resets it. +* **Per-frame offset**: the slider under each tile corrects that frame alone, on top of Offset and Drift. The tooltip shows its value; double-click resets it. +* **Size**: the tile size. The grid reflows to fit the dialog, with no new scan, and the size is remembered. Double-click resets it. * **Which frames**: each tile carries its own tick; **All** and **None** move the lot, and the count says how many will be scanned. On a measured strip the ticks and crops describe the piece of film in the transport, so ejecting clears them, per-frame offsets included; Offset and Drift survive, because they register the transport. --- diff --git a/negpy/desktop/view/sidebar/scan.py b/negpy/desktop/view/sidebar/scan.py index df92b0c5d..7632444f5 100644 --- a/negpy/desktop/view/sidebar/scan.py +++ b/negpy/desktop/view/sidebar/scan.py @@ -313,7 +313,7 @@ def _init_ui(self) -> None: self.fmt_combo = QComboBox() self.fmt_combo.addItems(list(OUTPUT_FORMATS)) - self.fmt_combo.setToolTip("Output file format. Mono writes one grey plane instead of three, for film with a single record.") + self.fmt_combo.setToolTip("Output file format. Mono writes one grey plane, for film with a single record.") self.form.addRow("Format", self.fmt_combo) folder_row = QHBoxLayout() diff --git a/negpy/desktop/view/widgets/scan_window_label.py b/negpy/desktop/view/widgets/scan_window_label.py index a6abe7f82..e5cd97820 100644 --- a/negpy/desktop/view/widgets/scan_window_label.py +++ b/negpy/desktop/view/widgets/scan_window_label.py @@ -8,7 +8,6 @@ from PyQt6.QtCore import QPoint, QRect, Qt, pyqtSignal from PyQt6.QtGui import QColor, QMouseEvent, QPainter, QPen, QPixmap from PyQt6.QtWidgets import QLabel, QSizePolicy -from negpy.desktop.view.styles.theme import THEME from negpy.desktop.view.styles.theme import THEME from negpy.desktop.view.widgets.scan_window_geometry import ( @@ -208,8 +207,7 @@ def paintEvent(self, _ev) -> None: painter.drawRect(QRect(x, draw_rect.top(), max(0, draw_rect.right() - x), draw_rect.height())) painter.setPen(pen) painter.drawLine(x, draw_rect.top(), x, draw_rect.bottom()) - # Last, so neither the offset band nor a crop drawn to the edge dims it: this is the - # boundary the offset is measured from, and it has to stay readable on any frame. + # Drawn last, so the offset band and a crop cannot hide the frame boundary. painter.setPen(QPen(QColor(THEME.accent_primary), 1)) painter.setBrush(Qt.BrushStyle.NoBrush) painter.drawRect(draw_rect.adjusted(0, 0, -1, -1)) diff --git a/negpy/desktop/view/widgets/strip_preview_dialog.py b/negpy/desktop/view/widgets/strip_preview_dialog.py index e41e59d7b..2e02f6255 100644 --- a/negpy/desktop/view/widgets/strip_preview_dialog.py +++ b/negpy/desktop/view/widgets/strip_preview_dialog.py @@ -42,8 +42,7 @@ # (pitch - frame) discards that much picture off the frame tail. _FRAME_LEN_MM = 36.0 _PREVIEW_FALLBACK_DPI = 500 # only when the device reports no DPI list at all -# ±10 mm, in the slider's tenths of a millimetre. A feeder's own range is already 0..10 mm, -# and a measured boundary can sit several millimetres off the picture, so the two match. +# ±10 mm in tenths: a measured boundary can sit several millimetres off the picture. _MAX_MEASURED_OFFSET_TENTHS = 100 _TILE_H = 140 # default tile height; width follows the device aspect _TILE_H_MIN, _TILE_H_MAX = 90, 340 # what the size slider spans @@ -53,8 +52,7 @@ # A transport that measures the strip reports its frame count only as previews arrive, so ask # for a roll's worth and keep the tiles it answers with. _DISCOVERY_SLOTS = 40 -# Stillness before a moved offset re-cuts its tiles out of the strip pass. Long enough that a -# drag makes one request, short enough that the pixels follow the hand. +# Pause after an offset moves before its tiles are re-cut, so one drag makes one request. _RECUT_DELAY_MS = 250 # A coolscan3 raster is portrait, with the feed axis vertical, so rotate each preview -90° @@ -170,8 +168,7 @@ def __init__( self._tile_aspect = (mm[1] / mm[0]) if (mm and len(mm) > 1 and mm[0]) else 1.5 self._previewing = False self._detected_on_open = False - # A measured strip is already in memory, so a moved offset re-reads the film instead of - # sliding the pixels already on show: the tile is the film the scan takes. + # On a measured strip a moved offset re-cuts the tile from the strip pass, not slides its pixels. self._recutting = False self._recut = QTimer(self) self._recut.setSingleShot(True) @@ -413,8 +410,7 @@ def _build_tile(self, frame: int, initial_window, checked: bool) -> _Tile: offset_slider = _ResetSlider() offset_slider.setRange(-_MAX_MEASURED_OFFSET_TENTHS, _MAX_MEASURED_OFFSET_TENTHS) offset_slider.setFixedSize(self._tile_size()[0], _TILE_SLIDER_H) - # Seeded before the connection, so building a tile never runs the refresh against a - # dialog that is still assembling itself. + # Set before connecting, so building a tile does not refresh a half-built dialog. offset_slider.setValue(int(round(self._initial_frame_offsets.get(frame, 0.0) * 10))) offset_slider.valueChanged.connect(lambda _v, f=frame: self._on_tile_offset_changed(f)) grid.addWidget(offset_slider, 1, 0) @@ -426,8 +422,7 @@ def _build_tile(self, frame: int, initial_window, checked: bool) -> _Tile: def _fitting_columns(self) -> int: """Tiles that fit across the strip area, at least one.""" tile_w = self._tile_size()[0] - # Until the dialog is shown its viewport carries a Qt default width unrelated to the - # size resize() asked for, so measure the dialog itself while that is the case. + # Before show, the viewport has a Qt default width, so measure the dialog instead. viewport = self._scroll.viewport() width = viewport.width() if (self.isVisible() and viewport is not None) else self.width() - _GRID_MARGIN available = width - 4 # the grid's own left/right margins @@ -445,12 +440,10 @@ def _relayout(self, *, force: bool = False) -> None: tile = self._tiles[frame] self._strip.removeWidget(tile.widget) self._strip.addWidget(tile.widget, (frame - 1) // cols, (frame - 1) % cols) - # Removed before it is re-added: adding a widget the grid already holds leaves the old - # cell behind as a second item. + # Remove first: re-adding a widget the grid holds leaves a second, stale item. self._strip.removeWidget(self._empty_hint) self._strip.addWidget(self._empty_hint, 0, 0, 1, cols) - # Pin the grid top-left so a partial last row does not spread across the viewport. - # The previous pin has to be released or a now-occupied cell keeps stretching. + # Pin the grid top-left. Release the previous pin, or an occupied cell keeps stretching. if self._stretch is not None: self._strip.setColumnStretch(self._stretch[0], 0) self._strip.setRowStretch(self._stretch[1], 0) @@ -464,13 +457,7 @@ def resizeEvent(self, event) -> None: self._relayout() def showEvent(self, event) -> None: - """Find the frames as the dialog opens, once. - - A measured strip has no tiles until it is measured, and every per-frame control lives - on a tile, so an operator reopening this dialog to adjust one had nothing to adjust. - The rects and the strip pass are cached until the film is ejected, so this is free for - every open after the first. - """ + """Find the frames once, as the dialog opens: every per-frame control lives on a tile.""" super().showEvent(event) self._relayout() if self._discovers and not self._detected_on_open: @@ -483,30 +470,24 @@ def _tile_size(self) -> tuple[int, int]: # ── result getters ──────────────────────────────────────────────── def selected_frames(self) -> tuple[int, ...]: - """Ticked frames, or what was saved where the strip was never measured. - - A dialog whose detection was stopped has no tiles to read, and an empty answer there - would drop a selection the operator made on a previous pass. - """ + """Ticked frames, or the saved selection while there are no tiles.""" if not self._tiles: return self._initial_selected return tuple(sorted(f for f, t in self._tiles.items() if t.checkbox.isChecked())) def frame_windows(self) -> dict: - """Per-frame crops. A frame with no tile keeps whatever was saved for it, as - ``frame_offsets`` does, so a stopped detection cannot erase the lot.""" + """Per-frame crops. A frame with no tile keeps its saved crop.""" merged = dict(self._initial_windows) for frame, tile in self._tiles.items(): window = tile.label.window() if window is None: - merged.pop(frame, None) # cleared on a tile the operator could see is deliberate + merged.pop(frame, None) else: merged[frame] = self._to_scan(window) return merged - # Both rasters are shown with the feed axis along display x and the sensor's high addresses - # at the top, so one rect transform serves both; the rotation only says which raster had to - # be turned to get there. + # Both rasters show the feed along display x and high sensor addresses at the top, so one + # transform serves both. def _to_display(self, rect): return _scan_to_display_rect(rect) @@ -517,19 +498,14 @@ def tile_height(self) -> int: return int(self.size_slider.value()) def frame_offsets(self) -> dict[int, float]: - """Per-frame corrections, non-zero entries only. - - A frame with no tile keeps whatever was saved for it. A measured strip opens with no - tiles at all, so reading the sliders alone would erase every correction the moment the - dialog was accepted without detecting the strip again. - """ + """Non-zero per-frame corrections. A frame with no tile keeps its saved correction.""" merged = dict(self._initial_frame_offsets) for frame, tile in self._tiles.items(): value = tile.offset_slider.value() / 10.0 if value: merged[frame] = value else: - merged.pop(frame, None) # reset on a tile the operator could see is deliberate + merged.pop(frame, None) return merged def frame_offset(self) -> float: @@ -717,16 +693,12 @@ def _on_preview_all(self) -> None: self._start_preview(tuple(range(1, slots + 1))) def done(self, result: int) -> None: - """A pending re-cut outlives nothing: the timer holds this dialog.""" + """Stop a pending re-cut: its timer holds this dialog.""" self._recut.stop() super().done(result) def _recut_moved_tiles(self) -> None: - """Re-read the frames whose offset no longer matches the pixels on show. - - Cutting a tile out of the strip pass costs no scanning, so a nudged frame shows the film - the scan will take rather than a slide of the film it took. - """ + """Re-cut, from the strip pass, the tiles whose offset no longer matches their pixels.""" if self._previewing: self._recut.start() return diff --git a/negpy/desktop/workers/scan_worker.py b/negpy/desktop/workers/scan_worker.py index d80130187..f614f0d7c 100644 --- a/negpy/desktop/workers/scan_worker.py +++ b/negpy/desktop/workers/scan_worker.py @@ -284,7 +284,7 @@ def _progress(fraction: float, phase: str = "Scanning", _base: float = base, _at def _whole_strip(self, service: ScannerService, req: BatchRequest) -> list[int]: """Every frame on the loaded film, for a request that named none.""" - count = service.detect_frames(req.device_id, film_format=req.params.film_format, film_type=req.params.film_type) + count = service.detect_frames(req.device_id, film_format=req.params.film_format) if count <= 0: raise RuntimeError("No frames were detected on the loaded film") return list(range(1, count + 1)) diff --git a/negpy/infrastructure/loaders/rawpy_loader.py b/negpy/infrastructure/loaders/rawpy_loader.py index 97311bf85..a1b10563e 100644 --- a/negpy/infrastructure/loaders/rawpy_loader.py +++ b/negpy/infrastructure/loaders/rawpy_loader.py @@ -137,8 +137,8 @@ def tag(name: str) -> Optional[Any]: def _peek_linearraw_4ch(file_path: str) -> Optional[Tuple[np.ndarray, np.ndarray]]: """Inspect a DNG. If it carries 4 linear samples (RGB + IR), return (rgb, ir) as float32 [0,1]. - A single-IFD DNG carries the data in IFD0; VueScan and Adobe-style DNGs put the full-res - data in a SubIFD behind a reduced-resolution thumbnail IFD0 — both are checked. Returns None for camera DNGs (Bayer, 3-channel, etc.) so rawpy can handle them. + Checks IFD0 and, for VueScan and Adobe-style DNGs, a SubIFD behind a thumbnail IFD0. + Returns None for camera DNGs (Bayer, 3-channel, etc.) so rawpy can handle them. """ if not _is_dng(file_path): return None diff --git a/negpy/infrastructure/scanners/nkscan_backend.py b/negpy/infrastructure/scanners/nkscan_backend.py index ceb24d412..1553ef5d8 100644 --- a/negpy/infrastructure/scanners/nkscan_backend.py +++ b/negpy/infrastructure/scanners/nkscan_backend.py @@ -9,7 +9,6 @@ from __future__ import annotations -import inspect import threading from collections.abc import Callable from contextlib import contextmanager, suppress @@ -30,7 +29,6 @@ ScanParams, dpi_stops_in_range, film_passes_infrared, - film_reads_positive, ) from negpy.infrastructure.scanners.result import ScanResult from negpy.kernel.system.logging import get_logger @@ -242,6 +240,7 @@ def __init__(self) -> None: # re-previewing a strip after a nudge must not cost another read of the film. self._frames: dict[str, list[tuple[int, int, int, int]]] = {} self._strips: dict[str, np.ndarray] = {} + self._columns: dict[str, float] = {} self._lock = threading.Lock() # ── enumeration ─────────────────────────────────────────────────── @@ -378,7 +377,6 @@ def _scan_on_session( lock_white_balance=self.locks_white_balance(params.film_type), exposures=exposures, progress=report, - frames=self._frames.get(device_id), ) if cancel.is_set(): raise RuntimeError("Scan cancelled") @@ -392,20 +390,14 @@ def scan_frame( rect: tuple[int, int, int, int], *, superfine: bool = False, - frames: list[tuple[int, int, int, int]] | None = None, **options: Any, ) -> Any: """One pass over `rect`. Every scan goes through here, previews included. A unit whose CCD cannot read its lines at once — the LS-50 cannot — has only the superfine ordering, and asking for the fast one is refused before the stage moves. - `frames` is the detected table: a unit that positions the film by it honours it only - as a whole, so a session that did not measure the strip is handed it back before a - shifted or cropped rect can land where it asks. Older bindings take no such argument. """ want = bool(superfine) or not bool(session.capabilities.multi_line) - if frames and "frames" in inspect.signature(session.scan_frame).parameters: - options["frames"] = [tuple(int(v) for v in rect) for rect in frames] return session.scan_frame(rect, superfine=want, **options) def locks_white_balance(self, film_type: str) -> bool: @@ -435,24 +427,20 @@ def discover_frames( device_id: str, *, film_format: str | None, - film_type: str = "negative", progress: Callable[..., bool] | None = None, ) -> Any: """Measure the loaded film, cache the rects, and return nkscan's Discovery.""" with self._mapped_errors(): - discovery = session.discover_frames( - format=film_format, - positive=film_reads_positive(film_type), - progress=progress, - ) + discovery = session.discover_frames(format=film_format, progress=progress) self._frames[device_id] = [tuple(int(v) for v in rect) for rect in discovery.frames] thumbnail = getattr(discovery, "thumbnail", None) if thumbnail: self._strips[device_id] = _stack_rgb(thumbnail) + self._columns[device_id] = float(discovery.addresses_per_column) logger.info("Detected %d frames on %s", len(self._frames[device_id]), device_id) return discovery - def detect_frames(self, device_id: str, *, film_format: str | None = None, film_type: str = "negative") -> int: + def detect_frames(self, device_id: str, *, film_format: str | None = None) -> int: """How many frames the loaded film carries, measuring it only if that is not known. A strip previewed a moment ago is already measured, so this usually costs nothing. @@ -464,12 +452,12 @@ def detect_frames(self, device_id: str, *, film_format: str | None = None, film_ held = self._sessions.get(device_id) if held is not None: with self._mapped_errors(): - self.discover_frames(held._session, device_id, film_format=film_format, film_type=film_type) + self.discover_frames(held._session, device_id, film_format=film_format) return len(self._frames.get(device_id, ())) session, _model = self._open(device_id) try: with self._mapped_errors(): - self.discover_frames(session, device_id, film_format=film_format, film_type=film_type) + self.discover_frames(session, device_id, film_format=film_format) finally: with suppress(Exception): session.close() @@ -482,6 +470,10 @@ def strip_pass(self, device_id: str) -> np.ndarray | None: """The whole-strip read the frames were measured on, where the mechanism took one.""" return self._strips.get(device_id) + def addresses_per_column(self, device_id: str) -> float | None: + """Feed addresses one column of the strip pass spans, as that pass measured it.""" + return self._columns.get(device_id) + def set_frame(self, device_id: str, slot: int, rect: tuple[int, int, int, int]) -> None: """Replace one detected rect, so a nudge in the preview reaches the fine scan.""" frames = self._frames.get(device_id) @@ -492,6 +484,7 @@ def forget_frames(self, device_id: str) -> None: """Drop the cached rects and the pass they came from: that film has moved.""" self._frames.pop(device_id, None) self._strips.pop(device_id, None) + self._columns.pop(device_id, None) def _resolve_frame( self, @@ -506,7 +499,6 @@ def _resolve_frame( session, device_id, film_format=params.film_format, - film_type=params.film_type, progress=report, ) frames = self._frames.get(device_id) diff --git a/negpy/infrastructure/scanners/nkscan_roll.py b/negpy/infrastructure/scanners/nkscan_roll.py index 58d6dbe34..903846e22 100644 --- a/negpy/infrastructure/scanners/nkscan_roll.py +++ b/negpy/infrastructure/scanners/nkscan_roll.py @@ -26,18 +26,6 @@ _PREVIEW_DEPTH_DPI = 0 # the strip pass has its own resolution; nothing chooses it -def thumbnail_scale(optical_dpi: int, thumbnail_dpi: int) -> float: - """Stage addresses per thumbnail column. - - A column is one line pitch of film and the pass starts at the axis origin, so a column is - a feed address. The pitch is a whole number of addresses; the resolution the unit reports - for the pass is that pitch rounded down, so the trip back through it has to round. - """ - if optical_dpi <= 0 or thumbnail_dpi <= 0: - return 0.0 - return float(round(optical_dpi / thumbnail_dpi)) - - def slice_frame(strip: np.ndarray, rect: tuple[int, int, int, int], scale: float) -> np.ndarray | None: """The frame's own pixels out of the strip pass, or None when it falls outside. @@ -120,17 +108,12 @@ def _preview_one(self, slot: int, cancel: threading.Event) -> np.ndarray: rect = self._rect(slot) strip = self.thumbnail if strip is not None: - tile = slice_frame(strip, rect, self._scale()) + tile = slice_frame(strip, rect, self._backend.addresses_per_column(self._device.id) or 0.0) if tile is not None: return tile logger.info("Slot %s falls outside the strip pass; scanning it instead", slot) return self._scan_preview(rect, cancel) - def _scale(self) -> float: - caps = self._session.capabilities - dpi = tuple(caps.thumbnail_dpi) - return thumbnail_scale(int(caps.optical_dpi), int(dpi[0]) if dpi else 0) - def _scan_preview(self, rect: tuple[int, int, int, int], cancel: threading.Event) -> np.ndarray: """A pass of one frame, for a mechanism that measured the film without a strip pass.""" with self._backend._mapped_errors(): @@ -144,7 +127,6 @@ def _scan_preview(self, rect: tuple[int, int, int, int], cancel: threading.Event lock_white_balance=self._backend.locks_white_balance(self._film_type), exposures=self._exposures, progress=_progress_bridge(None, cancel), - frames=self._backend.frames(self._device.id), ) if self._exposures is None: self._exposures = dict(result.exposures) @@ -157,7 +139,6 @@ def _ensure_frames(self, cancel: threading.Event) -> list[tuple[int, int, int, i self._session, self._device.id, film_format=self._film_format, - film_type=self._film_type, progress=_progress_bridge(None, cancel), ) frames = self._backend.frames(self._device.id) diff --git a/negpy/infrastructure/scanners/settings.py b/negpy/infrastructure/scanners/settings.py index ea185e74b..a8d145def 100644 --- a/negpy/infrastructure/scanners/settings.py +++ b/negpy/infrastructure/scanners/settings.py @@ -5,8 +5,7 @@ Rect = tuple[float, float, float, float] -#: Written as one 16-bit grey plane instead of three. Film that carries a single record -#: (a B&W negative) reads the same off one plane, at a third of the size. +#: One 16-bit grey plane, for film with a single record such as a B&W negative. MONO_TIFF = "TIFF (mono)" #: What the Format combo offers, in order. OUTPUT_FORMATS = ("TIFF", MONO_TIFF) @@ -48,11 +47,9 @@ class ScannerSettings: # switch to a sorted tuple of pairs if that ever changes. frame_windows: dict[int, Rect] = field(default_factory=dict) selected_frames: tuple[int, ...] = () - # Per-frame feed-axis correction (mm), added on top of frame_offset_mm + drift. An absent - # key means no correction for that frame. + # Per-frame feed-axis correction (mm) on top of frame_offset_mm + drift. frame_offsets: dict[int, float] = field(default_factory=dict) - # Height in px of one tile in the strip preview. Tile width follows the device aspect, and - # the grid reflows to whatever fits the dialog. + # Strip preview tile height (px); the width follows the device aspect. strip_tile_height: int = 140 def __post_init__(self) -> None: @@ -82,7 +79,7 @@ def from_dict(cls, data: dict) -> "ScannerSettings": every unrelated preference with it. """ data = dict(data) - # DNG output is retired; a saved one lands on TIFF, the other 16-bit master. + # A saved DNG output format loads as TIFF. if str(data.get("output_format", "")).upper() == "DNG": data["output_format"] = "TIFF" first, last = data.pop("frame_from", None), data.pop("frame_to", None) diff --git a/negpy/services/scanning/service.py b/negpy/services/scanning/service.py index e7ecd544f..23f0f47a2 100644 --- a/negpy/services/scanning/service.py +++ b/negpy/services/scanning/service.py @@ -62,13 +62,13 @@ def eject(self, device_id: str) -> bool: """ return self._get_backend().eject(device_id) - def detect_frames(self, device_id: str, *, film_format: str | None = None, film_type: str = "negative") -> int: + def detect_frames(self, device_id: str, *, film_format: str | None = None) -> int: """How many frames the loaded film carries, 0 where the transport cannot measure it. A feeder counts slots instead, and the caller has that from the device capabilities. """ detect = getattr(self._get_backend(), "detect_frames", None) - return 0 if detect is None else int(detect(device_id, film_format=film_format, film_type=film_type)) + return 0 if detect is None else int(detect(device_id, film_format=film_format)) def open_roll( self, diff --git a/negpy/services/scanning/writer.py b/negpy/services/scanning/writer.py index 271cb6451..cc58019bb 100644 --- a/negpy/services/scanning/writer.py +++ b/negpy/services/scanning/writer.py @@ -28,11 +28,10 @@ def _to_uint16(arr: np.ndarray) -> np.ndarray: def _to_grey(rgb: np.ndarray) -> np.ndarray: - """One plane from three, by their mean. + """The rounded mean of the three planes. - Film with a single record is metered with its channels locked, so the three planes are one - density read three times and their mean is the least noisy of them. Summing in uint32 keeps - the accumulator off the source dtype; `(s + 1) // 3` rounds rather than floors. + Single-record film is metered with locked channels, so the planes are one density read three + times. The sum is uint32 so a uint16 source cannot overflow. """ if rgb.ndim == 2: return rgb diff --git a/tests/scanners/fake_nkscan.py b/tests/scanners/fake_nkscan.py index 3a00510a5..781ec6cef 100644 --- a/tests/scanners/fake_nkscan.py +++ b/tests/scanners/fake_nkscan.py @@ -93,6 +93,7 @@ class FakeDevice: class FakeDiscovery: frames: list[tuple[int, int, int, int]] thumbnail: dict[str, np.ndarray] | None = None + addresses_per_column: float | None = None @dataclass(frozen=True) @@ -120,6 +121,8 @@ class FakeNkscanModule: cols: int = 6 thumbnail: bool = True strip_slack: int = 4 # columns past the last frame; negative pushes frames off the pass + # Not the caps' 4000 / 250, as on a real pass. + addresses_per_column: float = 16.3 scan_error: Exception | None = None discover_error: Exception | None = None open_error: Exception | None = None @@ -150,19 +153,14 @@ def list_devices(self) -> list[FakeDevice]: return [FakeDevice(location=loc) for loc in self.locations] def strip_pass(self) -> dict[str, np.ndarray] | None: - """The whole-strip pass, laid out the way the unit delivers one. - - A column is one line pitch of film, counted from the axis start, the same way the unit - lays one out. Each frame's band carries its own slot number, so a test can tell which - part of the strip a tile was cut from. - """ + """The whole-strip pass. Each frame's band holds its slot number, so a tile shows where it was cut.""" if not self.thumbnail or not self.frames: return None - scale = round(self.caps.optical_dpi / self.caps.thumbnail_dpi[0]) - cols = int(max(f[2] for f in self.frames) / scale) + self.strip_slack + scale = self.addresses_per_column + cols = round(max(f[2] for f in self.frames) / scale) + self.strip_slack plane = np.zeros((self.rows, cols), np.uint16) for slot, (top, _l, bottom, _r) in enumerate(self.frames, 1): - plane[:, int(top / scale) : int(bottom / scale)] = slot + plane[:, round(top / scale) : round(bottom / scale)] = slot return {c: plane.copy() for c in ("red", "green", "blue")} @property @@ -185,7 +183,6 @@ def __init__(self, location: str) -> None: self.loads = 0 self.ejects = 0 self.discoveries: list[str | None] = [] - self.polarities: list[bool] = [] self.scans: list[dict[str, Any]] = [] module.opened.append(self) @@ -216,17 +213,20 @@ def eject(self) -> bool: def discover_frames( self, format: str | None = None, # noqa: A002 - the binding's own name - positive: bool = False, progress: Callable[..., Any] | None = None, ) -> FakeDiscovery: module = self._module self.discoveries.append(format) - self.polarities.append(positive) if progress is not None: progress("discover", 0, 1, 1) if module.discover_error is not None: raise module.discover_error - return FakeDiscovery(frames=list(module.frames), thumbnail=module.strip_pass()) + thumbnail = module.strip_pass() + return FakeDiscovery( + frames=list(module.frames), + thumbnail=thumbnail, + addresses_per_column=module.addresses_per_column if thumbnail else None, + ) def scan_frame( self, @@ -239,7 +239,6 @@ def scan_frame( lock_white_balance: bool = True, exposures: dict[str, int] | None = None, progress: Callable[..., Any] | None = None, - frames: list[tuple[int, int, int, int]] | None = None, ) -> FakeScanResult: module = self._module # A unit whose CCD reads one line at a time refuses the fast ordering, in the recipe @@ -260,7 +259,6 @@ def scan_frame( "clean": clean, "lock_white_balance": lock_white_balance, "exposures": exposures, - "frames": frames, } ) for step in range(1, module.progress_steps + 1): diff --git a/tests/scanners/test_nkscan_backend.py b/tests/scanners/test_nkscan_backend.py index f2f49c2c2..e97bf7467 100644 --- a/tests/scanners/test_nkscan_backend.py +++ b/tests/scanners/test_nkscan_backend.py @@ -176,14 +176,6 @@ def test_the_offset_and_the_window_both_reach_the_scan() -> None: # ── result ──────────────────────────────────────────────────────────────── -def test_the_detected_table_goes_back_to_the_unit_with_every_scan() -> None: - # A perforation-framed unit honours its frame table only as a whole, and the fine scan - # runs in a session that never measured the strip. - backend, module = make_backend() - _scan(backend, dataclasses.replace(_PARAMS, frame=2, frame_offset_mm=3.0)) - assert module.opened[-1].scans[0]["frames"] == list(FRAMES) - - def test_the_planes_come_back_as_one_rgb_array() -> None: backend, _ = make_backend() result = _scan(backend) @@ -466,19 +458,6 @@ def test_every_other_film_keeps_its_factory_balance() -> None: # ── what is on the film ─────────────────────────────────────────────────── -def test_reversal_film_is_measured_the_other_way_round() -> None: - """Unexposed slide film develops to maximum density, a negative to its base.""" - backend, module = make_backend() - session = backend.open_session(DEVICE_ID) - backend.discover_frames(module.opened[-1], DEVICE_ID, film_format=None, film_type="positive") - assert module.opened[-1].polarities == [True] - - backend.forget_frames(DEVICE_ID) - backend.discover_frames(module.opened[-1], DEVICE_ID, film_format=None, film_type="mono") - assert module.opened[-1].polarities == [True, False] - session.close() - - def test_ir_on_black_and_white_is_refused_before_the_unit_moves() -> None: backend, module = make_backend() with pytest.raises(RuntimeError, match="B&W negative blocks infrared"): @@ -526,8 +505,7 @@ def test_the_films_the_backend_names_are_films_the_extension_knows() -> None: def test_a_per_frame_offset_slides_only_the_feed_axis_of_the_frame_asked_for() -> None: - """The rect handed to nkscan must move by the film distance the operator dialled, on the - feed axis alone, and keep the frame's own extent.""" + """The rect moves by the dialled distance on the feed axis only.""" shift = round(0.7 * 4000 / 25.4) # 0.7 mm at the fake's optical dpi for frame, offset_mm, expected in ((3, 0.0, 0), (3, 0.7, shift), (3, -0.7, -shift), (1, 0.7, shift)): backend, module = make_backend() diff --git a/tests/scanners/test_nkscan_roll.py b/tests/scanners/test_nkscan_roll.py index 14209604e..529d1a7b9 100644 --- a/tests/scanners/test_nkscan_roll.py +++ b/tests/scanners/test_nkscan_roll.py @@ -8,7 +8,7 @@ import pytest from negpy.infrastructure.scanners.base import ScannerDevice -from negpy.infrastructure.scanners.nkscan_roll import NkscanRollSession, thumbnail_scale +from negpy.infrastructure.scanners.nkscan_roll import NkscanRollSession from tests.scanners import fake_nkscan from tests.scanners.fake_nkscan import DEVICE_ID, FRAMES, make_backend @@ -233,26 +233,8 @@ def test_ejecting_forgets_the_strip_pass_because_that_film_is_gone() -> None: assert backend.strip_pass(device.id) is None -def test_a_thumbnail_column_is_a_whole_line_pitch() -> None: - """The pitch is a whole number of addresses, and the reported resolution rounds it down. - - An LS-50 answers 97 dpi for a 4000 dpi unit, where the pitch it actually laid the pass out - with is 41: every frame top a real strip reported was an exact multiple of it. Carrying the - unrounded 41.24, or a scale taken across the film instead, walks the tile off the film it - names, further with every frame down the strip. - """ - assert thumbnail_scale(4000, 97) == 41.0 - assert thumbnail_scale(4000, 250) == 16.0 - assert thumbnail_scale(4000, 0) == 0.0 - assert thumbnail_scale(0, 97) == 0.0 - - # Tops an LS-50 measured on a real strip, which the pitch has to divide exactly. - for top in (246, 6109, 12013, 17917, 23821, 29725): - assert top % int(thumbnail_scale(4000, 97)) == 0 - - def test_a_tile_is_cut_at_the_address_the_scan_will_use() -> None: - """The tile and the fine scan must name the same film, or the operator judges the wrong one.""" + """The tile uses the column pitch the pass measured, not optical over thumbnail dpi.""" backend, module = make_backend() device = backend.list_devices()[0] session = backend.open_roll(device, dpi=500) @@ -261,7 +243,7 @@ def test_a_tile_is_cut_at_the_address_the_scan_will_use() -> None: finally: session.close() - scale = int(round(module.caps.optical_dpi / module.caps.thumbnail_dpi[0])) + scale = module.addresses_per_column strip = backend.strip_pass(device.id) assert strip is not None for preview, (slot, rect) in zip(previews, enumerate(module.frames, 1)): diff --git a/tests/scanners/test_service.py b/tests/scanners/test_service.py index 61298a490..9ef65283d 100644 --- a/tests/scanners/test_service.py +++ b/tests/scanners/test_service.py @@ -292,8 +292,8 @@ def test_open_roll_wraps_a_backend_that_scans_one_frame_at_a_time(fake_device: S class _MeasuringBackend(FakeBackend): - def detect_frames(self, device_id: str, *, film_format: str | None = None, film_type: str = "negative") -> int: - self.detect_args = (device_id, film_format, film_type) + def detect_frames(self, device_id: str, *, film_format: str | None = None) -> int: + self.detect_args = (device_id, film_format) return 4 @@ -302,8 +302,8 @@ def test_detect_frames_asks_the_backend_that_can_measure(fake_device: ScannerDev backend = _MeasuringBackend([fake_device]) service._backend = backend - assert service.detect_frames("fake:001", film_format="135", film_type="positive") == 4 - assert backend.detect_args == ("fake:001", "135", "positive") + assert service.detect_frames("fake:001", film_format="135") == 4 + assert backend.detect_args == ("fake:001", "135") def test_a_backend_that_counts_slots_measures_nothing(fake_device: ScannerDevice) -> None: diff --git a/tests/test_scan_worker.py b/tests/test_scan_worker.py index 133b1f0ab..cb91a3952 100644 --- a/tests/test_scan_worker.py +++ b/tests/test_scan_worker.py @@ -79,8 +79,8 @@ def eject(self, device_id: str) -> bool: self.eject_calls.append(device_id) return True - def detect_frames(self, device_id: str, *, film_format: str | None = None, film_type: str = "negative") -> int: - self.detect_calls.append((film_format, film_type)) + def detect_frames(self, device_id: str, *, film_format: str | None = None) -> int: + self.detect_calls.append(film_format) return self.detected def run_scan(self, device_id, params, progress, cancel): @@ -586,7 +586,7 @@ def test_a_batch_with_no_frames_scans_every_frame_on_the_film() -> None: worker.run_batch(_batch_request(frames=(), film_format="66")) assert service.frames == [1, 2, 3] - assert service.detect_calls == [("66", "negative")] + assert service.detect_calls == ["66"] assert len(done[0]) == 3 diff --git a/tests/test_strip_preview_dialog.py b/tests/test_strip_preview_dialog.py index c21729def..f54cbe876 100644 --- a/tests/test_strip_preview_dialog.py +++ b/tests/test_strip_preview_dialog.py @@ -729,9 +729,8 @@ def test_a_feeder_raster_turns_landscape() -> None: def test_a_measured_strip_crop_maps_display_x_to_the_feed_and_display_top_to_the_sensor_end() -> None: - # A measured strip's tile arrives landscape, so it is not rotated, but the backend's window - # still has y along the feed and x along the sensor, whose high addresses are the top of the - # tile: the same transform a rotated feeder tile needs. Measured on an LS-50 over nkscan. + # The tile is not rotated, but the backend window has y along the feed and x along the + # sensor, high addresses at the top: the same transform a feeder tile needs. controller = _FakeController() dialog = StripPreviewDialog(controller, _discovery_device()) dialog._on_preview_all() @@ -1058,11 +1057,7 @@ def test_reflowing_does_not_accumulate_layout_items() -> None: def test_the_preview_and_the_scan_land_on_the_same_millimetres() -> None: - """The tile the operator judges must be the film the batch then scans. - - Preview speaks fractions of a pitch and the scan speaks millimetres, so the two halves - are only equivalent if base, drift and the per-frame correction compose the same way. - """ + """Preview (fractions of a pitch) and scan (mm) compose base, drift and correction alike.""" from negpy.desktop.workers.scan_worker import BatchRequest, ScanWorker from negpy.infrastructure.scanners.params import ScanParams @@ -1177,11 +1172,7 @@ def test_a_feeder_is_left_alone_because_previewing_it_costs_a_pass_per_frame() - def test_the_offset_sliders_reach_a_boundary_several_millimetres_out() -> None: - """A measured boundary can sit well off the picture, and the slider has to reach it. - - Observed on a real strip: gaps of 876 and 1339 stage units (5.6 mm and 8.5 mm) at 4000 dpi, - which a +-2.5 mm control could not cut. - """ + """A measured boundary can sit several millimetres off the picture.""" dialog = StripPreviewDialog(_FakeController(), _discovery_device()) ctl = _FakeController() grown = StripPreviewDialog(ctl, _device(3)) From 679f1a558db6a2e95c2f0eed48030d72b4186fef Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Mon, 14 Sep 2026 07:46:09 +0200 Subject: [PATCH 16/16] build(scan): require nkscan 0.11 The backend already follows the 0.11 Python API: discover_frames and scan_frame take no polarity, scan_frame takes no frame table, and the thumbnail geometry comes from Discovery.addresses_per_column. Pin the release that ships it instead of a locally built wheel. --- docs/USER_GUIDE.md | 2 +- pyproject.toml | 4 ++-- uv.lock | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 54a69ef3a..4633d75f6 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -980,7 +980,7 @@ The default Full window includes a little holder chrome top and bottom; host-pat Every control here follows what the unit reports. An LS-50 shows neither Samples nor Superfine: it reads one CCD line at a time whatever you ask, and it ignores repeated reads of a line, so both stay hidden and a setting saved from another scanner is never sent to it. -The driver is the optional **nkscan** package (0.9 or newer), which ships as a wheel: If running from source install it with `uv sync --group nkscan` or `pip install negpy[nkscan]`. On Linux a Coolscan on USB needs a udev rule for Nikon (vendor `04b0`), and one on FireWire/SCSI needs the `sg` kernel module. +The driver is the optional **nkscan** package (0.11 or newer), which ships as a wheel: If running from source install it with `uv sync --group nkscan` or `pip install negpy[nkscan]`. On Linux a Coolscan on USB needs a udev rule for Nikon (vendor `04b0`), and one on FireWire/SCSI needs the `sg` kernel module. **SANE scan window**: on a roll/strip feeder (a live frame count reported), **Preview strip…** previews every frame, sets a per-frame window, and picks which frames to scan. On a SANE device with a single manual holder and no feeder, the button reads **Preview…** instead: it previews just the current holder position and lets you drag one crop window, reused for the next scan (the pyOpticfilm backend's equivalent is **Prescan**, above). Either way, the window narrows the scanner's own hardware scan area, so the real scan only reads that region, rather than reading the full frame (holder margins and film rebate included) and cropping in software afterward. diff --git a/pyproject.toml b/pyproject.toml index 094ca9588..501bc06d1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,7 +40,7 @@ classifiers = [ ] [project.optional-dependencies] -nkscan = ["nkscan>=0.10"] +nkscan = ["nkscan>=0.11"] plustek = ["pyopticfilm>=1.3.3"] sane = ["python-sane>=2.9"] camera = ["gphoto2>=2.5 ; sys_platform != 'win32'"] @@ -69,7 +69,7 @@ pieusb = [ ] nkscan = [ # Nikon Coolscan over SCSI/USB. A Rust extension, shipped as wheels. - "nkscan>=0.10", + "nkscan>=0.11", ] [project.urls] diff --git a/uv.lock b/uv.lock index 867f6d3be..dd3b273ed 100644 --- a/uv.lock +++ b/uv.lock @@ -382,7 +382,7 @@ requires-dist = [ { name = "imagecodecs", specifier = "==2026.6.6" }, { name = "imageio", specifier = "==2.37.3" }, { name = "jinja2", specifier = "==3.1.6" }, - { name = "nkscan", marker = "extra == 'nkscan'", specifier = ">=0.10" }, + { name = "nkscan", marker = "extra == 'nkscan'", specifier = ">=0.11" }, { name = "numba", specifier = "==0.65.1" }, { name = "numpy", specifier = "==2.4.4" }, { name = "opencv-python-headless", specifier = "==4.13.0.92" }, @@ -409,21 +409,21 @@ dev = [ { name = "ruff", specifier = "==0.14.10" }, { name = "ty", specifier = ">=0.0.26" }, ] -nkscan = [{ name = "nkscan", specifier = ">=0.10" }] +nkscan = [{ name = "nkscan", specifier = ">=0.11" }] pieusb = [{ name = "pieusb", specifier = ">=0.3.7" }] plustek = [{ name = "pyopticfilm", specifier = ">=1.3.3" }] sane = [{ name = "python-sane", specifier = ">=2.9" }] [[package]] name = "nkscan" -version = "0.10.0" +version = "0.11.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/f7/cc/4642272d170d374924e2cc9f94486da83fa4fceec2b5b5911983b805a469/nkscan-0.10.0.tar.gz", hash = "sha256:57079bcf7dd97ae2e0f4f4ea04ea74126103af2f3433b99a09414254bae5ee74", size = 2808736, upload-time = "2026-09-04T19:13:45.82Z" } +sdist = { url = "https://files.pythonhosted.org/packages/8a/f1/fc1201744ac24ab5f0fbd7f79ff4d1d8a112b6b3e65d0970b906e8e1e03b/nkscan-0.11.0.tar.gz", hash = "sha256:2bf020eb7d4e74723a4f0a89c82f887a29a3514832fad0abfa088d66ec09f274", size = 2809850, upload-time = "2026-09-13T19:57:32.132Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/12/76/b68a6941616ff9e41343cebc4374bc8721a511ff472ea8c98f711bc49000/nkscan-0.10.0-cp313-abi3-macosx_10_12_x86_64.whl", hash = "sha256:5c6accfb802f9f851aa9a65a15b46be802a49f8fc642d07b8d5a540b6f8e30c2", size = 750584, upload-time = "2026-09-04T19:13:39.924Z" }, - { url = "https://files.pythonhosted.org/packages/a7/21/3c19378359b85a768d57fb2020377a31c39b9ad71d143ed77eb70a3cbbce/nkscan-0.10.0-cp313-abi3-macosx_11_0_arm64.whl", hash = "sha256:1de8491669add3e35e5dc6ed8928193b49bbbb5024f121de55977321dcd93975", size = 733790, upload-time = "2026-09-04T19:13:41.346Z" }, - { url = "https://files.pythonhosted.org/packages/28/6b/7362d7eb671a132923f454e74815457853bf74484774dd7a3f40084b45cd/nkscan-0.10.0-cp313-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:a46bbce25224e57f491ba326a8346f496fb4028262f30adc9f8eb3621b31ae7d", size = 842336, upload-time = "2026-09-04T19:13:42.799Z" }, - { url = "https://files.pythonhosted.org/packages/98/eb/9e25d21a957bee0c1e9c2d0513b935354688fc497d3b75bed2830902eb74/nkscan-0.10.0-cp313-abi3-win_amd64.whl", hash = "sha256:67184353e2a25e2be5d8fd32b335a43006178041e61db20c4baea2528c3da19f", size = 602004, upload-time = "2026-09-04T19:13:44.414Z" }, + { url = "https://files.pythonhosted.org/packages/21/0a/b48d696b32f2b628e1ad224b62d512a457538375e16a726aebc3710d1f88/nkscan-0.11.0-cp313-abi3-macosx_10_12_x86_64.whl", hash = "sha256:0089cd04b9cf0c9f4112d2c7f29a2eacaa3ce1b81da44a1bd3e8d343e8fd4758", size = 1129273, upload-time = "2026-09-13T19:57:25.596Z" }, + { url = "https://files.pythonhosted.org/packages/df/d7/f273b36dfba1bea62f6f41f107d6e804e510b117ad3018f3dc6aa8b33dfb/nkscan-0.11.0-cp313-abi3-macosx_11_0_arm64.whl", hash = "sha256:0082693f94a4c76b222a8dcac485e00bc4d69fa4e744f3fcb5c4924228a43b80", size = 1106963, upload-time = "2026-09-13T19:57:27.245Z" }, + { url = "https://files.pythonhosted.org/packages/83/68/56b54bd270d41fcab843ed7fc9fc6955a8a0a37109853835065b9fe763b0/nkscan-0.11.0-cp313-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:0b3270da6585ac0251127fb29c423576822f100116b4ae8f95e744ec9c2f0531", size = 1256726, upload-time = "2026-09-13T19:57:28.698Z" }, + { url = "https://files.pythonhosted.org/packages/e2/50/80503f55c53e4fa917c7e5c45f6ba3b0097b35a8d9971b10ec4041012bc6/nkscan-0.11.0-cp313-abi3-win_amd64.whl", hash = "sha256:6c83f7558cccc5dfaf0cbc0b6774385a4cc2bd08898aeaed0eb5550b4bb3986c", size = 966245, upload-time = "2026-09-13T19:57:30.502Z" }, ] [[package]]