From 584a26085cc252f1273aa996e1ddbdc4b8892cbd Mon Sep 17 00:00:00 2001 From: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Date: Sat, 10 Oct 2026 13:28:58 +0200 Subject: [PATCH] Hold Shift while dragging a crop handle to keep proportions crop.drag gains an optional keepRatio param that constrains the drag to the frame's current aspect even when the lock is off. The crop panel sends it while Shift is held. Closes #614 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/develop.rs | 6 ++++-- crates/engine/src/tests.rs | 30 +++++++++++++++++++++++++++++ crates/ui-egui/src/panels/detail.rs | 4 +++- docs/parity.md | 2 +- 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/develop.rs b/crates/engine/src/cmd/develop.rs index 33570f47..6ff0bf54 100644 --- a/crates/engine/src/cmd/develop.rs +++ b/crates/engine/src/cmd/develop.rs @@ -501,7 +501,7 @@ pub fn specs() -> Vec { "Drag Crop Handle", [], None, - "{handle: topLeft|topRight|bottomRight|bottomLeft|top|right|bottom|left|move | 0..8, from: [x,y], to: [x,y] (normalized, straightened frame), start?: [x0,y0,x1,y1] (default: the current crop)} — drags a crop handle; stops at the image edge, keeps the opposite edges, honours the aspect lock", + "{handle: topLeft|topRight|bottomRight|bottomLeft|top|right|bottom|left|move | 0..8, from: [x,y], to: [x,y] (normalized, straightened frame), start?: [x0,y0,x1,y1] (default: the current crop), keepRatio?: bool (default false)} — drags a crop handle; stops at the image edge, keeps the opposite edges, honours the aspect lock; keepRatio holds the frame's current proportions for this drag even with the lock off (the UI sends it while Shift is held)", has_active, |s, p| { let id = active(s, "crop.drag")?; @@ -526,11 +526,13 @@ pub fn specs() -> Vec { } _ => return Err(bad("crop.drag", "`start` needs 4 numbers")), }; + let keep_ratio = bool_or(p, "keepRatio", false); edit(s, "crop.drag", "Crop", |d| { let start = CropGeometry { rect: given.unwrap_or(d.crop.geometry.rect), angle: d.crop.geometry.angle }; // A locked crop keeps the shape it has: the lock stores a width:height tuple whose // orientation is reinterpreted per image, so the frame itself is the truth. - let ratio = d.crop.aspect.and(Some(start.rect_px(w, h).aspect())).filter(|a| a.is_finite() && *a > 0.0); + // `keepRatio` (Shift held) asks for the same thing for one drag without touching the lock. + let ratio = (d.crop.aspect.is_some() || keep_ratio).then(|| start.rect_px(w, h).aspect()).filter(|a| a.is_finite() && *a > 0.0); d.crop.geometry = drag_crop(start, handle, from, to, w, h, ratio); Ok(()) }) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index cea8475e..c8db7f43 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -530,6 +530,36 @@ fn crop_drag_keeps_a_locked_ratio_on_every_handle() { } } +// Feature: Shift while dragging keeps the current proportions (issue 614) +#[test] +fn crop_drag_keep_ratio_holds_the_shape_with_the_lock_off() { + let mut s = demo(); + s.execute("crop.set", &json!({"rect": [0.1, 0.2, 0.7, 0.6]})).unwrap(); + assert!(active_dev(&s).crop.aspect.is_none(), "lock is off"); + let r = active_dev(&s).crop.geometry.rect; + let want = r.aspect(); + for (handle, to) in [("left", [r.x0 + 0.1, 0.5]), ("top", [0.5, r.y0 + 0.05]), ("bottomRight", [1.3, 1.3]), ("right", [0.9, 0.5])] { + s.execute("crop.drag", &json!({"handle": handle, "from": [0.5, 0.5], "to": to, "start": [r.x0, r.y0, r.x1, r.y1], "keepRatio": true})) + .unwrap(); + let g = active_dev(&s).crop.geometry.rect; + assert!((g.aspect() - want).abs() < 1e-6, "{handle}: {} vs {want}", g.aspect()); + assert!(active_dev(&s).crop.aspect.is_none(), "the lock toggle is untouched"); + } +} + +#[test] +fn crop_drag_without_keep_ratio_and_lock_off_is_free() { + let mut s = demo(); + s.execute("crop.set", &json!({"rect": [0.1, 0.2, 0.7, 0.6]})).unwrap(); + let r = active_dev(&s).crop.geometry.rect; + let want = r.aspect(); + s.execute("crop.drag", &json!({"handle": "right", "from": [0.7, 0.4], "to": [0.9, 0.4], "start": [r.x0, r.y0, r.x1, r.y1]})).unwrap(); + assert!((active_dev(&s).crop.geometry.rect.aspect() - want).abs() > 1e-3, "ratio changes freely"); + s.execute("crop.drag", &json!({"handle": "right", "from": [0.7, 0.4], "to": [0.9, 0.4], "start": [r.x0, r.y0, r.x1, r.y1], "keepRatio": false})) + .unwrap(); + assert!((active_dev(&s).crop.geometry.rect.aspect() - want).abs() > 1e-3, "keepRatio false is the same"); +} + #[test] fn crop_drag_rejects_unknown_handles_and_bad_points() { let mut s = demo(); diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index c3678ae2..b475a771 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -1325,9 +1325,11 @@ fn crop_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons // `lightcraft_geom::drag_crop`; the UI only reports where the pointer started and is. let n = to_straight(map.norm(q), angle, frame); let orig = ui.input(|i| i.pointer.press_origin()).map(|q0| to_straight(map.norm(q0), angle, frame)).unwrap_or(n); + // Shift keeps the current proportions for this drag only; the Lock toggle is untouched. + let keep_ratio = ui.input(|i| i.modifiers.shift); let _ = app.run( "crop.drag", - json!({"handle": handle, "from": [orig.x, orig.y], "to": [n.x, n.y], "start": [start.x0, start.y0, start.x1, start.y1]}), + json!({"handle": handle, "from": [orig.x, orig.y], "to": [n.x, n.y], "start": [start.x0, start.y0, start.x1, start.y1], "keepRatio": keep_ratio}), ); } Some(Gesture::CropRotate { start_angle, a0 }) => { diff --git a/docs/parity.md b/docs/parity.md index d0bdc35c..6f74651c 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -311,7 +311,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | 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-ASPECT | Aspect ratios | P0 | ✅ | `cmd:crop.aspect`, `cmd:crop.rotateAspect`, `crates/ui-egui/src/panels/right.rs` | menu with presets + custom W × H, visible Lock toggle (also A), button names the active ratio; every handle honours the lock 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) | +| 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, 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) | | 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 (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) | | LR-CROP-AUTO | Auto straighten | P1 | ✅ | `cmd:crop.autoStraighten` | crop-angle leveling from detected horizon/plumb lines (consensus required) | | LR-CROP-ANGLE | Angle slider | P0 | ✅ | `ctl:crop.angle`, `crates/ui-egui/src/panels/right.rs` | the Straighten slider; an exact angle can also be typed in the Angle field under it (see LR-CROP-STRAIGHTEN) |