diff --git a/apps/solidcraft-cli/tests/corpus/README.md b/apps/solidcraft-cli/tests/corpus/README.md index 5216c111..d9266d8b 100644 --- a/apps/solidcraft-cli/tests/corpus/README.md +++ b/apps/solidcraft-cli/tests/corpus/README.md @@ -19,9 +19,10 @@ hostile variants of it never panic. | `move-bodies` | 00baf1d | `solid.move` before it could copy: two bodies, one translated in place, one translated and turned 90 deg about Z (no `copy` key anywhere in the file) | | `split-body-pieces` | v0.3.2 (07e263d) | a slot along X cuts a block into two bodies; a fillet, a chamfer and an appearance refer to the pieces by body name (`Body1`, `Body1 (1)`) | | `issue-489-stacked-join` | v0.6.0 (809ecf7), desktop | a 70 × 84 extrude, then a 70 × 62 strip joined on its top face, sketched partly over the face's projected edges (issue #489; expected values from main at 2880525) | +| `issue-616-lid-ribs` | v0.7.0 (a9879fa), CLI | a rounded lid with a skirt ring, a window and a slot cut through, then four ribs joined on the plate over the skirt, flush with the window's sides; v0.7.0 displaced the ribs and left a slit through the skirt and plate (issue #616; script `examples/regressions/lid_ribs_flush_with_window.json`, expected values from the fix) | The scripts that made them are in `scripts/` (`issue-489-stacked-join` was drawn in the desktop -app and has none). Every flat face of every final body must also take an attached sketch and an +app and has none; `issue-616-lid-ribs` is made by its regression script). Every flat face of every final body must also take an attached sketch and an offset plane; the faces that can't yet are listed with their issues in `design_corpus.rs`. Never regenerate a file with a newer build: the point is that it was written by the old one. Add new files for new eras instead. diff --git a/apps/solidcraft-cli/tests/corpus/issue-616-lid-ribs.expected.json b/apps/solidcraft-cli/tests/corpus/issue-616-lid-ribs.expected.json new file mode 100644 index 00000000..b605b13c --- /dev/null +++ b/apps/solidcraft-cli/tests/corpus/issue-616-lid-ribs.expected.json @@ -0,0 +1,227 @@ +{ + "bodies": [ + { + "area_mm2": 8430.29544246263, + "bbox": { + "max": [ + 176.8000000000002, + 89.30000000000001, + 14.100000000000001 + ], + "min": [ + 99.99999999999991, + -1.820765760385257e-14, + 0.0 + ] + }, + "center_of_mass": [ + 138.3999999716193, + 51.04392591016203, + 2.0990634391586322 + ], + "density_g_cm3": 1.29, + "edges": 156, + "face_types": { + "cylinder": 4, + "plane": 50 + }, + "faces": 54, + "kernel": { + "edges": 156, + "faces": 54, + "shells": 1, + "vertices": 104 + }, + "mass_g": 9.101726874183207, + "material": "Default", + "name": "Lid", + "vertices": 104, + "volume_mm3": 7055.602228048997 + } + ], + "body_count": 1, + "params": [], + "sketches": [ + { + "curves": 8, + "dof": 8, + "id": 3, + "name": "Skirt", + "profiles": 2, + "status": "solved", + "visible": false + } + ], + "timeline": [ + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 1, + "ms": 1.178957, + "name": "Box1", + "rolled_back": false, + "suppressed": false, + "type": "BoxFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 2, + "ms": 83.24342399999999, + "name": "Fillet1", + "rolled_back": false, + "suppressed": false, + "type": "FilletFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 3, + "ms": 0.150889, + "name": "Skirt", + "rolled_back": false, + "suppressed": false, + "type": "Sketch", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 4, + "ms": 94.465481, + "name": "Extrude1", + "rolled_back": false, + "suppressed": false, + "type": "ExtrudeFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 5, + "ms": 50.426736, + "name": "Box2", + "rolled_back": false, + "suppressed": false, + "type": "BoxFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 6, + "ms": 40.383005, + "name": "Box3", + "rolled_back": false, + "suppressed": false, + "type": "BoxFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 7, + "ms": 59.041461000000005, + "name": "Box4", + "rolled_back": false, + "suppressed": false, + "type": "BoxFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [ + { + "face_shift_mm": 0.0, + "rotation_rad": 0.0, + "strategy": "planar_fallback", + "tolerance_mm": 0.0, + "translation_mm": 0.0 + } + ], + "truncated": false + }, + "error": null, + "id": 8, + "ms": 562.5477970000001, + "name": "Rib1", + "rolled_back": false, + "suppressed": false, + "type": "BoxFeature", + "warning": "Boolean retry: exact planar fallback (no displacement)" + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 9, + "ms": 89.56076, + "name": "Rib2", + "rolled_back": false, + "suppressed": false, + "type": "BoxFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 10, + "ms": 65.330634, + "name": "Rib3", + "rolled_back": false, + "suppressed": false, + "type": "BoxFeature", + "warning": null + }, + { + "boolean_diagnostics": { + "events": [], + "truncated": false + }, + "error": null, + "id": 11, + "ms": 97.229403, + "name": "Rib4", + "rolled_back": false, + "suppressed": false, + "type": "BoxFeature", + "warning": null + } + ], + "total": { + "area_mm2": 8430.29544246263, + "edges": 156, + "faces": 54, + "mass_g": 9.101726874183207, + "vertices": 104, + "volume_mm3": 7055.602228048997 + } +} diff --git a/apps/solidcraft-cli/tests/corpus/issue-616-lid-ribs.solidcraft b/apps/solidcraft-cli/tests/corpus/issue-616-lid-ribs.solidcraft new file mode 100644 index 00000000..e2315262 --- /dev/null +++ b/apps/solidcraft-cli/tests/corpus/issue-616-lid-ribs.solidcraft @@ -0,0 +1,507 @@ +{ + "features": [ + { + "body_names": [ + "Lid" + ], + "corner": [ + 100.0, + 0.0, + 0.0 + ], + "height": "2.4", + "id": 1, + "length": "76.8", + "name": "Box1", + "operation": "new_body", + "param_names": [ + "d1", + "d2", + "d3" + ], + "suppressed": false, + "type": "box", + "width": "89.3" + }, + { + "edge_names": [ + "F1:-x|F1:-y", + "F1:+x|F1:-y", + "F1:+y|F1:-x", + "F1:+x|F1:+y" + ], + "edge_refs": [ + { + "dir": [ + 0.0, + 0.0, + -1.0 + ], + "max": [ + 176.8, + 89.3, + 2.4 + ], + "min": [ + 100.0, + 0.0, + 0.0 + ] + }, + { + "dir": [ + 0.0, + 0.0, + 1.0 + ], + "max": [ + 176.8, + 89.3, + 2.4 + ], + "min": [ + 100.0, + 0.0, + 0.0 + ] + }, + { + "dir": [ + 0.0, + 0.0, + 1.0 + ], + "max": [ + 176.8, + 89.3, + 2.4 + ], + "min": [ + 100.0, + 0.0, + 0.0 + ] + }, + { + "dir": [ + 0.0, + 0.0, + 1.0 + ], + "max": [ + 176.8, + 89.3, + 2.4 + ], + "min": [ + 100.0, + 0.0, + 0.0 + ] + } + ], + "edges": [ + [ + 100.0, + 0.0, + 1.2 + ], + [ + 176.8, + 0.0, + 1.2 + ], + [ + 100.0, + 89.3, + 1.2 + ], + [ + 176.8, + 89.3, + 1.2 + ] + ], + "id": 2, + "name": "Fillet1", + "param_names": [ + "d4" + ], + "radius": "3", + "suppressed": false, + "type": "fillet" + }, + { + "id": 3, + "name": "Skirt", + "param_names": [ + "d5" + ], + "plane": { + "base": { + "name": "XY", + "type": "origin" + }, + "distance": "2.4", + "type": "offset" + }, + "sketch": { + "constraints": [ + { + "id": "k1", + "l": 0, + "type": "horizontal" + }, + { + "id": "k2", + "l": 2, + "type": "horizontal" + }, + { + "id": "k3", + "l": 1, + "type": "vertical" + }, + { + "id": "k4", + "l": 3, + "type": "vertical" + }, + { + "id": "k5", + "l": 4, + "type": "horizontal" + }, + { + "id": "k6", + "l": 6, + "type": "horizontal" + }, + { + "id": "k7", + "l": 5, + "type": "vertical" + }, + { + "id": "k8", + "l": 7, + "type": "vertical" + } + ], + "counters": { + "k": 8, + "l": 8, + "p": 8 + }, + "curves": [ + { + "a": 1, + "b": 2, + "construction": false, + "id": "l1", + "type": "line" + }, + { + "a": 2, + "b": 3, + "construction": false, + "id": "l2", + "type": "line" + }, + { + "a": 3, + "b": 4, + "construction": false, + "id": "l3", + "type": "line" + }, + { + "a": 4, + "b": 1, + "construction": false, + "id": "l4", + "type": "line" + }, + { + "a": 5, + "b": 6, + "construction": false, + "id": "l5", + "type": "line" + }, + { + "a": 6, + "b": 7, + "construction": false, + "id": "l6", + "type": "line" + }, + { + "a": 7, + "b": 8, + "construction": false, + "id": "l7", + "type": "line" + }, + { + "a": 8, + "b": 5, + "construction": false, + "id": "l8", + "type": "line" + } + ], + "points": [ + { + "fixed": true, + "id": "origin", + "pos": [ + 0.0, + 0.0 + ] + }, + { + "fixed": false, + "id": "p1", + "pos": [ + 102.65, + 2.65 + ] + }, + { + "fixed": false, + "id": "p2", + "pos": [ + 174.15, + 2.65 + ] + }, + { + "fixed": false, + "id": "p3", + "pos": [ + 174.15, + 86.65 + ] + }, + { + "fixed": false, + "id": "p4", + "pos": [ + 102.65, + 86.65 + ] + }, + { + "fixed": false, + "id": "p5", + "pos": [ + 104.15, + 4.15 + ] + }, + { + "fixed": false, + "id": "p6", + "pos": [ + 172.65, + 4.15 + ] + }, + { + "fixed": false, + "id": "p7", + "pos": [ + 172.65, + 85.15 + ] + }, + { + "fixed": false, + "id": "p8", + "pos": [ + 104.15, + 85.15 + ] + } + ] + }, + "suppressed": false, + "type": "sketch" + }, + { + "extent": { + "direction": "positive", + "distance": "3" + }, + "id": 4, + "name": "Extrude1", + "operation": "join", + "param_names": [ + "d6" + ], + "profiles": { + "loops": [ + [ + "l1", + "l2", + "l3", + "l4" + ] + ], + "type": "curves" + }, + "sketch": 3, + "suppressed": false, + "targets": [ + "Lid" + ], + "type": "extrude" + }, + { + "corner": [ + 104.4, + 5.4, + -1.0 + ], + "height": "8", + "id": 5, + "length": "68", + "name": "Box2", + "operation": "cut", + "param_names": [ + "d7", + "d8", + "d9" + ], + "suppressed": false, + "type": "box", + "width": "61" + }, + { + "corner": [ + 116.4, + 72.0, + -1.0 + ], + "height": "8", + "id": 6, + "length": "44", + "name": "Box3", + "operation": "cut", + "param_names": [ + "d10", + "d11", + "d12" + ], + "suppressed": false, + "type": "box", + "width": "10" + }, + { + "corner": [ + 127.4, + 84.0, + 2.4 + ], + "height": "4", + "id": 7, + "length": "22", + "name": "Box4", + "operation": "cut", + "param_names": [ + "d13", + "d14", + "d15" + ], + "suppressed": false, + "type": "box", + "width": "6" + }, + { + "corner": [ + 102.8, + 14.0, + 2.4 + ], + "height": "11.7", + "id": 8, + "length": "1.6", + "name": "Rib1", + "operation": "join", + "param_names": [ + "d16", + "d17", + "d18" + ], + "suppressed": false, + "type": "box", + "width": "6" + }, + { + "corner": [ + 102.8, + 48.0, + 2.4 + ], + "height": "11.7", + "id": 9, + "length": "1.6", + "name": "Rib2", + "operation": "join", + "param_names": [ + "d19", + "d20", + "d21" + ], + "suppressed": false, + "type": "box", + "width": "6" + }, + { + "corner": [ + 172.4, + 14.0, + 2.4 + ], + "height": "11.7", + "id": 10, + "length": "1.6", + "name": "Rib3", + "operation": "join", + "param_names": [ + "d22", + "d23", + "d24" + ], + "suppressed": false, + "type": "box", + "width": "6" + }, + { + "corner": [ + 172.4, + 48.0, + 2.4 + ], + "height": "11.7", + "id": 11, + "length": "1.6", + "name": "Rib4", + "operation": "join", + "param_names": [ + "d25", + "d26", + "d27" + ], + "suppressed": false, + "type": "box", + "width": "6" + } + ], + "format": "solidcraft/15", + "name": "issue-616-lid-ribs", + "next_id": 12, + "params": [], + "units": "mm" +} \ No newline at end of file diff --git a/apps/solidcraft-cli/tests/design_corpus.rs b/apps/solidcraft-cli/tests/design_corpus.rs index 9e57b768..1edaadc5 100644 --- a/apps/solidcraft-cli/tests/design_corpus.rs +++ b/apps/solidcraft-cli/tests/design_corpus.rs @@ -137,11 +137,12 @@ fn body_names_saved_before_exact_history_keep_their_pieces() { /// ignored test below that checks its faces; a listed plane whose face attaches fails the main /// check until it is removed from here. type Planes = &'static [([f64; 3], f64)]; -const NO_ATTACHMENT_YET: [(&str, &str, Option, u32); 4] = [ +const NO_ATTACHMENT_YET: [(&str, &str, Option, u32); 5] = [ ("m0-revolve-primitives", "Body1", None, 494), ("naming-split-pieces", "Body1", None, 494), ("modern-plastic", "Body1", None, 494), ("modern-sheet-metal", "Body1", None, 495), + ("issue-616-lid-ribs", "Lid", None, 628), ]; /// The issue a face is listed under, and whether it is listed by its plane (not as every face). @@ -273,6 +274,12 @@ fn sheet_metal_thickness_faces_take_an_attached_sketch() { known_faces_attach(495); } +#[test] +#[ignore = "fails until #628 is fixed"] +fn faces_of_a_lid_with_joined_ribs_take_an_attached_sketch() { + known_faces_attach(628); +} + /// A small deterministic generator (xorshift). struct Rng(u64); impl Rng { diff --git a/crates/engine/src/boolean_regression_tests.rs b/crates/engine/src/boolean_regression_tests.rs index 3526ed89..2667b295 100644 --- a/crates/engine/src/boolean_regression_tests.rs +++ b/crates/engine/src/boolean_regression_tests.rs @@ -118,3 +118,38 @@ fn ring_cut_below_a_recess_keeps_the_part_in_one_body() { let got = volume(&s, "Frame"); assert!((got - want).abs() < 1e-9 * want, "{got} vs {want}"); } + +/// Four ribs joined on a rounded lid, standing on the plate over the skirt and flush with the +/// window's sides: each adds exactly its own material and nothing changes away from it. The +/// joins displaced the tool and one left a slit through the far side of the skirt and plate +/// (#616), which no volume shows: the area does. The reply of each join carries its warning, as +/// the timeline does (an agent placing parts over MCP saw none). +#[test] +fn ribs_flush_with_a_window_change_nothing_away_from_them() { + let script: Value = serde_json::from_str(include_str!("../../../examples/regressions/lid_ribs_flush_with_window.json")).unwrap(); + let steps = script.get("commands").and_then(Value::as_array).unwrap(); + let (lid, ribs) = steps.split_at(steps.len() - 4); + let mut s = Session::default(); + s.run_script(&Value::Array(lid.to_vec())).unwrap(); + let measure = |s: &Session| s.model.state().body("Lid").map(|b| solidcraft_kernel::measure(&b.body).unwrap()).unwrap(); + let (v0, a0) = (measure(&s).volume, measure(&s).area); + assert!((v0 - 6703.5222).abs() < 1e-3, "{v0}"); + for (i, rib) in ribs.iter().enumerate() { + let reply = s.execute(rib["command"].as_str().unwrap(), &rib["params"]).unwrap(); + let id = reply["feature"].as_u64().unwrap(); + let result = s.model.result(id).unwrap(); + assert_eq!(reply["warning"], json!(result.warning), "the reply says what the timeline says"); + assert!(!result.warning.as_deref().unwrap_or_default().contains("tool displacement"), "{:?}", result.warning); + let m = measure(&s); + let n = (i + 1) as f64; + // 1.6 x 6 x 11.7 less the 1.35 x 6 x 3 inside the skirt. + assert!((m.volume - (v0 + n * 88.02)).abs() < 1e-6, "rib {}: {}", i + 1, m.volume - v0); + // Top 9.6, outer side 6 x 8.7 above the skirt, inner side 6 x 11.7 (flush with the + // window's side), ends 2 x (1.6 x 11.7 - 1.35 x 3); the skirt top 8.1, its inner side + // 18 and 1.5 of the plate go under the rib. + let area = 9.6 + 52.2 + 70.2 + 29.34 - 8.1 - 18.0 - 1.5; + assert!((m.area - (a0 + n * area)).abs() < 1e-6, "rib {}: area {} (no slit adds any)", i + 1, m.area - a0); + } + let bounds = s.model.state().body("Lid").unwrap().body.tessellate(0.01).unwrap().bounds(); + assert!((bounds.max.z - 14.1).abs() < 1e-9, "{}", bounds.max.z); +} diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index 72af07f5..b19a3b39 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -483,8 +483,11 @@ fn add_feature_in(s: &mut Session, p: &Value, kind: FeatureKind, world: bool) -> st.bodies.iter().filter(|b| b.feature != id && old.get(b.name.as_str()).is_some_and(|o| !o.same(&b.body))).map(|b| b.name.as_str()).collect(); let removed: Vec<&str> = before.bodies.iter().map(|b| b.name.as_str()).filter(|n| !now.contains(n)).collect(); let fname = s.doc.feature(id).map(|f| f.name.clone()).unwrap_or_default(); + // A warning (a Boolean that needed a retry, a reference found again elsewhere) is part of + // the reply: a script or an agent sees it without asking the timeline (#616). + let warning = r.as_ref().and_then(|r| r.warning.clone()); Ok(json!({"feature": id, "name": fname, "bodies": bodies, "created_bodies": created, "changed_bodies": changed, - "removed_bodies": removed, "ms": r.map(|r| r.ms)})) + "removed_bodies": removed, "warning": warning, "ms": r.map(|r| r.ms)})) } fn extrude(s: &mut Session, p: &Value) -> Result { diff --git a/crates/kernel/src/boolean_guard.rs b/crates/kernel/src/boolean_guard.rs new file mode 100644 index 00000000..ddb04914 --- /dev/null +++ b/crates/kernel/src/boolean_guard.rs @@ -0,0 +1,183 @@ +//! The check every Boolean retry's result must pass before it is returned (issue #616). +//! +//! A retry moves things about (the tool nudged, faces pushed apart, pieces cut off and glued +//! back), so its result is held to what a join or a cut can do at all: +//! - it is closed; +//! - a join never loses volume and a cut never gains any; +//! - outside the tool's box nothing changes: there, the result's surface is the target's and +//! the target's surface is the result's. A join or a cut can only change the target where the +//! tool is, so geometry anywhere else (a slit through a wall, a face moved) is wrong. +//! +//! A result that fails is refused with an error: a wrong result that looks right is worse. + +use solidcraft_geom::{Aabb3, Mesh, Vec3}; + +use crate::body::Body; +use crate::{BoolOp, KernelError, Result}; + +/// Most triangles sampled from each surface (evenly spaced through the mesh). +const MAX_SAMPLES: usize = 20_000; + +/// Distance from `p` to the triangle `abc`. +fn point_triangle_dist(p: Vec3, a: Vec3, b: Vec3, c: Vec3) -> f64 { + let n = (b - a).cross(c - a); + let Some(nn) = n.normalized() else { + return [(a, b), (b, c), (c, a)].iter().map(|(u, v)| p.dist_to_segment(*u, *v)).fold(f64::INFINITY, f64::min); + }; + let h = (p - a).dot(nn); + let q = p - nn * h; + let inside = [(a, b), (b, c), (c, a)].iter().all(|(u, v)| (*v - *u).cross(q - *u).dot(n) >= 0.0); + if inside { h.abs() } else { [(a, b), (b, c), (c, a)].iter().map(|(u, v)| p.dist_to_segment(*u, *v)).fold(f64::INFINITY, f64::min) } +} + +/// The triangles of a mesh in a uniform grid of cells, each listing the triangles within `reach` +/// of it, to answer "is this point within `reach` of the surface?". +struct Near<'a> { + mesh: &'a Mesh, + min: Vec3, + cell: f64, + dims: [usize; 3], + cells: Vec>, + reach: f64, +} + +impl<'a> Near<'a> { + fn new(mesh: &'a Mesh, reach: f64) -> Option { + let bounds = mesh.bounds(); + if bounds.is_empty() { + return None; + } + let min = bounds.min - Vec3::new(reach, reach, reach); + let size = bounds.size() + Vec3::new(2.0 * reach, 2.0 * reach, 2.0 * reach); + // About a few triangles a cell, at most 64^3 cells. + let n = (mesh.triangles.len().max(1) as f64).cbrt().clamp(1.0, 64.0); + let cell = (size.x.max(size.y).max(size.z) / n).max(reach * 2.0).max(1e-9); + let dims = [size.x, size.y, size.z].map(|s| ((s / cell).ceil() as usize).clamp(1, 256)); + let mut cells = vec![Vec::new(); dims[0] * dims[1] * dims[2]]; + for (i, t) in mesh.triangles.iter().enumerate() { + let Some(tri) = mesh.tri(t) else { continue }; + let mut bx = Aabb3::EMPTY; + for p in tri { + bx.add(p); + } + let lo = Self::index(min, cell, dims, bx.min - Vec3::new(reach, reach, reach)); + let hi = Self::index(min, cell, dims, bx.max + Vec3::new(reach, reach, reach)); + for x in lo[0]..=hi[0] { + for y in lo[1]..=hi[1] { + for z in lo[2]..=hi[2] { + if let Some(c) = cells.get_mut((x * dims[1] + y) * dims[2] + z) { + c.push(i as u32); + } + } + } + } + } + Some(Near { mesh, min, cell, dims, cells, reach }) + } + + fn index(min: Vec3, cell: f64, dims: [usize; 3], p: Vec3) -> [usize; 3] { + let d = p - min; + let f = |v: f64, n: usize| ((v / cell).floor().max(0.0) as usize).min(n - 1); + [f(d.x, dims[0]), f(d.y, dims[1]), f(d.z, dims[2])] + } + + fn close(&self, p: Vec3) -> bool { + let d = p - self.min; + let s = [d.x, d.y, d.z]; + if s.iter().zip(self.dims).any(|(v, n)| *v < 0.0 || *v > self.cell * n as f64) { + return false; + } + let [x, y, z] = Self::index(self.min, self.cell, self.dims, p); + let Some(list) = self.cells.get((x * self.dims[1] + y) * self.dims[2] + z) else { return false }; + list.iter().any(|&t| { + self.mesh.triangles.get(t as usize).and_then(|t| self.mesh.tri(t)).is_some_and(|[a, b, c]| point_triangle_dist(p, a, b, c) <= self.reach) + }) + } +} + +/// A point of `from`'s surface outside `region` that is not on `onto`'s surface (within +/// `reach`), if there is one. +fn stray_point(from: &Mesh, onto: &Near, region: &Aabb3) -> Option { + let step = from.triangles.len().div_ceil(MAX_SAMPLES).max(1); + from.triangles.iter().step_by(step).filter_map(|t| from.tri(t)).find_map(|[a, b, c]| { + let p = (a + b + c) / 3.0; + let outside = + p.x < region.min.x || p.y < region.min.y || p.z < region.min.z || p.x > region.max.x || p.y > region.max.y || p.z > region.max.z; + (outside && !onto.close(p)).then_some(p) + }) +} + +fn bad(op: BoolOp, what: String) -> KernelError { + let verb = match op { + BoolOp::Union => "join", + BoolOp::Cut => "cut", + BoolOp::Intersect => "intersect", + }; + KernelError::Failed(format!( + "the {verb} could not be computed reliably: {what}. The result was refused rather than kept wrong; \ + where the bodies only touch (faces flush with each other), overlapping them slightly usually helps" + )) +} + +/// Check the result of a Boolean that needed a retry (see the module docs). `result` is `None` +/// for an empty result. +pub(crate) fn check_retry(a: &Body, b: &Body, op: BoolOp, result: Option<&Body>) -> Result<()> { + if op == BoolOp::Intersect || a.is_mesh() || b.is_mesh() { + return Ok(()); + } + let Some(r) = result else { + // Nothing left: only a cut whose tool covers the whole target (to within the meshes' + // chords: a tool touching a round side of the target). + let chord = a.size().max(b.size()) * 1e-3; + let (Ok(ma), Ok(mb)) = (a.tessellate(chord), b.tessellate(chord)) else { return Ok(()) }; + let (ba, bb) = (ma.bounds(), mb.bounds()); + let tol = 2.0 * chord; + let within = ba.min.x >= bb.min.x - tol + && ba.min.y >= bb.min.y - tol + && ba.min.z >= bb.min.z - tol + && ba.max.x <= bb.max.x + tol + && ba.max.y <= bb.max.y + tol + && ba.max.z <= bb.max.z + tol; + return if op == BoolOp::Cut && within { Ok(()) } else { Err(bad(op, "it came out empty".into())) }; + }; + if r.solid.boundaries().iter().any(|s| s.shell_condition() != truck_modeling::ShellCondition::Closed) { + return Err(bad(op, "the result is not closed".into())); + } + let size = a.size().max(b.size()); + let t = (size * 2e-4).max(1e-5); + let (Ok(ma), Ok(mb), Ok(mr)) = (a.tessellate_with(t, true), b.tessellate_with(t, true), r.tessellate_with(t, true)) else { + return Err(bad(op, "the result could not be checked".into())); + }; + let (va, vb, vr) = (ma.measure().volume, mb.measure().volume, mr.measure().volume); + let slack = 2e-4 * (va.abs() + vb.abs()) + 1e-9; + let volume_ok = vr.is_finite() + && match op { + BoolOp::Union => vr >= va.max(vb) - slack && vr <= va + vb + slack, + BoolOp::Cut => vr <= va + slack && vr >= va - vb - slack, + BoolOp::Intersect => true, + }; + if !volume_ok { + let what = match op { + BoolOp::Union => format!("the joined body's volume would be {vr:.4} mm³ (the target is {va:.4} mm³, the tool {vb:.4} mm³)"), + _ => format!("the cut body's volume would be {vr:.4} mm³ (the target is {va:.4} mm³, the tool {vb:.4} mm³)"), + }; + return Err(bad(op, what)); + } + // Outside the tool's box (with room for the meshes' chords and a nudge) the result's + // surface and the target's must be the same. + let reach = 3.0 * t + size * 1e-5; + let mut region = mb.bounds(); + region.min -= Vec3::new(reach, reach, reach) * 2.0; + region.max += Vec3::new(reach, reach, reach) * 2.0; + let (Some(near_a), Some(near_r)) = (Near::new(&ma, reach), Near::new(&mr, reach)) else { + return Err(bad(op, "the result could not be checked".into())); + }; + let at = |p: Vec3| format!("({:.3}, {:.3}, {:.3})", p.x, p.y, p.z); + if let Some(p) = stray_point(&mr, &near_a, ®ion) { + return Err(bad(op, format!("it would change the body away from the tool, at {} (outside the tool's region)", at(p)))); + } + if let Some(p) = stray_point(&ma, &near_r, ®ion) { + return Err(bad(op, format!("it would remove part of the body away from the tool, at {} (outside the tool's region)", at(p)))); + } + Ok(()) +} diff --git a/crates/kernel/src/coplanar.rs b/crates/kernel/src/coplanar.rs index 7fb944e4..02a76e45 100644 --- a/crates/kernel/src/coplanar.rs +++ b/crates/kernel/src/coplanar.rs @@ -335,19 +335,16 @@ pub(crate) fn boolean_apart(a: &Body, b: &Body, op: BoolOp) -> Option bool { !points.is_empty() && points.iter().all(|p| [0.05, 0.5, 1.0].iter().all(|k| inside.contains(*p + m * (depth * k)))) } -/// Union of bodies that touch along one identical planar face, back to back (a body joined -/// with its mirror image): the two faces go and the shells are stitched along their edges. -/// `None` when no such pair of faces exists. +/// Union of bodies that touch along identical planar faces, back to back (a body joined with +/// its mirror image, or the pieces of a body cut apart by a plane, which may meet in several +/// faces): every such pair of faces goes and the shells are stitched along their edges. `None` +/// when no such pair of faces exists. pub(crate) fn glue(a: &Body, b: &Body) -> Option> { use mt::{BoundedCurve, ParametricCurve}; let size = a.size().max(b.size()); @@ -484,13 +482,18 @@ pub(crate) fn glue(a: &Body, b: &Body) -> Option> { let (t0, t1) = c.range_tuple(); from_p3(c.subs((t0 + t1) * 0.5)) }; - // Matching faces: same plane, opposite normals, the same edges. + // Matching faces: same plane, opposite normals, the same edges. Every pair is glued: a + // pair left behind would stay inside the result as a zero-thickness slit (issue #616). let (pa, pb) = (planes(a), planes(b)); let fa_list: Vec<&mt::Face> = a.solid.face_iter().collect(); let fb_list: Vec<&mt::Face> = b.solid.face_iter().collect(); + // B edge → the matching A edge; B vertex → the matching A vertex. + let mut map: HashMap = HashMap::new(); + let mut vmap: HashMap = HashMap::new(); + let (mut glued_a, mut glued_b): (Vec, Vec) = (Vec::new(), Vec::new()); for (ia, x) in &pa { for (ib, y) in &pb { - if x.n.dot(y.n) > -1.0 + 1e-9 || (x.d + y.d).abs() > tol { + if glued_a.contains(ia) || glued_b.contains(ib) || x.n.dot(y.n) > -1.0 + 1e-9 || (x.d + y.d).abs() > tol { continue; } let (Some(fa), Some(fb)) = (fa_list.get(*ia), fb_list.get(*ib)) else { continue }; @@ -499,9 +502,7 @@ pub(crate) fn glue(a: &Body, b: &Body) -> Option> { if ea.len() != eb.len() { continue; } - // B edge → the matching A edge. - let mut map: HashMap = HashMap::new(); - let mut vmap: HashMap = HashMap::new(); + let (mut pair_map, mut pair_vmap) = (Vec::new(), Vec::new()); let mut ok = true; for e in &eb { let ends = (key(from_p3(e.absolute_front().point())), key(from_p3(e.absolute_back().point()))); @@ -515,75 +516,81 @@ pub(crate) fn glue(a: &Body, b: &Body) -> Option> { break; }; let same = key(from_p3(f.absolute_front().point())) == ends.0; - map.insert(e.id(), if same { f.absolute_clone() } else { f.absolute_clone().inverse() }); + pair_map.push((e.id(), if same { f.absolute_clone() } else { f.absolute_clone().inverse() })); let (fa_v, fb_v) = if same { (f.absolute_front().clone(), f.absolute_back().clone()) } else { (f.absolute_back().clone(), f.absolute_front().clone()) }; - vmap.insert(e.absolute_front().id(), fa_v); - vmap.insert(e.absolute_back().id(), fb_v); + pair_vmap.push((e.absolute_front().id(), fa_v)); + pair_vmap.push((e.absolute_back().id(), fb_v)); } if !ok { continue; } - return Some(guard("glue", || { - let fail = |m: &str| KernelError::Failed(format!("joining touching faces: {m}")); - // B's other edges, rebuilt on A's vertices where they meet the seam. - let mut edges: HashMap = map.clone(); - for e in b.solid.edge_iter() { - if edges.contains_key(&e.id()) { - continue; - } - let (f0, f1) = (e.absolute_front(), e.absolute_back()); - if !vmap.contains_key(&f0.id()) && !vmap.contains_key(&f1.id()) { - edges.insert(e.id(), e.absolute_clone()); - continue; - } - let v0 = vmap.get(&f0.id()).cloned().unwrap_or_else(|| f0.clone()); - let v1 = vmap.get(&f1.id()).cloned().unwrap_or_else(|| f1.clone()); - edges.insert(e.id(), mt::Edge::new(&v0, &v1, e.curve())); - } - let mut faces: Vec = fa_list.iter().enumerate().filter(|(i, _)| i != ia).map(|(_, f)| (*f).clone()).collect(); - // Every face but the two glued is kept; an edge along the seam is an edge of both. - let kept = |operand: usize, n: usize, skip: usize| { - (0..n).filter(move |i| *i != skip).map(move |i| record::from_face(operand, i, Change::Kept)) - }; - let from: Vec = kept(0, fa_list.len(), *ia).chain(kept(1, fb_list.len(), *ib)).collect(); - let mut origins = record::edge_origins(&a.solid, 0); - for (old, o) in record::edge_origins(&b.solid, 1) { - if let Some(new) = edges.get(&old) { - origins.entry(new.id()).or_default().extend(o); - } - } - for (i, f) in fb_list.iter().enumerate() { - if i == *ib { - continue; - } - let wires: Vec = f - .absolute_boundaries() - .iter() - .map(|w| { - w.edge_iter() - .filter_map(|e| edges.get(&e.id()).map(|ne| if e.front() == e.absolute_front() { ne.clone() } else { ne.inverse() })) - .collect::>() - .into() - }) - .collect(); - let mut nf = mt::Face::try_new(wires, f.surface()).map_err(|e| fail(&e.to_string()))?; - if !f.orientation() { - nf.invert(); - } - faces.push(nf); - } - let solid = Solid::try_new(vec![faces.into()]).map_err(|e| fail(&e.to_string()))?; - // Each edge's origins in the solid's edge order. - let origins = crate::body::edge_order(solid.face_iter()).0.iter().map(|e| origins.get(&e.id()).cloned()).collect(); - crate::boolean_record::heal_build(solid, size, &[], Some(from), origins, &record::counts(&[a, b])) - })); + map.extend(pair_map); + vmap.extend(pair_vmap); + glued_a.push(*ia); + glued_b.push(*ib); } } - None + if glued_a.is_empty() { + return None; + } + Some(guard("glue", || { + let fail = |m: &str| KernelError::Failed(format!("joining touching faces: {m}")); + // B's other edges, rebuilt on A's vertices where they meet a seam. + let mut edges: HashMap = map.clone(); + for e in b.solid.edge_iter() { + if edges.contains_key(&e.id()) { + continue; + } + let (f0, f1) = (e.absolute_front(), e.absolute_back()); + if !vmap.contains_key(&f0.id()) && !vmap.contains_key(&f1.id()) { + edges.insert(e.id(), e.absolute_clone()); + continue; + } + let v0 = vmap.get(&f0.id()).cloned().unwrap_or_else(|| f0.clone()); + let v1 = vmap.get(&f1.id()).cloned().unwrap_or_else(|| f1.clone()); + edges.insert(e.id(), mt::Edge::new(&v0, &v1, e.curve())); + } + let mut faces: Vec = fa_list.iter().enumerate().filter(|(i, _)| !glued_a.contains(i)).map(|(_, f)| (*f).clone()).collect(); + // Every face but the glued ones is kept; an edge along a seam is an edge of both. + let kept = |operand: usize, n: usize, skip: &[usize]| { + (0..n).filter(|i| !skip.contains(i)).map(move |i| record::from_face(operand, i, Change::Kept)).collect::>() + }; + let from: Vec = kept(0, fa_list.len(), &glued_a).into_iter().chain(kept(1, fb_list.len(), &glued_b)).collect(); + let mut origins = record::edge_origins(&a.solid, 0); + for (old, o) in record::edge_origins(&b.solid, 1) { + if let Some(new) = edges.get(&old) { + origins.entry(new.id()).or_default().extend(o); + } + } + for (i, f) in fb_list.iter().enumerate() { + if glued_b.contains(&i) { + continue; + } + let wires: Vec = f + .absolute_boundaries() + .iter() + .map(|w| { + w.edge_iter() + .filter_map(|e| edges.get(&e.id()).map(|ne| if e.front() == e.absolute_front() { ne.clone() } else { ne.inverse() })) + .collect::>() + .into() + }) + .collect(); + let mut nf = mt::Face::try_new(wires, f.surface()).map_err(|e| fail(&e.to_string()))?; + if !f.orientation() { + nf.invert(); + } + faces.push(nf); + } + let solid = Solid::try_new(vec![faces.into()]).map_err(|e| fail(&e.to_string()))?; + // Each edge's origins in the solid's edge order. + let origins = crate::body::edge_order(solid.face_iter()).0.iter().map(|e| origins.get(&e.id()).cloned()).collect(); + crate::boolean_record::heal_build(solid, size, &[], Some(from), origins, &record::counts(&[a, b])) + })) } /// Union of bodies touching back to back where a planar face of one lies wholly inside a diff --git a/crates/kernel/src/issue_616_tests.rs b/crates/kernel/src/issue_616_tests.rs new file mode 100644 index 00000000..5d6d9e0b --- /dev/null +++ b/crates/kernel/src/issue_616_tests.rs @@ -0,0 +1,185 @@ +//! Issue #616: small boxes joined flush to the walls and floor of a part never change it away +//! from the box, and a retry that would is refused. + +use solidcraft_geom::{Aabb3, Mesh, Vec3}; + +use crate::*; + +fn bx(c: [f64; 3], s: [f64; 3]) -> Body { + box_solid(Vec3::new(c[0], c[1], c[2]), Vec3::new(c[0] + s[0], c[1] + s[1], c[2] + s[2])).unwrap() +} + +fn volume(b: &Body) -> f64 { + measure(b).unwrap().volume +} + +/// The lid of the issue (public dimensions): a plate with rounded corners, a skirt ring on +/// it, a window and a slot cut through, and a notch cut in the skirt. +fn lid() -> Body { + let plate = bx([100., 0., 0.], [76.8, 89.3, 2.4]); + let corners: Vec = [[100., 0.], [176.8, 0.], [100., 89.3], [176.8, 89.3]].iter().map(|p| Vec3::new(p[0], p[1], 1.2)).collect(); + let plate = fillet(&plate, &corners, 3.0).unwrap(); + let ring = boolean(&bx([102.65, 2.65, 2.4], [71.5, 84., 3.]), &bx([104.15, 4.15, 2.0], [68.5, 81., 4.]), BoolOp::Cut).unwrap().unwrap(); + let mut b = boolean(&plate, &ring, BoolOp::Union).unwrap().unwrap(); + for (c, s) in [([104.4, 5.4, -1.], [68., 61., 8.]), ([116.4, 72., -1.], [44., 10., 8.]), ([127.4, 84., 2.4], [22., 6., 4.])] { + b = boolean(&b, &bx(c, s), BoolOp::Cut).unwrap().unwrap(); + } + b +} + +/// Distance from `p` to the triangle `abc` (brute force, independent of the guard's grid). +fn tri_dist(p: Vec3, [a, b, c]: [Vec3; 3]) -> f64 { + let n = (b - a).cross(c - a); + let Some(u) = n.normalized() else { return f64::INFINITY }; + let h = (p - a).dot(u); + let q = p - u * h; + if [(a, b), (b, c), (c, a)].iter().all(|(x, y)| (*y - *x).cross(q - *x).dot(n) >= 0.0) { + h.abs() + } else { + [(a, b), (b, c), (c, a)].iter().map(|(x, y)| p.dist_to_segment(*x, *y)).fold(f64::INFINITY, f64::min) + } +} + +/// A point of `from`'s surface outside `region` that is not on `onto`'s surface. +fn moved_outside(from: &Mesh, onto: &Mesh, region: &Aabb3, tol: f64) -> Option { + let tris: Vec<[Vec3; 3]> = onto.triangles.iter().filter_map(|t| onto.tri(t)).collect(); + from.triangles.iter().filter_map(|t| from.tri(t)).find_map(|[a, b, c]| { + let p = (a + b + c) / 3.0; + let inside = p.x > region.min.x && p.y > region.min.y && p.z > region.min.z && p.x < region.max.x && p.y < region.max.y && p.z < region.max.z; + (!inside && !tris.iter().any(|t| tri_dist(p, *t) < tol)).then_some(p) + }) +} + +/// The join of `tool` onto `target` left `target` as it was away from the tool: no new surface +/// (a slit, a moved face) and none lost, outside the tool's box. +fn assert_local(target: &Body, tool: &Body, joined: &Body, what: &str) { + let tol = 1e-3; + let (mt, mtool, mj) = (target.tessellate(0.01).unwrap(), tool.tessellate(0.01).unwrap(), joined.tessellate(0.01).unwrap()); + let mut region = mtool.bounds(); + region.min -= Vec3::new(0.01, 0.01, 0.01); + region.max += Vec3::new(0.01, 0.01, 0.01); + assert_eq!(moved_outside(&mj, &mt, ®ion, tol), None, "{what}: new surface away from the tool"); + assert_eq!(moved_outside(&mt, &mj, ®ion, tol), None, "{what}: surface lost away from the tool"); +} + +/// The four ribs of the issue, 1.6 × 6 × 11.7, sit on the plate (z = 2.4), over the skirt, flush +/// with the window's sides. Each join adds exactly what lies outside the skirt, changes nothing +/// else (it used to leave a slit through the far wall), and needs no displacement of the tool. +#[test] +fn ribs_joined_flush_to_the_lid_change_nothing_else() { + let mut b = lid(); + assert!((volume(&b) - 6703.5222).abs() < 1e-3, "{}", volume(&b)); + for c in [[102.8, 14., 2.4], [102.8, 48., 2.4], [172.4, 14., 2.4], [172.4, 48., 2.4]] { + let tool = bx(c, [1.6, 6., 11.7]); + let (r, report) = capture_boolean_diagnostics(|| boolean(&b, &tool, BoolOp::Union)); + let r = r.unwrap().unwrap(); + // 1.6 × 6 × 11.7 less the 1.35 × 6 × 3 inside the skirt. + assert!((volume(&r) - volume(&b) - 88.02).abs() < 1e-6, "{c:?}: {}", volume(&r) - volume(&b)); + assert!(!report.events.iter().any(|e| e.strategy == BooleanStrategy::ToolNudge), "{c:?}: {report:?}"); + assert_eq!(r.solid.boundaries().len(), 1); + assert_local(&b, &tool, &r, &format!("{c:?}")); + b = r; + } + let bounds = b.tessellate(0.01).unwrap().bounds(); + assert!((bounds.max.z - 14.1).abs() < 1e-9, "the ribs keep their height: {}", bounds.max.z); +} + +/// A plane cut through a frame meets it in two faces; gluing the halves back joins both (one +/// left over was the slit of #616). +#[test] +fn glue_joins_every_face_the_pieces_share() { + let frame = boolean(&bx([0., 0., 0.], [40., 30., 5.]), &bx([5., 5., -1.], [30., 20., 7.]), BoolOp::Cut).unwrap().unwrap(); + let low = boolean(&frame, &bx([-1., -1., -1.], [42., 16., 7.]), BoolOp::Intersect).unwrap().unwrap(); + let high = boolean(&frame, &bx([-1., 15., -1.], [42., 16., 7.]), BoolOp::Intersect).unwrap().unwrap(); + let glued = crate::coplanar::glue(&low, &high).unwrap().unwrap(); + assert_eq!(glued.solid.boundaries().len(), 1); + assert!((volume(&glued) - volume(&frame)).abs() < 1e-6); + assert!((measure(&glued).unwrap().area - measure(&frame).unwrap().area).abs() < 1e-6, "no faces left inside"); +} + +/// The guard refuses a result that changed the target away from the tool, or lost volume in a +/// join, and accepts the right one. +#[test] +fn the_retry_guard_refuses_changes_away_from_the_tool() { + let target = bx([0., 0., 0.], [40., 30., 5.]); + let tool = bx([0., 0., 5.], [4., 4., 4.]); + let right = boolean(&target, &tool, BoolOp::Union).unwrap().unwrap(); + assert!(crate::boolean_guard::check_retry(&target, &tool, BoolOp::Union, Some(&right)).is_ok()); + // A notch far from the tool (volume still up: the tool adds more than the notch takes). + let notched = boolean(&right, &bx([30., 10., 4.], [2., 2., 2.]), BoolOp::Cut).unwrap().unwrap(); + let e = crate::boolean_guard::check_retry(&target, &tool, BoolOp::Union, Some(¬ched)).unwrap_err().to_string(); + assert!(e.contains("away from the tool") && e.contains("refused"), "{e}"); + // A join that lost volume. + let e = crate::boolean_guard::check_retry(&target, &tool, BoolOp::Union, Some(&bx([0., 0., 0.], [40., 30., 4.]))).unwrap_err().to_string(); + assert!(e.contains("volume"), "{e}"); + // A cut that comes out empty while the tool covers only part of the target. + assert!(crate::boolean_guard::check_retry(&target, &tool, BoolOp::Cut, None).is_err()); + assert!(crate::boolean_guard::check_retry(&tool, &target, BoolOp::Cut, None).is_err()); + assert!(crate::boolean_guard::check_retry(&tool, &bx([-1., -1., 4.], [6., 6., 6.]), BoolOp::Cut, None).is_ok()); +} + +/// Random small boxes joined flush to the walls and floor of a shelled box with rounded corners +/// (sitting on the floor, against a wall, flush with a wall's top or into its corner), and of +/// the same box with a window through its floor (on the ledge beside the window, flush with its +/// side, as the ribs of #616), never lose volume and never change the part away from the box. A +/// join that fails is an error, never a wrong result. +#[test] +fn boxes_joined_flush_to_a_shelled_box_stay_local() { + let (w, d, h, t) = (60.0, 40.0, 20.0, 2.0); + let outer = bx([0., 0., 0.], [w, d, h]); + let corners: Vec = [[0., 0.], [w, 0.], [0., d], [w, d]].iter().map(|p| Vec3::new(p[0], p[1], h / 2.0)).collect(); + let outer = fillet(&outer, &corners, 3.0).unwrap(); + let shelled = boolean(&outer, &bx([t, t, t], [w - 2. * t, d - 2. * t, h]), BoolOp::Cut).unwrap().unwrap(); + // The window leaves a 1.5 ledge inside the walls. + let (ledge, gap) = (t + 1.5, 6.0); + let framed = boolean(&shelled, &bx([ledge, gap, -1.], [w - 2. * ledge, d - 2. * gap, t + 2.]), BoolOp::Cut).unwrap().unwrap(); + let mut seed = 0x616_u64; + let mut next = |lo: f64, hi: f64| { + seed = seed.wrapping_mul(6364136223846793005).wrapping_add(1442695040888963407); + lo + (hi - lo) * ((seed >> 11) as f64 / (1u64 << 53) as f64) + }; + // Sizes and places on a 0.1 grid, like typed values. + let q = |x: f64| (x * 10.0).round() / 10.0; + let (mut joined, mut refused) = (0, 0); + for (name, target) in [("shelled", &shelled), ("framed", &framed)] { + let base = volume(target); + for case in 0..20 { + let s = [q(next(1.0, 6.0)), q(next(1.0, 6.0)), q(next(1.0, 12.0))]; + let (x, y) = (q(next(t + 1.0, w - t - 8.0)), q(next(gap, d - gap - 8.0))); + let c = match (name, case % 4) { + // On the floor. + ("shelled", 0) => [x, y, t], + // On the floor against the -x wall. + ("shelled", 1) => [t, y, t], + // Into the +y wall, flush with its top, overlapping it. + ("shelled", 2) => [x, d - t - s[1] / 2.0, h - s[2]], + // On the floor in the corner of the -x and -y walls, sunk into both. + ("shelled", _) => [t - 0.5, t - 0.5, t], + // On the ledge over the wall, flush with the window's -x or +x side. + (_, 0) => [ledge - 1.6, y, t], + (_, 1) => [w - ledge, y, t], + // On the ledge against the -x wall, short of the window. + (_, 2) => [t, y, t], + // Into the +x wall from the ledge, flush with the window's side. + _ => [w - ledge, y, t], + }; + let s = if name == "framed" && case % 4 != 2 { [1.6, s[1], s[2]] } else { s }; + let tool = bx(c, s); + let what = format!("{name} case {case}: corner {c:?} size {s:?}"); + match boolean(target, &tool, BoolOp::Union) { + Ok(Some(r)) => { + assert!(volume(&r) >= base - 1e-6, "{what}: the join lost volume ({} < {base})", volume(&r)); + assert!(volume(&r) <= base + volume(&tool) + 1e-6, "{what}"); + assert_local(target, &tool, &r, &what); + joined += 1; + } + Ok(None) => panic!("{what}: a join came out empty"), + Err(e) => { + eprintln!("{what}: refused: {e}"); + refused += 1; + } + } + } + } + assert_eq!((joined, refused), (40, 0), "every one of these joins works"); +} diff --git a/crates/kernel/src/lib.rs b/crates/kernel/src/lib.rs index f88f6ed3..1e0bb115 100644 --- a/crates/kernel/src/lib.rs +++ b/crates/kernel/src/lib.rs @@ -17,6 +17,7 @@ mod blend_corner; mod blend_variable; mod body; mod boolean_diagnostics; +mod boolean_guard; mod boolean_record; mod bore_guard; mod build; @@ -36,6 +37,8 @@ mod iges_in; mod iges_out; #[cfg(test)] mod intersection_curve_tests; +#[cfg(test)] +mod issue_616_tests; mod lip; mod loopblend; mod measure; diff --git a/crates/kernel/src/ops.rs b/crates/kernel/src/ops.rs index 2f6fbf9d..0725e97f 100644 --- a/crates/kernel/src/ops.rs +++ b/crates/kernel/src/ops.rs @@ -286,11 +286,6 @@ thread_local! { static BORES: std::cell::Cell = const { std::cell::Cell::new(false) }; } -/// Is a union with a narrowed bore running (see [`crate::coplanar::bore_union`])? -pub(crate) fn narrowing_bores() -> bool { - BORES.with(|c| c.get()) -} - /// Boolean of two bodies. The result may be empty (`Ok(None)`) for a cut that removes /// everything or an intersection of disjoint bodies. Results are checked against volume bounds /// and retried with shifted copies and other tolerances when they fail or look wrong. @@ -340,6 +335,18 @@ pub fn boolean(a: &Body, b: &Body, op: BoolOp) -> Result> { BoolOp::Union => r = r.and_then(|result| result.map(|body| body.with_preserved_union_planar_sources(a, b)).transpose()), BoolOp::Intersect => {} } + // A retry's result is held to what the operation can do at all; one that fails is refused + // rather than returned wrong (issue #616). + if (!diagnostics.events.is_empty() || diagnostics.truncated) + && 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(); + r = Err(match e { + KernelError::Failed(m) => KernelError::Failed(format!("{m} ({})", tried.trim_start_matches("Boolean retry: "))), + other => other, + }); + } if r.is_ok() { merge(diagnostics); } diff --git a/crates/kernel/src/polybool.rs b/crates/kernel/src/polybool.rs index 35b9d819..32996d35 100644 --- a/crates/kernel/src/polybool.rs +++ b/crates/kernel/src/polybool.rs @@ -504,7 +504,7 @@ fn sliced_planar_boolean(a: &Body, b: &Body, op: BoolOp) -> Option = match r { + let pieces: Vec = match r { Some(m) => { let healed = recorded::heal_body(&m, m.deep_copy(), size, a.split_keep(), &record::counts(&[&middle.0, b]))?; let h = recorded::of_pair(&healed, &middle.1, &h_b); @@ -512,18 +512,51 @@ fn sliced_planar_boolean(a: &Body, b: &Body, op: BoolOp) -> Option Vec::new(), }; - // Each piece outside meets the middle along its cut face (glued one shell to another). - if pieces.iter().chain(&outside).any(|(x, _)| x.solid.boundaries().len() != 1) { + // Each piece in its lumps: a cut plane crossing a frame or a ring leaves the middle (or + // a piece outside) in several parts, which meet the others along several cut faces. + let lumps = |(whole, h): Traced| -> Result> { + if whole.solid.boundaries().len() == 1 { + return Ok(vec![(whole, h)]); + } + let parts = whole.lumps()?; + let histories = recorded::parts_of(&parts, &whole); + Ok(parts + .into_iter() + .zip(histories) + .map(|(p, hp)| { + let h = match (hp, h.as_ref()) { + (Some(hp), Some(h)) => recorded::composed(hp.substituted(0, h)), + _ => None, + }; + (p, h) + }) + .collect()) + }; + let mut all: Vec = Vec::new(); + for piece in pieces.into_iter().chain(outside) { + all.extend(lumps(piece)?); + } + // Glue pieces that meet along their cut faces until one is left; every cut face pair + // goes (a pair left behind would be a slit through the result). + let mut joined: Vec = Vec::new(); + while let Some(mut cur) = all.pop() { + loop { + let found = all.iter().enumerate().find_map(|(i, (o, _))| Some((i, crate::coplanar::glue(&cur.0, o)?.ok()?))); + let Some((i, j)) = found else { break }; + let other = all.swap_remove(i); + let h = recorded::of_pair(&j, &cur.1, &other.1); + cur = (j, h); + } + joined.push(cur); + } + if joined.iter().any(|(x, _)| x.solid.boundaries().len() != 1) { return Err(fail("a piece in several parts")); } - for (o, h_o) in outside { - let joined = pieces.iter().enumerate().find_map(|(i, (m, _))| Some((i, crate::coplanar::glue(m, &o)?.ok()?))); - let (i, j) = joined.ok_or_else(|| fail("a piece does not join back"))?; - if let Some(slot) = pieces.get_mut(i) { - let h = recorded::of_pair(&j, &slot.1, &h_o); - *slot = (j, h); - } + // A join onto one piece is one piece: anything apart is a cut face that did not glue. + if op == BoolOp::Union && a.solid.boundaries().len() == 1 && joined.len() != 1 { + return Err(fail("a piece does not join back")); } + let pieces = joined; recorded::join(pieces.into_iter().map(|(p, h)| (p, dropped(h))).collect(), &counts) }; Some(run()) diff --git a/docs/robustness.md b/docs/robustness.md index 52e9576b..3cb832e3 100644 --- a/docs/robustness.md +++ b/docs/robustness.md @@ -50,6 +50,18 @@ placement and reports both its translation magnitude and rotation angle. 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. + +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 +never loses volume and a cut never gains any, and outside the tool's box the +result's surface is the target's and the target's is the result's (a join or +a cut can only change the target where the tool is). A cut may come out empty +only when the tool covers the target. A result that fails is refused with an +error naming what was wrong and where, rather than returned: a wrong result +that looks right is worse than an error. + Reports belong to the evaluated feature and survive cached rebuilds and undo/redo. Cold reopening regenerates them from the embedded design. Failed or discarded retry branches do not contribute successful-retry events. Reports diff --git a/examples/regressions/lid_ribs_flush_with_window.json b/examples/regressions/lid_ribs_flush_with_window.json new file mode 100644 index 00000000..4b5c6e28 --- /dev/null +++ b/examples/regressions/lid_ribs_flush_with_window.json @@ -0,0 +1,19 @@ +{ + "description": "A 76.8 x 89.3 x 2.4 lid plate (corner at x = 100) with its four vertical edges rounded 3 mm, a 1.5 mm skirt ring 3 mm high on it (outside 102.65..174.15 x 2.65..86.65), a 68 x 61 window and a 44 x 10 slot cut through, and a 22 x 6 notch in the skirt. Four 1.6 x 6 x 11.7 ribs are then joined standing on the plate (z = 2.4) over the skirt, each flush with a side of the window (x = 104.4 and x = 172.4). The joins needed retries that displaced the tool, and one left a zero-thickness slit through the skirt and plate at y = 25.5, far from any rib (#616). Each rib adds 1.6*6*11.7 - 1.35*6*3 = 88.02 mm3 and nothing changes away from it: 6703.5222 + 4 * 88.02 mm3.", + "commands": [ + {"command": "solid.box", "params": {"corner": [100, 0, 0], "length": 76.8, "width": 89.3, "height": 2.4, "body_name": "Lid"}}, + {"command": "solid.fillet", "params": {"edges": [[100, 0, 1.2], [176.8, 0, 1.2], [100, 89.3, 1.2], [176.8, 89.3, 1.2]], "radius": 3}}, + {"command": "sketch.create", "params": {"plane": "XY", "offset": 2.4, "name": "Skirt"}}, + {"command": "sketch.rectangle.two_point", "params": {"p0": [102.65, 2.65], "p1": [174.15, 86.65]}}, + {"command": "sketch.rectangle.two_point", "params": {"p0": [104.15, 4.15], "p1": [172.65, 85.15]}}, + {"command": "sketch.finish", "params": {}}, + {"command": "solid.extrude", "params": {"sketch": "Skirt", "profiles": [{"point": [103.4, 40]}], "distance": 3, "operation": "join", "targets": ["Lid"]}}, + {"command": "solid.box", "params": {"corner": [104.4, 5.4, -1], "length": 68, "width": 61, "height": 8, "operation": "cut"}}, + {"command": "solid.box", "params": {"corner": [116.4, 72, -1], "length": 44, "width": 10, "height": 8, "operation": "cut"}}, + {"command": "solid.box", "params": {"corner": [127.4, 84, 2.4], "length": 22, "width": 6, "height": 4, "operation": "cut"}}, + {"command": "solid.box", "params": {"corner": [102.8, 14, 2.4], "length": 1.6, "width": 6, "height": 11.7, "operation": "join", "name": "Rib1"}}, + {"command": "solid.box", "params": {"corner": [102.8, 48, 2.4], "length": 1.6, "width": 6, "height": 11.7, "operation": "join", "name": "Rib2"}}, + {"command": "solid.box", "params": {"corner": [172.4, 14, 2.4], "length": 1.6, "width": 6, "height": 11.7, "operation": "join", "name": "Rib3"}}, + {"command": "solid.box", "params": {"corner": [172.4, 48, 2.4], "length": 1.6, "width": 6, "height": 11.7, "operation": "join", "name": "Rib4"}} + ] +} diff --git a/tests/action_matrix/baseline.json b/tests/action_matrix/baseline.json index 380a20b4..4f6098ed 100644 --- a/tests/action_matrix/baseline.json +++ b/tests/action_matrix/baseline.json @@ -2311,7 +2311,7 @@ "Lid/F14:side:l4|F15:blend:F14:side:l1|F14:side:l4": "edge.axis edge.axis+edit edge.normal_plane edge.project", "Lid/F14:side:l4|F15:blend:F14:side:l3|F14:side:l4": "edge.axis edge.normal_plane edge.project", "Lid/F14:side:l4|F2:top": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", - "Lid/F14:start": "face.angle_plane face.angle_plane+edit !face.draft_neutral !face.extrude face.hole face.hole+edit face.midplane face.offset_plane !face.press_pull face.shell face.shell+edit face.sketch", + "Lid/F14:start": "face.angle_plane face.angle_plane+edit !face.draft_neutral !face.extrude !face.hole face.hole+edit face.midplane face.offset_plane !face.press_pull face.shell face.shell+edit face.sketch", "Lid/F14:start|F18:side:a17": "edge.chamfer edge.fillet edge.normal_plane edge.project", "Lid/F14:start|F18:side:a18": "edge.chamfer edge.chamfer+edit edge.fillet edge.normal_plane edge.project", "Lid/F14:start|F18:side:a19": "edge.chamfer edge.fillet edge.normal_plane edge.project edge.project+edit", @@ -9219,7 +9219,7 @@ "Tool/F2:side:l3|F2:side:l4": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Tool/F2:side:l3|F2:top": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Tool/F2:side:l3|F4:side:l1": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", - "Tool/F2:side:l4": "face.angle_plane !face.draft_neutral face.extrude face.hole face.offset_plane face.press_pull !face.shell face.sketch", + "Tool/F2:side:l4": "face.angle_plane !face.draft_neutral face.extrude !face.hole face.offset_plane face.press_pull !face.shell face.sketch", "Tool/F2:side:l4|F2:top": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Tool/F2:side:l4|F4:side:l1": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Tool/F2:top": "face.angle_plane !face.draft_neutral face.extrude !face.hole face.offset_plane face.press_pull !face.shell face.sketch",