Merge pull request #12883 from rmsff/fix/warp-split-default-thirds-12495

Warp: a one-way split on the Default grid keeps only the thirds across it

crates/engine/src/cmd/warp_cmds.rs

@@ -508,19 +508,24 @@ pub enum Split {
 }
 
 /// The default warp draws rule-of-thirds guides on its single patch. Make those real sections
-/// before a split, so the cut divides the grid on screen instead of replacing it.
-fn commit_default_sections(mesh: &mut BezierMesh) {
-    let _ = mesh.split_u(1.0 / 3.0);
-    let _ = mesh.split_u(2.0 / 3.0);
-    let _ = mesh.split_v(1.0 / 3.0);
-    let _ = mesh.split_v(2.0 / 3.0);
+/// before a split, so the cut divides the grid on screen instead of replacing it. A one-way split
+/// keeps only the thirds across its line and drops the ones it replaces (#12495).
+fn commit_default_sections(mesh: &mut BezierMesh, how: Split) {
+    if !matches!(how, Split::Vertical) {
+        let _ = mesh.split_u(1.0 / 3.0);
+        let _ = mesh.split_u(2.0 / 3.0);
+    }
+    if !matches!(how, Split::Horizontal) {
+        let _ = mesh.split_v(1.0 / 3.0);
+        let _ = mesh.split_v(2.0 / 3.0);
+    }
 }
 
 /// Applies a split (or removes the split nearest `at`) on `w`'s mesh (presets become custom).
 pub fn split_warp(w: &Warp, at: Option<[f64; 2]>, how: Split) -> Option<Warp> {
     let mut mesh = w.to_mesh(1, 1);
     if !matches!(how, Split::Remove) && w.style == WarpStyle::Custom && mesh.us.len() == 2 && mesh.vs.len() == 2 {
-        commit_default_sections(&mut mesh);
+        commit_default_sections(&mut mesh, how);
     }
     let (s, t) = match at {
         Some(p) => mesh.param_at(p),
@@ -751,3 +756,7 @@ pub fn specs() -> Vec<CommandSpec> {
 
 #[cfg(test)]
 mod tests;
+
+#[cfg(test)]
+#[path = "warp_split_default_grid_tests.rs"]
+mod warp_split_default_grid_tests;

crates/engine/src/cmd/warp_cmds/tests.rs

@@ -308,9 +308,8 @@ fn split_cuts_existing_sections_instead_of_replacing_the_grid() {
     let plain = Warp::custom(BezierMesh::identity(b, 1, 1), b);
     let cut = split_warp(&plain, Some([b[0] + 0.25 * 90.0, 30.0]), Split::Vertical).unwrap();
     let m = cut.mesh.unwrap();
-    for u in [0.0, 0.25, 1.0 / 3.0, 2.0 / 3.0, 1.0] {
-        assert!(m.us.iter().any(|k| (k - u).abs() < 1e-6), "default guides stay, cut at 0.25: {:?}", m.us);
-    }
+    assert_eq!(m.us.len(), 3, "the cut at 0.25 replaces the vertical thirds: {:?}", m.us);
+    assert!((m.us[1] - 0.25).abs() < 1e-6, "{:?}", m.us);
     assert_eq!(m.vs, vec![0.0, 1.0 / 3.0, 2.0 / 3.0, 1.0]);
     let grid = BezierMesh::identity(b, 3, 3);
     let cut = split_warp(&Warp::custom(grid.clone(), b), Some([45.0, 10.0]), Split::Vertical).unwrap();

crates/engine/src/cmd/warp_split_default_grid_tests.rs

@@ -0,0 +1,87 @@
+//! Which rule-of-thirds guides a split on the Default warp grid turns into real sections (#12495).
+
+use super::*;
+
+/// The 400 x 300 layer box of the issue, at (200, 150).
+const BOX: [f64; 4] = [200.0, 150.0, 600.0, 450.0];
+
+fn session() -> Session {
+    let mut s = Session::new();
+    s.execute("file.new", json!({"width": 800, "height": 600})).unwrap();
+    s.execute("layer.new.layer", json!({})).unwrap();
+    s
+}
+
+/// Runs a split command on the warp being edited, as the Free Transform warp session does.
+fn split_cmd(s: &mut Session, cmd: &str, warp: Value, at: [f64; 2]) -> BezierMesh {
+    let r = s.execute(cmd, json!({"warp": warp, "at": at})).unwrap();
+    let out: Warp = serde_json::from_value(r["warp"].clone()).unwrap();
+    out.mesh.unwrap()
+}
+
+fn default_grid() -> Value {
+    serde_json::to_value(Warp::custom(BezierMesh::identity(BOX, 1, 1), BOX)).unwrap()
+}
+
+/// The anchor columns (x along the top row) and anchor rows (y down the left column).
+fn anchors(m: &BezierMesh) -> (Vec<f64>, Vec<f64>) {
+    let xs = (0..m.nx()).step_by(3).map(|i| m.point(i, 0)[0]).collect();
+    let ys = (0..m.ny()).step_by(3).map(|j| m.point(0, j)[1]).collect();
+    (xs, ys)
+}
+
+fn assert_near(got: &[f64], want: &[f64], what: &str) {
+    assert_eq!(got.len(), want.len(), "{what}: {got:?}, want {want:?}");
+    for (g, w) in got.iter().zip(want) {
+        assert!((g - w).abs() < 0.01, "{what}: {got:?}, want {want:?}");
+    }
+}
+
+const THIRDS_X: [f64; 4] = [200.0, 200.0 + 400.0 / 3.0, 200.0 + 800.0 / 3.0, 600.0];
+const THIRDS_Y: [f64; 4] = [150.0, 250.0, 350.0, 450.0];
+
+#[test]
+fn horizontal_split_of_the_default_grid_is_three_columns_by_two_rows() {
+    let mut s = session();
+    let m = split_cmd(&mut s, "edit.transform.splitWarpHorizontally", default_grid(), [400.0, 320.0]);
+    let (xs, ys) = anchors(&m);
+    assert_near(&xs, &THIRDS_X, "the vertical thirds become anchor columns");
+    assert_near(&ys, &[150.0, 320.0, 450.0], "one row line at the click, the horizontal thirds are gone");
+    assert!(m.is_valid());
+}
+
+#[test]
+fn vertical_split_of_the_default_grid_is_two_columns_by_three_rows() {
+    let mut s = session();
+    let m = split_cmd(&mut s, "edit.transform.splitWarpVertically", default_grid(), [300.0, 200.0]);
+    let (xs, ys) = anchors(&m);
+    assert_near(&xs, &[200.0, 300.0, 600.0], "one column line at the click, the vertical thirds are gone");
+    assert_near(&ys, &THIRDS_Y, "the horizontal thirds become anchor rows");
+    assert!(m.is_valid());
+}
+
+#[test]
+fn crosswise_split_of_the_default_grid_keeps_both_thirds() {
+    let mut s = session();
+    let m = split_cmd(&mut s, "edit.transform.splitWarpCrosswise", default_grid(), [300.0, 320.0]);
+    let (xs, ys) = anchors(&m);
+    assert_near(&xs, &[THIRDS_X[0], 300.0, THIRDS_X[1], THIRDS_X[2], THIRDS_X[3]], "columns");
+    assert_near(&ys, &[150.0, 250.0, 320.0, 350.0, 450.0], "rows");
+}
+
+#[test]
+fn a_split_of_an_already_split_grid_adds_only_its_line() {
+    let mut s = session();
+    let rows = split_cmd(&mut s, "edit.transform.splitWarpHorizontally", default_grid(), [400.0, 320.0]);
+    let warp = serde_json::to_value(Warp::custom(rows, BOX)).unwrap();
+    let m = split_cmd(&mut s, "edit.transform.splitWarpVertically", warp, [300.0, 200.0]);
+    let (xs, ys) = anchors(&m);
+    assert_near(&xs, &[THIRDS_X[0], 300.0, THIRDS_X[1], THIRDS_X[2], THIRDS_X[3]], "one more column");
+    assert_near(&ys, &[150.0, 320.0, 450.0], "rows are not rebuilt");
+    // A 3 x 3 grid that is not the Default one: a horizontal split keeps every section.
+    let grid = serde_json::to_value(Warp::custom(BezierMesh::identity(BOX, 3, 3), BOX)).unwrap();
+    let m = split_cmd(&mut s, "edit.transform.splitWarpHorizontally", grid, [400.0, 320.0]);
+    let (xs, ys) = anchors(&m);
+    assert_near(&xs, &THIRDS_X, "columns");
+    assert_near(&ys, &[150.0, 250.0, 320.0, 350.0, 450.0], "rows");
+}

crates/ui-egui/src/tools/transform_tool/tests.rs

@@ -879,8 +879,8 @@ fn split_tool_places_the_line_where_the_pointer_releases() {
     warp_pointer(&mut app, ToolEvent::Up { x: 14.0, y: 20.0 }, 2.0, mods);
     let us = knots(&app);
     let has = |ks: &[f64], u: f64| ks.iter().any(|k| (k - u).abs() < 0.02);
-    // The default grid's thirds stay; the release cuts the section that contains it.
-    assert!(has(&us, 0.25) && has(&us, 1.0 / 3.0) && has(&us, 2.0 / 3.0), "{us:?}");
+    // The release cuts the default grid where it is, in place of its vertical thirds.
+    assert!(us.len() == 3 && has(&us, 0.25), "{us:?}");
     let vs = app.ui.transform.as_ref().unwrap().warp.as_ref().unwrap().mesh.as_ref().unwrap().vs.clone();
     assert!(has(&vs, 1.0 / 3.0) && has(&vs, 2.0 / 3.0), "horizontal sections stay: {vs:?}");
     assert!(app.transform_preview.as_ref().unwrap().split_tool.is_some(), "the tool stays armed");