From ec55bed7b0e553cf2a18131ba967dcc50f753f1b Mon Sep 17 00:00:00 2001 From: ljkx <177671075+ljkx@users.noreply.github.com> Date: Thu, 12 Nov 2026 09:55:36 +0100 Subject: [PATCH] algo, engine, ui: Perspective Crop commits a quad mirrored on one axis (#12489) `convex_quad` accepts a quad only when it is wound clockwise on screen, so a Perspective Crop box flipped on one axis (an Alt+corner drag past the opposite corner along x only), which is convex but wound counter-clockwise, was drawn red and refused by `image.perspectiveCrop` and `quad_to_rect`. The reference application commits it mirrored, each corner mapped as it lies. Add `convex_quad_any_winding` (the same angle and area limits, either winding; self-crossing, degenerate and non-finite quads stay invalid) and use it for Perspective Crop only: the `corners` check and its error text, `quad_to_rect`, and the overlay's validity check. The homography already maps each corner onto its own rectangle corner, so the crop comes out mirrored. Perspective Warp steps keep the clockwise-only `convex_quad`; clicked corners are still sorted clockwise and commit upright; a box turned 180 degrees still commits upside down. Tests: algo (a counter-clockwise quad maps corner to corner; bow ties, collinear, sliver and non-finite quads stay refused), and the new perspective_crop_mirrored_tests.rs (the measured Alt-drag case commits mirrored and is drawn valid, the command takes counter-clockwise corners, clicks commit upright, 180 degrees upside down, invalid quads stay red). The mirrored cases fail without the fix. Fixes #12489 --- crates/algo/src/transform.rs | 14 +- crates/algo/src/transform/tests.rs | 28 +++- .../engine/src/cmd/perspective_crop_cmds.rs | 11 +- .../tools/perspective_crop_mirrored_tests.rs | 138 ++++++++++++++++++ .../ui-egui/src/tools/perspective_crop_ui.rs | 13 +- 5 files changed, 191 insertions(+), 13 deletions(-) create mode 100644 crates/ui-egui/src/tools/perspective_crop_mirrored_tests.rs diff --git a/crates/algo/src/transform.rs b/crates/algo/src/transform.rs index 5a46b472e9..5dabbd2a0c 100644 --- a/crates/algo/src/transform.rs +++ b/crates/algo/src/transform.rs @@ -210,6 +210,15 @@ pub fn convex_quad(q: &[[f64; 2]; 4]) -> bool { area2.is_finite() && area2 >= 2.0 } +/// Is `q` (TL, TR, BR, BL) a usable Perspective Crop quad in either winding: a [`convex_quad`], +/// or one wound counter-clockwise on screen (a box flipped on one axis, which crops mirrored, +/// #12489) with the same corner angle and area limits? Self-crossing, degenerate and non-finite +/// quads are not. The Perspective Warp steps keep the clockwise-only [`convex_quad`]. +pub fn convex_quad_any_winding(q: &[[f64; 2]; 4]) -> bool { + let [tl, tr, br, bl] = *q; + convex_quad(q) || convex_quad(&[tl, bl, br, tr]) +} + /// The output size a Perspective Crop of `q` (TL, TR, BR, BL) takes when none is given: the mean /// length of the top and bottom edges by the mean length of the left and right edges. pub fn quad_size(q: &[[f64; 2]; 4]) -> (f64, f64) { @@ -287,9 +296,10 @@ fn perspective_estimate(q: &[[f64; 2]; 4], [cx, cy]: [f64; 2]) -> Option<(f64, f /// The homography taking the quad `q` (TL, TR, BR, BL) onto the rectangle (0, 0)–(`w`, `h`), corner /// to corner: the inverse of the unit-square-to-quad map (Heckbert, "Fundamentals of Texture /// Mapping and Image Warping", 1989, §2.2.3) scaled to the rectangle. `None` when `q` isn't a -/// [`convex_quad`] or the size isn't positive. +/// [`convex_quad_any_winding`] or the size isn't positive. A quad wound counter-clockwise maps +/// mirrored, each corner onto its own rectangle corner. pub fn quad_to_rect(q: &[[f64; 2]; 4], w: f64, h: f64) -> Option { - if !(convex_quad(q) && w.is_finite() && h.is_finite() && w > 0.0 && h > 0.0) { + if !(convex_quad_any_winding(q) && w.is_finite() && h.is_finite() && w > 0.0 && h > 0.0) { return None; } Homography::rect_to_quad([0.0, 0.0, w, h], *q)?.inverse() diff --git a/crates/algo/src/transform/tests.rs b/crates/algo/src/transform/tests.rs index b70bc74c3d..af6e81b615 100644 --- a/crates/algo/src/transform/tests.rs +++ b/crates/algo/src/transform/tests.rs @@ -61,7 +61,7 @@ fn the_interruptible_warp_reports_progress_and_stops_when_cancelled() { } /// Perspective Crop's map lands each quad corner exactly on its rectangle corner; degenerate, -/// self-crossing, counter-clockwise and non-finite quads have none. +/// self-crossing and non-finite quads have none. #[test] fn quad_to_rect_maps_corners_and_refuses_bad_quads() { let q = [[30.0, 12.0], [140.0, 25.0], [128.0, 110.0], [14.0, 95.0]]; @@ -75,7 +75,7 @@ fn quad_to_rect_maps_corners_and_refuses_bad_quads() { for bad in [ [[0.0, 0.0], [50.0, 0.0], [100.0, 0.0], [0.0, 50.0]], [[0.0, 0.0], [50.0, 50.0], [50.0, 0.0], [0.0, 50.0]], - [[0.0, 0.0], [0.0, 50.0], [50.0, 50.0], [50.0, 0.0]], + [[0.0, 0.0], [50.0, 50.0], [0.0, 50.0], [50.0, 0.0]], [[0.0, 0.0], [f64::NAN, 0.0], [50.0, 50.0], [0.0, 50.0]], [[0.0, 0.0]; 4], ] { @@ -84,6 +84,30 @@ fn quad_to_rect_maps_corners_and_refuses_bad_quads() { assert!(quad_to_rect(&q, 0.0, 10.0).is_none()); } +/// #12489: a quad wound counter-clockwise (a box flipped on one axis) maps mirrored, each corner +/// onto its own rectangle corner. Perspective Warp's clockwise-only check still refuses it. +#[test] +fn quad_to_rect_maps_a_counter_clockwise_quad_mirrored() { + let q = [[140.0, 25.0], [30.0, 12.0], [14.0, 95.0], [128.0, 110.0]]; + assert!(!convex_quad(&q) && convex_quad_any_winding(&q)); + let h = quad_to_rect(&q, 100.0, 80.0).unwrap(); + for (c, want) in q.iter().zip([[0.0, 0.0], [100.0, 0.0], [100.0, 80.0], [0.0, 80.0]]) { + let (x, y) = h.apply(c[0], c[1]); + assert!((x - want[0]).abs() < 1e-9 && (y - want[1]).abs() < 1e-9, "{c:?} -> ({x}, {y})"); + } + // Self-crossing in either winding, three corners on a line, and a sliver stay refused. + for bad in [ + [[0.0, 0.0], [50.0, 0.0], [0.0, 50.0], [50.0, 50.0]], + [[50.0, 0.0], [0.0, 0.0], [50.0, 50.0], [0.0, 50.0]], + [[50.0, 0.0], [25.0, 0.0], [0.0, 0.0], [50.0, 50.0]], + [[50.0, 0.0], [0.0, 0.0], [0.0, 0.01], [50.0, 0.01]], + [[50.0, 0.0], [0.0, f64::INFINITY], [0.0, 50.0], [50.0, 50.0]], + ] { + assert!(!convex_quad_any_winding(&bad), "{bad:?}"); + assert!(quad_to_rect(&bad, 10.0, 10.0).is_none(), "{bad:?}"); + } +} + fn rgba() -> Surface { Surface::new(PixelFormat::RGBA8) } diff --git a/crates/engine/src/cmd/perspective_crop_cmds.rs b/crates/engine/src/cmd/perspective_crop_cmds.rs index d5d4db24f5..161fa35e70 100644 --- a/crates/engine/src/cmd/perspective_crop_cmds.rs +++ b/crates/engine/src/cmd/perspective_crop_cmds.rs @@ -12,7 +12,7 @@ //! editable equivalent for them. Vector masks, paths, notes, slices and guides follow the affine //! map closest to the homography (exact when the quad is a parallelogram). -use photocraft_algo::transform::{Homography, Interp, convex_quad, quad_free_size, quad_size, quad_to_rect, warp_surface_clipped}; +use photocraft_algo::transform::{Homography, Interp, convex_quad_any_winding, quad_free_size, quad_size, quad_to_rect, warp_surface_clipped}; use photocraft_color::PixelFormat; use photocraft_doc::{Document, Layer, LayerContent, LayerId, Size}; use photocraft_geom::{Affine, Rect}; @@ -39,7 +39,8 @@ fn has_doc(s: &Session) -> std::result::Result<(), String> { s.active().map(|_| ()).ok_or_else(|| "no document open".into()) } -/// `corners`: four `[x, y]` points, TL, TR, BR, BL, forming a convex quad clockwise on screen. +/// `corners`: four `[x, y]` points, TL, TR, BR, BL, forming a convex quad. Wound counter-clockwise +/// on screen (a box flipped on one axis) it crops mirrored, each corner as it lies (#12489). fn corners(p: &Value) -> Result<[[f64; 2]; 4]> { let shape = || bad("`corners` must be four [x, y] points: top-left, top-right, bottom-right, bottom-left"); let list = p.get("corners").and_then(Value::as_array).filter(|a| a.len() == 4).ok_or_else(shape)?; @@ -52,9 +53,9 @@ fn corners(p: &Value) -> Result<[[f64; 2]; 4]> { } *slot = [x, y]; } - if !convex_quad(&q) { + if !convex_quad_any_winding(&q) { return Err(bad( - "the corners must form a convex quadrilateral at least 1 px in area, in order top-left, top-right, bottom-right, bottom-left (clockwise on screen)", + "the corners must form a convex quadrilateral at least 1 px in area, in order top-left, top-right, bottom-right, bottom-left (counter-clockwise on screen crops mirrored)", )); } Ok(q) @@ -387,7 +388,7 @@ mod tests { json!({"corners": [[0, 0], [10, 0], [10, 10]]}), json!({"corners": [[0, 0], [50, 0], [100, 0], [0, 50]]}), json!({"corners": [[0, 0], [50, 50], [50, 0], [0, 50]]}), - json!({"corners": [[0, 0], [0, 50], [50, 50], [50, 0]]}), + json!({"corners": [[50, 0], [0, 0], [50, 50], [0, 50]]}), json!({"corners": [[0, 0], [0.5, 0], [0.5, 0.5], [0, 0.5]]}), json!({"corners": [[0, 0], [1e300, 0], [50, 50], [0, 50]]}), json!({"corners": [[0, 0], ["a", 0], [50, 50], [0, 50]]}), diff --git a/crates/ui-egui/src/tools/perspective_crop_mirrored_tests.rs b/crates/ui-egui/src/tools/perspective_crop_mirrored_tests.rs new file mode 100644 index 0000000000..e01e64387c --- /dev/null +++ b/crates/ui-egui/src/tools/perspective_crop_mirrored_tests.rs @@ -0,0 +1,138 @@ +//! #12489: a Perspective Crop box flipped on one axis (⌥ dragging a corner past the opposite one +//! along x only) is wound counter-clockwise. It is drawn valid and commits mirrored, each corner +//! mapped as it lies, as in the reference application; `image.perspectiveCrop` takes the same +//! corners. Clicked corners still commit upright, a box turned 180° still commits upside down, and +//! self-crossing and degenerate quads stay invalid. + +use std::sync::Arc; + +use egui::{Modifiers, Pos2, Rect, vec2}; +use serde_json::json; + +use super::*; + +const RED: [f32; 4] = [1.0, 0.0, 0.0, 1.0]; +const GREEN: [f32; 4] = [0.0, 1.0, 0.0, 1.0]; +const BLUE: [f32; 4] = [0.0, 0.0, 1.0, 1.0]; +const YELLOW: [f32; 4] = [1.0, 1.0, 0.0, 1.0]; + +/// The measured test document: 800 x 500, red (top left), green (top right), blue (bottom left) +/// and yellow (bottom right) quadrants with a black square in the middle; the Perspective Crop tool. +fn app() -> PhotocraftApp { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + app.run("file.new", json!({"width": 800, "height": 500})).unwrap(); + app.sync_views(); + app.ui.tool = Tool::PerspectiveCrop; + let surf = Arc::make_mut(&mut app.session.active_mut().unwrap().doc).layers[0].surface_mut().unwrap(); + for (x, y, c) in [(0, 0, RED), (400, 0, GREEN), (0, 250, BLUE), (400, 250, YELLOW)] { + surf.fill_rect(photocraft_geom::Rect::from_xywh(x, y, 400, 250), &c); + } + surf.fill_rect(photocraft_geom::Rect::from_xywh(395, 245, 10, 10), &[0.0, 0.0, 0.0, 1.0]); + app +} + +fn drag(app: &mut PhotocraftApp, from: [f64; 2], to: [f64; 2], mods: Modifiers) { + assert!(pointer(app, ToolEvent::Down { x: from[0], y: from[1], pressure: 1.0 }, mods)); + let mid = [(from[0] + to[0]) / 2.0, (from[1] + to[1]) / 2.0]; + assert!(pointer(app, ToolEvent::Move { x: mid[0], y: mid[1], pressure: 1.0 }, mods)); + assert!(pointer(app, ToolEvent::Move { x: to[0], y: to[1], pressure: 1.0 }, mods)); + assert!(pointer(app, ToolEvent::Up { x: to[0], y: to[1] }, mods)); +} + +fn click(app: &mut PhotocraftApp, p: [f64; 2]) { + drag(app, p, p, Modifiers::NONE); +} + +/// The overlay `draw_overlay` paints: (shield polygons, whether the frame is red). +fn overlay(app: &PhotocraftApp) -> (usize, bool) { + let ctx = egui::Context::default(); + let xf = + ViewXform { rect: Rect::from_min_size(Pos2::ZERO, vec2(800.0, 500.0)), zoom: 1.0, center: [400.0, 250.0], flip: false, rotation: 0.0, aspect: 1.0 }; + let mut o = ctx.run_ui(Default::default(), |ui| draw_overlay(app, ui.painter(), &xf)); + o.textures_delta.clear(); + let shield = o.shapes.iter().filter(|c| matches!(&c.shape, egui::Shape::Path(p) if p.fill == SHIELD)).count(); + let red = Color32::from_rgb(0xe5, 0x3e, 0x3e); + let frame_red = o.shapes.iter().any(|c| matches!(&c.shape, egui::Shape::Path(p) if p.closed && p.stroke.color == egui::epaint::ColorMode::Solid(red))); + (shield, frame_red) +} + +/// The committed canvas's size and the colours at its four corners (TL, TR, BL, BR). +fn corners(app: &PhotocraftApp) -> ((u32, u32), [[f32; 4]; 4]) { + let doc = &app.session.active().unwrap().doc; + let (w, h) = (doc.size.width as i32, doc.size.height as i32); + let surf = doc.layers[0].surface().unwrap(); + ((doc.size.width, doc.size.height), [surf.rgba(3, 3), surf.rgba(w - 4, 3), surf.rgba(3, h - 4), surf.rgba(w - 4, h - 4)]) +} + +#[test] +fn alt_corner_drag_past_the_opposite_corner_on_one_axis_commits_mirrored() { + let mut app = app(); + // A fresh 100 x 100 box, its bottom right corner ⌥ dragged past the fixed top left one on x only. + drag(&mut app, [450.0, 220.0], [550.0, 320.0], Modifiers::NONE); + drag(&mut app, [550.0, 320.0], [350.0, 300.0], Modifiers::ALT); + assert_eq!(app.ui.perspective_crop, Some([[450.0, 220.0], [350.0, 220.0], [350.0, 300.0], [450.0, 300.0]]), "flipped left to right"); + let (shield, red) = overlay(&app); + assert!(shield > 0 && !red, "drawn as a valid quad: accent frame and shield, not red"); + commit(&mut app); + assert!(app.ui.perspective_crop.is_none(), "committed"); + let (size, [tl, tr, bl, br]) = corners(&app); + assert_eq!(size, (100, 80)); + assert_eq!([tl, tr, bl, br], [GREEN, RED, YELLOW, BLUE], "mirrored left to right, top stays on top"); +} + +#[test] +fn the_command_takes_counter_clockwise_corners_and_mirrors() { + let mut app = app(); + let r = app.run("image.perspectiveCrop", json!({"corners": [[450, 220], [350, 220], [350, 300], [450, 300]]})).unwrap(); + assert_eq!((r["width"].as_u64(), r["height"].as_u64()), (Some(100), Some(80))); + assert_eq!(corners(&app).1, [GREEN, RED, YELLOW, BLUE]); + // Mirrored top to bottom the other way round. + let mut app = self::app(); + app.run("image.perspectiveCrop", json!({"corners": [[350, 300], [450, 300], [450, 220], [350, 220]]})).unwrap(); + assert_eq!(corners(&app).1, [BLUE, YELLOW, RED, GREEN]); +} + +#[test] +fn clicked_corners_commit_upright_in_any_order() { + let mut app = app(); + // Counter-clockwise clicks from the top left. + for p in [[350.0, 220.0], [350.0, 300.0], [450.0, 300.0], [450.0, 220.0]] { + click(&mut app, p); + } + assert_eq!(app.ui.perspective_crop, Some([[350.0, 220.0], [450.0, 220.0], [450.0, 300.0], [350.0, 300.0]]), "sorted clockwise"); + commit(&mut app); + assert_eq!(corners(&app), ((100, 80), [RED, GREEN, BLUE, YELLOW])); +} + +#[test] +fn a_box_turned_180_degrees_commits_upside_down() { + let mut app = app(); + drag(&mut app, [450.0, 300.0], [550.0, 400.0], Modifiers::NONE); + drag(&mut app, [550.0, 400.0], [350.0, 200.0], Modifiers::ALT); + assert_eq!(app.ui.perspective_crop, Some([[450.0, 300.0], [350.0, 300.0], [350.0, 200.0], [450.0, 200.0]])); + commit(&mut app); + assert_eq!(corners(&app), ((100, 100), [YELLOW, BLUE, GREEN, RED])); +} + +#[test] +fn self_crossing_and_degenerate_quads_stay_invalid() { + for q in [ + // Self-crossing (a bow tie), each way round. + [[350.0, 220.0], [450.0, 220.0], [350.0, 300.0], [450.0, 300.0]], + [[450.0, 220.0], [350.0, 220.0], [450.0, 300.0], [350.0, 300.0]], + // Three corners on a line, and no area. + [[350.0, 220.0], [400.0, 220.0], [450.0, 220.0], [350.0, 300.0]], + [[400.0, 250.0]; 4], + ] { + let mut app = app(); + app.ui.perspective_crop = Some(q); + let (shield, red) = overlay(&app); + assert!(shield == 0 && red, "{q:?} is drawn red without the shield"); + assert!(app.run("image.perspectiveCrop", json!({ "corners": q })).is_err(), "{q:?}"); + commit(&mut app); + assert_eq!(app.ui.perspective_crop, Some(q), "{q:?} stays for fixing"); + assert_eq!(app.session.active().unwrap().doc.size.width, 800); + } + let mut app = app(); + assert!(app.run("image.perspectiveCrop", json!({"corners": [[350, 220], [f64::NAN, 220], [450, 300], [350, 300]]})).is_err()); +} diff --git a/crates/ui-egui/src/tools/perspective_crop_ui.rs b/crates/ui-egui/src/tools/perspective_crop_ui.rs index bf0d491fb0..5a47aebe50 100644 --- a/crates/ui-egui/src/tools/perspective_crop_ui.rs +++ b/crates/ui-egui/src/tools/perspective_crop_ui.rs @@ -4,8 +4,8 @@ //! BL in document px; clicked corners are put in that order); //! - drag a corner to move it alone, onto an edge of a skewed object (a photographed page, a //! facade); drag inside the quad to move it; -//! - the quad shows its perspective grid (Show Grid) and turns red while it isn't a convex quad -//! wound like its box, which can't be committed; +//! - the quad shows its perspective grid (Show Grid) and turns red while it isn't a convex quad, +//! which can't be committed; one flipped on one axis (wound the other way) commits mirrored; //! - ↵ (or ✓) commits: `image.perspectiveCrop` maps the quad onto an upright rectangle, the whole //! document in one undo step. Its size comes from the options bar's W x H (and the resolution), //! which are the Crop tool's fields (`crop_size`), or with them empty from the rectangle the @@ -14,7 +14,7 @@ //! A pending quad belongs to the document it was drawn on, like the Crop tool's frame. use egui::{Color32, Pos2, Stroke}; -use photocraft_algo::transform::{Homography, convex_quad}; +use photocraft_algo::transform::{Homography, convex_quad, convex_quad_any_winding}; use photocraft_engine::prefs::Unit; use serde_json::{Value, json}; @@ -393,7 +393,8 @@ pub fn draw_overlay(app: &PhotocraftApp, painter: &egui::Painter, xf: &ViewXform _ => app.ui.perspective_crop, }; if let Some(q) = quad.filter(|q| q.iter().flatten().all(|v| v.is_finite())) { - let ok = convex_quad(&q); + // A quad flipped on one axis is wound counter-clockwise and commits mirrored (#12489). + let ok = convex_quad_any_winding(&q); let color = if ok { accent } else { Color32::from_rgb(0xe5, 0x3e, 0x3e) }; // The image outside the quad is dimmed (#11972), following the quad while it is dragged. if ok && let Some(size) = app.session.active().map(|d| d.doc.size) { @@ -521,6 +522,10 @@ mod perspective_crop_shield_tests; #[path = "perspective_crop_deselect_tests.rs"] mod perspective_crop_deselect_tests; +#[cfg(test)] +#[path = "perspective_crop_mirrored_tests.rs"] +mod perspective_crop_mirrored_tests; + #[cfg(test)] mod tests { use super::*;