Merge pull request #13485 from ljkx/fix/12489-perspective-crop-mirrored-quad

algo, engine, ui: Perspective Crop commits a quad mirrored on one axis (#12489)

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<Homography> {
-    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()

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)
 }

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]]}),

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());
+}

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};
 
@@ -426,7 +426,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) {
@@ -554,6 +555,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::*;