From 0c2a4ef371663ed4032d70e126de3ef9bab81f23 Mon Sep 17 00:00:00 2001 From: Andy Aragon Date: Fri, 17 Jul 2026 08:03:19 -0700 Subject: [PATCH 1/2] fix(algo): merge overlapping deepened wall openings in the internal-loops splitter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A later cut can re-open a wall area an earlier cut already opened, offset slightly (the snapClip slot stack's 0.01 ledge margins). The cutter's sections form a closed internal loop on the wall face that overlaps the existing inner wire; the internal-loops splitter attached the loop as an independent second hole, double-covering the overlap band — both rims stayed unpaired, the collinear band pieces traced twice, the analytic gate rejected the cut, and the whole 44-hole snapClip join-edges chain fell back to mesh booleans (F=615 -> 8203 at op-cut-3, export nm=5). split_face_with_internal_loops now merges each overlapping pair: the frame keeps ONE union outline and the removable disc is bounded by the loop pieces outside the hole plus the hole pieces inside the loop. Gated to plane faces with all-Line loop+hole; every 2D test projects 3D endpoints through one locally built frame (stored hole-wire UVs can be fitted in a foreign frame); any chain failure bails to the previous behavior. The full snapClip join-edges chain now replays analytic and watertight natively (F=881 posBad=0, was F=8207 bnd=86). --- .claude/skills/roadmap/SKILL.md | 39 +- .../builder/face_splitter/special_cases.rs | 381 +++++++++++++++++- .../io/tests/deepened_wall_opening_inmem.rs | 111 +++++ 3 files changed, 499 insertions(+), 32 deletions(-) create mode 100644 crates/io/tests/deepened_wall_opening_inmem.rs diff --git a/.claude/skills/roadmap/SKILL.md b/.claude/skills/roadmap/SKILL.md index 9fa478d3..7f37e22c 100644 --- a/.claude/skills/roadmap/SKILL.md +++ b/.claude/skills/roadmap/SKILL.md @@ -104,10 +104,15 @@ that does not exist yet; without it, stop. STEP-faithful in-memory repros now EXIST (`crates/io/tests/scoop*_inmem.rs`); the old "needs serialization tooling first" framing is stale. The real blocker is the coordinated split. -- **Snap-clip deepened-notch case.** A later cut deepening an earlier opening strands - the old floor edge. The 2D polygon-union primitive it needs now EXISTS - (`crates/math/src/polygon_boolean.rs::polygon_union`). What remains is sound - deepened-notch detection that does not false-positive on near-coincident faces. +- **Snap-clip deepened-notch case — NO LONGER TERMINAL; both faces of it are closed.** + The cone-face variant closed via the outer-region section clip (the #1102 dig). The + plane-face variant (a later cut's internal section loop OVERLAPPING an existing wall + opening — the snapClip join-edges export root) closed via the deepened-opening union + in `split_face_with_internal_loops` (`union_internal_loop_with_hole`, all-Line + + interaction-gated, bails to prior behavior on any chain failure; fixture + `crates/io/tests/deepened_wall_opening_inmem.rs`). Detection is geometric overlap in + a locally-built frame — no heuristic. Arc-bounded openings still bail; extend the + union to arcs only when a repro demands it. - **A universal smarter merge-key for duplicate edges. PROVEN UNBUILDABLE.** The gridfinity lip corner (chord + arc, same endpoints) MUST merge; the torus-box in-tube lens (line + co-endpoint arc) MUST stay distinct. No merge-key discriminant separates @@ -239,10 +244,15 @@ snapClip family — THREE roots CLOSED 2026-07-16 (#1080, #1082, #1085); deepene chain partners (weld at 100·tol). FOIL SET GREW: cylinder-slot + groove-mouth + junction-disc are now mandatory alongside d4/pcut3/divider for ANY section/clip change — three wrong gate choices were each caught by a different foil. -- REMAINING: hole-1+ deepened-notch/coplanar-margin family (0.01 offsets between - successive slot cutters → micro-edges + near-coincident conic pairs; the - deepened-notch TERMINAL row's missing piece), 0.6mm-nozzle nm, bed-flat clip - volume 46.701 vs 46.6±0.05 pin (run the reference kernel side-by-side first). +- REMAINING after the deepened-opening union (which closed the join-edges chain — + the full 44-hole snapClip plate replays analytic F=881 posBad=0, was F=8207 + bnd=86): the 0.6mm-nozzle chain still mesh-falls-back at op-cut-5 on a FRESH + native chain (nozzle-scaled cutter dims — dig recipe: replay driver + `replay_snapjoin.rs` in the 2026-07-17 capture cache, CHAIN=1 DUMP_AT=5 on + capture-snapnozzle to mint the honest pre-cut plate, then PLATE/HOLE minimal + repro; captured operands are fallback-poisoned, do NOT replay them directly), + and the bed-flat clip volume 46.701 vs 46.6±0.05 pin (genuine deviation, the + reference kernel passes — per-op dual-kernel volume diff). Fit-offset groove-mouth sliver family — CLOSED (2026-07-16, PR #1078, fixture `crates/io/tests/fitoffset_groove_mouth_inmem.rs`): each groove cutter's mouth @@ -543,10 +553,15 @@ snapClip family — THREE roots CLOSED 2026-07-16 (#1080, #1082, #1085); deepene chain partners (weld at 100·tol). FOIL SET GREW: cylinder-slot + groove-mouth + junction-disc are now mandatory alongside d4/pcut3/divider for ANY section/clip change — three wrong gate choices were each caught by a different foil. -- REMAINING: hole-1+ deepened-notch/coplanar-margin family (0.01 offsets between - successive slot cutters → micro-edges + near-coincident conic pairs; the - deepened-notch TERMINAL row's missing piece), 0.6mm-nozzle nm, bed-flat clip - volume 46.701 vs 46.6±0.05 pin (run the reference kernel side-by-side first). +- REMAINING after the deepened-opening union (which closed the join-edges chain — + the full 44-hole snapClip plate replays analytic F=881 posBad=0, was F=8207 + bnd=86): the 0.6mm-nozzle chain still mesh-falls-back at op-cut-5 on a FRESH + native chain (nozzle-scaled cutter dims — dig recipe: replay driver + `replay_snapjoin.rs` in the 2026-07-17 capture cache, CHAIN=1 DUMP_AT=5 on + capture-snapnozzle to mint the honest pre-cut plate, then PLATE/HOLE minimal + repro; captured operands are fallback-poisoned, do NOT replay them directly), + and the bed-flat clip volume 46.701 vs 46.6±0.05 pin (genuine deviation, the + reference kernel passes — per-op dual-kernel volume diff). Fit-offset groove-mouth sliver family — CLOSED (2026-07-16, PR #1078, fixture `crates/io/tests/fitoffset_groove_mouth_inmem.rs`): each groove cutter's mouth diff --git a/crates/algo/src/builder/face_splitter/special_cases.rs b/crates/algo/src/builder/face_splitter/special_cases.rs index 19b14de0..85633fb8 100644 --- a/crates/algo/src/builder/face_splitter/special_cases.rs +++ b/crates/algo/src/builder/face_splitter/special_cases.rs @@ -1,6 +1,6 @@ //! Special topology handlers for face splitting edge cases. -use brepkit_math::vec::Point3; +use brepkit_math::vec::{Point2, Point3}; use brepkit_topology::edge::EdgeCurve; use brepkit_topology::face::{FaceId, FaceSurface}; @@ -1353,6 +1353,50 @@ pub(super) fn split_face_with_internal_loops( loops.iter().map(Vec::len).collect::>() ); + // Deepened-opening union: a later cut's internal loop can OVERLAP an + // existing inner wire (the cutter re-opens an area an earlier cut already + // opened, offset slightly — the snapClip slot stack's 0.01 ledge margins). + // Emitting loop and hole as independent wires double-covers the overlap + // band: both rims stay as unpaired edges and the collinear band pieces + // trace twice. Merge each such pair instead: the frame keeps ONE union + // outline, and the removable disc is bounded by the loop pieces outside + // the hole plus the hole pieces inside the loop. + let mut consumed_holes: std::collections::HashSet = std::collections::HashSet::new(); + let mut union_hole_by_loop: Vec>> = vec![None; loops.len()]; + if let FaceSurface::Plane { normal, .. } = surface { + // Stored UVs on hole wires can be fitted in a foreign frame; build ONE + // local frame and project every 3D endpoint through it for all 2D + // tests (the pcurve-convention lesson). + let frame = PlaneFrame::from_plane_face(*normal, _wire_pts); + for (li, loop_edges) in loops.iter_mut().enumerate() { + let mut hit: Option = None; + for (hi, hole) in original_inner_wires.iter().enumerate() { + if consumed_holes.contains(&hi) { + continue; + } + if internal_loop_hole_interact(loop_edges, hole, &frame) { + if hit.is_some() { + hit = None; // >1 overlapping hole: bail to current behavior + break; + } + hit = Some(hi); + } + } + if let Some(hi) = hit + && let Some((disc, union_hole)) = union_internal_loop_with_hole( + loop_edges, + &original_inner_wires[hi], + &frame, + tol_3d, + ) + { + *loop_edges = disc; + union_hole_by_loop[li] = Some(union_hole); + consumed_holes.insert(hi); + } + } + } + let mut result = Vec::new(); // For each closed loop: create an "inside" sub-face. @@ -1360,7 +1404,7 @@ pub(super) fn split_face_with_internal_loops( // We want the SMALLER region (the Steinmetz lobe), so check signed area // in UV and reverse if the loop encloses the larger region. let mut all_holes: Vec> = Vec::new(); - for loop_edges in &mut loops { + for (li, loop_edges) in loops.iter_mut().enumerate() { // Compute signed area in UV. For single-edge closed curves // (circles), sample points along the pcurve since start_uv ~= end_uv // gives zero area with just the endpoints. @@ -1464,22 +1508,43 @@ pub(super) fn split_face_with_internal_loops( precomputed_interior: Some(disc_interior), }); - // Build reversed loop for the outside sub-face's hole. - let hole: Vec = loop_edges - .iter() - .rev() - .map(|e| OrientedPCurveEdge { - curve_3d: e.curve_3d.clone(), - pcurve: e.pcurve.clone(), - start_uv: e.end_uv, - end_uv: e.start_uv, - start_3d: e.end_3d, - end_3d: e.start_3d, - forward: !e.forward, - source_edge_idx: None, - pave_block_id: None, - }) - .collect(); + // Build the outside sub-face's hole: the merged union outline when + // this loop consumed an overlapping pre-existing hole, otherwise the + // reversed loop. + let hole: Vec = if let Some(u) = union_hole_by_loop[li].take() { + // Normalize to hole winding (CCW in UV — opposite of the CW + // interior-enclosing convention applied to the loop above). + let area: f64 = u + .iter() + .map(|e| (e.end_uv.x() - e.start_uv.x()) * (e.end_uv.y() + e.start_uv.y())) + .sum(); + let mut u = u; + if area > 0.0 { + u.reverse(); + for edge in &mut u { + std::mem::swap(&mut edge.start_uv, &mut edge.end_uv); + std::mem::swap(&mut edge.start_3d, &mut edge.end_3d); + edge.forward = !edge.forward; + } + } + u + } else { + loop_edges + .iter() + .rev() + .map(|e| OrientedPCurveEdge { + curve_3d: e.curve_3d.clone(), + pcurve: e.pcurve.clone(), + start_uv: e.end_uv, + end_uv: e.start_uv, + start_3d: e.end_3d, + end_3d: e.start_3d, + forward: !e.forward, + source_edge_idx: None, + pave_block_id: None, + }) + .collect() + }; // Verify hole is closed. if let (Some(first), Some(last)) = (hole.first(), hole.last()) && (last.end_3d - first.start_3d).length() < tol_3d * 100.0 @@ -1535,8 +1600,15 @@ pub(super) fn split_face_with_internal_loops( // The "outside" sub-face: original boundary with all loops as holes. // Pre-existing holes (from earlier boolean operations) stay with the // outside sub-face — dropping them would leave the faces ringing those - // holes with free edges. - all_holes.extend(original_inner_wires.iter().cloned()); + // holes with free edges. Holes consumed by a deepened-opening union are + // already represented by their union outline. + all_holes.extend( + original_inner_wires + .iter() + .enumerate() + .filter(|(hi, _)| !consumed_holes.contains(hi)) + .map(|(_, h)| h.clone()), + ); let remainder = SplitSubFace { surface: surface.clone(), outer_wire: boundary_edges.to_vec(), @@ -1572,6 +1644,275 @@ pub(super) fn split_face_with_internal_loops( result } +/// True when an internal section loop and a pre-existing inner wire (both +/// all-Line, in UV) overlap: a proper edge crossing, a vertex of one strictly +/// inside the other, or a collinear edge-overlap span. Disjoint pairs return +/// false and keep the independent-wires behavior. +fn internal_loop_hole_interact( + loop_edges: &[OrientedPCurveEdge], + hole: &[OrientedPCurveEdge], + frame: &PlaneFrame, +) -> bool { + let all_line = + |es: &[OrientedPCurveEdge]| es.iter().all(|e| matches!(e.curve_3d, EdgeCurve::Line)); + if !all_line(loop_edges) || !all_line(hole) { + return false; + } + let segs = |es: &[OrientedPCurveEdge]| -> Vec<(Point2, Point2)> { + es.iter() + .map(|e| (frame.project(e.start_3d), frame.project(e.end_3d))) + .collect() + }; + let (ls, hs) = (segs(loop_edges), segs(hole)); + let lp: Vec = ls.iter().map(|s| s.0).collect(); + let hp: Vec = hs.iter().map(|s| s.0).collect(); + let eps = 1e-5; + for &(a0, a1) in &ls { + for &(b0, b1) in &hs { + let (rx, ry) = (a1.x() - a0.x(), a1.y() - a0.y()); + let (sx, sy) = (b1.x() - b0.x(), b1.y() - b0.y()); + let denom = rx.mul_add(sy, -(ry * sx)); + let scale = (rx.hypot(ry) * sx.hypot(sy)).max(f64::MIN_POSITIVE); + if denom.abs() > 1e-9 * scale { + let (qx, qy) = (b0.x() - a0.x(), b0.y() - a0.y()); + let t = qx.mul_add(sy, -(qy * sx)) / denom; + let u = qx.mul_add(ry, -(qy * rx)) / denom; + if t > 1e-6 && t < 1.0 - 1e-6 && u > 1e-6 && u < 1.0 - 1e-6 { + return true; + } + } else { + // Parallel: collinear overlap counts as interaction. + let la = rx.hypot(ry).max(f64::MIN_POSITIVE); + let d0 = ((b0.x() - a0.x()) * ry - (b0.y() - a0.y()) * rx).abs() / la; + if d0 < eps { + let proj = + |p: Point2| ((p.x() - a0.x()) * rx + (p.y() - a0.y()) * ry) / (la * la); + let (u0, u1) = (proj(b0), proj(b1)); + let (lo, hi) = (u0.min(u1).max(0.0), u0.max(u1).min(1.0)); + if hi - lo > eps / la { + return true; + } + } + } + } + } + let strictly_inside = |p: Point2, poly: &[Point2], other: &[(Point2, Point2)]| -> bool { + let on = other.iter().any(|&(s0, s1)| { + let d = point_seg_dist_2d(p, s0, s1); + d < eps + }); + !on && super::super::classify_2d::point_in_polygon_2d(p, poly) + }; + lp.iter().any(|&p| strictly_inside(p, &hp, &hs)) + || hp.iter().any(|&p| strictly_inside(p, &lp, &ls)) +} + +fn point_seg_dist_2d(p: Point2, a: Point2, b: Point2) -> f64 { + let (dx, dy) = (b.x() - a.x(), b.y() - a.y()); + let len2 = dx.mul_add(dx, dy * dy); + if len2 < 1e-30 { + return (p.x() - a.x()).hypot(p.y() - a.y()); + } + let t = ((p.x() - a.x()) * dx + (p.y() - a.y()) * dy) / len2; + let t = t.clamp(0.0, 1.0); + let (fx, fy) = (a.x() + dx * t, a.y() + dy * t); + (p.x() - fx).hypot(p.y() - fy) +} + +/// Merge an internal section loop with the pre-existing inner wire it +/// overlaps (both all-Line). Returns `(disc_outer, union_hole)`: +/// +/// - `disc_outer` bounds the removable region `loop \ hole` — the loop pieces +/// outside the hole plus the hole pieces inside the loop; +/// - `union_hole` is the merged opening outline `loop ∪ hole` — the pieces of +/// both outside each other, with collinear overlap spans contributed once +/// (the hole's copy, so the pieces pair with the pave-split neighbor faces). +/// +/// Any inconsistency (chain fails to close, either chain empty, leftover +/// pieces) returns None and the caller keeps the current independent-wires +/// emission. +#[allow(clippy::too_many_lines)] +fn union_internal_loop_with_hole( + loop_edges: &[OrientedPCurveEdge], + hole: &[OrientedPCurveEdge], + frame: &PlaneFrame, + tol: f64, +) -> Option<(Vec, Vec)> { + use brepkit_math::curves2d::{Curve2D, Line2D}; + use brepkit_math::vec::Vec2; + #[derive(PartialEq, Clone, Copy)] + enum Verdict { + In, + On, + Out, + } + let eps = 1e-5; + let seg_of = |e: &OrientedPCurveEdge| (frame.project(e.start_3d), frame.project(e.end_3d)); + let lsegs: Vec<(Point2, Point2)> = loop_edges.iter().map(seg_of).collect(); + let hsegs: Vec<(Point2, Point2)> = hole.iter().map(seg_of).collect(); + let lp: Vec = lsegs.iter().map(|s| s.0).collect(); + let hp: Vec = hsegs.iter().map(|s| s.0).collect(); + + let mk_piece = |e: &OrientedPCurveEdge, t0: f64, t1: f64| -> Option { + let lerp3 = |t: f64| e.start_3d + (e.end_3d - e.start_3d) * t; + let (s3, e3) = (lerp3(t0), lerp3(t1)); + let (s_uv, e_uv) = (frame.project(s3), frame.project(e3)); + if (e3 - s3).length() < tol { + return None; + } + let d = Vec2::new(e_uv.x() - s_uv.x(), e_uv.y() - s_uv.y()); + let len = d.x().hypot(d.y()); + let dir = if len > 1e-12 { + Vec2::new(d.x() / len, d.y() / len) + } else { + Vec2::new(1.0, 0.0) + }; + let pcurve = Curve2D::Line( + Line2D::new(s_uv, dir) + .or_else(|_| Line2D::new(s_uv, Vec2::new(1.0, 0.0))) + .ok()?, + ); + Some(OrientedPCurveEdge { + curve_3d: EdgeCurve::Line, + pcurve, + start_uv: s_uv, + end_uv: e_uv, + start_3d: s3, + end_3d: e3, + forward: true, + source_edge_idx: None, + pave_block_id: None, + }) + }; + + // Split parameters on `edge` from proper crossings with the other set's + // segments plus the other outline's vertices lying on the edge interior + // (T-junctions and collinear overlap ends). + let split_ts = + |a0: Point2, a1: Point2, other: &[(Point2, Point2)], verts: &[Point2]| -> Vec { + let (rx, ry) = (a1.x() - a0.x(), a1.y() - a0.y()); + let la = rx.hypot(ry).max(f64::MIN_POSITIVE); + let mut ts = vec![0.0, 1.0]; + for &(b0, b1) in other { + let (sx, sy) = (b1.x() - b0.x(), b1.y() - b0.y()); + let denom = rx.mul_add(sy, -(ry * sx)); + let scale = (la * sx.hypot(sy)).max(f64::MIN_POSITIVE); + if denom.abs() <= 1e-9 * scale { + continue; + } + let (qx, qy) = (b0.x() - a0.x(), b0.y() - a0.y()); + let t = qx.mul_add(sy, -(qy * sx)) / denom; + let u = qx.mul_add(ry, -(qy * rx)) / denom; + if t > 1e-9 && t < 1.0 - 1e-9 && u > -1e-6 && u < 1.0 + 1e-6 { + ts.push(t); + } + } + for &v in verts { + if point_seg_dist_2d(v, a0, a1) < eps { + let t = ((v.x() - a0.x()) * rx + (v.y() - a0.y()) * ry) / (la * la); + if t > 1e-9 && t < 1.0 - 1e-9 { + ts.push(t); + } + } + } + ts.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal)); + ts.dedup_by(|a, b| (*a - *b).abs() * la < tol * 10.0); + ts + }; + + let classify = |mid: Point2, poly: &[Point2], segs: &[(Point2, Point2)]| -> Verdict { + if segs + .iter() + .any(|&(s0, s1)| point_seg_dist_2d(mid, s0, s1) < eps) + { + Verdict::On + } else if super::super::classify_2d::point_in_polygon_2d(mid, poly) { + Verdict::In + } else { + Verdict::Out + } + }; + + let mut union_pieces: Vec = Vec::new(); + let mut disc_pieces: Vec = Vec::new(); + for (e, &(s0, s1)) in loop_edges.iter().zip(&lsegs) { + let ts = split_ts(s0, s1, &hsegs, &hp); + for w in ts.windows(2) { + let Some(piece) = mk_piece(e, w[0], w[1]) else { + continue; + }; + let mid = Point2::new( + (piece.start_uv.x() + piece.end_uv.x()) * 0.5, + (piece.start_uv.y() + piece.end_uv.y()) * 0.5, + ); + if classify(mid, &hp, &hsegs) == Verdict::Out { + union_pieces.push(piece.clone()); + disc_pieces.push(piece); + } + // In: air inside the old opening. On: the hole's copy is kept. + } + } + for (e, &(s0, s1)) in hole.iter().zip(&hsegs) { + let ts = split_ts(s0, s1, &lsegs, &lp); + for w in ts.windows(2) { + let Some(piece) = mk_piece(e, w[0], w[1]) else { + continue; + }; + let mid = Point2::new( + (piece.start_uv.x() + piece.end_uv.x()) * 0.5, + (piece.start_uv.y() + piece.end_uv.y()) * 0.5, + ); + match classify(mid, &lp, &lsegs) { + Verdict::In => disc_pieces.push(piece), + Verdict::On | Verdict::Out => union_pieces.push(piece), + } + } + } + + let disc = chain_single_closed_loop(disc_pieces, tol)?; + let union_hole = chain_single_closed_loop(union_pieces, tol)?; + Some((disc, union_hole)) +} + +/// Chain pieces into exactly one closed loop consuming every piece; None on +/// any leftover, open chain, or fewer than 3 pieces. +fn chain_single_closed_loop( + mut pieces: Vec, + tol: f64, +) -> Option> { + let close = tol * 100.0; + if pieces.len() < 3 { + return None; + } + let mut chain = vec![pieces.swap_remove(0)]; + let start = chain[0].start_3d; + while !pieces.is_empty() { + let last_end = chain.last()?.end_3d; + let next = pieces + .iter() + .position(|e| (e.start_3d - last_end).length() < close); + let next_rev = if next.is_none() { + pieces + .iter() + .position(|e| (e.end_3d - last_end).length() < close) + } else { + None + }; + if let Some(i) = next { + chain.push(pieces.swap_remove(i)); + } else if let Some(i) = next_rev { + let mut e = pieces.swap_remove(i); + std::mem::swap(&mut e.start_uv, &mut e.end_uv); + std::mem::swap(&mut e.start_3d, &mut e.end_3d); + e.forward = !e.forward; + chain.push(e); + } else { + return None; + } + } + ((chain.last()?.end_3d - start).length() < close).then_some(chain) +} + /// Interior point on a cylinder/cone lateral remainder face that carries /// CURVED hole loops (the lens loops where another quadric crosses the wall). /// diff --git a/crates/io/tests/deepened_wall_opening_inmem.rs b/crates/io/tests/deepened_wall_opening_inmem.rs new file mode 100644 index 00000000..f5ce3249 --- /dev/null +++ b/crates/io/tests/deepened_wall_opening_inmem.rs @@ -0,0 +1,111 @@ +//! Guard for the deepened wall-opening union (the plane-face stranded-rim +//! case, the snapClip join-edges export root). +//! +//! The snap-slot cutter stack cuts the same seam wall twice: an earlier box +//! opens a rectangular window in the wall, and a later, deeper box re-opens +//! an overlapping window offset by 0.01 (its top floats 0.01 above the +//! earlier slot's floor). The later cutter's sections form a closed internal +//! loop on the wall face that OVERLAPS the existing inner wire; the +//! internal-loops splitter used to attach the loop as an independent second +//! hole, double-covering the 0.01 band — both rims stayed as unpaired edges +//! and the collinear band pieces traced twice (use=3 micro verticals), the +//! analytic gate rejected the cut, and the whole 44-hole snapClip chain fell +//! back to mesh booleans (F=615 → 8203 at op-cut-3). +//! +//! The union pre-pass in `split_face_with_internal_loops` now merges the pair: +//! the wall keeps ONE union outline and the removable piece is bounded by the +//! loop pieces outside the hole plus the hole pieces inside the loop. + +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use std::collections::HashMap; + +use brepkit_algo::bop::BooleanOp; +use brepkit_math::mat::Mat4; +use brepkit_math::vec::Point3; +use brepkit_operations::primitives::make_box; +use brepkit_operations::transform::transform_solid; +use brepkit_topology::Topology; +use brepkit_topology::explorer::solid_faces; +use brepkit_topology::solid::SolidId; + +fn boxed(topo: &mut Topology, lo: [f64; 3], hi: [f64; 3]) -> SolidId { + let s = make_box(topo, hi[0] - lo[0], hi[1] - lo[1], hi[2] - lo[2]).unwrap(); + transform_solid(topo, s, &Mat4::translation(lo[0], lo[1], lo[2])).unwrap(); + s +} + +type Q = (i64, i64, i64); + +fn pos_bad_edges(topo: &Topology, solid: SolidId) -> usize { + let sc = 1.0e5; + let q = |p: Point3| -> Q { + ( + (p.x() * sc).round() as i64, + (p.y() * sc).round() as i64, + (p.z() * sc).round() as i64, + ) + }; + let mut occ: HashMap<(Q, Q), usize> = HashMap::new(); + for &fid in &solid_faces(topo, solid).unwrap() { + let face = topo.face(fid).unwrap(); + for wid in std::iter::once(face.outer_wire()).chain(face.inner_wires().iter().copied()) { + for oe in topo.wire(wid).unwrap().edges() { + let e = topo.edge(oe.edge()).unwrap(); + let a = q(topo.vertex(e.start()).unwrap().point()); + let b = q(topo.vertex(e.end()).unwrap().point()); + let key = if a <= b { (a, b) } else { (b, a) }; + *occ.entry(key).or_default() += 1; + } + } + } + occ.values().filter(|&&c| c != 2).count() +} + +#[test] +fn deepened_wall_opening_merges_overlapping_hole() { + let mut topo = Topology::new(); + let base = boxed(&mut topo, [0.0, 0.0, 0.0], [10.0, 10.0, 10.0]); + + // First cut: tunnel through the x=10 wall, floor at z=4. + let a = boxed(&mut topo, [5.0, 3.0, 4.0], [11.0, 7.0, 6.0]); + let s1 = brepkit_algo::gfa::boolean(&mut topo, BooleanOp::Cut, base, a).unwrap(); + assert_eq!(pos_bad_edges(&topo, s1), 0, "first cut must be watertight"); + + // Second cut: deeper opening through the same wall whose top floats 0.01 + // above the first tunnel's floor (the snap-slot ledge margin). + let b = boxed(&mut topo, [5.0, 3.0, 1.0], [11.0, 7.0, 4.01]); + let s2 = brepkit_algo::gfa::boolean(&mut topo, BooleanOp::Cut, s1, b).unwrap(); + + assert_eq!( + pos_bad_edges(&topo, s2), + 0, + "stranded rims: wall kept both opening outlines over the 0.01 band" + ); + + // The wall must carry ONE merged opening, not two overlapping wires. + let wall_holes: Vec = solid_faces(&topo, s2) + .unwrap() + .iter() + .filter_map(|&fid| { + let face = topo.face(fid).ok()?; + let on_wall = std::iter::once(face.outer_wire()) + .chain(face.inner_wires().iter().copied()) + .flat_map(|wid| topo.wire(wid).unwrap().edges().to_vec()) + .all(|oe| { + let e = topo.edge(oe.edge()).unwrap(); + (topo.vertex(e.start()).unwrap().point().x() - 10.0).abs() < 1e-9 + && (topo.vertex(e.end()).unwrap().point().x() - 10.0).abs() < 1e-9 + }); + (on_wall && !face.inner_wires().is_empty()).then_some(face.inner_wires().len()) + }) + .collect(); + assert_eq!(wall_holes, vec![1], "wall must have exactly one union hole"); + + // Volume: 1000 - 40 (tunnel) - 60.2 (deep cut) + 0.2 (their overlap). + let vol = brepkit_operations::measure::solid_volume(&topo, s2, 0.01).unwrap(); + assert!( + (vol - 900.0).abs() < 0.05, + "volume {vol} deviates from expected 900" + ); +} From c131cf1dba68c5df506fa22a6ec2507b151fdb87 Mon Sep 17 00:00:00 2001 From: Andy Aragon Date: Fri, 17 Jul 2026 08:16:51 -0700 Subject: [PATCH 2/2] fix(algo): derive union pre-pass epsilons from the kernel tolerance Address review: the weld-band proximity threshold in the deepened-opening union is now tol*100 instead of a hardcoded 1e-5 (same value, tied to the kernel tolerance), the wire_pts parameter drops its stale underscore now that it drives the PlaneFrame construction, and the fixture's wall-face detection uses a tolerance-derived epsilon. --- .../src/builder/face_splitter/special_cases.rs | 14 +++++++++----- crates/io/tests/deepened_wall_opening_inmem.rs | 5 +++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/algo/src/builder/face_splitter/special_cases.rs b/crates/algo/src/builder/face_splitter/special_cases.rs index 85633fb8..9525ec00 100644 --- a/crates/algo/src/builder/face_splitter/special_cases.rs +++ b/crates/algo/src/builder/face_splitter/special_cases.rs @@ -1248,7 +1248,7 @@ pub(super) fn split_face_with_internal_loops( rank: Rank, reversed: bool, face_id: FaceId, - _wire_pts: &[Point3], + wire_pts: &[Point3], ) -> Vec { let tol_3d = brepkit_math::tolerance::Tolerance::new().linear; @@ -1367,14 +1367,14 @@ pub(super) fn split_face_with_internal_loops( // Stored UVs on hole wires can be fitted in a foreign frame; build ONE // local frame and project every 3D endpoint through it for all 2D // tests (the pcurve-convention lesson). - let frame = PlaneFrame::from_plane_face(*normal, _wire_pts); + let frame = PlaneFrame::from_plane_face(*normal, wire_pts); for (li, loop_edges) in loops.iter_mut().enumerate() { let mut hit: Option = None; for (hi, hole) in original_inner_wires.iter().enumerate() { if consumed_holes.contains(&hi) { continue; } - if internal_loop_hole_interact(loop_edges, hole, &frame) { + if internal_loop_hole_interact(loop_edges, hole, &frame, tol_3d) { if hit.is_some() { hit = None; // >1 overlapping hole: bail to current behavior break; @@ -1652,6 +1652,7 @@ fn internal_loop_hole_interact( loop_edges: &[OrientedPCurveEdge], hole: &[OrientedPCurveEdge], frame: &PlaneFrame, + tol: f64, ) -> bool { let all_line = |es: &[OrientedPCurveEdge]| es.iter().all(|e| matches!(e.curve_3d, EdgeCurve::Line)); @@ -1666,7 +1667,9 @@ fn internal_loop_hole_interact( let (ls, hs) = (segs(loop_edges), segs(hole)); let lp: Vec = ls.iter().map(|s| s.0).collect(); let hp: Vec = hs.iter().map(|s| s.0).collect(); - let eps = 1e-5; + // Weld-band proximity (100x the exact tolerance): junctions between two + // independently minted openings agree only to weld scale, not 1e-7. + let eps = tol * 100.0; for &(a0, a1) in &ls { for &(b0, b1) in &hs { let (rx, ry) = (a1.x() - a0.x(), a1.y() - a0.y()); @@ -1746,7 +1749,8 @@ fn union_internal_loop_with_hole( On, Out, } - let eps = 1e-5; + // Weld-band proximity, matching `internal_loop_hole_interact`. + let eps = tol * 100.0; let seg_of = |e: &OrientedPCurveEdge| (frame.project(e.start_3d), frame.project(e.end_3d)); let lsegs: Vec<(Point2, Point2)> = loop_edges.iter().map(seg_of).collect(); let hsegs: Vec<(Point2, Point2)> = hole.iter().map(seg_of).collect(); diff --git a/crates/io/tests/deepened_wall_opening_inmem.rs b/crates/io/tests/deepened_wall_opening_inmem.rs index f5ce3249..0dc1c5f5 100644 --- a/crates/io/tests/deepened_wall_opening_inmem.rs +++ b/crates/io/tests/deepened_wall_opening_inmem.rs @@ -89,13 +89,14 @@ fn deepened_wall_opening_merges_overlapping_hole() { .iter() .filter_map(|&fid| { let face = topo.face(fid).ok()?; + let wall_eps = brepkit_math::tolerance::Tolerance::new().linear * 100.0; let on_wall = std::iter::once(face.outer_wire()) .chain(face.inner_wires().iter().copied()) .flat_map(|wid| topo.wire(wid).unwrap().edges().to_vec()) .all(|oe| { let e = topo.edge(oe.edge()).unwrap(); - (topo.vertex(e.start()).unwrap().point().x() - 10.0).abs() < 1e-9 - && (topo.vertex(e.end()).unwrap().point().x() - 10.0).abs() < 1e-9 + (topo.vertex(e.start()).unwrap().point().x() - 10.0).abs() < wall_eps + && (topo.vertex(e.end()).unwrap().point().x() - 10.0).abs() < wall_eps }); (on_wall && !face.inner_wires().is_empty()).then_some(face.inner_wires().len()) })