Merge pull request #1840 from brett19/fix/fillet-slit

fix: the fillet clean-up removes an input slit, opt-in (WR-602, fork cac11ea)

crates/difftest/src/solidcraft.rs

@@ -1429,6 +1429,7 @@ fn enable_fix(k: &mut Kernel, name: &str) -> Result<(), String> {
         }
         "delete_vertex_joins_seam_iso_curve" => &mut k.fixes.delete_vertex_joins_seam_iso_curve,
         "cover_refuses_folded_continuation" => &mut k.fixes.cover_refuses_folded_continuation,
+        "fillet_cleanup_removes_input_slits" => &mut k.fixes.fillet_cleanup_removes_input_slits,
         "cover_wraps_across_closed_seam" => &mut k.fixes.cover_wraps_across_closed_seam,
         "fillet_refuse_non_manifold_result" => &mut k.fixes.fillet_refuse_non_manifold_result,
         "fillet_refuse_crossing_rails" => &mut k.fixes.fillet_refuse_crossing_rails,

crates/solidcore/src/boolean/merge/p1.rs

@@ -43,6 +43,9 @@
 //! it on both sides (slits), skipping any a later removal deleted.
 //! Opt-in `Kernel::fixes.fillet_cleanup_keeps_target_faces` (WR-459, #423): the edges in
 //! `CleanupRelations::kept_edges`, and those whose faces are all in `kept_faces`, stay.
+//! Opt-in `Kernel::fixes.fillet_cleanup_removes_input_slits` (WR-602, the fork's
+//! tangency-labs/smlib@cac11ea): of those, an edge with one face on both sides (a slit)
+//! is checked all the same, and the kept-faces test needs two faces.
 //! Opt-in `Kernel::fixes.cover_wraps_across_closed_seam` (WR-599, the fork's
 //! tangency-labs/smlib@16bf1cb): after each face merge `boolean_post_process` runs
 //! `HasSeamProblem` on the merged face and `SplitAtSeam` when a seam is crossed or missing.
@@ -579,12 +582,20 @@ pub(crate) fn boolean_post_process_with_notifications(
         } else if !check_all_edges && relations.mates.edges.other(e).is_none() {
             continue;
         }
-        if relations.kept_edges.contains(&e) {
+        // WR-602 (`Kernel::fixes.fillet_cleanup_removes_input_slits`, the fork's
+        // tangency-labs/smlib@cac11ea): an edge of the fillet's input, or one between two
+        // of its faces, stays, unless one face lies on both its sides: removing that slit
+        // merges no faces.
+        let input_slits = k.fixes.fillet_cleanup_removes_input_slits;
+        if relations.kept_edges.contains(&e)
+            && !(input_slits && edge::faces(brep, e, None)?.len() == 1)
+        {
             continue;
         }
         if !relations.kept_faces.is_empty() {
             let faces = edge::faces(brep, e, None)?;
-            if !faces.is_empty() && faces.iter().all(|f| relations.kept_faces.contains(f)) {
+            let least = if input_slits { 2 } else { 1 };
+            if faces.len() >= least && faces.iter().all(|f| relations.kept_faces.contains(f)) {
                 continue;
             }
         }

crates/solidcore/src/kernel.rs

@@ -438,6 +438,8 @@ pub struct UpstreamFixes {
     pub corner_blend_stays_inside_neighbours: bool,
     /// `DoFilleting`'s `BooleanPostProcess` removes none of the target's own edges, nor an edge between two of its faces (the fork's `m_sPostProcessKeptEdges`/`Faces`), so the patches of a sphere imported in six faces are not merged into a face no patch's parameterization can carry (WR-459, #423; the fork's tangency-labs/smlib@9ddf4a8).
     pub fillet_cleanup_keeps_target_faces: bool,
+    /// With `fillet_cleanup_keeps_target_faces` on, `DoFilleting`'s `BooleanPostProcess` still checks a kept edge (one of the target's, or one between two of its faces) that has one face on both sides: removing that slit merges no faces, and the test of an edge between two kept faces needs two faces. Upstream (with the fork's keep) leaves the slit where a round took the face on one side of a target edge away (WR-602; the fork's tangency-labs/smlib@cac11ea).
+    pub fillet_cleanup_removes_input_slits: bool,
     /// `SmFilletNx1Corner::MakeCornerTopology` skips a side edge whose faces meet tangentially at the corner's vertex (a flat top running smoothly onto a round) round the vertex to the edge past it, so the end face is the one beyond the tangent seam (the slot wall), not the round the fillet never meets, and `SmFilletEdge::CalcEdgeOnExtendedEdge` intersects the side face with the end face's extension when the side face is not one of the edge's own faces (WR-457, #418; the fork's tangency-labs/smlib@d5ebcb3).
     pub fillet_nx1_tangent_side_edge: bool,
     /// `SmFilletNx1Corner::MakeCornerTopology`'s tangent side-edge skip (`fillet_nx1_tangent_side_edge`, which must be on too) also passes, once, the filleted edge's G1 continuation across the vertex of the opposite convexity (past any tangent edges): the overhang's rim continuing the base circle of a cylinder standing over a block's edge. The edge past it, of the filleted edge's convexity, is the side edge extended and the face beyond it the end face, so the round ends there instead of failing with status 1001 (WR-499, #1474; the fork's tangency-labs/smlib@420e47d).

crates/solidcore/tests/fork_cac11ea_fillet_cleanup_removes_input_slits.rs

@@ -0,0 +1,123 @@
+//! WR-602 (the SolidCraft fork's tangency-labs/smlib@cac11ea): with the fork's keep
+//! (`Kernel::fixes.fillet_cleanup_keeps_target_faces`), `DoFilleting`'s
+//! `BooleanPostProcess` leaves every edge of the fillet's input, and every edge between two
+//! of its faces, alone. Where a round takes away the face on one side of a target edge,
+//! that edge is left with one face on both sides: a slit, kept as an input edge.
+//! `Kernel::fixes.fillet_cleanup_removes_input_slits` checks such an edge all the same
+//! (removing it merges no faces), and the kept-faces test needs two faces.
+//!
+//! The slit here is the one of #1120 (WR-304): a disc extruded from two half arcs joined
+//! to a cylinder of the same radius leaves the line between the halves' walls inside the
+//! merged wall. The clean-up then runs as after a fillet whose target is that body: every
+//! edge and face is kept.
+#![allow(
+    clippy::unwrap_used,
+    clippy::expect_used,
+    clippy::panic,
+    clippy::indexing_slicing
+)] // test helpers
+use solidcore::boolean::api_brep::boolean_union;
+use solidcore::boolean::merge::p1::{boolean_post_process, CleanupRelations};
+use solidcore::curve::curve::Curve;
+use solidcore::curve::ellipse::Ellipse;
+use solidcore::geom::attribute::{AttributeStore, ObjectKey};
+use solidcore::geom::axis2_placement::Axis2Placement;
+use solidcore::geom::extent1d::Extent1d;
+use solidcore::mass::integrator::properties::{brep_volume, PropertyAttributes};
+use solidcore::math::{Point3, Vec3};
+use solidcore::primitive::api_primitives::create_cylinder;
+use solidcore::sweep::api_brep::create_linear_sweep;
+use solidcore::topology::brep::p1;
+use solidcore::topology::brep::p2::is_manifold_solid;
+use solidcore::topology::{edge, EdgeId};
+use solidcore::{Brep, Kernel};
+
+const R: f64 = 30.0;
+
+/// The r 30 circle about the Z axis in z = 0 over `[start, end]` degrees.
+fn half_arc(k: &Kernel, start: f64, end: f64) -> Curve {
+    let mut frame = Axis2Placement::default();
+    frame
+        .set_canonical(
+            Point3::ZERO,
+            Vec3::new(1.0, 0.0, 0.0),
+            Vec3::new(0.0, 1.0, 0.0),
+        )
+        .unwrap();
+    let interval = Extent1d::from_min_max(start, end);
+    Curve::from(Ellipse::create_canonical_circle(k, &frame, R, Some(&interval), None).unwrap())
+}
+
+/// A cylinder r 30, z -6..0, joined to a disc r 30, z 0..5 extruded from two half arcs,
+/// with the slit between the halves left (`boolean_cleanup_removes_slits` off).
+fn joined(k: &mut Kernel) -> Brep {
+    let flange = create_cylinder(k, Point3::new(0.0, 0.0, -6.0), R, 6.0).unwrap();
+    let a = half_arc(k, 0.0, 180.0);
+    let b = half_arc(k, 180.0, 360.0);
+    let disc = create_linear_sweep(
+        k,
+        &[Some(&a), Some(&b)],
+        Vec3::new(0.0, 0.0, 1.0),
+        5.0,
+        true,
+    )
+    .unwrap();
+    boolean_union(k, flange, disc).unwrap()
+}
+
+/// The edges with the same face on both sides (seams and slits).
+fn one_face_edges(b: &Brep) -> Vec<EdgeId> {
+    p1::edges(b, None)
+        .unwrap()
+        .into_iter()
+        .filter(|&e| edge::faces(b, e, None).unwrap().len() == 1)
+        .collect()
+}
+
+fn volume(k: &Kernel, b: &mut Brep) -> f64 {
+    let store = AttributeStore::default();
+    let attrs = PropertyAttributes {
+        store: &store,
+        key_of: &|_| ObjectKey(0),
+    };
+    brep_volume(k, b, 1e-6, &attrs).unwrap()
+}
+
+/// The joined body before and after a clean-up that keeps all of it, as `(one-face edges,
+/// edges)` each, and the volume after.
+fn cleaned(remove_input_slits: bool) -> ((usize, usize), (usize, usize), f64) {
+    let mut k = Kernel::default();
+    let mut b = joined(&mut k);
+    let before = (one_face_edges(&b).len(), p1::edges(&b, None).unwrap().len());
+    k.fixes.fillet_cleanup_keeps_target_faces = true;
+    k.fixes.fillet_cleanup_removes_input_slits = remove_input_slits;
+    let relations = CleanupRelations {
+        kept_edges: p1::edges(&b, None).unwrap().into_iter().collect(),
+        kept_faces: p1::faces(&b, None).unwrap().into_iter().collect(),
+        ..CleanupRelations::default()
+    };
+    // `DoFilleting` edits the body it cleans up.
+    b.set_editing_enabled(true);
+    boolean_post_process(&mut k, &mut b, &relations, true).unwrap();
+    assert!(is_manifold_solid(&b).unwrap());
+    let after = (one_face_edges(&b).len(), p1::edges(&b, None).unwrap().len());
+    let v = volume(&k, &mut b);
+    (before, after, v)
+}
+
+#[test]
+fn wr602_default_keeps_the_input_slit() {
+    let (before, after, _) = cleaned(false);
+    // The merged wall has its seam and the slit.
+    assert!(before.0 >= 2, "{before:?}");
+    assert_eq!(after, before, "upstream keeps every input edge");
+}
+
+#[test]
+fn wr602_input_slit_removed_seam_kept() {
+    let (before, after, v) = cleaned(true);
+    assert_eq!(after.0, before.0 - 1, "the slit goes, the seam stays");
+    assert_eq!(after.1, before.1 - 1, "no other edge goes");
+    let want = core::f64::consts::PI * R * R * 11.0;
+    assert!((v - want).abs() <= 1e-6 * want, "volume {v}, want {want}");
+}

plan/upstream-issues/WR-602--fillet-cleanup-keeps-an-input-slit.md

@@ -0,0 +1,9 @@
+#### WR-602. The fillet clean-up keeps a slit: an input edge left with one face on both sides
+- **Severity:** Wrong result
+- **Area:** Fillets and blends
+- **Location:** `source/SMLib/src/SmMerge.cpp`, `SmMerge::BooleanPostProcess` (def. `:827`), the edge loop before `IsTopological` (`:881`), as the SolidCraft fork changed it for WR-459 (`m_bPostProcessKeepsFilletInput`, tangency-labs/smlib@1ccd461 and 9ddf4a8). The pinned upstream has no such keep. This record is about that keep, which the Rust port has as `Kernel::fixes.fillet_cleanup_keeps_target_faces`.
+- **What happens:** with the keep, the clean-up after `DoFilleting` skips every edge marked as the fillet's input and every edge whose faces are all input faces. A round can take away the face on one side of an input edge, so that the same face lies on both its sides. That edge is then a slit inside one face, and the clean-up keeps it because it is an input edge, even though removing it would merge no faces.
+- **Trigger:** the SolidCraft test-peg part (one wall face per extruded circle), with the bar's front top edge rounded r 2 to r 5. The round runs into the full-turn wall at the bar's tangent point. The top of the old edge between the front face and the wall (x 41.65, z 7 to 10 for r 3) loses the front face to the round and keeps the wall on both sides. The result has 23 edges and 15 vertices where 22 and 14 are expected.
+- **How found:** reported by SolidCraft. The fork's fix is tangency-labs/smlib@cac11ea ("fillets: the clean-up removes a slit, an input edge with one face on both sides").
+- **Confirmed:** yes, in the Rust port. With `fillet_cleanup_keeps_target_faces` on and every edge and face of a body kept, `boolean_post_process` leaves a slit in place.
+- **Rust port:** behaves as upstream with the fork's keep by default. With `Kernel::fixes.fillet_cleanup_removes_input_slits`, an edge in `CleanupRelations::kept_edges` that has exactly one face (`GetFaces`) is checked like any other edge, and the test for an edge between kept faces needs at least two faces (the fork's `sKeptFaces.GetSize() > 1`). `IsTopological` then removes the slit, as it does any edge with one face on both sides that is not a seam. `crates/solidcore/tests/fork_cac11ea_fillet_cleanup_removes_input_slits.rs` makes the slit of #1120 (WR-304: a disc extruded from two half arcs joined to a cylinder of the same radius) and runs the clean-up with every edge and face kept. With the flag on, the slit goes, the wall's seam stays, and the volume is unchanged. With the flag off, nothing is removed.