Merge pull request #6489 from dotCooCoo/fix/perspective-snap-first-unattached

Perspective grid: Snap to Grid reads a flat first object at the depth it attaches to

crates/engine/src/cmd/distortcmds/perspective.rs

@@ -263,6 +263,22 @@ fn record_again(s: &mut Session, again: Value) -> Result<()> {
     Ok(())
 }
 
+/// `d` with each of `ids` that is not in perspective attached to `fallback` as [`persp_edit`] attaches
+/// it; `d` itself when every one is in perspective already, there is no `fallback`, or one of them
+/// cannot attach.
+fn with_fallback<'a>(d: &'a Document, ids: &[NodeId], fallback: Option<Plane>) -> std::borrow::Cow<'a, Document> {
+    let mut g = grid_of(d);
+    let loose: Vec<NodeId> = ids.iter().copied().filter(|id| g.attachment_of(d, *id).is_none()).collect();
+    let Some(plane) = fallback.filter(|_| !loose.is_empty()) else { return std::borrow::Cow::Borrowed(d) };
+    let mut out = d.clone();
+    for id in loose {
+        if attach_in(&mut out, &mut g, id, plane, false, None).is_err() {
+            return std::borrow::Cow::Borrowed(d);
+        }
+    }
+    std::borrow::Cow::Owned(out)
+}
+
 fn ids_json(ids: &[NodeId]) -> Value {
     json!({ "ids": ids.iter().map(|i| i.0).collect::<Vec<_>>() })
 }
@@ -285,9 +301,11 @@ pub(super) fn persp_move(s: &mut Session, p: &Value) -> Result<Value> {
     let constrain = switch_or(p, "constrain", false, C)?;
     // `snap_fix` uses Snap to Grid's setting for a `snap` that is not true or false.
     check_switches(p, &["snap"], C)?;
+    // Shift and Snap to Grid read the objects as they are once those not in perspective are attached.
+    let doc = if perp { std::borrow::Cow::Borrowed(&*s.doc()?.doc) } else { with_fallback(&s.doc()?.doc, &ids, fallback) };
     // Shift: the constrained end point, read on the first object's plane and depth.
     let snap_to = if constrain && !perp {
-        let doc = &s.doc()?.doc;
+        let doc = &*doc;
         let g = grid_of(doc);
         g.plane_bounds(doc, &ids)
             .and_then(|(plane, depth, _)| {
@@ -305,10 +323,10 @@ pub(super) fn persp_move(s: &mut Session, p: &Value) -> Result<Value> {
     let fix = if perp {
         None
     } else {
-        let doc = &s.doc()?.doc;
-        crate::cmd::perspgrid::snap_fix(doc, &ids, p, (from, snap_to))
-            .and_then(|(plane, depth, correction)| Some((plane, grid_of(doc).plane_delta(plane, depth, from, to)?, correction)))
+        crate::cmd::perspgrid::snap_fix(&doc, &ids, p, (from, snap_to))
+            .and_then(|(plane, depth, correction)| Some((plane, grid_of(&doc).plane_delta(plane, depth, from, to)?, correction)))
     };
+    drop(doc);
     let snapped = move |pl: Plane, raw: vectorcraft_geom::Vec2| {
         let raw = fix.filter(|f| f.0 == pl).map_or(raw, |f| f.1);
         let (direction, dv) =

crates/engine/src/tests/persp_select.rs

@@ -262,6 +262,45 @@ fn snap_moves_objects_at_different_depths_by_one_plane_offset() {
     assert!(on_line(moved.y0) || on_line(moved.y1), "{moved:?}");
 }
 
+/// The anchors of `flat` and `far` after a snapped move of both on the left plane, where `flat`
+/// starts flat (attached by the move, or first with `attach_first`) and `far` sits 50 pt deeper.
+fn move_with_a_flat_first_object(attach_first: bool, constrain: bool) -> Vec<Point> {
+    let (mut s, _) = persp_session();
+    let rect = |s: &mut Session, x: f64| {
+        NodeId(s.execute("shape.rectangle", &json!({"x": x, "y": 380, "width": 40, "height": 40})).unwrap()["id"].as_u64().unwrap())
+    };
+    s.execute("perspective.plane.move", &json!({"plane": "left", "offset": 50})).unwrap();
+    let far = rect(&mut s, 150.0);
+    s.execute("perspective.attach", &json!({"ids": [far.0], "plane": "left"})).unwrap();
+    s.execute("perspective.plane.move", &json!({"plane": "left", "offset": 0})).unwrap();
+    let flat = rect(&mut s, 240.0);
+    if attach_first {
+        s.execute("perspective.attach", &json!({"ids": [flat.0], "plane": "left"})).unwrap();
+    }
+    s.execute("perspective.grid.snap", &json!({"on": true})).unwrap();
+    let h = grid(&s).homography_at(Plane::Left, 0.0).unwrap();
+    let (from, to) = (h.apply(Point::new(100.0, 100.0)).unwrap(), h.apply(Point::new(137.3, 121.7)).unwrap());
+    s.execute(
+        "perspective.move",
+        &json!({"ids": [flat.0, far.0], "plane": "left", "from": [from.x, from.y], "to": [to.x, to.y], "constrain": constrain}),
+    )
+    .unwrap();
+    assert_eq!(attached(&s, flat).map(|a| a.0), Some(Plane::Left));
+    let doc = &s.doc().unwrap().doc;
+    [flat, far].iter().flat_map(|id| doc.node(*id).unwrap().path_data().unwrap().anchors().map(|(_, _, a)| a.p).collect::<Vec<_>>()).collect()
+}
+
+#[test]
+fn snap_reads_a_flat_first_object_as_attached_by_the_move() {
+    for constrain in [false, true] {
+        let (moved, attached_first) = (move_with_a_flat_first_object(false, constrain), move_with_a_flat_first_object(true, constrain));
+        assert_eq!(moved.len(), attached_first.len());
+        for (a, b) in moved.iter().zip(&attached_first) {
+            assert!(a.distance(*b) < 1e-6, "constrain {constrain}: {moved:?} vs {attached_first:?}");
+        }
+    }
+}
+
 #[test]
 fn constrained_move_uses_each_objects_plane() {
     let (mut s, right) = persp_session();