From 01bf0e4eaf08d224bd0cfdaca3c538ba0438db5b Mon Sep 17 00:00:00 2001 From: Irakli Kourtis Date: Mon, 12 Oct 2026 19:35:43 -0400 Subject: [PATCH] perf: pattern joins skip pushing apart faces that don't meet (#437) Pattern copies of a post joined to a body already holding the original post had their tops in its plane, so every join first pushed faces apart (about 190 ms, then discarded). Faces now coincide only where their boxes meet, the push-apart retry runs once per boolean instead of twice, and splitting prism outlines skips the sampled arc projection for points off the arc's circle. Sensor enclosure evaluation: 3.07 s -> 1.63 s, same bodies. Co-authored-by: Claude Opus 5.5 --- crates/kernel/src/body.rs | 2 +- crates/kernel/src/bore_union_tests.rs | 17 ++++++ crates/kernel/src/coplanar.rs | 85 +++++++++++++++++++++------ crates/kernel/src/ops.rs | 49 ++++++--------- crates/kernel/src/prism.rs | 7 +++ crates/kernel/src/shell_groups.rs | 2 +- 6 files changed, 110 insertions(+), 52 deletions(-) diff --git a/crates/kernel/src/body.rs b/crates/kernel/src/body.rs index 5ca3adf6..11a23ef4 100644 --- a/crates/kernel/src/body.rs +++ b/crates/kernel/src/body.rs @@ -10,7 +10,7 @@ mod planar_regularize; #[path = "planar_sources.rs"] mod planar_sources; #[path = "shell_groups.rs"] -mod shell_groups; +pub(crate) mod shell_groups; pub(crate) type Solid = mt::Solid; diff --git a/crates/kernel/src/bore_union_tests.rs b/crates/kernel/src/bore_union_tests.rs index 54915b5a..3b75567e 100644 --- a/crates/kernel/src/bore_union_tests.rs +++ b/crates/kernel/src/bore_union_tests.rs @@ -116,3 +116,20 @@ fn unproved_curved_boundary_between_clear_endpoints_refuses_recut() { assert!(crate::bore_guard::boundaries_clear(&body, &[], Vec3::ZERO, Vec3::Z, 5., 14., 24., 1e-6).is_none()); assert_eq!(step_export(&[&body], "curved source").unwrap(), before); } + +/// Posts sunk into one floor have their tops in a common plane but apart: they don't +/// coincide, so joining another post doesn't go through pushing faces apart (#261). +#[test] +fn coplanar_faces_apart_from_each_other_do_not_coincide() { + let floor = box_solid(Vec3::new(0., 0., 0.), Vec3::new(80., 60., 2.)).unwrap(); + let post = |x: f64| cylinder(Vec3::new(x, 30., 1.), Vec3::Z, 3., 6.).unwrap(); + let with_one = boolean(&floor, &post(10.), BoolOp::Union).unwrap().unwrap(); + assert!(crate::coplanar::coincident_planes(&with_one, &post(60.)).is_empty()); + assert!(crate::coplanar::boolean_apart(&with_one, &post(60.), BoolOp::Union).is_none()); + // A post over the first one's top does meet it there. + assert!(!crate::coplanar::coincident_planes(&with_one, &cylinder(Vec3::new(12., 30., 7.), Vec3::Z, 3., 6.).unwrap()).is_empty()); + let joined = boolean(&with_one, &post(60.), BoolOp::Union).unwrap().unwrap(); + let v = joined.tessellate_with(1e-3, true).unwrap().measure().volume; + let expected = 80. * 60. * 2. + 2. * std::f64::consts::PI * 9. * 5.; + assert!((v - expected).abs() < 0.5, "volume {v}, expected {expected}"); +} diff --git a/crates/kernel/src/coplanar.rs b/crates/kernel/src/coplanar.rs index cd2816b6..f46e4ec4 100644 --- a/crates/kernel/src/coplanar.rs +++ b/crates/kernel/src/coplanar.rs @@ -13,9 +13,10 @@ use std::collections::HashMap; -use solidcraft_geom::Vec3; +use solidcraft_geom::{Aabb3, Vec3}; use truck_modeling::{self as mt, builder}; +use crate::body::shell_groups::boxes_separate; use crate::body::{Body, Solid, from_p3, p3, v3}; use crate::history::{Change, EdgeOrigin, FaceOrigin, FaceProv, History}; use crate::{BoolOp, KernelError, Result, guard, record}; @@ -243,11 +244,67 @@ fn slab_beyond(n: Vec3, d: f64, depth: f64, extent: f64) -> Result { crate::extrude(&pl, &[r], 0.0, depth)?.pop().ok_or_else(|| KernelError::Failed("slab".into())) } -/// Do the bodies have planar faces in a common plane? -pub(crate) fn has_coincident_faces(a: &Body, b: &Body) -> bool { - let tol = (a.size().max(b.size()) * 1e-7).max(1e-9); +/// The box of a face's boundary sampled to within `tol` of each curve: the face, planar, lies +/// within `tol` of it. `None` when the boundary can't be sampled. +fn face_box(f: &mt::Face, tol: f64) -> Option { + use mt::{BoundedCurve, ParameterDivision1D}; + const MAX_POINTS: usize = 100_000; + guard("face box", || { + let mut bb = Aabb3::EMPTY; + let mut count = 0usize; + for e in f.edge_iter() { + let points = match e.curve() { + mt::Curve::Line(_) => vec![e.front().point(), e.back().point()], + // The division checks its deviation at midpoints only (as in blend.rs). + c => c.parameter_division(c.range_tuple(), tol * 0.25).1, + }; + count = count.saturating_add(points.len()); + if count > MAX_POINTS { + return Err(KernelError::Failed("face box: too many boundary points".into())); + } + for p in points { + bb.add(from_p3(p)); + } + } + if bb.is_empty() { + return Err(KernelError::Failed("face box: no boundary".into())); + } + Ok(bb) + }) + .ok() +} + +/// The planes (normal, offset) in which a planar face of `a` and one of `b` coincide, as `a`'s +/// face lies in them, each with whether `b`'s face points the same way. Faces in a common plane but apart from +/// each other (the tops of two posts standing on one floor) don't coincide: they need no +/// pushing apart. A face whose extent can't be found counts as meeting every face in its plane. +pub(crate) fn coincident_planes(a: &Body, b: &Body) -> Vec<(Vec3, f64, bool)> { + let size = a.size().max(b.size()); + let tol = (size * 1e-7).max(1e-9) * 10.0; + let sample = (size * 1e-4).max(tol); + let (fa, fb): (Vec<&mt::Face>, Vec<&mt::Face>) = (a.solid.face_iter().collect(), b.solid.face_iter().collect()); let (pa, pb) = (planes(a), planes(b)); - pa.iter().any(|(_, x)| pb.iter().any(|(_, y)| x.n.dot(y.n).abs() > 1.0 - 1e-9 && (x.d - y.d * x.n.dot(y.n)).abs() < tol * 10.0)) + let (mut boxes_a, mut boxes_b): (Vec>>, Vec>>) = (vec![None; fa.len()], vec![None; fb.len()]); + let mut found: Vec<(Vec3, f64, bool)> = Vec::new(); + for (ia, x) in &pa { + for (ib, y) in &pb { + let cos = x.n.dot(y.n); + if cos.abs() <= 1.0 - 1e-9 || (x.d - y.d * cos).abs() >= tol { + continue; + } + let same = cos > 0.0; + if found.iter().any(|(n, d, s)| *s == same && n.dot(x.n) > 1.0 - 1e-9 && (d - x.d).abs() < tol) { + continue; + } + let ba = boxes_a.get_mut(*ia).and_then(|b| *b.get_or_insert_with(|| fa.get(*ia).and_then(|f| face_box(f, sample)))); + let bb = boxes_b.get_mut(*ib).and_then(|b| *b.get_or_insert_with(|| fb.get(*ib).and_then(|f| face_box(f, sample)))); + // Each box is within `sample` of its face. + if !ba.zip(bb).is_some_and(|(p, q)| boxes_separate(p, q, 2.0 * sample)) { + found.push((x.n, x.d, same)); + } + } + } + found } /// Retry a failed boolean with coincident planar faces pushed apart. `None` when there are no @@ -255,22 +312,12 @@ pub(crate) fn has_coincident_faces(a: &Body, b: &Body) -> bool { pub(crate) fn boolean_apart(a: &Body, b: &Body, op: BoolOp) -> Option>> { let size = a.size().max(b.size()); let tol = (size * 1e-7).max(1e-9); - let (pa, pb) = (planes(a), planes(b)); - let mut same: Vec<(Vec3, f64)> = Vec::new(); - let mut back: Vec<(Vec3, f64)> = Vec::new(); - for (_, x) in &pa { - for (_, y) in &pb { - if (x.d - y.d * x.n.dot(y.n)).abs() < tol * 10.0 && x.n.dot(y.n).abs() > 1.0 - 1e-9 { - let list = if x.n.dot(y.n) > 0.0 { &mut same } else { &mut back }; - if !list.iter().any(|(n, d)| n.dot(x.n) > 1.0 - 1e-9 && (d - x.d).abs() < tol * 10.0) { - list.push((x.n, x.d)); - } - } - } - } - if same.is_empty() && back.is_empty() { + let coincident = coincident_planes(a, b); + if coincident.is_empty() { return None; } + let same: Vec<(Vec3, f64)> = coincident.iter().filter(|(_, _, s)| *s).map(|(n, d, _)| (*n, *d)).collect(); + let back: Vec<(Vec3, f64)> = coincident.iter().filter(|(_, _, s)| !*s).map(|(n, d, _)| (*n, *d)).collect(); let (d1, d2) = (size * 0.0137, size * 0.0229); let counts = record::counts(&[a, b]); let run = || -> Result> { diff --git a/crates/kernel/src/ops.rs b/crates/kernel/src/ops.rs index 6ca87e09..a0444b0a 100644 --- a/crates/kernel/src/ops.rs +++ b/crates/kernel/src/ops.rs @@ -386,19 +386,27 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { return Ok(Some(body)); } } - if !APART.with(|c| c.get()) && crate::coplanar::has_coincident_faces(a, b) { + // Coincident planar faces: push them apart (exactly). Tried once, before the general + // boolean; why it failed is reported if everything else fails too. + let mut apart_failed = String::new(); + if !APART.with(|c| c.get()) { APART.with(|c| c.set(true)); let (r, diagnostics) = capture_boolean_diagnostics(|| crate::coplanar::boolean_apart(a, b, op)); APART.with(|c| c.set(false)); - if let Some(Ok(Some(body))) = r { - let (ok, _) = check(&body); - if ok { - merge(diagnostics); - let mut event = BooleanFallback::new(BooleanStrategy::CoplanarSeparation); - event.face_shift_mm = size * 0.0229; - record(event); - return Ok(Some(body)); + match r { + Some(Ok(Some(body))) => { + let (ok, v) = check(&body); + if ok { + merge(diagnostics); + let mut event = BooleanFallback::new(BooleanStrategy::CoplanarSeparation); + event.face_shift_mm = size * 0.0229; + record(event); + return Ok(Some(body)); + } + apart_failed = format!("; faces pushed apart gave an implausible volume {v:.4}"); } + Some(Err(e)) => apart_failed = format!("; {e}"), + Some(Ok(None)) | None => {} } } // Bores on a common cylinder (a pad's bore around the hole it is joined over). @@ -459,28 +467,7 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { return Ok(None); } } - // Coincident planar faces: push them apart (exactly) and try again. - if !APART.with(|c| c.get()) { - APART.with(|c| c.set(true)); - let (r, diagnostics) = capture_boolean_diagnostics(|| crate::coplanar::boolean_apart(a, b, op)); - APART.with(|c| c.set(false)); - match r { - Some(Ok(Some(body))) => { - let (ok, v) = check(&body); - if ok { - merge(diagnostics); - let mut event = BooleanFallback::new(BooleanStrategy::CoplanarSeparation); - event.face_shift_mm = size * 0.0229; - record(event); - return Ok(Some(body)); - } - last = format!("{last}; faces pushed apart gave an implausible volume {v:.4}"); - } - Some(Ok(None)) => {} - Some(Err(e)) => last = format!("{last}; {e}"), - None => {} - } - } + last.push_str(&apart_failed); // Coincident faces defeat the B-rep boolean; bodies with only planar faces have an exact // polygon fallback. match crate::polybool::planar_boolean(a, b, op) { diff --git a/crates/kernel/src/prism.rs b/crates/kernel/src/prism.rs index a3fa5bf7..881e49a1 100644 --- a/crates/kernel/src/prism.rs +++ b/crates/kernel/src/prism.rs @@ -1110,6 +1110,13 @@ fn split_seg(seg: &Seg2, pts: &[Vec2], tol: f64) -> Option> { let mut ts: Vec = pts .iter() .filter_map(|p| { + // A point within `tol` of an arc is within `tol` of its circle: the others need no + // (sampled) projection. + if let Seg2::Arc { center, radius, .. } = seg + && (p.dist(*center) - radius).abs() >= tol + { + return None; + } let t = seg.closest_param(*p); (t > 1e-9 && t < 1.0 - 1e-9 && seg.point_at(t).dist(*p) < tol).then_some(t) }) diff --git a/crates/kernel/src/shell_groups.rs b/crates/kernel/src/shell_groups.rs index 5d3bc192..cc2c8703 100644 --- a/crates/kernel/src/shell_groups.rs +++ b/crates/kernel/src/shell_groups.rs @@ -24,7 +24,7 @@ fn box_of(points: &[Vec3]) -> Aabb3 { bounds } -fn boxes_separate(a: Aabb3, b: Aabb3, gap: f64) -> bool { +pub(crate) fn boxes_separate(a: Aabb3, b: Aabb3, gap: f64) -> bool { a.max.x < b.min.x - gap || b.max.x < a.min.x - gap || a.max.y < b.min.y - gap