Merge pull request #1684 from tangency-labs/fork/5533bbb

fix: opt-in edge check orders tangent faces by where they go (WR-540, fork 5533bbb; #383)

crates/solidcore/src/kernel.rs

@@ -195,6 +195,8 @@ pub struct UpstreamFixes {
     pub check_loop_vertex_tolerance: bool,
     /// `CheckLoop` compares an end projected from its vertex (no UV trim) with its neighbour on the nearest natural period of a closed direction, so a one-period seam jump is not an `SM_ERR` (WR-440, #530).
     pub check_loop_projected_seam_ends: bool,
+    /// `SmEdge::AssertValid` test 9 orders the faces round an edge by the direction to each face's point 20 edge tolerances into it (square to the edge) where two of them leave the edge within `SM_ANG_TOL_DEG` of each other, instead of by the binormals alone, which tie for faces tangent along the edge (a cylinder resting on a plane, kept by a boolean as a four-face edge) and fail the valid order (WR-540, #383; the fork's tangency-labs/smlib@5533bbb).
+    pub edge_check_orders_tangent_faces: bool,
     /// `CreateOrientedTrimBoundary` takes a UV gap of whole periods (up to 1% of one) in a closed direction between consecutive curves for a seam crossing and moves the curves after it by the period, instead of filling it with a line across the whole domain that doubles the loop back on itself (WR-491). A loop that does not close that way keeps the line, and so does a gap along a singular side of the surface (a sphere's pole, a cone's apex; #1437).
     pub trim_boundary_seam_wrap: bool,
     /// `SmBSplineCurve::AssertValid` reads the minimum radius of curvature before tests 13/14 compare it (WR-382, #475).

crates/solidcore/src/topology/brep/validation/assert_valid/edge.rs

@@ -17,7 +17,11 @@
 //! evaluation propagates its error. Loop membership of an edge use (test 14) and wire
 //! membership (test 13) read sets built once per loop use / shell. `SmHyperbola` and
 //! `SmParabola` use the inherited `SmBSplineCurve` check with their own `MakeNurb`,
-//! `IsAnalytic` and `IsBounded`.
+//! `IsAnalytic` and `IsBounded`. Opt-in (WR-540,
+//! `Kernel::fixes.edge_check_orders_tangent_faces`, #383): where two faces round an edge
+//! leave it within `SM_ANG_TOL_DEG` of each other, test 9 orders every face by the
+//! direction to its point 20 edge tolerances into the face, square to the edge (the
+//! fork's tangency-labs/smlib@5533bbb).
 
 use std::collections::{BTreeMap, BTreeSet};
 
@@ -27,9 +31,9 @@ use crate::error::{EntityRef, Result};
 use crate::geom::assert_array::{AssertArray, ObjectKey};
 use crate::geom::extent1d::Extent1d;
 use crate::geom::extent2d::Extent2d;
-use crate::geom::tol::{is_zero, ScaledZero, ZONE_TOL_3D};
+use crate::geom::tol::{is_zero, ScaledZero, ANG_TOL_DEG, ZONE_TOL_3D};
 use crate::kernel::Kernel;
-use crate::math::{rad_to_deg, Point3, EFF_ZERO, UNDEF_DOUBLE};
+use crate::math::{rad_to_deg, Point3, EFF_ZERO, PI, UNDEF_DOUBLE};
 use crate::nurbs::b_spline_curve::p3::DerivedOverrides;
 use crate::nurbs::b_spline_surface::SurfParamType;
 use crate::nurbs::b_spline_surface_n_lib::p2::OrientType;
@@ -260,6 +264,46 @@ fn edge_point(k: &Kernel, b: &Brep, e: EdgeId, t: f64) -> Result<Point3> {
     c.evaluate_point(k, t)
 }
 
+/// The direction edge use `eu`'s face leaves the edge in at `t` (with its point,
+/// tangent and face normal): its binormal, or with `step_off` (WR-540) the direction to
+/// its point `step_off_dist` into the face, square to the edge (kept as the binormal when
+/// the step or a unitisation fails).
+fn sector_direction(
+    k: &Kernel,
+    b: &mut Brep,
+    eu: EdgeuseId,
+    t: f64,
+    options: BinormalOptions,
+    step_off: bool,
+    step_off_dist: f64,
+) -> Result<crate::topology::edgeuse::p2::BinormalEval> {
+    use crate::topology::edgeuse::p2::{evaluate_binormal_step_off, StepOffOptions};
+    let mut eval = evaluate_binormal(k, b, eu, t, true, &options)?;
+    if !step_off {
+        return Ok(eval);
+    }
+    let Some(stepped) = crate::error::ignored(evaluate_binormal_step_off(
+        k,
+        b,
+        eu,
+        t,
+        step_off_dist,
+        &StepOffOptions::default(),
+    ))?
+    else {
+        return Ok(eval);
+    };
+    let Ok(unit_tan) = eval.tangent.unitize() else {
+        return Ok(eval);
+    };
+    let mut away = stepped.point - eval.point;
+    away = away - away.dot(unit_tan) * unit_tan;
+    if let Ok(away) = away.unitize() {
+        eval.binormal = away;
+    }
+    Ok(eval)
+}
+
 /// The radial edge-use walk of `SmEdge::AssertValid` (tests 8-11).
 fn radial_order(
     k: &mut Kernel,
@@ -287,6 +331,45 @@ fn radial_order(
 
     // Must watch out for zero sectors: coincident faces.
     let mut have_nonzero_angle = false;
+
+    // Not upstream (WR-540), opt-in `Kernel::fixes.edge_check_orders_tangent_faces` (the
+    // fork's tangency-labs/smlib@5533bbb, #383): a sector between faces tangent along the
+    // edge (a cylinder touching a plane it rests on, left by a boolean as a non-manifold
+    // edge) has binormals that agree to first order, so the binormal angles tie (or differ
+    // by rounding, either way round) and cannot show the order. Where any sector is that
+    // narrow, every face is measured by the direction from the edge to its point a short
+    // step into the face (20 edge tolerances, as IsConvexRadialSector steps).
+    let seq_param = extent.evaluate(0.456);
+    let mut step_off = false;
+    let mut step_off_dist = 0.0;
+    if k.fixes.edge_check_orders_tangent_faces {
+        step_off_dist = 20.0 * tedge::tolerance(b, e)?;
+        let tie_dot = (ANG_TOL_DEG * PI / 180.0).cos();
+        let num_eus = b.edge(e)?.edgeuses.len();
+        let mut eu = target;
+        let mut sectors = 0usize;
+        while let Some(rad) = eu1::radial(b, eu)? {
+            if !eu1::is_loop_edgeuse(b, eu)? || !eu1::is_loop_edgeuse(b, rad)? {
+                break;
+            }
+            let b0 = crate::error::ignored(evaluate_binormal(k, b, eu, seq_param, true, &options))?;
+            let b1 =
+                crate::error::ignored(evaluate_binormal(k, b, rad, seq_param, true, &options))?;
+            if let (Some(b0), Some(b1)) = (b0, b1) {
+                if let (Ok(u0), Ok(u1)) = (b0.binormal.unitize(), b1.binormal.unitize()) {
+                    if u0.dot(u1) > tie_dot {
+                        step_off = true;
+                    }
+                }
+            }
+            sectors += 1;
+            match eu1::mate(b, rad)? {
+                Some(next) if !step_off && next != target && sectors < num_eus => eu = next,
+                _ => break,
+            }
+        }
+    }
+
     let mut steps = 0;
     while Some(target) != primary {
         steps += 1;
@@ -305,8 +388,8 @@ fn radial_order(
             ));
         }
         let t = extent.evaluate(0.456);
-        let target_eval = evaluate_binormal(k, b, target, t, true, &options)?;
-        let radial_eval = evaluate_binormal(k, b, radial, t, true, &options)?;
+        let target_eval = sector_direction(k, b, target, t, options, step_off, step_off_dist)?;
+        let radial_eval = sector_direction(k, b, radial, t, options, step_off, step_off_dist)?;
 
         // 1st iteration save variables
         if primary.is_none() {

crates/solidcore/tests/wr_540_tangent_cylinder_edge_order.rs

@@ -0,0 +1,38 @@
+//! Regression test for WR-540 (tangency-labs/solidcore#383; the fork's
+//! tangency-labs/smlib@5533bbb, capi test `booleans tangent_cylinders`): a box minus a
+//! cylinder that rests on one of its faces, box (20, 5, 5)-(35, 20, 32.5) minus a z
+//! cylinder at (25, 12.5, 22.5) r 5 h 25, tangent to x = 20 along a line. The boolean
+//! keeps the tangency as an edge four faces share, in the right radial order; their
+//! binormals tie there, and `SmEdge::AssertValid` test 9 reads the order as out of
+//! sequence. With `edge_check_orders_tangent_faces` the check orders the faces by where
+//! they go and passes the edge.
+
+#![allow(
+    clippy::unwrap_used,
+    clippy::expect_used,
+    clippy::panic,
+    clippy::indexing_slicing
+)] // test helpers
+
+use solidcore::boolean::api_brep::boolean_difference;
+use solidcore::primitive::api_primitives::{create_box, create_cylinder};
+use solidcore::topology::brep::validation::{check_brep, CheckLevel};
+use solidcore::{Kernel, Vec3};
+
+/// The failed report lines of SMLib's check (level 0) of the cut.
+fn check_lines(fixed: bool) -> Vec<String> {
+    let mut k = Kernel::default();
+    k.fixes.edge_check_orders_tangent_faces = fixed;
+    let block = create_box(&mut k, Vec3::new(20.0, 5.0, 5.0), 15.0, 15.0, 27.5).unwrap();
+    let pin = create_cylinder(&mut k, Vec3::new(25.0, 12.5, 22.5), 5.0, 25.0).unwrap();
+    let mut cut = boolean_difference(&mut k, block, pin).unwrap();
+    let c = check_brep(&mut k, &mut cut, CheckLevel::Level0).unwrap();
+    c.lines()
+}
+
+#[test]
+fn wr_540_tangent_cylinder_edge_order_passes_with_the_fix() {
+    // the body is closed and only non-manifold along the tangency: the check passes it
+    let fixed = check_lines(true);
+    assert!(fixed.is_empty(), "{fixed:?}");
+}

plan/upstream-issues/WR-540--edge-check-tangent-faces-out-of-sequence.md

@@ -0,0 +1,9 @@
+#### WR-540. `SmEdge::AssertValid` test 9 refuses the valid radial order of faces tangent along an edge
+- **Severity:** Wrong result (a valid body refused: "Bad Sector Ang Sequence", offset -180, or a few 1e-6 degrees on tori)
+- **Area:** Validity checks
+- **Location:** `source/SMLib/src/SmEdge.cpp`, `SmEdge::AssertValid` (def. `:7478`), test 9 of the radial edge-use walk over manifold and spine edges: the faces round the edge are ordered by their binormals at the edge.
+- **What happens:** A boolean keeps the tangency of a cylinder or torus resting on a planar face as an edge that four faces share: the plane's two sides and the curved face's two halves. They are in the right radial order, and the body is closed and only non-manifold there. Tangent faces leave the edge in the same direction, so their binormals tie, or differ by rounding either way round. Test 9 then reads the right order as out of sequence.
+- **Trigger:** box (20, 5, 5)-(35, 20, 32.5) minus a z cylinder at (25, 12.5, 22.5) r 5 h 25, which rests on x = 20. These are the seeds of tangency-labs/solidcore#383 (1, 4, 32, 36, 39, 65, 74, 85, 86): boxes minus or with cylinders and tori tangent to a face.
+- **How found:** tangency-labs/solidcore#383. The fork's fix is tangency-labs/smlib@5533bbb ("the check orders faces round a tangent edge by where they go"), which is not merged upstream.
+- **Confirmed:** no C++ run was made here. The Rust port's `radial_order` follows the pinned C++ (`3fd5b95`).
+- **Rust port:** behaves as upstream by default. With `Kernel::fixes.edge_check_orders_tangent_faces`, `radial_order` first walks the sectors. If any two faces' unit binormals at the test parameter are within `SM_ANG_TOL_DEG` (dot above its cosine), every face is then measured by the direction from the edge to its point 20 edge tolerances into the face (`EvaluateBinormalStepOff`, `sector_direction`), square to the edge. A failed step or unitisation keeps the binormal. `tests/wr_540_tangent_cylinder_edge_order.rs` checks that, with the fix, SMLib's check passes the box minus the resting cylinder.