fix(boolean): the self-touch glue buckets edges by vertex pair (#3167 review)

`glue_self_touching_edges` compared every edge pair and kept its glued edges
in a Vec, close to cubic on a body of thousands of edges, on every boolean
while the flag is on. The edges are now bucketed by their vertex pair, only
a bucket of two or more edges has its curves evaluated and its pairs
compared, and the glued edges are a HashSet. A body without self-touch costs
one pass over its edges.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

solidcore/crates/solidcore-ops/tests/boolean_join_fills_pocket_with_hole_at_its_corner.rs

@@ -0,0 +1,146 @@
+//! solidcraft#12785: a block with a hexagonal pocket through it and a round hole drilled from
+//! an end face along the pocket's axis plane, ending exactly at a corner of the pocket, so
+//! the hole's bottom meets the two pocket walls along a segment of the corner edge (a solid
+//! touching itself along a line). The cut leaves that segment as one edge of four faces; a
+//! record round trip (`to_record`, `from_record`: SolidCraft's body between features) writes
+//! it as two edges of two faces each, and filling the pocket with the same hexagon as a join
+//! then fails (`topology_intersector::p1::relate`, status 1001, or a result that is not a
+//! closed solid). Without the hole, or without the round trip, the same join works.
+#![allow(
+    clippy::unwrap_used,
+    clippy::expect_used,
+    clippy::panic,
+    clippy::indexing_slicing
+)] // test helpers
+
+use std::f64::consts::TAU;
+
+use solidcore_ops::boolean::{boolean, BooleanOp};
+use solidcore_ops::sweep::{extrude, Curve, DraftCorner};
+use solidcore_ops::{session, Body, CheckLevel};
+
+/// The hexagon's circumradius.
+const R: f64 = 6.3685;
+const Z: [f64; 3] = [0.0, 0.0, 1.0];
+
+fn line(start: [f64; 3], end: [f64; 3]) -> Curve {
+    Curve::Line { start, end }
+}
+
+/// The regular hexagon of circumradius `R` about the origin with a corner on +X, at z = 0
+/// (the reporter's sketch: the corner at (R, 0), the next at (R/2, R sqrt(3)/2)).
+fn hexagon() -> Vec<Curve> {
+    const H: f64 = 5.515_282_784_001_197;
+    let corners = [
+        [R, 0.0, 0.0],
+        [R / 2.0, H, 0.0],
+        [-R / 2.0, H, 0.0],
+        [-R, 0.0, 0.0],
+        [-R / 2.0, -H, 0.0],
+        [R / 2.0, -H, 0.0],
+    ];
+    (0..6)
+        .map(|i| line(corners[i], corners[(i + 1) % 6]))
+        .collect()
+}
+
+/// The block 100.29 x 32 x 10.
+fn block() -> Body {
+    extrude(
+        &[
+            line([-50.145, -16.0, 0.0], [50.145, -16.0, 0.0]),
+            line([50.145, -16.0, 0.0], [50.145, 16.0, 0.0]),
+            line([50.145, 16.0, 0.0], [-50.145, 16.0, 0.0]),
+            line([-50.145, 16.0, 0.0], [-50.145, -16.0, 0.0]),
+        ],
+        Z,
+        10.0,
+        0.0,
+        DraftCorner::Curvature,
+        true,
+    )
+    .unwrap()
+}
+
+/// The hexagon extruded through the block's height: the pocket's cutter and the join's tool.
+fn tool() -> Body {
+    extrude(&hexagon(), Z, 10.0, 0.0, DraftCorner::Curvature, true).unwrap()
+}
+
+/// The block with the pocket through it and the hole: 2.5 mm across, along -X from the end
+/// face x = 50.145, 43.7765 deep, so it ends at the pocket's corner (R, 0).
+fn target() -> Body {
+    let pocketed = boolean(block(), tool(), BooleanOp::Subtract)
+        .unwrap()
+        .expect("the pocketed block");
+    let hole = extrude(
+        &[Curve::Arc {
+            center: [50.145, 0.0, 5.0],
+            axis: [-1.0, 0.0, 0.0],
+            reference: [0.0, 1.0, 0.0],
+            radius: 1.25,
+            start: 0.0,
+            end: TAU,
+        }],
+        [-1.0, 0.0, 0.0],
+        50.145 - R,
+        0.0,
+        DraftCorner::Curvature,
+        true,
+    )
+    .unwrap();
+    boolean(pocketed, hole, BooleanOp::Subtract)
+        .unwrap()
+        .expect("the drilled block")
+}
+
+/// `body` as SolidCraft hands it to the next feature: through its record.
+fn round_tripped(body: &Body) -> Body {
+    let (record, _) = body.to_record().unwrap();
+    Body::from_record(&record, true).unwrap().0
+}
+
+/// Joins `target` and the hexagon, checks the result and compares its volume with the sum.
+fn join_fills(target: Body, what: &str) {
+    let want = target.volume(1e-9).unwrap() + tool().volume(1e-9).unwrap();
+    let b = match boolean(target, tool(), BooleanOp::Union) {
+        Ok(Some(b)) => b,
+        Ok(None) => panic!("{what}: no body"),
+        Err(e) => panic!("{what}: {e}"),
+    };
+    let report = b.check(CheckLevel::All).unwrap();
+    assert!(report.is_valid(), "{what}:\n{}", report.text);
+    let got = b.volume(1e-9).unwrap();
+    assert!(
+        (got - want).abs() <= 1e-6 * want,
+        "{what}: volume {got} vs {want}"
+    );
+}
+
+/// The cut's own body: the corner segment is one edge of four faces, and the join works.
+#[test]
+fn filling_a_pocket_whose_corner_a_hole_reaches_joins() {
+    let target = target();
+    assert_eq!(target.counts().unwrap(), [15, 37, 24]);
+    join_fills(target, "the cut's body");
+}
+
+/// The same body after its record round trip: the corner segment is two coincident edges of
+/// two faces each, and with `boolean_glues_self_touching_edges` the join works as well.
+#[test]
+fn filling_a_pocket_whose_corner_a_hole_reaches_joins_after_a_record_round_trip() {
+    session::set_fixes(["boolean_glues_self_touching_edges"]).unwrap();
+    let target = round_tripped(&target());
+    assert_eq!(target.counts().unwrap(), [15, 38, 24]);
+    assert!(target.check(CheckLevel::All).unwrap().is_valid());
+    join_fills(target, "the round-tripped body");
+}
+
+/// The control: without the hole the same join works, round trip or not.
+#[test]
+fn filling_a_pocket_without_the_hole_joins() {
+    let pocketed = boolean(block(), tool(), BooleanOp::Subtract)
+        .unwrap()
+        .expect("the pocketed block");
+    join_fills(round_tripped(&pocketed), "the pocketed block");
+}

solidcore/crates/solidcore/src/boolean/api_brep.rs

@@ -208,6 +208,197 @@ enum MergeMethod {
 /// (`SmMerge sMerge(*pContext, pBrep1, pBrep2)`, or with `0, 0, &sOptions` when
 /// `options` is given), then run `ManifoldBoolean` when both operands are manifold
 /// solids (`SmBrep::IsManifoldSolid`) and `NonManifoldBoolean` otherwise.
+/// One edge of a boolean operand, as [`glue_self_touching_edges`] compares them: its
+/// vertices (the lower index first), its shell, its faces, its curve's kind and three
+/// points along it (the kind and points only for an edge that shares its vertices with
+/// another).
+struct TouchingEdge {
+    edge: EdgeId,
+    start: crate::topology::VertexId,
+    ends: (u32, u32),
+    shell: Option<crate::topology::ShellId>,
+    faces: Vec<crate::topology::FaceId>,
+    kind: Option<core::mem::Discriminant<crate::curve::curve::p2::CurveKind>>,
+    samples: [crate::Point3; 3],
+}
+
+impl TouchingEdge {
+    /// Whether `other` is a second copy of this edge: the same vertices and curve kind,
+    /// points within `tol`, both on faces (not wires), and no face in common (two faces
+    /// meeting across the line, the kernel's crack edges, are not copies). The shells may
+    /// differ: the record round trip leaves a solid touching itself in two shells joined
+    /// along the line, and the kernel shares one edge between shells that touch.
+    fn is_copy_of(&self, other: &Self, tol: f64) -> bool {
+        if self.shell.is_none()
+            || other.shell.is_none()
+            || self.ends != other.ends
+            || self.kind.is_none()
+            || self.kind != other.kind
+            || self.faces.iter().any(|f| other.faces.contains(f))
+        {
+            return false;
+        }
+        if self.start == other.start {
+            self.samples
+                .iter()
+                .zip(&other.samples)
+                .all(|(a, c)| a.distance(*c) <= tol)
+        } else {
+            self.samples
+                .iter()
+                .zip(other.samples.iter().rev())
+                .all(|(a, c)| a.distance(*c) <= tol)
+        }
+    }
+}
+
+/// Not upstream (opt-in, `Kernel::fixes.boolean_glues_self_touching_edges`, WR-754): the
+/// coincident edges along which a solid touches itself, glued back into one edge each.
+///
+/// A solid touching itself along a line (a hole's bottom meeting two pocket walls along a
+/// segment of their corner edge) is one edge of four faces in the kernel's own results, and
+/// its merge handles that form. The record format writes such an edge as one edge per face
+/// pair (`split_touching`), and the merge of an operand in that form gives a body that is not
+/// a closed solid, or fails in `topology_intersector::p1::relate`
+/// (tangency-labs/solidcraft#12785). So each group of edges with the same two vertices, the
+/// same curve kind and coincident points (three samples within the B-rep tolerance), whose
+/// faces are all distinct, is glued into its first edge (`GlueEdgesGeneral`, no gap, with
+/// region nesting), whether the copies lie in one shell or in two shells that touch along the
+/// line (the round trip leaves a self-touching solid in two). Edges whose faces repeat (a face
+/// on both sides of the line: the kernel's crack edges, which its check refuses glued) are
+/// left alone. Returns the number of edges glued away.
+///
+/// The edges are bucketed by their vertex pair; only a bucket of two or more edges has its
+/// curves evaluated and its pairs compared, so a body without self-touch costs one pass over
+/// its edges.
+///
+/// # Errors
+///
+/// Those of the topology walk, the curve evaluations and `GlueEdgesGeneral`.
+fn glue_self_touching_edges(k: &mut Kernel, b: &mut Brep) -> Result<usize> {
+    use std::collections::{HashMap, HashSet};
+
+    use crate::nurbs::b_spline_surface_n_lib::p2::OrientType;
+    use crate::topology::brep::p2::ArenaNotifyState;
+    use crate::topology::brep::p7::glue_edges_general;
+    use crate::topology::edge;
+    use crate::topology::edgeuse::p1 as eu1;
+
+    let tol = b.tolerance();
+    // Every edge with two vertices, by vertex pair.
+    let mut touches: Vec<TouchingEdge> = Vec::new();
+    let mut buckets: HashMap<(u32, u32), Vec<usize>> = HashMap::new();
+    for &e in b.edge_list() {
+        let data = b.edge(e)?;
+        if data.curve.is_none() {
+            continue;
+        }
+        let Some(start) = edge::start_vertex(b, e)? else {
+            continue;
+        };
+        let Some(end) = edge::other_vertex(b, e, Some(start))? else {
+            continue;
+        };
+        if start == end {
+            continue;
+        }
+        let (a, z) = (start.index(), end.index());
+        let ends = (a.min(z), a.max(z));
+        buckets.entry(ends).or_default().push(touches.len());
+        touches.push(TouchingEdge {
+            edge: e,
+            start,
+            ends,
+            shell: None,
+            faces: Vec::new(),
+            kind: None,
+            samples: [crate::Point3::ZERO; 3],
+        });
+    }
+    let mut crowded: Vec<Vec<usize>> = buckets.into_values().filter(|v| v.len() > 1).collect();
+    crowded.sort_unstable();
+    // The shells, faces, curve kinds and points of the edges that share their vertices.
+    for &i in crowded.iter().flatten() {
+        let Some(touch) = touches.get_mut(i) else {
+            continue;
+        };
+        let data = b.edge(touch.edge)?;
+        let Some(curve) = data.curve.as_ref() else {
+            continue;
+        };
+        let interval = data.interval;
+        for (s, t) in touch.samples.iter_mut().zip([0.25, 0.5, 0.75]) {
+            *s = curve.evaluate_point(k, interval.evaluate(t))?;
+        }
+        touch.kind = Some(core::mem::discriminant(curve.kind()));
+        for &eu in &data.edgeuses {
+            let Some(fu) = eu1::faceuse(b, eu)? else {
+                continue;
+            };
+            if let Some(f) = b.faceuse(fu)?.face {
+                touch.faces.push(f);
+            }
+            if touch.shell.is_none() {
+                touch.shell = eu1::faceuse_shell(b, fu)?;
+            }
+        }
+        touch.faces.sort_unstable();
+        touch.faces.dedup();
+    }
+    let mut pairs: Vec<(EdgeId, EdgeId, OrientType)> = Vec::new();
+    let mut taken: HashSet<EdgeId> = HashSet::new();
+    for bucket in &crowded {
+        for (n, &i) in bucket.iter().enumerate() {
+            let Some(first) = touches.get(i) else {
+                continue;
+            };
+            if taken.contains(&first.edge) {
+                continue;
+            }
+            for &j in bucket.iter().skip(n + 1) {
+                let Some(other) = touches.get(j) else {
+                    continue;
+                };
+                if taken.contains(&other.edge) || !first.is_copy_of(other, tol) {
+                    continue;
+                }
+                let orientation = if first.start == other.start {
+                    OrientType::Same
+                } else {
+                    OrientType::Opposite
+                };
+                pairs.push((first.edge, other.edge, orientation));
+                taken.insert(other.edge);
+            }
+        }
+    }
+    if pairs.is_empty() {
+        return Ok(0);
+    }
+    let mut state = ArenaNotifyState::with_attributes_of(b);
+    let mut result = Ok(());
+    for &(surviving, to_delete, orientation) in &pairs {
+        if let Err(e) = glue_edges_general(
+            k,
+            b,
+            Some(surviving),
+            orientation,
+            0.0,
+            true,
+            Some(to_delete),
+            None,
+            &mut state.env(None),
+        ) {
+            result = Err(e);
+            break;
+        }
+    }
+    state.restore_attributes(b)?;
+    result?;
+    b.refresh_cached_topology();
+    Ok(pairs.len())
+}
+
 fn merge_boolean(
     k: &mut Kernel,
     brep1: Brep,
@@ -223,8 +414,8 @@ fn merge_boolean(
 /// B-rep (`SmMerge::m_vEdges`) as it left them.
 fn merge_boolean_by(
     k: &mut Kernel,
-    brep1: Brep,
-    brep2: Brep,
+    mut brep1: Brep,
+    mut brep2: Brep,
     operation: BooleanOperation,
     options: Option<MergeOptions>,
     method: MergeMethod,
@@ -232,6 +423,11 @@ fn merge_boolean_by(
     // SmMerge sMerge(*pContext, pBrep1, pBrep2) (or 0, 0, &sOptions): tolerance and
     // angle 0, both computed by the constructor.
     use crate::boolean::merge::p3::BooleanDriver;
+    // (not upstream, opt-in: WR-754) the operands' self-touch edge copies glued first
+    if k.fixes.boolean_glues_self_touching_edges {
+        glue_self_touching_edges(k, &mut brep1)?;
+        glue_self_touching_edges(k, &mut brep2)?;
+    }
     // (not upstream) the driver carries the operands' attributes through the merge, as
     // upstream's live on the objects the merge edits
     let mut driver = crate::boolean::merge::p3::DependencyMerge::carrying_attributes();

solidcore/crates/solidcore/src/kernel.rs

@@ -1171,6 +1171,8 @@ pub struct UpstreamFixes {
     pub imprint_extends_edge_to_proto_vertex: bool,
     /// With `imprint_extends_edge_to_proto_vertex`, a line, circle or ellipse is continued to its proto vertex over its NURB and stays that kind: a circle or ellipse keeps its frame, radii and inside-out sense, its angular domain carried on to the new end, and a line runs between the continued ends (the bar-top/boss arc of a union, an exact `SmCircle` from `SmCone::IntersectWithPlane`, stopped 5.4e-10 short of its tangent point). Without it those kinds are not continued (the fork before this change made them plain B-splines). The new curve must evaluate as the continued NURB does, or the curve is not continued (WR-601; the fork's tangency-labs/smlib@bec8a0b).
     pub imprint_extension_keeps_analytic_curve: bool,
+    /// `SmApiBoolean` first glues, in each operand, the coincident edges that share both vertices, the curve kind and their points, and whose faces are all distinct (in one shell or in two shells that touch along the line), into one edge of all those faces (`GlueEdgesGeneral`): a solid that touches itself along a line is written by the record format as one edge per face pair (`split_touching`), and the merge of such an operand gives a body that is not a closed solid or fails in `topology_intersector::p1::relate` (1001), while the kernel's own form of the same solid, one edge of four faces, merges (a hexagon pocket joined back where a hole ends at its corner; WR-754, tangency-labs/solidcraft#12785). Off by default: upstream merges the operand as given.
+    pub boolean_glues_self_touching_edges: bool,
     /// `SmEdgeDefinition::Imprint` takes an edge Brep2's face already has between the definition's two vertices along its curve (`SmTools::FindCoincidentEdge` within the larger of the face's and the definition's tolerance) as the definition's edge there, where upstream makes a second edge beside it: the face then has coincident edges and the merge copies the boundary edge into Brep1 beside the curve's (`SmFace` tests 33 and 34; two equal tubes meeting end to end) (WR-580, #1519; the fork's tangency-labs/smlib@81cd61a).
     pub imprint_reuses_coincident_face_edge: bool,
     /// `SmFilletSolver::ComputePointValues` chooses its finite-difference step for each point (1e-3 at a tangent point, 1e-5 otherwise), not once per process as the function-local static `sdStaticStep`, so a blend does not depend on the blends before it ([`Kernel::fillet_point_step`] is then neither read nor set, and an existing latch is kept as it is) (ND-3, #1173; the fork's tangency-labs/smlib@9e17f64 (was 0a05860)).

solidcore/plan/upstream-issues/WR-754--boolean-operand-touching-itself-along-two-coincident-edges.md

@@ -0,0 +1,11 @@
+#### WR-754. `SmApiBoolean` fails on an operand that touches itself along a line carried as two coincident edges of two faces each
+- **Severity:** Wrong result or refusal (a union filling a pocket gives a body that is not a closed solid with the default fixes, or fails in `boolean::topology_intersector::p1::relate` with 1001 under SolidCraft's fix set; SolidCraft refuses the Join)
+- **Area:** Booleans / operand representation
+- **Location:** `crates/solidcore/src/boolean/api_brep.rs`, `merge_boolean_by`: the operands go to the merge as given. The representation comes from the record round trip: `crates/solidcore-convert/src/brep/record.rs` `split_touching` writes an edge a shell uses more than twice as one edge per face pair, and `to_kernel` builds one kernel edge per record edge (its contract: counts and `Order` match the record).
+- **What happens:** the kernel's own results carry a solid that touches itself along a line as one edge of all four faces (a hole's bottom meeting two pocket walls along a segment of their corner edge), and the merge handles that form. The same solid after a record round trip (SolidCraft's body between features) carries the line as two coincident edges of two faces each, sharing both vertices; the body passes `check(All)`, but the merge of a tool whose edge lies along that line relates to two operand edges and either builds a bad body or fails in `relate`.
+- **Trigger:** tangency-labs/solidcraft#12785: a block with a hexagonal pocket through it and a 2.5 mm hole from an end face ending exactly at a pocket corner; filling the pocket with the same hexagon as a Join. The cut's own body joins; after `to_record`/`from_record` the join fails. Sonnet's stand-in with the hole ending in a flat wall (no self-touch) joins. The rc.3 rig's dumped operands (`SmApiBoolean-a.smb`, 38 edges) fail the same way on upstream main and on the pin a445bdaac; the cut's body has 37 edges.
+- **How found:** SolidCraft triage of #12785; the rig's `--dumps` of the failing call, a topology listing of the dumped operand against the ops-crate stand-in (one 8-use edge versus two 4-use edges), and a probe gluing the two edges back before the union (works with the default fixes and with the imprint flags).
+- **Confirmed:** yes, in the port, with the ops crate (`crates/solidcore-ops/tests/boolean_join_fills_pocket_with_hole_at_its_corner.rs`, committed first, failing) and the rig's dumps. Not against the C++ fork: upstream SMLib has no record format and never meets this form from its own results; given such a body it would merge it as given.
+- **Fix:** `Kernel::fixes.boolean_glues_self_touching_edges` (opt-in, off by default): `merge_boolean_by` first glues, in each operand, the coincident edges that share both vertices, the curve kind and their points (three samples within the B-rep tolerance), whose faces are all distinct, into one edge (in one shell, or in two shells that touch along the line: the round trip leaves the self-touching solid as two shells, ShellId 0 and 1 in the test's body) (`GlueEdgesGeneral`, no gap, with region nesting), restoring the kernel's own form. Edges whose faces repeat (a face on both sides of the line: the kernel's crack edges, which its check refuses glued) are left alone; the result's export splits the edge again as before. The import keeps its one-edge-per-record-edge contract.
+- **Also covered:** two solids touching along an edge (`split_shared`: one copy per shell) are glued the same way, as the kernel's own union of such solids shares the edge between the shells (the convert crate's `lumps_and_solids_touching_along_an_edge_go_round`).
+- Fixes tangency-labs/solidcore#3160.