From d63ce8e5f86ec88ad9b8ae58efcf89f45645af40 Mon Sep 17 00:00:00 2001 From: chordpli Date: Mon, 19 Oct 2026 19:18:11 +0900 Subject: [PATCH 1/5] LR-CROP-RECT: a moved crop slides along a straightened photo's tilted edges (#2299) On a straightened photo, a crop that fills the photo's height could not be moved sideways: the move went across and then down, and no straight sideways step stays inside the turned photo. The frame now goes to the place nearest the pointer's that keeps it inside, so it slides along the tilted edges to the photo's side. Unstraightened, that place is the old across-then-down clamp. --- crates/geom/src/crop_drag.rs | 99 ++++++++++++++++++++++++++++++++++-- docs/parity-checklist.md | 2 +- 2 files changed, 95 insertions(+), 6 deletions(-) diff --git a/crates/geom/src/crop_drag.rs b/crates/geom/src/crop_drag.rs index f600de6d3..ee61c613e 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,18 @@ 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, the frame goes to the place nearest the pointer's that keeps it inside, + // so it slides along the tilted edges: across-then-down stops a frame that fills the + // photo's height dead, as no straight sideways step stays inside (issue #2299). + let nearest = (base.angle != 0.0) + .then(|| nearest_move(sr, base.angle, w, h, Vec2::new(dx, dy))) + .flatten() + .map(|v| sr.translate(v)) + .filter(|r| feasible(*r)); + nearest.unwrap_or_else(|| slide(slide(sr, dx, 0.0), 0.0, dy)) } (hd, None) => { let (mx0, mx1, my0, my1) = sides(hd); @@ -202,6 +209,59 @@ 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: every corner of the moved frame, turned back into the image, must lie in +/// `0..w × 0..h`. That is 16 half-planes, a convex polygon of translations; `None` when it's empty. +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)); + // how a translation `v` moves a turned-back point: x by `nx · v`, y by `ny · v` + 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)); + // half-planes `n · v <= b`, with a slack under `is_within_image`'s tolerance: a frame that fills + // the photo's height leaves a strip of zero width, which rounding would otherwise empty + let (sx, sy) = (1e-7 * w, 1e-7 * h); + let mut planes = Vec::with_capacity(16); + for c in r.corners() { + let q = back.apply(c); + planes.extend([(nx, w - q.x + sx), (-nx, q.x + sx), (ny, h - q.y + sy), (-ny, q.y + sy)]); + } + if planes.iter().all(|(n, b)| n.dot(want) <= *b) { + return Some(want); + } + // the polygon, clipped from a box well past any move that matters (a move is at most 2× the image) + 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); + } + // outside it, the nearest translation lies on its boundary + 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())) +} + +/// 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 +540,33 @@ 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"); + // it reaches the photo's left side: no frame of this size fits further left + 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); + // and back: the pointer's own path is free again + 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); + } } diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 02f5bf08f..f0408c345 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) | From 4bd40cda60a1bc4cb8af618dc3ffbb13f798d418 Mon Sep 17 00:00:00 2001 From: chordpli Date: Mon, 19 Oct 2026 19:56:23 +0900 Subject: [PATCH 2/5] LR-CROP-RECT: a crop just past a tilted edge still moves, and a click leaves it be (#2299) A handle drag can leave the frame up to is_within_image's tolerance past a tilted edge. The move's bounds were tighter than that, so such a frame was stuck again, and a click inside it moved it. The bounds are now never tighter than where the frame is. A test pins how a smaller frame pushed past a tilted edge slides along it, and the parity checklist's revision history notes the fix. --- crates/geom/src/crop_drag.rs | 61 +++++++++++++++++++++++++++++------- docs/parity-checklist.md | 1 + 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/crates/geom/src/crop_drag.rs b/crates/geom/src/crop_drag.rs index ee61c613e..e1f307c79 100644 --- a/crates/geom/src/crop_drag.rs +++ b/crates/geom/src/crop_drag.rs @@ -115,12 +115,11 @@ pub fn drag_crop(start: CropGeometry, handle: CropHandle, grab: Point, pointer: // Straightened, the frame goes to the place nearest the pointer's that keeps it inside, // so it slides along the tilted edges: across-then-down stops a frame that fills the // photo's height dead, as no straight sideways step stays inside (issue #2299). - let nearest = (base.angle != 0.0) - .then(|| nearest_move(sr, base.angle, w, h, Vec2::new(dx, dy))) - .flatten() - .map(|v| sr.translate(v)) - .filter(|r| feasible(*r)); - nearest.unwrap_or_else(|| slide(slide(sr, dx, 0.0), 0.0, dy)) + let nearest = if base.angle != 0.0 { nearest_move(sr, base.angle, w, h, Vec2::new(dx, dy)) } else { None }; + match nearest.map(|v| sr.translate(v)).filter(|r| feasible(*r)) { + Some(r) => r, + None => slide(slide(sr, dx, 0.0), 0.0, dy), + } } (hd, None) => { let (mx0, mx1, my0, my1) = sides(hd); @@ -211,19 +210,24 @@ pub fn drag_crop(start: CropGeometry, handle: CropHandle, grab: Point, pointer: /// The translation (pixels) nearest `want` that keeps the frame `r` inside a `w × h` image turned /// by `angle` degrees: every corner of the moved frame, turned back into the image, must lie in -/// `0..w × 0..h`. That is 16 half-planes, a convex polygon of translations; `None` when it's empty. +/// `0..w × 0..h`. That is 16 half-planes, a convex polygon of translations. The frame where it is +/// always counts as inside (it may sit within `is_within_image`'s tolerance past an edge, as a +/// handle drag leaves it), so the polygon holds `0` and a move of nothing moves nothing. 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)); // how a translation `v` moves a turned-back point: x by `nx · v`, y by `ny · v` 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)); - // half-planes `n · v <= b`, with a slack under `is_within_image`'s tolerance: a frame that fills - // the photo's height leaves a strip of zero width, which rounding would otherwise empty - let (sx, sy) = (1e-7 * w, 1e-7 * h); + // half-planes `n · v <= b`, never tighter than where the frame is, with a slack far under + // `is_within_image`'s tolerance: a frame that fills the photo's height leaves a strip of zero + // width, which rounding would otherwise empty + 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); - planes.extend([(nx, w - q.x + sx), (-nx, q.x + sx), (ny, h - q.y + sy), (-ny, q.y + sy)]); + 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); @@ -569,4 +573,39 @@ mod tests { 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)); + // 5e-7 of the height past the top and bottom edges: still inside `is_within_image`'s tolerance + 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 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(); + // short of the edge it follows the pointer straight across + 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); + // past it, the frame stays on the photo's (tilted) left edge and moves along it, further the + // further the pointer goes, as the place nearest the pointer's + 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 f0408c345..7ce5779fe 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -798,6 +798,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| | 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 (✅) | From f443bed56fc142098934ff386cca53f05753418b Mon Sep 17 00:00:00 2001 From: chordpli Date: Tue, 20 Oct 2026 07:20:07 +0900 Subject: [PATCH 3/5] LR-CROP-RECT: trim the crop move comments (#2299) --- crates/geom/src/crop_drag.rs | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/crates/geom/src/crop_drag.rs b/crates/geom/src/crop_drag.rs index e1f307c79..add22a136 100644 --- a/crates/geom/src/crop_drag.rs +++ b/crates/geom/src/crop_drag.rs @@ -112,9 +112,7 @@ pub fn drag_crop(start: CropGeometry, handle: CropHandle, grab: Point, pointer: 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)) }; - // Straightened, the frame goes to the place nearest the pointer's that keeps it inside, - // so it slides along the tilted edges: across-then-down stops a frame that fills the - // photo's height dead, as no straight sideways step stays inside (issue #2299). + // straightened, go to the nearest place that fits, so the frame slides along tilted edges (#2299) let nearest = if base.angle != 0.0 { nearest_move(sr, base.angle, w, h, Vec2::new(dx, dy)) } else { None }; match nearest.map(|v| sr.translate(v)).filter(|r| feasible(*r)) { Some(r) => r, @@ -208,19 +206,12 @@ 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: every corner of the moved frame, turned back into the image, must lie in -/// `0..w × 0..h`. That is 16 half-planes, a convex polygon of translations. The frame where it is -/// always counts as inside (it may sit within `is_within_image`'s tolerance past an edge, as a -/// handle drag leaves it), so the polygon holds `0` and a move of nothing moves nothing. +/// 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)); - // how a translation `v` moves a turned-back point: x by `nx · v`, y by `ny · v` 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)); - // half-planes `n · v <= b`, never tighter than where the frame is, with a slack far under - // `is_within_image`'s tolerance: a frame that fills the photo's height leaves a strip of zero - // width, which rounding would otherwise empty + // 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() { @@ -232,13 +223,11 @@ fn nearest_move(r: Rect, angle: f64, w: f64, h: f64, want: Vec2) -> Option if planes.iter().all(|(n, b)| n.dot(want) <= *b) { return Some(want); } - // the polygon, clipped from a box well past any move that matters (a move is at most 2× the image) 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); } - // outside it, the nearest translation lies on its boundary poly.iter() .zip(poly.iter().cycle().skip(1)) .map(|(a, b)| { @@ -555,12 +544,10 @@ mod tests { 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"); - // it reaches the photo's left side: no frame of this size fits further left 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); - // and back: the pointer's own path is free again 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); } @@ -579,7 +566,6 @@ mod tests { 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)); - // 5e-7 of the height past the top and bottom edges: still inside `is_within_image`'s tolerance 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)); @@ -597,11 +583,8 @@ mod tests { 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(); - // short of the edge it follows the pointer straight across 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); - // past it, the frame stays on the photo's (tilted) left edge and moves along it, further the - // further the pointer goes, as the place nearest the pointer's 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); From 89a35ded91a4b1e9d10bc8f364c946aeaa934f28 Mon Sep 17 00:00:00 2001 From: chordpli Date: Tue, 20 Oct 2026 09:01:45 +0900 Subject: [PATCH 4/5] LR-CROP-RECT: a crop a handle drag left at a tilted edge still moves (#2299) A handle drag stops where is_within_image's 1e-6 tolerance ends, so a corner can sit right at it. The move solve then let that corner go 1e-9 further, the result failed the check, and the across-then-down fallback left the frame stuck. Solve first with every corner kept within half the tolerance, and only then where it is. --- crates/geom/src/crop_drag.rs | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/geom/src/crop_drag.rs b/crates/geom/src/crop_drag.rs index add22a136..c868e156e 100644 --- a/crates/geom/src/crop_drag.rs +++ b/crates/geom/src/crop_drag.rs @@ -208,16 +208,22 @@ pub fn drag_crop(start: CropGeometry, handle: CropHandle, grab: Point, pointer: /// 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 { + // first within half of `is_within_image`'s tolerance, so a frame a handle drag left at its edge still moves + solve_move(r, angle, (w, h), want, 5e-7).or_else(|| solve_move(r, angle, (w, h), want, f64::INFINITY)) +} + +/// [`nearest_move`] with each corner kept at most `pull` (a fraction of the size) past the image, or where it is. +fn solve_move(r: Rect, angle: f64, (w, h): (f64, f64), want: Vec2, pull: f64) -> 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 + // 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)); + for (n, b, s, size) in [(nx, w - q.x, sx, w), (-nx, q.x, sx, w), (ny, h - q.y, sy, h), (-ny, q.y, sy, h)] { + planes.push((n, b.max(0.0).min(b + pull * size) + s)); } } if planes.iter().all(|(n, b)| n.dot(want) <= *b) { @@ -578,6 +584,25 @@ mod tests { 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 }; + let big = drag(small, BottomRight, p(small.rect.x1, small.rect.y1), 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) && near(g.rect.width(), big.rect.width()) && near(g.rect.height(), big.rect.height()), + "{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() { From c28a537a1558aff9d6978e8f6aa6cfc1cc1a757c Mon Sep 17 00:00:00 2001 From: chordpli Date: Tue, 20 Oct 2026 09:53:39 +0900 Subject: [PATCH 5/5] LR-CROP-RECT: a crop a handle drag filled to the tilted edges still moves (#2299) The E2E showed the previous fix missed the common case: enlarging a locked crop until it stops leaves a frame that fills the photo's height, with its top and bottom both at the edge of is_within_image's 1e-6 tolerance. No translation keeps both there with room to spare, so the move stayed stuck. When the frame can't move as it is, solve again with it shrunk about its centre by the least that puts it strictly inside (at most 1e-5 of its size, aspect unchanged). This also covers a frame at the edge on one side, so the half-tolerance first pass is dropped and a click moves nothing again. --- crates/geom/src/crop_drag.rs | 61 ++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 20 deletions(-) diff --git a/crates/geom/src/crop_drag.rs b/crates/geom/src/crop_drag.rs index c868e156e..3e8f2484f 100644 --- a/crates/geom/src/crop_drag.rs +++ b/crates/geom/src/crop_drag.rs @@ -113,8 +113,14 @@ pub fn drag_crop(start: CropGeometry, handle: CropHandle, grab: Point, pointer: cur.translate(Vec2::new(ax * t, ay * t)) }; // straightened, go to the nearest place that fits, so the frame slides along tilted edges (#2299) - let nearest = if base.angle != 0.0 { nearest_move(sr, base.angle, w, h, Vec2::new(dx, dy)) } else { None }; - match nearest.map(|v| sr.translate(v)).filter(|r| feasible(*r)) { + 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), } @@ -208,22 +214,16 @@ pub fn drag_crop(start: CropGeometry, handle: CropHandle, grab: Point, pointer: /// 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 { - // first within half of `is_within_image`'s tolerance, so a frame a handle drag left at its edge still moves - solve_move(r, angle, (w, h), want, 5e-7).or_else(|| solve_move(r, angle, (w, h), want, f64::INFINITY)) -} - -/// [`nearest_move`] with each corner kept at most `pull` (a fraction of the size) past the image, or where it is. -fn solve_move(r: Rect, angle: f64, (w, h): (f64, f64), want: Vec2, pull: f64) -> 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)); - // plus a slack so a zero-width strip survives rounding + // 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, size) in [(nx, w - q.x, sx, w), (-nx, q.x, sx, w), (ny, h - q.y, sy, h), (-ny, q.y, sy, h)] { - planes.push((n, b.max(0.0).min(b + pull * size) + s)); + 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) { @@ -246,6 +246,21 @@ fn solve_move(r: Rect, angle: f64, (w, h): (f64, f64), want: Vec2, pull: f64) -> .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); @@ -591,15 +606,21 @@ mod tests { 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 }; - let big = drag(small, BottomRight, p(small.rect.x1, small.rect.y1), 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) && near(g.rect.width(), big.rect.width()) && near(g.rect.height(), big.rect.height()), - "{angle}: {:?}", - g.rect - ); + // 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 + ); + } } }