diff --git a/crates/engine/src/tests/robustness.rs b/crates/engine/src/tests/robustness.rs index c4d6e801..7bfcebca 100644 --- a/crates/engine/src/tests/robustness.rs +++ b/crates/engine/src/tests/robustness.rs @@ -54,6 +54,44 @@ fn shell_with_a_hole_through_a_curved_side() { ])); } +/// #871: the fillet from the report (its feature as saved) on a plate with a block on top and +/// a window through both. Radius 1 on the window's edges and on the block's sides overlaps in +/// the 1.26 mm wall between them; the result had openings in its end faces and was accepted. +/// It is refused now, and the part is left as it was. +#[test] +fn fillet_that_would_open_the_surface_is_refused() { + let (a, b) = (12.74437365538526, 22.69275963366936); + let mut s = script(json!([ + {"command": "solid.box", "params": {"length": 14, "width": 21.5, "height": 10, "corner": [9.5, 8, 0]}}, + {"command": "solid.box", "params": {"length": 14, "width": b - a, "height": 10, "corner": [9.5, a, 10], "operation": "join", "targets": ["Body1"]}}, + {"command": "solid.box", "params": {"length": 14, "width": 5.2, "height": 7.1, "corner": [9.5, 14, 7], "operation": "cut", "targets": ["Body1"]}} + ])); + let feature = json!({ + "type": "fillet", + "edges": [ + [23.5, 12.74437365538526, 15.0], [23.5, 10.37218682769263, 10.0], [23.5, 8.0, 5.0], [23.5, 18.75, 0.0], + [23.5, 29.5, 5.0], [23.5, 26.09637981683468, 10.0], [23.5, 22.69275963366936, 15.0], [23.5, 17.71856664452731, 20.0], + [23.5, 16.6, 7.0], [23.5, 14.0, 10.55], [23.5, 16.6, 14.100000000000001], [23.5, 19.200000000000003, 10.55], + [9.5, 17.71856664452731, 20.0], [9.5, 22.69275963366936, 15.0], [9.5, 26.09637981683468, 10.0], [9.5, 29.5, 5.0], + [9.5, 18.75, 0.0], [9.5, 8.0, 5.0], [9.5, 10.37218682769263, 10.0], [9.5, 12.74437365538526, 15.0], + [9.5, 19.200000000000003, 10.55], [9.5, 16.6, 14.100000000000001], [9.5, 14.0, 10.55], [9.5, 16.6, 7.0] + ], + "radius": "1" + }); + let before = total_volume(&s); + let result = s.execute("solid.fillet", &json!({"edges": feature["edges"], "radius": feature["radius"]})); + assert!(result.is_err(), "a fillet with openings in the model was accepted"); + assert!((total_volume(&s) - before).abs() < 1e-9); + for body in &s.model.state().bodies { + assert!(body.body.validity().is_empty() && body.body.crossed_face_loops().unwrap() == 0); + } + // At half the radius the rounds fit. + run(&mut s, "solid.fillet", json!({"edges": feature["edges"], "radius": "0.5"})); + for body in &s.model.state().bodies { + assert!(body.body.validity().is_empty() && body.body.crossed_face_loops().unwrap() == 0); + } +} + /// Shell of a body in two pieces (a cylinder joined clear of a ring): each piece is shelled. #[test] fn shell_of_a_body_in_two_pieces() { diff --git a/crates/kernel/src/blend.rs b/crates/kernel/src/blend.rs index cd2d49f9..29c50d81 100644 --- a/crates/kernel/src/blend.rs +++ b/crates/kernel/src/blend.rs @@ -1623,23 +1623,39 @@ fn loop_split(cur: &Body, pts: &[Vec3]) -> Result>, Vec Result { + match b.crossed_face_loops()? { + 0 => Ok(b), + _ => Err(KernelError::Failed( + "fillet: the rounds would leave openings in the surface (they overlap where a wall or a face is narrower than both)".into(), + )), + } +} + /// Constant-radius fillet of the edges nearest to the given points. A radius too large for the /// faces around the edges is refused with [`KernelError::TooLarge`] ([`fillet_max`] finds the /// largest that works). pub fn fillet(body: &Body, edges: &[Vec3], radius: f64) -> Result { body.require_brep("fillet")?; // Corners the local blends refuse (an earlier round at an edge's end): radius per edge. - blend(body, edges, radius, Shape::Round, "fillet").or_else(|e| { + blend(body, edges, radius, Shape::Round, "fillet").and_then(closed).or_else(|e| { let sel: Vec<(Vec3, f64)> = edges.iter().map(|p| (*p, radius)).collect(); // The local blend's refusal, unless only the corner build found the radius too large. crate::blend_corner::fillet_radii(body, &sel) + .and_then(closed) .map_err(|e2| match (&e, e2) { (KernelError::TooLarge(_), _) => e, (_, e2 @ KernelError::TooLarge(_)) => e2, _ => e, }) // Three edges at a vertex of any faces end in a sphere. - .or_else(|e| if edges.len() >= 3 { crate::blend_vertex::fillet_ball_vertices(body, edges, radius).map_err(|_| e) } else { Err(e) }) + .or_else(|e| { + if edges.len() >= 3 { crate::blend_vertex::fillet_ball_vertices(body, edges, radius).and_then(closed).map_err(|_| e) } else { Err(e) } + }) }) } @@ -1726,7 +1742,7 @@ pub fn fillet_each(body: &Body, edges: &[Vec3], radii: &[f64]) -> Result { return fillet(body, edges, *r); } let sel: Vec<(Vec3, f64)> = edges.iter().copied().zip(radii.iter().copied()).collect(); - if let Ok(b) = crate::blend_corner::fillet_radii(body, &sel) { + if let Ok(b) = crate::blend_corner::fillet_radii(body, &sel).and_then(closed) { return Ok(b); } let mut by_radius: Vec<(f64, Vec)> = Vec::new(); diff --git a/crates/kernel/src/body.rs b/crates/kernel/src/body.rs index 669feeca..0fe66742 100644 --- a/crates/kernel/src/body.rs +++ b/crates/kernel/src/body.rs @@ -1090,6 +1090,79 @@ impl Body { guard("tessellate", || crate::tess::plan(&self.solid, tol, turn).map(|(plan, _)| plan)) } + /// Planar faces whose boundary crosses itself: two of its loops crossing each other, or a + /// loop crossing itself. Such a face has no inside, and its mesh covers what it shouldn't + /// and leaves openings beside it, while the shell is still closed and every face meshes + /// (rounds that overlap where a wall is thinner than both, #871). Boundaries that only + /// touch (at a vertex they share, or within the size's millionth) don't count. + pub fn crossed_face_loops(&self) -> Result { + use mt::{BoundedCurve, ParameterDivision1D}; + if self.is_mesh() { + return Ok(0); + } + const MAX_SEGMENTS: usize = 20_000; + let size = self.size(); + let tol = (size * 1e-6).max(1e-9); + let chord = (size * 1e-3).max(1e-4); + let mut crossed = 0; + for f in self.solid.face_iter() { + let mt::Surface::Plane(pl) = f.oriented_surface() else { continue }; + let o = from_p3(pl.origin()); + let (Some(u), Some(n)) = ( + Vec3::new(pl.u_axis().x, pl.u_axis().y, pl.u_axis().z).normalized(), + Vec3::new(pl.normal().x, pl.normal().y, pl.normal().z).normalized(), + ) else { + continue; + }; + let v = n.cross(u); + let flat = |p: mt::Point3| { + let d = from_p3(p) - o; + (d.dot(u), d.dot(v)) + }; + // Each loop's segments, in the face's plane. + let mut loops: Vec> = Vec::new(); + let mut total = 0usize; + for w in f.boundaries() { + let mut segs = Vec::new(); + for e in w.edge_iter() { + let pts = guard("face loops", || { + Ok(match e.curve() { + mt::Curve::Line(_) => vec![e.front().point(), e.back().point()], + c => c.parameter_division(c.range_tuple(), chord).1, + }) + })?; + segs.extend(pts.windows(2).filter_map(|q| Some((flat(*q.first()?), flat(*q.get(1)?))))); + } + total = total.saturating_add(segs.len()); + if total > MAX_SEGMENTS { + break; + } + loops.push(segs); + } + if total > MAX_SEGMENTS { + continue; + } + // Proper crossings only: each segment's ends strictly on either side of the other. + let side = |a: (f64, f64), b: (f64, f64), p: (f64, f64)| { + let (dx, dy) = (b.0 - a.0, b.1 - a.1); + let len = (dx * dx + dy * dy).sqrt(); + if len <= tol { 0.0 } else { (dx * (p.1 - a.1) - dy * (p.0 - a.0)) / len } + }; + let cross = |s: &((f64, f64), (f64, f64)), t: &((f64, f64), (f64, f64))| { + let (d1, d2) = (side(s.0, s.1, t.0), side(s.0, s.1, t.1)); + let (d3, d4) = (side(t.0, t.1, s.0), side(t.0, t.1, s.1)); + ((d1 > tol && d2 < -tol) || (d1 < -tol && d2 > tol)) && ((d3 > tol && d4 < -tol) || (d3 < -tol && d4 > tol)) + }; + let all: Vec<(usize, &((f64, f64), (f64, f64)))> = + loops.iter().enumerate().flat_map(|(l, segs)| segs.iter().map(move |s| (l, s))).collect(); + let bad = all.iter().enumerate().any(|(i, (_, s))| all.iter().skip(i + 1).any(|(_, t)| cross(s, t))); + if bad { + crossed += 1; + } + } + Ok(crossed) + } + /// Faces with no triangles in a coarse tessellation (the mesher failed or gave up). pub fn unmeshed_faces(&self) -> usize { if self.is_mesh() { diff --git a/crates/kernel/src/issue_871_tests.rs b/crates/kernel/src/issue_871_tests.rs new file mode 100644 index 00000000..a9430c33 --- /dev/null +++ b/crates/kernel/src/issue_871_tests.rs @@ -0,0 +1,60 @@ +//! #871: a fillet that opened holes in the model must be refused, not accepted. + +use solidcraft_geom::Vec3; + +use crate::{Body, BoolOp, build::box_solid, ops::boolean}; + +/// The part from #871: a plate with a narrower block on top and a window through both, filleted +/// at radius 1 along every edge of both end faces. +fn part() -> Body { + let (a, b) = (12.74437365538526, 22.69275963366936); + let base = box_solid(Vec3::new(9.5, 8.0, 0.0), Vec3::new(23.5, 29.5, 10.0)).unwrap(); + let top = box_solid(Vec3::new(9.5, a, 10.0), Vec3::new(23.5, b, 20.0)).unwrap(); + let joined = boolean(&base, &top, BoolOp::Union).unwrap().unwrap(); + let window = box_solid(Vec3::new(9.5, 14.0, 7.0), Vec3::new(23.5, 19.2, 14.1)).unwrap(); + boolean(&joined, &window, BoolOp::Cut).unwrap().unwrap() +} + +fn edges() -> Vec { + let yz = [ + (12.74437365538526, 15.0), + (10.37218682769263, 10.0), + (8.0, 5.0), + (18.75, 0.0), + (29.5, 5.0), + (26.09637981683468, 10.0), + (22.69275963366936, 15.0), + (17.71856664452731, 20.0), + (16.6, 7.0), + (14.0, 10.55), + (16.6, 14.100000000000001), + (19.200000000000003, 10.55), + ]; + [23.5, 9.5].iter().flat_map(|x| yz.iter().map(move |(y, z)| Vec3::new(*x, *y, *z))).collect() +} + +/// Radius 1 on both the window's edges and the block's sides: the wall between them is 1.26 +/// mm thick, so the rounds overlap and the end faces' boundaries cross. That result has +/// openings and is refused. +#[test] +fn overlapping_rounds_are_refused() { + let p = part(); + assert_eq!(p.crossed_face_loops().unwrap(), 0); + assert!(crate::fillet(&p, &edges(), 1.0).is_err(), "a fillet with openings in its surface was accepted"); + // The corner build makes that body (what #871 got): closed as topology, every face meshes, + // but the loops of its end faces cross. + let sel: Vec<(Vec3, f64)> = edges().into_iter().map(|e| (e, 1.0)).collect(); + if let Ok(b) = crate::blend_corner::fillet_radii(&p, &sel) { + assert!(b.validity().is_empty() && b.crossed_face_loops().unwrap() > 0); + } +} + +/// Below half the wall the rounds fit: a closed solid with a round on every edge. +#[test] +fn rounds_that_fit_are_closed() { + let p = part(); + let b = crate::fillet(&p, &edges(), 0.5).unwrap(); + assert!(b.validity().is_empty(), "{:?}", b.validity()); + assert_eq!(b.crossed_face_loops().unwrap(), 0); + assert_eq!(b.face_count(), p.face_count() + edges().len()); +} diff --git a/crates/kernel/src/lib.rs b/crates/kernel/src/lib.rs index 89d8d993..2258ea2b 100644 --- a/crates/kernel/src/lib.rs +++ b/crates/kernel/src/lib.rs @@ -47,6 +47,8 @@ mod issue_674_tests; mod issue_679_tests; #[cfg(test)] mod issue_851_tests; +#[cfg(test)] +mod issue_871_tests; mod lip; mod loopblend; mod measure; diff --git a/tests/action_matrix/baseline.json b/tests/action_matrix/baseline.json index 272e459e..9add915d 100644 --- a/tests/action_matrix/baseline.json +++ b/tests/action_matrix/baseline.json @@ -3666,20 +3666,20 @@ "Box/F2:side:l1|F2:side:l4~1": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Box/F2:side:l1|F2:side:l4~2": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Box/F2:side:l1|F2:start": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", - "Box/F2:side:l1|F2:top": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", + "Box/F2:side:l1|F2:top": "edge.axis edge.chamfer !edge.fillet edge.normal_plane edge.project", "Box/F2:side:l2": "face.angle_plane face.draft_neutral face.extrude face.hole face.midplane face.offset_plane face.press_pull face.shell face.sketch", "Box/F2:side:l2|F2:side:l3~1": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Box/F2:side:l2|F2:side:l3~2": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Box/F2:side:l2|F2:start": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", - "Box/F2:side:l2|F2:top": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", + "Box/F2:side:l2|F2:top": "edge.axis edge.chamfer !edge.fillet edge.normal_plane edge.project", "Box/F2:side:l3": "face.angle_plane face.draft_neutral face.extrude face.hole face.midplane face.offset_plane face.press_pull face.shell face.sketch", "Box/F2:side:l3|F2:side:l4~1": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Box/F2:side:l3|F2:side:l4~2": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", "Box/F2:side:l3|F2:start": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", - "Box/F2:side:l3|F2:top": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", + "Box/F2:side:l3|F2:top": "edge.axis edge.chamfer !edge.fillet edge.normal_plane edge.project", "Box/F2:side:l4": "face.angle_plane face.draft_neutral face.extrude face.hole face.midplane face.offset_plane face.press_pull face.shell face.sketch", "Box/F2:side:l4|F2:start": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", - "Box/F2:side:l4|F2:top": "edge.axis edge.chamfer edge.fillet edge.normal_plane edge.project", + "Box/F2:side:l4|F2:top": "edge.axis edge.chamfer !edge.fillet edge.normal_plane edge.project", "Box/F2:start": "face.angle_plane face.draft_neutral face.extrude face.hole face.midplane face.offset_plane face.press_pull face.shell face.sketch", "Box/F2:top": "face.angle_plane face.draft_neutral face.extrude face.hole face.midplane face.offset_plane face.press_pull face.shell face.sketch", "Box/F2:top|F3:inner:F2:side:l1": "edge.axis edge.chamfer !edge.fillet edge.normal_plane edge.project", @@ -7698,9 +7698,9 @@ }, "oracle/60-pipe-flange": { "Flange/F2:side:l1": "face.extrude face.hole face.midplane face.offset_plane face.press_pull face.shell face.sketch", - "Flange/F2:side:l1|F2:side:l2#1": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Flange/F2:side:l1|F2:side:l2#2": "edge.chamfer edge.fillet edge.normal_plane edge.project", - "Flange/F2:side:l1|F2:side:l2#3": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Flange/F2:side:l1|F2:side:l2#1": "edge.chamfer !edge.fillet edge.normal_plane edge.project", + "Flange/F2:side:l1|F2:side:l2#2": "edge.chamfer !edge.fillet edge.normal_plane edge.project", + "Flange/F2:side:l1|F2:side:l2#3": "edge.chamfer !edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l1|F2:side:l6#1": "edge.chamfer edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l1|F2:side:l6#2": "edge.chamfer edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l1|F2:side:l6#3": "edge.chamfer edge.fillet edge.normal_plane edge.project", @@ -7721,11 +7721,11 @@ "Flange/F2:side:l1|F4:side:c1@F5.7#1": "edge.chamfer edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l1|F4:side:c1@F5.7#2": "edge.chamfer edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l2#1": "curved.axis curved.tangent_plane", - "Flange/F2:side:l2#1|F2:side:l3": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Flange/F2:side:l2#1|F2:side:l3": "edge.chamfer !edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l2#2": "curved.axis curved.tangent_plane", - "Flange/F2:side:l2#2|F2:side:l3": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Flange/F2:side:l2#2|F2:side:l3": "edge.chamfer !edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l2#3": "curved.axis curved.tangent_plane", - "Flange/F2:side:l2#3|F2:side:l3": "edge.chamfer edge.fillet edge.normal_plane edge.project", + "Flange/F2:side:l2#3|F2:side:l3": "edge.chamfer !edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l3": "face.extrude face.hole face.offset_plane face.press_pull !face.shell face.sketch", "Flange/F2:side:l3|F4:side:c1#1": "edge.chamfer edge.fillet edge.normal_plane edge.project", "Flange/F2:side:l3|F4:side:c1#2": "edge.chamfer edge.fillet edge.normal_plane edge.project",