From 33fe0acc4a961280539c33939f3b0ab2f43fbeb5 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 29 Jul 2026 09:54:14 +0200 Subject: [PATCH 1/2] feat: colour ring-around, the RA4 filtration proof MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The classic Kodak/Fuji ring-around as a canvas proof: the frame printed as a 3x3 mosaic with the centre patch at the current filtration and the ring 5cc either way on the magenta and yellow axes, so the direction of a colour cast is visible rather than guessed. Click a patch to keep its filtration. Shift+F, the target button beside the Filtration sliders, or the toolbar's overflow menu. The step is grounded rather than picked: filtration_offsets documents 1.0 slider = 20cc, so RING_CC_STEP = 0.25 is the classic RA4 5cc increment, and it keeps the centre unclamped anywhere in [-0.75, +0.75]. One proof slot, not two. _clear_test_strip() has six call sites — request_render, the toggle, the pick, and the compare / flat-peek / load_file navigate-back paths — so a parallel ring lifecycle would mean six more places to keep in sync forever, and the navigate-back path is exactly the one that gets missed. Instead the strip's slot gains a `kind`, which means every existing drop path covers the ring for free and asking for one proof while the other is up swaps rather than dismisses. The worker is generalized rather than duplicated: TestStripTask carries per-patch ExposureConfig overrides plus a grid shape, and build_strip applies them. The mosaic slicer takes the grid too. The tone strip becomes one caller, and its tests passing untouched is the regression check. Relative, not absolute — the opposite choice from the strip's ladders, for a reason: a ring-around asks "is this print neutral, and if not, which way", which an absolute filtration grid cannot ask once a correction is already dialled in. Three consequences fall out. The centre patch is the current filtration by construction, so the accent needs no nearest-rung search. The memo must NOT pin filtration — picking correctly invalidates, because the ring is meant to be iterated toward neutral and each round is genuinely new pixels. And at a slider rail the ring prints duplicate patches, which is the head running out of travel, not a bug; a test documents it rather than engineering it away. Verified end to end on samples/DSC00448.ARW from a non-neutral M=0.2 Y=-0.1 start: the centre patch is bit-identical to a plain render at those settings (max abs diff 0.0), all nine patch means are distinct, the two magenta rungs move green in opposite directions and the two yellow rungs move blue, and picking leaves cyan, density, grade, Cast Removal and both Auto toggles untouched. Per-patch cost measures 0.06s against the 36-patch strip's 0.05s, confirming the ring gets the same metering reuse. --- docs/CHANGELOG.md | 1 + negpy/desktop/controller.py | 107 +++++++++--- negpy/desktop/session.py | 6 + negpy/desktop/view/canvas/overlay.py | 113 ++++++++---- negpy/desktop/view/canvas/toolbar.py | 17 ++ negpy/desktop/view/keyboard_shortcuts.py | 1 + negpy/desktop/view/shortcut_registry.py | 1 + negpy/desktop/view/sidebar/colour.py | 31 ++++ negpy/desktop/view/sidebar/tone.py | 7 +- negpy/desktop/workers/render.py | 28 +-- negpy/features/exposure/analysis.py | 79 +++++++-- tests/test_canvas_toolbar.py | 2 + tests/test_ring_around.py | 96 ++++++++++ tests/test_ring_around_controller.py | 214 +++++++++++++++++++++++ tests/test_test_strip.py | 54 +++--- tests/test_test_strip_controller.py | 12 +- tests/test_test_strip_overlay.py | 37 +++- 17 files changed, 688 insertions(+), 118 deletions(-) create mode 100644 tests/test_ring_around.py create mode 100644 tests/test_ring_around_controller.py diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index c19723a7..f0715021 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,7 @@ ## 0.45.0 +- New: **Colour ring-around** — the Filtration panel (or Shift+F) prints the frame as a 3×3 mosaic: the centre patch at the current filtration, the ring 5cc either way on the magenta and yellow axes, so the direction of a colour cast is visible instead of guessed. Each patch is a real render; click one to keep its filtration. Unlike the test strip the ladder is relative, so the centre is always the print you are judging and each round starts from wherever you left off. The two proofs share the canvas — asking for one while the other is up swaps it. - New: **Test strip** — the Tone panel prints the frame as a 6×6 grid, Print Density increasing left to right and ISO-R Grade softening top to bottom, so the diagonals read light-to-dark and soft-to-hard like a split-filter test strip. Each patch is a real render at its own settings; click one to keep it. Escape or a second press clears it, and any edit drops it. ## 0.44.0 diff --git a/negpy/desktop/controller.py b/negpy/desktop/controller.py index 651a31df..932119dc 100644 --- a/negpy/desktop/controller.py +++ b/negpy/desktop/controller.py @@ -53,7 +53,15 @@ ) from negpy.services.assets.half_frame import base_hash, slice_half from negpy.services.assets.sidecar import load_or_promote, write_sidecar -from negpy.features.exposure.analysis import STRIP_DENSITIES, STRIP_GRADES, strip_cells +from negpy.features.exposure.analysis import ( + RING_GRID, + STRIP_DENSITIES, + STRIP_GRADES, + STRIP_GRID, + ring_cells, + ring_overrides, + strip_overrides, +) from negpy.features.exposure.logic import ( calculate_wb_shifts, calculate_wb_shifts_from_log, @@ -925,16 +933,22 @@ def _render_memo_key(self, config: Optional[WorkspaceConfig] = None) -> str: ) return hashlib.md5(repr(parts).encode()).hexdigest() - def _strip_memo_key(self) -> str: - """The render key with density and grade pinned to fixed values. + def _strip_memo_key(self, kind: str = "tone") -> str: + """The render key for a proof mosaic, prefixed by kind so the two can never collide. + + The tone strip pins density and grade to fixed values: it supplies those two itself, + one per patch, so its mosaic is invariant to whatever they currently are — which makes + the print/pick/print-again loop a cache hit. Any other edit (crop, filtration, paper, + toning...) lands on a different key and re-prints. - The strip supplies those two itself, one per patch, so the mosaic is invariant - to whatever they currently are — which makes the print/pick/print-again loop a - cache hit, since picking a patch changes nothing else. Any other edit (crop, - filtration, paper, toning...) lands on a different key and re-prints. + The ring pins nothing. Its ladder is relative to the filtration in force, so the mosaic + is *not* invariant to it — picking a patch has to invalidate, and should: a ring-around + is meant to be iterated toward neutral, and each round is genuinely new pixels. """ - pinned = replace(self.state.config.exposure, density=1.0, grade=115.0) - return self._render_memo_key(replace(self.state.config, exposure=pinned)) + config = self.state.config + if kind == "tone": + config = replace(config, exposure=replace(config.exposure, density=1.0, grade=115.0)) + return f"{kind}:{self._render_memo_key(config)}" def load_file(self, file_path: str, preserve_zoom: bool = False, force_detect: bool = False) -> None: """ @@ -1201,39 +1215,67 @@ def toggle_zones_overlay(self, force: Optional[bool] = None) -> None: self.state.zones_overlay = (not self.state.zones_overlay) if force is None else bool(force) self.zones_overlay_changed.emit(self.state.zones_overlay) - def toggle_test_strip(self, force: Optional[bool] = None) -> None: - """Print (or clear) the density × grade test strip. Unlike the zone overlay this - needs pixels the canvas doesn't have, so entering dispatches one strip job.""" - target = (not (self.state.test_strip or self.state.test_strip_pending)) if force is None else bool(force) + def toggle_ring_around(self, force: Optional[bool] = None) -> None: + """Print (or clear) the colour ring-around — the M/Y filtration proof.""" + self.toggle_test_strip(force, kind="colour") + + def toggle_test_strip(self, force: Optional[bool] = None, kind: str = "tone") -> None: + """Print (or clear) a proof mosaic: the density × grade test strip, or the colour + ring-around. Unlike the zone overlay these need pixels the canvas doesn't have, so + entering dispatches one job. + + Both share the one proof slot, so asking for the other kind while a proof is up swaps + it rather than dismissing it. + """ + showing = (self.state.test_strip or self.state.test_strip_pending) and self.state.test_strip_kind == kind + target = (not showing) if force is None else bool(force) if not target: self._clear_test_strip() return if self.state.preview_raw is None: return - # Reprinting an unchanged strip is 36 renders for pixels we already have. - cached = self._strip_memo.get(self.state.current_file_hash or "", self._strip_memo_key()) + exposure = self.state.config.exposure + if kind == "colour": + origin = (exposure.wb_magenta, exposure.wb_yellow) + overrides = ring_overrides(*origin) + grid = RING_GRID + toast = "Printing the colour ring-around…" + else: + origin = (0.0, 0.0) + overrides = strip_overrides() + grid = STRIP_GRID + toast = "Printing test strip…" + + # Reprinting an unchanged proof is a pile of renders for pixels we already have. + cached = self._strip_memo.get(self.state.current_file_hash or "", self._strip_memo_key(kind)) if cached is not None: + self.state.test_strip_kind = kind + self.state.test_strip_origin = origin self.on_strip_finished(cached["mosaic"], cached["content_rect"], from_cache=True) return proofing = self.state.soft_proof_enabled icc_input = self.effective_input_icc() if (proofing or self.state.config.process.narrowband_scan) else None + self.state.test_strip_kind = kind + self.state.test_strip_origin = origin self.state.test_strip_pending = True self.test_strip_changed.emit(False) - # 36 renders take a few seconds; say so up front and tick the HUD progress bar + # These renders take a few seconds; say so up front and tick the HUD progress bar # per patch so it's clear the app is working rather than wedged. - self.status_message_requested.emit("Printing test strip…", 2500) - self.status_progress_requested.emit(0, len(strip_cells())) + self.status_message_requested.emit(toast, 2500) + self.status_progress_requested.emit(0, len(overrides)) self.strip_requested.emit( TestStripTask( buffer=self.state.preview_raw, config=self.state.config, source_hash=self.state.current_file_hash or "preview", - # Always preview res, never HQ: 36 full-res renders would take minutes, - # and the patches are shown at a sixth of the frame's width anyway. The + # Always preview res, never HQ: full-res renders per patch would take minutes, + # and each patch is shown at a fraction of the frame's width anyway. The # mosaic scales to the content rect, so a smaller one costs nothing here. preview_size=float(APP_CONFIG.preview_render_size), + overrides=tuple(overrides), + grid=grid, icc_input_path=icc_input, icc_output_path=self.effective_output_icc() if proofing else None, color_space=self.state.workspace_color_space, @@ -1272,7 +1314,7 @@ def on_strip_finished(self, mosaic: Any, content_rect: Any, from_cache: bool = F # cancels the strip outright. self._strip_memo.store( self.state.current_file_hash or "", - self._strip_memo_key(), + self._strip_memo_key(self.state.test_strip_kind), {"mosaic": mosaic, "content_rect": content_rect}, ) self.state.test_strip_pending = False @@ -1280,18 +1322,27 @@ def on_strip_finished(self, mosaic: Any, content_rect: Any, from_cache: bool = F self.state.test_strip_mosaic = mosaic self.state.test_strip_content_rect = content_rect self.test_strip_changed.emit(True) - self.status_message_requested.emit("Test strip ready — click a patch to keep it", 4000) + label = "Ring-around" if self.state.test_strip_kind == "colour" else "Test strip" + self.status_message_requested.emit(f"{label} ready — click a patch to keep it", 4000) def apply_test_strip_pick(self, row: int, col: int) -> None: - """Commit the clicked patch's density/grade, then drop the strip.""" + """Commit the clicked patch's settings, then drop the proof. + + Cast Removal and the Auto toggles are left exactly as they were: the patches were + rendered under them, so changing them would render something other than the patch + that was clicked. The ring reads its ladder from the origin it was printed around, + not from the live config — which this is about to change. + """ if not self.state.test_strip: return - density, grade = STRIP_DENSITIES[col], STRIP_GRADES[row] + exposure = self.state.config.exposure + if self.state.test_strip_kind == "colour": + cells = ring_cells(*self.state.test_strip_origin) + _, _, magenta, yellow = cells[row * RING_GRID[1] + col] + new_exposure = replace(exposure, wb_magenta=magenta, wb_yellow=yellow) + else: + new_exposure = replace(exposure, density=STRIP_DENSITIES[col], grade=STRIP_GRADES[row]) self._clear_test_strip() - # Auto Density / Auto Grade are left exactly as they were: the patches were - # rendered under them, so changing them would render something other than the - # patch that was clicked. - new_exposure = replace(self.state.config.exposure, density=density, grade=grade) self.session.update_config(replace(self.state.config, exposure=new_exposure), persist=True) self.request_render() diff --git a/negpy/desktop/session.py b/negpy/desktop/session.py index fec49667..97976f71 100644 --- a/negpy/desktop/session.py +++ b/negpy/desktop/session.py @@ -108,6 +108,12 @@ class AppState: test_strip_pending: bool = False test_strip_mosaic: Optional[Any] = None test_strip_content_rect: Optional[tuple] = None + # Which proof owns the canvas: "tone" (density × grade) or "colour" (M/Y ring-around). + # One slot rather than two, so every path that drops a proof drops both kinds. + test_strip_kind: str = "tone" + # (wb_magenta, wb_yellow) the ring was printed around — the pick reads its ladder from + # this, not from the live config, which the pick itself is about to change. + test_strip_origin: tuple = (0.0, 0.0) # Reverse scroll-wheel zoom direction on the image viewer (scroll up = zoom out). invert_zoom_scroll: bool = False diff --git a/negpy/desktop/view/canvas/overlay.py b/negpy/desktop/view/canvas/overlay.py index a0802008..c7f474e6 100644 --- a/negpy/desktop/view/canvas/overlay.py +++ b/negpy/desktop/view/canvas/overlay.py @@ -14,8 +14,11 @@ from negpy.desktop.view.canvas.crop_guides import CropGuide, guide_shapes from negpy.desktop.view.styles.theme import THEME from negpy.features.exposure.analysis import ( + RING_GRID, STRIP_DENSITIES, STRIP_GRADES, + STRIP_GRID, + ring_cc_offset, strip_cell_at, strip_nearest_cell, zone_grid, @@ -768,18 +771,63 @@ def _strip_qimage(self) -> Optional[QImage]: self._strip_cache = (key, img) return img + def _strip_grid(self) -> Tuple[int, int]: + """(rows, cols) of the proof currently on the canvas.""" + return RING_GRID if self.state.test_strip_kind == "colour" else STRIP_GRID + def _strip_patch_rects(self, rect: QRectF) -> List[Tuple[int, int, QRectF]]: """(row, col, screen rect) per patch. Edges are computed from the same fractions the mosaic was sliced on, so the drawn separators sit on the real seams.""" - rows, cols = len(STRIP_GRADES), len(STRIP_DENSITIES) + rows, cols = self._strip_grid() xs = [rect.x() + rect.width() * c / cols for c in range(cols + 1)] ys = [rect.y() + rect.height() * r / rows for r in range(rows + 1)] return [(r, c, QRectF(xs[c], ys[r], xs[c + 1] - xs[c], ys[r + 1] - ys[r])) for r in range(rows) for c in range(cols)] + def _draw_strip_labels( + self, + painter: QPainter, + patches: List[Tuple[int, int, QRectF]], + top_texts: List[str], + left_texts: List[str], + current: Tuple[int, int], + ) -> None: + """One label per axis: `top_texts` along the top edge, `left_texts` down the left. The + rung matching the settings in force is accented, standing in for the box this used to + draw around that patch.""" + painter.save() + painter.setFont(_overlay_label_font(painter)) + inset = _STRIP_LABEL_INSET_PX + accent = QColor(THEME.accent_primary) + for row, col, cell in patches: + for text, flags, on_axis in ( + ( + top_texts[col] if row == 0 else "", + Qt.AlignmentFlag.AlignTop | Qt.AlignmentFlag.AlignHCenter, + col == current[1], + ), + ( + left_texts[row] if col == 0 else "", + Qt.AlignmentFlag.AlignVCenter | Qt.AlignmentFlag.AlignLeft, + row == current[0], + ), + ): + if not text: + continue + box = cell.adjusted(inset, inset, -inset, -inset) + painter.setPen(QColor(0, 0, 0, 190)) + painter.drawText(box.translated(1.0, 1.0), flags, text) + painter.setPen(accent if on_axis else QColor(255, 255, 255, 235)) + painter.drawText(box, flags, text) + painter.restore() + def _draw_test_strip(self, painter: QPainter) -> None: - """Darkroom test strip: the frame sliced into a density × grade grid, each patch - printed at its own settings. Columns darken left to right, rows soften top to - bottom, so the diagonals read light/dark and hard/soft. Click a patch to keep it. + """A darkroom proof mosaic: the frame sliced into a grid, each patch printed at its own + settings. Click a patch to keep it. + + Tone strip — columns darken left to right, rows soften top to bottom, so the diagonals + read light/dark and hard/soft. Colour ring-around — the centre patch is the filtration + in force and the ring steps ±5cc on the magenta and yellow axes, so the direction of a + cast is visible instead of guessed. The mosaic replaces the canvas frame over the content rect rather than tinting it — these are real renders, and a wash over them would misreport the tone being judged. @@ -793,7 +841,7 @@ def _draw_test_strip(self, painter: QPainter) -> None: painter.drawImage(rect, img) patches = self._strip_patch_rects(rect) - current = strip_nearest_cell(self.state.config.exposure.density, self.state.config.exposure.grade) + rows, cols = self._strip_grid() # No grid: the patches are meant to be read as one print, the way a real test # strip is. Only the patch under the cursor gets an outline, so it's obvious what @@ -808,36 +856,27 @@ def _draw_test_strip(self, painter: QPainter) -> None: painter.setPen(pen) painter.drawRect(cell) - if min(rect.width() / len(STRIP_DENSITIES), rect.height() / len(STRIP_GRADES)) < _STRIP_LABEL_MIN_PX: - return - painter.save() - painter.setFont(_overlay_label_font(painter)) - inset = _STRIP_LABEL_INSET_PX - accent = QColor(THEME.accent_primary) - for row, col, cell in patches: - # Density along the top edge, grade down the left — each axis labelled once. - # The rung matching the current settings is accented, standing in for the - # box this used to draw around that patch. - for text, flags, on_axis in ( - ( - f"D {STRIP_DENSITIES[col]:.1f}" if row == 0 else "", - Qt.AlignmentFlag.AlignTop | Qt.AlignmentFlag.AlignHCenter, - col == current[1], - ), - ( - f"R {STRIP_GRADES[row]:.0f}" if col == 0 else "", - Qt.AlignmentFlag.AlignVCenter | Qt.AlignmentFlag.AlignLeft, - row == current[0], - ), - ): - if not text: - continue - box = cell.adjusted(inset, inset, -inset, -inset) - painter.setPen(QColor(0, 0, 0, 190)) - painter.drawText(box.translated(1.0, 1.0), flags, text) - painter.setPen(accent if on_axis else QColor(255, 255, 255, 235)) - painter.drawText(box, flags, text) - painter.restore() + if min(rect.width() / cols, rect.height() / rows) < _STRIP_LABEL_MIN_PX: + return + if self.state.test_strip_kind == "colour": + # The centre patch is the filtration in force by construction, so the accent needs + # no search for the nearest rung. + self._draw_strip_labels( + painter, + patches, + [f"Y {ring_cc_offset(c):+.0f}" for c in range(cols)], + [f"M {ring_cc_offset(r):+.0f}" for r in range(rows)], + (1, 1), + ) + else: + exposure = self.state.config.exposure + self._draw_strip_labels( + painter, + patches, + [f"D {d:.1f}" for d in STRIP_DENSITIES], + [f"R {g:.0f}" for g in STRIP_GRADES], + strip_nearest_cell(exposure.density, exposure.grade), + ) def _draw_placed_heals(self, painter: QPainter) -> None: """Thin outlines of committed heals (strokes + legacy spots) while a retouch tool is active.""" @@ -1460,7 +1499,7 @@ def mousePressEvent(self, event: QMouseEvent) -> None: if event.button() == Qt.MouseButton.LeftButton and self.state.test_strip: coords = self._map_to_image_coords(event.position()) if coords is not None: - self.test_strip_picked.emit(*strip_cell_at(*coords)) + self.test_strip_picked.emit(*strip_cell_at(*coords, self._strip_grid())) event.accept() return @@ -1600,7 +1639,7 @@ def mouseMoveEvent(self, event: QMouseEvent) -> None: self.cursor_left.emit() if self.state.test_strip: - hover = strip_cell_at(*coords) if coords is not None else None + hover = strip_cell_at(*coords, self._strip_grid()) if coords is not None else None if hover != self._strip_hover: self._strip_hover = hover self.update() diff --git a/negpy/desktop/view/canvas/toolbar.py b/negpy/desktop/view/canvas/toolbar.py index 903671bd..f0ece09a 100644 --- a/negpy/desktop/view/canvas/toolbar.py +++ b/negpy/desktop/view/canvas/toolbar.py @@ -234,6 +234,15 @@ def _init_ui(self) -> None: self._ov_zones_action.setToolTip( tooltip_with_shortcut("Zone overlay — label each region of the print with its Adams zone", "toggle_zones") ) + self._ov_ring_action = overflow_menu.addAction(qta.icon("mdi.target", color=icon_color), "Colour Ring-Around") + self._ov_ring_action.setCheckable(True) + self._ov_ring_action.setToolTip( + tooltip_with_shortcut( + "Colour ring-around — print the frame as a 3x3 mosaic, the centre at the current " + "filtration and the ring 5cc either way on magenta and yellow", + "toggle_ring_around", + ) + ) self._ov_undo_action = overflow_menu.addAction(qta.icon("mdi.undo", color=icon_color), "Undo") self._ov_undo_action.setToolTip(tooltip_with_shortcut("Undo", "undo")) self._ov_redo_action = overflow_menu.addAction(qta.icon("mdi.redo", color=icon_color), "Redo") @@ -397,6 +406,12 @@ def _on_zones_changed(self, active: bool) -> None: widget.setChecked(active) widget.blockSignals(False) + def _sync_ring_action(self) -> None: + # Both proofs share one slot, so the kind gates this or the tone strip checks it too. + state = self.controller.state + active = (state.test_strip or state.test_strip_pending) and state.test_strip_kind == "colour" + self._ov_ring_action.setChecked(active) + def _on_flat_peek_changed(self, active: bool) -> None: self.btn_flat_peek.blockSignals(True) self.btn_flat_peek.setChecked(active) @@ -429,6 +444,8 @@ def _connect_signals(self) -> None: self.btn_zones.toggled.connect(lambda checked: self.controller.toggle_zones_overlay(force=checked)) self._ov_zones_action.triggered.connect(lambda checked: self.controller.toggle_zones_overlay(force=checked)) self.controller.zones_overlay_changed.connect(self._on_zones_changed) + self._ov_ring_action.triggered.connect(lambda checked: self.controller.toggle_ring_around(force=checked)) + self.controller.test_strip_changed.connect(lambda _up: self._sync_ring_action()) self._ov_gpu_action.toggled.connect(self._on_gpu_toggled) self.controller.zoom_changed.connect(self._on_zoom_changed) diff --git a/negpy/desktop/view/keyboard_shortcuts.py b/negpy/desktop/view/keyboard_shortcuts.py index c753406c..9f04dca7 100644 --- a/negpy/desktop/view/keyboard_shortcuts.py +++ b/negpy/desktop/view/keyboard_shortcuts.py @@ -98,6 +98,7 @@ def _build_actions(self) -> dict[str, Callable[[], None]]: "toggle_flat_peek": controller.toggle_flat_peek, "toggle_zones": controller.toggle_zones_overlay, "toggle_test_strip": controller.toggle_test_strip, + "toggle_ring_around": controller.toggle_ring_around, "cancel_tool": lambda: _context_cancel(controller, self.window), "toggle_left_panel": self.window.toggle_session_dock, "toggle_right_panel": self.window.toggle_controls_dock, diff --git a/negpy/desktop/view/shortcut_registry.py b/negpy/desktop/view/shortcut_registry.py index 76aee5a9..f710214a 100644 --- a/negpy/desktop/view/shortcut_registry.py +++ b/negpy/desktop/view/shortcut_registry.py @@ -43,6 +43,7 @@ class ShortcutEntry: "toggle_flat_peek": ShortcutEntry("|", "Peek flat scan (digital intermediate)", "Tools"), "toggle_zones": ShortcutEntry("Shift+Z", "Adams zone overlay", "Tools"), "toggle_test_strip": ShortcutEntry("Shift+T", "Density × grade test strip", "Tools"), + "toggle_ring_around": ShortcutEntry("Shift+F", "Colour ring-around (M/Y filtration)", "Tools"), "cancel_tool": ShortcutEntry("Esc", "Cancel active tool (first press clears in-progress points)", "Tools"), "cyan_dec": ShortcutEntry("", "Cyan down", "Exposure"), "cyan_inc": ShortcutEntry("", "Cyan up", "Exposure"), diff --git a/negpy/desktop/view/sidebar/colour.py b/negpy/desktop/view/sidebar/colour.py index 28a2b63f..dd06205d 100644 --- a/negpy/desktop/view/sidebar/colour.py +++ b/negpy/desktop/view/sidebar/colour.py @@ -4,6 +4,7 @@ from negpy.desktop.session import ToolMode from negpy.desktop.view.shortcut_registry import tooltip_with_shortcut from negpy.desktop.view.sidebar.base import BaseSidebar +from negpy.desktop.view.styles.templates import wrap_tooltip from negpy.desktop.view.styles.theme import THEME from negpy.desktop.view.widgets.sliders import CompactSlider, KelvinSlider from negpy.features.exposure.logic import kelvin_to_wb, wb_to_kelvin @@ -60,10 +61,14 @@ def _init_ui(self) -> None: self.region_reset_btn = QPushButton(" Reset") self.region_reset_btn.setIcon(qta.icon("fa5s.undo", color=THEME.text_primary, color_disabled=THEME.text_muted)) self.region_reset_btn.setToolTip("Reset the selected region's white balance — Temperature and Cyan/Magenta/Yellow back to neutral") + self.ring_btn = self._tool_toggle("mdi.target", "", self._ring_tooltip()) + self.ring_btn.setFixedWidth(36) + tools_row = QHBoxLayout() tools_row.addWidget(self.pick_wb_btn, 1) tools_row.addWidget(self.temp_lock_btn, 1) tools_row.addWidget(self.region_reset_btn, 1) + tools_row.addWidget(self.ring_btn) self.layout.addLayout(tools_row) # Temperature lever over the selected region's M/Y pair (real darkroom: cyan stays 0). @@ -127,6 +132,10 @@ def _connect_signals(self) -> None: self.yellow_slider.valueCommitted.connect(lambda v: self._on_yellow_changed(v, persist=True)) self.pick_wb_btn.toggled.connect(self._on_pick_wb_toggled) + self.ring_btn.clicked.connect(lambda checked: self.controller.toggle_ring_around(force=checked)) + # The ring is session state, not config, and any render drops it — so the button has + # to follow the controller rather than sync_ui. + self.controller.test_strip_changed.connect(self._sync_ring_btn) self.cast_removal_slider.valueChanged.connect( lambda v: self.update_config_section("exposure", render=True, persist=False, readback_metrics=False, cast_removal_strength=v) ) @@ -134,6 +143,27 @@ def _connect_signals(self) -> None: lambda v: self.update_config_section("exposure", render=True, persist=True, readback_metrics=True, cast_removal_strength=v) ) + @staticmethod + def _ring_tooltip(printing: bool = False) -> str: + if printing: + return "Printing the colour ring-around…" + return tooltip_with_shortcut( + "Colour Ring-Around: print the frame as a 3×3 mosaic — the centre patch at the current " + "filtration, the ring ±5cc on the magenta and yellow axes. Click the patch that looks " + "neutral to keep its filtration. With Cast Removal on the patches separate less, since " + "it corrects toward neutral underneath.", + "toggle_ring_around", + ) + + def _sync_ring_btn(self, _up: bool) -> None: + # Both proofs share one slot, so the kind has to gate this or the tone strip lights + # this button too. + mine = self.state.test_strip_kind == "colour" + pending = self.state.test_strip_pending and mine + self.ring_btn.setChecked((self.state.test_strip or self.state.test_strip_pending) and mine) + self.ring_btn.setEnabled(not pending) + self.ring_btn.setToolTip(wrap_tooltip(self._ring_tooltip(printing=pending))) + def _on_temp_drag_started(self) -> None: # Anchor (M, Y) for the whole drag: re-projecting an already-clipped # pair on every tick would corrupt the tint component. @@ -207,6 +237,7 @@ def block_signals(self, blocked: bool) -> None: self.magenta_slider, self.yellow_slider, self.pick_wb_btn, + self.ring_btn, self.cast_removal_slider, ): w.blockSignals(blocked) diff --git a/negpy/desktop/view/sidebar/tone.py b/negpy/desktop/view/sidebar/tone.py index 7375b664..2722363b 100644 --- a/negpy/desktop/view/sidebar/tone.py +++ b/negpy/desktop/view/sidebar/tone.py @@ -295,8 +295,11 @@ def _sync_test_strip_btn(self, _up: bool) -> None: """ponytail: the whole strip is one job with no progress reporting — the button is icon-only, so it goes dead with a 'printing' tooltip rather than changing its label. Per-patch progress if 36 renders ever feels long.""" - pending = self.state.test_strip_pending - self.test_strip_btn.setChecked(self.state.test_strip or pending) + # Both proofs share one slot, so the kind has to gate this or the ring lights this + # button too. + mine = self.state.test_strip_kind == "tone" + pending = self.state.test_strip_pending and mine + self.test_strip_btn.setChecked((self.state.test_strip or self.state.test_strip_pending) and mine) self.test_strip_btn.setEnabled(not pending) self.test_strip_btn.setToolTip(wrap_tooltip(self._test_strip_tooltip(printing=pending))) diff --git a/negpy/desktop/workers/render.py b/negpy/desktop/workers/render.py index 91ba96ed..3445f343 100644 --- a/negpy/desktop/workers/render.py +++ b/negpy/desktop/workers/render.py @@ -8,7 +8,7 @@ from negpy.domain.interfaces import PipelineContext from negpy.domain.models import WorkspaceConfig -from negpy.features.exposure.analysis import output_histogram, strip_cells, strip_mosaic +from negpy.features.exposure.analysis import output_histogram, strip_mosaic from negpy.features.flatfield.logic import apply_flatfield from negpy.features.geometry.batch_autocrop import detect_crop_candidate, resolve_roll_crops from negpy.features.process.sensor import apply_sensor_correction, effective_sensor_matrix @@ -55,12 +55,16 @@ class RenderTask: @dataclass(frozen=True) class TestStripTask: - """Request to print a density × grade test strip off one frame.""" + """Request to print a proof mosaic off one frame — the density × grade test strip, or the + colour ring-around. `overrides` carries one ExposureConfig field-override dict per patch, + row-major over `grid`.""" buffer: np.ndarray config: WorkspaceConfig source_hash: str preview_size: float + overrides: tuple + grid: tuple icc_input_path: Optional[str] = None icc_output_path: Optional[str] = None color_space: str = "Adobe RGB" @@ -264,20 +268,20 @@ def _soft_proof( @pyqtSlot(TestStripTask) def build_strip(self, task: TestStripTask) -> None: - """Render the frame once per ladder rung and keep only each rung's patch. + """Render the frame once per patch and keep only that patch. Runs on the render thread with the canvas's own ImageProcessor, so the patches are - the same pixels the canvas would show — density/grade are absent from the analysis - cache key, so the per-frame metering is reused and only the exposure stage onward - re-dispatches. Metrics are dropped on the floor: a proof must not disturb the - histogram/bounds writeback the real render owns. + the same pixels the canvas would show. The fields any proof overrides — density and + grade for the tone strip, the colour head's magenta/yellow for the ring-around — are + all absent from the analysis cache key, so the per-frame metering is reused and only + the exposure stage onward re-dispatches. Metrics are dropped on the floor: a proof + must not disturb the histogram/bounds writeback the real render owns. """ try: tiles = [] content_rect = None - cells = strip_cells() - for _, _, density, grade in cells: - config = replace(task.config, exposure=replace(task.config.exposure, density=density, grade=grade)) + for override in task.overrides: + config = replace(task.config, exposure=replace(task.config.exposure, **override)) result, metrics = self._processor.run_pipeline( task.buffer, config, @@ -293,9 +297,9 @@ def build_strip(self, task: TestStripTask) -> None: tiles.append(result) if content_rect is None: content_rect = metrics.get("content_rect") - self.strip_progress.emit(len(tiles), len(cells)) + self.strip_progress.emit(len(tiles), len(task.overrides)) - mosaic = strip_mosaic(tiles) + mosaic = strip_mosaic(tiles, task.grid) # Proof the assembled mosaic, not each tile: the transform is per-pixel, so # one pass over the finished frame is identical and 16x cheaper. if task.icc_input_path or task.icc_output_path: diff --git a/negpy/features/exposure/analysis.py b/negpy/features/exposure/analysis.py index 1d728de7..96caef73 100644 --- a/negpy/features/exposure/analysis.py +++ b/negpy/features/exposure/analysis.py @@ -137,43 +137,92 @@ def zone_region_labels(zones: np.ndarray) -> List[Tuple[int, int, int]]: STRIP_GRADES = (55.0, 80.0, 105.0, 130.0, 155.0, 180.0) +STRIP_GRID = (len(STRIP_GRADES), len(STRIP_DENSITIES)) # (rows, cols) + +# Colour ring-around: ±1 CC step on the magenta and yellow axes around the filtration in +# force, so the centre patch is the print being judged. Relative rather than absolute like +# the strip's ladders — the question is which way the cast lies *from here*, which an +# absolute grid can't ask once a correction is already dialled in. 1.0 slider = 20cc +# (see filtration_offsets), so 0.25 is the classic RA4 5cc increment; it also keeps the +# centre unclamped anywhere in [-0.75, +0.75]. Calibration knob: retune by eye. +RING_CC_STEP = 0.25 +RING_CC_PER_UNIT = 20.0 +RING_GRID = (3, 3) + + def strip_cells() -> List[Tuple[int, int, float, float]]: """(row, col, density, grade) for every patch, row-major.""" return [(r, c, d, g) for r, g in enumerate(STRIP_GRADES) for c, d in enumerate(STRIP_DENSITIES)] +def strip_overrides() -> List[dict]: + """ExposureConfig field overrides per patch, row-major over STRIP_GRID.""" + return [{"density": d, "grade": g} for _, _, d, g in strip_cells()] + + +def ring_cells(magenta: float, yellow: float) -> List[Tuple[int, int, float, float]]: + """(row, col, wb_magenta, wb_yellow) row-major; cell (1, 1) is exactly the filtration + passed in. Rows step magenta, columns step yellow — cyan stays 0, as in a real + subtractive head. + + Values are clipped to the slider domain, so at a rail two rows print the same patch. + That is the head running out of travel, not a bug. + """ + rows, cols = RING_GRID + out: List[Tuple[int, int, float, float]] = [] + for r in range(rows): + m = float(np.clip(magenta + (r - 1) * RING_CC_STEP, -1.0, 1.0)) if r != 1 else magenta + for c in range(cols): + y = float(np.clip(yellow + (c - 1) * RING_CC_STEP, -1.0, 1.0)) if c != 1 else yellow + out.append((r, c, m, y)) + return out + + +def ring_overrides(magenta: float, yellow: float) -> List[dict]: + """ExposureConfig field overrides per patch — only the two colour-head fields, so a + replace() built from these can't disturb density, grade or cyan.""" + return [{"wb_magenta": m, "wb_yellow": y} for _, _, m, y in ring_cells(magenta, yellow)] + + +def ring_cc_offset(index: int) -> float: + """Signed CC offset of grid index 0/1/2 — the number the ring's axis labels show.""" + return (index - 1) * RING_CC_STEP * RING_CC_PER_UNIT + + def _strip_bounds(extent: int, divisions: int, index: int) -> Tuple[int, int]: return round(extent * index / divisions), round(extent * (index + 1) / divisions) -def strip_mosaic(tiles: List[np.ndarray]) -> np.ndarray: - """One frame assembled from `strip_cells()`-ordered renders, each contributing only - its own patch. Bounds are rounded from the same fractions on both sides of a seam, - so patches tile exactly — no gap, no overlap.""" - cells = strip_cells() - if len(tiles) != len(cells): - raise ValueError(f"expected {len(cells)} tiles, got {len(tiles)}") +def strip_mosaic(tiles: List[np.ndarray], grid: Tuple[int, int]) -> np.ndarray: + """One frame assembled from row-major renders over `grid`, each contributing only its own + patch. Bounds are rounded from the same fractions on both sides of a seam, so patches + tile exactly — no gap, no overlap.""" + rows, cols = grid + if len(tiles) != rows * cols: + raise ValueError(f"expected {rows * cols} tiles, got {len(tiles)}") out = np.empty_like(tiles[0]) h, w = out.shape[:2] - for (row, col, _, _), tile in zip(cells, tiles): + for i, tile in enumerate(tiles): if tile.shape != out.shape: raise ValueError(f"tile shape {tile.shape} != {out.shape}") - y0, y1 = _strip_bounds(h, len(STRIP_GRADES), row) - x0, x1 = _strip_bounds(w, len(STRIP_DENSITIES), col) + row, col = divmod(i, cols) + y0, y1 = _strip_bounds(h, rows, row) + x0, x1 = _strip_bounds(w, cols, col) out[y0:y1, x0:x1] = tile[y0:y1, x0:x1] return out -def strip_patch_rect(h: int, w: int, row: int, col: int) -> Tuple[int, int, int, int]: +def strip_patch_rect(h: int, w: int, row: int, col: int, grid: Tuple[int, int]) -> Tuple[int, int, int, int]: """(x0, y0, x1, y1) of one patch inside an h×w frame — the bounds `strip_mosaic` filled.""" - y0, y1 = _strip_bounds(h, len(STRIP_GRADES), row) - x0, x1 = _strip_bounds(w, len(STRIP_DENSITIES), col) + rows, cols = grid + y0, y1 = _strip_bounds(h, rows, row) + x0, x1 = _strip_bounds(w, cols, col) return x0, y0, x1, y1 -def strip_cell_at(nx: float, ny: float) -> Tuple[int, int]: +def strip_cell_at(nx: float, ny: float, grid: Tuple[int, int]) -> Tuple[int, int]: """Content-normalized position (0..1) -> (row, col).""" - rows, cols = len(STRIP_GRADES), len(STRIP_DENSITIES) + rows, cols = grid return ( int(np.clip(int(ny * rows), 0, rows - 1)), int(np.clip(int(nx * cols), 0, cols - 1)), diff --git a/tests/test_canvas_toolbar.py b/tests/test_canvas_toolbar.py index 93cd1de4..68190e85 100644 --- a/tests/test_canvas_toolbar.py +++ b/tests/test_canvas_toolbar.py @@ -101,6 +101,8 @@ def _all_overflow_actions(self, tb: ActionToolbar) -> list: tb._ov_original_action, tb._ov_compare_action, tb._ov_flat_peek_action, + tb._ov_zones_action, + tb._ov_ring_action, tb._ov_undo_action, tb._ov_redo_action, tb._ov_rot_l_action, diff --git a/tests/test_ring_around.py b/tests/test_ring_around.py new file mode 100644 index 00000000..e4c2b0db --- /dev/null +++ b/tests/test_ring_around.py @@ -0,0 +1,96 @@ +"""Colour ring-around ladder: ±5cc on the magenta and yellow axes around the filtration in +force, so the centre patch is the print being judged. + +Relative rather than absolute, which makes two properties load-bearing: the centre must be +*exactly* the input, and at a slider rail the ring legitimately prints duplicate patches. +""" + +import pytest + +from negpy.features.exposure.analysis import ( + RING_CC_PER_UNIT, + RING_CC_STEP, + RING_GRID, + ring_cc_offset, + ring_cells, + ring_overrides, + strip_cell_at, +) +from negpy.features.exposure.models import ExposureConfig + + +def _cell(cells, row, col): + return cells[row * RING_GRID[1] + col] + + +def test_the_centre_patch_is_exactly_the_filtration_passed_in(): + """Not merely close: the centre is the print being judged, so any rounding there would + make the ring compare against something the user isn't looking at.""" + cells = ring_cells(0.2, -0.1) + assert _cell(cells, 1, 1) == (1, 1, 0.2, -0.1) + + +def test_the_grid_is_nine_cells_row_major(): + cells = ring_cells(0.0, 0.0) + assert len(cells) == RING_GRID[0] * RING_GRID[1] == 9 + assert [(r, c) for r, c, _, _ in cells] == [(r, c) for r in range(3) for c in range(3)] + + +def test_rows_step_magenta_and_columns_step_yellow(): + cells = ring_cells(0.0, 0.0) + # Within a row only yellow varies... + for row in range(3): + magentas = {_cell(cells, row, c)[2] for c in range(3)} + assert len(magentas) == 1 + yellows = [_cell(cells, row, c)[3] for c in range(3)] + assert yellows == [-RING_CC_STEP, 0.0, RING_CC_STEP] + # ...and within a column only magenta. + for col in range(3): + yellows = {_cell(cells, r, col)[3] for r in range(3)} + assert len(yellows) == 1 + magentas = [_cell(cells, r, col)[2] for r in range(3)] + assert magentas == [-RING_CC_STEP, 0.0, RING_CC_STEP] + + +def test_the_step_is_the_classic_five_cc(): + # filtration_offsets documents 1.0 slider = 20cc, so this pins the step to a real + # darkroom increment rather than an arbitrary slider fraction. + assert RING_CC_STEP * RING_CC_PER_UNIT == 5.0 + assert [ring_cc_offset(i) for i in range(3)] == [-5.0, 0.0, 5.0] + + +def test_the_centre_stays_unclamped_across_the_usable_range(): + for value in (-0.75, -0.4, 0.0, 0.4, 0.75): + cells = ring_cells(value, value) + assert _cell(cells, 1, 1) == (1, 1, value, value) + # Every rung is a distinct printable value away from the rails. + assert len({m for _, _, m, _ in cells}) == 3 + assert len({y for _, _, _, y in cells}) == 3 + + +def test_at_a_rail_the_ring_prints_duplicate_patches(): + """The head has run out of travel. That is real behaviour, not a bug — the strip forbids + duplicates, the ring documents when they legitimately happen.""" + cells = ring_cells(1.0, 0.0) + assert _cell(cells, 1, 1)[2] == 1.0 # centre still exact + # The plus row clamps onto the centre row's magenta, so those patches coincide. + assert _cell(cells, 2, 0)[2] == _cell(cells, 1, 0)[2] == 1.0 + assert _cell(cells, 0, 0)[2] == pytest.approx(1.0 - RING_CC_STEP) + + +def test_overrides_touch_only_the_two_colour_head_fields(): + """A replace() built from these must not be able to disturb density, grade or cyan.""" + overrides = ring_overrides(0.1, -0.2) + assert len(overrides) == 9 + for override in overrides: + assert set(override) == {"wb_magenta", "wb_yellow"} + # And the values really do reach an ExposureConfig unchanged. + cfg = ExposureConfig(**overrides[0]) + assert cfg.wb_cyan == 0.0 and cfg.wb_magenta == overrides[0]["wb_magenta"] + + +def test_ring_clicks_map_across_the_three_by_three_grid(): + assert strip_cell_at(0.0, 0.0, RING_GRID) == (0, 0) + assert strip_cell_at(0.5, 0.5, RING_GRID) == (1, 1) + assert strip_cell_at(0.99, 0.99, RING_GRID) == (2, 2) + assert strip_cell_at(1.4, -0.2, RING_GRID) == (0, 2) # clamped, not wrapped diff --git a/tests/test_ring_around_controller.py b/tests/test_ring_around_controller.py new file mode 100644 index 00000000..09de3c02 --- /dev/null +++ b/tests/test_ring_around_controller.py @@ -0,0 +1,214 @@ +"""Ring-around lifecycle: print → pick → commit, sharing one proof slot with the test strip. + +The interesting differences from the strip are deliberate: the ring's ladder is relative, so +its memo must NOT be pinned on filtration (picking has to invalidate), and asking for the +other kind while a proof is up swaps rather than dismisses. +""" + +import unittest +from dataclasses import replace +from unittest.mock import MagicMock, patch + +import numpy as np + +from negpy.domain.models import WorkspaceConfig +from negpy.features.exposure.analysis import RING_GRID, STRIP_GRID, ring_cells, ring_overrides + + +class RingAroundWorker(unittest.TestCase): + def test_every_patch_is_rendered_at_its_own_filtration(self): + with patch("negpy.desktop.workers.render.ImageProcessor") as MockIP: + from negpy.desktop.workers.render import RenderWorker, TestStripTask + + seen: list = [] + + def fake_pipeline(_buf, config, *a, **k): + e = config.exposure + seen.append((e.wb_magenta, e.wb_yellow, e.wb_cyan, e.density, e.grade)) + return np.full((9, 9, 3), float(len(seen) - 1), np.float32), {"content_rect": (0, 0, 9, 9)} + + MockIP.return_value.run_pipeline.side_effect = fake_pipeline + worker = RenderWorker() + done: list = [] + worker.strip_finished.connect(lambda m, r: done.append((m, r))) + + base = WorkspaceConfig() + base = replace(base, exposure=replace(base.exposure, wb_magenta=0.2, wb_yellow=-0.1)) + worker.build_strip( + TestStripTask( + buffer=np.zeros((9, 9, 3), np.float32), + config=base, + source_hash="f1", + preview_size=512.0, + overrides=tuple(ring_overrides(0.2, -0.1)), + grid=RING_GRID, + ) + ) + + expected = [(m, y) for _, _, m, y in ring_cells(0.2, -0.1)] + self.assertEqual([(m, y) for m, y, _, _, _ in seen], expected) + # Nothing else moved: cyan, density and grade are the base config's on every patch. + for _m, _y, cyan, density, grade in seen: + self.assertEqual(cyan, base.exposure.wb_cyan) + self.assertEqual(density, base.exposure.density) + self.assertEqual(grade, base.exposure.grade) + + mosaic, _rect = done[0] + # Top-left patch from render 0, bottom-right from render 8 — the 3x3 slicing is right. + self.assertEqual(mosaic[0, 0, 0], 0.0) + self.assertEqual(mosaic[-1, -1, 0], 8.0) + + +class RingAroundLifecycle(unittest.TestCase): + def setUp(self): + from negpy.desktop.controller import AppController + from negpy.desktop.session import AppState, DesktopSessionManager + from negpy.services.rendering.preview_manager import PreviewManager + + self.mock_session_manager = MagicMock(spec=DesktopSessionManager) + self.mock_session_manager.state = AppState() + self.mock_session_manager.repo = MagicMock() + with ( + patch("negpy.desktop.controller.RenderWorker") as mock_rw_class, + patch("negpy.desktop.controller.PreviewManager") as mock_pm_class, + ): + mock_rw_class.return_value = MagicMock() + mock_pm_class.return_value = MagicMock(spec=PreviewManager) + mock_pm_class.return_value.load_linear_preview.return_value = (None, (0, 0), {}) + self.controller = AppController(self.mock_session_manager) + self.controller.state.preview_raw = np.empty((8, 8, 3), dtype=np.float32) + self.controller.state.current_file_hash = "f1" + self.strip_tasks: list = [] + self.controller.strip_requested.connect(self.strip_tasks.append) + # update_config on a MagicMock session doesn't write through; mirror it so the + # controller sees the committed config the real session would have stored. + self.mock_session_manager.update_config.side_effect = self._commit + + def _commit(self, config, persist=False, **kwargs): + self.controller.state.config = config + + def tearDown(self): + import gc + + for thread in [ + self.controller.render_thread, + self.controller.export_thread, + self.controller.thumb_thread, + self.controller.norm_thread, + self.controller.discovery_thread, + self.controller.preview_load_thread, + self.controller.scan_thread, + ]: + if thread is not None and thread.isRunning(): + thread.quit() + thread.wait() + del self.controller + gc.collect() + + def _mosaic(self) -> np.ndarray: + return np.zeros((9, 9, 3), dtype=np.float32) + + def _print_ring(self) -> None: + self.controller.toggle_ring_around() + self.controller.on_strip_finished(self._mosaic(), (0, 0, 9, 9)) + + def _set_filtration(self, magenta: float, yellow: float) -> None: + cfg = self.controller.state.config + self.controller.state.config = replace(cfg, exposure=replace(cfg.exposure, wb_magenta=magenta, wb_yellow=yellow)) + + def test_toggling_on_dispatches_a_nine_patch_job_around_the_current_filtration(self): + self._set_filtration(0.2, -0.1) + self.controller.toggle_ring_around() + + self.assertEqual(len(self.strip_tasks), 1) + task = self.strip_tasks[0] + self.assertEqual(task.grid, RING_GRID) + self.assertEqual(len(task.overrides), 9) + self.assertEqual(self.controller.state.test_strip_kind, "colour") + self.assertEqual(self.controller.state.test_strip_origin, (0.2, -0.1)) + + def test_picking_a_patch_commits_only_its_filtration(self): + self._set_filtration(0.2, -0.1) + self._print_ring() + before = self.controller.state.config.exposure + + self.controller.apply_test_strip_pick(0, 0) + + after = self.controller.state.config.exposure + _, _, m, y = ring_cells(0.2, -0.1)[0] + self.assertAlmostEqual(after.wb_magenta, m) + self.assertAlmostEqual(after.wb_yellow, y) + # Everything the patches were rendered under is left exactly as it was. + self.assertEqual(after.wb_cyan, before.wb_cyan) + self.assertEqual(after.density, before.density) + self.assertEqual(after.grade, before.grade) + self.assertEqual(after.cast_removal_strength, before.cast_removal_strength) + self.assertEqual(after.auto_exposure, before.auto_exposure) + self.assertEqual(after.auto_normalize_contrast, before.auto_normalize_contrast) + self.assertFalse(self.controller.state.test_strip) + + def test_picking_the_centre_keeps_the_filtration_and_still_clears(self): + self._set_filtration(0.2, -0.1) + self._print_ring() + self.controller.apply_test_strip_pick(1, 1) + + after = self.controller.state.config.exposure + self.assertEqual((after.wb_magenta, after.wb_yellow), (0.2, -0.1)) + self.assertFalse(self.controller.state.test_strip) + + def test_picking_a_patch_invalidates_the_ring_cache(self): + """The inverse of the tone strip's cache-hit test, and deliberately so: the ring's + ladder is relative to the filtration in force, so once that moves the old mosaic is + no longer a proof of anything. A ring-around is meant to be iterated.""" + self._print_ring() + self.controller.apply_test_strip_pick(0, 0) + self.controller.toggle_ring_around() + self.assertEqual(len(self.strip_tasks), 2) # a second real job, not a cache hit + + def test_the_two_proof_kinds_never_share_a_cache_entry(self): + """RenderMemo holds one entry per file hash, so the kind is folded into the key — + otherwise printing the ring then the strip would paint the ring's mosaic.""" + self._print_ring() + self.controller._clear_test_strip() + self.controller.toggle_test_strip() + self.assertEqual(len(self.strip_tasks), 2) + self.assertEqual(self.strip_tasks[1].grid, STRIP_GRID) + + def test_asking_for_the_other_kind_swaps_the_proof(self): + self._print_ring() + self.assertEqual(self.controller.state.test_strip_kind, "colour") + + self.controller.toggle_test_strip() + self.assertEqual(self.controller.state.test_strip_kind, "tone") + self.assertEqual(len(self.strip_tasks), 2) + + self.controller.on_strip_finished(self._mosaic(), (0, 0, 9, 9)) + self.controller.toggle_ring_around() + self.assertEqual(self.controller.state.test_strip_kind, "colour") + self.assertEqual(len(self.strip_tasks), 3) + + def test_toggling_the_same_kind_again_dismisses_it(self): + self._print_ring() + self.controller.toggle_ring_around() + self.assertFalse(self.controller.state.test_strip) + + def test_a_render_drops_the_ring(self): + self._print_ring() + self.controller.request_render() + self.assertFalse(self.controller.state.test_strip) + self.assertIsNone(self.controller.state.test_strip_mosaic) + + def test_compare_and_flat_peek_take_the_canvas_from_the_ring(self): + for enter_mode in (self.controller.toggle_compare, lambda: self.controller.toggle_flat_peek(force=True)): + self.controller.state.compare_mode = False + self.controller.state.flat_peek = False + self.controller._is_rendering = False + self._print_ring() + + enter_mode() + self.assertFalse(self.controller.state.test_strip) + self.assertIsNone(self.controller.state.test_strip_mosaic) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_test_strip.py b/tests/test_test_strip.py index 4a441994..72d76904 100644 --- a/tests/test_test_strip.py +++ b/tests/test_test_strip.py @@ -2,8 +2,10 @@ import pytest from negpy.features.exposure.analysis import ( + RING_GRID, STRIP_DENSITIES, STRIP_GRADES, + STRIP_GRID, strip_cell_at, strip_cells, strip_mosaic, @@ -13,9 +15,9 @@ from negpy.features.exposure.models import EXPOSURE_CONSTANTS, ExposureConfig -def _tiles(h: int = 40, w: int = 60) -> list[np.ndarray]: +def _tiles(h: int = 40, w: int = 60, grid=STRIP_GRID) -> list[np.ndarray]: """One flat tile per cell, each a distinct value = its index.""" - return [np.full((h, w, 3), float(i), dtype=np.float32) for i in range(len(strip_cells()))] + return [np.full((h, w, 3), float(i), dtype=np.float32) for i in range(grid[0] * grid[1])] def test_ladder_covers_every_cell_row_major(): @@ -33,44 +35,52 @@ def test_grade_ladder_survives_the_legacy_post_init_ladder(): assert ExposureConfig(grade=grade).grade == grade -def test_every_patch_comes_from_its_own_tile(): - tiles = _tiles() - mosaic = strip_mosaic(tiles) +# One test per geometry helper, run over both proof grids — the tone strip and the ring share +# the slicer, so a change to it must be pinned for both. +@pytest.mark.parametrize("grid", [STRIP_GRID, RING_GRID]) +def test_every_patch_comes_from_its_own_tile(grid): + tiles = _tiles(grid=grid) + mosaic = strip_mosaic(tiles, grid) h, w = mosaic.shape[:2] - for i, (row, col, _, _) in enumerate(strip_cells()): - x0, y0, x1, y1 = strip_patch_rect(h, w, row, col) + for i in range(grid[0] * grid[1]): + row, col = divmod(i, grid[1]) + x0, y0, x1, y1 = strip_patch_rect(h, w, row, col, grid) assert np.all(mosaic[y0:y1, x0:x1] == float(i)) -def test_patches_tile_exactly_with_no_gap_or_overlap(): - h, w = 41, 63 # deliberately not divisible by the grid +@pytest.mark.parametrize("grid", [STRIP_GRID, RING_GRID]) +def test_patches_tile_exactly_with_no_gap_or_overlap(grid): + h, w = 41, 63 # deliberately not divisible by either grid covered = np.zeros((h, w), dtype=int) - for row, col, _, _ in strip_cells(): - x0, y0, x1, y1 = strip_patch_rect(h, w, row, col) + for i in range(grid[0] * grid[1]): + row, col = divmod(i, grid[1]) + x0, y0, x1, y1 = strip_patch_rect(h, w, row, col, grid) covered[y0:y1, x0:x1] += 1 assert np.all(covered == 1) -def test_mosaic_rejects_a_short_or_mismatched_tile_set(): +@pytest.mark.parametrize("grid", [STRIP_GRID, RING_GRID]) +def test_mosaic_rejects_a_short_or_mismatched_tile_set(grid): with pytest.raises(ValueError): - strip_mosaic(_tiles()[:-1]) - tiles = _tiles() - tiles[5] = np.zeros((10, 10, 3), dtype=np.float32) + strip_mosaic(_tiles(grid=grid)[:-1], grid) + tiles = _tiles(grid=grid) + tiles[-1] = np.zeros((10, 10, 3), dtype=np.float32) with pytest.raises(ValueError): - strip_mosaic(tiles) + strip_mosaic(tiles, grid) def test_click_maps_to_the_patch_under_it(): - rows, cols = len(STRIP_GRADES), len(STRIP_DENSITIES) - assert strip_cell_at(0.0, 0.0) == (0, 0) - assert strip_cell_at(0.99, 0.99) == (rows - 1, cols - 1) + rows, cols = STRIP_GRID + assert strip_cell_at(0.0, 0.0, STRIP_GRID) == (0, 0) + assert strip_cell_at(0.99, 0.99, STRIP_GRID) == (rows - 1, cols - 1) # Dead centre of the patch at (1, 2). - assert strip_cell_at(2.5 / cols, 1.5 / rows) == (1, 2) + assert strip_cell_at(2.5 / cols, 1.5 / rows, STRIP_GRID) == (1, 2) def test_click_on_the_far_edge_stays_in_the_grid(): - assert strip_cell_at(1.0, 1.0) == (len(STRIP_GRADES) - 1, len(STRIP_DENSITIES) - 1) - assert strip_cell_at(1.4, -0.2) == (0, len(STRIP_DENSITIES) - 1) + rows, cols = STRIP_GRID + assert strip_cell_at(1.0, 1.0, STRIP_GRID) == (rows - 1, cols - 1) + assert strip_cell_at(1.4, -0.2, STRIP_GRID) == (0, cols - 1) def test_current_settings_highlight_the_nearest_patch(): diff --git a/tests/test_test_strip_controller.py b/tests/test_test_strip_controller.py index c9a2bdaf..7529d3ef 100644 --- a/tests/test_test_strip_controller.py +++ b/tests/test_test_strip_controller.py @@ -12,7 +12,13 @@ import numpy as np from negpy.domain.models import WorkspaceConfig -from negpy.features.exposure.analysis import STRIP_DENSITIES, STRIP_GRADES, strip_cells +from negpy.features.exposure.analysis import ( + STRIP_DENSITIES, + STRIP_GRADES, + STRIP_GRID, + strip_cells, + strip_overrides, +) class TestStripWorker(unittest.TestCase): @@ -40,6 +46,8 @@ def fake_pipeline(_buf, config, *a, **k): config=WorkspaceConfig(), source_hash="f1", preview_size=512.0, + overrides=tuple(strip_overrides()), + grid=STRIP_GRID, ) ) @@ -69,6 +77,8 @@ def test_metrics_never_escape_the_strip(self): config=WorkspaceConfig(), source_hash="f1", preview_size=512.0, + overrides=tuple(strip_overrides()), + grid=STRIP_GRID, ) ) # readback_metrics must be off for every variant. diff --git a/tests/test_test_strip_overlay.py b/tests/test_test_strip_overlay.py index 717e960b..58064cfb 100644 --- a/tests/test_test_strip_overlay.py +++ b/tests/test_test_strip_overlay.py @@ -9,7 +9,7 @@ from negpy.desktop.session import AppState, ToolMode from negpy.desktop.view.canvas.overlay import CanvasOverlay -from negpy.features.exposure.analysis import STRIP_DENSITIES, STRIP_GRADES +from negpy.features.exposure.analysis import RING_GRID, STRIP_DENSITIES, STRIP_GRADES def _press(pos: QPointF) -> QMouseEvent: @@ -120,3 +120,38 @@ def test_clearing_the_strip_drops_the_hover_and_the_picker_cursor() -> None: assert overlay._strip_hover is None assert overlay._strip_cache is None + + +def _ring_overlay() -> CanvasOverlay: + state = AppState() + state.test_strip = True + state.test_strip_kind = "colour" + state.test_strip_mosaic = np.zeros((90, 90, 3), dtype=np.float32) + overlay = CanvasOverlay(state) + overlay._view_rect = QRectF(0, 0, 90, 90) + overlay._current_size = (90, 90) + return overlay + + +def test_the_ring_picks_across_its_own_three_by_three_grid() -> None: + """The overlay reads the grid off the proof kind, so a click on the ring must map to 3x3 + rather than the tone strip's 6x6.""" + overlay = _ring_overlay() + picked: list = [] + overlay.test_strip_picked.connect(lambda r, c: picked.append((r, c))) + + overlay.mousePressEvent(_press(QPointF(5, 5))) + overlay.mousePressEvent(_press(QPointF(45, 45))) + overlay.mousePressEvent(_press(QPointF(85, 85))) + + assert picked == [(0, 0), (1, 1), (RING_GRID[0] - 1, RING_GRID[1] - 1)] + + +def test_the_ring_lays_out_nine_patches() -> None: + overlay = _ring_overlay() + rects = overlay._strip_patch_rects(QRectF(0, 0, 90, 90)) + assert len(rects) == 9 + assert overlay._strip_grid() == RING_GRID + # And the tone strip still gets its own grid off the same helper. + overlay.state.test_strip_kind = "tone" + assert overlay._strip_grid() == (len(STRIP_GRADES), len(STRIP_DENSITIES)) From 79d2bf21d5b612ec09d296a6fe904d76b3315c78 Mon Sep 17 00:00:00 2001 From: Marcin Zawalski Date: Wed, 29 Jul 2026 10:28:59 +0200 Subject: [PATCH 2/2] Ring-around and test strip become 5x5, both ladders absolute Ring-around: - 5x5 instead of 3x3, stepping 1cc at a time out to 2cc either way on magenta and yellow. More resolution near neutral rather than a wider swing. - The ladder is now absolute and centred on neutral, like the strip's. That makes the mosaic invariant to the filtration in force, so _strip_memo_key pins M/Y the way it pins density and grade for the strip, and picking a patch no longer forces a reprint. test_strip_origin goes away with it, and so does the rail-clamping the relative ladder needed: every rung now sits well inside the sliders' travel. - The accented rung follows the filtration in force via ring_nearest_cell, the same way the strip accents the nearest density and grade. Test strip: 5x5 instead of 6x6, with both ladders centred exactly on their defaults (density 1.0, grade R115), so the settings already in force are the middle patch. Density spans 0.4-1.6 in 0.3 steps and grade R75-R155 in 20-point steps. Consequence worth knowing: with an absolute ladder the pick sets filtration outright rather than nudging it, so the ring only reaches as far as its own span. Verified on samples/DSC00448.ARW: the neutral centre patch is bit-identical to a plain neutral render (max abs diff 0.0), all 25 patch means distinct, the outer magenta rungs move green in opposite directions and the outer yellow rungs move blue, and per-patch cost matches the tone strip's at 0.05s. --- docs/CHANGELOG.md | 4 +- negpy/desktop/controller.py | 53 +++++--------- negpy/desktop/session.py | 5 +- negpy/desktop/view/canvas/overlay.py | 22 +++--- negpy/desktop/view/canvas/toolbar.py | 6 +- negpy/desktop/view/sidebar/colour.py | 14 ++-- negpy/desktop/view/sidebar/tone.py | 5 +- negpy/desktop/workers/render.py | 17 ++--- negpy/features/exposure/analysis.py | 76 +++++++++---------- tests/test_ring_around.py | 105 ++++++++++++++------------- tests/test_ring_around_controller.py | 69 ++++++++++++------ tests/test_test_strip.py | 13 ++-- tests/test_test_strip_overlay.py | 15 ++-- 13 files changed, 203 insertions(+), 201 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f0715021..16a6770e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,8 +2,8 @@ ## 0.45.0 -- New: **Colour ring-around** — the Filtration panel (or Shift+F) prints the frame as a 3×3 mosaic: the centre patch at the current filtration, the ring 5cc either way on the magenta and yellow axes, so the direction of a colour cast is visible instead of guessed. Each patch is a real render; click one to keep its filtration. Unlike the test strip the ladder is relative, so the centre is always the print you are judging and each round starts from wherever you left off. The two proofs share the canvas — asking for one while the other is up swaps it. -- New: **Test strip** — the Tone panel prints the frame as a 6×6 grid, Print Density increasing left to right and ISO-R Grade softening top to bottom, so the diagonals read light-to-dark and soft-to-hard like a split-filter test strip. Each patch is a real render at its own settings; click one to keep it. Escape or a second press clears it, and any edit drops it. +- New: **Colour ring-around** — the Filtration panel (or Shift+F) prints the frame as a 5×5 mosaic stepping 1cc at a time out to ±2cc on the magenta and yellow axes, so the direction of a colour cast is visible instead of guessed. Each patch is a real render; click one to keep its filtration. Like the test strip the ladder is absolute and centred on neutral, so a ring printed off one frame compares to the next and picking a patch doesn't force a reprint. The two proofs share the canvas — asking for one while the other is up swaps it. +- New: **Test strip** — the Tone panel prints the frame as a 5×5 grid, Print Density increasing left to right and ISO-R Grade softening top to bottom, each ladder centred on its default so the settings you already have are one of the patches, so the diagonals read light-to-dark and soft-to-hard like a split-filter test strip. Each patch is a real render at its own settings; click one to keep it. Escape or a second press clears it, and any edit drops it. ## 0.44.0 diff --git a/negpy/desktop/controller.py b/negpy/desktop/controller.py index 932119dc..67afb5e2 100644 --- a/negpy/desktop/controller.py +++ b/negpy/desktop/controller.py @@ -934,21 +934,18 @@ def _render_memo_key(self, config: Optional[WorkspaceConfig] = None) -> str: return hashlib.md5(repr(parts).encode()).hexdigest() def _strip_memo_key(self, kind: str = "tone") -> str: - """The render key for a proof mosaic, prefixed by kind so the two can never collide. + """The render key for a proof mosaic, prefixed by kind so the two can't collide. - The tone strip pins density and grade to fixed values: it supplies those two itself, - one per patch, so its mosaic is invariant to whatever they currently are — which makes - the print/pick/print-again loop a cache hit. Any other edit (crop, filtration, paper, - toning...) lands on a different key and re-prints. - - The ring pins nothing. Its ladder is relative to the filtration in force, so the mosaic - is *not* invariant to it — picking a patch has to invalidate, and should: a ring-around - is meant to be iterated toward neutral, and each round is genuinely new pixels. + Both ladders are absolute, so each proof supplies the fields it varies and its mosaic + is invariant to whatever those currently are. Pinning them makes print/pick/print again + a cache hit. Any other edit (crop, paper, toning...) lands on a different key. """ - config = self.state.config - if kind == "tone": - config = replace(config, exposure=replace(config.exposure, density=1.0, grade=115.0)) - return f"{kind}:{self._render_memo_key(config)}" + exposure = self.state.config.exposure + if kind == "colour": + exposure = replace(exposure, wb_magenta=0.0, wb_yellow=0.0) + else: + exposure = replace(exposure, density=1.0, grade=115.0) + return f"{kind}:{self._render_memo_key(replace(self.state.config, exposure=exposure))}" def load_file(self, file_path: str, preserve_zoom: bool = False, force_detect: bool = False) -> None: """ @@ -1220,12 +1217,10 @@ def toggle_ring_around(self, force: Optional[bool] = None) -> None: self.toggle_test_strip(force, kind="colour") def toggle_test_strip(self, force: Optional[bool] = None, kind: str = "tone") -> None: - """Print (or clear) a proof mosaic: the density × grade test strip, or the colour - ring-around. Unlike the zone overlay these need pixels the canvas doesn't have, so - entering dispatches one job. + """Print (or clear) a proof mosaic: the density × grade strip, or the colour ring-around. + Entering dispatches one job, since these need pixels the canvas doesn't have. - Both share the one proof slot, so asking for the other kind while a proof is up swaps - it rather than dismissing it. + Both share the one proof slot, so asking for the other kind swaps it. """ showing = (self.state.test_strip or self.state.test_strip_pending) and self.state.test_strip_kind == kind target = (not showing) if force is None else bool(force) @@ -1235,14 +1230,11 @@ def toggle_test_strip(self, force: Optional[bool] = None, kind: str = "tone") -> if self.state.preview_raw is None: return - exposure = self.state.config.exposure if kind == "colour": - origin = (exposure.wb_magenta, exposure.wb_yellow) - overrides = ring_overrides(*origin) + overrides = ring_overrides() grid = RING_GRID toast = "Printing the colour ring-around…" else: - origin = (0.0, 0.0) overrides = strip_overrides() grid = STRIP_GRID toast = "Printing test strip…" @@ -1251,18 +1243,15 @@ def toggle_test_strip(self, force: Optional[bool] = None, kind: str = "tone") -> cached = self._strip_memo.get(self.state.current_file_hash or "", self._strip_memo_key(kind)) if cached is not None: self.state.test_strip_kind = kind - self.state.test_strip_origin = origin self.on_strip_finished(cached["mosaic"], cached["content_rect"], from_cache=True) return proofing = self.state.soft_proof_enabled icc_input = self.effective_input_icc() if (proofing or self.state.config.process.narrowband_scan) else None self.state.test_strip_kind = kind - self.state.test_strip_origin = origin self.state.test_strip_pending = True self.test_strip_changed.emit(False) - # These renders take a few seconds; say so up front and tick the HUD progress bar - # per patch so it's clear the app is working rather than wedged. + # A few seconds of renders: tick the HUD so it doesn't read as wedged. self.status_message_requested.emit(toast, 2500) self.status_progress_requested.emit(0, len(overrides)) self.strip_requested.emit( @@ -1271,8 +1260,7 @@ def toggle_test_strip(self, force: Optional[bool] = None, kind: str = "tone") -> config=self.state.config, source_hash=self.state.current_file_hash or "preview", # Always preview res, never HQ: full-res renders per patch would take minutes, - # and each patch is shown at a fraction of the frame's width anyway. The - # mosaic scales to the content rect, so a smaller one costs nothing here. + # and each patch is shown at a fraction of the frame's width anyway. preview_size=float(APP_CONFIG.preview_render_size), overrides=tuple(overrides), grid=grid, @@ -1328,17 +1316,14 @@ def on_strip_finished(self, mosaic: Any, content_rect: Any, from_cache: bool = F def apply_test_strip_pick(self, row: int, col: int) -> None: """Commit the clicked patch's settings, then drop the proof. - Cast Removal and the Auto toggles are left exactly as they were: the patches were - rendered under them, so changing them would render something other than the patch - that was clicked. The ring reads its ladder from the origin it was printed around, - not from the live config — which this is about to change. + Cast Removal and the Auto toggles are left alone: the patches were rendered under + them, so flipping one would render something other than the patch that was clicked. """ if not self.state.test_strip: return exposure = self.state.config.exposure if self.state.test_strip_kind == "colour": - cells = ring_cells(*self.state.test_strip_origin) - _, _, magenta, yellow = cells[row * RING_GRID[1] + col] + _, _, magenta, yellow = ring_cells()[row * RING_GRID[1] + col] new_exposure = replace(exposure, wb_magenta=magenta, wb_yellow=yellow) else: new_exposure = replace(exposure, density=STRIP_DENSITIES[col], grade=STRIP_GRADES[row]) diff --git a/negpy/desktop/session.py b/negpy/desktop/session.py index 97976f71..00825652 100644 --- a/negpy/desktop/session.py +++ b/negpy/desktop/session.py @@ -109,11 +109,8 @@ class AppState: test_strip_mosaic: Optional[Any] = None test_strip_content_rect: Optional[tuple] = None # Which proof owns the canvas: "tone" (density × grade) or "colour" (M/Y ring-around). - # One slot rather than two, so every path that drops a proof drops both kinds. + # One slot, so every path that drops a proof drops both kinds. test_strip_kind: str = "tone" - # (wb_magenta, wb_yellow) the ring was printed around — the pick reads its ladder from - # this, not from the live config, which the pick itself is about to change. - test_strip_origin: tuple = (0.0, 0.0) # Reverse scroll-wheel zoom direction on the image viewer (scroll up = zoom out). invert_zoom_scroll: bool = False diff --git a/negpy/desktop/view/canvas/overlay.py b/negpy/desktop/view/canvas/overlay.py index c7f474e6..7a35e823 100644 --- a/negpy/desktop/view/canvas/overlay.py +++ b/negpy/desktop/view/canvas/overlay.py @@ -18,7 +18,8 @@ STRIP_DENSITIES, STRIP_GRADES, STRIP_GRID, - ring_cc_offset, + ring_cc, + ring_nearest_cell, strip_cell_at, strip_nearest_cell, zone_grid, @@ -791,9 +792,8 @@ def _draw_strip_labels( left_texts: List[str], current: Tuple[int, int], ) -> None: - """One label per axis: `top_texts` along the top edge, `left_texts` down the left. The - rung matching the settings in force is accented, standing in for the box this used to - draw around that patch.""" + """`top_texts` along the top edge, `left_texts` down the left, each axis labelled once. + The rung matching the settings in force is accented.""" painter.save() painter.setFont(_overlay_label_font(painter)) inset = _STRIP_LABEL_INSET_PX @@ -826,8 +826,8 @@ def _draw_test_strip(self, painter: QPainter) -> None: Tone strip — columns darken left to right, rows soften top to bottom, so the diagonals read light/dark and hard/soft. Colour ring-around — the centre patch is the filtration - in force and the ring steps ±5cc on the magenta and yellow axes, so the direction of a - cast is visible instead of guessed. + in force and the ring steps out to ±5cc on the magenta and yellow axes, so the + direction of a cast is visible instead of guessed. The mosaic replaces the canvas frame over the content rect rather than tinting it — these are real renders, and a wash over them would misreport the tone being judged. @@ -859,14 +859,14 @@ def _draw_test_strip(self, painter: QPainter) -> None: if min(rect.width() / cols, rect.height() / rows) < _STRIP_LABEL_MIN_PX: return if self.state.test_strip_kind == "colour": - # The centre patch is the filtration in force by construction, so the accent needs - # no search for the nearest rung. + exposure = self.state.config.exposure self._draw_strip_labels( painter, patches, - [f"Y {ring_cc_offset(c):+.0f}" for c in range(cols)], - [f"M {ring_cc_offset(r):+.0f}" for r in range(rows)], - (1, 1), + # :+g so a fractional step prints as one rather than rounding away. + [f"Y {ring_cc(c):+g}" for c in range(cols)], + [f"M {ring_cc(r):+g}" for r in range(rows)], + ring_nearest_cell(exposure.wb_magenta, exposure.wb_yellow), ) else: exposure = self.state.config.exposure diff --git a/negpy/desktop/view/canvas/toolbar.py b/negpy/desktop/view/canvas/toolbar.py index f0ece09a..9d1f5342 100644 --- a/negpy/desktop/view/canvas/toolbar.py +++ b/negpy/desktop/view/canvas/toolbar.py @@ -238,8 +238,8 @@ def _init_ui(self) -> None: self._ov_ring_action.setCheckable(True) self._ov_ring_action.setToolTip( tooltip_with_shortcut( - "Colour ring-around — print the frame as a 3x3 mosaic, the centre at the current " - "filtration and the ring 5cc either way on magenta and yellow", + "Colour ring-around — print the frame as a 5x5 mosaic, the centre at the current " + "filtration and the ring stepping 1cc at a time out to 2cc on magenta and yellow", "toggle_ring_around", ) ) @@ -407,7 +407,7 @@ def _on_zones_changed(self, active: bool) -> None: widget.blockSignals(False) def _sync_ring_action(self) -> None: - # Both proofs share one slot, so the kind gates this or the tone strip checks it too. + # One shared slot, so the kind gates this or the tone strip checks it too. state = self.controller.state active = (state.test_strip or state.test_strip_pending) and state.test_strip_kind == "colour" self._ov_ring_action.setChecked(active) diff --git a/negpy/desktop/view/sidebar/colour.py b/negpy/desktop/view/sidebar/colour.py index dd06205d..d2e6de0e 100644 --- a/negpy/desktop/view/sidebar/colour.py +++ b/negpy/desktop/view/sidebar/colour.py @@ -133,8 +133,7 @@ def _connect_signals(self) -> None: self.pick_wb_btn.toggled.connect(self._on_pick_wb_toggled) self.ring_btn.clicked.connect(lambda checked: self.controller.toggle_ring_around(force=checked)) - # The ring is session state, not config, and any render drops it — so the button has - # to follow the controller rather than sync_ui. + # Session state, not config, so the button follows the controller rather than sync_ui. self.controller.test_strip_changed.connect(self._sync_ring_btn) self.cast_removal_slider.valueChanged.connect( lambda v: self.update_config_section("exposure", render=True, persist=False, readback_metrics=False, cast_removal_strength=v) @@ -148,16 +147,15 @@ def _ring_tooltip(printing: bool = False) -> str: if printing: return "Printing the colour ring-around…" return tooltip_with_shortcut( - "Colour Ring-Around: print the frame as a 3×3 mosaic — the centre patch at the current " - "filtration, the ring ±5cc on the magenta and yellow axes. Click the patch that looks " - "neutral to keep its filtration. With Cast Removal on the patches separate less, since " - "it corrects toward neutral underneath.", + "Colour Ring-Around: print the frame as a 5×5 mosaic — the centre patch at the current " + "filtration, the ring stepping 1cc at a time out to ±2cc on the magenta and yellow axes. " + "Click the patch that looks neutral to keep its filtration. With Cast Removal on the " + "patches separate less, since it corrects toward neutral underneath.", "toggle_ring_around", ) def _sync_ring_btn(self, _up: bool) -> None: - # Both proofs share one slot, so the kind has to gate this or the tone strip lights - # this button too. + # One shared slot, so the kind gates this or the tone strip lights this button too. mine = self.state.test_strip_kind == "colour" pending = self.state.test_strip_pending and mine self.ring_btn.setChecked((self.state.test_strip or self.state.test_strip_pending) and mine) diff --git a/negpy/desktop/view/sidebar/tone.py b/negpy/desktop/view/sidebar/tone.py index 2722363b..0c181605 100644 --- a/negpy/desktop/view/sidebar/tone.py +++ b/negpy/desktop/view/sidebar/tone.py @@ -286,7 +286,7 @@ def _test_strip_tooltip(printing: bool = False) -> str: if printing: return "Printing the test strip…" return tooltip_with_shortcut( - "Test Strip: print the frame as a 6×6 grid — Print Density increasing left to right, " + "Test Strip: print the frame as a 5×5 grid — Print Density increasing left to right, " "ISO-R Grade softening top to bottom. Click the patch you like to keep its settings.", "toggle_test_strip", ) @@ -295,8 +295,7 @@ def _sync_test_strip_btn(self, _up: bool) -> None: """ponytail: the whole strip is one job with no progress reporting — the button is icon-only, so it goes dead with a 'printing' tooltip rather than changing its label. Per-patch progress if 36 renders ever feels long.""" - # Both proofs share one slot, so the kind has to gate this or the ring lights this - # button too. + # One shared slot, so the kind gates this or the ring lights this button too. mine = self.state.test_strip_kind == "tone" pending = self.state.test_strip_pending and mine self.test_strip_btn.setChecked((self.state.test_strip or self.state.test_strip_pending) and mine) diff --git a/negpy/desktop/workers/render.py b/negpy/desktop/workers/render.py index 3445f343..7b04c1ee 100644 --- a/negpy/desktop/workers/render.py +++ b/negpy/desktop/workers/render.py @@ -55,9 +55,9 @@ class RenderTask: @dataclass(frozen=True) class TestStripTask: - """Request to print a proof mosaic off one frame — the density × grade test strip, or the - colour ring-around. `overrides` carries one ExposureConfig field-override dict per patch, - row-major over `grid`.""" + """Request to print a proof mosaic off one frame: the density × grade strip or the colour + ring-around. `overrides` is one ExposureConfig field-override dict per patch, row-major + over `grid`.""" buffer: np.ndarray config: WorkspaceConfig @@ -270,12 +270,11 @@ def _soft_proof( def build_strip(self, task: TestStripTask) -> None: """Render the frame once per patch and keep only that patch. - Runs on the render thread with the canvas's own ImageProcessor, so the patches are - the same pixels the canvas would show. The fields any proof overrides — density and - grade for the tone strip, the colour head's magenta/yellow for the ring-around — are - all absent from the analysis cache key, so the per-frame metering is reused and only - the exposure stage onward re-dispatches. Metrics are dropped on the floor: a proof - must not disturb the histogram/bounds writeback the real render owns. + Runs on the render thread with the canvas's own ImageProcessor, so the patches are the + pixels the canvas would show. Every field a proof overrides (density/grade, or the + colour head's magenta/yellow) is absent from the analysis cache key, so the per-frame + metering is reused and only the exposure stage onward re-dispatches. Metrics are + dropped: a proof must not disturb the writeback the real render owns. """ try: tiles = [] diff --git a/negpy/features/exposure/analysis.py b/negpy/features/exposure/analysis.py index 96caef73..97c75178 100644 --- a/negpy/features/exposure/analysis.py +++ b/negpy/features/exposure/analysis.py @@ -128,26 +128,22 @@ def zone_region_labels(zones: np.ndarray) -> List[Tuple[int, int, int]]: return out -# Test strip ladder: a fixed absolute grid, not an offset around the current -# settings, so a strip printed off one frame is comparable to the next. Columns -# darken left to right, rows soften top to bottom — the two diagonals then read as -# the darkroom's light/dark and soft/hard axes. Named strip_* rather than -# test_strip_*: pytest collects any test_-prefixed callable a test module imports. -STRIP_DENSITIES = (0.4, 0.7, 1.0, 1.3, 1.6, 1.9) -STRIP_GRADES = (55.0, 80.0, 105.0, 130.0, 155.0, 180.0) +# Absolute ladders, centred on the defaults and inside the sliders' travel (density 0-2, +# grade R50-R180). Named strip_* not test_strip_*: pytest collects any test_-prefixed +# callable a test module imports. +STRIP_DENSITIES = (0.4, 0.7, 1.0, 1.3, 1.6) +STRIP_GRADES = (75.0, 95.0, 115.0, 135.0, 155.0) STRIP_GRID = (len(STRIP_GRADES), len(STRIP_DENSITIES)) # (rows, cols) -# Colour ring-around: ±1 CC step on the magenta and yellow axes around the filtration in -# force, so the centre patch is the print being judged. Relative rather than absolute like -# the strip's ladders — the question is which way the cast lies *from here*, which an -# absolute grid can't ask once a correction is already dialled in. 1.0 slider = 20cc -# (see filtration_offsets), so 0.25 is the classic RA4 5cc increment; it also keeps the -# centre unclamped anywhere in [-0.75, +0.75]. Calibration knob: retune by eye. -RING_CC_STEP = 0.25 +# Ring-around rungs: absolute filtration centred on neutral, like the strip's ladders, so a +# ring printed off one frame is comparable to the next and the mosaic is invariant to the +# filtration in force. 1.0 slider = 20cc (see filtration_offsets), so the step is 1cc and the +# outer rung 2cc. Calibration knobs. +RING_CC_STEP = 0.05 RING_CC_PER_UNIT = 20.0 -RING_GRID = (3, 3) +RING_GRID = (5, 5) def strip_cells() -> List[Tuple[int, int, float, float]]: @@ -160,33 +156,38 @@ def strip_overrides() -> List[dict]: return [{"density": d, "grade": g} for _, _, d, g in strip_cells()] -def ring_cells(magenta: float, yellow: float) -> List[Tuple[int, int, float, float]]: - """(row, col, wb_magenta, wb_yellow) row-major; cell (1, 1) is exactly the filtration - passed in. Rows step magenta, columns step yellow — cyan stays 0, as in a real - subtractive head. +def ring_rungs() -> Tuple[float, ...]: + """The absolute wb values one axis steps through, centred on neutral.""" + mid = RING_GRID[0] // 2 + return tuple(round((i - mid) * RING_CC_STEP, 6) for i in range(RING_GRID[0])) - Values are clipped to the slider domain, so at a rail two rows print the same patch. - That is the head running out of travel, not a bug. - """ + +def ring_cells() -> List[Tuple[int, int, float, float]]: + """(row, col, wb_magenta, wb_yellow) row-major. Rows step magenta, columns yellow, cyan + stays 0. Absolute, so the centre patch is neutral rather than whatever is dialled in.""" + rungs = ring_rungs() rows, cols = RING_GRID - out: List[Tuple[int, int, float, float]] = [] - for r in range(rows): - m = float(np.clip(magenta + (r - 1) * RING_CC_STEP, -1.0, 1.0)) if r != 1 else magenta - for c in range(cols): - y = float(np.clip(yellow + (c - 1) * RING_CC_STEP, -1.0, 1.0)) if c != 1 else yellow - out.append((r, c, m, y)) - return out + return [(r, c, rungs[r], rungs[c]) for r in range(rows) for c in range(cols)] + +def ring_overrides() -> List[dict]: + """Per-patch ExposureConfig overrides. Only the two colour-head fields, so a replace() + built from these cannot disturb density, grade or cyan.""" + return [{"wb_magenta": m, "wb_yellow": y} for _, _, m, y in ring_cells()] -def ring_overrides(magenta: float, yellow: float) -> List[dict]: - """ExposureConfig field overrides per patch — only the two colour-head fields, so a - replace() built from these can't disturb density, grade or cyan.""" - return [{"wb_magenta": m, "wb_yellow": y} for _, _, m, y in ring_cells(magenta, yellow)] +def ring_cc(index: int) -> float: + """A rung's filtration in cc, as the axis labels show it.""" + return ring_rungs()[index] * RING_CC_PER_UNIT -def ring_cc_offset(index: int) -> float: - """Signed CC offset of grid index 0/1/2 — the number the ring's axis labels show.""" - return (index - 1) * RING_CC_STEP * RING_CC_PER_UNIT + +def ring_nearest_cell(magenta: float, yellow: float) -> Tuple[int, int]: + """(row, col) of the patch closest to the filtration in force.""" + rungs = ring_rungs() + return ( + int(np.argmin([abs(m - magenta) for m in rungs])), + int(np.argmin([abs(y - yellow) for y in rungs])), + ) def _strip_bounds(extent: int, divisions: int, index: int) -> Tuple[int, int]: @@ -195,8 +196,7 @@ def _strip_bounds(extent: int, divisions: int, index: int) -> Tuple[int, int]: def strip_mosaic(tiles: List[np.ndarray], grid: Tuple[int, int]) -> np.ndarray: """One frame assembled from row-major renders over `grid`, each contributing only its own - patch. Bounds are rounded from the same fractions on both sides of a seam, so patches - tile exactly — no gap, no overlap.""" + patch. Both sides of a seam round the same fraction, so patches tile exactly.""" rows, cols = grid if len(tiles) != rows * cols: raise ValueError(f"expected {rows * cols} tiles, got {len(tiles)}") diff --git a/tests/test_ring_around.py b/tests/test_ring_around.py index e4c2b0db..fb143e40 100644 --- a/tests/test_ring_around.py +++ b/tests/test_ring_around.py @@ -1,8 +1,8 @@ -"""Colour ring-around ladder: ±5cc on the magenta and yellow axes around the filtration in -force, so the centre patch is the print being judged. +"""Colour ring-around ladder: absolute filtration, 1cc steps out to ±2cc either way of +neutral on the magenta and yellow axes. -Relative rather than absolute, which makes two properties load-bearing: the centre must be -*exactly* the input, and at a slider rail the ring legitimately prints duplicate patches. +Absolute like the strip's ladders, which is what makes the mosaic invariant to the filtration +in force: the memo can pin M/Y, so picking a patch doesn't force a reprint. """ import pytest @@ -11,77 +11,78 @@ RING_CC_PER_UNIT, RING_CC_STEP, RING_GRID, - ring_cc_offset, + ring_cc, ring_cells, + ring_nearest_cell, ring_overrides, + ring_rungs, strip_cell_at, ) from negpy.features.exposure.models import ExposureConfig +_ROWS, _COLS = RING_GRID +_MID = _ROWS // 2 + def _cell(cells, row, col): - return cells[row * RING_GRID[1] + col] + return cells[row * _COLS + col] -def test_the_centre_patch_is_exactly_the_filtration_passed_in(): - """Not merely close: the centre is the print being judged, so any rounding there would - make the ring compare against something the user isn't looking at.""" - cells = ring_cells(0.2, -0.1) - assert _cell(cells, 1, 1) == (1, 1, 0.2, -0.1) +def test_the_rungs_are_absolute_and_centred_on_neutral(): + """Absolute is what makes the mosaic invariant to the filtration in force, and so + cacheable across a pick.""" + rungs = ring_rungs() + assert len(rungs) == _ROWS + assert rungs[_MID] == 0.0 + assert rungs == pytest.approx([-2 * RING_CC_STEP, -RING_CC_STEP, 0.0, RING_CC_STEP, 2 * RING_CC_STEP]) + # Symmetric, so the ring never favours one direction. + assert rungs[0] == pytest.approx(-rungs[-1]) -def test_the_grid_is_nine_cells_row_major(): - cells = ring_cells(0.0, 0.0) - assert len(cells) == RING_GRID[0] * RING_GRID[1] == 9 - assert [(r, c) for r, c, _, _ in cells] == [(r, c) for r in range(3) for c in range(3)] +def test_the_grid_is_five_by_five_row_major(): + cells = ring_cells() + assert RING_GRID == (5, 5) + assert len(cells) == 25 + assert [(r, c) for r, c, _, _ in cells] == [(r, c) for r in range(_ROWS) for c in range(_COLS)] + assert _cell(cells, _MID, _MID)[2:] == (0.0, 0.0) # centre prints neutral def test_rows_step_magenta_and_columns_step_yellow(): - cells = ring_cells(0.0, 0.0) + cells = ring_cells() + rungs = list(ring_rungs()) # Within a row only yellow varies... - for row in range(3): - magentas = {_cell(cells, row, c)[2] for c in range(3)} - assert len(magentas) == 1 - yellows = [_cell(cells, row, c)[3] for c in range(3)] - assert yellows == [-RING_CC_STEP, 0.0, RING_CC_STEP] + for row in range(_ROWS): + assert len({_cell(cells, row, c)[2] for c in range(_COLS)}) == 1 + assert [_cell(cells, row, c)[3] for c in range(_COLS)] == pytest.approx(rungs) # ...and within a column only magenta. - for col in range(3): - yellows = {_cell(cells, r, col)[3] for r in range(3)} - assert len(yellows) == 1 - magentas = [_cell(cells, r, col)[2] for r in range(3)] - assert magentas == [-RING_CC_STEP, 0.0, RING_CC_STEP] + for col in range(_COLS): + assert len({_cell(cells, r, col)[3] for r in range(_ROWS)}) == 1 + assert [_cell(cells, r, col)[2] for r in range(_ROWS)] == pytest.approx(rungs) -def test_the_step_is_the_classic_five_cc(): - # filtration_offsets documents 1.0 slider = 20cc, so this pins the step to a real - # darkroom increment rather than an arbitrary slider fraction. - assert RING_CC_STEP * RING_CC_PER_UNIT == 5.0 - assert [ring_cc_offset(i) for i in range(3)] == [-5.0, 0.0, 5.0] +def test_the_rungs_run_one_cc_at_a_time_out_to_two(): + # filtration_offsets documents 1.0 slider = 20cc, which sets the cc scale. + assert RING_CC_STEP * RING_CC_PER_UNIT == 1.0 + assert [ring_cc(i) for i in range(_COLS)] == [-2.0, -1.0, 0.0, 1.0, 2.0] + assert [f"{ring_cc(i):+g}" for i in range(_COLS)] == ["-2", "-1", "+0", "+1", "+2"] -def test_the_centre_stays_unclamped_across_the_usable_range(): - for value in (-0.75, -0.4, 0.0, 0.4, 0.75): - cells = ring_cells(value, value) - assert _cell(cells, 1, 1) == (1, 1, value, value) - # Every rung is a distinct printable value away from the rails. - assert len({m for _, _, m, _ in cells}) == 3 - assert len({y for _, _, _, y in cells}) == 3 +def test_every_rung_is_inside_the_sliders_travel(): + """Outside [-1, 1] the value clamps and two rungs would print the same patch.""" + assert all(-1.0 <= m <= 1.0 for m in ring_rungs()) -def test_at_a_rail_the_ring_prints_duplicate_patches(): - """The head has run out of travel. That is real behaviour, not a bug — the strip forbids - duplicates, the ring documents when they legitimately happen.""" - cells = ring_cells(1.0, 0.0) - assert _cell(cells, 1, 1)[2] == 1.0 # centre still exact - # The plus row clamps onto the centre row's magenta, so those patches coincide. - assert _cell(cells, 2, 0)[2] == _cell(cells, 1, 0)[2] == 1.0 - assert _cell(cells, 0, 0)[2] == pytest.approx(1.0 - RING_CC_STEP) +def test_the_accent_follows_the_filtration_in_force(): + assert ring_nearest_cell(0.0, 0.0) == (_MID, _MID) + assert ring_nearest_cell(RING_CC_STEP, -RING_CC_STEP) == (_MID + 1, _MID - 1) + # Beyond the ladder's reach the nearest end wins rather than wrapping. + assert ring_nearest_cell(0.9, -0.9) == (_ROWS - 1, 0) def test_overrides_touch_only_the_two_colour_head_fields(): """A replace() built from these must not be able to disturb density, grade or cyan.""" - overrides = ring_overrides(0.1, -0.2) - assert len(overrides) == 9 + overrides = ring_overrides() + assert len(overrides) == _ROWS * _COLS for override in overrides: assert set(override) == {"wb_magenta", "wb_yellow"} # And the values really do reach an ExposureConfig unchanged. @@ -89,8 +90,8 @@ def test_overrides_touch_only_the_two_colour_head_fields(): assert cfg.wb_cyan == 0.0 and cfg.wb_magenta == overrides[0]["wb_magenta"] -def test_ring_clicks_map_across_the_three_by_three_grid(): +def test_ring_clicks_map_across_the_whole_grid(): assert strip_cell_at(0.0, 0.0, RING_GRID) == (0, 0) - assert strip_cell_at(0.5, 0.5, RING_GRID) == (1, 1) - assert strip_cell_at(0.99, 0.99, RING_GRID) == (2, 2) - assert strip_cell_at(1.4, -0.2, RING_GRID) == (0, 2) # clamped, not wrapped + assert strip_cell_at(0.5, 0.5, RING_GRID) == (_MID, _MID) + assert strip_cell_at(0.99, 0.99, RING_GRID) == (_ROWS - 1, _COLS - 1) + assert strip_cell_at(1.4, -0.2, RING_GRID) == (0, _COLS - 1) # clamped, not wrapped diff --git a/tests/test_ring_around_controller.py b/tests/test_ring_around_controller.py index 09de3c02..f36860ee 100644 --- a/tests/test_ring_around_controller.py +++ b/tests/test_ring_around_controller.py @@ -1,8 +1,8 @@ """Ring-around lifecycle: print → pick → commit, sharing one proof slot with the test strip. -The interesting differences from the strip are deliberate: the ring's ladder is relative, so -its memo must NOT be pinned on filtration (picking has to invalidate), and asking for the -other kind while a proof is up swaps rather than dismisses. +Its ladder is absolute, so the mosaic is invariant to the filtration in force and the memo +pins M/Y: picking a patch leaves the cache valid. Asking for the other kind swaps the proof +rather than dismissing it. """ import unittest @@ -25,7 +25,7 @@ def test_every_patch_is_rendered_at_its_own_filtration(self): def fake_pipeline(_buf, config, *a, **k): e = config.exposure seen.append((e.wb_magenta, e.wb_yellow, e.wb_cyan, e.density, e.grade)) - return np.full((9, 9, 3), float(len(seen) - 1), np.float32), {"content_rect": (0, 0, 9, 9)} + return np.full((10, 10, 3), float(len(seen) - 1), np.float32), {"content_rect": (0, 0, 10, 10)} MockIP.return_value.run_pipeline.side_effect = fake_pipeline worker = RenderWorker() @@ -36,16 +36,16 @@ def fake_pipeline(_buf, config, *a, **k): base = replace(base, exposure=replace(base.exposure, wb_magenta=0.2, wb_yellow=-0.1)) worker.build_strip( TestStripTask( - buffer=np.zeros((9, 9, 3), np.float32), + buffer=np.zeros((10, 10, 3), np.float32), config=base, source_hash="f1", preview_size=512.0, - overrides=tuple(ring_overrides(0.2, -0.1)), + overrides=tuple(ring_overrides()), grid=RING_GRID, ) ) - expected = [(m, y) for _, _, m, y in ring_cells(0.2, -0.1)] + expected = [(m, y) for _, _, m, y in ring_cells()] self.assertEqual([(m, y) for m, y, _, _, _ in seen], expected) # Nothing else moved: cyan, density and grade are the base config's on every patch. for _m, _y, cyan, density, grade in seen: @@ -54,9 +54,9 @@ def fake_pipeline(_buf, config, *a, **k): self.assertEqual(grade, base.exposure.grade) mosaic, _rect = done[0] - # Top-left patch from render 0, bottom-right from render 8 — the 3x3 slicing is right. + # Top-left patch from the first render, bottom-right from the last — slicing is right. self.assertEqual(mosaic[0, 0, 0], 0.0) - self.assertEqual(mosaic[-1, -1, 0], 8.0) + self.assertEqual(mosaic[-1, -1, 0], float(RING_GRID[0] * RING_GRID[1] - 1)) class RingAroundLifecycle(unittest.TestCase): @@ -106,26 +106,27 @@ def tearDown(self): gc.collect() def _mosaic(self) -> np.ndarray: - return np.zeros((9, 9, 3), dtype=np.float32) + return np.zeros((10, 10, 3), dtype=np.float32) def _print_ring(self) -> None: self.controller.toggle_ring_around() - self.controller.on_strip_finished(self._mosaic(), (0, 0, 9, 9)) + self.controller.on_strip_finished(self._mosaic(), (0, 0, 10, 10)) def _set_filtration(self, magenta: float, yellow: float) -> None: cfg = self.controller.state.config self.controller.state.config = replace(cfg, exposure=replace(cfg.exposure, wb_magenta=magenta, wb_yellow=yellow)) - def test_toggling_on_dispatches_a_nine_patch_job_around_the_current_filtration(self): + def test_toggling_on_dispatches_a_full_grid_job(self): self._set_filtration(0.2, -0.1) self.controller.toggle_ring_around() self.assertEqual(len(self.strip_tasks), 1) task = self.strip_tasks[0] self.assertEqual(task.grid, RING_GRID) - self.assertEqual(len(task.overrides), 9) + self.assertEqual(len(task.overrides), RING_GRID[0] * RING_GRID[1]) self.assertEqual(self.controller.state.test_strip_kind, "colour") - self.assertEqual(self.controller.state.test_strip_origin, (0.2, -0.1)) + # Absolute ladder: the rungs don't depend on what is currently dialled in. + self.assertEqual(tuple(task.overrides), tuple(ring_overrides())) def test_picking_a_patch_commits_only_its_filtration(self): self._set_filtration(0.2, -0.1) @@ -135,7 +136,7 @@ def test_picking_a_patch_commits_only_its_filtration(self): self.controller.apply_test_strip_pick(0, 0) after = self.controller.state.config.exposure - _, _, m, y = ring_cells(0.2, -0.1)[0] + _, _, m, y = ring_cells()[0] self.assertAlmostEqual(after.wb_magenta, m) self.assertAlmostEqual(after.wb_yellow, y) # Everything the patches were rendered under is left exactly as it was. @@ -147,23 +148,43 @@ def test_picking_a_patch_commits_only_its_filtration(self): self.assertEqual(after.auto_normalize_contrast, before.auto_normalize_contrast) self.assertFalse(self.controller.state.test_strip) - def test_picking_the_centre_keeps_the_filtration_and_still_clears(self): + def test_picking_the_centre_commits_neutral_filtration(self): self._set_filtration(0.2, -0.1) self._print_ring() - self.controller.apply_test_strip_pick(1, 1) + mid = RING_GRID[0] // 2 + self.controller.apply_test_strip_pick(mid, mid) after = self.controller.state.config.exposure - self.assertEqual((after.wb_magenta, after.wb_yellow), (0.2, -0.1)) + self.assertEqual((after.wb_magenta, after.wb_yellow), (0.0, 0.0)) self.assertFalse(self.controller.state.test_strip) - def test_picking_a_patch_invalidates_the_ring_cache(self): - """The inverse of the tone strip's cache-hit test, and deliberately so: the ring's - ladder is relative to the filtration in force, so once that moves the old mosaic is - no longer a proof of anything. A ring-around is meant to be iterated.""" + def test_reprinting_an_unchanged_ring_is_a_cache_hit(self): + """Same deal as the tone strip: 25 renders for pixels already in hand is worth + avoiding, so toggling the ring off and back on with no edit between must not reprint.""" + self._print_ring() + self.controller._clear_test_strip() + self.controller.toggle_ring_around() + + self.assertEqual(len(self.strip_tasks), 1) + self.assertTrue(self.controller.state.test_strip) + self.assertIsNotNone(self.controller.state.test_strip_mosaic) + + def test_any_other_edit_invalidates_the_ring_cache(self): + self._print_ring() + self.controller._clear_test_strip() + cfg = self.controller.state.config + self.controller.state.config = replace(cfg, exposure=replace(cfg.exposure, density=1.4)) + self.controller.toggle_ring_around() + self.assertEqual(len(self.strip_tasks), 2) + + def test_picking_a_patch_leaves_the_ring_cache_valid(self): + """What the absolute ladder buys: the mosaic varies M/Y itself, so it is invariant to + them and picking a patch changes nothing the patches depended on.""" self._print_ring() self.controller.apply_test_strip_pick(0, 0) self.controller.toggle_ring_around() - self.assertEqual(len(self.strip_tasks), 2) # a second real job, not a cache hit + self.assertEqual(len(self.strip_tasks), 1) # cache hit, no reprint + self.assertTrue(self.controller.state.test_strip) def test_the_two_proof_kinds_never_share_a_cache_entry(self): """RenderMemo holds one entry per file hash, so the kind is folded into the key — @@ -182,7 +203,7 @@ def test_asking_for_the_other_kind_swaps_the_proof(self): self.assertEqual(self.controller.state.test_strip_kind, "tone") self.assertEqual(len(self.strip_tasks), 2) - self.controller.on_strip_finished(self._mosaic(), (0, 0, 9, 9)) + self.controller.on_strip_finished(self._mosaic(), (0, 0, 10, 10)) self.controller.toggle_ring_around() self.assertEqual(self.controller.state.test_strip_kind, "colour") self.assertEqual(len(self.strip_tasks), 3) diff --git a/tests/test_test_strip.py b/tests/test_test_strip.py index 72d76904..57314c92 100644 --- a/tests/test_test_strip.py +++ b/tests/test_test_strip.py @@ -85,20 +85,21 @@ def test_click_on_the_far_edge_stays_in_the_grid(): def test_current_settings_highlight_the_nearest_patch(): assert strip_nearest_cell(STRIP_DENSITIES[2], STRIP_GRADES[1]) == (1, 2) - # The default density is a rung of its own; the default grade sits between two and the - # nearest wins. Either way the highlight lands where the sliders sit. - assert ExposureConfig().density in STRIP_DENSITIES - assert strip_nearest_cell(1.0, 115.0) == (2, STRIP_DENSITIES.index(1.0)) + # Both defaults are rungs of their own, so the default frame highlights the dead centre. + assert strip_nearest_cell(1.0, 115.0) == (2, 2) assert strip_nearest_cell(-5.0, 999.0) == (len(STRIP_GRADES) - 1, 0) -def test_both_ladders_straddle_the_defaults_in_even_steps(): +def test_both_ladders_are_centred_on_their_default_in_even_steps(): + """Centred, not merely straddling: the middle patch is the settings already in force, so + the strip reads as a comparison against the print you have rather than against nothing.""" defaults = ExposureConfig() for ladder, current in ((STRIP_DENSITIES, defaults.density), (STRIP_GRADES, defaults.grade)): - assert len(ladder) == 6 + assert len(ladder) == 5 steps = {round(b - a, 6) for a, b in zip(ladder, ladder[1:])} assert len(steps) == 1 and steps.pop() > 0 # evenly spaced, ascending assert ladder[0] < current < ladder[-1] + assert ladder[len(ladder) // 2] == current def test_ladders_stay_inside_the_ranges_their_controls_accept(): diff --git a/tests/test_test_strip_overlay.py b/tests/test_test_strip_overlay.py index 58064cfb..d4657d41 100644 --- a/tests/test_test_strip_overlay.py +++ b/tests/test_test_strip_overlay.py @@ -133,9 +133,9 @@ def _ring_overlay() -> CanvasOverlay: return overlay -def test_the_ring_picks_across_its_own_three_by_three_grid() -> None: - """The overlay reads the grid off the proof kind, so a click on the ring must map to 3x3 - rather than the tone strip's 6x6.""" +def test_the_ring_picks_across_its_own_grid() -> None: + """The overlay reads the grid off the proof kind, so a click on the ring must map to the + ring's geometry rather than the tone strip's.""" overlay = _ring_overlay() picked: list = [] overlay.test_strip_picked.connect(lambda r, c: picked.append((r, c))) @@ -144,14 +144,15 @@ def test_the_ring_picks_across_its_own_three_by_three_grid() -> None: overlay.mousePressEvent(_press(QPointF(45, 45))) overlay.mousePressEvent(_press(QPointF(85, 85))) - assert picked == [(0, 0), (1, 1), (RING_GRID[0] - 1, RING_GRID[1] - 1)] + mid = (RING_GRID[0] // 2, RING_GRID[1] // 2) + assert picked == [(0, 0), mid, (RING_GRID[0] - 1, RING_GRID[1] - 1)] -def test_the_ring_lays_out_nine_patches() -> None: +def test_each_proof_kind_lays_out_its_own_grid() -> None: overlay = _ring_overlay() - rects = overlay._strip_patch_rects(QRectF(0, 0, 90, 90)) - assert len(rects) == 9 assert overlay._strip_grid() == RING_GRID + assert len(overlay._strip_patch_rects(QRectF(0, 0, 90, 90))) == RING_GRID[0] * RING_GRID[1] # And the tone strip still gets its own grid off the same helper. overlay.state.test_strip_kind = "tone" assert overlay._strip_grid() == (len(STRIP_GRADES), len(STRIP_DENSITIES)) + assert len(overlay._strip_patch_rects(QRectF(0, 0, 90, 90))) == len(STRIP_GRADES) * len(STRIP_DENSITIES)