From c38992fc09f1119bf52bfd7354648e8037f71235 Mon Sep 17 00:00:00 2001 From: NOWN-DEV <257771410+NOWN-DEV@users.noreply.github.com> Date: Sat, 17 Oct 2026 10:08:34 +0200 Subject: [PATCH] fix: booleans with whole-turn round faces read from STEP are exact instead of failing (#1892) Thanks @NOWN-DEV. --- crates/kernel/src/lib.rs | 1 + crates/kernel/src/ops.rs | 12 +++ crates/kernel/src/seam_faces.rs | 92 ++++++++++++++++++ crates/kernel/src/seam_faces_tests.rs | 94 +++++++++++++++++++ vendor/README.md | 6 ++ .../src/decorators/revolved_curve.rs | 32 ++++++- 6 files changed, 233 insertions(+), 4 deletions(-) create mode 100644 crates/kernel/src/seam_faces.rs create mode 100644 crates/kernel/src/seam_faces_tests.rs diff --git a/crates/kernel/src/lib.rs b/crates/kernel/src/lib.rs index 7775d2b4..b443effa 100644 --- a/crates/kernel/src/lib.rs +++ b/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; diff --git a/crates/kernel/src/ops.rs b/crates/kernel/src/ops.rs index c6dedf91..a8b57a48 100644 --- a/crates/kernel/src/ops.rs +++ b/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); diff --git a/crates/kernel/src/seam_faces.rs b/crates/kernel/src/seam_faces.rs new file mode 100644 index 00000000..d2ec9044 --- /dev/null +++ b/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 = 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)> { + 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>> { + 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; diff --git a/crates/kernel/src/seam_faces_tests.rs b/crates/kernel/src/seam_faces_tests.rs new file mode 100644 index 00000000..9bf32aee --- /dev/null +++ b/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 { + 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}"); +} diff --git a/vendor/README.md b/vendor/README.md index d3c24a17..36bc8f17 100644 --- a/vendor/README.md +++ b/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) diff --git a/vendor/truck-geometry/src/decorators/revolved_curve.rs b/vendor/truck-geometry/src/decorators/revolved_curve.rs index ee4bd7c6..5bee821b 100644 --- a/vendor/truck-geometry/src/decorators/revolved_curve.rs +++ b/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 { + 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 ParametricSurface for RevolutedCurve { type Point = Point3; type Vector = Vector3; @@ -330,14 +344,19 @@ impl SearchParameter 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 SearchNearestParameter 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))) } } }