M1: address guide snapping review findings

Hidden containers hide their whole branch from snapping, the 5 px reach is converted through the owner's scaled axis so a zoomed frame snaps at 5 screen pixels, and a click on a guide only selects it. Tests cover all three in guide_snap_tests.rs.

Signed-off-by: Ahmed Mahmoud <ahmedmhmouad41@gmail.com>

crates/engine/src/canvas/snap.rs

@@ -159,13 +159,21 @@ impl Targets {
                 t.ys.push((y, b.x0, b.x1));
             }
         }
-        // Everything in the frame. Nested layers count, so a guide over a group reaches its parts.
-        for id in doc.subtree(owner).into_iter().skip(1) {
-            if doc.node(id).is_some_and(|n| n.visible)
-                && let Some(b) = bbox(id)
-            {
+        // Everything in the frame. Nested layers count, so a guide over a group reaches its parts; a
+        // hidden layer hides its whole branch, so a visible child of a hidden group isn't a target.
+        let mut stack: Vec<(NodeId, bool)> = doc.node(owner).map(|n| n.children.iter().rev().map(|c| (*c, true)).collect()).unwrap_or_default();
+        let mut seen = 0;
+        while let Some((id, visible)) = stack.pop() {
+            seen += 1;
+            if seen > 5_000_000 {
+                break;
+            }
+            let Some(n) = doc.node(id) else { continue };
+            let visible = visible && n.visible;
+            if visible && let Some(b) = bbox(id) {
                 t.add_box(b);
             }
+            stack.extend(n.children.iter().rev().map(|c| (*c, visible)));
         }
         t
     }
@@ -402,7 +410,11 @@ impl Targets {
 /// its own axis, within `SNAP_PX` screen pixels at `zoom`. Returns the new offset in `owner`'s
 /// coordinates, or `None` when nothing is in reach.
 pub fn snap_guide(doc: &Document, page: NodeId, owner: NodeId, vertical: bool, offset: f64, zoom: f64) -> Option<f64> {
-    let threshold = SNAP_PX / zoom.max(1e-6);
+    // The guide moves along the owner's own axis (x for a vertical guide, y for a horizontal one),
+    // which the owner's transform may scale: convert the screen-pixel reach into owner units.
+    let [a, b, c, d, _, _] = doc.world_transform(owner).as_coeffs();
+    let axis = if vertical { a.hypot(b) } else { c.hypot(d) };
+    let threshold = SNAP_PX / (zoom.max(1e-6) * axis.max(1e-6));
     Targets::gather_for_guide(doc, page, owner).snap_axis(vertical, offset, threshold).map(|d| offset + d).filter(|v| v.is_finite())
 }
 

crates/engine/src/guide_snap_tests.rs

@@ -88,6 +88,41 @@ fn a_frame_guide_snaps_along_the_frames_own_axes_when_it_is_turned() {
     assert!(snap_guide(doc, page, frame, false, 21.0, 1.0).is_some_and(|v| near(v, 20.0)));
 }
 
+#[test]
+fn a_hidden_container_hides_its_children_from_snapping() {
+    let mut s = Session::new();
+    let frame = id(s.execute("node.create", &json!({"type": "frame", "x": 100, "y": 50, "width": 300, "height": 200})).unwrap());
+    let inner = id(s.execute("node.create", &json!({"type": "frame", "parent": frame, "x": 10, "y": 10, "width": 60, "height": 60})).unwrap());
+    let child = id(s.execute("node.create", &json!({"type": "rectangle", "parent": inner, "x": 20, "y": 20, "width": 20, "height": 20})).unwrap());
+    let page = s.page().unwrap();
+    let snap = |s: &Session, offset: f64| snap_guide(s.doc().unwrap(), page, frame, true, offset, 1.0);
+    // The inner frame's left edge is 10 in the frame; the nested rectangle's is 30.
+    assert_eq!(snap(&s, 11.0), Some(10.0));
+    assert_eq!(snap(&s, 31.0), Some(30.0));
+    // Hiding the inner frame hides its branch: the child's own flag stays true, but its edge is
+    // not a target any more (nor is the hidden frame's).
+    s.execute("node.set", &json!({"ids": [inner], "props": {"visible": false}})).unwrap();
+    assert!(s.doc().unwrap().node(child).unwrap().visible);
+    assert_eq!(snap(&s, 11.0), None);
+    assert_eq!(snap(&s, 31.0), None);
+}
+
+#[test]
+fn the_reach_is_five_screen_pixels_across_a_scaled_frame() {
+    let mut s = Session::new();
+    let frame = id(s.execute("node.create", &json!({"type": "frame", "x": 100, "y": 50, "width": 300, "height": 200})).unwrap());
+    s.execute("node.create", &json!({"type": "rectangle", "parent": frame, "x": 100, "y": 10, "width": 20, "height": 20})).unwrap();
+    // The frame is drawn at twice its own size, so an owner unit is two screen pixels at zoom 1.
+    s.doc_mut().unwrap().node_mut(frame).unwrap().transform = Affine::scale(2.0);
+    let page = s.page().unwrap();
+    let snap = |s: &Session, offset: f64, zoom: f64| snap_guide(s.doc().unwrap(), page, frame, true, offset, zoom);
+    // Five screen pixels are 2.5 frame units: two snaps to the edge at 100, three doesn't.
+    assert_eq!(snap(&s, 102.0, 1.0), Some(100.0));
+    assert_eq!(snap(&s, 103.0, 1.0), None);
+    // Zooming in halves the owner-space reach again (1.25 frame units at 2x zoom).
+    assert_eq!(snap(&s, 102.0, 2.0), None);
+}
+
 #[test]
 fn a_guide_with_nothing_near_stays_where_it_is() {
     let s = Session::new();

crates/ui-egui/src/canvas/guides.rs

@@ -23,7 +23,8 @@ enum GuideDrag {
     /// Dragging a new guide out of a ruler.
     New { vertical: bool, frame: Option<NodeId> },
     /// Moving guide `index` of `owner` (a page or frame); `copy` (Alt at the press) drops a new one.
-    Move { owner: NodeId, index: usize, vertical: bool, copy: bool, frame: Option<NodeId> },
+    /// `from` is where it was pressed, so a click without movement only selects it.
+    Move { owner: NodeId, index: usize, vertical: bool, copy: bool, frame: Option<NodeId>, from: Pos2 },
 }
 
 impl GuideDrag {
@@ -204,7 +205,11 @@ pub fn input(app: &mut FigApp, ctx: &egui::Context, pos: Pos2) -> bool {
             app.session.ds().map(|d| (d.view.snap_pixel, (d.view.snap_objects && !suspend).then_some(d.view.zoom))).unwrap_or((true, None));
         let at = match drag {
             GuideDrag::New { vertical, .. } => drop_target(app, p, vertical, pixel, object_zoom),
-            GuideDrag::Move { owner, index, vertical, .. } => through(app, owner, index, vertical, p, pixel, object_zoom),
+            // A click only selects the guide: object snapping starts once the pointer moves.
+            GuideDrag::Move { owner, index, vertical, from, .. } => {
+                let moved = (pos - from).length() > 0.5;
+                through(app, owner, index, vertical, p, pixel, if moved { object_zoom } else { None })
+            }
         };
         // Alt: distances from the guide to the layers in the frame it crosses.
         let frame = drag.frame().or_else(|| at.map(|g| g.owner).filter(|o| app.session.ds().is_ok_and(|d| d.page != *o)));
@@ -283,7 +288,7 @@ pub fn input(app: &mut FigApp, ctx: &egui::Context, pos: Pos2) -> bool {
                 app.ui.context_menu_shown = false;
             } else {
                 ctx.data_mut(|d| {
-                    d.insert_temp(drag_id(), GuideDrag::Move { owner: g.owner, index: g.index, vertical: g.vertical, copy: alt, frame })
+                    d.insert_temp(drag_id(), GuideDrag::Move { owner: g.owner, index: g.index, vertical: g.vertical, copy: alt, frame, from: pos })
                 });
             }
         }

crates/ui-egui/src/guide_snap_tests.rs

@@ -85,6 +85,19 @@ fn with_snap_to_objects_off_the_guide_keeps_its_pixel_rounding() {
     assert_eq!(guides(&u, page(&u)), vec![(false, 102.0)], "rounded to the pixel, not the edge");
 }
 
+#[test]
+fn a_click_on_a_guide_only_selects_it() {
+    let mut s = Session::new();
+    s.execute("node.create", &json!({"type": "rectangle", "x": 100, "y": 100, "width": 120, "height": 80})).unwrap();
+    s.execute("guide.add", &json!({"vertical": false, "offset": 102})).unwrap();
+    let mut u = rulers(s);
+    // Two pixels above the rectangle's top edge, but a click without movement leaves it there.
+    let at = u.app.to_screen(500.0, 102.0);
+    u.button(at, true);
+    u.button(at, false);
+    assert_eq!(guides(&u, page(&u)), vec![(false, 102.0)], "a click must not snap it to the edge at 100");
+}
+
 #[test]
 fn cmd_or_ctrl_suspends_guide_snapping() {
     let mut s = Session::new();