diff --git a/ROADMAP.md b/ROADMAP.md index 90a2c261..82459115 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -307,7 +307,9 @@ annotations and exact projected curves remain deferred. See [view DXF](docs/view is an equivalent circular setback. Curved faces/edges, sloped ends and corner junctions remain unsupported. - Fillets and chamfers: constant, variable-radius and chord-length fillets; a radius per edge, rounds next to earlier rounds of another radius, and smooth corner patches where three rounds - meet (a sphere for equal radii); straight edges (convex or concave) between planar faces with perpendicular + meet (a sphere for equal radii); perpendicular corner fillets after a closed ring of earlier + rounds preserve the ring's radii and the enclosed planar face (#1557); + straight edges (convex or concave) between planar faces with perpendicular planar end faces (equal-distance chamfers also meet sloped planar end faces, including inner basin rims within the rim width); convex fillets also meet sloped planar ends and reflex corners around pocket rims, with unequal wall slopes and extensions confined to the face; diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index 6adb6aeb..8c6944c1 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -1888,3 +1888,6 @@ mod thin_extrude_tests; #[cfg(test)] #[path = "extrude_convention_tests.rs"] mod extrude_convention_tests; +#[cfg(test)] +#[path = "fillet_connected_rounds_tests.rs"] +mod fillet_connected_rounds_tests; diff --git a/crates/engine/src/cmd/fillet_connected_rounds_tests.rs b/crates/engine/src/cmd/fillet_connected_rounds_tests.rs new file mode 100644 index 00000000..5e060298 --- /dev/null +++ b/crates/engine/src/cmd/fillet_connected_rounds_tests.rs @@ -0,0 +1,64 @@ +use serde_json::{Value, json}; + +use crate::Session; + +fn run(s: &mut Session, command: &str, params: Value) -> Value { + s.execute(command, ¶ms).unwrap_or_else(|e| panic!("{command}: {e}")) +} + +fn bottom_rounds() -> Session { + let mut s = Session::default(); + run(&mut s, "parameters.change", json!({"name":"corner_radius", "expression":"2 mm"})); + run(&mut s, "sketch.create", json!({"plane":"XY", "name":"Rectangle"})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0":[0,0], "p1":[25,25]})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"distance":"10 mm", "body_name":"Block"})); + run(&mut s, "solid.fillet", json!({"edges":[[12.5,0,0],[25,12.5,0],[12.5,25,0],[0,12.5,0]], "radius":"2 mm"})); + s +} + +fn volume(s: &mut Session) -> f64 { + let value = run(s, "inspect.measure", json!({})); + assert_eq!(value["body_count"], 1); + let state = s.model.state(); + let body = &state.body("Block").unwrap().body; + assert!(body.validity().is_empty(), "{:?}", body.validity()); + value["total"]["volume_mm3"].as_f64().unwrap() +} + +#[test] +fn issue1557_fillet_recomputes_and_survives_undo_and_reload() { + for edges in [json!([[25, 0, 6]]), json!([[25, 0, 6], [24.287266336668306, 0.7127336633316916, 0.48266373325510686]])] { + let mut s = bottom_rounds(); + let before = volume(&mut s); + let out = run(&mut s, "solid.fillet", json!({"edges":edges, "radius":"corner_radius"})); + let id = out["feature"].as_u64().unwrap(); + let after = volume(&mut s); + assert!(after < before - 1.0); + assert_eq!(s.doc.features.len(), 4); + run(&mut s, "edit.undo", json!({})); + assert!((volume(&mut s) - before).abs() < 1e-6); + run(&mut s, "edit.redo", json!({})); + assert!((volume(&mut s) - after).abs() < 1e-6); + run(&mut s, "parameters.change", json!({"name":"corner_radius", "expression":"1 mm"})); + let smaller = volume(&mut s); + assert!(smaller > after && smaller < before); + assert!(run(&mut s, "timeline.inspect", json!({"feature":id}))["error"].is_null()); + let bytes = crate::io::write_design_container(&s.doc).unwrap(); + let mut reopened = Session::new(crate::io::read_design(&bytes).unwrap()); + assert!((volume(&mut reopened) - smaller).abs() < 1e-6); + assert_eq!(reopened.doc.features.len(), 4); + } +} + +#[test] +fn issue1557_failed_radius_keeps_the_existing_rounds() { + let mut s = bottom_rounds(); + let before = s.doc.to_json(); + let v = volume(&mut s); + let undo = s.undo.len(); + assert!(s.execute("solid.fillet", &json!({"edges":[[25,0,6]], "radius":"1000 mm"})).is_err()); + assert_eq!(s.doc.to_json(), before); + assert_eq!(s.undo.len(), undo); + assert!((volume(&mut s) - v).abs() < 1e-6); +} diff --git a/crates/kernel/src/blend.rs b/crates/kernel/src/blend.rs index 08c3bd62..324250ba 100644 --- a/crates/kernel/src/blend.rs +++ b/crates/kernel/src/blend.rs @@ -1611,7 +1611,7 @@ fn tangent_faces(solid: &Solid, edge: &mt::Edge) -> bool { /// The chain of edges continuing `edge` smoothly at its ends (its ids): closed, or open where /// the next edge turns sharply. -fn smooth_chain(solid: &Solid, edge: &mt::Edge) -> Option> { +pub(crate) fn smooth_chain(solid: &Solid, edge: &mt::Edge) -> Option> { smooth_chain_within(solid, edge, &|_| true) } diff --git a/crates/kernel/src/blend_corner.rs b/crates/kernel/src/blend_corner.rs index ea3cd7de..a4152866 100644 --- a/crates/kernel/src/blend_corner.rs +++ b/crates/kernel/src/blend_corner.rs @@ -44,12 +44,13 @@ pub fn fillet_radii(body: &Body, edges: &[(Vec3, f64)]) -> Result { Ok(b) => return Ok(crate::record::after(b, &cur, &counts)), Err(e) => e, }; - let rounds = rounds_at(&cur, &sel); + let rounds = guard("recognise earlier rounds", || rounds_at(&cur, &sel))?; if rounds.is_empty() { return Err(first.unwrap_or(err)); } first.get_or_insert(err); let idx: Vec = rounds.iter().map(|r| r.0).collect(); + sel = guard("map picks across earlier rounds", || surviving_picks(&cur, &sel, &idx))?; cur = crate::record::after(crate::delete_faces(&cur, &idx)?, &cur, &counts); sel.extend(rounds.iter().map(|r| (r.1, r.2))); } @@ -793,9 +794,35 @@ fn crease(a: &Sel, b: &Sel, q: Vec3, end: Vec3, size: f64) -> Result crate::splitface::fit(&pts, size) } -/// Faces at the filleted edges' ends that are rounds of a straight edge between planar faces: +/// Keep selected sharp edges while consuming a selected mitre that is on the same smooth +/// chain as one of them. After removing the rounds, that mitre no longer exists; using its +/// old point would silently choose a different nearby edge, possibly with a different radius. +fn surviving_picks(cur: &Body, sel: &[(Vec3, f64)], removed: &[usize]) -> Result> { + let solid = &*cur.solid; + let edges = Body::unique_edges(solid); + let faces: Vec<_> = solid.face_iter().collect(); + let mut selected = Vec::new(); + for (p, r) in sel { + let (idx, _) = cur.nearest_edge(*p, (cur.size() * 2e-3).max(1e-3))?.ok_or_else(|| unsupported("no edge at a fillet pick"))?; + let e = edges.get(idx).ok_or_else(|| unsupported("no edge at a fillet pick"))?; + let count = removed.iter().filter_map(|i| faces.get(*i)).filter(|f| f.edge_iter().any(|x| x.id() == e.id())).count(); + selected.push((e, *p, *r, count)); + } + for (e, _, r, count) in &selected { + if *count == 0 { + continue; + } + let chain = crate::blend::smooth_chain(solid, e).unwrap_or_default(); + if *count != 2 || !selected.iter().any(|(other, _, rr, n)| *n == 0 && (r - rr).abs() <= r * 1e-9 && chain.contains(&other.id())) { + return Err(unsupported("a selected earlier-round boundary cannot be mapped to a sharp edge")); + } + } + Ok(selected.into_iter().filter(|(_, _, _, n)| *n == 0).map(|(_, p, r, _)| (p, r)).collect()) +} + +/// Connected faces at the filleted edges' ends that are rounds of straight edges between planar faces: /// (face index, a point on the sharp edge it replaced, its radius). -fn rounds_at(cur: &Body, sel: &[(Vec3, f64)]) -> Vec<(usize, Vec3, f64)> { +fn rounds_at(cur: &Body, sel: &[(Vec3, f64)]) -> Result> { let size = cur.size(); let tol = (size * 2e-3).max(1e-3); let solid = &*cur.solid; @@ -810,16 +837,43 @@ fn rounds_at(cur: &Body, sel: &[(Vec3, f64)]) -> Vec<(usize, Vec3, f64)> { } } let faces: Vec<&mt::Face> = solid.face_iter().collect(); - let mut out = Vec::new(); + let mut neighbours: HashMap> = HashMap::new(); for (fi, f) in faces.iter().enumerate() { - if plane_normal(f).is_some() || !f.vertex_iter().any(|v| ends.iter().any(|p| p.dist(vtx(&v)) <= size * 1e-9)) { + for e in f.edge_iter() { + neighbours.entry(e.id()).or_default().push(fi); + } + } + let mut pending: Vec = faces + .iter() + .enumerate() + .filter(|(_, f)| f.vertex_iter().any(|v| ends.iter().any(|p| p.dist(vtx(&v)) <= size * 1e-9))) + .map(|(fi, _)| fi) + .collect(); + let mut visited = HashSet::new(); + let mut out = Vec::new(); + while let Some(fi) = pending.pop() { + if !visited.insert(fi) { + continue; + } + let Some(f) = faces.get(fi) else { continue }; + if plane_normal(f).is_some() { continue; } if let Some((p, r)) = round_of(&faces, f, size) { + if out.len() + sel.len() >= 1000 { + return Err(unsupported("too many connected earlier rounds to rebuild")); + } out.push((fi, p, r)); + // Healing requires planar neighbours. Include the other recognised rounds + // sharing this one's mitres, rather than leaving a curved neighbour across + // the gap. A closed ring must be restored and reblended as a whole (#1557). + for e in f.edge_iter() { + pending.extend(neighbours.get(&e.id()).into_iter().flatten().copied().filter(|i| !visited.contains(i))); + } } } - out + out.sort_by_key(|r| r.0); + Ok(out) } /// A round between two planes: two parallel straight boundary edges on planar faces, the @@ -1274,6 +1328,76 @@ mod tests { assert!((vol(&e) - strips(2.0, 2.0, 0.0)).abs() < 0.05); } + #[test] + fn issue1557_vertical_after_bottom_rounds() { + let b = box_solid(Vec3::ZERO, Vec3::new(25.0, 25.0, 10.0)).unwrap(); + let bottom = [Vec3::new(12.5, 0.0, 0.0), Vec3::new(25.0, 12.5, 0.0), Vec3::new(12.5, 25.0, 0.0), Vec3::new(0.0, 12.5, 0.0)]; + let rounded = fillet(&b, &bottom, 2.0).unwrap(); + let vertical = Vec3::new(25.0, 0.0, 6.0); + let seam = Vec3::new(24.287266336668306, 0.7127336633316916, 0.48266373325510686); + let mut together = bottom.to_vec(); + together.push(vertical); + let reference = fillet(&b, &together, 2.0).unwrap(); + for picks in [vec![vertical], vec![vertical, seam], vec![seam, vertical]] { + let result = fillet(&rounded, &picks, 2.0).unwrap(); + assert_eq!(result.face_count(), reference.face_count()); + assert_eq!(result.unmeshed_faces(), 0); + assert!(result.validity().is_empty(), "{:?}", result.validity()); + assert!((vol(&result) - vol(&reference)).abs() < 0.05); + assert!(!result.contains(Vec3::new(24.9, 0.1, 5.0))); + assert!(result.contains(Vec3::new(24.0, 1.0, 5.0))); + } + } + + #[test] + fn issue1557_connected_rounds_keep_their_radii() { + let b = box_solid(Vec3::ZERO, Vec3::new(25.0, 25.0, 10.0)).unwrap(); + let bottom = [Vec3::new(12.5, 0.0, 0.0), Vec3::new(25.0, 12.5, 0.0), Vec3::new(12.5, 25.0, 0.0), Vec3::new(0.0, 12.5, 0.0)]; + let rounded = fillet(&b, &bottom, 2.0).unwrap(); + let vertical = Vec3::new(25.0, 0.0, 6.0); + let seam = Vec3::new(24.287266336668306, 0.7127336633316916, 0.48266373325510686); + for radius in [0.5, 1.0, 3.0] { + let mut together: Vec<_> = bottom.iter().map(|p| (*p, 2.0)).collect(); + together.push((vertical, radius)); + let reference = fillet_radii(&b, &together).unwrap(); + let result = fillet(&rounded, &[seam, vertical], radius).unwrap(); + assert!(result.validity().is_empty(), "{:?}", result.validity()); + assert!((vol(&result) - vol(&reference)).abs() < 0.05, "radius {radius}"); + // The old bottom radius is still 2, away from the newly rounded corner. + assert!(!result.contains(Vec3::new(12.5, 0.5, 0.5))); + assert!(result.contains(Vec3::new(12.5, 1.0, 1.0))); + } + // A tangent boundary of an earlier round is not a sharp edge, and cannot be + // reinterpreted as the sharp edge exposed when that round is taken off. + assert!(fillet(&rounded, &[Vec3::new(12.5, 0.0, 2.0)], 1.0).is_err()); + // Consuming a mitre must not silently discard its independently requested radius, + // or turn a mitre-only selection into a different sharp edge. + assert!(fillet_radii(&rounded, &[(vertical, 1.0), (seam, 2.0)]).is_err()); + assert!(fillet_radii(&rounded, &[(seam, 2.0)]).is_err()); + } + + #[test] + fn issue1557_all_vertical_corners_match_analytic_volume() { + let origin = Vec3::new(10.0, -5.0, 7.0); + let b = box_solid(origin, origin + Vec3::new(25.0, 25.0, 10.0)).unwrap(); + let bottom = + [Vec3::new(12.5, 0.0, 0.0), Vec3::new(25.0, 12.5, 0.0), Vec3::new(12.5, 25.0, 0.0), Vec3::new(0.0, 12.5, 0.0)].map(|p| p + origin); + let vertical = + [Vec3::new(0.0, 0.0, 6.0), Vec3::new(25.0, 0.0, 6.0), Vec3::new(25.0, 25.0, 6.0), Vec3::new(0.0, 25.0, 6.0)].map(|p| p + origin); + let rounded = fillet(&b, &bottom, 2.0).unwrap(); + let result = fillet(&rounded, &vertical, 2.0).unwrap(); + assert!(result.validity().is_empty(), "{:?}", result.validity()); + assert!(kink(&result) < 0.05); + // Integrate rounded-square horizontal sections. Above z=r the area is + // L²-(4-π)r²; below, a square of side L-2r plus a disk offset of radius + // sqrt(r²-(z-r)²). This includes the four spherical bottom corners. + let (l, h, r) = (25.0, 10.0, 2.0); + let pi = std::f64::consts::PI; + let a = l - 2.0 * r; + let expected = (h - r) * (l * l - (4.0 - pi) * r * r) + a * a * r + pi * a * r * r + 2.0 * pi * r * r * r / 3.0; + assert!((vol(&result) - expected).abs() < 0.1, "{} vs {expected}", vol(&result)); + } + #[test] fn three_rounds_at_a_corner() { let (bx, a, b, c) = setup(); diff --git a/crates/kernel/src/delete_face.rs b/crates/kernel/src/delete_face.rs index 3da5f113..3817463d 100644 --- a/crates/kernel/src/delete_face.rs +++ b/crates/kernel/src/delete_face.rs @@ -3,7 +3,8 @@ //! //! The body is rebuilt from its own topology. Faces away from the gap are kept as they are. //! Each neighbouring face (planar) keeps its edges that don't touch the gap; a loop that ran -//! entirely along deleted faces (a hole's rim, a boss's footprint) is dropped; a corner on the +//! entirely along deleted faces is dropped for a hole's rim or a boss's footprint, or restored +//! at the moved corners for a face enclosed by rounds. A corner on the //! gap moves to where its planes meet the neighbours across the gap (a fillet's tangent points //! go to the sharp corner), and the stretch of loop that bordered deleted faces closes with a //! straight edge between the moved corners. Anything else (curved neighbours, a gap the @@ -216,6 +217,28 @@ pub fn delete_faces(body: &Body, faces: &[usize]) -> Result { for w in f.boundaries() { let edges: Vec = w.edge_iter().cloned().collect(); if edges.iter().all(gap) { + // A hole or boss footprint disappears when its walls are removed. A + // face surrounded by rounds does not: its whole boundary moves back to + // the intersections of the remaining planes (#1557). Keep that loop, + // sharing its restored edges with the adjacent rebuilt faces. + let moving = edges.iter().any(|e| moved.contains_key(&e.front().id()) || moved.contains_key(&e.back().id())); + if moving { + if !edges.iter().all(|e| moved.contains_key(&e.front().id()) && moved.contains_key(&e.back().id())) { + return Err(fail(format!("face {fi} has a closed gap boundary that cannot be restored completely"))); + } + let mut boundary = Vec::new(); + for e in &edges { + let (a, b) = (pos(e.front()), pos(e.back())); + if a.dist(b) > q * 10.0 { + let restored = line(a, b, &mut verts); + rec.piece(&restored, e); + boundary.push(restored); + } + } + if !boundary.is_empty() { + wires.push(boundary.into()); + } + } continue; } // Start after a gap stretch so stretches don't wrap around. @@ -321,6 +344,21 @@ mod tests { .collect() } + #[test] + fn issue1557_delete_closed_fillet_ring_keeps_the_floor() { + let b = box_solid(Vec3::ZERO, Vec3::new(25.0, 25.0, 10.0)).unwrap(); + let picks = [Vec3::new(12.5, 0.0, 0.0), Vec3::new(25.0, 12.5, 0.0), Vec3::new(12.5, 25.0, 0.0), Vec3::new(0.0, 12.5, 0.0)]; + let rounded = fillet(&b, &picks, 2.0).unwrap(); + let rounds: Vec = + rounded.solid.face_iter().enumerate().filter(|(_, f)| crate::blend::plane_normal(f).is_none()).map(|(i, _)| i).collect(); + assert_eq!(rounds.len(), 4); + let restored = delete_faces(&rounded, &rounds).unwrap(); + assert_eq!(restored.face_count(), 6); + assert_eq!(restored.unmeshed_faces(), 0); + assert!((volume(&restored) - 6250.0).abs() < 1e-6); + assert!(restored.contains(Vec3::new(0.1, 0.1, 0.1))); + } + #[test] fn delete_fillet_chamfer_hole_and_boss_faces() { let sharp = 24000.0;