diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 53f52b5c..f178e239 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,7 @@ ## 0.42.0 +- Fix: **Tool clicks no longer offset by the print border** — heal spots, dodge & burn masks and white-balance picks land where clicked when a border or paper size pads the preview; the crop tool's preview is no longer padded either. - Change: **Presets UX pass** — presets now live in a visible list (tooltips show contents), and Save, Edit and Apply all use the per-setting picker from copy/paste: a preset stores exactly the settings you tick, and applying shows them again with a choice of current frame, selection or whole roll, laid over existing edits or replacing the look. Per-frame crop, rotation, metadata, dust and masks are never touched; existing preset files keep working. ## 0.41.0 diff --git a/negpy/desktop/controller.py b/negpy/desktop/controller.py index f2cb75a8..d2a72e27 100644 --- a/negpy/desktop/controller.py +++ b/negpy/desktop/controller.py @@ -412,12 +412,14 @@ def _compute_densitometer_reading(self, nx: float, ny: float, display_rgb: tuple norm_h, norm_w = nl.shape[:2] else: norm_w, norm_h = nl.width, nl.height + # nx,ny arrive content-normalized (the overlay subtracts the border) — + # passing content_rect here would compensate twice. pos = map_display_to_norm( nx, ny, disp[0], disp[1], - self.canvas.content_rect(), + None, metrics.get("active_roi"), self.state.active_tool in (ToolMode.CROP_MANUAL, ToolMode.ANALYSIS_DRAW), norm_w, diff --git a/negpy/desktop/view/canvas/overlay.py b/negpy/desktop/view/canvas/overlay.py index 6cdddc5f..033ff230 100644 --- a/negpy/desktop/view/canvas/overlay.py +++ b/negpy/desktop/view/canvas/overlay.py @@ -523,13 +523,14 @@ def _draw_brush(self, painter: QPainter) -> None: painter.drawEllipse(self._mouse_pos, radius, radius) def _brush_screen_radius(self, size: float) -> float: - max_screen_dim = max(self._view_rect.width(), self._view_rect.height()) + rect = self._content_view_rect() + max_screen_dim = max(rect.width(), rect.height()) return (size / (2.0 * HEAL_SIZE_REF)) * max_screen_dim def _preview_curve_path(self, pts: List[QPointF]) -> QPainterPath: """Smoothed path through the placed points plus the live cursor.""" scr = [(p.x(), p.y()) for p in pts] - if self._view_rect.contains(self._mouse_pos): + if self._content_view_rect().contains(self._mouse_pos): scr.append((self._mouse_pos.x(), self._mouse_pos.y())) if len(scr) >= 3: scr = smooth_polyline(scr, closed=False) @@ -663,7 +664,7 @@ def _draw_dust_overlay(self, painter: QPainter) -> None: if mode == "ir": img = self._ir_layer_qimage() if img is not None: - painter.drawImage(self._view_rect, img) + painter.drawImage(self._content_view_rect(), img) return # Dim wash over the auto-corrected regions (IR division + inpainted hairs); @@ -671,7 +672,7 @@ def _draw_dust_overlay(self, painter: QPainter) -> None: for mask in self._corrected_masks(): wash = self._mask_wash_qimage(mask) if wash is not None: - painter.drawImage(self._view_rect, wash) + painter.drawImage(self._content_view_rect(), wash) with self.state.metrics_lock: luma = self.state.last_metrics.get("detected_dust_luma") @@ -774,6 +775,17 @@ def _mask_wash_qimage(self, mask: np.ndarray) -> Optional[QImage]: self._wash_cache[id(mask)] = (key, img) return img + def _content_view_rect(self) -> QRectF: + """Screen rect of the image content inside the (possibly padded) view.""" + if self._content_rect is None or self._view_rect.isEmpty() or not self._current_size: + return self._view_rect + dw, dh = self._current_size + off_x, off_y, cw, ch = self._content_rect + if cw <= 0 or ch <= 0 or (off_x, off_y, cw, ch) == (0, 0, dw, dh): + return self._view_rect + sx, sy = self._view_rect.width() / dw, self._view_rect.height() / dh + return QRectF(self._view_rect.x() + off_x * sx, self._view_rect.y() + off_y * sy, cw * sx, ch * sy) + def _raw_to_screen(self, rx: float, ry: float, uv_grid: np.ndarray, buckets: int = 100) -> QPointF: """ Inverse UV-grid lookup: raw-normalised (0-1) -> screen position. @@ -804,10 +816,8 @@ def _raw_to_screen(self, rx: float, ry: float, uv_grid: np.ndarray, buckets: int nx = min((x0 + wx + 0.5) / w_uv, 1.0) ny = min((y0 + wy + 0.5) / h_uv, 1.0) - return QPointF( - self._view_rect.x() + nx * self._view_rect.width(), - self._view_rect.y() + ny * self._view_rect.height(), - ) + rect = self._content_view_rect() + return QPointF(rect.x() + nx * rect.width(), rect.y() + ny * rect.height()) def _norm_to_screen(self, nx: float, ny: float) -> QPointF: """Transformed-image normalized coords (0-1) -> screen position.""" @@ -1214,11 +1224,12 @@ def _draw_lasso_in_progress(self, painter: QPainter) -> None: painter.drawEllipse(first, r, r) def _map_to_image_coords(self, screen_pos: QPointF) -> Optional[Tuple[float, float]]: - if self._view_rect.isEmpty() or not self._view_rect.contains(screen_pos): + rect = self._content_view_rect() + if rect.isEmpty() or not rect.contains(screen_pos): return None - nb_x = (screen_pos.x() - self._view_rect.x()) / self._view_rect.width() - nb_y = (screen_pos.y() - self._view_rect.y()) / self._view_rect.height() + nb_x = (screen_pos.x() - rect.x()) / rect.width() + nb_y = (screen_pos.y() - rect.y()) / rect.height() return float(np.clip(nb_x, 0, 1)), float(np.clip(nb_y, 0, 1)) @@ -1253,7 +1264,7 @@ def mousePressEvent(self, event: QMouseEvent) -> None: return if self._tool_mode == ToolMode.SCRATCH_PICK: - if self._view_rect.contains(event.position()): + if self._content_view_rect().contains(event.position()): self._scratch_pts.append(event.position()) self.update() event.accept() @@ -1262,7 +1273,7 @@ def mousePressEvent(self, event: QMouseEvent) -> None: if self._tool_mode == ToolMode.DUST_PICK: # Heal commits on release: a plain click heals the spot, a drag paints a # continuous stroke healed as one region (one undo step, one render). - if self._view_rect.contains(event.position()): + if self._content_view_rect().contains(event.position()): self._heal_drag_pts = [event.position()] self.update() event.accept() @@ -1376,8 +1387,9 @@ def mouseMoveEvent(self, event: QMouseEvent) -> None: return if self._local_drag_vertex is not None and self._local_edit_verts is not None and not self._view_rect.isEmpty(): - px = float(np.clip(event.position().x(), self._view_rect.left(), self._view_rect.right())) - py = float(np.clip(event.position().y(), self._view_rect.top(), self._view_rect.bottom())) + rect = self._content_view_rect() + px = float(np.clip(event.position().x(), rect.left(), rect.right())) + py = float(np.clip(event.position().y(), rect.top(), rect.bottom())) self._local_edit_verts[self._local_drag_vertex] = QPointF(px, py) self.update() event.accept() @@ -1387,9 +1399,10 @@ def mouseMoveEvent(self, event: QMouseEvent) -> None: # radius so long drags stay a sane number of capsule segments), clamped to # the image so the stroke can't run off into the border. if self._tool_mode == ToolMode.DUST_PICK and self._heal_drag_pts and event.buttons() & Qt.MouseButton.LeftButton: + rect = self._content_view_rect() pos = QPointF( - float(np.clip(event.position().x(), self._view_rect.left(), self._view_rect.right())), - float(np.clip(event.position().y(), self._view_rect.top(), self._view_rect.bottom())), + float(np.clip(event.position().x(), rect.left(), rect.right())), + float(np.clip(event.position().y(), rect.top(), rect.bottom())), ) spacing = max(6.0, self._brush_screen_radius(self.state.config.retouch.manual_dust_size) * 0.5) if (pos - self._heal_drag_pts[-1]).manhattanLength() >= spacing: @@ -1717,9 +1730,10 @@ def mouseReleaseEvent(self, event: QMouseEvent) -> None: if self._tool_mode == ToolMode.DUST_PICK and self._heal_drag_pts and event.button() == Qt.MouseButton.LeftButton: pts = self._heal_drag_pts self._heal_drag_pts = [] + rect = self._content_view_rect() end = QPointF( - float(np.clip(event.position().x(), self._view_rect.left(), self._view_rect.right())), - float(np.clip(event.position().y(), self._view_rect.top(), self._view_rect.bottom())), + float(np.clip(event.position().x(), rect.left(), rect.right())), + float(np.clip(event.position().y(), rect.top(), rect.bottom())), ) if (end - pts[-1]).manhattanLength() > 2.0: pts.append(end) @@ -1739,11 +1753,12 @@ def mouseReleaseEvent(self, event: QMouseEvent) -> None: selected = getattr(self.state, "local_selected_mask", -1) self._end_local_edit() if verts and selected >= 0 and not self._view_rect.isEmpty(): - w, h = self._view_rect.width(), self._view_rect.height() + rect = self._content_view_rect() + w, h = rect.width(), rect.height() vp = [ ( - float(np.clip((p.x() - self._view_rect.x()) / w, 0.0, 1.0)), - float(np.clip((p.y() - self._view_rect.y()) / h, 0.0, 1.0)), + float(np.clip((p.x() - rect.x()) / w, 0.0, 1.0)), + float(np.clip((p.y() - rect.y()) / h, 0.0, 1.0)), ) for p in verts ] diff --git a/negpy/desktop/view/canvas/widget.py b/negpy/desktop/view/canvas/widget.py index 3d6a6d1d..af5456d2 100644 --- a/negpy/desktop/view/canvas/widget.py +++ b/negpy/desktop/view/canvas/widget.py @@ -322,16 +322,24 @@ def display_size(self) -> Optional[Tuple[int, int]]: return None def get_pixel_rgb(self, nx: float, ny: float) -> Optional[Tuple[float, float, float]]: - """Returns the displayed sRGB triplet in 0..1 at normalized image coords, or None.""" + """Returns the displayed sRGB triplet in 0..1 at content-normalized coords, or None.""" import numpy as np buf = self._last_buffer if buf is None: return None + rect = self.content_rect() + + def _xy(w: int, h: int) -> Tuple[int, int]: + if rect is not None: + off_x, off_y, cw, ch = rect + fx, fy = off_x + nx * cw, off_y + ny * ch + else: + fx, fy = nx * w, ny * h + return int(max(0, min(w - 1, fx))), int(max(0, min(h - 1, fy))) + if isinstance(buf, GPUTexture): - w, h = buf.width, buf.height - x = int(max(0, min(w - 1, nx * w))) - y = int(max(0, min(h - 1, ny * h))) + x, y = _xy(buf.width, buf.height) try: arr = buf.readback_region(x, y, 1, 1) except Exception: @@ -339,8 +347,7 @@ def get_pixel_rgb(self, nx: float, ny: float) -> Optional[Tuple[float, float, fl return (float(arr[0, 0, 0]), float(arr[0, 0, 1]), float(arr[0, 0, 2])) if isinstance(buf, np.ndarray): h, w = buf.shape[:2] - x = int(max(0, min(w - 1, nx * w))) - y = int(max(0, min(h - 1, ny * h))) + x, y = _xy(w, h) px = buf[y, x] scale = 1.0 / 255.0 if buf.dtype == np.uint8 else 1.0 px = np.atleast_1d(px) diff --git a/negpy/desktop/view/main_window.py b/negpy/desktop/view/main_window.py index e921ad47..8cb0af0b 100644 --- a/negpy/desktop/view/main_window.py +++ b/negpy/desktop/view/main_window.py @@ -466,7 +466,11 @@ def _on_image_updated(self) -> None: if isinstance(buffer, np.ndarray) and not self.state.gpu_enabled: finish_conf = self.state.config.finish export_conf = self.state.config.export - should_preview = finish_conf.border_size > 0 or export_conf.paper_aspect_ratio != AspectRatio.ORIGINAL + # Crop/analysis render the uncropped border-less frame — padding it would + # misalign the tool rect (GPU skips the layout pass there too). + should_preview = ( + finish_conf.border_size > 0 or export_conf.paper_aspect_ratio != AspectRatio.ORIGINAL + ) and self.state.active_tool not in (ToolMode.CROP_MANUAL, ToolMode.ANALYSIS_DRAW) if should_preview: pil_img = Image.fromarray(float_to_uint8(buffer)) diff --git a/tests/test_canvas_border_mapping.py b/tests/test_canvas_border_mapping.py new file mode 100644 index 00000000..0c6cff79 --- /dev/null +++ b/tests/test_canvas_border_mapping.py @@ -0,0 +1,165 @@ +"""Tool coordinates must be content-normalized when the preview is padded +with the print border/mat (finish.border_size / paper aspect layout).""" + +from types import SimpleNamespace + +import numpy as np +from PyQt6.QtCore import QEvent, QPointF, QRectF, Qt +from PyQt6.QtGui import QMouseEvent + +from negpy.desktop.session import AppState, ToolMode +from negpy.desktop.view.canvas.overlay import CanvasOverlay +from negpy.features.retouch.models import HEAL_SIZE_REF + +# Padded display buffer 200x160 shown 1:1; image content inset by the border. +_DISPLAY = (200, 160) +_CONTENT = (20, 16, 160, 128) + + +def _mouse_event(kind: QEvent.Type, pos: QPointF, buttons=Qt.MouseButton.LeftButton) -> QMouseEvent: + return QMouseEvent(kind, pos, Qt.MouseButton.LeftButton, buttons, Qt.KeyboardModifier.NoModifier) + + +def _identity_uv(h: int, w: int) -> np.ndarray: + u, v = np.meshgrid(np.linspace(0, 1, w, dtype=np.float32), np.linspace(0, 1, h, dtype=np.float32)) + return np.ascontiguousarray(np.stack([u, v], axis=-1)) + + +def _bordered_overlay(with_parent: bool = False) -> CanvasOverlay: + if with_parent: + from PyQt6.QtWidgets import QWidget + + parent = QWidget() + parent._is_panning = False + overlay = CanvasOverlay(AppState(), parent) + overlay._test_parent = parent + else: + overlay = CanvasOverlay(AppState()) + overlay._view_rect = QRectF(0, 0, *_DISPLAY) + overlay._current_size = _DISPLAY + overlay._content_rect = _CONTENT + return overlay + + +def test_heal_click_on_bordered_preview_emits_content_coords() -> None: + overlay = _bordered_overlay(with_parent=True) + overlay.set_tool_mode(ToolMode.DUST_PICK) + clicks: list = [] + overlay.clicked.connect(lambda x, y: clicks.append((x, y))) + + overlay.mousePressEvent(_mouse_event(QEvent.Type.MouseButtonPress, QPointF(60, 48))) + overlay.mouseReleaseEvent(_mouse_event(QEvent.Type.MouseButtonRelease, QPointF(60, 48), Qt.MouseButton.NoButton)) + + assert len(clicks) == 1 + assert abs(clicks[0][0] - 0.25) < 1e-6 and abs(clicks[0][1] - 0.25) < 1e-6 + + +def test_click_on_border_mat_is_ignored() -> None: + overlay = _bordered_overlay(with_parent=True) + overlay.set_tool_mode(ToolMode.DUST_PICK) + clicks: list = [] + strokes: list = [] + overlay.clicked.connect(lambda x, y: clicks.append((x, y))) + overlay.scratch_completed.connect(strokes.append) + + overlay.mousePressEvent(_mouse_event(QEvent.Type.MouseButtonPress, QPointF(10, 8))) + overlay.mouseReleaseEvent(_mouse_event(QEvent.Type.MouseButtonRelease, QPointF(10, 8), Qt.MouseButton.NoButton)) + + assert clicks == [] + assert strokes == [] + + +def test_raw_to_screen_lands_inside_content() -> None: + overlay = _bordered_overlay() + uv = _identity_uv(64, 80) # one uv cell = 2px on screen + + pt = overlay._raw_to_screen(0.25, 0.25, uv) + + assert abs(pt.x() - 60.0) <= 2.5 + assert abs(pt.y() - 48.0) <= 2.5 + + +def test_forward_reverse_roundtrip_with_border() -> None: + # Consistency guard: marker must draw back at the click point. + overlay = _bordered_overlay(with_parent=True) + overlay.set_tool_mode(ToolMode.DUST_PICK) + clicks: list = [] + overlay.clicked.connect(lambda x, y: clicks.append((x, y))) + + start = QPointF(108, 100) + overlay.mousePressEvent(_mouse_event(QEvent.Type.MouseButtonPress, start)) + overlay.mouseReleaseEvent(_mouse_event(QEvent.Type.MouseButtonRelease, start, Qt.MouseButton.NoButton)) + + assert len(clicks) == 1 + back = overlay._raw_to_screen(clicks[0][0], clicks[0][1], _identity_uv(64, 80)) + assert abs(back.x() - start.x()) <= 2.5 + assert abs(back.y() - start.y()) <= 2.5 + + +def test_lasso_vertices_content_normalized() -> None: + overlay = _bordered_overlay() + overlay.set_tool_mode(ToolMode.LOCAL_DRAW) + overlay._lasso_drawing = True + overlay._lasso_pts = [QPointF(20, 16), QPointF(180, 16), QPointF(100, 144)] + emitted: list = [] + overlay.lasso_completed.connect(emitted.append) + + overlay._finish_lasso() + + assert len(emitted) == 1 + expected = [(0.0, 0.0), (1.0, 0.0), (0.5, 1.0)] + for (nx, ny), (ex, ey) in zip(emitted[0], expected): + assert abs(nx - ex) < 1e-6 and abs(ny - ey) < 1e-6 + + +def test_mask_vertex_release_content_normalized() -> None: + overlay = _bordered_overlay(with_parent=True) + overlay.state.local_selected_mask = 0 + overlay._local_drag_vertex = 0 + overlay._local_edit_verts = [QPointF(60, 48), QPointF(100, 48), QPointF(80, 80)] + emitted: list = [] + overlay.local_mask_edited.connect(lambda i, vp: emitted.append((i, vp))) + + overlay.mouseReleaseEvent(_mouse_event(QEvent.Type.MouseButtonRelease, QPointF(60, 48), Qt.MouseButton.NoButton)) + + assert len(emitted) == 1 + index, vp = emitted[0] + assert index == 0 + assert abs(vp[0][0] - 0.25) < 1e-6 and abs(vp[0][1] - 0.25) < 1e-6 + + +def test_brush_radius_scales_with_content() -> None: + overlay = _bordered_overlay() + assert abs(overlay._brush_screen_radius(12.0) - 12.0 / (2.0 * HEAL_SIZE_REF) * 160.0) < 1e-6 + + +def test_densitometer_not_double_compensated() -> None: + # Overlay emits content-normalized coords; the controller must not subtract + # content_rect a second time. + from negpy.desktop.controller import AppController + + nl = np.zeros((80, 100, 3), dtype=np.float32) + nl[20, 25] = 0.7 + bounds = SimpleNamespace(floors=(0.0, 0.0, 0.0), ceils=(1.0, 1.0, 1.0)) + stub = SimpleNamespace( + state=SimpleNamespace( + last_metrics={"normalized_log": nl, "final_bounds": bounds}, + active_tool=ToolMode.NONE, + ), + canvas=SimpleNamespace( + display_size=lambda: _DISPLAY, + content_rect=lambda: _CONTENT, + ), + ) + + reading = AppController._compute_densitometer_reading(stub, 0.25, 0.25, (0.5, 0.5, 0.5)) + + assert reading is not None + assert all(abs(d - 0.7) < 1e-6 for d in reading.dd_rgb) + + +def test_no_border_fallback_unchanged() -> None: + overlay = _bordered_overlay() + overlay._content_rect = None + + assert overlay._map_to_image_coords(QPointF(50, 40)) == (0.25, 0.25)