diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2ad0dbe8..9ecfc01a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -195,6 +195,35 @@ fn join_meeting_nothing_keeps_the_first_body() { assert!(rel(volume(&mut s), 1064.0) < 1e-6); } +/// A 1 mm floor extruded into a through hole, from the sketch the plate was extruded from: the +/// disc's wall lies on the hole's and its foot in the plate's bottom face. A wedge extruded +/// sideways on top keeps the plate from being a prism along Z (the 2D route would take it +/// otherwise), and the join failed with "boolean Union: the operation failed: no result". +#[test] +fn join_a_floor_into_a_through_hole() { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY", "name": "Plate"})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [100, 60]})); + run(&mut s, "sketch.circle.center", json!({"center": [60, 30], "radius": 20})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"sketch": "Plate", "profiles": [{"point": [5, 5]}], "distance": 10})); + run(&mut s, "sketch.create", json!({"plane": "XZ", "name": "Wedge"})); + run(&mut s, "sketch.line", json!({"points": [[5, -10], [25, -10], [5, -16]], "closed": true})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"sketch": "Wedge", "distance": 40, "operation": "join"})); + let before = volume(&mut s); + run(&mut s, "solid.extrude", json!({"sketch": "Plate", "profiles": [{"point": [60, 30]}], "distance": 1, "operation": "join"})); + let st = s.model.state(); + assert_eq!(st.bodies.len(), 1); + assert_eq!(st.bodies[0].body.lumps().unwrap().len(), 1); + assert!(st.bodies[0].body.validity().is_empty(), "{:?}", st.bodies[0].body.validity()); + // Exact: the disc isn't moved to get the bodies to intersect, so the feature has no warning. + let warning = s.model.results.last().and_then(|r| r.warning.clone()); + assert!(warning.is_none(), "{warning:?}"); + let added = volume(&mut s) - before; + assert!(rel(added, PI * 400.0) < 1e-3, "{added}"); +} + /// Two spheres of one size: the join's contact test tried every pair of their finely meshed /// triangles and refused the join as too much work (#232). #[test] diff --git a/crates/kernel/src/boolean_diagnostics.rs b/crates/kernel/src/boolean_diagnostics.rs index f29dc10e..e12e407e 100644 --- a/crates/kernel/src/boolean_diagnostics.rs +++ b/crates/kernel/src/boolean_diagnostics.rs @@ -14,6 +14,8 @@ pub enum BooleanStrategy { ShiftedOperands, CoplanarSeparation, CommonBore, + /// A boss in the other body's bore, grown into the material around it (nothing recut). + GrownBoss, PlanarFallback, ToolNudge, Piecewise, @@ -34,6 +36,15 @@ pub struct BooleanFallback { pub tolerance_mm: f64, } +impl BooleanStrategy { + /// Whether a result made this way can differ from the exact one: the tool was moved + /// against the other body, or part of where they meet was ignored. The other retries work + /// on temporary geometry or tolerances only, and their results are the exact ones. + pub fn alters_result(self) -> bool { + matches!(self, BooleanStrategy::ToolNudge | BooleanStrategy::UnverifiedShortcut) + } +} + impl BooleanFallback { pub(crate) fn new(strategy: BooleanStrategy) -> Self { Self { strategy, translation_mm: 0., rotation_rad: 0., face_shift_mm: 0., tolerance_mm: 0. } @@ -47,6 +58,7 @@ impl BooleanFallback { } BooleanStrategy::CoplanarSeparation => format!("temporary coincident-face separation up to {:.6e} mm per face", self.face_shift_mm), BooleanStrategy::CommonBore => format!("temporary bore offset {:.6e} mm, followed by recutting the original bore", self.face_shift_mm), + BooleanStrategy::GrownBoss => format!("boss grown {:.6e} mm into the material around the bore it fills", self.face_shift_mm), BooleanStrategy::PlanarFallback => "exact planar fallback (no displacement)".into(), BooleanStrategy::ToolNudge => format!("tool displacement {:.6e} mm, rotation {:.6e} rad", self.translation_mm, self.rotation_rad), BooleanStrategy::Piecewise => "piecewise retry (no displacement)".into(), @@ -79,13 +91,26 @@ impl BooleanDiagnostics { self.events.iter().find(|event| event.strategy == BooleanStrategy::UnverifiedShortcut).map(BooleanFallback::description) } + /// Every retry the result took, for a message about the boolean (one that failed its + /// checks lists what was tried). + pub fn summary(&self) -> Option { + self.described(&self.events.iter().collect::>()) + } + + /// The note a feature carries: the retries that can make its result differ from the exact + /// one ([`BooleanStrategy::alters_result`]). Exact retries are in `events` all the same + /// (`timeline.inspect`), but they leave the result as it should be: no reason to warn. pub fn warning(&self) -> Option { - if self.events.is_empty() && !self.truncated { + self.described(&self.events.iter().filter(|e| e.strategy.alters_result()).collect::>()) + } + + fn described(&self, events: &[&BooleanFallback]) -> Option { + if events.is_empty() && !self.truncated { return None; } - let mut descriptions: Vec = self.events.iter().take(4).map(BooleanFallback::description).collect(); - if self.events.len() > 4 { - descriptions.push(format!("{} additional retries", self.events.len() - 4)); + let mut descriptions: Vec = events.iter().take(4).map(|e| e.description()).collect(); + if events.len() > 4 { + descriptions.push(format!("{} additional retries", events.len() - 4)); } if self.truncated { descriptions.push(format!("report capped at {MAX_EVENTS} retries")); @@ -169,6 +194,26 @@ mod tests { assert!(capture_boolean_diagnostics(|| {}).1.events.is_empty()); } + /// Only retries whose result can differ from the exact one make a warning; the summary + /// lists every retry. + #[test] + fn only_retries_that_alter_the_result_warn() { + let report = |strategies: &[BooleanStrategy]| { + capture_boolean_diagnostics(|| { + for s in strategies { + record(BooleanFallback::new(*s)); + } + }) + .1 + }; + let exact = report(&[BooleanStrategy::ShiftedOperands, BooleanStrategy::GrownBoss, BooleanStrategy::CoplanarSeparation]); + assert!(exact.warning().is_none(), "{exact:?}"); + assert!(exact.summary().is_some_and(|s| s.contains("shifted operands") && s.contains("boss grown"))); + let nudged = report(&[BooleanStrategy::GrownBoss, BooleanStrategy::ToolNudge]); + let warning = nudged.warning().unwrap(); + assert!(warning.contains("tool displacement") && !warning.contains("boss grown"), "{warning}"); + } + #[test] fn exact_operations_and_failed_calls_have_no_retry_report() { use solidcraft_geom::Vec3; diff --git a/crates/kernel/src/coplanar.rs b/crates/kernel/src/coplanar.rs index ab0f71ed..364bc955 100644 --- a/crates/kernel/src/coplanar.rs +++ b/crates/kernel/src/coplanar.rs @@ -989,6 +989,9 @@ pub(crate) fn has_coincident_cylinders(a: &Body, b: &Body) -> bool { /// This is not a general emptiness or Boolean-equivalence certificate. Each direction /// returns a result only when `accept` takes it; otherwise this returns `None`. The result's /// history is over `[a, b]`. +/// +/// A boss standing in the other body's bore on that cylinder (a disc joined into the bottom of +/// a hole it fills) is grown instead ([`grown_boss_union`]). pub(crate) fn bore_union(a: &Body, b: &Body, accept: &dyn Fn(&Body) -> bool) -> Option { let size = a.size().max(b.size()); let tol = (size * 1e-7).max(1e-9) * 10.0; @@ -997,10 +1000,10 @@ pub(crate) fn bore_union(a: &Body, b: &Body, accept: &dyn Fn(&Body) -> bool) -> let (ia, ib) = (ma.inside_index(), mb.inside_index()); let empty = |p: Vec3| !ia.contains(p) && !ib.contains(p); let (ca, cb) = (cylinders(a), cylinders(b)); - // (body to narrow, its operand number, its mesh and inside test, the other body, their faces - // on the cylinder). - let ways = [(b, 1, &mb, &ib, a, &cb, &ca), (a, 0, &ma, &ia, b, &ca, &cb)]; - for (tool, tool_operand, mesh, inside, target, ct, co) in ways { + // (body to narrow, its operand number, its mesh and inside test, the other body and its + // inside test, their faces on the cylinder). + let ways = [(b, 1, &mb, &ib, a, &ia, &cb, &ca), (a, 0, &ma, &ia, b, &ib, &ca, &cb)]; + for (tool, tool_operand, mesh, inside, target, target_inside, ct, co) in ways { for c in ct { if !co.iter().any(|o| same_cylinder(c, o, tol)) { continue; @@ -1032,6 +1035,13 @@ pub(crate) fn bore_union(a: &Body, b: &Body, accept: &dyn Fn(&Body) -> bool) -> let Some(radial) = (q - along).normalized() else { continue }; let on = along + radial * c.radius; let e = (c.radius * 0.05).max(size * 1e-4); + if inside.contains(on - radial * e) && !inside.contains(on + radial * e) { + let boss = Boss { tool, operand: tool_operand, cylinder: c, faces: &bore, t0, t1 }; + if let Some(r) = grown_boss_union(a, b, &boss, target_inside, size, accept) { + return Some(r); + } + continue; + } if !(inside.contains(on + radial * e) && !inside.contains(on - radial * e)) { continue; } @@ -1099,6 +1109,127 @@ pub(crate) fn bore_union(a: &Body, b: &Body, accept: &dyn Fn(&Body) -> bool) -> None } +/// A tool (operand `operand`) with faces on a cylinder it shares with the other body, the +/// material inside them (a boss), and its extent `t0..t1` along the axis there. +struct Boss<'a> { + tool: &'a Body, + operand: usize, + cylinder: &'a CylFace, + faces: &'a [usize], + t0: f64, + t1: f64, +} + +/// Union of `a` and `b` where one has a boss on a cylinder that is a bore of the other body (a disc joined into the bottom of a hole it fills): the boss's walls +/// coincide with the bore's, which the mesh intersection can't take. Grown outward by a step, +/// the boss reaches into the material around the bore instead, and joining it adds nothing +/// more, provided that material is there all along the boss. The ring the step adds is sampled +/// against the other body for that, and `accept` checks the result as a whole. The result's +/// history is over `[a, b]` (the grown tool keeps the tool's faces and edges). +fn grown_boss_union( + a: &Body, + b: &Body, + boss: &Boss, + target_inside: &solidcraft_geom::InsideIndex, + size: f64, + accept: &dyn Fn(&Body) -> bool, +) -> Option { + let c = boss.cylinder; + let d = (c.radius * 0.2).min(size * 0.01); + let (u, w) = (c.axis.any_perp(), c.axis.cross(c.axis.any_perp())); + let at = |s: f64, r: f64, ang: f64| c.origin + c.axis * s + (u * ang.cos() + w * ang.sin()) * r; + // The other body's material all around the boss, as far out as it grows, over its length. + let backed = (1..=9).all(|i| { + let s = boss.t0 + (boss.t1 - boss.t0) * f64::from(i) / 10.0; + (0..16).all(|k| { + let ang = f64::from(k) * std::f64::consts::FRAC_PI_8 + 0.3; + [0.1, 0.5, 0.95].iter().all(|f| target_inside.contains(at(s, c.radius + d * f, ang))) + }) + }); + if !backed { + return None; + } + // Ends where the bore comes out of the other body and the boss's cap lies flush with the + // face it comes out of (a floor at the bottom of a through hole): faces in one plane, which + // the intersection can't take either. There the cap is moved out by the same step, and the + // stub that leaves past the face is deleted afterwards, the face healing over it. + let tol = (size * 1e-7).max(1e-9) * 10.0; + let target = if boss.operand == 1 { a } else { b }; + let along = |p: Vec3| (p - c.origin).dot(c.axis); + let radial = |p: Vec3| (p - c.origin - c.axis * along(p)).len(); + let plane_at = |body: &Body, i: usize, s: f64, out: f64| { + matches!(crate::face_surface(body, i), Some(crate::FaceSurface::Plane { normal, point }) + if normal.dot(c.axis) * out > 1.0 - 1e-9 && (along(point) - s).abs() < tol) + }; + let mut caps: Vec<(usize, f64, f64)> = Vec::new(); + for (s, out) in [(boss.t0, -1.0), (boss.t1, 1.0)] { + let open = (0..8).all(|k| { + let ang = f64::from(k) * std::f64::consts::FRAC_PI_4 + 0.3; + !target_inside.contains(at(s + out * d * 0.5, c.radius + d * 0.5, ang)) + }); + if !open || !(0..target.face_count()).any(|i| plane_at(target, i, s, out)) { + continue; + } + let cap = boss + .tool + .solid + .face_iter() + .enumerate() + .position(|(i, f)| plane_at(boss.tool, i, s, out) && f.vertex_iter().all(|v| radial(from_p3(v.point())) < c.radius + tol)); + // A flush end with no cap of its own to move: the faces would stay in one plane. + caps.push((cap?, s, out)); + } + let counts = record::counts(&[a, b]); + let run = || -> Result> { + let grown = crate::offset::offset_body(boss.tool, |i, _| if boss.faces.contains(&i) || caps.iter().any(|x| x.0 == i) { d } else { 0.0 })?; + if !grown.validity().is_empty() { + return Err(KernelError::Failed("growing a boss: an unsound result".into())); + } + // In the operands' order, so the history is over [a, b] once the grown tool's own is + // composed in. + let joined = if boss.operand == 1 { crate::ops::boolean(a, &grown, BoolOp::Union)? } else { crate::ops::boolean(&grown, b, BoolOp::Union)? }; + let Some(joined) = joined else { return Ok(None) }; + let joined = { + let h = crate::boolean_record::through(&joined, boss.operand, grown.history()); + joined.with_checked_history(h, &counts) + }; + if caps.is_empty() { + return Ok(Some(joined)); + } + // The stubs: faces wholly past a flush end, within the grown boss, reaching out to its cap. + let stubs: Vec = joined + .solid + .face_iter() + .enumerate() + .filter(|(_, f)| { + let ps: Vec = f.vertex_iter().map(|v| from_p3(v.point())).collect(); + caps.iter().any(|&(_, s, out)| { + let past = |p: &Vec3| (along(*p) - s) * out; + !ps.is_empty() && ps.iter().all(|p| past(p) > -tol && radial(*p) < c.radius + d * 1.01) && ps.iter().any(|p| past(p) > d * 0.5) + }) + }) + .map(|(i, _)| i) + .collect(); + if stubs.is_empty() { + return Err(KernelError::Failed("growing a boss: no stub past its flush end".into())); + } + let healed = crate::delete_faces(&joined, &stubs)?; + let h = crate::boolean_record::through(&healed, 0, joined.history()); + Ok(Some(healed.with_checked_history(h, &counts))) + }; + let (result, diagnostics) = crate::capture_boolean_diagnostics(run); + match result { + Ok(Some(r)) if accept(&r) => { + crate::boolean_diagnostics::merge(diagnostics); + let mut event = crate::BooleanFallback::new(crate::BooleanStrategy::GrownBoss); + event.face_shift_mm = d; + crate::boolean_diagnostics::record(event); + Some(r) + } + _ => None, + } +} + #[cfg(test)] #[path = "coplanar_tests.rs"] mod tests; diff --git a/crates/kernel/src/coplanar_tests.rs b/crates/kernel/src/coplanar_tests.rs index a93382a1..a474c03c 100644 --- a/crates/kernel/src/coplanar_tests.rs +++ b/crates/kernel/src/coplanar_tests.rs @@ -75,3 +75,24 @@ fn faces_with_room_are_pushed_apart() { let (added, lumps, how) = change(&block, &pad, BoolOp::Union); assert!((added - 30_000.0).abs() < 1e-6 && lumps == 1 && used(&how, BooleanStrategy::CoplanarSeparation), "{added}, {how:?}"); } + +/// A disc joined into a round hole it fills (issue: "boolean Union: no result" for a 1 mm +/// floor extruded into a through hole of a part that isn't a prism). The disc's wall lies on +/// the hole's, and at either end its face lies in the block's bottom or top face: it adds its +/// own volume, the block stays one piece, and the hole is closed where the disc is. +#[test] +fn a_disc_joined_into_the_hole_it_fills() { + let hole = cylinder(Vec3::new(150.0, 50.0, -1.0), Vec3::Z, 20.0, 22.0).unwrap(); + let drilled = boolean(&block(200.0, 100.0, 20.0), &hole, BoolOp::Cut).unwrap().unwrap(); + for (z0, z1) in [(0.0, 1.0), (5.0, 8.0), (19.0, 20.0)] { + let disc = cylinder(Vec3::new(150.0, 50.0, z0), Vec3::Z, 20.0, z1 - z0).unwrap(); + let want = measure(&disc).unwrap().volume; + let (r, how) = capture_boolean_diagnostics(|| boolean(&drilled, &disc, BoolOp::Union)); + let r = r.unwrap().unwrap(); + let added = measure(&r).unwrap().volume - measure(&drilled).unwrap().volume; + assert!((added - want).abs() < want * 1e-3, "{z0}..{z1}: {added} vs {want}, {how:?}"); + assert!(r.lumps().unwrap().len() == 1 && r.validity().is_empty(), "{z0}..{z1}: {:?}", r.validity()); + // Exact: nothing of the tool is moved. + assert!(used(&how, BooleanStrategy::GrownBoss) && !used(&how, BooleanStrategy::ToolNudge), "{z0}..{z1}: {how:?}"); + } +} diff --git a/crates/kernel/src/ops.rs b/crates/kernel/src/ops.rs index 14eaf396..4ea3dfc4 100644 --- a/crates/kernel/src/ops.rs +++ b/crates/kernel/src/ops.rs @@ -362,7 +362,7 @@ fn boolean_via(a: &Body, b: &Body, op: BoolOp, whole: &dyn Fn(&Body, &Body, Bool && let Ok(result) = &r && let Err(e) = crate::boolean_guard::check_retry(a, b, op, result.as_ref()) { - let tried = diagnostics.warning().unwrap_or_default(); + let tried = diagnostics.summary().unwrap_or_default(); r = Err(match e { KernelError::Failed(m) => KernelError::Failed(format!("{m} ({})", tried.trim_start_matches("Boolean retry: "))), other => other, @@ -593,6 +593,18 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { return Ok(Some(body)); } } + // Bores on a common cylinder (a pad's bore around the hole it is joined over, a disc filling + // a hole). Asked before faces are pushed apart: that doesn't part walls on one cylinder, and + // its tries at the general boolean, all failing on those walls, cost seconds. + if op == BoolOp::Union && !BORES.with(|c| c.get()) && crate::coplanar::has_coincident_cylinders(a, b) { + BORES.with(|c| c.set(true)); + let (r, diagnostics) = capture_boolean_diagnostics(|| crate::coplanar::bore_union(a, b, &|body| check(body).0)); + BORES.with(|c| c.set(false)); + if let Some(body) = r { + merge(diagnostics); + return Ok(Some(body)); + } + } // 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(); @@ -646,16 +658,6 @@ fn boolean_whole(a: &Body, b: &Body, op: BoolOp) -> Result> { { return Ok(Some(body)); } - // Bores on a common cylinder (a pad's bore around the hole it is joined over). - if op == BoolOp::Union && !BORES.with(|c| c.get()) && crate::coplanar::has_coincident_cylinders(a, b) { - BORES.with(|c| c.set(true)); - let (r, diagnostics) = capture_boolean_diagnostics(|| crate::coplanar::bore_union(a, b, &|body| check(body).0)); - BORES.with(|c| c.set(false)); - if let Some(body) = r { - merge(diagnostics); - return Ok(Some(body)); - } - } // Faces back to back over part of their area: imprinted on each other and glued (exact; held // to the retry guard's checks all the same). if op == BoolOp::Union && crate::coplanar::faces_touch(a, b) { diff --git a/docs/robustness.md b/docs/robustness.md index f775f153..3516f468 100644 --- a/docs/robustness.md +++ b/docs/robustness.md @@ -56,8 +56,11 @@ The changed seeds and the reason are added to the file's `notes`; say why in the ## Successful Boolean retries -When an accepted Boolean result needs a numerical retry, the feature shows a -warning in the timeline. `timeline.inspect` and `doc.inspect` expose its +When an accepted Boolean result needs a retry that can make it differ from the +exact one (a tool nudge, or the unverified shortcut), the feature shows a +warning in the timeline. Every other retry works on temporary geometry or +tolerances and leaves the exact result, so it is recorded but not warned about. +`timeline.inspect` and `doc.inspect` expose every retry in the feature's `boolean_diagnostics`, including the strategy and the applied translation, rotation, face offset and tessellation tolerance in millimetres/radians. @@ -77,7 +80,7 @@ Exact planar and piecewise fallbacks report zero displacement. The existing volume and spatial acceptance checks still apply to all results. The command that adds a feature carries the same `warning` in its reply, so a -script or an agent sees a retry without asking the timeline. +script or an agent sees such a retry without asking the timeline. A join or a cut that needed any retry is then held to what it can do at all (`crates/kernel/src/boolean_guard.rs`, #616): the result is closed, a join