From 340ac0313ad6bcb53f8aac90f9b6c2c97ff0cb26 Mon Sep 17 00:00:00 2001 From: NOWN-DEV <257771410+NOWN-DEV@users.noreply.github.com> Date: Tue, 13 Oct 2026 06:13:04 +0200 Subject: [PATCH] fix: a hole placed on a face follows the face along its axis only, and warns when off the face (#510) A hole positioned on a face now follows that face along the hole's own axis instead of sliding to the face's nearest point. When the face no longer reaches the position, the hole stays where it was placed and the feature says so. Co-Authored-By: Claude Opus 5.5 --- crates/doc/src/eval.rs | 22 ++++++- crates/doc/src/name_lookup.rs | 29 +++++++++ crates/doc/src/naming.rs | 2 +- crates/engine/src/hole_face_tests.rs | 89 ++++++++++++++++++++++++++++ crates/engine/src/lib.rs | 2 + docs/book/src/dev/naming.md | 6 ++ 6 files changed, 147 insertions(+), 3 deletions(-) create mode 100644 crates/engine/src/hole_face_tests.rs diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index ed870374..e55a8cdb 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -924,7 +924,14 @@ impl Model { let f = if !explicit_emboss && f.face_names.iter().any(|n| !n.is_empty()) { let mut g = f.clone(); let mut notes = Vec::new(); - for (p, n) in g.kind.face_points_mut().into_iter().zip(&f.face_names) { + // A hole's position is a place on its face, not a pick of the face: it follows + // the face along the hole's axis and never slides sideways to the face's nearest + // point when the face no longer reaches it. + let hole_axis = match &f.kind { + FeatureKind::Hole { direction, points: None, .. } => direction.normalized(), + _ => None, + }; + for (k, (p, n)) in g.kind.face_points_mut().into_iter().zip(&f.face_names).enumerate() { if n.is_empty() { continue; } @@ -933,7 +940,18 @@ impl Model { if let Some(note) = hit.note(n) { notes.push(note); } - *p = hit.point; + match (k, hole_axis) { + (0, Some(axis)) => match hit.along(&state, *p, axis) { + Some(q) => *p = q, + // The place still lies on the face (the axis may run along it). + None if hit.on_point => {} + None => notes.push(format!( + "the hole's position [{}, {}, {}] is no longer on face `{n}`: it stays where it was placed", + p.x, p.y, p.z + )), + }, + _ => *p = hit.point, + } } None => notes.push(format!("face `{n}` no longer exists; using the face at the picked point")), } diff --git a/crates/doc/src/name_lookup.rs b/crates/doc/src/name_lookup.rs index c9036e7b..6dba47d4 100644 --- a/crates/doc/src/name_lookup.rs +++ b/crates/doc/src/name_lookup.rs @@ -211,6 +211,35 @@ impl FaceHit { } } +impl FaceHit { + /// Where the line through `p` along `dir` meets the face found (the hit nearest `p`, on + /// either side of it), `None` when the line misses the face or runs along it. A place on + /// a face follows the face this way, along its own axis, instead of sliding sideways to + /// [`point`](Self::point), the face's nearest point. + pub fn along(&self, st: &ModelState, p: Vec3, dir: Vec3) -> Option { + let b = st.bodies.get(self.body)?; + let d = dir.normalized()?; + let m = b.mesh(); + let slack = (b.body.size() * 1e-9).max(1e-9); + let mut best: Option = None; + for (t, f) in m.triangles.iter().zip(&m.tri_face) { + if *f as usize != self.face { + continue; + } + let Some([a, bb, c]) = m.tri(t) else { continue }; + // A hair's width around each triangle, so a place on the seam of two is on the face. + let centre = (a + bb + c) * (1.0 / 3.0); + let grow = |v: Vec3| v + (v - centre).normalized().unwrap_or(Vec3::ZERO) * slack; + if let Some(hit) = naming::ray_hit(p, d, grow(a), grow(bb), grow(c)) + && best.is_none_or(|old| hit.abs() < old.abs()) + { + best = Some(hit); + } + } + best.map(|t| p + d * t) + } +} + impl EdgeHit { /// What a feature says about how it found an edge by its stored name `name` (`None`: exactly). pub fn note(&self, name: &str) -> Option { diff --git a/crates/doc/src/naming.rs b/crates/doc/src/naming.rs index a2d0204e..cfd38b7a 100644 --- a/crates/doc/src/naming.rs +++ b/crates/doc/src/naming.rs @@ -386,7 +386,7 @@ fn behind(ps: &[Vec3], ns: &[Vec3], inputs: &[(&ModelBody, FaceIndex, Arc= ps.len().max(1)).map(|(n, _)| n.clone()) } -fn ray_hit(o: Vec3, d: Vec3, a: Vec3, b: Vec3, c: Vec3) -> Option { +pub(crate) fn ray_hit(o: Vec3, d: Vec3, a: Vec3, b: Vec3, c: Vec3) -> Option { let (e1, e2) = (b - a, c - a); let h = d.cross(e2); let det = e1.dot(h); diff --git a/crates/engine/src/hole_face_tests.rs b/crates/engine/src/hole_face_tests.rs new file mode 100644 index 00000000..5a51867f --- /dev/null +++ b/crates/engine/src/hole_face_tests.rs @@ -0,0 +1,89 @@ +//! A hole placed on a face follows that face along the hole's axis, never sideways: when the face +//! moves the hole moves with it, and when the face no longer reaches the place the hole stays +//! where it was put and says so instead of sliding to the face's nearest point. + +use serde_json::{Value, json}; +use solidcraft_geom::Vec3; + +use crate::Session; + +fn run(s: &mut Session, id: &str, p: Value) -> Value { + match s.execute(id, &p) { + Ok(v) => v, + Err(e) => panic!("{id} {p}: {e}"), + } +} + +fn change(s: &mut Session, name: &str, expression: &str) { + run(s, "parameters.change", json!({"name": name, "expression": expression})); +} + +fn volume(s: &mut Session) -> f64 { + run(s, "inspect.measure", json!({}))["total"]["volume_mm3"].as_f64().unwrap() +} + +/// Whether the model holds material at (x, y, z). +fn solid_at(s: &Session, x: f64, y: f64, z: f64) -> bool { + s.model.state().bodies.iter().any(|b| b.body.contains(Vec3::new(x, y, z))) +} + +/// What the hole feature says (its error and its warning); empty when it is silent. +fn said(s: &Session, feature: &str) -> String { + let id = s.doc.find_feature(feature).unwrap().id; + let r = s.model.result(id).unwrap(); + format!("{}{}", r.error.clone().unwrap_or_default(), r.warning.clone().unwrap_or_default()) +} + +#[test] +fn a_hole_the_face_no_longer_reaches_stays_where_it_was_placed_and_says_so() { + let mut s = Session::default(); + run(&mut s, "parameters.add", json!({"name": "length", "expression": "100 mm"})); + run(&mut s, "solid.box", json!({"length": "length", "width": 50, "height": 20, "name": "Plate"})); + run(&mut s, "solid.hole", json!({"position": [80, 25, 20], "diameter": "6 mm", "name": "Hole"})); + // The bore is at x = 80, 3 mm either way. + let drilled_at_80 = |s: &Session| !solid_at(s, 80.0, 25.0, 10.0) && solid_at(s, 76.0, 25.0, 10.0) && solid_at(s, 84.0, 25.0, 10.0); + assert!(drilled_at_80(&s)); + // Longer: the place is still on the top face, and the hole stays at x = 80. + change(&mut s, "length", "150 mm"); + assert_eq!(said(&s, "Hole"), ""); + assert!(drilled_at_80(&s)); + // Shorter than the place: nothing is drilled at the new end, and the hole says why. A hole + // moved to the face's nearest point would cut half a hole there (59 717 mm³ left). + change(&mut s, "length", "60 mm"); + let left = volume(&mut s); + assert!((left - 60_000.0).abs() < 1.0, "half a hole was cut at the plate's end: {left} mm³ of 60000"); + let msg = said(&s, "Hole"); + assert!(msg.contains("no longer on face") && msg.contains("[80, 25, 20]"), "{msg}"); + // Long again: the hole is back where it was put, with no message. + change(&mut s, "length", "100 mm"); + assert_eq!(said(&s, "Hole"), ""); + assert!(drilled_at_80(&s)); +} + +#[test] +fn a_slanted_hole_stays_on_its_axis_when_its_face_moves() { + let mut s = Session::default(); + run(&mut s, "parameters.add", json!({"name": "height", "expression": "20 mm"})); + run(&mut s, "solid.box", json!({"length": 60, "width": 30, "height": "height", "name": "Block"})); + // Entering the top face at x = 20, running on in x as it goes down: 1 across for every 2 down. + run(&mut s, "solid.hole", json!({"position": [20, 15, 20], "direction": [1, 0, -2], "diameter": "2 mm", "name": "Hole"})); + // The axis is the line through the place along the direction, whatever the height of the block: + // x = 20 + (20 - z) / 2. At mid-height the bore is empty on it and solid 3 mm to either side + // (the bore is about 2.2 mm wide there). + let on_axis = |s: &Session, h: f64| { + let (x, z) = (20.0 + (20.0 - h / 2.0) / 2.0, h / 2.0); + assert!(!solid_at(s, x, 15.0, z), "block height {h}: the bore should pass through x = {x} at z = {z}"); + assert!(solid_at(s, x - 3.0, 15.0, z) && solid_at(s, x + 3.0, 15.0, z), "block height {h}: material 3 mm beside the axis"); + // (The kernel may note a boolean retry for a slanted bore; the hole's place is on its face.) + assert!(!said(s, "Hole").contains("no longer on face"), "block height {h}: {}", said(s, "Hole")); + }; + on_axis(&s, 20.0); + // Taller: the top face moves up. Moved to the face's nearest point the hole would slide + // sideways onto another axis (5 mm further along x at mid-height); along its axis it enters + // higher up the same line. + change(&mut s, "height", "30 mm"); + on_axis(&s, 30.0); + // Lower: the same line enters further along. + change(&mut s, "height", "12 mm"); + on_axis(&s, 12.0); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index b9b25a07..8e6f73ff 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -795,6 +795,8 @@ mod delete_face_tests; #[cfg(test)] mod extent_tests; #[cfg(test)] +mod hole_face_tests; +#[cfg(test)] mod naming_history_tests; #[cfg(test)] mod pattern_suppress_tests; diff --git a/docs/book/src/dev/naming.md b/docs/book/src/dev/naming.md index 660684c4..15569af1 100644 --- a/docs/book/src/dev/naming.md +++ b/docs/book/src/dev/naming.md @@ -80,6 +80,12 @@ longer lies on the face says so: `face ... was found by geometry, not exact hist An edge name is found as the edge between the two faces it names, each found as above: when exactly one edge runs between them it is that edge, even after a split of one of its faces. +A hole placed by a position on a face is the exception to "the point nearest the reference's +point": its position is a place, not a pick. The hole follows its face along its own axis, so it +moves with the face when the face moves, and when the face no longer reaches the position it stays +where it was placed and the feature says so, instead of sliding to the face's edge and drilling +half a hole there. (Holes at sketch points follow their sketch.) + ## Older designs Designs saved before format 14 stored names worked out from geometry, and a few of them name a