From 3d5c2ac62baf406feb6cd8ab6aad122eb223656d Mon Sep 17 00:00:00 2001 From: David Mann Date: Sun, 25 Oct 2026 02:15:19 +0100 Subject: [PATCH 1/2] Crop: a click on the frame or a handle never snaps it (#9283) A release at the press point went through the snapping like any drag's, so a click inside an edited frame (or a default-mode click moving the image) or on a handle pulled an edge within reach onto the document's and recorded a box Undo step; a double-click to commit cropped that rect. A click now leaves the frame alone, as a Move-tool click leaves the layer. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/crop_ui.rs | 21 +++++++++++++++++++++ crates/ui-egui/src/snap_ui.rs | 17 +++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/crates/ui-egui/src/crop_ui.rs b/crates/ui-egui/src/crop_ui.rs index 73e9ce5642..4814ad5331 100644 --- a/crates/ui-egui/src/crop_ui.rs +++ b/crates/ui-egui/src/crop_ui.rs @@ -1388,6 +1388,27 @@ mod tests { assert_eq!(app.ui.crop_rect, Some([0.0, 10.0, 40.0, 40.0]), "left edge snaps to the document edge"); } + /// A click on the frame (released where it was pressed) changes nothing, as a Move-tool click + /// doesn't move the layer: snapping the release would pull an edge within reach onto the + /// document's, so a double-click to commit cropped another rect. + #[test] + fn a_click_on_the_frame_or_a_handle_never_snaps_it() { + let frame = [5.0, 10.0, 105.0, 60.0]; + for classic in [true, false] { + // Inside (Classic: moves the frame; default: moves the image), the left edge's handle, the top-left corner's. + for p in [[50.0, 30.0], [5.0, 35.0], [5.0, 10.0]] { + let mut app = app(SampleType::U8); + app.ui.tool_options.crop_shield.classic_mode = classic; + app.ui.extras.snap = true; + app.ui.crop_rect = Some(frame); + app.crop.default_frame = false; + drag(&mut app, &[p], NONE); + assert_eq!(app.ui.crop_rect, Some(frame), "classic {classic}, click at {p:?}"); + assert!(!crate::menus::is_enabled(&app, "edit.undo"), "classic {classic}, click at {p:?}: no box step"); + } + } + } + /// #3145: Edit › Undo while a crop is pending steps the *box*, not the document (Photoshop keeps /// Undo live during a crop), so a moved frame comes back. #[test] diff --git a/crates/ui-egui/src/snap_ui.rs b/crates/ui-egui/src/snap_ui.rs index e19c95a2e7..ce8f2347df 100644 --- a/crates/ui-egui/src/snap_ui.rs +++ b/crates/ui-egui/src/snap_ui.rs @@ -487,6 +487,18 @@ fn follow_reposition(app: &mut PhotocraftApp) { } } +/// A click on a crop frame or one of its handles (released where it was pressed, the frame not +/// moved) changes nothing, as a Move-tool click doesn't move the layer (`canvas::tool_event`): +/// snapping the release would pull an edge within reach onto a nearby one. +fn crop_click(app: &PhotocraftApp, p: [f64; 2]) -> bool { + use crate::crop_ui::CropDrag; + let Some(snap) = app.prefs_rt.snap.as_ref() else { return false }; + let Some(CropDrag::Move { rect, .. } | CropDrag::Resize { rect, .. } | CropDrag::MoveImage { rect, .. }) = app.crop.drag else { + return false; + }; + app.ui.tool == Tool::Crop && p == snap.start && app.ui.crop_rect == Some(rect) +} + /// Snap a tool event's coordinates (called by `canvas::tool_event` before the tools see it). pub fn filter_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers) -> ToolEvent { match ev { @@ -527,6 +539,11 @@ pub fn filter_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifier if app.prefs_rt.snap.is_none() { return ev; } + if crop_click(app, [x, y]) { + app.prefs_rt.snap = None; + app.prefs_rt.snap_lines.clear(); + return ev; + } follow_reposition(app); let disabled = override_for_gesture(app, mods); if let Some(s) = app.prefs_rt.snap.as_mut() { From 0dfbbbf1a93d3aa80feda6fcef26c5eefe86c933 Mon Sep 17 00:00:00 2001 From: David Mann Date: Thu, 29 Oct 2026 17:01:52 +0100 Subject: [PATCH 2/2] Crop: the click guard skips the snap on release again An earlier merge (f35107a8ea, snap_ui.rs moved into canvas/) applied the guard's hunk to the Move branch, whose context lines match the Up branch's. A click has no Move, so its release still snapped the frame and the test a_click_on_the_frame_or_a_handle_never_snaps_it failed. The guard is back on Up, where a release at the press point leaves the frame alone. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/canvas/snap_ui.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/canvas/snap_ui.rs b/crates/ui-egui/src/canvas/snap_ui.rs index 92123bb38b..d52e6f28cb 100644 --- a/crates/ui-egui/src/canvas/snap_ui.rs +++ b/crates/ui-egui/src/canvas/snap_ui.rs @@ -526,11 +526,6 @@ pub fn filter_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifier if app.prefs_rt.snap.is_none() { return ev; } - if crop_click(app, [x, y]) { - app.prefs_rt.snap = None; - app.prefs_rt.snap_lines.clear(); - return ev; - } follow_reposition(app); let disabled = override_for_gesture(app, mods); if let Some(s) = app.prefs_rt.snap.as_mut() { @@ -544,6 +539,11 @@ pub fn filter_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifier if app.prefs_rt.snap.is_none() { return ev; } + if crop_click(app, [x, y]) { + app.prefs_rt.snap = None; + app.prefs_rt.snap_lines.clear(); + return ev; + } follow_reposition(app); let disabled = override_for_gesture(app, mods); if let Some(s) = app.prefs_rt.snap.as_mut() {