STEP export: unique ids, plain curves, right revolution senses; seams on split faces

Vendored truck-stepio (writer only): an intersection curve's second surface
was written at the first one's id (duplicate ids for every boolean edge),
and revolved surfaces lost their orientation (the modeling-surface sense was
always true; inverted or mirrored revolutions came out inside out). Export
now writes intersection edges as C1 cubic B-splines through exact points,
so files carry plain 3D curves and no redundant surfaces.

Vendored truck-meshalgo: hinted parameter searches that run off a B-spline's
domain are rejected or clamped (meshing their grid never finished).

STEP import: straight revolution profiles are bounded in the meridian plane
and stop at the axis; loops on revolutions start away from the axis; face
extents include edge end points. Edges between pieces of one split surface
draw as seams.

kernel::step_validate checks a STEP file (duplicate ids, dangling
references, dense numbering); the oracle replay now exports every rebuilt
part, validates it and re-imports it with no warnings and the same volume
(all 32 rebuildable cases pass).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01AS5Tn3xieewUSg3uxrMVdL

crates/kernel/src/lib.rs

@@ -36,7 +36,7 @@ pub use ops::{BoolOp, boolean, split_by_plane, transform, transform_matrix};
 pub use polybool::planar_boolean;
 pub use polyhedron::{HalfSpace, convex_polyhedron, draft, offset_faces, shell};
 pub use step::step_export;
-pub use step_in::{ImportedBody, StepImport, StepNode, step_import, step_import_shared};
+pub use step_in::{ImportedBody, StepImport, StepNode, step_import, step_import_shared, step_validate};
 pub use topo::{CylinderFace, cylinder_face_at};
 pub use topo::{TopoCounts, merged_topology, seam_flags};
 

crates/kernel/src/step.rs

@@ -1,8 +1,67 @@
+use truck_modeling as mt;
 use truck_stepio::out;
 
 use crate::body::Body;
 use crate::{KernelError, Result, guard};
 
+/// Intersection-curve edges (from booleans) as cubic B-splines through the exact curve: STEP
+/// readers get plain 3D geometry that lies on both faces, and the file does not repeat the two
+/// surfaces for every such edge.
+fn plain_curves(b: &Body) -> mt::Solid {
+    let s = b.deep_copy();
+    let size = b.size();
+    let mut seen = std::collections::HashSet::new();
+    for e in s.edge_iter() {
+        if !seen.insert(e.id()) {
+            continue;
+        }
+        if let mt::Curve::IntersectionCurve(ic) = e.curve()
+            && let Some(c) = hermite_through(&ic, size * 1e-4)
+        {
+            e.set_curve(mt::Curve::BSplineCurve(c));
+        }
+    }
+    s
+}
+
+/// A C¹ cubic B-spline through exact points of `c` spaced so that chords deviate at most `tol`
+/// (Catmull–Rom tangents over chord length; the cubic is far closer than the chords).
+fn hermite_through<C>(c: &C, tol: f64) -> Option<mt::BSplineCurve<mt::Point3>>
+where
+    C: mt::ParametricCurve<Point = mt::Point3, Vector = mt::Vector3> + mt::BoundedCurve + mt::ParameterDivision1D<Point = mt::Point3>,
+{
+    use mt::{InnerSpace, MetricSpace};
+    let (_, pts) = c.parameter_division(c.range_tuple(), tol);
+    let pts: Vec<mt::Point3> = pts.into_iter().fold(Vec::new(), |mut v: Vec<mt::Point3>, p| {
+        if v.last().is_none_or(|q| q.distance(p) > tol * 1e-3) {
+            v.push(p);
+        }
+        v
+    });
+    let n = pts.len().checked_sub(1).filter(|n| *n >= 1 && *n < 1_000_000)?;
+    let mut u = vec![0.0; n + 1];
+    for i in 1..=n {
+        u[i] = u[i - 1] + pts[i].distance(pts[i - 1]);
+    }
+    let tangent = |i: usize| {
+        let (a, b) = (i.saturating_sub(1), (i + 1).min(n));
+        (pts[b] - pts[a]) / (u[b] - u[a])
+    };
+    let mut ctrl = vec![pts[0]];
+    let mut knots = vec![u[0]; 4];
+    for i in 0..n {
+        let h = u[i + 1] - u[i];
+        ctrl.push(pts[i] + tangent(i) * (h / 3.0));
+        ctrl.push(pts[i + 1] - tangent(i + 1) * (h / 3.0));
+        ctrl.push(pts[i + 1]);
+        knots.extend(std::iter::repeat_n(u[i + 1], if i + 1 == n { 4 } else { 3 }));
+    }
+    if ctrl.iter().any(|p| !(p.x.is_finite() && p.y.is_finite() && p.z.is_finite())) || tangent(0).magnitude() == 0.0 {
+        return None;
+    }
+    mt::BSplineCurve::try_new(mt::KnotVec::from(knots), ctrl).ok()
+}
+
 /// STEP (ISO 10303-21) text for the given bodies.
 pub fn step_export(bodies: &[&Body], system: &str) -> Result<String> {
     if bodies.is_empty() {
@@ -12,7 +71,7 @@ pub fn step_export(bodies: &[&Body], system: &str) -> Result<String> {
         b.require_brep("STEP export")?;
     }
     guard("step export", || {
-        let cs: Vec<_> = bodies.iter().map(|b| b.solid.compress()).collect();
+        let cs: Vec<_> = bodies.iter().map(|b| plain_curves(b).compress()).collect();
         let models: out::StepModels<_, _, _> = cs.iter().collect();
         let header = out::StepHeaderDescriptor { organization_system: system.to_owned(), ..Default::default() };
         Ok(out::CompleteStepDisplay::new(models, header).to_string())

crates/kernel/src/step_in/brep.rs

@@ -209,7 +209,7 @@ impl Builder {
             .flat_map(|e| {
                 let c = e.curve();
                 let (t0, t1) = c.range_tuple();
-                (0..8).map(move |i| c.subs(t0 + (t1 - t0) * i as f64 / 8.0)).collect::<Vec<_>>()
+                (0..=8).map(move |i| c.subs(t0 + (t1 - t0) * i as f64 / 8.0)).collect::<Vec<_>>()
             })
             .collect();
         // Seam edges: used twice by the face's loops.
@@ -236,6 +236,27 @@ impl Builder {
             guard("step surface", || geom::surface(cx, sid, &extent, &seam, sense, 0).map_err(crate::KernelError::Failed))
                 .map_err(|e| e.to_string())?
         };
+        // Loops on a surface of revolution start away from the axis: a pole (cone apex, sphere
+        // pole) has no definite rotation parameter, and the mesher takes the first point's
+        // parameter as the anchor for the rest of the loop.
+        let wires: Vec<mt::Wire> = match &surface {
+            mt::Surface::RevolutedCurve(rc) => {
+                let (o, a) = (rc.entity().origin(), rc.entity().axis());
+                let dist = |e: &mt::Edge| {
+                    let v = e.front().point() - o;
+                    (v - a * v.dot(a)).magnitude()
+                };
+                wires
+                    .into_iter()
+                    .map(|w| {
+                        let edges: Vec<mt::Edge> = w.edge_iter().cloned().collect();
+                        let start = edges.iter().enumerate().max_by(|x, y| dist(x.1).total_cmp(&dist(y.1))).map(|(i, _)| i).unwrap_or(0);
+                        edges.iter().skip(start).chain(edges.iter().take(start)).cloned().collect::<Vec<_>>().into()
+                    })
+                    .collect()
+            }
+            _ => wires,
+        };
         match mt::Face::try_new(wires.clone(), surface.clone()) {
             Ok(f) => Ok(f),
             // Faces on periodic surfaces run along a seam edge twice: valid in STEP.

crates/kernel/src/step_in/geom.rs

@@ -611,6 +611,55 @@ fn tube_angle(f: &Frame, big: f64, p: mt::Point3) -> f64 {
     z.atan2(rho - big)
 }
 
+/// A straight profile of a surface of revolution, bounded by the face: boundary points are
+/// turned into the profile's meridian plane and projected onto the line, and the line stops at
+/// the axis (beyond it the surface would fold over itself).
+fn revolved_line(p: mt::Point3, d: mt::Vector3, o: mt::Point3, axis: mt::Vector3, extent: &[mt::Point3]) -> R<mt::Curve> {
+    let radial = |q: mt::Point3| {
+        let v = q - o;
+        v - axis * v.dot(axis)
+    };
+    // The meridian half-plane of the profile.
+    let e = {
+        let r = radial(p);
+        let r2 = radial(p + d);
+        let r = if r.magnitude() >= r2.magnitude() { r } else { r2 };
+        if r.magnitude() < 1e-12 {
+            return Err("profile line on the axis".into());
+        }
+        r / r.magnitude()
+    };
+    let (mut lo, mut hi) = (f64::INFINITY, f64::NEG_INFINITY);
+    for q in extent {
+        let v = q - o;
+        let h = v.dot(axis);
+        let r = radial(*q).magnitude();
+        let t = (o + axis * h + e * r - p).dot(d);
+        lo = lo.min(t);
+        hi = hi.max(t);
+    }
+    if !(lo.is_finite() && hi.is_finite()) {
+        return Err("surface of revolution without extent".into());
+    }
+    let m = (hi - lo) * 0.02 + 1e-6;
+    let (mut lo, mut hi) = (lo - m, hi + m);
+    // Where the line meets the axis (radial component along e vanishes).
+    let de = d.dot(e);
+    if de.abs() > 1e-12 {
+        let t_axis = -(p - o).dot(e) / de;
+        // The meridian half-plane has a non-negative radius along `e`.
+        if de > 0.0 {
+            lo = lo.max(t_axis);
+        } else {
+            hi = hi.min(t_axis);
+        }
+    }
+    if hi - lo <= 1e-12 {
+        return Err("degenerate surface of revolution".into());
+    }
+    Ok(mt::Curve::Line(mt::Line(p + d * lo, p + d * hi)))
+}
+
 fn revolved(curve: mt::Curve, o: mt::Point3, axis: mt::Vector3) -> mt::Surface {
     mt::Surface::RevolutedCurve(mt::Processor::new(mt::RevolutedCurve::by_revolution(curve, o, axis)))
 }
@@ -802,8 +851,12 @@ pub(crate) fn surface(cx: &Ctx, id: u64, extent: &[mt::Point3], seam: &[mt::Poin
         "SURFACE_OF_REVOLUTION" => {
             let g = curve(cx, p.get(1).and_then(Param::as_ref_id).ok_or("revolution without profile")?, depth + 1)?;
             let (o, axis) = axis1(cx, p.get(2).and_then(Param::as_ref_id).ok_or("revolution without axis")?)?;
+            let profile = match &g {
+                CurveGeo::Line { p, d } => revolved_line(*p, *d, o, axis, extent)?,
+                _ => bounded_curve(&g, extent)?,
+            };
             // Reversed sense: turn the other way round the axis.
-            Ok(revolved(bounded_curve(&g, extent)?, o, axis * sign))
+            Ok(revolved(profile, o, axis * sign))
         }
         "SURFACE_OF_LINEAR_EXTRUSION" => {
             let g = curve(cx, p.get(1).and_then(Param::as_ref_id).ok_or("extrusion without profile")?, depth + 1)?;

crates/kernel/src/step_in/mod.rs

@@ -576,6 +576,38 @@ impl<'a> Reader<'a> {
     }
 }
 
+/// Check a STEP file's structure: it parses, no entity id is defined twice, every reference
+/// names a defined entity, and ids are dense (`#1`…`#n`). Returns the entity count.
+pub fn step_validate(text: &str) -> std::result::Result<usize, String> {
+    let ex = p21::parse(text)?;
+    if let Some(d) = ex.duplicates.first() {
+        return Err(format!("{} duplicate entity ids (first #{d})", ex.duplicates.len()));
+    }
+    fn refs(p: &Param, out: &mut Vec<u64>) {
+        match p {
+            Param::Ref(r) => out.push(*r),
+            Param::List(v) | Param::Typed(_, v) => v.iter().for_each(|x| refs(x, out)),
+            _ => {}
+        }
+    }
+    let mut ids: Vec<&u64> = ex.entities.keys().collect();
+    ids.sort();
+    for id in &ids {
+        let mut out = Vec::new();
+        if let Some(e) = ex.get(**id) {
+            e.records.iter().flat_map(|r| &r.params).for_each(|p| refs(p, &mut out));
+        }
+        if let Some(r) = out.iter().find(|r| ex.get(**r).is_none()) {
+            return Err(format!("#{id} refers to undefined #{r}"));
+        }
+    }
+    let n = ex.entities.len();
+    if ids.last().is_some_and(|m| **m != n as u64) || ids.first().is_some_and(|m| **m != 1) {
+        return Err(format!("entity ids are not dense: {n} entities, #{}…#{}", ids.first().map_or(0, |x| **x), ids.last().map_or(0, |x| **x)));
+    }
+    Ok(n)
+}
+
 /// Recent imports, so a file read for a command and then evaluated in the timeline is only
 /// read once.
 static RECENT: std::sync::Mutex<Vec<(u64, usize, std::sync::Arc<StepImport>)>> = std::sync::Mutex::new(Vec::new());

crates/kernel/src/step_in/tests.rs

@@ -10,10 +10,10 @@ fn rel(a: f64, b: f64) -> f64 {
 /// Export → import → same volume, area and face count.
 fn round_trip(b: &Body) -> StepImport {
     let text = step_export(&[b], "test").unwrap();
+    step_validate(&text).unwrap();
     let imp = step_import(&text).unwrap();
     assert_eq!(imp.bodies.len(), 1, "{:?}", imp.warnings);
-    // truck's writer repeats entity ids for the surfaces of intersection curves (unused here).
-    assert!(imp.warnings.iter().all(|w| w.contains("defined twice")), "{:?}", imp.warnings);
+    assert!(imp.warnings.is_empty(), "{:?}", imp.warnings);
     let (m0, m1) = (measure(b).unwrap(), measure(&imp.bodies[0].body).unwrap());
     assert!(rel(m1.volume, m0.volume) < 1e-3, "volume {} vs {}", m1.volume, m0.volume);
     assert!(rel(m1.area, m0.area) < 1e-3, "area {} vs {}", m1.area, m0.area);
@@ -306,3 +306,61 @@ fn exponential_assemblies_are_bounded() {
     assert!(!imp.bodies.is_empty() && imp.bodies.len() <= MAX_BODIES);
     assert!(imp.warnings.iter().any(|w| w.contains("left out")), "{:?}", imp.warnings);
 }
+
+#[test]
+fn validator_catches_broken_files() {
+    assert!(step_validate(&format!("{HEAD}#1=A(#2);\n#2=B();\n{TAIL}")).is_ok());
+    assert!(step_validate(&format!("{HEAD}#1=A(#2);\n#2=B();\n#2=C();\n{TAIL}")).unwrap_err().contains("duplicate"));
+    assert!(step_validate(&format!("{HEAD}#1=A((#7));\n{TAIL}")).unwrap_err().contains("undefined"));
+    assert!(step_validate(&format!("{HEAD}#1=A();\n#5=B();\n{TAIL}")).unwrap_err().contains("dense"));
+}
+
+#[test]
+fn split_band_edges_are_seams() {
+    // A quarter pipe (torus R 10, r 3) whose face wraps around the tube with a seam edge: it is
+    // split for the kernel, and the split edge is drawn as a seam.
+    let mut t = String::from(HEAD);
+    t += "#1=CARTESIAN_POINT('',(0.,0.,0.));\n#2=DIRECTION('',(0.,0.,1.));\n#3=DIRECTION('',(1.,0.,0.));\n#4=AXIS2_PLACEMENT_3D('',#1,#2,#3);\n";
+    t += "#5=TOROIDAL_SURFACE('',#4,10.,3.);\n";
+    t += "#10=CARTESIAN_POINT('',(13.,0.,0.));\n#11=CARTESIAN_POINT('',(0.,13.,0.));\n#12=VERTEX_POINT('',#10);\n#13=VERTEX_POINT('',#11);\n";
+    // Tube circles at azimuth 0 (plane XZ) and 90 degrees (plane YZ), the seam around Z.
+    t += "#20=CARTESIAN_POINT('',(10.,0.,0.));\n#21=DIRECTION('',(0.,-1.,0.));\n#22=AXIS2_PLACEMENT_3D('',#20,#21,#3);\n#23=CIRCLE('',#22,3.);\n";
+    t += "#24=CARTESIAN_POINT('',(0.,10.,0.));\n#25=DIRECTION('',(1.,0.,0.));\n#26=DIRECTION('',(0.,1.,0.));\n#27=AXIS2_PLACEMENT_3D('',#24,#25,#26);\n#28=CIRCLE('',#27,3.);\n";
+    t += "#29=CIRCLE('',#4,13.);\n";
+    t += "#30=EDGE_CURVE('',#12,#12,#23,.T.);\n#31=EDGE_CURVE('',#13,#13,#28,.T.);\n#32=EDGE_CURVE('',#12,#13,#29,.T.);\n";
+    t += "#40=ORIENTED_EDGE('',*,*,#30,.T.);\n#41=ORIENTED_EDGE('',*,*,#32,.T.);\n#42=ORIENTED_EDGE('',*,*,#31,.F.);\n#43=ORIENTED_EDGE('',*,*,#32,.F.);\n";
+    t += "#44=EDGE_LOOP('',(#40,#41,#42,#43));\n#45=FACE_OUTER_BOUND('',#44,.T.);\n#46=ADVANCED_FACE('',(#45),#5,.F.);\n";
+    t += "#47=PLANE('',#22);\n#48=ORIENTED_EDGE('',*,*,#30,.F.);\n#49=EDGE_LOOP('',(#48));\n#50=FACE_OUTER_BOUND('',#49,.T.);\n#51=ADVANCED_FACE('',(#50),#47,.T.);\n";
+    t += "#52=PLANE('',#27);\n#53=ORIENTED_EDGE('',*,*,#31,.T.);\n#54=EDGE_LOOP('',(#53));\n#55=FACE_OUTER_BOUND('',#54,.T.);\n#56=ADVANCED_FACE('',(#55),#52,.T.);\n";
+    t += "#60=CLOSED_SHELL('',(#46,#51,#56));\n#61=MANIFOLD_SOLID_BREP('Pipe',#60);\n#62=SHAPE_REPRESENTATION('',(#61),$);\n";
+    t += TAIL;
+    let imp = step_import(&t).unwrap();
+    let ib = &imp.bodies[0];
+    assert_eq!(ib.file_faces, 3);
+    assert_eq!(ib.body.face_count(), 4, "{:?}", imp.warnings);
+    // The two halves of the tube face make a quarter torus: area π²·R·r.
+    let curved: f64 = ib.body.faces(1e-3).unwrap().iter().filter(|f| f.plane_normal.is_none()).map(|f| f.area).sum();
+    let want = std::f64::consts::PI.powi(2) * 30.0;
+    assert!(rel(curved, want) < 2e-3, "{curved} vs {want}");
+    let m = ib.body.display_mesh(ib.body.size() * 1e-3).unwrap();
+    // Seams: the file's own seam (inside the tube face) and the split edge.
+    assert!(m.seams.iter().filter(|s| **s).count() >= 2, "{:?}", m.seams);
+}
+
+/// Revolved faces of a mirrored tool keep their orientation through STEP (the writer used to
+/// drop an inverted or mirrored revolution's sense).
+#[test]
+fn mirrored_revolved_faces_round_trip() {
+    for mirror in [false, true] {
+        let pl = Plane { origin: Vec3::new(20.0, 15.0, 0.0), ..Plane::XZ };
+        let prof =
+            Region2 { outer: Loop2::polygon(&[Vec2::new(0.0, 4.0), Vec2::new(3.0, 5.0), Vec2::new(3.0, 12.0), Vec2::new(0.0, 12.0)]), holes: vec![] };
+        let mut tool = revolve(&pl, &[prof], Vec2::ZERO, Vec2::Y, std::f64::consts::TAU).unwrap().pop().unwrap();
+        if mirror {
+            tool =
+                crate::transform_matrix(&tool, [[1.0, 0.0, 0.0, 0.0], [0.0, 1.0, 0.0, 0.0], [0.0, 0.0, -1.0, 0.0], [0.0, 0.0, 16.0, 1.0]]).unwrap();
+        }
+        let plate = box_solid(Vec3::ZERO, Vec3::new(40.0, 30.0, 10.0)).unwrap();
+        round_trip(&boolean(&plate, &tool, BoolOp::Cut).unwrap().unwrap());
+    }
+}

crates/kernel/src/topo.rs

@@ -239,6 +239,9 @@ pub fn seam_flags(b: &Body, mesh: &Mesh) -> Vec<bool> {
     let tol = (b.size() * 1e-4).max(1e-6);
     let surfs = classify_faces(mesh, nf, tol);
     let verts = face_vertices(mesh, nf);
+    // Faces cut from one surface (an imported face split where it wraps around a closed
+    // surface) carry the very same surface: the edge between them is a seam.
+    let surf_keys: Vec<String> = solid.face_iter().map(|f| format!("{:?}", f.surface())).collect();
     let mut faces_of: HashMap<String, Vec<usize>> = HashMap::new();
     let mut order: Vec<String> = Vec::new();
     for (fi, f) in solid.face_iter().enumerate() {
@@ -258,7 +261,8 @@ pub fn seam_flags(b: &Body, mesh: &Mesh) -> Vec<bool> {
         .enumerate()
         .map(|(ei, k)| match faces_of.get(k).map(Vec::as_slice) {
             Some([a, b2]) => {
-                !matches!(surfs.get(*a), Some(Surf::Plane { .. })) && same_surface(&surfs, &verts, mesh.edges.get(ei), *a, *b2, tol * 10.0)
+                (!matches!(surfs.get(*a), Some(Surf::Plane { .. })) && same_surface(&surfs, &verts, mesh.edges.get(ei), *a, *b2, tol * 10.0))
+                    || (surf_keys.get(*a).is_some_and(|k| Some(k) == surf_keys.get(*b2)) && !matches!(surfs.get(*a), Some(Surf::Plane { .. })))
             }
             _ => false,
         })