From ad59fffeb987f2c3b8496f4cc3d958d947fcfe4f Mon Sep 17 00:00:00 2001 From: Andy Aragon Date: Fri, 17 Jul 2026 04:33:22 -0700 Subject: [PATCH] fix(algo): accept reversed NURBS sub-spans and run lens interior search as a last resort Two coupled fixes from the deepened-notch dig: 1. topology: domain_with_endpoints now also accepts a REVERSED validated sub-span on a clearly open NURBS curve, returning t0 > t1 so start-to-end interpolation stays truthful. Closed curves keep the full-domain fallback: a reversed projection pair there is usually a seam-crossing forward sub-arc, and interpolating backward would trace the complement arc. 2. algo: a cylinder/cone wall can reach classification with a closed curved inner wire (e.g. a pre-existing notch outline kept through the generic split path) but no precomputed interior, because only the internal-loops splitter ran the dedicated remainder search; the curved-lens flag then aborted the analytic split unconditionally. Run cylinder_cone_remainder_interior as a last resort at the consumption point in fill_images_faces; abort only when even the dense grid finds no contained point. On the deepened-notch raw repro the pair takes posBad 10 -> 6 and resolves both mirrored micro-edge junction chains. The remaining 6 unpaired edges are one root (cone face not split by marched cone-plane sections), documented in the roadmap row updated here. Full workspace green; d4 canary green. --- .claude/skills/roadmap/SKILL.md | 28 ++++++---- crates/algo/src/builder/face_splitter/mod.rs | 1 + .../builder/face_splitter/special_cases.rs | 2 +- crates/algo/src/builder/fill_images_faces.rs | 8 ++- crates/topology/src/edge.rs | 51 +++++++++++++++---- 5 files changed, 67 insertions(+), 23 deletions(-) diff --git a/.claude/skills/roadmap/SKILL.md b/.claude/skills/roadmap/SKILL.md index 5b0cec3e..89e2ff2c 100644 --- a/.claude/skills/roadmap/SKILL.md +++ b/.claude/skills/roadmap/SKILL.md @@ -371,16 +371,24 @@ nozzle nm 1→15, clip volume 46.78→46.70 vs 46.6±0.05). Dovetail residuals: within the 1e-5 weld band, span > 1e-6·domain) returns the projected trimmed `[t₀, t₁]`. On the RAWN=1 raw repro this cleaned one of the two mirrored junction signatures (the use=3 triple + micro-edge chain at - y=−39.4). RESIDUAL: the mirrored twin needs REVERSED sub-spans (SCDOM - instrumentation showed on-curve reversed pairs at 1e-14), but accepting - them (returning `t₀ > t₁`, even gated to open curves) makes the - arrangement mint a DEGENERATE single-edge closed loop (endpoint gap ~4e-9 - — a phantom point-hole at the micro-junction, below vertex-merge reach) - as an inner wire on the cone wall, which trips the curved-lens - no-contained-interior abort (wall Id(1137) in the repro). Next step is - the arrangement/wire-builder side: collapse or reject degenerate - (sub-weld-extent) closed loops before they become inner wires, then - re-attempt reversed acceptance. Repro: cached replay_scplate.rs, RAWN=1, + y=−39.4). SECOND LANDING (same day): REVERSED sub-spans accepted on + clearly-open curves (`t₀ > t₁`, start→end interpolation stays truthful; + closed curves keep the full-domain fallback — a reversed pair there is + usually a seam-crossing forward sub-arc). The "degenerate phantom loop" + that blocked reversed acceptance was a MISREAD: the single-edge closed + NURBS inner wire on the aborting cone wall (seam gap 5e-9, source_edge_idx + Some) is a LEGITIMATE pre-existing notch outline; the real defect was a + COVERAGE hole — walls reaching `fill_images_faces` through split paths + that never run the dedicated `cylinder_cone_remainder_interior` search + aborted unconditionally on the lens flag. Fixed by running that search as + a last resort at the consumption point (fill_images_faces; abort only if + even the dense grid finds nothing). Together: RAWN=1 posBad 10 → 6, both + mirrored micro-edge chains resolved. REMAINING 6 (one root): the CONE + face keeps its un-split rim circle A→B while the notch's wall planes + carry the true route A→C→B — the marched cone×plane sections never split + the cone face itself (the pave-machinery-bypass row; sections carry + pave_block_id=None). Fix at phase-FF/make_blocks altitude or the curved + face splitter's section intake. Repro: cached replay_scplate.rs, RAWN=1, capture capture-snapclip-plate-fresh. Dig provenance: memory project_snapclip-plate-bore.md. - **Mesh-boolean fallback emits OPEN meshes that get CONSUMED — OPEN diff --git a/crates/algo/src/builder/face_splitter/mod.rs b/crates/algo/src/builder/face_splitter/mod.rs index 8c6dbe7e..d6cf99d8 100644 --- a/crates/algo/src/builder/face_splitter/mod.rs +++ b/crates/algo/src/builder/face_splitter/mod.rs @@ -9,6 +9,7 @@ mod conversion; mod edge_splitting; mod sampling; mod special_cases; +pub(in crate::builder) use special_cases::cylinder_cone_remainder_interior; pub use conversion::collect_wire_points; diff --git a/crates/algo/src/builder/face_splitter/special_cases.rs b/crates/algo/src/builder/face_splitter/special_cases.rs index b33e3adb..19b14de0 100644 --- a/crates/algo/src/builder/face_splitter/special_cases.rs +++ b/crates/algo/src/builder/face_splitter/special_cases.rs @@ -1582,7 +1582,7 @@ pub(super) fn split_face_with_internal_loops( /// distance to every hole loop is largest, which is guaranteed to lie on the /// kept wall well clear of every lens. Returns `None` if the surface evaluator /// or boundary v-range is unusable, so the caller keeps the unset interior. -fn cylinder_cone_remainder_interior(remainder: &SplitSubFace) -> Option { +pub fn cylinder_cone_remainder_interior(remainder: &SplitSubFace) -> Option { use std::f64::consts::{PI, TAU}; // Densely sample every hole loop into 3D points AND project each to a (u, v) // polyline so candidate interior points can be tested for face containment diff --git a/crates/algo/src/builder/fill_images_faces.rs b/crates/algo/src/builder/fill_images_faces.rs index e03be45d..116f6665 100644 --- a/crates/algo/src/builder/fill_images_faces.rs +++ b/crates/algo/src/builder/fill_images_faces.rs @@ -461,7 +461,13 @@ pub fn fill_images_faces( resolved_face_id, ) => { - None + // Not every split path that can leave a closed curved hole + // on a cylinder/cone wall runs the dedicated remainder + // search (e.g. a wall keeping a pre-existing lens hole + // through the generic split). Run it here as a last resort; + // only when even the dense grid finds nothing does the + // classifier abort to mesh. + super::face_splitter::cylinder_cone_remainder_interior(split) } None => Some(super::face_splitter::interior_point_3d( split, diff --git a/crates/topology/src/edge.rs b/crates/topology/src/edge.rs index df517de5..e210e90d 100644 --- a/crates/topology/src/edge.rs +++ b/crates/topology/src/edge.rs @@ -116,17 +116,20 @@ impl EdgeCurve { } let proj = |p| brepkit_math::nurbs::projection::project_point_to_curve(n, p, 1e-9); if let (Ok(pa), Ok(pb)) = (proj(start), proj(end)) { - // Accept only a forward, non-degenerate, on-curve span. - // Everything else — reversed pairs (on a closed curve these - // are usually seam-crossing forward sub-arcs, where - // interpolating backward would trace the complement arc; - // downstream arrangement consumers also mint degenerate - // sliver loops from reversed spans), degenerate spans, - // off-curve endpoints — keeps the historical full-domain - // behaviour. + // Accept a non-degenerate on-curve span. A reversed pair + // (`t₀ > t₁`, interpolation still traces start→end) is + // trusted only on a clearly OPEN curve: on a (nearly) + // closed curve a reversed projection pair is usually a + // seam-crossing forward sub-arc, and interpolating + // backward would trace the complement arc. Degenerate + // spans and off-curve endpoints keep the historical + // full-domain behaviour. + let dt = pb.parameter - pa.parameter; + let curve_open = (p0 - p1).length() >= WELD_EPS; if pa.distance < WELD_EPS && pb.distance < WELD_EPS - && pb.parameter - pa.parameter > 1e-6 * (d1 - d0) + && dt.abs() > 1e-6 * (d1 - d0) + && (dt > 0.0 || curve_open) { return (pa.parameter, pb.parameter); } @@ -386,7 +389,7 @@ mod tests { } #[test] - fn nurbs_domain_reversed_sub_span_falls_back_to_full_domain() { + fn nurbs_domain_reversed_sub_span_on_open_curve_trims_backward() { let curve = open_nurbs(); let EdgeCurve::NurbsCurve(n) = &curve else { unreachable!() @@ -396,7 +399,33 @@ mod tests { let tb = d0 + 0.7 * (d1 - d0); let pa = brepkit_math::traits::ParametricCurve::evaluate(n, ta); let pb = brepkit_math::traits::ParametricCurve::evaluate(n, tb); - assert_full_domain(curve.domain_with_endpoints(pb, pa), d0, d1); + // Edge runs pb -> pa (reversed relative to curve parameterization): + // the trimmed domain keeps t0 > t1 so t0->t1 interpolation still + // traces start -> end. + let (t0, t1) = curve.domain_with_endpoints(pb, pa); + assert!(t0 > t1, "expected reversed span, got ({t0}, {t1})"); + assert!((curve.evaluate_with_endpoints(t0, pb, pa) - pb).length() < 1e-6); + assert!((curve.evaluate_with_endpoints(t1, pb, pa) - pa).length() < 1e-6); + } + + #[test] + fn nurbs_domain_reversed_pair_on_closed_curve_falls_back_to_full_domain() { + // A closed fitted loop: a reversed projection pair here is ambiguous + // (usually a seam-crossing forward sub-arc), so the full domain wins. + let pts: Vec = (0..=12) + .map(|k| { + let a = std::f64::consts::TAU * f64::from(k) / 12.0; + Point3::new(a.cos(), a.sin(), 0.0) + }) + .collect(); + let n = brepkit_math::nurbs::fitting::interpolate(&pts, 3).unwrap(); + let (d0, d1) = brepkit_math::traits::ParametricCurve::domain(&n); + let ta = d0 + 0.6 * (d1 - d0); + let tb = d0 + 0.2 * (d1 - d0); + let pa = brepkit_math::traits::ParametricCurve::evaluate(&n, ta); + let pb = brepkit_math::traits::ParametricCurve::evaluate(&n, tb); + let curve = EdgeCurve::NurbsCurve(n); + assert_full_domain(curve.domain_with_endpoints(pa, pb), d0, d1); } #[test]