fix: a hole at sketch points drills into the part its sketch touches (#910)

Hole centres from a sketch now drill into the material on whichever side of the sketch plane it lies: material the spot starts in wins, then the nearest body ahead, else against the sketch normal as before. A sketch on a part's underside or below it now drills up into the part. Covers counterbores, through-all, edge and corner centres, patterns and reopening.

crates/doc/src/document.rs

@@ -485,7 +485,7 @@ pub enum FeatureKind {
         depth: Option<String>,
         #[serde(default)]
         hole: HoleKind,
-        /// Drill at these sketch points instead (one hole each, perpendicular to the sketch);
+        /// Drill at these sketch points instead (one hole each, perpendicular to the sketch, into the material: against the sketch normal, or along it when only that side has material);
         /// the holes follow the points when the sketch changes.
         #[serde(default, skip_serializing_if = "Option::is_none")]
         points: Option<SketchPoints>,

crates/doc/src/eval.rs

@@ -1956,6 +1956,39 @@ pub(crate) fn revolve_axis(ss: &SolvedSketch, axis: &AxisRef) -> Result<(Vec2, V
     }
 }
 
+/// Whether a drill from `p` along `d` reaches material: it starts into a body (`touches`), or,
+/// when `ahead` is set, a body lies further along `d`. A spot exactly on a surface counts only
+/// when the drill enters there, not when it leaves.
+fn hole_reaches(st: &ModelState, p: Vec3, d: Vec3, ahead: bool) -> bool {
+    let size = st.bodies.iter().map(|b| b.body.size()).fold(1.0, f64::max);
+    let eps = size * 1e-6;
+    let q = p + d * eps;
+    st.bodies.iter().any(|b| {
+        let m = b.mesh();
+        m.contains(q) || (ahead && m.raycast(q, d).is_some())
+    })
+}
+
+/// The side hole centres on a sketch drill to: into the material, whichever way the sketch plane
+/// faces. Material that touches a spot (the drill starts in it) wins over material further away,
+/// so a sketch on a plate's face drills that plate rather than one across an air gap: `-n` (away
+/// from the plane's normal) if any spot touches material that way, else `+n` if any spot touches
+/// material that way. With no touching on either side it goes by the nearest body ahead: `-n` if
+/// any spot has a body ahead that way, else `+n` if any has, else `-n`.
+fn sketch_hole_dir(st: &ModelState, spots: &[Vec3], n: Vec3) -> Vec3 {
+    let down = -n;
+    let any = |d: Vec3, ahead: bool| spots.iter().any(|p| hole_reaches(st, *p, d, ahead));
+    if any(down, false) {
+        down
+    } else if any(n, false) {
+        n
+    } else if any(down, true) || !any(n, true) {
+        down
+    } else {
+        n
+    }
+}
+
 /// Where a hole feature drills (each on the surface) and in which direction: at its sketch
 /// points perpendicular to the sketch (into the material), else at its position.
 fn hole_spots(st: &ModelState, f: &Feature) -> Result<(Vec<Vec3>, Vec3)> {
@@ -1968,7 +2001,8 @@ fn hole_spots(st: &ModelState, f: &Feature) -> Result<(Vec<Vec3>, Vec3)> {
                 let p = ss.sketch.points.iter().find(|p| &p.id == id).ok_or_else(|| DocError::Unknown(format!("sketch point `{id}`")))?;
                 v.push(ss.plane.to_world(p.pos));
             }
-            (v, -ss.plane.normal())
+            let dir = sketch_hole_dir(st, &v, ss.plane.normal());
+            (v, dir)
         }
         None => (vec![*position], direction.normalized().ok_or_else(|| DocError::Invalid("hole direction".into()))?),
     };
@@ -2008,7 +2042,8 @@ fn hole_spots_raw(st: &ModelState, f: &Feature) -> Result<(Vec<Vec3>, Vec3)> {
             let ss = st.sketch(sp.sketch).ok_or_else(|| DocError::Unknown(format!("sketch {}", sp.sketch)))?;
             let v: Vec<Vec3> =
                 sp.ids.iter().filter_map(|id| ss.sketch.points.iter().find(|p| &p.id == id)).map(|p| ss.plane.to_world(p.pos)).take(10_000).collect();
-            Ok((v, -ss.plane.normal()))
+            let dir = sketch_hole_dir(st, &v, ss.plane.normal());
+            Ok((v, dir))
         }
         None => Ok((vec![*position], direction.normalized().ok_or_else(|| DocError::Invalid("hole direction".into()))?)),
     }

crates/engine/src/cmd/features.rs

@@ -51,7 +51,7 @@ pub static COMMANDS: &[CommandSpec] = &[
         .at("SOLID", "CREATE")
         .icon("hole")
         .key("H")
-        .params("position: [x,y,z] on a face | sketch + points: [sketch point ids] (one hole each, perpendicular to the sketch) | sketch alone (every sketch point: standalone points and circle centres); direction?: [x,y,z] (default: into the face); diameter | thread (a tapped hole: the tap drill, major − pitch) | clearance: \"M6\" with fit?: close|normal|loose (ISO 273); depth? (default through all) | to?: [x,y,z] (To Object: drill to that face or point); type?: simple|drilled|counterbore|countersink; tip_angle?; cb_diameter?, cb_depth?; cs_diameter?, cs_angle?; thread?: \"M6\" (cosmetic)"),
+        .params("position: [x,y,z] on a face | sketch + points: [sketch point ids] (one hole each, perpendicular to the sketch, into the material on whichever side of the sketch plane it is) | sketch alone (every sketch point: standalone points and circle centres); direction?: [x,y,z] (default: into the face; ignored with sketch points); diameter | thread (a tapped hole: the tap drill, major − pitch) | clearance: \"M6\" with fit?: close|normal|loose (ISO 273); depth? (default through all) | to?: [x,y,z] (To Object: drill to that face or point); type?: simple|drilled|counterbore|countersink; tip_angle?; cb_diameter?, cb_depth?; cs_diameter?, cs_angle?; thread?: \"M6\" (cosmetic)"),
     CommandSpec::new("solid.pipe", "Pipe", pipe)
         .at("SOLID", "CREATE")
         .icon("pipe")

crates/engine/src/hole_face_tests.rs

@@ -87,3 +87,238 @@ fn a_slanted_hole_stays_on_its_axis_when_its_face_moves() {
     change(&mut s, "height", "12 mm");
     on_axis(&s, 12.0);
 }
+
+// Hole centres on a sketch drill into the material whichever way the sketch plane faces. Base
+// part: a 40 x 40 x 20 box at z 0..20 (32000 mm³).
+
+fn box_part(s: &mut Session) {
+    run(s, "solid.box", json!({"length": 40, "width": 40, "height": 20, "name": "Part", "position": [20, 20, 0]}));
+}
+
+/// A sketch named `Pts` on the XY plane lifted to `z`, holding the given points (ids p0, p1, ...).
+fn points_sketch(s: &mut Session, z: f64, at: &[[f64; 2]]) -> Vec<String> {
+    let plane = if z == 0.0 { json!("XY") } else { json!({"origin": [0, 0, z], "x_dir": [1, 0, 0], "y_dir": [0, 1, 0]}) };
+    run(s, "sketch.create", json!({"plane": plane, "name": "Pts"}));
+    let ids: Vec<String> = (0..at.len()).map(|i| format!("pt{z}_{i}").replace(['-', '.'], "m")).collect();
+    for (id, p) in ids.iter().zip(at) {
+        run(s, "sketch.point", json!({"point": p, "id": id}));
+    }
+    run(s, "sketch.finish", json!({}));
+    ids
+}
+
+fn drill(s: &mut Session, ids: &[String], extra: Value) {
+    let mut p = json!({"sketch": "Pts", "points": ids, "diameter": 10, "name": "H"});
+    if let (Some(m), Some(e)) = (p.as_object_mut(), extra.as_object()) {
+        m.extend(e.clone());
+    }
+    run(s, "solid.hole", p);
+}
+
+fn removed(s: &mut Session) -> f64 {
+    32_000.0 - volume(s)
+}
+
+fn near(a: f64, b: f64) {
+    assert!((a - b).abs() < 1.0, "{a} vs {b}");
+}
+
+const CB: fn() -> Value = || json!({"type": "counterbore", "diameter": 6, "cb_diameter": 12, "cb_depth": 3, "depth": 10});
+
+#[test]
+fn sketch_under_the_part_drills_up_into_it() {
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 0.0, &[[20.0, 20.0]]);
+    drill(&mut s, &ids, json!({"depth": 8}));
+    near(removed(&mut s), 628.3);
+    assert!(!solid_at(&s, 20.0, 20.0, 4.0) && solid_at(&s, 20.0, 20.0, 10.0));
+}
+
+#[test]
+fn sketch_under_the_part_counterbore_and_four_points() {
+    let one = std::f64::consts::PI * 36.0 * 3.0 + std::f64::consts::PI * 9.0 * 7.0;
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 0.0, &[[20.0, 20.0]]);
+    drill(&mut s, &ids, CB());
+    near(removed(&mut s), one);
+    near(one, 537.2);
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 0.0, &[[10.0, 10.0], [30.0, 10.0], [10.0, 30.0], [30.0, 30.0]]);
+    drill(&mut s, &ids, CB());
+    near(removed(&mut s), 4.0 * one);
+}
+
+#[test]
+fn sketch_under_the_part_through_all() {
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 0.0, &[[20.0, 20.0]]);
+    drill(&mut s, &ids, json!({}));
+    near(removed(&mut s), 1570.8);
+    assert!(!solid_at(&s, 20.0, 20.0, 19.0));
+}
+
+#[test]
+fn sketch_in_the_air_below_the_part() {
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, -10.0, &[[20.0, 20.0]]);
+    drill(&mut s, &ids, json!({"depth": 8}));
+    near(removed(&mut s), 628.3);
+}
+
+#[test]
+fn sketch_on_top_face_height_is_unchanged() {
+    let one = std::f64::consts::PI * 36.0 * 3.0 + std::f64::consts::PI * 9.0 * 7.0;
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 20.0, &[[20.0, 20.0]]);
+    drill(&mut s, &ids, json!({"depth": 8}));
+    near(removed(&mut s), 628.3);
+    assert!(!solid_at(&s, 20.0, 20.0, 16.0) && solid_at(&s, 20.0, 20.0, 10.0));
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 20.0, &[[10.0, 10.0], [30.0, 10.0], [10.0, 30.0], [30.0, 30.0]]);
+    drill(&mut s, &ids, CB());
+    near(removed(&mut s), 4.0 * one);
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 20.0, &[[20.0, 20.0]]);
+    drill(&mut s, &ids, json!({}));
+    near(removed(&mut s), 1570.8);
+}
+
+#[test]
+fn sketch_inside_the_body_still_drills_down() {
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 10.0, &[[20.0, 20.0]]);
+    drill(&mut s, &ids, json!({"depth": 8}));
+    near(removed(&mut s), 628.3);
+    // As before: the spot moves back up to the top face and drills down from there (z 12..20).
+    assert!(!solid_at(&s, 20.0, 20.0, 16.0) && solid_at(&s, 20.0, 20.0, 6.0));
+}
+
+// A center exactly on an edge or a corner of the part: half or a quarter of the hole is in the
+// material, and the side is chosen the same way as for a center well inside the face.
+
+#[test]
+fn center_on_an_edge_drills_half_a_hole_from_either_side() {
+    let half = std::f64::consts::PI * 25.0 * 8.0 / 2.0;
+    near(half, 314.2);
+    for (z, cut, kept) in [(20.0, 16.0, 6.0), (0.0, 4.0, 14.0)] {
+        let mut s = Session::default();
+        box_part(&mut s);
+        let ids = points_sketch(&mut s, z, &[[0.0, 20.0]]);
+        drill(&mut s, &ids, json!({"depth": 8}));
+        near(removed(&mut s), half);
+        assert!(!solid_at(&s, 2.0, 20.0, cut) && solid_at(&s, 2.0, 20.0, kept), "sketch at z {z}");
+    }
+}
+
+#[test]
+fn center_on_a_corner_drills_a_quarter_of_a_hole_from_either_side() {
+    let quarter = std::f64::consts::PI * 25.0 * 8.0 / 4.0;
+    near(quarter, 157.1);
+    for (z, cut, kept) in [(20.0, 16.0, 6.0), (0.0, 4.0, 14.0)] {
+        let mut s = Session::default();
+        box_part(&mut s);
+        let ids = points_sketch(&mut s, z, &[[0.0, 0.0]]);
+        drill(&mut s, &ids, json!({"depth": 8}));
+        near(removed(&mut s), quarter);
+        assert!(!solid_at(&s, 2.0, 2.0, cut) && solid_at(&s, 2.0, 2.0, kept), "sketch at z {z}");
+    }
+}
+
+#[test]
+fn pattern_of_a_flipped_hole_and_round_trip() {
+    let mut s = Session::default();
+    box_part(&mut s);
+    let ids = points_sketch(&mut s, 0.0, &[[8.0, 8.0]]);
+    drill(&mut s, &ids, json!({"depth": 8}));
+    near(removed(&mut s), 628.3);
+    run(&mut s, "solid.pattern.rectangular", json!({"features": ["H"], "dir1": [1, 0, 0], "count1": 3, "spacing1": 12, "name": "Row"}));
+    near(removed(&mut s), 3.0 * 628.3);
+    let before = volume(&mut s);
+    let doc = solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap();
+    let mut again = Session::new(doc);
+    near(volume(&mut again), before);
+}
+
+// Two plates of the box, joined by a wall, with an air gap between them: the sketch plane sits on
+// the face of one plate and the holes go into that plate, not across the gap into the other one.
+
+/// The 40 x 40 x 20 box with a slot cut through it from z = `lo` to `hi` (a wall stays at x < 5).
+fn slotted_part(s: &mut Session, lo: f64, hi: f64) {
+    run(s, "solid.box", json!({"length": 40, "width": 40, "height": 20}));
+    run(s, "sketch.create", json!({"plane": "XY", "offset": format!("{lo} mm"), "name": "Slot"}));
+    run(s, "sketch.rectangle.two_point", json!({"p0": [5, -1], "p1": [45, 41]}));
+    run(s, "sketch.finish", json!({}));
+    run(s, "solid.extrude", json!({"sketch": "Slot", "distance": format!("{} mm", hi - lo), "operation": "cut", "direction": "positive"}));
+}
+
+/// A sketch `Pts` at z = `z` with its normal pointing down (-z), holding one point over (20, 20).
+fn down_facing_point(s: &mut Session, z: f64) {
+    let plane = json!({"origin": [0, 0, z], "x_dir": [1, 0, 0], "y_dir": [0, -1, 0]});
+    run(s, "sketch.create", json!({"plane": plane, "name": "Pts"}));
+    run(s, "sketch.point", json!({"point": [20, -20], "id": "p1"}));
+    run(s, "sketch.finish", json!({}));
+}
+
+fn pi() -> f64 {
+    std::f64::consts::PI
+}
+
+#[test]
+fn plates_sketch_normal_into_the_gap_drills_the_plate_it_touches() {
+    let mut s = Session::default();
+    slotted_part(&mut s, 6.0, 14.0);
+    let base = volume(&mut s);
+    down_facing_point(&mut s, 6.0);
+    let cb = json!({"type": "counterbore", "diameter": 4, "cb_diameter": 10, "cb_depth": 2, "depth": 9});
+    drill(&mut s, &["p1".to_string()], cb);
+    near(base - volume(&mut s), pi() * 25.0 * 2.0 + pi() * 4.0 * 4.0);
+    assert!(!solid_at(&s, 20.0, 20.0, 5.0) && !solid_at(&s, 20.0, 20.0, 1.0) && solid_at(&s, 20.0, 20.0, 16.0));
+}
+
+#[test]
+fn plates_sketch_normal_away_from_the_gap_drills_the_plate_it_touches() {
+    let mut s = Session::default();
+    slotted_part(&mut s, 8.0, 14.0);
+    let base = volume(&mut s);
+    run(&mut s, "sketch.create", json!({"plane": {"origin": [0, 0, 14], "x_dir": [1, 0, 0], "y_dir": [0, 1, 0]}, "name": "Pts"}));
+    run(&mut s, "sketch.point", json!({"point": [20, 20], "id": "p1"}));
+    run(&mut s, "sketch.finish", json!({}));
+    drill(&mut s, &["p1".to_string()], json!({"depth": 7}));
+    near(base - volume(&mut s), pi() * 25.0 * 6.0);
+    assert!(!solid_at(&s, 20.0, 20.0, 17.0) && solid_at(&s, 20.0, 20.0, 4.0));
+}
+
+#[test]
+fn two_separate_bodies_sketch_touching_one_drills_it_not_the_far_one() {
+    let mut s = Session::default();
+    run(&mut s, "solid.box", json!({"length": 40, "width": 40, "height": 6, "name": "Low"}));
+    run(&mut s, "solid.box", json!({"length": 40, "width": 40, "height": 6, "corner": [0, 0, 14], "name": "High"}));
+    let base = volume(&mut s);
+    near(base, 2.0 * 40.0 * 40.0 * 6.0);
+    down_facing_point(&mut s, 6.0);
+    drill(&mut s, &["p1".to_string()], json!({"depth": 9}));
+    near(base - volume(&mut s), pi() * 25.0 * 6.0);
+    assert!(!solid_at(&s, 20.0, 20.0, 3.0) && !solid_at(&s, 20.0, 20.0, 5.9));
+    assert!(solid_at(&s, 20.0, 20.0, 15.0) && solid_at(&s, 20.0, 20.0, 19.0));
+}
+
+#[test]
+fn sketch_floating_in_the_gap_touching_neither_still_drills_down() {
+    let mut s = Session::default();
+    slotted_part(&mut s, 6.0, 14.0);
+    let base = volume(&mut s);
+    // Normal +z (the default direction), material ahead on both sides, touching neither.
+    points_sketch(&mut s, 10.0, &[[20.0, 20.0]]);
+    drill(&mut s, &["pt10_0".to_string()], json!({"depth": 4}));
+    near(base - volume(&mut s), pi() * 25.0 * 4.0);
+    assert!(!solid_at(&s, 20.0, 20.0, 4.0) && solid_at(&s, 20.0, 20.0, 16.0));
+}