Single-hole snapping takes only shown points and checks the face once per pick (#798)

Follow-up to #718: a face place snaps only to sketch points that are shown, and the face under the cursor is looked up once per pick.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

crates/ui-egui/src/dialogs.rs

@@ -10,7 +10,7 @@ use solidcraft_engine::Sel;
 use solidcraft_engine::Session;
 use solidcraft_engine::doc::expr::Kind as ValueKind;
 use solidcraft_engine::doc::{AxisRef, Direction, FeatureKind, FilletStyle, HoleKind, Operation, PatternKind, PlaneRef, ProfileSel};
-use solidcraft_engine::geom::Vec3;
+use solidcraft_engine::geom::{Aabb3, Vec3};
 
 use crate::SolveApp;
 use crate::selection::{self, AXES, Accept, BODIES, CURVES, EDGES, FACES, PLANAR_FACES, PLANES, PROFILES, SelInput};
@@ -747,10 +747,32 @@ impl Dialog {
         self.candidate_on(s, h, None)
     }
 
-    /// The selection a hit would add to the active input, knowing the face under the cursor (the
-    /// nearest face the pick ray meets). A face place picked on or near a sketch point that lies
-    /// on that face (a circle's centre) is taken at the point, as a points input picks it.
-    pub fn candidate_on(&self, s: &Session, h: &Hit, face: Option<&Hit>) -> Option<Sel> {
+    /// The face under the cursor (the nearest face the pick ray meets, first among `hits`), looked
+    /// up once for all the sketch points among `hits` when the active input snaps face places to
+    /// them (the Single hole's); `None` otherwise.
+    pub(crate) fn face_snap(&self, s: &Session, hits: &[Hit]) -> Option<FaceSnap> {
+        if !self.active_input().is_some_and(|i| snaps_to_points(i.accept)) {
+            return None;
+        }
+        let Some(Hit::Face { body, index, .. }) = hits.iter().find(|h| matches!(h, Hit::Face { .. })) else { return None };
+        let st = s.world_state();
+        let near: Vec<Vec3> = hits
+            .iter()
+            .filter_map(|h| match h {
+                Hit::SketchPoint { sketch, at, .. } => st.sketch(*sketch).map(|ss| ss.plane.to_world(*at)),
+                _ => None,
+            })
+            .collect();
+        if near.is_empty() {
+            return None;
+        }
+        Some(FaceSnap::new(s, body, *index, &near))
+    }
+
+    /// The selection a hit would add to the active input, given the face under the cursor
+    /// ([`Dialog::face_snap`] of the same hits). A face place picked on or near a sketch point that
+    /// lies on that face (a circle's centre) is taken at the point, as a points input picks it.
+    pub(crate) fn candidate_on(&self, s: &Session, h: &Hit, snap: Option<&FaceSnap>) -> Option<Sel> {
         let inp = self.active_input()?;
         // Features are also picked on the canvas: a face stands for the feature that made it.
         if inp.accept == selection::FEATURES {
@@ -759,16 +781,14 @@ impl Dialog {
                 _ => None,
             };
         }
-        if inp.accept & selection::POINTS != 0
-            && inp.accept & PLANAR_FACES != 0
+        if snaps_to_points(inp.accept)
             && let Hit::SketchPoint { sketch, at, .. } = h
         {
-            return match face {
-                Some(Hit::Face { body, index, .. }) => face_place_at_sketch_point(s, *sketch, *at, body, *index),
-                _ => None,
-            };
+            let snap = snap?;
+            return snap.place_at(s.world_state().sketch(*sketch)?.plane.to_world(*at));
         }
-        inp.accepts(h, |body, face| planar_face(s, body, face).is_some())
+        // The snap looked the face under the cursor up already, planar or not.
+        inp.accepts(h, |body, face| snap.and_then(|f| f.planar(body, face)).unwrap_or_else(|| planar_face(s, body, face).is_some()))
     }
 
     /// A pick in the viewport (already accepted by the active input).
@@ -855,20 +875,248 @@ fn default_profiles(s: &Session) -> Vec<Sel> {
     }
 }
 
-/// A place on planar face `face` of `body` at a sketch point (a circle's centre or a point), when
-/// the point lies on that face (within a micrometre); `None` otherwise.
-fn face_place_at_sketch_point(s: &Session, sketch: u64, at: solidcraft_engine::geom::Vec2, body: &str, face: usize) -> Option<Sel> {
-    let st = s.world_state();
-    let p = st.sketch(sketch)?.plane.to_world(at);
-    let m = st.body(body)?.mesh();
-    // The same wherever the part is: a planar face's triangles lie in its plane.
-    let tol = 1e-3;
-    let on = m
-        .triangles
-        .iter()
-        .zip(&m.tri_face)
-        .any(|(t, f)| *f as usize == face && m.tri(t).is_some_and(|[a, b, c]| crate::preview::point_triangle_dist(p, a, b, c) < tol));
-    (p.is_finite() && on && planar_face(s, body, face).is_some()).then(|| Sel::Face { body: body.to_string(), index: face, point: p })
+/// Does an input take face places that snap to sketch points on the face (the Single hole's)?
+pub(crate) fn snaps_to_points(accept: Accept) -> bool {
+    accept & selection::POINTS != 0 && accept & PLANAR_FACES != 0
+}
+
+/// Which of a sketch's points a face place snaps to (the Single hole's): the points of geometry
+/// the sketch shows. Not a text outline's points, which are never drawn or picked on their own
+/// (as `view::sketch_point_visible` has it), nor projected points while the sketch hides
+/// projected geometry (Show Projected off), whose curves aren't picked either. Hidden sketches
+/// offer no points at all. Show Points doesn't count: it only hides the markers of the sketch
+/// being edited, and a finished sketch's circle centres are snapped to without markers.
+pub(crate) struct SnapPoints<'a> {
+    /// Show Projected is off: no linked point is taken.
+    hide_linked: bool,
+    /// The text sources' link ids (looked up once, however many points and links there are).
+    text: std::collections::HashSet<&'a str>,
+}
+
+impl<'a> SnapPoints<'a> {
+    pub(crate) fn of(sk: &'a solidcraft_engine::sketch::Sketch) -> SnapPoints<'a> {
+        let hide_linked = sk.view.hide_projected;
+        let text = if hide_linked {
+            Default::default()
+        } else {
+            // An id names its first link, as `Sketch::link` finds it.
+            let mut seen = std::collections::HashSet::new();
+            sk.links
+                .iter()
+                .filter(|l| seen.insert(l.id.as_str()))
+                .filter(|l| l.kind == solidcraft_engine::sketch::LinkKind::Text)
+                .map(|l| l.id.as_str())
+                .collect()
+        };
+        SnapPoints { hide_linked, text }
+    }
+
+    /// Does a face place snap to point `p` of the sketch?
+    pub(crate) fn takes(&self, p: &solidcraft_engine::sketch::SPoint) -> bool {
+        match p.link.as_deref() {
+            None => true,
+            Some(_) if self.hide_linked => false,
+            Some(l) => !self.text.contains(l),
+        }
+    }
+}
+
+/// How near a sketch point must be to a face to lie on it: a micrometre, wherever the part is (a
+/// planar face's triangles lie in its plane), by the true distance ([`triangle_dist`]).
+const ON_FACE: f64 = 1e-3;
+
+/// The distance from `p` to the triangle `abc`, exactly: from its plane when `p` is over it,
+/// otherwise from its nearest side. (`preview::point_triangle_dist` takes a point just past a
+/// side for one over the triangle, with an absolute tolerance that is wide for a sliver.)
+fn triangle_dist(p: Vec3, a: Vec3, b: Vec3, c: Vec3) -> f64 {
+    let sides = [(a, b), (b, c), (c, a)];
+    let to_sides = || sides.iter().map(|(u, v)| p.dist_to_segment(*u, *v)).fold(f64::INFINITY, f64::min);
+    // However small the triangle: only one without area (or not finite) has no plane.
+    let n = (b - a).cross(c - a);
+    let len = n.len();
+    if !(len > 0.0 && len.is_finite()) {
+        return to_sides();
+    }
+    let unit = n / len;
+    let h = (p - a).dot(unit);
+    let q = p - unit * h;
+    if sides.iter().all(|(u, v)| (*v - *u).cross(q - *u).dot(unit) >= 0.0) { h.abs() } else { to_sides() }
+}
+
+/// The most point-in-triangle tests one pick makes for the snap. Points past it aren't snapped
+/// to (the face as picked still is), so no sketch, however dense, can stall the viewport.
+const SNAP_BUDGET: usize = 1 << 18;
+
+#[cfg(test)]
+thread_local! {
+    /// A smaller [`SNAP_BUDGET`] for a test that runs it out.
+    static TEST_BUDGET: std::cell::Cell<Option<usize>> = const { std::cell::Cell::new(None) };
+}
+
+/// Run the snap with a budget of `n` tests per pick on this thread (`None`: [`SNAP_BUDGET`]).
+#[cfg(test)]
+pub(crate) fn set_snap_budget(n: Option<usize>) {
+    TEST_BUDGET.with(|b| b.set(n));
+}
+
+fn snap_budget() -> usize {
+    #[cfg(test)]
+    if let Some(n) = TEST_BUDGET.with(std::cell::Cell::get) {
+        return n;
+    }
+    SNAP_BUDGET
+}
+
+/// The face under the cursor, looked up once for all the sketch points one pick offers a face
+/// place to (the Single hole's snap): whether it is planar, its plane, and those of its
+/// triangles near the points on that plane, with their bounding boxes. A dense sketch over a
+/// finely tessellated part costs one planarity lookup and one pass over the body's triangles per
+/// pick; then a point off the plane is turned down at once, and a point on it is tested against
+/// the face's triangles near the cursor, within [`SNAP_BUDGET`] tests in all.
+pub(crate) struct FaceSnap {
+    body: String,
+    face: usize,
+    /// A point of the plane, its unit normal, and how far a point on the face may be from it
+    /// (the triangles' own distance plus [`ON_FACE`]); `None` when the face isn't planar.
+    plane: Option<(Vec3, Vec3, f64)>,
+    tris: Vec<([Vec3; 3], Aabb3)>,
+    /// Point-in-triangle tests left for this pick.
+    budget: std::cell::Cell<usize>,
+}
+
+impl FaceSnap {
+    /// Face `face` of `body`, for sketch points at `near` (in the world).
+    fn new(s: &Session, body: &str, face: usize, near: &[Vec3]) -> FaceSnap {
+        let mut snap = FaceSnap { body: body.to_string(), face, plane: None, tris: Vec::new(), budget: std::cell::Cell::new(snap_budget()) };
+        let st = s.world_state();
+        let (Some((origin, normal)), Some(b)) = (planar_face(s, body, face), st.body(body)) else { return snap };
+        let m = b.mesh();
+        #[cfg(test)]
+        SNAP_WORK.with(|w| w.set((w.get().0, w.get().1 + m.triangles.len())));
+        let tris: Vec<[Vec3; 3]> = m.triangles.iter().zip(&m.tri_face).filter(|(_, f)| **f as usize == face).filter_map(|(t, _)| m.tri(t)).collect();
+        // Anything within ON_FACE of a triangle is within this of the plane.
+        let stray = tris.iter().flatten().map(|v| (*v - origin).dot(normal).abs()).fold(0.0, f64::max);
+        let plane = (origin, normal, stray + ON_FACE);
+        let mut region = Aabb3::EMPTY;
+        for p in near.iter().filter(|p| near_plane(plane, **p)) {
+            region.add(*p);
+        }
+        snap.tris = tris
+            .into_iter()
+            .map(|t| {
+                let mut b = Aabb3::EMPTY;
+                t.iter().for_each(|v| b.add(*v));
+                (t, b)
+            })
+            .filter(|(_, b)| boxes_meet(b, &region, ON_FACE))
+            .collect();
+        snap.plane = Some(plane);
+        snap
+    }
+
+    /// Is face `face` of `body` planar, when it is this snap's face?
+    fn planar(&self, body: &str, face: usize) -> Option<bool> {
+        (self.face == face && self.body == body).then_some(self.plane.is_some())
+    }
+
+    /// The face place at `p` (one of the points the snap was made for), when `p` lies on the face.
+    fn place_at(&self, p: Vec3) -> Option<Sel> {
+        let plane = self.plane?;
+        if !(p.is_finite() && near_plane(plane, p)) {
+            return None;
+        }
+        let at = Aabb3 { min: p, max: p };
+        for ([a, b, c], bb) in &self.tris {
+            let left = self.budget.get();
+            if left == 0 {
+                return None;
+            }
+            self.budget.set(left - 1);
+            #[cfg(test)]
+            SNAP_WORK.with(|w| w.set((w.get().0, w.get().1 + 1)));
+            if boxes_meet(bb, &at, ON_FACE) && triangle_dist(p, *a, *b, *c) < ON_FACE {
+                return Some(Sel::Face { body: self.body.clone(), index: self.face, point: p });
+            }
+        }
+        None
+    }
+}
+
+/// Is `p` within reach of the plane (a point of it, unit normal, reach)?
+fn near_plane((origin, normal, reach): (Vec3, Vec3, f64), p: Vec3) -> bool {
+    (p - origin).dot(normal).abs() <= reach
+}
+
+/// Do boxes `a` and `b` meet once `b` is grown by `pad` on every side?
+fn boxes_meet(a: &Aabb3, b: &Aabb3, pad: f64) -> bool {
+    a.min.x <= b.max.x + pad
+        && b.min.x - pad <= a.max.x
+        && a.min.y <= b.max.y + pad
+        && b.min.y - pad <= a.max.y
+        && a.min.z <= b.max.z + pad
+        && b.min.z - pad <= a.max.z
+}
+
+#[cfg(test)]
+thread_local! {
+    /// Planar face lookups (each tessellates the body) and triangles the snap looked at, on this
+    /// thread: tests bound the work of a pick with them.
+    static SNAP_WORK: std::cell::Cell<(usize, usize)> = const { std::cell::Cell::new((0, 0)) };
+}
+
+/// (planar face lookups, triangles the snap looked at) on this thread since the last call.
+#[cfg(test)]
+pub(crate) fn take_snap_work() -> (usize, usize) {
+    SNAP_WORK.with(|w| w.replace((0, 0)))
+}
+
+#[cfg(test)]
+mod face_snap_tests {
+    use super::*;
+
+    #[test]
+    fn a_point_more_than_a_micrometre_off_a_sliver_is_off_the_face() {
+        // A triangle with half-micrometre legs. Points 2 µm below its lower side and 1.6 µm past
+        // its long side get past `preview::point_triangle_dist`'s inside test (an absolute
+        // tolerance); the snap goes by the true distance and turns them down. Points on a side,
+        // or half a micrometre beyond one, are on the face.
+        let (a, b, c) = (Vec3::ZERO, Vec3::new(5e-4, 0.0, 0.0), Vec3::new(0.0, 5e-4, 0.0));
+        let out = Vec3::new(1.0, 1.0, 0.0) * std::f64::consts::FRAC_1_SQRT_2;
+        let mid = Vec3::new(2.5e-4, 2.5e-4, 0.0);
+        let off = [Vec3::new(2.5e-4, -2e-3, 0.0), Vec3::new(1.4e-3, 1.4e-3, 0.0)];
+        let on = [mid, mid + out * 5e-4, Vec3::new(1e-4, -5e-4, 0.0), Vec3::new(1e-4, 1e-4, 5e-4)];
+        let mut bb = Aabb3::EMPTY;
+        [a, b, c].into_iter().for_each(|v| bb.add(v));
+        let snap = FaceSnap {
+            body: "Body1".into(),
+            face: 0,
+            plane: Some((Vec3::ZERO, Vec3::Z, ON_FACE)),
+            tris: vec![([a, b, c], bb)],
+            budget: std::cell::Cell::new(SNAP_BUDGET),
+        };
+        for p in off {
+            assert!(crate::preview::point_triangle_dist(p, a, b, c) < ON_FACE, "the inside test alone takes {p:?}");
+            assert!(triangle_dist(p, a, b, c) > 1.5e-3, "{p:?}");
+            assert_eq!(snap.place_at(p), None, "{p:?}");
+        }
+        for p in on {
+            assert_eq!(snap.place_at(p), Some(Sel::Face { body: "Body1".into(), index: 0, point: p }), "{p:?}");
+        }
+    }
+
+    #[test]
+    fn the_distance_to_a_tiny_triangle_is_from_its_plane_over_it() {
+        // Legs of 20 nm: the triangle still has a plane, and a point just under a micrometre
+        // above it is that far from it (not from its nearest side), so it is on the face. A
+        // triangle without area is measured from its sides; nothing here panics on non-finite input.
+        let (a, b, c) = (Vec3::ZERO, Vec3::new(2e-5, 0.0, 0.0), Vec3::new(0.0, 2e-5, 0.0));
+        let p = Vec3::new(5e-6, 5e-6, 0.99999e-3);
+        assert!((triangle_dist(p, a, b, c) - 0.99999e-3).abs() < 1e-12, "{}", triangle_dist(p, a, b, c));
+        assert!((triangle_dist(Vec3::new(0.0, 3.0, 4.0), a, b, b) - 5.0).abs() < 1e-9, "a triangle without area");
+        let far = |d: f64| d.is_nan() || d >= ON_FACE;
+        assert!(far(triangle_dist(Vec3::new(f64::NAN, 0.0, 0.0), a, b, c)), "a point that isn't finite is never near");
+        assert!(far(triangle_dist(p, a, b, Vec3::new(f64::INFINITY, 0.0, 0.0))), "nor is a triangle that isn't finite");
+    }
 }
 
 /// Does a feature build from this sketch's profiles (extrude, revolve, sweep, loft, emboss…)?
@@ -878,6 +1126,8 @@ fn profiles_used(s: &Session, sketch: u64) -> bool {
 
 /// The plane of a planar body face: (point on it, unit normal).
 pub fn planar_face(s: &Session, body: &str, face: usize) -> Option<(Vec3, Vec3)> {
+    #[cfg(test)]
+    SNAP_WORK.with(|w| w.set((w.get().0 + 1, w.get().1)));
     let st = s.world_state();
     let b = st.body(body)?;
     let tol = (b.body.size() * 1e-3).max(1e-3);

crates/ui-egui/src/hole_placement_ui_tests.rs

@@ -1,12 +1,14 @@
 //! Hole placement on the canvas. A Single hole picked on or near a sketch point on the face (a
 //! circle's centre) goes to that point, as a Multiple pick of the same point does (#657); a pick
 //! away from sketch points, or near one off the face under the cursor, stays where it was clicked.
+//! Only points the sketch shows are snapped to (not lettering's outline points), and Select Other
+//! looks the face up once for every point near the cursor.
 
 use serde_json::{Value, json};
 use solidcraft_engine::Sel;
 use solidcraft_engine::doc::FeatureKind;
-use solidcraft_engine::geom::Vec3;
-use solidcraft_engine::sketch::CurveKind;
+use solidcraft_engine::geom::{Aabb3, Vec2, Vec3};
+use solidcraft_engine::sketch::{CurveKind, LinkKind, SPoint};
 
 use crate::dialogs::Kind;
 use crate::scenario::Harness;
@@ -236,3 +238,289 @@ fn select_other_keeps_the_face_as_picked_beside_many_snapped_points() {
     assert_eq!((count("Point"), count("Face")), (crate::select_other::MAX_PER_KIND, 1), "{list}");
     assert_eq!(list["total"], json!(many + 1), "{list}");
 }
+
+/// The block, seen from the top, with an "L" 10 mm high written on its top face from (5, 5), and
+/// with a circle of radius 0.5 about `circle` (millimetres right of the foot's lower right corner)
+/// when given. Returns the corner, then the circle centre, in the world.
+fn block_with_lettering(circle: Option<f64>) -> (Harness, Vec3, Option<Vec3>) {
+    let mut h = Harness::new();
+    h.app.ui.show_origin = false;
+    h.app.run("solid.box", json!({"length": 40, "width": 30, "height": 20})).unwrap();
+    h.app.run("sketch.create", json!({"plane": {"face": [20, 15, 20]}})).unwrap();
+    h.app.run("sketch.text", json!({"text": "L", "at": [5, 5], "height": 10})).unwrap();
+    let outline = |h: &Harness| {
+        let st = h.app.session.world_state();
+        let ss = st.sketches.first().unwrap();
+        let sk = &ss.sketch;
+        let text = |p: &&SPoint| p.link.as_deref().and_then(|l| sk.link(l)).is_some_and(|l| l.kind == LinkKind::Text);
+        sk.points.iter().filter(text).map(|p| p.pos).collect::<Vec<Vec2>>()
+    };
+    let corner = outline(&h).into_iter().max_by(|a, b| (a.x - a.y).total_cmp(&(b.x - b.y))).unwrap();
+    let centre = circle.map(|dx| Vec2::new(corner.x + dx, corner.y));
+    if let Some(c) = centre {
+        h.app.run("sketch.circle.center", json!({"center": [c.x, c.y], "radius": 0.5})).unwrap();
+    }
+    h.app.run("sketch.finish", json!({})).unwrap();
+    h.step(&json!({"call": "ui.view", "params": {"view": "top"}})).unwrap();
+    h.settle();
+    let st = h.app.session.world_state();
+    let plane = st.sketches.first().unwrap().plane;
+    assert_eq!(outline(&h).len(), 6, "the L's outline has six corners");
+    (h, plane.to_world(corner), centre.map(|c| plane.to_world(c)))
+}
+
+/// Where the ray through a screen point meets the nearest face.
+fn face_point_at(h: &Harness, at: [f64; 2]) -> Vec3 {
+    let ray = hits_at(h, at);
+    match ray.iter().find(|x| matches!(x, Hit::Face { .. })) {
+        Some(Hit::Face { point, .. }) => *point,
+        _ => panic!("no face under {at:?}: {ray:?}"),
+    }
+}
+
+#[test]
+fn a_single_hole_picked_beside_lettering_stays_where_it_was_clicked() {
+    // Text outlines' points are not drawn and not picked on their own (#718 review): a click 5 px
+    // off a letter's corner keeps the face point it was clicked at.
+    let (mut h, corner, _) = block_with_lettering(None);
+    assert!((corner.z - 20.0).abs() < 1e-9, "the lettering lies on the top face: {corner:?}");
+    let click = near(&mut h, corner);
+    h.step(&json!({"start": "solid.hole"})).unwrap();
+    let raw = face_point_at(&h, click);
+    h.step(&json!({"move": click})).unwrap();
+    assert!(matches!(&h.app.viewport.hover, Some(Hit::Face { .. })), "the face lights up, not a letter's corner: {:?}", h.app.viewport.hover);
+    h.step(&json!({"click": click})).unwrap();
+    let places = face_places(&h);
+    assert!(matches!(places.as_slice(), [(_, p)] if p.dist(raw) < 1e-9), "the face is picked where clicked {raw:?}, not {places:?}");
+    h.step(&json!({"key": "Enter"})).unwrap();
+    assert!(h.app.dialog.is_none(), "OK closed the dialog: {:?}", h.app.dialog.as_ref().map(|d| &d.error));
+    let (_, position, at_points) = hole(&h);
+    assert!(!at_points && position.dist(raw) < 1e-9 && position.dist(corner) > 0.5, "{position:?}");
+}
+
+#[test]
+fn a_single_hole_picked_beside_lettering_snaps_to_a_shown_point_in_reach() {
+    // A circle centre 1.2 mm (8 px) right of a letter's corner. The click is nearer the corner
+    // (2.4 px) but within reach of the centre (5.6 px): the centre is the point the pick snaps to.
+    let (mut h, corner, centre) = block_with_lettering(Some(1.2));
+    let centre = centre.unwrap();
+    let (c, p) = (screen(&mut h, corner), screen(&mut h, centre));
+    let d = ((p[0] - c[0]).powi(2) + (p[1] - c[1]).powi(2)).sqrt();
+    assert!(d > 6.0 && d < 10.0, "the centre is {d} px from the corner");
+    let click = [c[0] + (p[0] - c[0]) * 0.3, c[1] + (p[1] - c[1]) * 0.3];
+    h.step(&json!({"start": "solid.hole"})).unwrap();
+    h.step(&json!({"click": click})).unwrap();
+    let places = face_places(&h);
+    assert!(matches!(places.as_slice(), [(_, p)] if p.dist(centre) < 1e-9), "the face is picked at the centre {centre:?}, not {places:?}");
+}
+
+#[test]
+fn a_single_hole_snaps_only_to_points_its_sketch_shows() {
+    // A face sketch projects the face's edges, so the face's corners are points of the sketch: a
+    // click 5 px inside a corner snaps to it while the sketch shows projected geometry, and stays
+    // where it was clicked while the sketch hides it (Show Projected off).
+    for shown in [true, false] {
+        let mut h = Harness::new();
+        h.app.ui.show_origin = false;
+        h.app.run("solid.box", json!({"length": 40, "width": 30, "height": 20})).unwrap();
+        h.app.run("sketch.create", json!({"plane": {"face": [20, 15, 20]}})).unwrap();
+        h.app.run("sketch.options", json!({"show_projected": shown})).unwrap();
+        h.app.run("sketch.finish", json!({})).unwrap();
+        h.step(&json!({"call": "ui.view", "params": {"view": "top"}})).unwrap();
+        h.settle();
+        let corner = Vec3::new(40.0, 30.0, 20.0);
+        let c = screen(&mut h, corner);
+        let click = [c[0] - 4.0, c[1] + 3.0];
+        h.step(&json!({"start": "solid.hole"})).unwrap();
+        let raw = face_point_at(&h, click);
+        assert!(raw.dist(corner) > 0.5 && (raw.z - 20.0).abs() < 1e-9, "{raw:?}");
+        h.step(&json!({"click": click})).unwrap();
+        let want = if shown { corner } else { raw };
+        let places = face_places(&h);
+        assert!(matches!(places.as_slice(), [(_, p)] if p.dist(want) < 1e-9), "projected shown: {shown}; at {want:?}, not {places:?}");
+    }
+    // A hidden sketch offers no points: its circle's centre is not snapped to.
+    let (mut h, centre) = block_with_circle();
+    let name = h.app.session.doc.features.iter().find(|f| matches!(f.kind, FeatureKind::Sketch { .. })).unwrap().name.clone();
+    h.app.run("browser.visibility", json!({"items": [name], "visible": false})).unwrap();
+    let click = near(&mut h, centre);
+    h.step(&json!({"start": "solid.hole"})).unwrap();
+    let raw = face_point_at(&h, click);
+    h.step(&json!({"click": click})).unwrap();
+    let places = face_places(&h);
+    assert!(matches!(places.as_slice(), [(_, p)] if p.dist(raw) < 1e-9 && p.dist(centre) > 0.1), "{places:?}");
+    // Show Points off only hides the markers of the sketch being edited (a finished sketch draws
+    // none): its circle's centre is still snapped to.
+    let mut h = Harness::new();
+    h.app.ui.show_origin = false;
+    h.app.run("solid.box", json!({"length": 40, "width": 30, "height": 20})).unwrap();
+    h.app.run("sketch.create", json!({"plane": {"face": [20, 15, 20]}})).unwrap();
+    h.app.run("sketch.options", json!({"show_points": false})).unwrap();
+    h.app.run("sketch.circle.center", json!({"center": [12, 9], "radius": 4})).unwrap();
+    h.app.run("sketch.finish", json!({})).unwrap();
+    h.step(&json!({"call": "ui.view", "params": {"view": "top"}})).unwrap();
+    h.settle();
+    let centre = Vec3::new(12.0, 9.0, 20.0);
+    let click = near(&mut h, centre);
+    h.step(&json!({"start": "solid.hole"})).unwrap();
+    h.step(&json!({"click": click})).unwrap();
+    let places = face_places(&h);
+    assert!(matches!(places.as_slice(), [(_, p)] if p.dist(centre) < 1e-9), "Show Points off: at the centre, not {places:?}");
+}
+
+/// Three Select Other lists' worth of sketch points under the cursor, with a Single hole started.
+struct Cluster {
+    h: Harness,
+    /// How many points.
+    many: usize,
+    /// 5 px off them.
+    click: [f64; 2],
+    /// Triangles of the body under the click, of the face under it, and of that face's triangles
+    /// the ones whose bounding box comes within a micrometre of the points' bounding box.
+    body_tris: usize,
+    face_tris: usize,
+    near_tris: usize,
+}
+
+impl Cluster {
+    /// Select Other at the click: the list, then (planar face lookups, triangles the snap looked
+    /// at) while making it.
+    fn select_other(&mut self) -> (Value, (usize, usize)) {
+        crate::dialogs::take_snap_work();
+        let list = self.h.call("ui.selectOther", json!({"x": self.click[0], "y": self.click[1]}))["result"].clone();
+        (list, crate::dialogs::take_snap_work())
+    }
+}
+
+/// How many rows of a Select Other list are labelled `prefix`….
+fn rows_named(list: &Value, prefix: &str) -> usize {
+    list["rows"].as_array().into_iter().flatten().filter(|r| r["label"].as_str().is_some_and(|l| l.starts_with(prefix))).count()
+}
+
+/// A cylinder of radius 15 and height 20 standing on the origin, seen from `view`, with the
+/// points within 0.1 mm of `at` in the sketch `sketch.create` makes with `sketch`.
+fn cylinder_with_cluster(sketch: Value, at: [f64; 2], view: &str) -> Cluster {
+    let mut h = Harness::new();
+    h.app.ui.show_origin = false;
+    h.app.run("solid.cylinder", json!({"radius": 15, "height": 20})).unwrap();
+    h.app.run("sketch.create", sketch).unwrap();
+    let many = 3 * crate::select_other::MAX_PER_KIND;
+    let local: Vec<Vec2> = (0..many).map(|i| i as f64 * 0.3).map(|a| Vec2::new(at[0] + 0.1 * a.cos(), at[1] + 0.1 * a.sin())).collect();
+    for p in &local {
+        h.app.run("sketch.point", json!({"point": [p.x, p.y]})).unwrap();
+    }
+    h.app.run("sketch.finish", json!({})).unwrap();
+    h.step(&json!({"call": "ui.view", "params": {"view": view}})).unwrap();
+    h.settle();
+    let plane = h.app.session.world_state().sketches.first().unwrap().plane;
+    let centre = plane.to_world(Vec2::new(at[0], at[1]));
+    let mut points = Aabb3::EMPTY;
+    local.iter().for_each(|p| points.add(plane.to_world(*p)));
+    let click = near(&mut h, centre);
+    h.step(&json!({"start": "solid.hole"})).unwrap();
+    let ray = hits_at(&h, click);
+    let Some(Hit::Face { body, index, .. }) = ray.iter().find(|x| matches!(x, Hit::Face { .. })) else { panic!("no face under {centre:?}: {ray:?}") };
+    let m = h.app.session.world_state().body(body).unwrap().mesh();
+    let face: Vec<[Vec3; 3]> = m.triangles.iter().zip(&m.tri_face).filter(|(_, f)| **f as usize == *index).filter_map(|(t, _)| m.tri(t)).collect();
+    let near_points = |t: &[Vec3; 3]| {
+        let mut b = Aabb3::EMPTY;
+        t.iter().for_each(|v| b.add(*v));
+        let gap = (b.min - points.max).max(points.min - b.max);
+        gap.x.max(gap.y).max(gap.z) <= 1e-3
+    };
+    let (body_tris, face_tris, near_tris) = (m.triangles.len(), face.len(), face.iter().filter(|t| near_points(t)).count());
+    assert!(body_tris >= 2 * face_tris, "the body has other faces: {body_tris} and {face_tris} triangles");
+    Cluster { h, many, click, body_tris, face_tris, near_tris }
+}
+
+#[test]
+fn select_other_over_a_dense_cluster_looks_the_face_up_once() {
+    // The points on the cylinder's top face, a disc of a tenth of the body's triangles. Select
+    // Other looks the face under the cursor up once for all of them, not once per point (#718
+    // review): one planarity lookup, which tessellates the body, and one pass over the body's
+    // triangles. Each point is then tested against the face's triangles near the points only,
+    // not the face's or the body's; every point on the face is still counted.
+    let mut c = cylinder_with_cluster(json!({"plane": {"face": [0, 0, 20]}}), [5.0, 4.0], "top");
+    let (many, body, face, near) = (c.many, c.body_tris, c.face_tris, c.near_tris);
+    assert!(body >= 8 * face && 4 * near <= face, "{body} triangles in the body, {face} in the face, {near} of them near the points");
+    let (list, (lookups, tested)) = c.select_other();
+    assert_eq!((rows_named(&list, "Point"), rows_named(&list, "Face")), (crate::select_other::MAX_PER_KIND, 1), "{list}");
+    assert_eq!(list["total"], json!(many + 1), "every point on the face is counted: {list}");
+    assert_eq!(lookups, 1, "the face is looked up once for {many} points");
+    assert!(tested <= body + many * near, "{tested} triangles looked at for {many} points ({body} in the body, {near} near the points)");
+}
+
+#[test]
+fn select_other_over_a_dense_cluster_off_the_face_tests_no_triangles() {
+    // The points 10 mm above the cylinder's top face, seen from the top: under the cursor, but
+    // off the face's plane. Each is turned down without a triangle test, and the face as picked
+    // is all that is offered.
+    let mut c = cylinder_with_cluster(json!({"plane": "XY", "offset": 30}), [5.0, 4.0], "top");
+    let (many, body) = (c.many, c.body_tris);
+    let (list, (lookups, tested)) = c.select_other();
+    assert_eq!((rows_named(&list, "Point"), rows_named(&list, "Face")), (0, 1), "{list}");
+    assert_eq!(list["total"], json!(1), "{list}");
+    assert_eq!(lookups, 1, "the face is looked up once for {many} points");
+    assert_eq!(tested, body, "one pass over the body's {body} triangles, and no test for the {many} points off the face");
+}
+
+#[test]
+fn select_other_over_a_dense_cluster_by_a_curved_face_looks_it_up_once() {
+    // The points in a sketch through the cylinder's axis, seen from the front: the face under the
+    // cursor is the curved side, which takes no hole. It is looked up once, its triangles not at
+    // all, and nothing is offered.
+    let mut c = cylinder_with_cluster(json!({"plane": "XZ"}), [5.0, -10.0], "front");
+    let many = c.many;
+    let (list, (lookups, tested)) = c.select_other();
+    assert_eq!(list["open"], json!(false), "nothing for a Single hole here: {list}");
+    assert_eq!(lookups, 1, "the curved face is looked up once for {many} points");
+    assert_eq!(tested, 0, "no triangle of a curved face is looked at");
+}
+
+#[test]
+fn select_other_snaps_no_more_points_than_the_budget_allows() {
+    // With the snap's budget cut to 30 point-in-triangle tests, at most 30 of the points on the
+    // face are offered (each takes one test at least), and the face as picked keeps its row.
+    let mut c = cylinder_with_cluster(json!({"plane": {"face": [0, 0, 20]}}), [5.0, 4.0], "top");
+    let (many, body) = (c.many, c.body_tris);
+    crate::dialogs::set_snap_budget(Some(30));
+    let (list, (_, tested)) = c.select_other();
+    crate::dialogs::set_snap_budget(None);
+    assert!(many > 30, "more points than the budget");
+    assert_eq!(rows_named(&list, "Face"), 1, "{list}");
+    let (points, total) = (rows_named(&list, "Point"), list["total"].as_u64().unwrap() as usize);
+    assert!(points >= 1 && total > points && total <= 30 + 1, "some points within the budget, and the face: {list}");
+    assert!(tested <= body + 30, "{tested} triangles looked at");
+}
+
+#[test]
+fn a_face_place_snaps_to_the_points_of_shown_geometry_only() {
+    // Which points a Single hole snaps to, against the rule the viewport draws them by: not a
+    // text outline's points (`view::sketch_point_visible`), and no linked point while Show
+    // Projected is off. An id names its first link; a point whose link is gone counts as shown.
+    let (h, _, _) = block_with_lettering(None);
+    let mut sk = h.app.session.world_state().sketches.first().unwrap().sketch.clone();
+    let text = sk.links.iter().find(|l| l.kind == LinkKind::Text).unwrap().clone();
+    let edges = sk.links.iter().find(|l| l.kind != LinkKind::Text).unwrap().clone();
+    // Later links reusing the ids, with the other kind: the first link of an id is the one.
+    let (mut not_text, mut now_text) = (text.clone(), edges.clone());
+    not_text.kind = edges.kind;
+    now_text.kind = LinkKind::Text;
+    sk.links.extend([not_text, now_text]);
+    let mut lost = sk.points.last().unwrap().clone();
+    lost.id = "lost".into();
+    lost.link = Some("gone".into());
+    sk.points.push(lost);
+    let linked = |l: &str| sk.points.iter().filter(|p| p.link.as_deref() == Some(l)).count();
+    assert!(linked(&text.id) == 6 && linked(&edges.id) == 4, "the L's corners and the face's");
+    for hide in [false, true] {
+        sk.view.hide_projected = hide;
+        let takes = crate::dialogs::SnapPoints::of(&sk);
+        for (i, p) in sk.points.iter().enumerate() {
+            let want = solidcraft_engine::view::sketch_point_visible(&sk, i) && !(hide && p.link.is_some());
+            assert_eq!(takes.takes(p), want, "point {} (link {:?}), Show Projected off: {hide}", p.id, p.link);
+        }
+        let taken = sk.points.iter().filter(|p| takes.takes(p)).count();
+        assert_eq!(taken, if hide { 1 } else { 1 + 4 + 1 }, "the origin, and unless hidden the face's corners and the lost point");
+    }
+}

crates/ui-egui/src/viewport.rs

@@ -829,10 +829,17 @@ fn pick_hits(app: &SolveApp, proj: &Proj, pos: Pos2, other_sketch_points: bool,
     let external_points =
         wants_points || other_sketch_points || (app.dialog.is_none() && app.tool.as_ref().is_none_or(|tool| tool.cmd == "sketch.project"));
     let point_sketches: Vec<u64> = if external_points { visible_sketches(app) } else { s.active_sketch.into_iter().collect() };
+    // A face place (the Single hole's) snaps only to points their sketches show, so a nearer
+    // point that isn't shown (a letter's outline) neither takes the pick nor hides a shown one.
+    let snapping = app.dialog.as_ref().and_then(|d| d.active_input()).is_some_and(|i| crate::dialogs::snaps_to_points(i.accept));
     let mut best = Near::new(all);
     for ss in point_sketches.iter().filter_map(|sid| st.sketch(*sid)) {
         let sid = ss.feature;
+        let shown = snapping.then(|| crate::dialogs::SnapPoints::of(&ss.sketch));
         for p in &ss.sketch.points {
+            if shown.as_ref().is_some_and(|f| !f.takes(p)) {
+                continue;
+            }
             if let Some(sp) = proj.to_screen(ss.plane.to_world(p.pos)) {
                 let dd = sp.distance(pos) - if s.active_sketch == Some(sid) { 2.0 } else { 0.0 };
                 if dd < 8.0 && best.wants(dd) {
@@ -1215,8 +1222,8 @@ pub fn hit_sel(h: &Hit) -> Option<Sel> {
 /// dialog the first hit (sketch entities only belong to the active sketch).
 pub fn candidate(app: &SolveApp, hits: &[Hit]) -> Option<(Hit, Sel)> {
     if let Some(d) = app.dialog.as_ref().filter(|d| d.wants_picks()) {
-        let face = face_under(hits);
-        return hits.iter().find_map(|h| d.candidate_on(&app.session, h, face).map(|s| (h.clone(), s)));
+        let snap = d.face_snap(&app.session, hits);
+        return hits.iter().find_map(|h| d.candidate_on(&app.session, h, snap.as_ref()).map(|s| (h.clone(), s)));
     }
     hits.iter().filter(|h| pick_allowed(app, h)).find_map(|h| hit_candidate(app, h))
 }
@@ -1224,17 +1231,14 @@ pub fn candidate(app: &SolveApp, hits: &[Hit]) -> Option<(Hit, Sel)> {
 /// Every hit a click could pick, in order, each with its selection (Select Other).
 pub fn candidates(app: &SolveApp, hits: &[Hit]) -> Vec<(Hit, Sel)> {
     if let Some(d) = app.dialog.as_ref().filter(|d| d.wants_picks()) {
-        let face = face_under(hits);
-        return hits.iter().filter_map(|h| d.candidate_on(&app.session, h, face).map(|s| (h.clone(), s))).collect();
+        // The face under the cursor is looked up once for every sketch point a face place may
+        // snap to, before Select Other caps its rows.
+        let snap = d.face_snap(&app.session, hits);
+        return hits.iter().filter_map(|h| d.candidate_on(&app.session, h, snap.as_ref()).map(|s| (h.clone(), s))).collect();
     }
     hits.iter().filter(|h| pick_allowed(app, h)).filter_map(|h| hit_candidate(app, h)).collect()
 }
 
-/// The face under the cursor: the nearest face the pick ray meets (faces come nearest first).
-fn face_under(hits: &[Hit]) -> Option<&Hit> {
-    hits.iter().find(|h| matches!(h, Hit::Face { .. }))
-}
-
 /// What the selection filter lets a click pick with no dialog.
 fn pick_allowed(app: &SolveApp, h: &Hit) -> bool {
     let off = |k: &str| app.ui.pick_off.iter().any(|x| x == k);