Merge pull request #1313 from itrycodinganditsbad/fix-1274

Snap to Point: drag a selection by the anchor pressed near (#1274)

crates/engine/src/tests_prefs.rs

@@ -952,3 +952,51 @@ fn hyphenation_exceptions_preference_reaches_the_hyphenator() {
     set_pref(&mut s, "hyphenationExceptions", json!(""));
     assert!(vectorcraft_text::hyphenation_exceptions().is_empty());
 }
+
+/// #1274: View › Snap to Point lands the anchor a selection is grabbed by (a press within a few
+/// pixels of it) on another object's anchor, with Smart Guides on or off, not the pressed spot
+/// beside it.
+#[test]
+fn snap_to_point_lands_the_grabbed_anchor_on_another_anchor() {
+    use vectorcraft_tools::PointerKind::{Down, Drag, Up};
+    let mut s = new_doc();
+    let a = square(&mut s, 100.0, 100.0);
+    square(&mut s, 300.0, 100.0);
+    for v in [ViewInfo { smart_guides: false, ..ViewInfo::default() }, ViewInfo::default()] {
+        s.select_tool("selection", v).unwrap();
+        // Nothing selected, so no bounding box handle sits on the corner.
+        s.execute("select.none", &json!({})).unwrap();
+        // Pressed 2 px left of and below the top-right anchor (150, 100), released with the
+        // pointer 1.4 px from the other square's top-left anchor (300, 100).
+        gesture(&mut s, v, &[(Down, 148.0, 102.0), (Drag, 200.0, 110.0), (Drag, 299.0, 101.0), (Up, 299.0, 101.0)]);
+        assert_eq!(top_left(&s, a), (250.0, 100.0), "the grabbed anchor lands on the anchor ({v:?})");
+        s.execute("edit.undo", &json!({})).unwrap();
+    }
+}
+
+/// #1274: with Smart Guides off, View › Snap to Point turns the selection arrow hollow over an
+/// anchor a press would grab, and lands a Pen click within 2 px of an anchor on it.
+#[test]
+fn snap_to_point_cues_anchors_and_lands_pen_clicks_on_them() {
+    use vectorcraft_tools::PointerKind::{Down, Up};
+    use vectorcraft_tools::{Cursor, Mods};
+    let mut s = new_doc();
+    square(&mut s, 300.0, 100.0);
+    s.execute("select.none", &json!({})).unwrap();
+    let on = ViewInfo { smart_guides: false, ..ViewInfo::default() };
+    let off = ViewInfo { snap_to_point: false, ..on };
+    s.select_tool("selection", on).unwrap();
+    let near = vectorcraft_geom::Point::new(302.0, 102.0);
+    assert_eq!(s.cursor(near, Mods::default(), on), Cursor::ArrowHollow);
+    assert_eq!(s.cursor(vectorcraft_geom::Point::new(325.0, 125.0), Mods::default(), on), Cursor::Arrow, "away from the anchors");
+    assert_eq!(s.cursor(near, Mods::default(), off), Cursor::Arrow, "Snap to Point off");
+    for (v, first) in [(on, (300.0, 100.0)), (off, (301.0, 101.0))] {
+        s.select_tool("pen", v).unwrap();
+        gesture(&mut s, v, &[(Down, 301.0, 101.0), (Up, 301.0, 101.0), (Down, 200.0, 250.0), (Up, 200.0, 250.0)]);
+        let id = *s.doc().unwrap().selection.objects.first().unwrap();
+        let p = s.doc().unwrap().doc.node(id).unwrap().path_data().unwrap().subpaths[0].anchors[0].p;
+        assert_eq!((p.x, p.y), first, "{v:?}");
+        s.execute("edit.undo", &json!({})).unwrap();
+        s.execute("select.none", &json!({})).unwrap();
+    }
+}

crates/tools/src/select.rs

@@ -79,9 +79,9 @@ pub struct SelectionTool {
 /// Snapping for the selection moved as a whole, its targets gathered when the move begins: View →
 /// Snap to Grid lands its bounds on the grid (#740), taking over from the rest; otherwise Smart
 /// Guides line its bounds up with the other art, and with them off, View → Snap to Point lands the
-/// point it was grabbed by on an anchor or a ruler guide. Snap to Pixel puts its top-left on whole
-/// pixels. A copy (Alt, which can go down or up at any time during the drag) leaves the original
-/// behind, so it snaps to the original too.
+/// point it was grabbed by (an anchor pressed near, else the pressed point) on an anchor or a ruler
+/// guide. Snap to Pixel puts its top-left on whole pixels. A copy (Alt, which can go down or up at
+/// any time during the drag) leaves the original behind, so it snaps to the original too.
 pub(crate) struct MoveSnap {
     bounds: Option<Rect>,
     targets: Option<Targets>,
@@ -194,11 +194,13 @@ impl SelectionTool {
         cx.tol(3.0)
     }
 
-    /// With Smart Guides on, the anchor or the centre of an object near `p` (the one under the
-    /// pointer, else a selected one) and its label, "anchor" or "center", as Illustrator shows
-    /// them while hovering (#812). A press there grabs the object by that point.
+    /// The anchor of an object near `p` (the one under the pointer, else a selected one when a
+    /// press would keep the selection), or with Smart Guides on its centre, and its label,
+    /// "anchor" or "center", as Illustrator shows them while hovering with Smart Guides (#812). A
+    /// press there grabs the object by that point, so with Smart Guides or View → Snap to Point
+    /// that point is the one that snaps (#1274).
     fn hover_point(cx: &ToolContext, p: Point) -> Option<(Point, &'static str)> {
-        if !cx.smart_guides {
+        if !cx.smart_guides && !cx.snap_to_point {
             return None;
         }
         let reach = cx.tol(HOVER_REACH);
@@ -206,10 +208,15 @@ impl SelectionTool {
         let nearest = |points: &mut dyn Iterator<Item = Point>| {
             points.filter(|q| q.distance(p) <= reach).min_by(|a, b| a.distance(p).total_cmp(&b.distance(p)))
         };
-        under.into_iter().chain(cx.selection.objects.iter().copied()).find_map(|id| {
+        // A press on an unselected object selects it alone: the selected ones stay behind.
+        let selected = if under.is_none_or(|u| cx.selection.contains(u)) { &cx.selection.objects[..] } else { &[] };
+        under.into_iter().chain(selected.iter().copied()).find_map(|id| {
             let n = cx.doc.node(id)?;
             let anchor = n.path_data().and_then(|pd| nearest(&mut pd.subpaths.iter().flat_map(|sp| sp.anchors.iter().map(|a| a.p))));
             anchor.map(|q| (q, "anchor")).or_else(|| {
+                if !cx.smart_guides {
+                    return None;
+                }
                 let c = cx.doc.bounds_of(&[id], cx.preview_bounds)?.center();
                 (c.distance(p) <= reach).then_some((c, "center"))
             })
@@ -236,8 +243,9 @@ impl Tool for SelectionTool {
         }
         let p = ev.pos;
         let m = ev.mods;
-        // A press near the hovered centre or anchor grabs the object by it.
-        let grab = self.hover.take().map(|(q, _)| q).filter(|q| q.distance(p) <= cx.tol(HOVER_REACH)).unwrap_or(p);
+        // 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 };
         if matches!(ev.kind, PointerKind::Move) && matches!(self.state, State::Idle) {
             self.hover = Self::hover_point(cx, p);
         }
@@ -450,7 +458,7 @@ impl Tool for SelectionTool {
         }
         o.extend(self.guides.iter().cloned());
         o.extend(self.guide.overlays(cx));
-        if let (Some((q, label)), State::Idle) = (self.hover, &self.state) {
+        if let (Some((q, label)), State::Idle, true) = (self.hover, &self.state, cx.smart_guides) {
             let style = crate::guides::GuideStyle::of(cx);
             if style.labels {
                 o.push(Overlay::Label { p: q, text: label.into(), color: style.color });
@@ -495,6 +503,11 @@ impl Tool for SelectionTool {
         if let Some(c) = self.guide.cursor(cx, p) {
             return c;
         }
+        // View → Snap to Point: over an anchor a press would grab the object by, the arrow turns
+        // hollow.
+        if cx.snap_to_point && matches!(Self::hover_point(cx, p), Some((_, "anchor"))) {
+            return Cursor::ArrowHollow;
+        }
         if let Some(h) = hit_test(cx.doc, p, cx.hit_options()) {
             if cx.selection.contains(h.top_object(cx.isolation)) || m.alt {
                 return Cursor::Move;