Merge PR #617

# Conflicts:
#	docs/parity-checklist.md

crates/ui-egui/src/headless.rs

@@ -1917,6 +1917,49 @@ mod tests {
         assert!(ids(&mut h, "notice:previewOnly:").is_empty());
     }
 
+    #[test]
+    fn crop_default_browsing_is_read_only_and_drag_is_one_edit() {
+        let mut h = demo([1200.0, 800.0]);
+        let photos: Vec<_> = h.app.session.visible_cloned().into_iter().filter(|id| !h.app.session.catalog.photo(*id).unwrap().is_edited()).collect();
+        h.app.run("library.select", json!({"ids": [photos[0].0]})).unwrap();
+        h.app.ui.right = crate::state::RightPanel::Crop;
+        h.app.run("crop.aspect", json!({"aspect": "1x1"})).unwrap();
+        let saved = serde_json::to_value(&h.app.ui).unwrap();
+        for &photo in &photos[1..3] {
+            h.app.run("library.select", json!({"ids": [photo.0]})).unwrap();
+            let before = h.app.session.catalog.photo(photo).unwrap().clone();
+            let undo = h.app.session.undo.len();
+            h.settle(SETTLE);
+            let after = h.app.session.catalog.photo(photo).unwrap();
+            assert_eq!(after.develop, before.develop);
+            assert_eq!(after.history.len(), before.history.len());
+            assert!(!after.is_edited());
+            assert_eq!(h.app.session.undo.len(), undo);
+            assert_eq!(h.app.crop_tool_aspect(), Some((100, 100)));
+        }
+        let photo = photos[2];
+        let before = h.app.session.develop_of(photo).unwrap();
+        let undo = h.app.session.undo.len();
+        h.app.run("develop.beginInteraction", json!({"label": "Crop"})).unwrap();
+        h.app.begin_crop().unwrap();
+        h.app.run("crop.drag", json!({"handle": "bottomRight", "from": [0.8, 0.8], "to": [0.7, 0.7]})).unwrap();
+        h.app.run("develop.endInteraction", json!({})).unwrap();
+        assert_eq!(h.app.session.develop_of(photo).unwrap().crop.aspect, Some((100, 100)));
+        assert_eq!(h.app.session.undo.len(), undo + 1);
+        h.app.run("edit.undo", json!({})).unwrap();
+        assert_eq!(h.app.session.develop_of(photo).unwrap(), before);
+        h.app.ui = serde_json::from_value(saved).unwrap();
+        assert_eq!(h.app.crop_tool_aspect(), Some((100, 100)));
+        h.app.run("crop.aspect", json!({"aspect": "toggle"})).unwrap();
+        assert_eq!(h.app.ui.crop_default_aspect, Some(json!("free")));
+        assert_eq!(h.app.session.develop_of(photo).unwrap(), before);
+        h.app.ui.crop_default_aspect = Some(json!("original"));
+        let p = h.app.session.catalog.photo(photo).unwrap();
+        assert_eq!(h.app.crop_tool_aspect(), Some((p.width * 100, p.height * 100)));
+        h.settle(SETTLE);
+        assert_eq!(h.app.session.develop_of(photo).unwrap(), before);
+    }
+
     /// While cropping: O cycles the guides, ⇧O mirrors them, A locks / unlocks the aspect.
     #[test]
     fn crop_keys() {

crates/ui-egui/src/lib.rs

@@ -476,7 +476,26 @@ impl LightcraftApp {
             }
             return r;
         }
+        let params = if id == "crop.aspect"
+            && params.get("aspect").and_then(Value::as_str) == Some("toggle")
+            && self.crop_tool_aspect().is_some()
+            && self.session.active().and_then(|id| self.session.develop_of(id)).is_some_and(|d| d.crop == lightcraft_develop::Crop::default())
+        {
+            serde_json::json!({"aspect": "free"})
+        } else {
+            params
+        };
         let r = self.session.execute(id, &params).map_err(|e| e.to_string());
+        if r.is_ok() && matches!(id, "crop.aspect" | "crop.rotateAspect") {
+            self.ui.crop_default_aspect = if id == "crop.aspect" && params.get("aspect").and_then(Value::as_str) == Some("original") {
+                Some(serde_json::json!("original"))
+            } else {
+                self.session
+                    .active()
+                    .and_then(|photo| self.session.develop_of(photo))
+                    .map(|d| d.crop.aspect.map_or(serde_json::json!("free"), |(w, h)| serde_json::json!([w as f64 / 100.0, h as f64 / 100.0])))
+            };
+        }
         if r.is_ok() && id == "mask.adjust" {
             // Judge local adjustments on the photo, without the selection overlay obscuring them.
             // Keep it hidden after release; O / the overlay eye can show it again.
@@ -526,6 +545,46 @@ impl LightcraftApp {
         r
     }
 
+    /// The remembered ratio belongs to the tool until the user starts editing the crop.
+    pub(crate) fn crop_tool_aspect(&self) -> Option<(u32, u32)> {
+        let photo = self.session.active()?;
+        let d = self.session.develop_of(photo)?;
+        if d.crop != lightcraft_develop::Crop::default() {
+            return d.crop.aspect;
+        }
+        match self.ui.crop_default_aspect.as_ref()? {
+            Value::String(s) if s == "original" => {
+                let p = self.session.catalog.photo(photo)?;
+                let (w, h) = (p.width.max(1).saturating_mul(100), p.height.max(1).saturating_mul(100));
+                Some(if d.orientation.swaps_axes() { (h, w) } else { (w, h) })
+            }
+            Value::Array(a) => {
+                let number = |i: usize| {
+                    let v = a.get(i)?.as_f64()? * 100.0;
+                    (v.is_finite() && v >= 1.0 && v <= f64::from(u32::MAX)).then_some(v.round() as u32)
+                };
+                Some((number(0)?, number(1)?))
+            }
+            _ => None,
+        }
+    }
+
+    /// Called only after a crop gesture begins, inside its undo transaction.
+    pub(crate) fn begin_crop(&mut self) -> Result<(), String> {
+        if self.session.interaction.is_some()
+            && self.session.active().and_then(|id| self.session.develop_of(id)).is_some_and(|d| d.crop == lightcraft_develop::Crop::default())
+            && let Some(aspect) = self.ui.crop_default_aspect.clone()
+            && aspect != serde_json::json!("free")
+            && let Err(e) = self.session.execute("crop.aspect", &serde_json::json!({"aspect": aspect}))
+        {
+            // A remembered value the command refuses (a hand-edited settings file) would refuse
+            // every crop drag on an untouched photo: forget it and let this drag go ahead.
+            self.ui.crop_default_aspect = None;
+            self.ui.status = e.to_string();
+        }
+        Ok(())
+    }
+
     /// Show a transient toast at the bottom of the canvas (like the reference app's HUD).
     /// Advance a running slideshow (wrapping around at the end).
     fn slideshow_tick(&mut self, ctx: &egui::Context) {

crates/ui-egui/src/panels/detail.rs

@@ -1346,7 +1346,13 @@ fn crop_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons
     if resp.drag_started()
         && let Some(q) = resp.interact_pointer_pos()
     {
-        let _ = app.run("develop.beginInteraction", json!({"label": "Crop"}));
+        if let Err(e) = app.run("develop.beginInteraction", json!({"label": "Crop"})).and_then(|_| app.begin_crop().map(|_| serde_json::Value::Null))
+        {
+            app.ui.status = e;
+            let _ = app.session.cancel_interaction();
+            return;
+        }
+        let d = app.session.develop_of(id).unwrap_or_default();
         app.gesture = Some(match handles.iter().position(|h| h.distance(q) < 12.0) {
             Some(h) => Gesture::CropHandle { handle: h as u8, start: d.crop.geometry.rect, angle: d.crop.geometry.angle },
             None if inside(q) => Gesture::CropHandle { handle: 8, start: d.crop.geometry.rect, angle: d.crop.geometry.angle },

crates/ui-egui/src/panels/right.rs

@@ -144,7 +144,7 @@ fn crop(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
         });
         ui.horizontal(|ui| {
             ui.label(crate::i18n::tr("Aspect Ratio"));
-            let cur = aspect_label(d.crop.aspect, original);
+            let cur = aspect_label(app.crop_tool_aspect(), original);
             let t = Tokens::get(ui.ctx());
             let r = crate::widgets::dropdown(ui, "cropAspect", crate::i18n::tr(&cur), t.font(12.5), t.text);
 
@@ -157,7 +157,7 @@ fn crop(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) {
                 ui.separator();
                 custom_aspect(app, ui);
             });
-            let locked = d.crop.aspect.is_some();
+            let locked = app.crop_tool_aspect().is_some();
             if text_button(ui, "cropLock", if locked { "Locked" } else { "Lock" }, locked).clicked() {
                 let _ = app.run("crop.aspect", json!({"aspect": "toggle"}));
             }

crates/ui-egui/src/state.rs

@@ -377,6 +377,8 @@ pub struct UiState {
     /// Mirroring of the triangle / spiral crop guides (0..4).
     pub crop_overlay_orient: u8,
     pub crop_overlay: CropOverlay,
+    /// Last explicitly chosen aspect, retained across photos and app launches.
+    pub crop_default_aspect: Option<serde_json::Value>,
     pub show_filenames: bool,
     /// What the square grid's caption shows: `filename`, `exposure` (shutter · aperture · ISO ·
     /// focal length) or `date`.
@@ -882,6 +884,7 @@ impl Default for UiState {
             mask_overlay_opacity: 50.0,
             mask_pins: true,
             crop_overlay: CropOverlay::Thirds,
+            crop_default_aspect: None,
             crop_overlay_orient: 0,
             show_filenames: true,
             grid_info: "filename".into(),

crates/ui-egui/src/tests_masking.rs

@@ -478,6 +478,54 @@ fn command_drag_straightens_in_crop() {
     h.settle(SETTLE);
 }
 
+#[test]
+fn crop_drag_activates_the_remembered_ratio_and_undo_restores_the_photo() {
+    let mut h = detail("panel.crop");
+    h.app.ui.crop_default_aspect = Some(json!([1, 1]));
+    h.settle(SETTLE);
+    let before = develop(&h);
+    let undo = h.app.session.undo.len();
+    assert!(before.crop.aspect.is_none());
+    let r = h.request(
+        "ui.pointer",
+        json!({"events": [
+            {"kind": "down", "x": 0.995, "y": 0.995},
+            {"kind": "drag", "x": 0.85, "y": 0.85},
+            {"kind": "drag", "x": 0.75, "y": 0.75},
+            {"kind": "up", "x": 0.75, "y": 0.75}
+        ]}),
+        T,
+    );
+    assert_eq!(r["ok"], true, "{r}");
+    h.settle(SETTLE);
+    assert_eq!(develop(&h).crop.aspect, Some((100, 100)));
+    assert_eq!(h.app.session.undo.len(), undo + 1);
+    exec(&mut h, "edit.undo", json!({}));
+    assert_eq!(develop(&h), before);
+}
+
+/// A remembered ratio the crop command refuses (a hand-edited settings file) is forgotten, and the
+/// drag still crops: it must not block every crop of an untouched photo.
+#[test]
+fn a_bad_remembered_ratio_is_dropped_and_the_drag_still_crops() {
+    let mut h = detail("panel.crop");
+    h.app.ui.crop_default_aspect = Some(json!([0, -3]));
+    h.settle(SETTLE);
+    let before = develop(&h);
+    // a rotating drag outside the box (any crop drag starts cropping); the release on its own
+    for events in [
+        json!([{"kind": "down", "x": -0.05, "y": 0.2}, {"kind": "drag", "x": -0.05, "y": 0.25}, {"kind": "drag", "x": -0.05, "y": 0.3}]),
+        json!([{"kind": "up", "x": -0.05, "y": 0.3}]),
+    ] {
+        let r = h.request("ui.pointer", json!({ "events": events }), T);
+        assert_eq!(r["ok"], true, "{r}");
+        h.settle(SETTLE);
+    }
+    assert_eq!(h.app.ui.crop_default_aspect, None, "the bad value is forgotten");
+    assert_ne!(develop(&h).crop, before.crop, "the drag cropped");
+    assert_eq!(develop(&h).crop.aspect, None);
+}
+
 #[test]
 fn double_click_in_crop_box_applies_the_crop() {
     let mut h = detail("panel.crop");

docs/parity-checklist.md

@@ -245,7 +245,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-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-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); 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 |
 | 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 to the largest crop of its aspect inside the 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) |
 | 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); setting `crop.angle` through `develop.set` fits the crop inside the photo as Straighten does (issue #742) |