Merge pull request #3728 from Taha-Mahmoodi/fix/guide-snap-exact

Fix: a path moved by its stroke or edge lands exactly on a ruler guide

crates/engine/src/tests_prefs.rs

@@ -1601,3 +1601,65 @@ fn snap_to_point_cues_anchors_and_lands_pen_clicks_on_them() {
         s.execute("select.none", &json!({})).unwrap();
     }
 }
+
+/// A millimetre document with a rectangle (100, 100)–(300, 250) stroked 1 pt and a vertical
+/// ruler guide at no whole number of millimetres or points (62.31 mm): the rectangle and the
+/// guide's x.
+fn stroked_rect_and_guide() -> (Session, NodeId, f64) {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"width": 600, "height": 600, "units": "Millimeters"})).unwrap();
+    let id = NodeId(s.execute("shape.rectangle", &json!({"x": 100, "y": 100, "width": 200, "height": 150})).unwrap()["id"].as_u64().unwrap());
+    s.execute("stroke.set", &json!({"weight": 1})).unwrap();
+    let g = 62.31 / 25.4 * 72.0;
+    s.execute("guide.add", &json!({"vertical": true, "pos": g})).unwrap();
+    (s, id, g)
+}
+
+/// #3701: at 3,891%, the Selection tool pressed on a rectangle's 1 pt stroke inside its left edge,
+/// or on its fill 3 px inside it, and dragged until the edge is 1 px from a ruler guide lands the
+/// edge on the guide exactly, with "align". The pressed spot used to be what snapped, so the edge
+/// stopped that far beside the guide however far one zoomed in.
+#[test]
+fn a_move_held_by_a_paths_stroke_lands_the_path_on_a_ruler_guide() {
+    use vectorcraft_tools::Overlay;
+    use vectorcraft_tools::PointerKind::{Down, Drag, Up};
+    let (mut s, id, g) = stroked_rect_and_guide();
+    let v = ViewInfo { zoom: 38.91, ..ViewInfo::default() };
+    s.select_tool("selection", v).unwrap();
+    // 0.3 pt (12 px) into the stroke, and 3 px into the fill.
+    for x in [100.3, 100.0 + 3.0 / v.zoom] {
+        s.execute("select.none", &json!({})).unwrap();
+        let to = x + g + 1.0 / v.zoom - 100.0;
+        gesture(&mut s, v, &[(Down, x, 175.0), (Drag, (x + to) / 2.0, 175.0), (Drag, to, 175.0)]);
+        let align = s.overlays(v).iter().any(|o| matches!(o, Overlay::Label { text, .. } if text == "align"));
+        gesture(&mut s, v, &[(Up, to, 175.0)]);
+        let edge = top_left(&s, id).0;
+        assert!((edge - g).abs() < 1e-9 && align, "pressed at {x}: the edge is {} pt off the guide (align: {align})", edge - g);
+        s.execute("edit.undo", &json!({})).unwrap();
+    }
+}
+
+/// #3701: at 28,757%, Direct Selection pressed 1.5 px beside a rectangle's straight left side and
+/// dragged until the side is 1 px from a ruler guide lands both its anchors on the guide; so does
+/// the same press and drag on the rectangle selected whole, which moves it.
+#[test]
+fn a_straight_segment_dragged_by_direct_selection_lands_on_a_ruler_guide() {
+    use vectorcraft_tools::PointerKind::{Down, Drag, Up};
+    let (mut s, id, g) = stroked_rect_and_guide();
+    let v = ViewInfo { zoom: 287.57, ..ViewInfo::default() };
+    s.select_tool("directSelection", v).unwrap();
+    for whole in [false, true] {
+        s.execute("select.set", &json!({"ids": if whole { vec![id.0] } else { vec![] }})).unwrap();
+        let x = 100.0 + 1.5 / v.zoom;
+        let to = x + g + 1.0 / v.zoom - 100.0;
+        gesture(&mut s, v, &[(Down, x, 175.0), (Drag, (x + to) / 2.0, 175.0), (Drag, to, 175.0), (Up, to, 175.0)]);
+        let mut xs: Vec<f64> = s.doc().unwrap().doc.node(id).unwrap().path_data().unwrap().subpaths[0].anchors.iter().map(|a| a.p.x).collect();
+        xs.sort_by(f64::total_cmp);
+        assert!(
+            xs[..2].iter().all(|x| (x - g).abs() < 1e-9),
+            "whole: {whole}, the side's anchors are {:?} pt off the guide",
+            xs[..2].iter().map(|x| x - g).collect::<Vec<_>>()
+        );
+        s.execute("edit.undo", &json!({})).unwrap();
+    }
+}

crates/tools/src/direct.rs

@@ -43,24 +43,27 @@ use crate::guides::{HandleSnap, Leave, PointSnap, Targets};
 use crate::meshedit::MeshEdit;
 use crate::pathtype::{self, BracketDrag, over_bracket};
 use crate::rulerguide::GuideEdit;
-use crate::select::{MoveSnap, is_area_type, matrix_json, release_guide_at};
+use crate::select::{MoveSnap, hold_point, is_area_type, matrix_json, release_guide_at};
 use crate::{Action, Cursor, Mods, Overlay, PointerEvent, PointerKind, Tool, ToolContext, ToolKey};
 
 #[derive(Clone, Debug)]
 enum State {
     Idle,
     /// Dragging the selected anchors, which move alike: `grab` is where the anchor pressed on (or
-    /// the pointer on a segment) was, the point that snaps.
+    /// the point of a straight segment nearest the pointer pressed on it) was, the point that
+    /// snaps.
     MoveAnchors {
         start: Point,
         grab: Point,
         began: bool,
     },
     /// Moving (Alt: copying) the selected objects; `grab`: the anchor an Alt-press picked the
-    /// path by, which snaps as a dragged anchor does.
+    /// path by, which snaps as a dragged anchor does; else `hold`, where the press took hold of
+    /// the object ([`hold_point`]), snaps as the Selection tool's does.
     MoveObject {
         start: Point,
         grab: Option<Point>,
+        hold: Point,
         began: bool,
         /// Group Selection: Shift-click on a selected object toggles it off on release.
         deselect: Option<NodeId>,
@@ -473,7 +476,7 @@ impl Tool for DirectSelectionTool {
                         out.push(Action::Exec("select.set".into(), json!({"ids": [target.0]})));
                     }
                 }
-                self.state = State::MoveObject { start: p, grab: None, began: false, deselect };
+                self.state = State::MoveObject { start: p, grab: None, hold: hold_point(cx, &h, p), began: false, deselect };
                 out
             }
             (PointerKind::DoubleClick, _) if self.group => release_guide_at(cx, p, ev.mods).into_iter().collect(),
@@ -538,7 +541,7 @@ impl Tool for DirectSelectionTool {
                     }
                     // Alt picks the whole path (and Alt-drag copies it), as on a segment.
                     if ev.mods.alt {
-                        self.state = State::MoveObject { start: p, grab: Some(grab), began: false, deselect: None };
+                        self.state = State::MoveObject { start: p, grab: Some(grab), hold: grab, began: false, deselect: None };
                         let whole = cx.selection.contains(id) && cx.selection.partial(id).is_none();
                         return if whole { vec![] } else { vec![Action::Exec("select.set".into(), json!({"ids": [id.0]}))] };
                     }
@@ -549,7 +552,7 @@ impl Tool for DirectSelectionTool {
                     // Paths all selected whole move as objects, held by this anchor, so Alt can turn
                     // the copy on and off during the drag as with the Selection tool.
                     self.state = if already && cx.selection.anchors.is_empty() {
-                        State::MoveObject { start: p, grab: Some(grab), began: false, deselect: None }
+                        State::MoveObject { start: p, grab: Some(grab), hold: grab, began: false, deselect: None }
                     } else {
                         State::MoveAnchors { start: p, grab, began: false }
                     };
@@ -593,7 +596,9 @@ impl Tool for DirectSelectionTool {
                             self.state = if s.curved {
                                 State::Segment { id: h.leaf, si: s.si, seg: s.seg, t: s.t, start: p, began: false }
                             } else {
-                                State::MoveAnchors { start: p, grab: p, began: false }
+                                // Held by the segment itself, so the segment is what lands on a
+                                // ruler guide or lines up, not the pressed spot beside it (#3701).
+                                State::MoveAnchors { start: p, grab: hold_point(cx, &h, p), began: false }
                             };
                             if selected {
                                 return vec![];
@@ -602,7 +607,7 @@ impl Tool for DirectSelectionTool {
                         }
                     }
                     // Clicking the fill selects the whole leaf path (all anchors).
-                    self.state = State::MoveObject { start: p, grab: None, began: false, deselect: None };
+                    self.state = State::MoveObject { start: p, grab: None, hold: hold_point(cx, &h, p), began: false, deselect: None };
                     if ev.mods.shift {
                         return vec![Action::Exec("select.toggle".into(), json!({"id": h.leaf.0}))];
                     }
@@ -654,14 +659,14 @@ impl Tool for DirectSelectionTool {
                 ));
                 out
             }
-            (PointerKind::Drag, State::MoveObject { start, grab, began, .. }) => {
+            (PointerKind::Drag, State::MoveObject { start, grab, hold, began, .. }) => {
                 let mut out = vec![];
                 if !began {
                     if p.distance(start) < cx.tol(3.0) {
                         return out;
                     }
                     out.push(Action::Begin(if ev.mods.alt { "Copy".into() } else { "Move".into() }));
-                    self.state = State::MoveObject { start, grab, began: true, deselect: None };
+                    self.state = State::MoveObject { start, grab, hold, began: true, deselect: None };
                     if grab.is_some() {
                         // Alt can go down or up at any time: gather both while nothing has moved.
                         self.anchor_snap = Some(PointSnap::new(cx, || Targets::for_anchor_drag(cx.doc, cx.selection)));
@@ -679,7 +684,7 @@ impl Tool for DirectSelectionTool {
                     let (q, guides) = snap.snap_from(cx, grab + (p - start), shift.as_ref());
                     (d, self.guides) = (q - grab, guides);
                 } else if let Some(snap) = &self.move_snap {
-                    (d, self.guides) = if ev.mods.shift { snap.snap_along(cx, start, d, ev.mods.alt) } else { snap.snap(cx, start, d, ev.mods.alt) };
+                    (d, self.guides) = if ev.mods.shift { snap.snap_along(cx, hold, d, ev.mods.alt) } else { snap.snap(cx, hold, d, ev.mods.alt) };
                 }
                 self.snapped = snapped(d, unsnapped, ev.mods.shift, &self.guides);
                 out.push(Action::Preview(

crates/tools/src/guides.rs

@@ -1225,6 +1225,24 @@ fn touch_params(seg: &PathSeg, at: Point, perpendicular: bool) -> Vec<f64> {
     out
 }
 
+/// Where a press at `p` takes hold of `path` by its outline: the point of the path nearest `p`,
+/// when it is within `reach`, and on a straight segment exactly on its line. A path dragged by a
+/// press on its stroke or beside its edge snaps by that point of the path, so it is the path that
+/// lands on a ruler guide or lines up, not the pressed spot a few pixels beside it (#3701).
+pub(crate) fn hold_on_path(path: &vectorcraft_geom::PathData, p: Point, reach: f64) -> Option<Point> {
+    let (si, seg, _, q, d) = path.nearest(p)?;
+    if d > reach {
+        return None;
+    }
+    let sp = path.subpaths.get(si)?;
+    if !sp.segment_is_line(seg) {
+        return Some(q);
+    }
+    let c = sp.segment(seg);
+    let line = Line::new(c.p0, c.p3);
+    Some(line.eval(line.nearest(p, 0.0).t))
+}
+
 /// Snap `p` for a drawing tool when smart guides (or grid snapping) are on.
 pub fn snap_draw(cx: &ToolContext, p: Point, exclude: &[NodeId]) -> (Point, Vec<Overlay>) {
     snap_with(cx, p, || Targets::collect(cx.doc, exclude, None))

crates/tools/src/select.rs

@@ -27,13 +27,13 @@ use std::cell::RefCell;
 use std::collections::HashMap;
 
 use serde_json::{Value, json};
-use vectorcraft_doc::hit::{hit_test, marquee, objects_at};
+use vectorcraft_doc::hit::{Hit, HitKind, hit_test, marquee, objects_at};
 use vectorcraft_doc::{NodeId, OrientedBox};
 use vectorcraft_geom::{Affine, Point, Rect, Vec2};
 
 use crate::bbox::{Handle, hit_handle, in_rotate_zone, move_delta, rotate_for_drag, scale_for_drag};
 use crate::corners::{self, CornerDrag, over_widget};
-use crate::guides::{PointSnap, Targets};
+use crate::guides::{PointSnap, Targets, hold_on_path};
 use crate::pathtype::{self, BracketDrag, over_bracket};
 use crate::pie::{self, PieDrag};
 use crate::rulerguide::GuideEdit;
@@ -48,6 +48,9 @@ enum State {
     /// Pressed on an object; becomes Moving after the drag threshold.
     Moving {
         start: Point,
+        /// The point that snaps, moving with the pointer: `start`, or where a press on a path's
+        /// stroke or beside its edge took hold of the path ([`hold_point`]).
+        hold: Point,
         began: bool,
         /// Shift-pressed on this selected object: released without a drag, it leaves the
         /// selection.
@@ -397,6 +400,14 @@ pub(crate) fn bbox_handle_at(cx: &ToolContext, p: Point) -> bool {
 /// to label it while hovering.
 const HOVER_REACH: f64 = 6.0;
 
+/// Where a press at `p` holds the object it hit (`h`) for a move: the nearest point of the path
+/// hit, when the press is on its stroke or outline or within [`HOVER_REACH`] of it, else `p`
+/// ([`hold_on_path`]). That point snaps, so the path itself lands on a ruler guide (#3701).
+pub(crate) fn hold_point(cx: &ToolContext, h: &Hit, p: Point) -> Point {
+    let reach = if matches!(h.kind, HitKind::Stroke | HitKind::Outline) { f64::INFINITY } else { cx.tol(HOVER_REACH) };
+    cx.doc.node(h.leaf).and_then(|n| n.path_data()).and_then(|pd| hold_on_path(pd, p, reach)).unwrap_or(p)
+}
+
 impl SelectionTool {
     fn drag_threshold(cx: &ToolContext) -> f64 {
         cx.tol(3.0)
@@ -560,7 +571,8 @@ impl Tool for SelectionTool {
         let m = ev.mods;
         // A press near a centre or an anchor grabs the object by it.
         self.hover = None;
-        let grab = if matches!(ev.kind, PointerKind::Down) { Self::hover_point(cx, p).map_or(p, |(q, _)| q) } else { p };
+        let grabbed = if matches!(ev.kind, PointerKind::Down) { Self::hover_point(cx, p).map(|(q, _)| q) } else { None };
+        let grab = grabbed.unwrap_or(p);
         if matches!(ev.kind, PointerKind::Move) && matches!(self.state, State::Idle) {
             self.hover = Self::hover_point(cx, p);
         }
@@ -658,7 +670,7 @@ impl Tool for SelectionTool {
                     && cx.select_behind
                     && let Some(behind) = object_behind(cx, p)
                 {
-                    self.state = State::Moving { start: p, began: false, deselect: None, key: None };
+                    self.state = State::Moving { start: p, hold: p, began: false, deselect: None, key: None };
                     return vec![Action::Exec("select.set".into(), json!({ "ids": [behind.0] }))];
                 }
                 match hit_test(cx.doc, p, cx.selection_hit_options()) {
@@ -679,7 +691,10 @@ impl Tool for SelectionTool {
                         } else if (cx.selection.objects.len() > 1 || !cx.selection.guides.is_empty()) && !m.cmd && !m.alt {
                             key = Some(top);
                         }
-                        self.state = State::Moving { start: grab, began: false, deselect, key };
+                        // Grabbed by no anchor or centre, a press on a path's stroke or beside its
+                        // edge holds the path there, and that point of it snaps (#3701).
+                        let hold = if grabbed.is_some() { grab } else { hold_point(cx, &h, p) };
+                        self.state = State::Moving { start: grab, hold, began: false, deselect, key };
                         out
                     }
                     None => {
@@ -688,7 +703,7 @@ impl Tool for SelectionTool {
                     }
                 }
             }
-            (PointerKind::Drag, State::Moving { start, began, .. }) => {
+            (PointerKind::Drag, State::Moving { start, hold, began, .. }) => {
                 let mut out = vec![];
                 if !began {
                     if p.distance(start) < Self::drag_threshold(cx) {
@@ -700,11 +715,11 @@ impl Tool for SelectionTool {
                 let mut d = move_delta(start, p, m.shift);
                 let unsnapped = d;
                 if let Some(snap) = &self.moving {
-                    (d, self.guides) = if m.shift { snap.snap_along(cx, start, d, m.alt) } else { snap.snap(cx, start, d, m.alt) };
+                    (d, self.guides) = if m.shift { snap.snap_along(cx, hold, d, m.alt) } else { snap.snap(cx, hold, d, m.alt) };
                 }
                 // Shift has already constrained `d`: any change is a snap.
                 self.snapped = (d - unsnapped).hypot() > 1e-9;
-                self.state = State::Moving { start, began: true, deselect: None, key: None };
+                self.state = State::Moving { start, hold, began: true, deselect: None, key: None };
                 self.measure = cx.measurement_labels.then(|| (p, cx.offset_label(d.x, d.y)));
                 out.push(Action::Preview("object.transform".into(), json!({ "matrix": matrix_json(Affine::translate(d)), "copy": m.alt })));
                 out