diff --git a/crates/geom/src/crop_drag.rs b/crates/geom/src/crop_drag.rs index f600de6d3..3e8f2484f 100644 --- a/crates/geom/src/crop_drag.rs +++ b/crates/geom/src/crop_drag.rs @@ -6,7 +6,7 @@ //! bounds decide how far the drag may go. [`drag_crop`] answers all three in one place so the app, //! CLI, control channel and MCP agree. -use crate::{CropGeometry, Point, Rect}; +use crate::{Affine, CropGeometry, Point, Rect, Vec2}; /// Smallest crop side, as a fraction of the image's shorter side. pub const MIN_CROP_FRACTION: f64 = 0.02; @@ -109,11 +109,21 @@ pub fn drag_crop(start: CropGeometry, handle: CropHandle, grab: Point, pointer: let out = match (handle, aspect) { (CropHandle::Move, _) => { let slide = |cur: Rect, ax: f64, ay: f64| { - let t = bisect(0.0, 1.0, |t| feasible(cur.translate(crate::Vec2::new(ax * t, ay * t)))); - cur.translate(crate::Vec2::new(ax * t, ay * t)) + let t = bisect(0.0, 1.0, |t| feasible(cur.translate(Vec2::new(ax * t, ay * t)))); + cur.translate(Vec2::new(ax * t, ay * t)) }; - let cur = slide(sr, dx, 0.0); - slide(cur, 0.0, dy) + // straightened, go to the nearest place that fits, so the frame slides along tilted edges (#2299) + let nearest = if base.angle != 0.0 { + let from = |r: Rect| nearest_move(r, base.angle, w, h, Vec2::new(dx, dy)).map(|v| r.translate(v)).filter(|r| feasible(*r)); + // a frame at the tolerance's edge on opposite sides moves only once it is inside it + from(sr).or_else(|| shrunk_inside(sr, base.angle, w, h).and_then(from)) + } else { + None + }; + match nearest { + Some(r) => r, + None => slide(slide(sr, dx, 0.0), 0.0, dy), + } } (hd, None) => { let (mx0, mx1, my0, my1) = sides(hd); @@ -202,6 +212,70 @@ pub fn drag_crop(start: CropGeometry, handle: CropHandle, grab: Point, pointer: if [rect.x0, rect.y0, rect.x1, rect.y1].iter().all(|v| v.is_finite()) { CropGeometry { rect, angle: base.angle } } else { start } } +/// The translation (pixels) nearest `want` that keeps the frame `r` inside a `w × h` image turned by `angle` degrees. +fn nearest_move(r: Rect, angle: f64, w: f64, h: f64, want: Vec2) -> Option { + let back = Affine::rotate_about(-angle.to_radians(), Point::new(w / 2.0, h / 2.0)); + let (ex, ey) = (back.apply_vec(Vec2::new(1.0, 0.0)), back.apply_vec(Vec2::new(0.0, 1.0))); + let (nx, ny) = (Vec2::new(ex.x, ey.x), Vec2::new(ex.y, ey.y)); + // never tighter than where the frame is, plus a slack so a zero-width strip survives rounding + let (sx, sy) = (1e-9 * w, 1e-9 * h); + let mut planes = Vec::with_capacity(16); + for c in r.corners() { + let q = back.apply(c); + for (n, b, s) in [(nx, w - q.x, sx), (-nx, q.x, sx), (ny, h - q.y, sy), (-ny, q.y, sy)] { + planes.push((n, b.max(0.0) + s)); + } + } + if planes.iter().all(|(n, b)| n.dot(want) <= *b) { + return Some(want); + } + let lim = 4.0 * w.max(h); + let mut poly = vec![Vec2::new(-lim, -lim), Vec2::new(lim, -lim), Vec2::new(lim, lim), Vec2::new(-lim, lim)]; + for (n, b) in &planes { + poly = clip_half_plane(&poly, *n, *b); + } + poly.iter() + .zip(poly.iter().cycle().skip(1)) + .map(|(a, b)| { + let ab = *b - *a; + let l2 = ab.dot(ab); + let t = if l2 > 0.0 { ((want - *a).dot(ab) / l2).clamp(0.0, 1.0) } else { 0.0 }; + *a + ab * t + }) + .filter(|v| v.x.is_finite() && v.y.is_finite()) + .min_by(|a, b| (*a - want).len().total_cmp(&(*b - want).len())) +} + +/// `r` scaled about its centre by the largest factor down to `1 - 1e-5` that puts it exactly inside a `w × h` image +/// turned by `angle` degrees. +fn shrunk_inside(r: Rect, angle: f64, w: f64, h: f64) -> Option { + let back = Affine::rotate_about(-angle.to_radians(), Point::new(w / 2.0, h / 2.0)); + let scaled = |k: f64| Rect::from_center(r.center(), r.width() * k, r.height() * k); + let inside = |k: f64| { + scaled(k).corners().iter().all(|c| { + let q = back.apply(*c); + q.x >= 0.0 && q.y >= 0.0 && q.x <= w && q.y <= h + }) + }; + let lo = 1.0 - 1e-5; + inside(lo).then(|| scaled(bisect_range(lo, 1.0, inside))) +} + +/// The part of the convex polygon `poly` where `n · v <= b`. +fn clip_half_plane(poly: &[Vec2], n: Vec2, b: f64) -> Vec { + let mut out = Vec::with_capacity(poly.len() + 1); + for (p, q) in poly.iter().zip(poly.iter().cycle().skip(1)) { + let (dp, dq) = (n.dot(*p) - b, n.dot(*q) - b); + if dp <= 0.0 { + out.push(*p); + } + if (dp <= 0.0) != (dq <= 0.0) { + out.push(*p + (*q - *p) * (dp / (dp - dq))); + } + } + out +} + /// Which sides a handle moves: (left, right, top, bottom). fn sides(h: CropHandle) -> (bool, bool, bool, bool) { use CropHandle::*; @@ -480,4 +554,87 @@ mod tests { } } } + + // Scenario: moving a frame that fills a straightened photo's height (issue #2299) + #[test] + fn given_largest_locked_frame_on_a_straightened_photo_when_moved_sideways_then_it_slides_along_the_tilt() { + for angle in [3.0, -10.0] { + let a = 2.0 / 3.0; + let start = crate::crop_fit_angle(W, H, angle, Some(a)); + let c = start.rect.center(); + let g = drag(start, Move, c, p(c.x - 0.5, c.y), Some(a)); + assert!(g.is_within_image(W, H), "{angle}: {:?}", g.rect); + assert!(near(g.rect.width(), start.rect.width()) && near(g.rect.height(), start.rect.height()), "a move keeps the size"); + assert!(g.rect.x0 < start.rect.x0 - 0.2, "{angle}: stuck at {:?} (from {:?})", g.rect, start.rect); + let gc = g.rect.center(); + let further = drag(g, Move, gc, p(gc.x - 0.5, gc.y), Some(a)); + assert!((further.rect.x0 - g.rect.x0).abs() < 1e-4, "{angle}: not at the left side: {:?} → {:?}", g.rect, further.rect); + let back = drag(g, Move, c, p(c.x + 0.5, c.y), Some(a)); + assert!(back.rect.x1 > start.rect.x1 + 0.2, "{angle}: {:?}", back.rect); + } + } + + // Scenario: a straightened move that fits goes exactly where the pointer says + #[test] + fn given_straightened_photo_when_a_small_frame_is_moved_inside_then_it_follows_the_pointer() { + let start = CropGeometry { rect: Rect::new(0.4, 0.4, 0.5, 0.5), angle: 8.0 }; + let g = drag(start, Move, p(0.45, 0.45), p(0.5, 0.42), None); + assert!(near(g.rect.x0, 0.45) && near(g.rect.y0, 0.37), "{:?}", g.rect); + } + + // Scenario: a frame a handle drag left within the tolerance past a tilted edge (review of #2299) + #[test] + fn given_a_frame_just_past_a_tilted_edge_when_moved_then_it_still_slides_and_a_click_changes_nothing() { + let a = 2.0 / 3.0; + let fit = crate::crop_fit_angle(W, H, 3.0, Some(a)); + let k = 1.0 + 5e-7 / fit.rect.height(); + let start = CropGeometry { rect: Rect::from_center(fit.rect.center(), fit.rect.width() * k, fit.rect.height() * k), angle: 3.0 }; + assert!(start.is_within_image(W, H)); + let c = start.rect.center(); + let z = drag(start, Move, c, c, Some(a)).rect; + let s = start.rect; + assert!(near(z.x0, s.x0) && near(z.y0, s.y0) && near(z.x1, s.x1) && near(z.y1, s.y1), "a click inside the frame moves nothing: {z:?}"); + let g = drag(start, Move, c, p(c.x - 0.5, c.y), Some(a)); + assert!(g.rect.x0 < start.rect.x0 - 0.2, "stuck at {:?}", g.rect); + assert!(g.is_within_image(W, H)); + } + + // Scenario: a frame a handle drag enlarged to a tilted edge (review of #2299) + #[test] + fn given_a_frame_enlarged_to_a_tilted_edge_by_a_handle_when_moved_then_it_still_slides() { + let a = 2.0 / 3.0; + for angle in [3.0, 6.0, -10.0, 12.0] { + let fit = crate::crop_fit_angle(W, H, angle, Some(a)); + let small = CropGeometry { rect: Rect::from_center(fit.rect.center(), fit.rect.width() * 0.6, fit.rect.height() * 0.6), angle }; + // the second fills the photo's height, at the tolerance's edge top and bottom + let shrunk = drag(fit, BottomRight, p(0.5, 0.5), p(0.35, 0.35), Some(a)); + for start in [small, shrunk] { + let big = drag(start, BottomRight, p(0.5, 0.5), p(1.5, 1.5), Some(a)); + let c = big.rect.center(); + let g = drag(big, Move, c, p(c.x - 0.5, c.y), Some(a)); + assert!(g.rect.x0 < big.rect.x0 - 0.2, "{angle}: stuck at {:?}", g.rect); + assert!( + g.is_within_image(W, H) + && (g.rect.width() / big.rect.width() - 1.0).abs() < 1e-5 + && (g.rect.height() / big.rect.height() - 1.0).abs() < 1e-5, + "{angle}: {:?}", + g.rect + ); + } + } + } + + // Scenario: a smaller frame pushed sideways past a tilted edge slides along it (#2299) + #[test] + fn given_a_small_frame_pushed_past_a_tilted_edge_then_it_stops_on_the_edge_and_slides_along_it() { + let start = CropGeometry { rect: Rect::new(0.35, 0.35, 0.55, 0.65), angle: 6.0 }; + let c = start.rect.center(); + let g = drag(start, Move, c, p(c.x - 0.2, c.y), None); + assert!(near(g.rect.x0, 0.15) && near(g.rect.y0, 0.35), "{:?}", g.rect); + let (a, b) = (drag(start, Move, c, p(c.x - 0.6, c.y), None), drag(start, Move, c, p(c.x - 1.0, c.y), None)); + for g in [a, b] { + assert!(g.is_within_image(W, H) && g.rect.x0 < 0.01, "{:?}", g.rect); + } + assert!(b.rect.y0 > a.rect.y0 + 0.01 && a.rect.y0 > start.rect.y0, "{:?} then {:?}", a.rect, b.rect); + } } diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 70b2ef7a5..de92318af 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -249,7 +249,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-CROP-RECT | Crop rectangle | P0 | ✅ | `cmd:crop.set`, `cmd:crop.drag`, `crates/geom/src/crop_drag.rs`, `crates/ui-egui/src/panels/detail.rs` | double-click inside the box applies the crop (like Return) | +| LR-CROP-RECT | Crop rectangle | P0 | ✅ | `cmd:crop.set`, `cmd:crop.drag`, `crates/geom/src/crop_drag.rs`, `crates/ui-egui/src/panels/detail.rs` | double-click inside the box applies the crop (like Return); on a straightened photo a moved box slides along the tilted edges to where the pointer asks, so a box that fills the photo's height still reaches its sides (issue #2299) | | LR-CROP-ASPECT | Aspect ratios | P0 | ✅ | `cmd:crop.aspect`, `cmd:crop.rotateAspect`, `crates/ui-egui/src/panels/right.rs` | menu with presets + custom W × H (a preset takes the crop's orientation, portrait after X; a custom ratio is used as entered, issue #2253), visible Lock toggle (also A), button names the active ratio; every handle honours the lock, holding Shift while dragging a handle keeps the frame's proportions for that drag (issue #614), and the drag stops at the image edge instead of shifting the frame (issue #295); "Original" is the photo's own shape (no separate As Shot); the last chosen aspect is saved as a crop tool default, resolving Original per photo and preserving existing crops; opening Crop or browsing never edits a photo: the default applies only when a crop gesture starts, within its single undo step; a crop starts locked to the photo's own shape until the Lock is switched off once, then as last set (#991) | | LR-CROP-STRAIGHTEN | Straighten tool | P0 | ✅ | `cmd:crop.straighten`, `cmd:crop.autoStraighten`, `crates/ui-egui/src/panels/detail.rs` (`rotate_cursor`, `angle_readout`), `crates/ui-egui/src/panels/right.rs` (Angle field), `crates/ui-egui/src/tests_crop_rotate.rs` | Straighten Tool button: drag along a horizon/vertical; double-click or Auto levels automatically. Dragging outside the crop box rotates it: there the pointer is a curved double arrow that bulges away from the side of the box it is on, diagonally off a corner and following a rotated box (`rotate_cursor_turn`, tested by `rotate_cursor_faces_away_from_the_crop_box`) (with ⌘ held, the straighten crosshair), and while rotating the angle shows next to it, kept on the canvas (issue #534); an exact angle goes in the Angle field under Straighten (`cropAngleField`: applies on Return or leaving it, Esc keeps the old angle, takes a decimal comma; a drag on it is one undo step) or by clicking the Straighten value (its hover says so). Every way of setting the angle (rotate handle, Straighten slider, Angle field, `crop.straighten`, `crop.set {angle}`, and the generic `develop.set` / `develop.adjust` / `develop.quickAdjust` / `develop.resetControl` on `crop.angle`, as MCP, the control channel and `lightcraft-cli render --set` use them) keeps the crop if it still fits, else shrinks it about its own centre, keeping its aspect, until it fits; the full photo (or a crop an earlier turn shrank to the largest of its aspect) stays the largest, so it grows back towards 0° (issue #1388), and no other crop ever grows (issue #1794: a crop dragged in from a corner turned into nearly the whole photo) (`CropGeometry::with_angle`, `crates/engine/src/tests_crop_angle.rs`; before issue #742 the generic path left the box, and the export, past the photo with smeared edge pixels in the corners). The dimming outside the box is the photo minus the (rotated) box, drawn as one mesh (`outside_crop_mesh`), so rotating never dims wedges inside the box or doubles them outside (issue #731) | | LR-CROP-AUTO | Auto straighten | P1 | ✅ | `cmd:crop.autoStraighten` | crop-angle leveling from detected horizon/plumb lines (consensus required) | @@ -800,6 +800,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-21 | patch | KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the slider's row; leaving it (even between two frames) or Tab focus alone does not arm them (issue #1355); status unchanged | | 2026-10-20 | minor | LR-PRE-PANEL: the preset applied to a photo is marked again (with its Amount slider) on returning to the photo, until the photo is edited further (#2392); status unchanged (✅) | | 2026-10-20 | patch | LR-VIEW-ZOOM: the 20 ms rule clears a slow-drag verdict only on a drag timed at full size; a drag that drafts small leaves it unchanged, as the code does; status unchanged | +| 2026-10-19 | patch | LR-CROP-RECT: on a straightened photo a moved crop slides along the tilted edges, so a box that fills the photo's height reaches its sides (#2299); status unchanged | | 2026-10-19 | patch | LR-VIEW-ZOOM: the slow slider-drag verdict is described as per photo and control, as the code keys it; status unchanged | | 2026-10-15 | minor | LR-VIEW-ZOOM: a slow slider-drag verdict needs two drags over 20 ms and expires after 30 s (doubling to 240 s); hidden-window frame times are ignored; the slow set forgets its oldest entry, not all (issue #330); status unchanged | | 2026-10-15 | minor | LR-BEHAV-BG-TASKS: Reload from Disk reads its files on a worker thread with a cancellable activity row instead of freezing the window on a selection of raws; its result is committed through the command wrapper (XMP sidecars auto-written as before) and an older reload finishing late no longer overwrites a newer one; status unchanged (✅) |