From 3d55e094e5ff7b00002028ee0ad349a3bd7b5b5b Mon Sep 17 00:00:00 2001 From: Li <175231748+linkmodo@users.noreply.github.com> Date: Tue, 7 Jul 2026 09:31:54 -0400 Subject: [PATCH] Add UX refinements, fix crop under fine rotation, add freehand analysis region UI/UX: - Files sidebar: single click on a thumbnail now selects and loads it into the viewer (Ctrl/Shift click still builds a multi-selection for batch actions). - Crop tool: double-click inside the crop box confirms the crop and closes the tool, so you no longer have to return to the Crop button. - Right panel: collapsing the sticky "Analysis" section now pins its header at the top and hands the freed splitter space to the tabs below. - Rename the geometry slider "Fine Rot" -> "Fine Rotation". Fix (crop inflates under Fine Rotation): - GeometryConfig.manual_crop_rect is now normalized in the transformed (display) image instead of raw space. get_manual_rect_coords is a plain axis-aligned slice (rect x transformed dims); previously it corner-mapped the rect through the fine rotation and took the axis-aligned bounding box, which inflated the cropped region as the tilt grew. The overlay maps screen<->rect linearly through its view rect (no uv-grid round-trip for crop). Note: manual crops saved in an older edits.db re-interpret once. Feature (freehand analysis region, additive): - New optional ProcessConfig.analysis_rect (transformed-normalized, like the crop rect). resolve_analysis_region turns it into the analysis ROI (buffer 0) at the single chokepoint in NormalizationProcessor and the GPU analysis path; when set it overrides the centered Analysis Buffer, otherwise the slider still applies. - New ANALYSIS_DRAW canvas tool (draw + move + double-click confirm) plus draw/clear buttons on the Process page; the tool shows the uncropped preview. Tests: geometry crop semantics + fine-rotation regression, analysis-region resolution + pipeline wiring, and analysis_rect serialization round-trip. make lint / type / test all pass (the one failing export-path test is a pre-existing Windows path-separator issue, unrelated to this change). Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 4 + negpy/desktop/controller.py | 47 ++- negpy/desktop/session.py | 1 + negpy/desktop/view/canvas/overlay.py | 308 +++++++++++++------ negpy/desktop/view/canvas/widget.py | 7 + negpy/desktop/view/main_window.py | 3 + negpy/desktop/view/sidebar/controls_panel.py | 2 + negpy/desktop/view/sidebar/files.py | 18 +- negpy/desktop/view/sidebar/geometry.py | 2 +- negpy/desktop/view/sidebar/process.py | 32 +- negpy/desktop/view/sidebar/right_panel.py | 21 ++ negpy/features/exposure/normalization.py | 27 ++ negpy/features/exposure/processor.py | 15 +- negpy/features/geometry/logic.py | 37 +-- negpy/features/geometry/processor.py | 7 - negpy/features/process/models.py | 7 + negpy/services/rendering/engine.py | 1 + negpy/services/rendering/gpu_engine.py | 27 +- tests/test_analysis_region.py | 50 +++ tests/test_config_deserialization.py | 12 + tests/test_geometry_logic.py | 69 ++--- 21 files changed, 499 insertions(+), 198 deletions(-) create mode 100644 tests/test_analysis_region.py diff --git a/CLAUDE.md b/CLAUDE.md index cc5aeb57..12900d45 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -43,6 +43,10 @@ Every feature under `negpy/features//` follows this structure: `DarkroomEngine.process()` runs stages in order: base (geometry + normalization) → exposure → retouch → lab → toning → crop → finish. The cached stages (base, exposure, retouch, lab) go through `_run_stage()`, which hashes the stage config and skips re-execution if the hash matches the cached `CacheEntry`; toning/crop/finish run unconditionally. Source image change clears the whole cache; a process-mode change invalidates only base/exposure/retouch/lab. Note `settings.geometry` drives both the early geometry transform and the late crop stage. +**Manual crop rect is in transformed (display) space.** `GeometryConfig.manual_crop_rect` is normalized to the **already-transformed** image (post rotation/fine-rotation/flip/distortion) — the space the user draws it on in the canvas overlay — so `get_manual_rect_coords` is a plain axis-aligned slice (`rect × transformed dims`), **not** a corner-map through `map_coords_to_geometry` + AABB (that inflated the crop as fine rotation tilted the mapped rect). The overlay maps screen↔rect linearly through its view rect (no uv_grid). Autocrop/retouch/dodge-burn placements still round-trip through `map_coords_to_geometry`/uv_grid; only the manual crop is display-space. + +**Freehand analysis region.** `ProcessConfig.analysis_rect` (optional, transformed-normalized like the crop rect) overrides the centered `analysis_buffer` inset for the exposure meters: `resolve_analysis_region` (`exposure/normalization.py`) turns it into the analysis ROI with buffer 0, applied at the single chokepoint in `NormalizationProcessor` (CPU) and the GPU analysis path. It keys the CPU `base_key` and rides in `settings.process` for the GPU `_analysis_cache_key`. UI: a "draw region" tool (`ToolMode.ANALYSIS_DRAW`) on the Process page; the tool shows the uncropped preview (`crop_preview_full`) like the crop tool. + **Dodge/burn lives in the exposure stage.** Polygon masks (`negpy/features/local/`, `LocalAdjustmentsConfig`) rasterise to a per-pixel **EV map** (`compute_local_ev_map`, Σ strength·alpha) consumed by the print curve as a print-exposure offset next to the WB `cmy_offsets` (`local_ev_scale` = −log10(2)/stretch range per channel) — burns roll into paper black through the toe, dodges lift through the shoulder, like enlarger exposure. There is no separate local stage or shader: the exposure `_run_stage` is keyed on `(settings.exposure, settings.local)`, the GPU binds the EV map as a texture in the exposure pass (`ev_scale.w` gates a 1×1 dummy when no masks), and tiled export slices the full-res EV map per tile. The UI section sits on the Exposure ("tone") page. **Flat-field profile (rig-level) + lens distortion.** Flat-field is a per-rig **profile** (DB `flatfield_profiles`: name, path, **k1**), not a per-image edit. `FlatFieldConfig` (`apply`, `reference_path`, `k1`) mirrors the active profile into the recipe but the profile is canonical — `reference_path` and `k1` are **re-seeded from the active profile on every file load** (`session.py`), so a roll shares them. The *photometric* correction (illumination falloff) is baked at **source-load** (`apply_flatfield`, before the pipeline → before any warp, so the flat stays registered to the raw sensor grid). The *geometric* correction is a single radial coefficient **k1** (poly3 / Brown-Conrady, half-diagonal–normalized so it's rotation/aspect invariant) folded into the **geometry transform** (`apply_radial_distortion` / `transform.wgsl`), gated by the same `apply` toggle (`k1_eff = k1 if apply else 0`). Being a geometry op, k1 invalidates the **base stage** (CPU `base_key`; GPU `_detect_invalidated_stage` checks `flatfield.apply`/`k1`) — **not** `source_hash`/`flatfield_token` (that would force a RAW re-decode per slider step). Scale-to-fill is baked into the same remap (`compute_distortion_scale`, no empty borders). The correction must stay consistent across the image warp **and** the coordinate mappers (`create_uv_grid` forward, `map_coords_to_geometry` inverse via `map_point_radial`) so crop/autocrop/retouch/dodge-burn placements stay aligned; CPU stages read the effective k1 from `context.metrics["distortion_k1"]`, GPU passes `k1_eff` directly. Tiled export applies it on the CPU `img_rot`/IR path (the shader geometry uniform is zeroed there). diff --git a/negpy/desktop/controller.py b/negpy/desktop/controller.py index 8906596a..004d4441 100644 --- a/negpy/desktop/controller.py +++ b/negpy/desktop/controller.py @@ -606,7 +606,10 @@ def handle_canvas_clicked(self, nx: float, ny: float) -> None: self._handle_dust_pick(nx, ny) def set_active_tool(self, mode: ToolMode) -> None: - crop_tool_changed = ToolMode.CROP_MANUAL in (self.state.active_tool, mode) + # Both the crop and analysis-region tools show the full uncropped frame, so + # crossing into/out of that set must re-render to swap the preview. + uncropped = {ToolMode.CROP_MANUAL, ToolMode.ANALYSIS_DRAW} + preview_mode_changed = (self.state.active_tool in uncropped) != (mode in uncropped) leaving_crop = self.state.active_tool == ToolMode.CROP_MANUAL and mode != ToolMode.CROP_MANUAL self.state.active_tool = mode self.tool_sync_requested.emit() @@ -615,9 +618,7 @@ def set_active_tool(self, mode: ToolMode) -> None: new_proc = replace(self.state.config.process, **invalidate_local_bounds(self.state.config.process)) self.session.update_config(replace(self.state.config, process=new_proc), render=False) self._crop_bounds_dirty = False - if crop_tool_changed: - # Entering/leaving the crop tool swaps between the full uncropped preview - # and the normal cropped preview, so the canvas must re-render immediately. + if preview_mode_changed: self.request_render() def cancel_active_tool(self) -> None: @@ -652,6 +653,42 @@ def handle_crop_rect_changed(self, nx1: float, ny1: float, nx2: float, ny2: floa else: self._render_debounce.start() + def confirm_manual_crop(self) -> None: + """Close the crop tool (committing the current rect) — invoked by a double-click + inside the crop box so the user needn't return to the Crop button.""" + if self.state.active_tool == ToolMode.CROP_MANUAL: + self.set_active_tool(ToolMode.NONE) + + def handle_analysis_rect_changed(self, nx1: float, ny1: float, nx2: float, ny2: float, persist: bool) -> None: + """Live-update (persist=False) or commit (persist=True) the freehand analysis + region while the tool is open. Setting a region re-meters the frame, so a commit + clears the per-file bounds (unless bounds are locked) and re-renders.""" + if self.state.active_tool != ToolMode.ANALYSIS_DRAW: + return + rect = (min(nx1, nx2), min(ny1, ny2), max(nx1, nx2), max(ny1, ny2)) + proc = replace(self.state.config.process, analysis_rect=rect) + if persist: + proc = replace(proc, **invalidate_local_bounds(proc)) + self.session.update_config(replace(self.state.config, process=proc), persist=persist) + if persist: + self.request_render() + else: + self._render_debounce.start() + + def clear_analysis_region(self) -> None: + """Drop the freehand analysis region; metering falls back to the Analysis Buffer slider.""" + if self.state.config.process.analysis_rect is None: + return + proc = replace(self.state.config.process, analysis_rect=None) + proc = replace(proc, **invalidate_local_bounds(proc)) + self.session.update_config(replace(self.state.config, process=proc), persist=True) + self.request_render() + + def confirm_analysis_region(self) -> None: + """Close the analysis-region tool (double-click inside the region).""" + if self.state.active_tool == ToolMode.ANALYSIS_DRAW: + self.set_active_tool(ToolMode.NONE) + def reset_crop(self) -> None: self._crop_bounds_dirty = False new_proc = replace(self.state.config.process, **invalidate_local_bounds(self.state.config.process)) @@ -1271,7 +1308,7 @@ def request_render( readback_metrics=readback_metrics, ir_buffer=self.state.preview_ir, monitor_icc_bytes=self.state.monitor_icc_bytes, - crop_preview_full=self.state.active_tool == ToolMode.CROP_MANUAL, + crop_preview_full=self.state.active_tool in (ToolMode.CROP_MANUAL, ToolMode.ANALYSIS_DRAW), ephemeral=ephemeral, ) diff --git a/negpy/desktop/session.py b/negpy/desktop/session.py index b5398629..40fee6fe 100644 --- a/negpy/desktop/session.py +++ b/negpy/desktop/session.py @@ -21,6 +21,7 @@ class ToolMode(Enum): DUST_PICK = auto() SCRATCH_PICK = auto() LOCAL_DRAW = auto() + ANALYSIS_DRAW = auto() @dataclass diff --git a/negpy/desktop/view/canvas/overlay.py b/negpy/desktop/view/canvas/overlay.py index e2a15577..48bd4729 100644 --- a/negpy/desktop/view/canvas/overlay.py +++ b/negpy/desktop/view/canvas/overlay.py @@ -12,7 +12,6 @@ from negpy.features.geometry.logic import translate_manual_crop_rect from negpy.features.local.logic import _rasterise_mask from negpy.kernel.system.config import APP_CONFIG -from negpy.services.view.coordinate_mapping import CoordinateMapping _LASSO_SNAP_PX = 12.0 _CROP_HANDLE_PX = 10.0 @@ -51,6 +50,9 @@ class CanvasOverlay(QWidget): clicked = pyqtSignal(float, float) crop_rect_changed = pyqtSignal(float, float, float, float, bool) + crop_confirmed = pyqtSignal() + analysis_rect_changed = pyqtSignal(float, float, float, float, bool) + analysis_confirmed = pyqtSignal() cursor_moved = pyqtSignal(float, float) cursor_left = pyqtSignal() lasso_completed = pyqtSignal(list) @@ -66,14 +68,22 @@ def __init__(self, state: AppState, parent=None): # Crop tool interaction state: corner-resize, interior move, or fresh draw # (when the click lands outside the existing rect). - self._crop_rect_raw: Optional[Tuple[float, float, float, float]] = None + self._crop_rect_norm: Optional[Tuple[float, float, float, float]] = None self._crop_drag_mode: Optional[str] = None # "corner" | "move" | "draw" self._crop_anchor_screen: Optional[QPointF] = None - self._crop_press_raw: Optional[Tuple[float, float]] = None + self._crop_press_norm: Optional[Tuple[float, float]] = None self._crop_orig_rect: Optional[Tuple[float, float, float, float]] = None - self._crop_uv_grid: Optional[np.ndarray] = None self._crop_draw_p1: Optional[QPointF] = None self._crop_draw_p2: Optional[QPointF] = None + + # Freehand analysis-region interaction (transformed-normalized, like the crop rect). + self._analysis_rect_norm: Optional[Tuple[float, float, float, float]] = None + self._analysis_drag_mode: Optional[str] = None # "move" | "draw" + self._analysis_press_norm: Optional[Tuple[float, float]] = None + self._analysis_orig_rect: Optional[Tuple[float, float, float, float]] = None + self._analysis_draw_p1: Optional[QPointF] = None + self._analysis_draw_p2: Optional[QPointF] = None + self._tool_mode: ToolMode = ToolMode.NONE self._mouse_pos: QPointF = QPointF() @@ -152,10 +162,15 @@ def _hide_rotation_grid(self) -> None: def set_tool_mode(self, mode: ToolMode) -> None: self._tool_mode = mode if mode == ToolMode.CROP_MANUAL: - self._crop_rect_raw = self.state.config.geometry.manual_crop_rect + self._crop_rect_norm = self.state.config.geometry.manual_crop_rect else: - self._crop_rect_raw = None + self._crop_rect_norm = None self._end_crop_drag() + if mode == ToolMode.ANALYSIS_DRAW: + self._analysis_rect_norm = self.state.config.process.analysis_rect + else: + self._analysis_rect_norm = None + self._end_analysis_drag() if mode != ToolMode.LOCAL_DRAW: self._lasso_pts = [] self._lasso_drawing = False @@ -166,12 +181,18 @@ def set_tool_mode(self, mode: ToolMode) -> None: def _end_crop_drag(self) -> None: self._crop_drag_mode = None self._crop_anchor_screen = None - self._crop_press_raw = None + self._crop_press_norm = None self._crop_orig_rect = None - self._crop_uv_grid = None self._crop_draw_p1 = None self._crop_draw_p2 = None + def _end_analysis_drag(self) -> None: + self._analysis_drag_mode = None + self._analysis_press_norm = None + self._analysis_orig_rect = None + self._analysis_draw_p1 = None + self._analysis_draw_p2 = None + def _cancel_lasso(self) -> None: if self._tool_mode == ToolMode.LOCAL_DRAW and self._lasso_drawing: self._lasso_pts = [] @@ -198,7 +219,9 @@ def update_buffer( self._current_size = gpu_size if self._tool_mode == ToolMode.CROP_MANUAL and self._crop_drag_mode is None: - self._crop_rect_raw = self.state.config.geometry.manual_crop_rect + self._crop_rect_norm = self.state.config.geometry.manual_crop_rect + if self._tool_mode == ToolMode.ANALYSIS_DRAW and self._analysis_drag_mode is None: + self._analysis_rect_norm = self.state.config.process.analysis_rect self._recalc_view_rect() self.update() @@ -267,7 +290,14 @@ def _draw_ui(self, painter: QPainter) -> None: if self._tool_mode == ToolMode.CROP_MANUAL: self._draw_crop_tool(painter) - if self._buffer_overlay_visible and self._buffer_overlay_ratio > 1e-4 and self._tool_mode != ToolMode.CROP_MANUAL: + if self._tool_mode == ToolMode.ANALYSIS_DRAW: + self._draw_analysis_tool(painter) + + if ( + self._buffer_overlay_visible + and self._buffer_overlay_ratio > 1e-4 + and self._tool_mode not in (ToolMode.CROP_MANUAL, ToolMode.ANALYSIS_DRAW) + ): d = visible_rect margin_w = d.width() * self._buffer_overlay_ratio margin_h = d.height() * self._buffer_overlay_ratio @@ -446,40 +476,37 @@ def _raw_to_screen(self, rx: float, ry: float, uv_grid: np.ndarray, buckets: int self._view_rect.y() + ny * self._view_rect.height(), ) + def _norm_to_screen(self, nx: float, ny: float) -> QPointF: + """Transformed-image normalized coords (0-1) -> screen position.""" + return QPointF( + self._view_rect.x() + nx * self._view_rect.width(), + self._view_rect.y() + ny * self._view_rect.height(), + ) + + def _screen_to_norm(self, screen_pos: QPointF) -> Tuple[float, float]: + """Screen position -> transformed-image normalized coords, clamped to 0-1.""" + if self._view_rect.isEmpty(): + return 0.0, 0.0 + nx = (screen_pos.x() - self._view_rect.x()) / self._view_rect.width() + ny = (screen_pos.y() - self._view_rect.y()) / self._view_rect.height() + return float(np.clip(nx, 0.0, 1.0)), float(np.clip(ny, 0.0, 1.0)) + def _crop_corner_screen_points(self) -> Optional[Dict[str, QPointF]]: - """Maps the current raw crop rect to its on-screen axis-aligned bounding box. - - The actual crop (`get_manual_rect_coords` / `CropProcessor`) always takes the - axis-aligned bounding box of the rect's transformed corners - it's a plain - array slice, never a rotated one. So under fine rotation the 4 raw corners - land at a tilted quadrilateral, but we deliberately collapse that to its AABB - here rather than drawing the tilt, so the overlay shows what will actually be - cropped instead of a shape the pipeline can't produce. + """Screen positions of the crop rect's four corners. + + The rect is stored in the transformed (display) image's normalized coords — the + same space it is drawn on — so it maps linearly through the view rect and stays a + true axis-aligned rectangle. The box shown is exactly the box `CropProcessor` + slices (no fine-rotation bounding-box inflation). """ - if self._crop_rect_raw is None: + if self._crop_rect_norm is None or self._view_rect.isEmpty(): return None - with self.state.metrics_lock: - uv_grid = self.state.last_metrics.get("uv_grid") - if uv_grid is None: - return None - x1, y1, x2, y2 = self._crop_rect_raw - h_uv, w_uv = uv_grid.shape[:2] - buckets = max(h_uv, w_uv) - pts = [ - self._raw_to_screen(x1, y1, uv_grid, buckets), - self._raw_to_screen(x2, y1, uv_grid, buckets), - self._raw_to_screen(x2, y2, uv_grid, buckets), - self._raw_to_screen(x1, y2, uv_grid, buckets), - ] - left = min(p.x() for p in pts) - right = max(p.x() for p in pts) - top = min(p.y() for p in pts) - bottom = max(p.y() for p in pts) + x1, y1, x2, y2 = self._crop_rect_norm return { - "tl": QPointF(left, top), - "tr": QPointF(right, top), - "br": QPointF(right, bottom), - "bl": QPointF(left, bottom), + "tl": self._norm_to_screen(x1, y1), + "tr": self._norm_to_screen(x2, y1), + "br": self._norm_to_screen(x2, y2), + "bl": self._norm_to_screen(x1, y2), } def _hit_test_crop_corner(self, pos: QPointF, corners: Dict[str, QPointF]) -> Optional[str]: @@ -489,13 +516,13 @@ def _hit_test_crop_corner(self, pos: QPointF, corners: Dict[str, QPointF]) -> Op return name return None - def _apply_aspect_and_min(self, anchor_screen: QPointF, cur_screen: QPointF, uv_grid: np.ndarray) -> Tuple[float, float, float, float]: + def _apply_aspect_and_min(self, anchor_screen: QPointF, cur_screen: QPointF) -> Tuple[float, float, float, float]: """Resizes a rect anchored at `anchor_screen` towards `cur_screen`, honoring the configured aspect ratio (if any) and a minimum rect size. - Done entirely in screen-pixel space: raw-normalised (0-1) fractions only equal - physical aspect ratio when the source image is square, so applying a target - ratio to raw-space deltas distorts it by the image's actual width/height ratio. + Done entirely in screen-pixel space: normalised (0-1) fractions only equal + physical aspect ratio when the displayed image is square, so applying a target + ratio to normalised deltas distorts it by the image's actual width/height ratio. Screen pixels reflect the image as displayed, so ratios computed there are correct. """ ax, ay = anchor_screen.x(), anchor_screen.y() @@ -531,13 +558,11 @@ def _apply_aspect_and_min(self, anchor_screen: QPointF, cur_screen: QPointF, uv_ dy = _CROP_MIN_SCREEN_PX if dy >= 0 else -_CROP_MIN_SCREEN_PX end_screen = QPointF(ax + dx, ay + dy) - c1 = self._raw_from_screen_with_grid(anchor_screen, uv_grid) - c2 = self._raw_from_screen_with_grid(end_screen, uv_grid) - if c1 is None or c2 is None: - return self._crop_rect_raw or (0.0, 0.0, 1.0, 1.0) + c1 = self._screen_to_norm(anchor_screen) + c2 = self._screen_to_norm(end_screen) x1, x2 = sorted((c1[0], c2[0])) y1, y2 = sorted((c1[1], c2[1])) - return (max(0.0, x1), max(0.0, y1), min(1.0, x2), min(1.0, y2)) + return (x1, y1, x2, y2) def _draw_crop_tool(self, painter: QPainter) -> None: if self._crop_drag_mode == "draw" and self._crop_draw_p1 is not None: @@ -579,6 +604,33 @@ def _draw_crop_tool(self, painter: QPainter) -> None: for pt in corners.values(): painter.drawRect(QRectF(pt.x() - 5, pt.y() - 5, 10, 10)) + def _analysis_rect_screen(self) -> Optional[QRectF]: + """Screen rect for the current analysis region, or None if unset.""" + if self._analysis_drag_mode == "draw" and self._analysis_draw_p1 is not None: + return QRectF(self._analysis_draw_p1, self._analysis_draw_p2 or self._analysis_draw_p1).normalized() + if self._analysis_rect_norm is None or self._view_rect.isEmpty(): + return None + x1, y1, x2, y2 = self._analysis_rect_norm + return QRectF(self._norm_to_screen(x1, y1), self._norm_to_screen(x2, y2)).normalized() + + def _draw_analysis_tool(self, painter: QPainter) -> None: + """Green-dashed analysis region: the exact area the exposure meters read.""" + rect = self._analysis_rect_screen() + if rect is None: + return + rect = rect.intersected(self._view_rect) + fill = QColor(THEME.channel_green) + fill.setAlpha(28) + painter.setBrush(fill) + painter.setPen(Qt.PenStyle.NoPen) + painter.drawRect(rect) + + pen = QPen(QColor(THEME.channel_green), 1.5, Qt.PenStyle.DashLine) + pen.setCosmetic(True) + painter.setBrush(Qt.BrushStyle.NoBrush) + painter.setPen(pen) + painter.drawRect(rect) + def _draw_local_masks(self, painter: QPainter) -> None: if self._view_rect.isEmpty(): return @@ -668,13 +720,6 @@ def _map_to_image_coords(self, screen_pos: QPointF) -> Optional[Tuple[float, flo return float(np.clip(nb_x, 0, 1)), float(np.clip(nb_y, 0, 1)) - def _raw_from_screen_with_grid(self, screen_pos: QPointF, uv_grid: np.ndarray) -> Optional[Tuple[float, float]]: - if self._view_rect.isEmpty(): - return None - nb_x = float(np.clip((screen_pos.x() - self._view_rect.x()) / self._view_rect.width(), 0.0, 1.0)) - nb_y = float(np.clip((screen_pos.y() - self._view_rect.y()) / self._view_rect.height(), 0.0, 1.0)) - return CoordinateMapping.map_click_to_raw(nb_x, nb_y, uv_grid) - def mousePressEvent(self, event: QMouseEvent) -> None: if event.button() == Qt.MouseButton.MiddleButton or ( event.button() == Qt.MouseButton.LeftButton and self.zoom_level > 1.0 and self._tool_mode == ToolMode.NONE @@ -697,6 +742,12 @@ def mousePressEvent(self, event: QMouseEvent) -> None: event.accept() return + if self._tool_mode == ToolMode.ANALYSIS_DRAW: + self._start_analysis_drag(event.position()) + self.update() + event.accept() + return + coords = self._map_to_image_coords(event.position()) if coords: self.clicked.emit(*coords) @@ -704,10 +755,25 @@ def mousePressEvent(self, event: QMouseEvent) -> None: self._start_crop_drag(event.position()) self.update() + def _start_analysis_drag(self, pos: QPointF) -> None: + if self._view_rect.isEmpty(): + return + rect = self._analysis_rect_screen() + if rect is not None and rect.contains(pos): + self._analysis_drag_mode = "move" + self._analysis_press_norm = self._screen_to_norm(pos) + self._analysis_orig_rect = self._analysis_rect_norm + self.setCursor(Qt.CursorShape.ClosedHandCursor) + return + # Fresh region: drag out a new rectangle. + px = np.clip(pos.x(), self._view_rect.left(), self._view_rect.right()) + py = np.clip(pos.y(), self._view_rect.top(), self._view_rect.bottom()) + self._analysis_drag_mode = "draw" + self._analysis_draw_p1 = QPointF(px, py) + self._analysis_draw_p2 = QPointF(px, py) + def _start_crop_drag(self, pos: QPointF) -> None: - with self.state.metrics_lock: - uv_grid = self.state.last_metrics.get("uv_grid") - if uv_grid is None: + if self._view_rect.isEmpty(): return corners = self._crop_corner_screen_points() @@ -716,18 +782,14 @@ def _start_crop_drag(self, pos: QPointF) -> None: anchor_name = {"tl": "br", "tr": "bl", "br": "tl", "bl": "tr"}[corner] self._crop_drag_mode = "corner" self._crop_anchor_screen = corners[anchor_name] - self._crop_uv_grid = uv_grid self.setCursor(Qt.CursorShape.SizeFDiagCursor if corner in ("tl", "br") else Qt.CursorShape.SizeBDiagCursor) return if corners is not None and QPolygonF(list(corners.values())).containsPoint(pos, Qt.FillRule.OddEvenFill): - press_raw = self._raw_from_screen_with_grid(pos, uv_grid) - if press_raw is not None: - self._crop_drag_mode = "move" - self._crop_press_raw = press_raw - self._crop_orig_rect = self._crop_rect_raw - self._crop_uv_grid = uv_grid - self.setCursor(Qt.CursorShape.ClosedHandCursor) + self._crop_drag_mode = "move" + self._crop_press_norm = self._screen_to_norm(pos) + self._crop_orig_rect = self._crop_rect_norm + self.setCursor(Qt.CursorShape.ClosedHandCursor) return # Clicked outside the existing rect: draw a fresh one from scratch. @@ -736,7 +798,6 @@ def _start_crop_drag(self, pos: QPointF) -> None: self._crop_drag_mode = "draw" self._crop_draw_p1 = QPointF(px, py) self._crop_draw_p2 = QPointF(px, py) - self._crop_uv_grid = uv_grid def mouseMoveEvent(self, event: QMouseEvent) -> None: self._mouse_pos = event.position() @@ -755,35 +816,51 @@ def mouseMoveEvent(self, event: QMouseEvent) -> None: event.accept() return - if self._crop_drag_mode == "corner" and self._crop_anchor_screen is not None and self._crop_uv_grid is not None: + if self._analysis_drag_mode == "move" and self._analysis_press_norm is not None and self._analysis_orig_rect is not None: + curr_norm = self._screen_to_norm(event.position()) + dx = curr_norm[0] - self._analysis_press_norm[0] + dy = curr_norm[1] - self._analysis_press_norm[1] + new_rect = translate_manual_crop_rect(self._analysis_orig_rect, dx, dy) + if any(abs(a - b) > 5e-4 for a, b in zip(new_rect, self._analysis_rect_norm or new_rect)): + self._analysis_rect_norm = new_rect + self.analysis_rect_changed.emit(*new_rect, False) + self.update() + event.accept() + return + + if self._analysis_drag_mode == "draw" and self._analysis_draw_p1 is not None: + mx = np.clip(event.position().x(), self._view_rect.left(), self._view_rect.right()) + my = np.clip(event.position().y(), self._view_rect.top(), self._view_rect.bottom()) + self._analysis_draw_p2 = QPointF(mx, my) + self.update() + event.accept() + return + + if self._crop_drag_mode == "corner" and self._crop_anchor_screen is not None: cur_screen = 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())), ) - rect = self._apply_aspect_and_min(self._crop_anchor_screen, cur_screen, self._crop_uv_grid) - self._crop_rect_raw = rect + rect = self._apply_aspect_and_min(self._crop_anchor_screen, cur_screen) + self._crop_rect_norm = rect self.crop_rect_changed.emit(*rect, False) self.update() event.accept() return - if ( - self._crop_drag_mode == "move" - and self._crop_press_raw is not None - and self._crop_orig_rect is not None - and self._crop_uv_grid is not None - ): - curr_raw = self._raw_from_screen_with_grid(event.position(), self._crop_uv_grid) - if curr_raw is not None: - fine = bool(event.modifiers() & Qt.KeyboardModifier.ShiftModifier) - sensitivity = 0.2 if fine else 0.5 - dx = (curr_raw[0] - self._crop_press_raw[0]) * sensitivity - dy = (curr_raw[1] - self._crop_press_raw[1]) * sensitivity - new_rect = translate_manual_crop_rect(self._crop_orig_rect, dx, dy) - if any(abs(a - b) > 5e-4 for a, b in zip(new_rect, self._crop_rect_raw or new_rect)): - self._crop_rect_raw = new_rect - self.crop_rect_changed.emit(*new_rect, False) - self.update() + if self._crop_drag_mode == "move" and self._crop_press_norm is not None and self._crop_orig_rect is not None: + curr_norm = self._screen_to_norm(event.position()) + # Normalized coords track the cursor 1:1, so a plain drag moves the crop + # exactly with the mouse; Shift halves it for fine placement. + fine = bool(event.modifiers() & Qt.KeyboardModifier.ShiftModifier) + sensitivity = 0.5 if fine else 1.0 + dx = (curr_norm[0] - self._crop_press_norm[0]) * sensitivity + dy = (curr_norm[1] - self._crop_press_norm[1]) * sensitivity + new_rect = translate_manual_crop_rect(self._crop_orig_rect, dx, dy) + if any(abs(a - b) > 5e-4 for a, b in zip(new_rect, self._crop_rect_norm or new_rect)): + self._crop_rect_norm = new_rect + self.crop_rect_changed.emit(*new_rect, False) + self.update() event.accept() return @@ -865,6 +942,23 @@ def mouseDoubleClickEvent(self, event: QMouseEvent) -> None: self._finish_scratch() event.accept() return + # Double-clicking inside the crop box confirms the crop and closes the tool, so + # the user never has to leave the canvas to press the Crop button again. + if self._tool_mode == ToolMode.CROP_MANUAL: + corners = self._crop_corner_screen_points() + if corners is not None and QPolygonF(list(corners.values())).containsPoint(event.position(), Qt.FillRule.OddEvenFill): + self._end_crop_drag() + self.crop_confirmed.emit() + event.accept() + return + # Double-clicking inside the analysis region confirms it and closes the tool. + if self._tool_mode == ToolMode.ANALYSIS_DRAW: + rect = self._analysis_rect_screen() + if rect is not None and rect.contains(event.position()): + self._end_analysis_drag() + self.analysis_confirmed.emit() + event.accept() + return super().mouseDoubleClickEvent(event) def _finish_draw_if_active(self) -> None: @@ -900,8 +994,8 @@ def mouseReleaseEvent(self, event: QMouseEvent) -> None: return if self._crop_drag_mode in ("corner", "move"): - if self._crop_rect_raw is not None: - self.crop_rect_changed.emit(*self._crop_rect_raw, True) + if self._crop_rect_norm is not None: + self.crop_rect_changed.emit(*self._crop_rect_norm, True) self._end_crop_drag() self.unsetCursor() event.accept() @@ -910,17 +1004,35 @@ def mouseReleaseEvent(self, event: QMouseEvent) -> None: if self._crop_drag_mode == "draw": r = QRectF(self._crop_draw_p1, self._crop_draw_p2 or self._crop_draw_p1).normalized() r = r.intersected(self._view_rect) - uv_grid = self._crop_uv_grid - if r.width() > 5 and r.height() > 5 and uv_grid is not None: - c1 = self._raw_from_screen_with_grid(r.topLeft(), uv_grid) - c2 = self._raw_from_screen_with_grid(r.bottomRight(), uv_grid) - if c1 and c2: - rect = (min(c1[0], c2[0]), min(c1[1], c2[1]), max(c1[0], c2[0]), max(c1[1], c2[1])) - self._crop_rect_raw = rect - self.crop_rect_changed.emit(*rect, True) + if r.width() > 5 and r.height() > 5: + c1 = self._screen_to_norm(r.topLeft()) + c2 = self._screen_to_norm(r.bottomRight()) + rect = (min(c1[0], c2[0]), min(c1[1], c2[1]), max(c1[0], c2[0]), max(c1[1], c2[1])) + self._crop_rect_norm = rect + self.crop_rect_changed.emit(*rect, True) self._end_crop_drag() self.update() + if self._analysis_drag_mode == "move": + if self._analysis_rect_norm is not None: + self.analysis_rect_changed.emit(*self._analysis_rect_norm, True) + self._end_analysis_drag() + self.unsetCursor() + event.accept() + return + + if self._analysis_drag_mode == "draw": + r = QRectF(self._analysis_draw_p1, self._analysis_draw_p2 or self._analysis_draw_p1).normalized() + r = r.intersected(self._view_rect) + if r.width() > 5 and r.height() > 5: + c1 = self._screen_to_norm(r.topLeft()) + c2 = self._screen_to_norm(r.bottomRight()) + rect = (min(c1[0], c2[0]), min(c1[1], c2[1]), max(c1[0], c2[0]), max(c1[1], c2[1])) + self._analysis_rect_norm = rect + self.analysis_rect_changed.emit(*rect, True) + self._end_analysis_drag() + self.update() + def leaveEvent(self, event) -> None: self.cursor_left.emit() super().leaveEvent(event) diff --git a/negpy/desktop/view/canvas/widget.py b/negpy/desktop/view/canvas/widget.py index ffbfbf95..f928433e 100644 --- a/negpy/desktop/view/canvas/widget.py +++ b/negpy/desktop/view/canvas/widget.py @@ -34,6 +34,7 @@ def clamp_canvas_zoom_level(zoom: float) -> float: ToolMode.CROP_MANUAL: Qt.CursorShape.CrossCursor, ToolMode.DUST_PICK: Qt.CursorShape.BlankCursor, ToolMode.LOCAL_DRAW: Qt.CursorShape.CrossCursor, + ToolMode.ANALYSIS_DRAW: Qt.CursorShape.CrossCursor, } # Do not apply more than this many notch-equivalents in a single event (huge flings). _WHEEL_MAX_NOTCHES = 4.0 @@ -74,6 +75,9 @@ class ImageCanvas(QWidget): clicked = pyqtSignal(float, float) crop_rect_changed = pyqtSignal(float, float, float, float, bool) + crop_confirmed = pyqtSignal() + analysis_rect_changed = pyqtSignal(float, float, float, float, bool) + analysis_confirmed = pyqtSignal() zoom_changed = pyqtSignal(float) cursor_position_changed = pyqtSignal(float, float) cursor_left_canvas = pyqtSignal() @@ -121,6 +125,9 @@ def __init__(self, state: AppState, parent=None): self.overlay.clicked.connect(self.clicked.emit) self.overlay.crop_rect_changed.connect(self.crop_rect_changed.emit) + self.overlay.crop_confirmed.connect(self.crop_confirmed.emit) + self.overlay.analysis_rect_changed.connect(self.analysis_rect_changed.emit) + self.overlay.analysis_confirmed.connect(self.analysis_confirmed.emit) self.overlay.cursor_moved.connect(self.cursor_position_changed.emit) self.overlay.cursor_left.connect(self.cursor_left_canvas.emit) self.overlay.lasso_completed.connect(self.lasso_completed.emit) diff --git a/negpy/desktop/view/main_window.py b/negpy/desktop/view/main_window.py index 3d529fec..2b82dbdd 100644 --- a/negpy/desktop/view/main_window.py +++ b/negpy/desktop/view/main_window.py @@ -327,6 +327,9 @@ def _connect_signals(self) -> None: self.canvas.clicked.connect(self.controller.handle_canvas_clicked) self.canvas.crop_rect_changed.connect(self.controller.handle_crop_rect_changed) + self.canvas.crop_confirmed.connect(self.controller.confirm_manual_crop) + self.canvas.analysis_rect_changed.connect(self.controller.handle_analysis_rect_changed) + self.canvas.analysis_confirmed.connect(self.controller.confirm_analysis_region) self.canvas.lasso_completed.connect(self.controller.handle_lasso_completed) self.canvas.scratch_completed.connect(self.controller.handle_heal_stroke_completed) self.canvas.local_mask_selected.connect(self.controller.select_local_mask) diff --git a/negpy/desktop/view/sidebar/controls_panel.py b/negpy/desktop/view/sidebar/controls_panel.py index 01d1ade4..80337fe6 100644 --- a/negpy/desktop/view/sidebar/controls_panel.py +++ b/negpy/desktop/view/sidebar/controls_panel.py @@ -641,6 +641,7 @@ def _sync_modified_dots(self) -> None: proc.process_mode != _proc.process_mode, proc.linear_raw != _proc.linear_raw, proc.analysis_buffer != _proc.analysis_buffer, + proc.analysis_rect is not None, proc.luma_range_clip != _proc.luma_range_clip, proc.color_range_clip != _proc.color_range_clip, proc.white_point_offset != _proc.white_point_offset, @@ -677,3 +678,4 @@ def _sync_tool_buttons(self) -> None: """Updates toggle button states to match active_tool.""" self.geometry_sidebar.sync_ui() self.local_sidebar.sync_ui() + self.process_sidebar.sync_ui() diff --git a/negpy/desktop/view/sidebar/files.py b/negpy/desktop/view/sidebar/files.py index 0c0798f4..3255da09 100644 --- a/negpy/desktop/view/sidebar/files.py +++ b/negpy/desktop/view/sidebar/files.py @@ -4,6 +4,7 @@ from PyQt6.QtCore import Qt, QItemSelectionModel, QModelIndex, QRect, QSize, QTimer, pyqtSignal from PyQt6.QtGui import QActionGroup, QColor, QPainter, QPen from PyQt6.QtWidgets import ( + QApplication, QCheckBox, QDialog, QDialogButtonBox, @@ -297,6 +298,7 @@ def _connect_signals(self) -> None: self.add_files_btn.clicked.connect(self._on_add_files) self.add_folder_btn.clicked.connect(self._on_add_folder) self.unload_btn.clicked.connect(self._on_unload_clicked) + self.list_view.clicked.connect(self._on_item_clicked) self.list_view.doubleClicked.connect(self._on_item_double_clicked) self.list_view.customContextMenuRequested.connect(self._show_context_menu) self.list_view.selectionModel().selectionChanged.connect(self._on_selection_changed) @@ -469,11 +471,23 @@ def _on_add_folder(self) -> None: self.session.repo.save_global_setting("last_open_folder", os.path.dirname(folder)) self.controller.request_asset_discovery([folder], auto_open=True) - def _on_item_double_clicked(self, index) -> None: + def _activate_file(self, index) -> None: + """Load a thumbnail into the main viewer, skipping a redundant reload of the + already-active frame.""" actual = self.session.asset_model.display_to_actual(index.row()) - if actual >= 0: + if actual >= 0 and actual != self.session.state.selected_file_idx: self.session.select_file(actual) + def _on_item_clicked(self, index) -> None: + # Plain single click sets the active frame instantly. Ctrl/Shift clicks build a + # multi-selection for batch actions and are left to the selectionChanged handler. + if QApplication.keyboardModifiers() & (Qt.KeyboardModifier.ControlModifier | Qt.KeyboardModifier.ShiftModifier): + return + self._activate_file(index) + + def _on_item_double_clicked(self, index) -> None: + self._activate_file(index) + def _show_context_menu(self, pos) -> None: index = self.list_view.indexAt(pos) if not index.isValid(): diff --git a/negpy/desktop/view/sidebar/geometry.py b/negpy/desktop/view/sidebar/geometry.py index 9519e5de..0c3d7db1 100644 --- a/negpy/desktop/view/sidebar/geometry.py +++ b/negpy/desktop/view/sidebar/geometry.py @@ -114,7 +114,7 @@ def _init_ui(self) -> None: unit=" px", ) self.offset_slider.setToolTip(tooltip_with_shortcut("Insets the crop border from the auto-detected film edge (px)", "offset_inc")) - self.fine_rot_slider = CompactSlider("Fine Rot", -5.0, 5.0, conf.fine_rotation, unit="°") + self.fine_rot_slider = CompactSlider("Fine Rotation", -5.0, 5.0, conf.fine_rotation, unit="°") self.fine_rot_slider.setToolTip("Fine-tunes rotation to correct slight tilt (degrees)") slider_row.addWidget(self.offset_slider) slider_row.addWidget(self.fine_rot_slider) diff --git a/negpy/desktop/view/sidebar/process.py b/negpy/desktop/view/sidebar/process.py index 2780de9e..f4c48f90 100644 --- a/negpy/desktop/view/sidebar/process.py +++ b/negpy/desktop/view/sidebar/process.py @@ -7,6 +7,7 @@ QPushButton, ) +from negpy.desktop.session import ToolMode from negpy.desktop.view.sidebar.base import BaseSidebar from negpy.desktop.view.styles.templates import field_label, section_subheader from negpy.desktop.view.styles.theme import THEME @@ -101,7 +102,21 @@ def _init_ui(self) -> None: self.analysis_buffer_slider.setToolTip( "Crops the analysis region inward to exclude film borders and rebate from exposure calculations" ) + self.analysis_region_btn = QPushButton() + self.analysis_region_btn.setCheckable(True) + self.analysis_region_btn.setIcon(qta.icon("fa5s.vector-square", color=THEME.text_primary)) + self.analysis_region_btn.setFixedWidth(32) + self.analysis_region_btn.setToolTip( + "Draw a freehand analysis region on the image — the meters read exactly that area " + "(overrides the Analysis Buffer). Double-click inside it to confirm." + ) + self.clear_analysis_region_btn = QPushButton() + self.clear_analysis_region_btn.setIcon(qta.icon("fa5s.times", color=THEME.text_primary)) + self.clear_analysis_region_btn.setFixedWidth(32) + self.clear_analysis_region_btn.setToolTip("Clear the freehand analysis region (fall back to the Analysis Buffer)") buf_row.addWidget(self.analysis_buffer_slider) + buf_row.addWidget(self.analysis_region_btn) + buf_row.addWidget(self.clear_analysis_region_btn) self.layout.addLayout(buf_row) clip_row = QHBoxLayout() @@ -174,6 +189,8 @@ def _connect_signals(self) -> None: self.analysis_buffer_slider.valueChanged.connect(lambda v: self._on_buffer_changed(v, persist=False)) self.analysis_buffer_slider.valueCommitted.connect(lambda v: self._on_buffer_changed(v, persist=True)) + self.analysis_region_btn.toggled.connect(self._on_analysis_region_toggled) + self.clear_analysis_region_btn.clicked.connect(self.controller.clear_analysis_region) self.luma_range_clip_slider.valueChanged.connect(lambda v: self._on_luma_range_clip_changed(v, persist=False)) self.luma_range_clip_slider.valueCommitted.connect(lambda v: self._on_luma_range_clip_changed(v, persist=True)) @@ -262,6 +279,9 @@ def _on_normalize_e6_toggled(self, checked: bool) -> None: **invalidate_local_bounds(self.state.config.process), ) + def _on_analysis_region_toggled(self, checked: bool) -> None: + self.controller.set_active_tool(ToolMode.ANALYSIS_DRAW if checked else ToolMode.NONE) + def _on_buffer_changed(self, val: float, persist: bool = True) -> None: self.update_config_section( "process", @@ -320,10 +340,17 @@ def sync_ui(self) -> None: self.crosstalk_strength_slider.setVisible(not is_bw) self.autodetect_btn.setChecked(self.state.autodetect_enabled) + has_region = conf.analysis_rect is not None + self.analysis_region_btn.setChecked(self.state.active_tool == ToolMode.ANALYSIS_DRAW) + self.clear_analysis_region_btn.setEnabled(has_region) + locked = conf.lock_bounds # Each clip slider is disabled when its axis rides the roll baseline; the - # analysis buffer only matters when at least one axis still analyzes locally. - self.analysis_buffer_slider.setEnabled(not locked and not (conf.use_luma_average and conf.use_colour_average)) + # analysis buffer only matters when at least one axis still analyzes locally, + # and is overridden entirely by a freehand analysis region. + self.analysis_buffer_slider.setEnabled( + not locked and not has_region and not (conf.use_luma_average and conf.use_colour_average) + ) self.luma_range_clip_slider.setEnabled(not locked and not conf.use_luma_average) self.color_range_clip_slider.setEnabled(not locked and not conf.use_colour_average) for w in (self.white_point_slider, self.black_point_slider): @@ -341,6 +368,7 @@ def block_signals(self, blocked: bool) -> None: self.lock_bounds_btn, self.linear_raw_btn, self.analysis_buffer_slider, + self.analysis_region_btn, self.luma_range_clip_slider, self.color_range_clip_slider, self.white_point_slider, diff --git a/negpy/desktop/view/sidebar/right_panel.py b/negpy/desktop/view/sidebar/right_panel.py index 0728dbc6..6c7fb6ee 100644 --- a/negpy/desktop/view/sidebar/right_panel.py +++ b/negpy/desktop/view/sidebar/right_panel.py @@ -186,6 +186,13 @@ def wrap_scroll(widget: QWidget) -> QScrollArea: self.splitter.setSizes([320, 600]) self.splitter.splitterMoved.connect(lambda *_: repo.save_global_setting("analysis_splitter_sizes", self.splitter.sizes())) + # Collapsing the Analysis section should hand its splitter space back to the tabs + # below (pinning the header at the top) instead of leaving a large empty pane. + self._analysis_expanded_size = self.splitter.sizes()[0] + self.analysis_section.expanded_changed.connect(self._resize_splitter_for_analysis) + if not analysis_expanded: + self._resize_splitter_for_analysis(False) + layout.addWidget(self.splitter, 1) self.apply_shortcut_tooltips() @@ -193,6 +200,20 @@ def wrap_scroll(widget: QWidget) -> QScrollArea: # Default tab (Setup) self._switch_tab(0) + def _resize_splitter_for_analysis(self, expanded: bool) -> None: + """Pin the collapsed Analysis header at the top: shrink pane 0 to the header and + give the rest to the tabs; restore the prior size when re-expanded.""" + sizes = self.splitter.sizes() + total = sum(sizes) + if total <= 0: + return + if expanded: + top = min(max(self._analysis_expanded_size, 120), max(120, total - 120)) + else: + self._analysis_expanded_size = sizes[0] + top = max(1, self.analysis_section.sizeHint().height()) + self.splitter.setSizes([top, max(0, total - top)]) + def apply_shortcut_tooltips(self) -> None: """Append the current keyboard shortcut (action id `tab_`) to each tab tooltip.""" for btn, key, base in zip(self._tab_buttons, self._tab_keys, self._tab_tooltips): diff --git a/negpy/features/exposure/normalization.py b/negpy/features/exposure/normalization.py index 501f20c3..e45b763b 100644 --- a/negpy/features/exposure/normalization.py +++ b/negpy/features/exposure/normalization.py @@ -52,6 +52,33 @@ def __init__(self, floors: Tuple[float, float, float], ceils: Tuple[float, float self.ceils = ceils +def resolve_analysis_region( + image_shape: tuple[int, ...], + active_roi: Optional[tuple[int, int, int, int]], + analysis_buffer: float, + analysis_rect: Optional[tuple[float, float, float, float]], +) -> tuple[Optional[tuple[int, int, int, int]], float]: + """Resolve the (roi, buffer) the meters should read. + + A freehand `analysis_rect` (normalized in the transformed image) wins over the crop + ROI + centered buffer: it maps directly to a pixel ROI and disables the symmetric + inset. Otherwise the crop `active_roi` and the `analysis_buffer` slider apply as before. + """ + if analysis_rect is not None: + h, w = image_shape[:2] + x1, y1, x2, y2 = analysis_rect + roi = ( + int(min(y1, y2) * h), + int(max(y1, y2) * h), + int(min(x1, x2) * w), + int(max(x1, x2) * w), + ) + # Degenerate rect (zero area) is ignored so a stray click can't blank analysis. + if roi[1] - roi[0] >= 2 and roi[3] - roi[2] >= 2: + return roi, 0.0 + return active_roi, analysis_buffer + + def get_analysis_crop(img: ImageBuffer, buffer_ratio: float) -> ImageBuffer: """ Returns a center crop of the image for analysis purposes. diff --git a/negpy/features/exposure/processor.py b/negpy/features/exposure/processor.py index edb77bc7..3f0ae95c 100644 --- a/negpy/features/exposure/processor.py +++ b/negpy/features/exposure/processor.py @@ -30,6 +30,7 @@ measure_textural_range_from_log, normalize_log_image, prefilter_log_grid, + resolve_analysis_region, resolve_bounds_detailed, resolve_crosstalk_matrix, unmix_log_image, @@ -53,9 +54,11 @@ def process(self, image: ImageBuffer, context: PipelineContext) -> ImageBuffer: # No upper clamp: mirrors normalization.wgsl (only the low side is clamped); # values above 1.0 only occur with flat-field gain and must match the GPU. img_log = np.log10(np.clip(np.nan_to_num(image, nan=epsilon, posinf=1.0, neginf=epsilon), epsilon, None)) - # Shared prefilter, once for all five meters (ROI/buffer applied here). - prefiltered = prefilter_log_grid(image, context.active_roi, self.config.analysis_buffer) - context.metrics["scan_clip_fractions"] = measure_clip_fractions(image, context.active_roi, self.config.analysis_buffer) + # Shared prefilter, once for all five meters (ROI/buffer applied here). A freehand + # analysis_rect overrides the crop ROI + centered buffer for the metered region. + an_roi, an_buffer = resolve_analysis_region(image.shape, context.active_roi, self.config.analysis_buffer, self.config.analysis_rect) + prefiltered = prefilter_log_grid(image, an_roi, an_buffer) + context.metrics["scan_clip_fractions"] = measure_clip_fractions(image, an_roi, an_buffer) # Capture-side dye unmix on the negative densities, before any metering, # so bounds/anchor/cast refs all read the unmixed film. @@ -65,6 +68,7 @@ def process(self, image: ImageBuffer, context: PipelineContext) -> ImageBuffer: def analyze_base() -> LogNegativeBounds: cached_buffer = context.metrics.get("log_bounds_buffer_val") + cached_rect = context.metrics.get("log_bounds_rect_val") cached_norm = context.metrics.get("log_bounds_norm_val") cached_mode = context.metrics.get("log_bounds_mode_val") @@ -75,6 +79,7 @@ def analyze_base() -> LogNegativeBounds: "log_bounds" not in context.metrics or cached_buffer is None or abs(cached_buffer - self.config.analysis_buffer) > 1e-5 + or cached_rect != self.config.analysis_rect or cached_clip is None or abs(cached_clip - self.config.luma_range_clip) > 1e-6 or cached_color_clip is None @@ -98,6 +103,7 @@ def analyze_base() -> LogNegativeBounds: ) context.metrics["log_bounds"] = analyzed context.metrics["log_bounds_buffer_val"] = self.config.analysis_buffer + context.metrics["log_bounds_rect_val"] = self.config.analysis_rect context.metrics["log_bounds_clip_val"] = self.config.luma_range_clip context.metrics["log_bounds_color_clip_val"] = self.config.color_range_clip context.metrics["log_bounds_norm_val"] = self.config.e6_normalize @@ -112,11 +118,13 @@ def analyze_base() -> LogNegativeBounds: if context.process_mode == ProcessMode.C41: cached_ref_buffer = context.metrics.get("shadow_refs_buffer_val") + cached_ref_rect = context.metrics.get("shadow_refs_rect_val") cached_ref_unmix = context.metrics.get("shadow_refs_crosstalk_val") if ( "shadow_log_refs" not in context.metrics or cached_ref_buffer is None or abs(cached_ref_buffer - self.config.analysis_buffer) > 1e-5 + or cached_ref_rect != self.config.analysis_rect or cached_ref_unmix != (self.config.crosstalk_strength, self.config.crosstalk_matrix) ): context.metrics["shadow_log_refs"] = measure_shadow_refs_from_log( @@ -125,6 +133,7 @@ def analyze_base() -> LogNegativeBounds: 0.0, ) context.metrics["shadow_refs_buffer_val"] = self.config.analysis_buffer + context.metrics["shadow_refs_rect_val"] = self.config.analysis_rect context.metrics["shadow_refs_crosstalk_val"] = (self.config.crosstalk_strength, self.config.crosstalk_matrix) if self.config.white_point_offset != 0.0 or self.config.black_point_offset != 0.0: diff --git a/negpy/features/geometry/logic.py b/negpy/features/geometry/logic.py index aa1fcbe2..8880ba25 100644 --- a/negpy/features/geometry/logic.py +++ b/negpy/features/geometry/logic.py @@ -935,17 +935,18 @@ def _enforce_ratio_by_occupancy( def get_manual_rect_coords( img_or_shape: ImageBuffer | Tuple[int, int], manual_rect: Tuple[float, float, float, float], - orig_shape: Tuple[int, int], - rotation_k: int = 0, - fine_rotation: float = 0.0, - flip_horizontal: bool = False, - flip_vertical: bool = False, offset_px: int = 0, scale_factor: float = 1.0, - distortion_k1: float = 0.0, ) -> ROI: """ - Maps normalized manual crop rect (RAW coords) to pixel ROI in TRANSFORMED image space. + Maps a normalized manual crop rect to a pixel ROI in the TRANSFORMED image. + + The rect is expressed in the coordinate space of the already-transformed image + (post rotation / fine-rotation / flip / distortion) — the same space the user draws + it on in the canvas overlay — so it is a plain axis-aligned slice: no corner mapping, + no bounding-box collapse. Storing it in raw space instead forced the crop through + `map_coords_to_geometry` + an AABB, which inflated the region as fine rotation tilted + the mapped rect (the crop grew larger than the drawn box). """ if isinstance(img_or_shape, tuple): h_curr, w_curr = img_or_shape @@ -953,26 +954,8 @@ def get_manual_rect_coords( h_curr, w_curr = img_or_shape.shape[:2] x1_n, y1_n, x2_n, y2_n = manual_rect - - corners = [(x1_n, y1_n), (x2_n, y1_n), (x2_n, y2_n), (x1_n, y2_n)] - mapped_corners = [] - - for nx, ny in corners: - mx, my = map_coords_to_geometry( - nx, - ny, - orig_shape, - rotation_k, - fine_rotation, - flip_horizontal, - flip_vertical, - roi=None, - distortion_k1=distortion_k1, - ) - mapped_corners.append((mx, my)) - - xs = [p[0] * w_curr for p in mapped_corners] - ys = [p[1] * h_curr for p in mapped_corners] + xs = (x1_n * w_curr, x2_n * w_curr) + ys = (y1_n * h_curr, y2_n * h_curr) ix1, ix2 = int(min(xs)), int(max(xs)) iy1, iy2 = int(min(ys)), int(max(ys)) diff --git a/negpy/features/geometry/processor.py b/negpy/features/geometry/processor.py index c4a29c75..26f44441 100644 --- a/negpy/features/geometry/processor.py +++ b/negpy/features/geometry/processor.py @@ -21,7 +21,6 @@ def __init__(self, config: GeometryConfig, distortion_k1: float = 0.0): self.distortion_k1 = distortion_k1 def process(self, image: ImageBuffer, context: PipelineContext) -> ImageBuffer: - orig_shape = (image.shape[0], image.shape[1]) img = image ir = context.ir_buffer @@ -64,14 +63,8 @@ def process(self, image: ImageBuffer, context: PipelineContext) -> ImageBuffer: roi = get_manual_rect_coords( img, self.config.manual_crop_rect, - orig_shape=orig_shape, - rotation_k=self.config.rotation, - fine_rotation=self.config.fine_rotation, - flip_horizontal=self.config.flip_horizontal, - flip_vertical=self.config.flip_vertical, offset_px=self.config.autocrop_offset, scale_factor=context.scale_factor, - distortion_k1=self.distortion_k1, ) context.active_roi = roi elif self.config.auto_crop_enabled: diff --git a/negpy/features/process/models.py b/negpy/features/process/models.py index 384d92b2..4d0b14e1 100644 --- a/negpy/features/process/models.py +++ b/negpy/features/process/models.py @@ -24,6 +24,11 @@ class ProcessConfig: process_mode: ProcessMode = ProcessMode.C41 linear_raw: bool = True analysis_buffer: float = 0.05 + # Optional freehand analysis region, normalized in the transformed (display) image — + # the same space as the manual crop rect. When set it defines the exact area the + # black/white-point meters read (the centered analysis_buffer inset is bypassed); + # None falls back to the analysis_buffer slider. + analysis_rect: Optional[tuple] = None # Two independent normalization clip axes: luma drives black/white-point span # (dynamic range), colour is the per-channel-balance clip percentile (orange-mask # cast removal), defaulting to the robust base_color_clip neutral. @@ -66,6 +71,8 @@ def __post_init__(self) -> None: object.__setattr__(self, "local_ceils", tuple(self.local_ceils)) if self.crosstalk_matrix is not None: object.__setattr__(self, "crosstalk_matrix", tuple(self.crosstalk_matrix)) + if self.analysis_rect is not None: + object.__setattr__(self, "analysis_rect", tuple(self.analysis_rect)) @property def is_local_initialized(self) -> bool: diff --git a/negpy/services/rendering/engine.py b/negpy/services/rendering/engine.py index 2fd33a54..0d6f611e 100644 --- a/negpy/services/rendering/engine.py +++ b/negpy/services/rendering/engine.py @@ -120,6 +120,7 @@ def run_base(img_in: ImageBuffer, ctx: PipelineContext) -> ImageBuffer: settings.process.e6_normalize, geometry_key, settings.process.analysis_buffer, + settings.process.analysis_rect, settings.process.luma_range_clip, settings.process.color_range_clip, settings.process.use_luma_average, diff --git a/negpy/services/rendering/gpu_engine.py b/negpy/services/rendering/gpu_engine.py index 8d467b52..b252afa3 100644 --- a/negpy/services/rendering/gpu_engine.py +++ b/negpy/services/rendering/gpu_engine.py @@ -27,6 +27,7 @@ unmix_log_image, measure_textural_range_from_log, prefilter_log_grid, + resolve_analysis_region, resolve_bounds_detailed, ) from negpy.features.geometry.logic import ( @@ -442,14 +443,8 @@ def process_to_texture( roi = get_manual_rect_coords( (h_rot, w_rot), settings.geometry.manual_crop_rect, - orig_shape=orig_shape, - rotation_k=settings.geometry.rotation, - fine_rotation=settings.geometry.fine_rotation, - flip_horizontal=settings.geometry.flip_horizontal, - flip_vertical=settings.geometry.flip_vertical, offset_px=settings.geometry.autocrop_offset, scale_factor=scale_factor, - distortion_k1=k1_eff, ) elif settings.geometry.auto_crop_enabled: roi = _detect_autocrop_roi(img, settings, h_rot, w_rot) @@ -510,7 +505,15 @@ def process_to_texture( analysis_source = np.fliplr(analysis_source) if settings.geometry.flip_vertical: analysis_source = np.flipud(analysis_source) - analysis_roi = roi if not tiling_mode else None + # A freehand analysis_rect overrides the crop ROI + centered buffer (mirrors + # the CPU path); tiled export uses explicit overrides so it stays on the ROI. + base_roi = roi if not tiling_mode else None + analysis_roi, an_buffer = resolve_analysis_region( + analysis_source.shape, + base_roi, + settings.process.analysis_buffer, + settings.process.analysis_rect if not tiling_mode else None, + ) if analysis_roi is not None: ay1, ay2, ax1, ax2 = analysis_roi analysis_source = np.ascontiguousarray(analysis_source[ay1:ay2, ax1:ax2]) @@ -520,11 +523,11 @@ def process_to_texture( analysis_source = _downsample_for_analysis(analysis_source, APP_CONFIG.preview_render_size) # Shared prefilter, once for all five meters (ROI already applied). # Unmixed like the CPU path so every meter reads the unmixed film. - prefiltered = unmix_log_image(prefilter_log_grid(analysis_source, None, settings.process.analysis_buffer), unmix_m) + prefiltered = unmix_log_image(prefilter_log_grid(analysis_source, None, an_buffer), unmix_m) scan_clip_fractions = None if analysis_source is not None: - scan_clip_fractions = measure_clip_fractions(analysis_source, None, settings.process.analysis_buffer) + scan_clip_fractions = measure_clip_fractions(analysis_source, None, an_buffer) if analysis_key is not None: self._clip_cache = (analysis_key, scan_clip_fractions) elif analysis_key is not None and self._clip_cache is not None and self._clip_cache[0] == analysis_key: @@ -1630,14 +1633,8 @@ def _process_tiled( roi = get_manual_rect_coords( (h_rot, w_rot), settings.geometry.manual_crop_rect, - orig_shape=(h, w), - rotation_k=settings.geometry.rotation, - fine_rotation=settings.geometry.fine_rotation, - flip_horizontal=settings.geometry.flip_horizontal, - flip_vertical=settings.geometry.flip_vertical, offset_px=settings.geometry.autocrop_offset, scale_factor=scale_factor, - distortion_k1=k1_eff, ) elif settings.geometry.auto_crop_enabled: roi = _detect_autocrop_roi(img, settings, h_rot, w_rot) diff --git a/tests/test_analysis_region.py b/tests/test_analysis_region.py new file mode 100644 index 00000000..75535f60 --- /dev/null +++ b/tests/test_analysis_region.py @@ -0,0 +1,50 @@ +import numpy as np + +from negpy.domain.interfaces import PipelineContext +from negpy.features.exposure.normalization import resolve_analysis_region +from negpy.features.exposure.processor import NormalizationProcessor +from negpy.features.process.models import ProcessConfig, ProcessMode + + +def test_resolve_falls_back_to_roi_and_buffer_when_rect_none(): + roi, buf = resolve_analysis_region((100, 200), (10, 90, 20, 180), 0.05, None) + assert roi == (10, 90, 20, 180) + assert buf == 0.05 + + +def test_resolve_rect_overrides_roi_and_zeroes_buffer(): + # Rect is normalized in the transformed image (H=100, W=200). + roi, buf = resolve_analysis_region((100, 200), (10, 90, 20, 180), 0.05, (0.25, 0.5, 0.75, 1.0)) + assert roi == (50, 100, 50, 150) + assert buf == 0.0 + + +def test_resolve_ignores_degenerate_rect(): + # Zero-area rect can't blank analysis: fall back to the ROI + buffer. + roi, buf = resolve_analysis_region((100, 100), None, 0.1, (0.5, 0.5, 0.5, 0.5)) + assert roi is None + assert buf == 0.1 + + +def _split_negative() -> np.ndarray: + # Linear "negative": a dense (dark) left half and a thin (bright) right half. + img = np.empty((200, 200, 3), dtype=np.float32) + img[:, :100] = 0.15 + img[:, 100:] = 0.85 + return img + + +def _bounds_for_rect(rect): + img = _split_negative() + cfg = ProcessConfig(process_mode=ProcessMode.C41, analysis_buffer=0.0, analysis_rect=rect) + ctx = PipelineContext(scale_factor=1.0, original_size=(200, 200)) + NormalizationProcessor(cfg).process(img, ctx) + return ctx.metrics["log_bounds"] + + +def test_analysis_rect_changes_metered_bounds(): + # Metering only the dense (dark) half vs only the thin (bright) half must yield + # different normalization bounds — proof the rect is wired through the meters. + dark = _bounds_for_rect((0.0, 0.0, 0.5, 1.0)) + bright = _bounds_for_rect((0.5, 0.0, 1.0, 1.0)) + assert not np.allclose(dark.floors, bright.floors) or not np.allclose(dark.ceils, bright.ceils) diff --git a/tests/test_config_deserialization.py b/tests/test_config_deserialization.py index 5e160859..a141481c 100644 --- a/tests/test_config_deserialization.py +++ b/tests/test_config_deserialization.py @@ -130,6 +130,18 @@ def test_manual_crop_rect_hashable_in_engine_base_key(self): ) self.assertIsInstance(calculate_config_hash(base_key), str) + def test_analysis_rect_survives_db_roundtrip_as_tuple(self): + """The freehand analysis region must reload as a tuple so the frozen + ProcessConfig stays hashable for the pipeline cache key.""" + config = WorkspaceConfig() + config = replace(config, process=replace(config.process, analysis_rect=(0.1, 0.2, 0.8, 0.9))) + + reloaded = WorkspaceConfig.from_flat_dict(json.loads(json.dumps(config.to_dict(), default=str))) + + self.assertIsInstance(reloaded.process.analysis_rect, tuple) + self.assertEqual(reloaded.process.analysis_rect, (0.1, 0.2, 0.8, 0.9)) + hash(reloaded.process) # must not raise + def test_autocrop_mode_defaults_to_image_for_legacy_dicts(self): config = WorkspaceConfig.from_flat_dict({"process_mode": ProcessMode.C41}) self.assertEqual(config.geometry.autocrop_mode, "image") diff --git a/tests/test_geometry_logic.py b/tests/test_geometry_logic.py index c220dcbc..8cf7db8e 100644 --- a/tests/test_geometry_logic.py +++ b/tests/test_geometry_logic.py @@ -188,48 +188,41 @@ def test_map_coords_to_geometry_flips(): assert abs(fny - 0.7) < 0.001 -def test_get_manual_rect_coords_rotation(): +def test_get_manual_rect_coords_transformed_dims(): from negpy.features.geometry.logic import get_manual_rect_coords - # Raw image: 100x200 (H, W) - # Rotated 90 deg CCW: 200x100 - img_rot = np.zeros((200, 100, 3), dtype=np.float32) - manual_rect = (0.0, 0.0, 0.5, 0.5) # Top-left quadrant in raw space + # The rect is normalized in the already-transformed image, so it scales directly by + # the passed (transformed) dims — no rotation/flip re-mapping, no bounding box. + img = np.zeros((200, 100, 3), dtype=np.float32) # transformed H=200, W=100 + roi = get_manual_rect_coords(img, (0.0, 0.0, 0.5, 0.5), offset_px=0) + assert roi == (0, 100, 0, 50) - # k=1 is 90 deg CCW - roi = get_manual_rect_coords( - img_rot, - manual_rect, - orig_shape=(100, 200), - rotation_k=1, - offset_px=0, - ) - - # In raw space: y=0..50, x=0..100 - # CCW 90 rotation maps: - # Top-Left (0,0) -> Bottom-Left (100, 0) ? No, standard numpy rot90 behavior - # Let's just assert it produces a non-empty valid ROI for now - assert roi[1] > roi[0] - assert roi[3] > roi[2] - assert roi[1] <= 200 - assert roi[3] <= 100 - -def test_get_manual_rect_coords_flips(): +def test_get_manual_rect_coords_offset_inset(): from negpy.features.geometry.logic import get_manual_rect_coords img = np.zeros((100, 100, 3), dtype=np.float32) - manual_rect = (0.0, 0.0, 0.5, 0.5) # Top-left quadrant + roi = get_manual_rect_coords(img, (0.2, 0.2, 0.8, 0.8), offset_px=5) + # 0.2..0.8 of 100 = 20..80, inset 5px each side => 25..75 + assert roi == (25, 75, 25, 75) - # Horizontal flip - roi = get_manual_rect_coords(img, manual_rect, orig_shape=(100, 100), flip_horizontal=True) - # Should become top-right quadrant: x=50..100 - assert roi == (0, 50, 50, 100) - # Vertical flip - roi = get_manual_rect_coords(img, manual_rect, orig_shape=(100, 100), flip_vertical=True) - # Should become bottom-left quadrant: y=50..100 - assert roi == (50, 100, 0, 50) +def test_manual_crop_no_inflation_under_fine_rotation(): + # Regression (#task6): the manual crop rect lives in transformed (display) space, so + # fine rotation — which preserves the image dims — must resolve to the identical ROI + # instead of inflating it via a corner-mapped bounding box. + rect = (0.2, 0.2, 0.8, 0.8) + img = np.zeros((400, 600, 3), dtype=np.float32) + + ctx_base = PipelineContext(scale_factor=1.0, original_size=(400, 600)) + GeometryProcessor(GeometryConfig(manual_crop_rect=rect, autocrop_offset=0)).process(img, ctx_base) + + ctx_rot = PipelineContext(scale_factor=1.0, original_size=(400, 600)) + GeometryProcessor(GeometryConfig(manual_crop_rect=rect, autocrop_offset=0, fine_rotation=4.0)).process(img, ctx_rot) + + assert ctx_base.active_roi == ctx_rot.active_roi + # Exactly the fractional slice of the transformed frame, un-inflated. + assert ctx_base.active_roi == (80, 320, 120, 480) def test_translate_within_bounds(): @@ -355,7 +348,6 @@ def test_manual_rect_coords_fractional_inset_scale_invariant(): roi_prev = get_manual_rect_coords( (PREV_H, PREV_W), manual_rect, - orig_shape=(PREV_H, PREV_W), offset_px=offset_px, scale_factor=1.0, ) @@ -370,7 +362,6 @@ def test_manual_rect_coords_fractional_inset_scale_invariant(): roi_full = get_manual_rect_coords( (FULL_H, FULL_W), manual_rect, - orig_shape=(FULL_H, FULL_W), offset_px=offset_px, scale_factor=scale_factor, ) @@ -526,13 +517,15 @@ def test_manual_crop_roi_consistent_preview_vs_export(rotation_k, flip_h): @pytest.mark.parametrize("rotation_k", [0, 1, 2, 3]) def test_manual_crop_extracts_same_marker_at_preview_and_export(rotation_k): # End-to-end content parity: a marker filling the crop rect must survive the crop - # identically at full-res (export) and downsampled (preview) resolution. + # identically at full-res (export) and downsampled (preview) resolution. The crop + # rect is now in transformed (display) space, so a centered marker + centered rect + # stay aligned under any 90° rotation. full_h, full_w = 600, 900 full = np.zeros((full_h, full_w, 3), dtype=np.float32) - full[60:180, 90:270] = 1.0 # exactly normalized (0.1..0.3, 0.1..0.3) + full[210:390, 315:585] = 1.0 # centered block, normalized (0.35..0.65) in both axes prev = cv2.resize(full, (300, 200), interpolation=cv2.INTER_AREA) - config = GeometryConfig(manual_crop_rect=(0.1, 0.1, 0.3, 0.3), rotation=rotation_k) + config = GeometryConfig(manual_crop_rect=(0.35, 0.35, 0.65, 0.65), rotation=rotation_k) proc = GeometryProcessor(config) cropper = CropProcessor(config)