Crop tool: a resized frame snaps its handle, not the pointer (#3638) (#3640)

With Auto Center Preview off, or in Classic Mode, dragging a crop edge or
corner snapped the pointer, which grabs the handle up to the 8 px grab
radius away from it. The snap line showed on the target while the frame
edge stopped as far from it as the grab offset, so recropping toward the
pixels a Delete Cropped Pixels-off crop kept left a transparent strip
along the top and sides (or cut a few pixels off).

The crop handle snap (snap_ui::crop_handle) now covers every resize: it
snaps the handle of the frame crop_ui would make and moves the pointer by
the difference, 1:1 without Auto Center Preview and holding the ratio on
a corner as before.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
David Mann
2026-10-12 04:10:50 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 55b3301384
commit b623db1ed7
2 changed files with 113 additions and 16 deletions
+85
View File
@@ -820,4 +820,89 @@ mod tests {
}
}
}
/// #1919: with Delete Cropped Pixels off, a second crop's handle dragged out toward the hidden
/// pixels snaps exactly onto their edge (a layer edge), however far from the handle's centre
/// the press grabbed it, in both modes, with and without Auto Center Preview: the committed
/// crop has no transparent strip.
#[test]
fn a_recrop_handle_snaps_exactly_to_the_hidden_pixels_edge() {
// The image 200 × 100 cropped to [40, 20, 140, 70]: its pixels stay at [-40, -20, 160, 80].
let hidden = [-40.0, -20.0, 160.0, 80.0];
for classic in [false, true] {
for auto in [true, false] {
for (hx, hy) in [(1i8, 0i8), (0, -1), (-1, -1), (1, 1)] {
// A corner also with the image's 2:1 ratio held, which both hidden corners keep.
for (grab, ratio) in [(-4.0f32, false), (0.0, false), (4.0, false), (-4.0, true), (4.0, true)] {
if ratio && (hx == 0 || hy == 0) {
continue;
}
let what = format!("Classic Mode {classic}, Auto Center {auto}, handle ({hx}, {hy}), grabbed {grab} px off, ratio {ratio}");
let mut app = app();
app.ui.tool_options.crop_delete = false;
app.ui.tool_options.crop_shield.classic_mode = classic;
app.ui.tool_options.crop_shield.auto_center_preview = auto;
crate::crop_ui::ensure_frame(&mut app);
app.ui.crop_rect = Some([40.0, 20.0, 140.0, 70.0]);
app.crop.default_frame = false;
crate::canvas::commit_crop(&mut app);
let v = &mut app.ui.views[0];
(v.zoom, v.center, v.fit_pending, v.rotation) = (1.0, [50.0, 25.0], false, 0.0);
app.ui.extras.snap = true;
crate::crop_ui::ensure_frame(&mut app);
assert_eq!(app.ui.crop_rect, Some([0.0, 0.0, 100.0, 50.0]));
if ratio {
app.ui.tool_options.crop_ratio = "original".into();
}
let grab = vec2(f32::from(hx), f32::from(hy)) * grab;
let from = xf(&app).to_screen(50.0 + f32::from(hx) * 50.0, 25.0 + f32::from(hy) * 25.0) + grab;
// Toward the hidden corner (or edge), a screen pixel at a time on the longer axis.
let gap = vec2(f32::from(hx) * if hx < 0 { 40.0 } else { 60.0 }, f32::from(hy) * if hy < 0 { 20.0 } else { 30.0 });
let dir = gap / gap.x.abs().max(gap.y.abs());
let d = xf(&app).to_doc(from);
tool_event(&mut app, ToolEvent::Down { x: d[0], y: d[1], pressure: 1.0 }, NONE);
// Drag outward a screen pixel at a time; let go once the moving edges show
// the snap to the hidden pixels' edges (as the user would).
let snapped = |app: &PhotocraftApp| {
let on = |vertical: bool, pos: f64| {
app.prefs_rt
.snap_lines
.iter()
.any(|l| l.vertical == vertical && l.pos == pos && l.kind == photocraft_engine::snap::SnapKind::LayerEdge)
};
(hx == 0 || on(true, if hx < 0 { hidden[0] } else { hidden[2] }))
&& (hy == 0 || on(false, if hy < 0 { hidden[1] } else { hidden[3] }))
};
let mut end = None;
for i in 1..=80 {
let d = xf(&app).to_doc(from + dir * i as f32);
tool_event(&mut app, ToolEvent::Move { x: d[0], y: d[1], pressure: 1.0 }, NONE);
if snapped(&app) {
end = Some(from + dir * i as f32);
break;
}
}
let end = end.unwrap_or_else(|| panic!("{what}: never snapped to the hidden edge"));
// Released where it stopped, read through the view as it is now.
let end = xf(&app).to_doc(end);
tool_event(&mut app, ToolEvent::Up { x: end[0], y: end[1] }, NONE);
let pick = |h: i8, lo: f64, hi: f64, a: f64, b: f64| match h {
-1 => [a, hi],
1 => [lo, b],
_ => [lo, hi],
};
let [x0, x1] = pick(hx, 0.0, 100.0, hidden[0], hidden[2]);
let [y0, y1] = pick(hy, 0.0, 50.0, hidden[1], hidden[3]);
assert_eq!(app.ui.crop_rect, Some([x0, y0, x1, y1]), "{what}: the frame lands exactly on the hidden edge");
crate::canvas::commit_crop(&mut app);
let st = app.session.active().unwrap();
assert_eq!((st.doc.size.width, st.doc.size.height), ((x1 - x0) as u32, (y1 - y0) as u32), "{what}");
let layer = st.doc.walk().into_iter().map(|(_, _, l)| l.id).next().unwrap();
let r = photocraft_engine::snap::layer_rect(&st.doc, layer).unwrap();
assert_eq!(r, [hidden[0] - x0, hidden[1] - y0, hidden[2] - x0, hidden[3] - y0], "{what}: no transparent strip");
}
}
}
}
}
}
+28 -16
View File
@@ -261,7 +261,7 @@ fn apply(app: &mut PhotocraftApp, p: [f64; 2], mods: egui::Modifiers) -> [f64; 2
return p;
}
let (out, lines) = match &snap.gesture {
Gesture::Point => live_crop_handle(app, p, mods, &snap.targets, tol).unwrap_or_else(|| snap.targets.snap_point(p, tol)),
Gesture::Point => crop_handle(app, p, mods, &snap.targets, tol).unwrap_or_else(|| snap.targets.snap_point(p, tol)),
Gesture::Guide => snap.targets.snap_point(p, tol),
Gesture::Move { rect } | Gesture::TransformMove { rect } | Gesture::SelectionMove { rect } => {
let d = [p[0] - snap.start[0], p[1] - snap.start[1]];
@@ -275,18 +275,26 @@ fn apply(app: &mut PhotocraftApp, p: [f64; 2], mods: egui::Modifiers) -> [f64; 2
out
}
/// Snap the proposed handle on the image, calculated from the original camera. Snapping the
/// pointer in the moving camera feeds the previous frame's pan back into the resize and makes
/// Snap a crop handle being dragged: the handle itself (the frame's edge or corner), not the
/// pointer, which grabbed it up to the grab radius away (#1919: snapping the pointer left a strip
/// as wide as that offset between the frame and the edge it showed as snapped to). With Auto
/// Center Preview the handle is found on the image, calculated from the original camera: snapping
/// the pointer in the moving camera feeds the previous frame's pan back into the resize and makes
/// a stationary pointer oscillate near the canvas edges.
fn live_crop_handle(app: &PhotocraftApp, p: [f64; 2], mods: egui::Modifiers, targets: &SnapTargets, tol: f64) -> Option<([f64; 2], Vec<SnapLine>)> {
fn crop_handle(app: &PhotocraftApp, p: [f64; 2], mods: egui::Modifiers, targets: &SnapTargets, tol: f64) -> Option<([f64; 2], Vec<SnapLine>)> {
if app.ui.tool != Tool::Crop {
return None;
}
let crate::crop_ui::CropDrag::Resize { hx, hy, start, rect, view: Some(view) } = app.crop.drag? else { return None };
let crate::crop_ui::CropDrag::Resize { hx, hy, start, rect, view } = app.crop.drag? else { return None };
let deg = crate::crop_ui::angle(app);
let q = crate::crop_mode::to_press(app, Some(view), p);
let g = crate::crop_mode::LiveResize { rect, deg, hx, hy, start, q, ratio: crate::crop_ui::resize_ratio(app, rect, mods), alt: mods.alt };
let (frame, _) = crate::crop_mode::resized_frame(g)?;
let ratio = crate::crop_ui::resize_ratio(app, rect, mods);
let q = view.map_or(p, |view| crate::crop_mode::to_press(app, Some(view), p));
// The frame for the pointer at `q`, as `crop_ui` resizes it.
let frame_at = |q: [f64; 2]| match view {
Some(_) => crate::crop_mode::resized_frame(crate::crop_mode::LiveResize { rect, deg, hx, hy, start, q, ratio, alt: mods.alt }).map(|(f, _)| f),
None => Some(crate::crop_ui::resized_turned(rect, deg, hx, hy, [q[0] - start[0], q[1] - start[1]], ratio, mods.alt)),
};
let frame = frame_at(q)?;
let at = |r: [f64; 4]| {
let c = crate::crop_ui::center(r);
crate::crop_ui::turn([c[0] + f64::from(hx) * (r[2] - r[0]) / 2.0, c[1] + f64::from(hy) * (r[3] - r[1]) / 2.0], c, deg)
@@ -304,35 +312,39 @@ fn live_crop_handle(app: &PhotocraftApp, p: [f64; 2], mods: egui::Modifiers, tar
if deg == 0.0 {
lines.retain(|line| if line.vertical { hx != 0 } else { hy != 0 });
}
// Auto Center Preview moves both screen edges. On the image the opposite edge stays put,
// so the grabbed edge travels twice as far as the pointer (once with Alt).
let scale = if mods.alt { 1.0 } else { 2.0 };
if lines.is_empty() {
return Some((p, lines));
}
// Auto Center Preview moves both screen edges. On the image the opposite edge stays put,
// so the grabbed edge travels twice as far as the pointer (once with Alt). Without it the
// handle follows the pointer.
let scale = if view.is_some() && !mods.alt { 2.0 } else { 1.0 };
// How much the frame's extent grows per unit of pointer travel: both sides with Auto Center
// Preview or Alt, else the grabbed one.
let grow = if view.is_some() || mods.alt { 2.0 } else { 1.0 };
let original = at(rect);
let travel = crate::crop_ui::turn([handle[0] - original[0], handle[1] - original[1]], [0.0, 0.0], -deg);
let mut local = [(travel[0] + d[0]) / scale, (travel[1] + d[1]) / scale];
if deg == 0.0
&& hx != 0
&& hy != 0
&& let Some(k) = g.ratio
&& let Some(k) = ratio
{
// A corner with a ratio has one degree of freedom. Put the other axis on the same
// ratio, rather than letting the raw pointer's unused axis override the snapped edge.
if lines.iter().any(|line| line.vertical) {
local[1] = f64::from(hy) * ((rect[2] - rect[0] + 2.0 * f64::from(hx) * local[0]) / k - (rect[3] - rect[1])) / 2.0;
local[1] = f64::from(hy) * ((rect[2] - rect[0] + grow * f64::from(hx) * local[0]) / k - (rect[3] - rect[1])) / grow;
} else {
local[0] = f64::from(hx) * ((rect[3] - rect[1] + 2.0 * f64::from(hy) * local[1]) * k - (rect[2] - rect[0])) / 2.0;
local[0] = f64::from(hx) * ((rect[3] - rect[1] + grow * f64::from(hy) * local[1]) * k - (rect[2] - rect[0])) / grow;
}
}
let d = crate::crop_ui::turn(local, [0.0, 0.0], deg);
let q = [start[0] + d[0], start[1] + d[1]];
if let Some((snapped_frame, _)) = crate::crop_mode::resized_frame(crate::crop_mode::LiveResize { q, ..g }) {
if let Some(snapped_frame) = frame_at(q) {
let h = at(snapped_frame);
lines.retain(|line| (if line.vertical { h[0] } else { h[1] } - line.pos).abs() < 1e-3);
}
Some((crate::crop_mode::from_press(app, view, q), lines))
Some((view.map_or(q, |view| crate::crop_mode::from_press(app, view, q)), lines))
}
/// Snap a tool event's coordinates (called by `canvas::tool_event` before the tools see it).