Merge pull request #1633 from tangency-labs/fork/b283ea2

fix: opt-in extended offset extends a cone or cylinder as itself (WR-532, fork b283ea2, 3626d4b)

crates/solidcore/src/kernel.rs

@@ -173,6 +173,8 @@ pub struct UpstreamFixes {
     pub revolution_offset_axis_both_sides: bool,
     /// With extended convex edges, `piecewise_merge` relates the mated offsets of a manifold edge instead of intersecting them again (WR-344, #469).
     pub offset_extended_edges_mated: bool,
+    /// The extended offset (`SmOffsetExecutive::GetCreateExtendedSurface`) extends a cone or cylinder as itself, along its rulings and around its axis with `SM_CT_CINFINITY` (around by no more than half the turn it leaves open on each side), where upstream extends every surface by a tangent extension (`SM_CT_G1R`) that leaves a cylinder at once; a face that reaches a hair into that extension is later fitted back into a cylinder off its radius (WR-532; the fork's tangency-labs/smlib@b283ea2).
+    pub offset_extends_cones_as_themselves: bool,
     /// A volumetric Boolean removes the zero-area loops a contact curve inside a face leaves, and splits shells joined only through them (WR-360, #746).
     pub remove_slit_loops_where_solids_touch: bool,
     /// A face a Boolean takes over from its second operand keeps that operand's exact UV trims (WR-361, #488).

crates/solidcore/src/offset/offset_executive/p1.rs

@@ -90,7 +90,7 @@ use crate::heal::stitch::p2 as stitch;
 use crate::intersect::topology_solver::p1::TopologySolver;
 use crate::intersect::topology_solver::p2::TopologySolveOptions;
 use crate::kernel::Kernel;
-use crate::math::{Vec3, BIG_DOUBLE, EFF_ZERO, EFF_ZERO_SQRT, PI};
+use crate::math::{min2, Vec3, BIG_DOUBLE, EFF_ZERO, EFF_ZERO_SQRT, PI};
 use crate::nurbs::b_spline_surface_n_lib::p1::OffsetParamType;
 use crate::nurbs::b_spline_surface_n_lib::p2::OrientType;
 use crate::nurbs::knots::Side;
@@ -180,6 +180,122 @@ pub struct VertexOffset {
     pub curve_orientations: Vec<OrientType>,
 }
 
+/// Not upstream (WR-532; the fork's tangency-labs/smlib@b283ea2, `sm_ExtendConeAsItself`):
+/// cone (or cylinder) `surface` extended by `dist` along its rulings and around its axis
+/// as itself, each direction's end spans carried on (`SM_CT_CINFINITY`), which stays on
+/// the circle where a tangent extension (`SM_CT_G1R`) leaves it. Around the axis it goes
+/// no farther than half the turn the cone leaves open on each side, so it never laps
+/// itself; a closed cone is extended along its rulings only. Around the axis each
+/// side is extended on its own and each old end made a knot of multiplicity equal to
+/// the degree again ([`full_knot`]; tangency-labs/smlib@3626d4b).
+///
+/// # Errors
+///
+/// Those of the extensions and knot insertions.
+fn extend_cone_as_itself(
+    kernel: &Kernel,
+    surface: &Surface,
+    cone: &crate::surface::cone::Cone,
+    dist: f64,
+) -> Result<Surface> {
+    use crate::nurbs::b_spline_surface::SurfParamType;
+    let (around, along) = if cone.rev().swap_uv() {
+        (SurfParamType::V, SurfParamType::U)
+    } else {
+        (SurfParamType::U, SurfParamType::V)
+    };
+    let along_srf =
+        surface.create_extended_surface_dir(kernel, along, dist, ContinuityType::CInfinity)?;
+
+    // around: half the open turn on each side, in arc length on the smaller end circle
+    let sweep_rad = cone.rev().step_uv_domain().u_interval().length() * PI / 180.0;
+    let radius = min2(cone.bot_radius(), cone.top_radius());
+    let open = 2.0 * PI - sweep_rad;
+    let around_dist = min2(dist, 0.5 * 0.5 * open * radius);
+    // (`SM_CAST_PTR(SmBSplineSurface, ..)`: the B-spline surface and its subclasses)
+    let is_bspline = !matches!(
+        along_srf.kind(),
+        SurfaceKind::OffsetSurface(_)
+            | SurfaceKind::CurveBounded(_)
+            | SurfaceKind::Step(_)
+            | SurfaceKind::SrfInVolume(_)
+    );
+    if !is_bspline || open <= 0.0 || around_dist <= EFF_ZERO_SQRT * (1.0 + radius) {
+        return Ok(along_srf);
+    }
+    // (tangency-labs/smlib@3626d4b) each side on its own, each old end then a knot of
+    // multiplicity equal to the degree again: SMLib's own form for a circle
+    // (`SmCone::IsNurbSurfaceCone` takes only a triple knot at each end and double knots
+    // inside); both sides in one call extend the first side's extended span again and
+    // leave a single knot at the old end, and a face on that surface is no longer found
+    // to be a cone
+    let around_ivl = if around == SurfParamType::U {
+        along_srf.natural_uv_domain_with_nurb(kernel)?.u_interval()
+    } else {
+        along_srf.natural_uv_domain_with_nurb(kernel)?.v_interval()
+    };
+    let (min_side, max_side) = if around == SurfParamType::U {
+        (SurfParamType::UMin, SurfParamType::UMax)
+    } else {
+        (SurfParamType::VMin, SurfParamType::VMax)
+    };
+    let mut min_srf = along_srf.create_extended_surface_dir(
+        kernel,
+        min_side,
+        around_dist,
+        ContinuityType::CInfinity,
+    )?;
+    full_knot(&mut min_srf, around, around_ivl.min())?;
+    let mut both = min_srf.create_extended_surface_dir(
+        kernel,
+        max_side,
+        around_dist,
+        ContinuityType::CInfinity,
+    )?;
+    full_knot(&mut both, around, around_ivl.max())?;
+    Ok(both)
+}
+
+/// Not upstream (WR-532; tangency-labs/smlib@3626d4b): makes the knot of B-spline
+/// `surface` at `at` in direction `dir` of multiplicity equal to the degree
+/// (`InsertOneKnot` of the missing count; a knot within `EFF_ZERO` of the interval's
+/// length counts as the same).
+///
+/// # Errors
+///
+/// `SM_ERR_NULL_POINTER` for a surface that is not a B-spline surface (upstream's
+/// `NER` of the cast); those of the knot query and insertion.
+fn full_knot(
+    surface: &mut Surface,
+    dir: crate::nurbs::b_spline_surface::SurfParamType,
+    at: f64,
+) -> Result<()> {
+    use crate::nurbs::b_spline_surface::SurfParamType;
+    const OP: &str = "offset::offset_executive::p1::extend_cone_as_itself";
+    let SurfaceKind::BSpline(bs) = surface.kind_mut() else {
+        return Err(SolidError::null_pointer(OP));
+    };
+    let (knots, mults) = bs.knots(dir, None)?;
+    let domain = bs.natural_uv_domain();
+    let ivl = if dir == SurfParamType::U {
+        domain.u_interval()
+    } else {
+        domain.v_interval()
+    };
+    let same = EFF_ZERO * (1.0 + ivl.length());
+    let mut have = 0;
+    for (&knot, &mult) in knots.iter().zip(&mults) {
+        if (knot - at).abs() <= same {
+            have = mult;
+        }
+    }
+    let degree = bs.degree(dir);
+    if have < degree {
+        bs.insert_one_knot(at, degree - have, dir)?;
+    }
+    Ok(())
+}
+
 /// The 27-bit object namespace under which the lamina clean-up's `DeleteFaces`
 /// (merge p4's arena adapter) records its notifications; distinct from the Boolean
 /// kernels' namespaces, which share no attribute store with an offset executive.
@@ -613,11 +729,14 @@ impl OffsetExecutive {
             std::collections::btree_map::Entry::Vacant(entry) => {
                 // The obsolete offset-specific `else` at cpp:2158 is unreachable:
                 // NER(pBase) already established pOrigSurf == pBase != NULL.
-                let extension = base.create_extended_surface(
-                    kernel,
-                    self.offset_distance * 5.0,
-                    ContinuityType::G1R,
-                )?;
+                let dist = self.offset_distance * 5.0;
+                let extension = match base.kind() {
+                    // Not upstream (WR-532), opt-in `offset_extends_cones_as_themselves`
+                    SurfaceKind::Cone(cone) if kernel.fixes.offset_extends_cones_as_themselves => {
+                        extend_cone_as_itself(kernel, base, cone, dist)?
+                    }
+                    _ => base.create_extended_surface(kernel, dist, ContinuityType::G1R)?,
+                };
                 Ok(entry.insert(extension))
             }
         }

crates/solidcore/src/surface/surface/approximate.rs

@@ -17,6 +17,7 @@
 //! | `root_surface` (private)                | `GetRootSurface` (virtual)                   |
 //! | [`BSplineSurface::degree_elevate`]      | `SmBSplineSurface::DegreeElevate`            |
 //! | [`BSplineSurface::insert_knots`]        | `SmBSplineSurface::InsertKnots` (NLib form)  |
+//! | [`BSplineSurface::insert_one_knot`]     | `SmBSplineSurface::InsertOneKnot`            |
 //! | [`BSplineSurface::remove_knots`]        | `SmBSplineSurface::RemoveKnots`              |
 //!
 //! None of these is in `harness/callgraph/units.json`: they are listed in
@@ -73,7 +74,7 @@ use crate::nurbs::b_spline_surface::{
 use crate::nurbs::b_spline_surface_n_lib::p2::EvaluateFlags;
 use crate::nurbs::knots::Side;
 use crate::nurbs::srf_basic::Direction;
-use crate::nurbs::srf_tool::p1::insert_knots;
+use crate::nurbs::srf_tool::p1::{insert_knot, insert_knots};
 use crate::nurbs::srf_tool::p2::elevate_degree;
 use crate::nurbs::srf_tool::p3::remove_all_knots;
 use crate::nurbs::srf_tool::p4::remove_knots_constrained;
@@ -85,6 +86,7 @@ const OP_BEZIER: &str = "surface::surface::approximate::compute_cubic_bezier_app
 const OP_ROOT: &str = "surface::surface::approximate::root_surface";
 const OP_ELEVATE: &str = "surface::surface::approximate::degree_elevate";
 const OP_INSERT: &str = "surface::surface::approximate::insert_knots";
+const OP_INSERT_ONE: &str = "surface::surface::approximate::insert_one_knot";
 const OP_REMOVE: &str = "surface::surface::approximate::remove_knots";
 
 /// A bicubic Bezier patch (`SmCubicBezierSurface`): its 4x4 control points,
@@ -740,6 +742,37 @@ impl BSplineSurface {
         Ok(())
     }
 
+    /// Inserts the interior knot `new_knot` `count` times in `param`'s direction (v for
+    /// [`SurfParamType::V`], else u), its old and new multiplicities summing to at most
+    /// the degree (`InsertOneKnot`).
+    ///
+    /// # Errors
+    ///
+    /// `NULL` pointer without NURB; those of the NLib insertion (the surface is then
+    /// unchanged).
+    pub fn insert_one_knot(
+        &mut self,
+        new_knot: f64,
+        count: usize,
+        param: SurfParamType,
+    ) -> Result<()> {
+        const OP: &str = OP_INSERT_ONE;
+        let dir = if param == SurfParamType::V {
+            Direction::V
+        } else {
+            Direction::U
+        };
+        let sur_p = self
+            .nurb
+            .as_ref()
+            .ok_or_else(|| SolidError::null_pointer(OP))?;
+        let sur = insert_knot(sur_p, new_knot, count, dir).map_err(SolidError::nl_ser(OP))?;
+
+        // build a surface from it and swap the NURBs
+        self.nurb = Self::from_gw_nurb(&sur).nurb;
+        Ok(())
+    }
+
     /// Removes the knots whose removal keeps the surface within `approx_tol_3d`, in u
     /// and/or v (both when neither is asked for), except the knots in `kept_u_knots` /
     /// `kept_v_knots` (`RemoveKnots`).

crates/solidcore/tests/wr_532_shell_rounds_stay_on_radius.rs

@@ -0,0 +1,102 @@
+//! Regression test for WR-532 (the fork's tangency-labs/smlib@b283ea2, capi test
+//! `shelled_rounded_box_rim`): a 60 x 40 x 20 box with its four vertical edges rounded
+//! r6, shelled 2 mm inward open at the top. The cavity's rounds are the r4 offsets of
+//! the outer ones. With `offset_extends_cones_as_themselves` the extended offset
+//! extends the rounds' cylinders as themselves, so the rim faces that reach a hair
+//! past the rounds' ends stay on the circle, and every cylinder of the result has the
+//! radius of its round to 1e-12. A single r5 round comes back a cylinder of radius 3
+//! (tangency-labs/smlib@3626d4b, capi `shelled_single_round_stays_a_cylinder`).
+#![allow(
+    clippy::unwrap_used,
+    clippy::expect_used,
+    clippy::panic,
+    clippy::indexing_slicing
+)] // test helpers
+
+#[path = "support/offset_fixture.rs"]
+mod offset_fixture;
+
+use offset_fixture::{planar_faces_at, shell};
+use solidcore::blend::fillet_executive::api::fillet_edges;
+use solidcore::math::Point3;
+use solidcore::primitive::api_primitives::create_box;
+use solidcore::surface::surface::SurfaceKind;
+use solidcore::Kernel;
+
+/// A 60 x 40 x 20 box with `count` of its vertical edges rounded `radius`, shelled 2 mm
+/// inward open at the top with the opt-in: the radii of the result's cylinder faces.
+fn shelled_round_radii(count: usize, radius: f64) -> Vec<f64> {
+    let mut k = Kernel::default();
+    k.fixes.offset_extends_cones_as_themselves = true;
+    let mut b = create_box(&mut k, Point3::new(0.0, 0.0, 0.0), 60.0, 40.0, 20.0).unwrap();
+    let vertical: Vec<_> = b
+        .edges()
+        .iter()
+        .filter(|(_, e)| {
+            let c = e.curve.as_ref().unwrap();
+            let p = c.evaluate_point(&k, e.interval.min()).unwrap();
+            let q = c.evaluate_point(&k, e.interval.max()).unwrap();
+            (p.x - q.x).abs() < 1e-9 && (p.y - q.y).abs() < 1e-9
+        })
+        .map(|(id, _)| id)
+        .take(count)
+        .collect();
+    assert_eq!(vertical.len(), count, "the vertical edges");
+    fillet_edges(&mut k, &mut b, &vertical, radius, 1, 1, 1.0).unwrap();
+    b.refresh_cached_topology();
+    let top = planar_faces_at(&k, &b, 2, 20.0);
+    assert_eq!(top.len(), 1, "the top face");
+    let result = shell(&mut k, b, -2.0, &top).unwrap();
+    let mut radii = Vec::new();
+    for &f in result.face_list() {
+        if let Some(SurfaceKind::Cone(cone)) = result
+            .face(f)
+            .unwrap()
+            .surface
+            .as_ref()
+            .map(solidcore::surface::surface::Surface::kind)
+        {
+            radii.push(cone.bot_radius());
+        }
+    }
+    radii
+}
+
+/// All four vertical edges rounded r6: the cavity's four r4 rounds, and the r6 ones
+/// where the merge made them cylinders too, on their radii to 1e-12.
+#[test]
+fn wr_532_shelled_rounds_keep_their_radii() {
+    let radii = shelled_round_radii(4, 6.0);
+    let inner: Vec<f64> = radii
+        .iter()
+        .copied()
+        .filter(|r| (r - 4.0).abs() < 1e-3)
+        .collect();
+    assert_eq!(inner.len(), 4, "the cavity's four rounds: {radii:?}");
+    for r in radii
+        .into_iter()
+        .filter(|r| (r - 4.0).abs() < 1e-3 || (r - 6.0).abs() < 1e-3)
+    {
+        let want = if r > 5.0 { 6.0 } else { 4.0 };
+        assert!(
+            (r - want).abs() <= 1e-12,
+            "a round of radius {r}, want {want}"
+        );
+    }
+}
+
+/// tangency-labs/smlib@3626d4b (capi `shelled_single_round_stays_a_cylinder`): one
+/// vertical edge rounded r5, a round in one rational span. Extended round its axis one
+/// side at a time with SMLib's circle knots kept, the cavity's round is found to be a
+/// cylinder again, of radius 3.
+#[test]
+fn wr_532_single_round_stays_a_cylinder() {
+    let radii = shelled_round_radii(1, 5.0);
+    let inner: Vec<f64> = radii
+        .iter()
+        .copied()
+        .filter(|r| (r - 3.0).abs() < 1e-3)
+        .collect();
+    assert_eq!(inner.len(), 1, "the cavity's round: {radii:?}");
+    assert!((inner[0] - 3.0).abs() <= 1e-12, "radius {}", inner[0]);
+}

plan/upstream-issues/WR-532--extended-offset-tangent-extension-leaves-a-cylinder.md

@@ -0,0 +1,13 @@
+#### WR-532. The extended offset extends a cylinder by a tangent extension that leaves it, and a face reaching into it is fitted back off radius
+- **Severity:** Wrong result (a re-derived cylinder 3.2e-10 off its radius, by an amount that depends on the platform's rounding; arcs built on the exact offset miss it)
+- **Area:** Offsets and shells
+- **Location:** `source/SMLib/src/SmOffsetExecutive.cpp`, `SmOffsetExecutive::GetCreateExtendedSurface` (def. `:2237`): every surface is extended with `CreateExtendedSurface(.., SM_CT_G1R, ..)`.
+- **What happens:** A tangent extension (`SM_CT_G1R`) of a cylinder leaves the circle at once: 10 mm round an R4 offset, it is 6.8 mm off. A shell's rim faces reach a hair (5e-5 mm) past the offset rounds' ends into that extension. `SmMerge::MakeFaceuse` then turns each face's surface back into an analytic one (`CopyAndAddAnalytics`, `IsNurbSurfaceCone`). The fit comes out a cylinder 3.2e-10 off R4, and the rim's arcs, built on the rounds' exact offsets, miss it by 3.75e-10 on some platforms.
+- **Trigger:** a box with rounded edges shelled inward (the fork's capi `shelled_rounded_box_rim`).
+- **How found:** reported by SolidCraft. The fork's fix is tangency-labs/smlib@b283ea2 ("offset: a cone or cylinder is extended as itself"), refined by tangency-labs/smlib@3626d4b ("a cylinder extended round its axis keeps SMLib's circle knots and comes back a cone"). Neither is merged upstream.
+- **Confirmed:** no C++ run was made here.
+- **Rust port:** behaves as upstream by default. With `Kernel::fixes.offset_extends_cones_as_themselves`, `OffsetExecutive::get_create_extended_surface` extends a cone or cylinder as itself (`extend_cone_as_itself`, the fork's `sm_ExtendConeAsItself`):
+  - along its rulings with `SM_CT_CINFINITY`;
+  - then around its axis with `SM_CT_CINFINITY`, by at most half the turn it leaves open on each side, measured as arc length on its smaller end circle (none for a closed cone). Each side is extended on its own, and each old end is then made a knot of multiplicity equal to the degree again (`full_knot`, `BSplineSurface::insert_one_knot`). That is SMLib's own form for a circle: extending both sides in one call leaves a single knot at the old end, and `IsNurbSurfaceCone` then no longer finds a cone.
+
+  `tests/wr_532_shell_rounds_stay_on_radius.rs` shells a 60 x 40 x 20 box whose vertical edges are rounded r6, 2 mm inward and open at the top. It holds the cavity's four r4 cylinders (and any r6 ones) to 1e-12. A second test rounds a single edge r5 and checks that the cavity's round is a cylinder of radius 3.