fix: booleans with whole-turn round faces read from STEP are exact instead of failing (#1892)

Thanks @NOWN-DEV.

crates/kernel/src/lib.rs

@@ -101,6 +101,7 @@ mod prism_local;
 mod prism_measure;
 pub mod provenance;
 mod record;
+mod seam_faces;
 mod sew;
 mod smooth_edges;
 mod spline_conv;

crates/kernel/src/ops.rs

@@ -908,6 +908,18 @@ fn boolean_whole_with_refinement(a: &Body, b: &Body, op: BoolOp, refine: bool) -
                 return Ok(Some(body));
             }
         }
+        // A whole turn of a cylinder or cone bounded by one wire along its seam (as STEP files
+        // write them) can't be divided by a loop lying inside it, and the tries below end in a
+        // nudged tool, or fail: the same boolean with such faces in two halves, which is exact.
+        if attempt == 1 && crate::seam_faces::needed(a, b) {
+            let (halves, diagnostics) = capture_boolean_diagnostics(|| crate::seam_faces::boolean(a, b, op, &boolean));
+            if let Some(Ok(r)) = halves
+                && r.as_ref().is_none_or(|body| check(body).0)
+            {
+                merge(diagnostics);
+                return Ok(r);
+            }
+        }
         let shift = mt::Vector3::new(j[0], j[1], j[2]) * (size * 0.01);
         for k in [2e-3, 5e-4] {
             let tol = (size * k).max(1e-5);

crates/kernel/src/seam_faces.rs

@@ -0,0 +1,92 @@
+//! Faces that run along a seam edge twice: a whole turn of a cylinder or a cone bounded by one
+//! wire, as STEP files write them (the wire goes round one end, along the seam, round the other
+//! end and back along the seam). SolidCraft builds its own round faces in pieces with no such
+//! seam, and a boolean that has to divide one of these faces can fail where the same bodies with
+//! the face in two halves work: an intersection loop lying wholly inside the face is a hole in a
+//! piece whose outer wire visits the seam's vertices twice, which the boolean refuses. So when the
+//! first tries at the boolean of the bodies as they are fail, each such face is split in two along
+//! a plane through its axis, a quarter turn either side of the seam (the solids are unchanged), and
+//! the boolean is asked again; the result's history is composed back over the operands as they
+//! were given.
+
+use solidcraft_geom::{Plane, Vec3};
+
+use crate::body::{Body, from_p3};
+use crate::boolean_record as recorded;
+use crate::history::History;
+use crate::splitface::{SplitTool, split_any};
+use crate::{BoolOp, FaceSurface, Result, face_surface};
+
+/// Most faces split before the boolean.
+const MAX_SPLITS: usize = 64;
+
+thread_local! {
+    /// Set while the boolean of the split bodies runs (no second round of splitting).
+    static SPLITTING: std::cell::Cell<bool> = const { std::cell::Cell::new(false) };
+}
+
+/// The first face of `body` whose wire runs along a seam edge twice, on a cylinder or a cone,
+/// with the plane that splits it in two: through its axis, square to the seam.
+fn next_split(body: &Body) -> Option<(usize, Plane)> {
+    body.solid.face_iter().enumerate().find_map(|(i, face)| {
+        let mut seen = std::collections::HashSet::new();
+        let seam = face.absolute_boundaries().iter().flat_map(|w| w.edge_iter()).find(|e| !seen.insert(e.id()))?;
+        let (origin, axis) = match face_surface(body, i)? {
+            FaceSurface::Cylinder { origin, axis, .. } => (origin, axis),
+            FaceSurface::Cone { apex, axis, .. } => (apex, axis),
+            _ => return None,
+        };
+        let axis = axis.normalized()?;
+        let p = from_p3(seam.front().point());
+        let off = p - origin;
+        let radial: Vec3 = (off - axis * off.dot(axis)).normalized()?;
+        Some((i, Plane::from_normal(origin, radial)?))
+    })
+}
+
+/// Whether either body has a face for [`boolean`] to split.
+pub(crate) fn needed(a: &Body, b: &Body) -> bool {
+    !a.is_mesh() && !b.is_mesh() && !SPLITTING.with(|c| c.get()) && (next_split(a).is_some() || next_split(b).is_some())
+}
+
+/// `body` with every face that runs along a seam edge twice split in two, and its history over
+/// `body`. `None` when a split fails.
+fn halved(body: &Body) -> Option<(Body, Option<History>)> {
+    let (mut x, mut h) = (body.clone(), Some(History::identity(body)));
+    for _ in 0..MAX_SPLITS {
+        let Some((face, plane)) = next_split(&x) else { return Some((x, h)) };
+        let (mut done, _) = split_any(&x, &[face], &SplitTool::Plane(plane)).ok()?;
+        // The split is no authored boundary: a later feature may still merge what it separates.
+        done.regularization_protected = x.regularization_protected;
+        let through = recorded::through(&done, 0, h.as_ref());
+        (x, h) = (done, through);
+    }
+    next_split(&x).is_none().then_some((x, h))
+}
+
+/// The boolean `op` of `a` and `b` with their faces that run along a seam edge twice split in
+/// two, `run` doing the boolean of the split bodies. `None` when neither body has such a face, a
+/// split fails, or the boolean of the split bodies gave no (checked) result: the caller then
+/// keeps what it had.
+pub(crate) fn boolean(a: &Body, b: &Body, op: BoolOp, run: &dyn Fn(&Body, &Body, BoolOp) -> Result<Option<Body>>) -> Option<Result<Option<Body>>> {
+    if !needed(a, b) {
+        return None;
+    }
+    let ((x, hx), (y, hy)) = (halved(a)?, halved(b)?);
+    SPLITTING.with(|c| c.set(true));
+    let result = crate::guard("boolean of faces split off their seams", || run(&x, &y, op));
+    SPLITTING.with(|c| c.set(false));
+    let Ok(result) = result else { return None };
+    // Held to what a boolean can do, as a retry's result is (see `boolean_guard`).
+    crate::boolean_guard::check_retry(a, b, op, result.as_ref()).ok()?;
+    let counts = crate::record::counts(&[a, b]);
+    Some(Ok(result.map(|body| {
+        // Each step's history over the operands `[a, b]`.
+        let history = recorded::of_pair(&body, &recorded::placed(hx.clone(), 0, 2), &recorded::placed(hy.clone(), 1, 2));
+        body.with_checked_history(history, &counts)
+    })))
+}
+
+#[cfg(test)]
+#[path = "seam_faces_tests.rs"]
+mod tests;

crates/kernel/src/seam_faces_tests.rs

@@ -0,0 +1,94 @@
+//! Round faces read from STEP: a whole turn bounded by one wire that runs along its seam twice.
+//! A boolean whose intersection loop lies wholly inside such a face takes it in two halves
+//! (exactly), where it used to end in a nudged tool or fail.
+use crate::*;
+use solidcraft_geom::{Loop2, Plane, Region2, Vec2, Vec3};
+use std::f64::consts::PI;
+
+const R: f64 = 4.5;
+
+fn volume(b: &Body) -> f64 {
+    measure(b).unwrap().volume
+}
+
+fn rel(a: f64, b: f64) -> f64 {
+    (a - b).abs() / b.abs().max(1e-12)
+}
+
+/// `body` written to STEP and read back: its round faces come back whole, as other systems
+/// write them.
+fn through_step(body: &Body) -> Body {
+    step_import(&step_export(&[body], "t").unwrap()).unwrap().bodies.into_iter().next().unwrap().body
+}
+
+/// A pin of radius `R` along X, 40 long, centred on the origin.
+fn pin() -> Body {
+    cylinder(Vec3::new(-20.0, 0.0, 0.0), Vec3::X, R, 40.0).unwrap()
+}
+
+/// A block pressed into the top of the pin's side: 10 long, 6 wide, from 2 above the axis up.
+fn block() -> Body {
+    box_solid(Vec3::new(-5.0, -3.0, 2.0), Vec3::new(5.0, 3.0, 9.0)).unwrap()
+}
+
+/// The part of the pin inside [`block`]: the circle's segment above 2 and within 3 of the
+/// middle, 10 long.
+fn pressed_volume() -> f64 {
+    let segment = 3.0 * (R * R - 9.0).sqrt() + R * R * (3.0 / R).asin() - 6.0 * 2.0;
+    10.0 * segment
+}
+
+/// The boolean, which must be exact: no retry that moves the tool or ignores part of where the
+/// bodies meet.
+fn exact(a: &Body, b: &Body, op: BoolOp) -> Option<Body> {
+    let (r, report) = capture_boolean_diagnostics(|| boolean(a, b, op));
+    assert!(report.warning().is_none(), "{op:?}: {:?}", report.warning());
+    r.unwrap()
+}
+
+#[test]
+fn a_pin_read_from_step_has_one_face_along_its_seam() {
+    let read = through_step(&pin());
+    assert_eq!(read.face_count(), 3);
+    assert!(super::needed(&read, &block()));
+    assert!(!super::needed(&pin(), &block()));
+    // Split in two halves, with the same solid.
+    let (halved, history) = super::halved(&read).unwrap();
+    assert!(super::next_split(&halved).is_none() && halved.face_count() > 3);
+    assert!(history.is_some());
+    assert!(rel(volume(&halved), PI * R * R * 40.0) < 1e-3);
+}
+
+#[test]
+fn a_block_pressed_into_a_read_pins_side_is_intersected_and_cut_exactly() {
+    let (read, block) = (through_step(&pin()), block());
+    let want = pressed_volume();
+    let shared = exact(&read, &block, BoolOp::Intersect).unwrap();
+    assert!(rel(volume(&shared), want) < 1e-3, "{} {want}", volume(&shared));
+    let dented = exact(&read, &block, BoolOp::Cut).unwrap();
+    assert!(rel(volume(&dented), PI * R * R * 40.0 - want) < 1e-3, "{}", volume(&dented));
+    // The operands' faces carry over: the dented pin's round face is still the pin's.
+    assert!(dented.history().is_some());
+}
+
+#[test]
+fn a_ball_pressed_into_a_read_pins_side_matches_the_built_pin() {
+    let ball = sphere(Vec3::new(3.0, 1.0, 5.5), 3.0).unwrap();
+    let built = volume(&boolean(&pin(), &ball, BoolOp::Intersect).unwrap().unwrap());
+    let read = volume(&exact(&through_step(&pin()), &ball, BoolOp::Intersect).unwrap());
+    assert!(rel(read, built) < 1e-3, "{read} {built}");
+}
+
+/// A cone's side read from STEP is one face along its seam as well.
+#[test]
+fn a_block_pressed_into_a_read_cones_side_matches_the_built_cone() {
+    let outline =
+        Region2 { outer: Loop2::polygon(&[Vec2::new(0.0, 0.0), Vec2::new(8.0, 0.0), Vec2::new(5.0, 20.0), Vec2::new(0.0, 20.0)]), holes: vec![] };
+    let cone = revolve(&Plane::YZ, &[outline], Vec2::ZERO, Vec2::Y, 2.0 * PI).unwrap().pop().unwrap();
+    let read = through_step(&cone);
+    assert!(super::needed(&read, &cone));
+    let block = box_solid(Vec3::new(-2.0, 4.0, 6.0), Vec3::new(2.0, 12.0, 12.0)).unwrap();
+    let built = volume(&boolean(&cone, &block, BoolOp::Intersect).unwrap().unwrap());
+    let read = volume(&exact(&read, &block, BoolOp::Intersect).unwrap());
+    assert!(rel(read, built) < 1e-3, "{read} {built}");
+}

vendor/README.md

@@ -53,6 +53,12 @@ Changes:
   farther from the leader's point than the curve's tolerance (the projection settled on another
   part of a surface, such as the far side of a small hole) is dropped for the search from
   nothing, as before. `crates/kernel/src/issue_1038_tests.rs` holds it to that search.
+- `decorators/revolved_curve.rs`: a point on a revolved surface's seam (angle 0, the same place
+  as 2π) takes the end of the period nearer the hinted angle. The angle was decided by the sign of
+  a cross product that is rounding noise there, so a boundary followed point by point could come
+  back to the seam at the other end of the period and fold its parameter polygon over itself (no
+  intersection loop then lay inside the face). `crates/kernel/src/seam_faces_tests.rs` holds it
+  to a cone read from STEP.
 
 ## truck-stepio 0.3.0 (Apache-2.0, © RICOS Co. Ltd., https://github.com/ricosjp/truck)
 

vendor/truck-geometry/src/decorators/revolved_curve.rs

@@ -82,6 +82,20 @@ impl Revolution {
     }
 }
 
+/// SolidCraft: the angle of a point on the seam, where 0 and 2π are the same place, taken at the end
+/// of the period nearer the hint's angle. [`Revolution::proj_angle`] decides it by the sign of a cross
+/// product that is rounding noise there, so a boundary followed point by point could come back to
+/// the seam at the other end of the period from where it left, and its parameter polygon folded
+/// over itself (a whole turn of a cylinder had no area, and no intersection loop lay inside it).
+fn seam_side(angle: f64, hint: Option<f64>) -> f64 {
+    const SEAM: f64 = 1.0e-7;
+    match hint {
+        Some(h) if angle < SEAM && h > PI => angle + 2.0 * PI,
+        Some(h) if angle > 2.0 * PI - SEAM && h < PI => angle - 2.0 * PI,
+        _ => angle,
+    }
+}
+
 impl<C: ParametricCurve3D> ParametricSurface for RevolutedCurve<C> {
     type Point = Point3;
     type Vector = Vector3;
@@ -330,14 +344,19 @@ impl<C: ParametricCurve3D + BoundedCurve> SearchParameter<D2> for RevolutedCurve
                 revolution: self.revolution,
             };
             let p = self.revolution.proj_point(point);
-            let hint0 = match hint.into() {
+            let hint = hint.into();
+            let hint_angle = match hint {
+                SPHint2D::Parameter(_, y) => Some(y),
+                _ => None,
+            };
+            let hint0 = match hint {
                 SPHint2D::Parameter(x, _) => SPHint1D::Parameter(x),
                 SPHint2D::Range((x0, _), (x1, _)) => SPHint1D::Range(x0, x1),
                 SPHint2D::None => SPHint1D::None,
             };
             let t = proj_curve.search_parameter(p, hint0, trials)?;
             let p = self.curve.subs(t);
-            let ang = self.revolution.proj_angle(p, point);
+            let ang = seam_side(self.revolution.proj_angle(p, point), hint_angle);
             match self.subs(t, ang).near(&point) {
                 true => Some((t, ang)),
                 false => None,
@@ -377,14 +396,19 @@ impl<C: ParametricCurve3D + BoundedCurve> SearchNearestParameter<D2> for Revolut
                 revolution: self.revolution,
             };
             let p = self.revolution.proj_point(point);
-            let hint0 = match hint.into() {
+            let hint = hint.into();
+            let hint_angle = match hint {
+                SPHint2D::Parameter(_, y) => Some(y),
+                _ => None,
+            };
+            let hint0 = match hint {
                 SPHint2D::Parameter(x, _) => SPHint1D::Parameter(x),
                 SPHint2D::Range((x0, _), (x1, _)) => SPHint1D::Range(x0, x1),
                 SPHint2D::None => SPHint1D::None,
             };
             let t = proj_curve.search_nearest_parameter(p, hint0, trials)?;
             let p = self.curve.subs(t);
-            Some((t, self.revolution.proj_angle(p, point)))
+            Some((t, seam_side(self.revolution.proj_angle(p, point), hint_angle)))
         }
     }
 }