diff --git a/crates/ui-egui/src/crop_mode.rs b/crates/ui-egui/src/crop_mode.rs index e19f01334a..676c97dbec 100644 --- a/crates/ui-egui/src/crop_mode.rs +++ b/crates/ui-egui/src/crop_mode.rs @@ -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"); + } + } + } + } + } } diff --git a/crates/ui-egui/src/snap_ui.rs b/crates/ui-egui/src/snap_ui.rs index 83d973739d..56fecd01eb 100644 --- a/crates/ui-egui/src/snap_ui.rs +++ b/crates/ui-egui/src/snap_ui.rs @@ -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)> { +fn crop_handle(app: &PhotocraftApp, p: [f64; 2], mods: egui::Modifiers, targets: &SnapTargets, tol: f64) -> Option<([f64; 2], Vec)> { 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).