STEP export: a merged partial cylinder keeps its cylinder (silent volume loss) (#1497)

Thanks @Ian-cmd-ops.

crates/engine/tests/step_merge_regressions.rs

@@ -0,0 +1,80 @@
+//! STEP export regressions found by round-tripping the robustness designs: faces the writer
+//! merges must keep their whole surface, so the body reads back as the same solid.
+
+use serde_json::{Value, json};
+use solidcraft_engine::Session;
+use solidcraft_kernel::{measure, step_export, step_import};
+
+fn run(s: &mut Session, script: Value) {
+    for c in script.as_array().into_iter().flatten() {
+        let (id, p) = (c["command"].as_str().unwrap_or_default(), &c["params"]);
+        if let Err(e) = s.execute(id, p) {
+            panic!("{id} {p}: {e}");
+        }
+    }
+}
+
+/// The design's one body exports, and reads back closed with the same volume and area.
+fn round_trips(s: &Session) {
+    let st = s.model.state();
+    assert_eq!(st.bodies.len(), 1);
+    let body = &st.bodies[0].body;
+    let text = step_export(&[body], "test").unwrap_or_else(|e| panic!("export: {e}"));
+    let imp = step_import(&text).unwrap_or_else(|e| panic!("import: {e}"));
+    assert_eq!(imp.bodies.len(), 1, "{:?}", imp.warnings);
+    assert!(imp.bodies[0].closed && imp.warnings.is_empty(), "{:?}", imp.warnings);
+    let (a, b) = (measure(body).unwrap(), measure(&imp.bodies[0].body).unwrap());
+    assert!((a.volume - b.volume).abs() < 1e-3 * a.volume, "volume {} -> {}", a.volume, b.volume);
+    assert!((a.area - b.area).abs() < 1e-3 * a.area, "area {} -> {}", a.area, b.area);
+}
+
+/// Robustness seed 17, shrunk: a thin three-quarter tube shelled 1 mm with its bottom open. The
+/// export merged each wall's three quarter pieces into one face but kept the first piece's
+/// quarter sheet as its surface; the body read back closed, with no warning, 7% too small.
+#[test]
+fn shelled_thin_three_quarter_tube_keeps_its_volume() {
+    let mut s = Session::default();
+    run(
+        &mut s,
+        json!([
+            {"command":"sketch.create","params":{"plane":"XZ"}},
+            {"command":"sketch.rectangle.two_point","params":{"p0":[10.0,0],"p1":[12.5,27.5]}},
+            {"command":"sketch.finish","params":{}},
+            {"command":"solid.revolve","params":{"angle":"270 deg","axis":"Z"}},
+            {"command":"solid.shell","params":{"faces":[[-7.182422893157272,7.182422893245109,0.0]],"thickness":1.0}}
+        ]),
+    );
+    round_trips(&s);
+}
+
+/// The same tube turned further: the merged wall leaves a gap of 60°, 30° or 10°. Its two straight
+/// end edges lie either side of the gap, so the mean direction of the face's points no longer
+/// points into the face, and a seam put opposite it falls inside the face. The seam belongs in the
+/// middle of the gap.
+#[test]
+fn shelled_thin_wide_tube_keeps_its_volume() {
+    for turn in [300.0_f64, 330.0, 350.0] {
+        let mid = (turn / 2.0).to_radians();
+        let mut s = Session::default();
+        run(
+            &mut s,
+            json!([
+                {"command":"sketch.create","params":{"plane":"XZ"}},
+                {"command":"sketch.rectangle.two_point","params":{"p0":[10.0,0],"p1":[12.5,27.5]}},
+                {"command":"sketch.finish","params":{}},
+                {"command":"solid.revolve","params":{"angle":format!("{turn} deg"),"axis":"Z"}},
+                {"command":"solid.shell","params":{"faces":[[11.0 * mid.cos(), 11.0 * mid.sin(), 0.0]],"thickness":1.0}}
+            ]),
+        );
+        let st = s.model.state();
+        assert_eq!(st.bodies.len(), 1, "{turn} deg");
+        let body = &st.bodies[0].body;
+        let text = step_export(&[body], "test").unwrap_or_else(|e| panic!("{turn} deg export: {e}"));
+        let imp = step_import(&text).unwrap_or_else(|e| panic!("{turn} deg import: {e}"));
+        assert_eq!(imp.bodies.len(), 1, "{turn} deg {:?}", imp.warnings);
+        assert!(imp.bodies[0].closed && imp.warnings.is_empty(), "{turn} deg {:?}", imp.warnings);
+        let (a, b) = (measure(body).unwrap(), measure(&imp.bodies[0].body).unwrap());
+        assert!((a.volume - b.volume).abs() < 1e-3 * a.volume, "{turn} deg volume {} -> {}", a.volume, b.volume);
+        assert!((a.area - b.area).abs() < 1e-3 * a.area, "{turn} deg area {} -> {}", a.area, b.area);
+    }
+}

crates/kernel/src/step_in/tests.rs

@@ -238,6 +238,20 @@ fn round_trip_features() {
     round_trip(&donut[0]);
 }
 
+/// Robustness seed 17: a thin three-quarter tube shelled with its bottom open. truck writes each
+/// wall as three quarter-turn B-spline pieces; the export merges them into one 270° face and must
+/// write that face's cylinder, not the first piece's quarter sheet. The seam direction came from
+/// the mean direction of the face's vertices, which is zero at 0°, 90°, 180° and 270°, so the
+/// merged face kept the quarter sheet and read back about 7% short with no warning.
+#[test]
+fn shelled_three_quarter_tube_round_trips() {
+    let rect =
+        Region2 { outer: Loop2::polygon(&[Vec2::new(10.0, 0.0), Vec2::new(12.5, 0.0), Vec2::new(12.5, 27.5), Vec2::new(10.0, 27.5)]), holes: vec![] };
+    let tube = revolve(&Plane::XZ, &[rect], Vec2::ZERO, Vec2::Y, 1.5 * std::f64::consts::PI).unwrap().pop().unwrap();
+    let b = crate::shell(&tube, &[Vec3::new(-7.182422893157272, 7.182422893245109, 0.0)], 1.0).unwrap();
+    round_trip(&b);
+}
+
 #[test]
 fn several_bodies_and_names() {
     let a = box_solid(Vec3::ZERO, Vec3::new(10.0, 10.0, 10.0)).unwrap();

crates/kernel/src/step_out.rs

@@ -1252,7 +1252,8 @@ fn merge_split_faces(ex: &mut p21::Exchange, paint: &mut HashMap<u64, FaceLook>,
 
 /// B-spline cylinder and cone sheets written as CYLINDRICAL_SURFACE and CONICAL_SURFACE (the
 /// sheet covers only its own piece, and a fillet's may reach far past it), with the direction of
-/// the parameter seam: on the face's seam edge for a full turn, else opposite the face's middle.
+/// the parameter seam: on the face's seam edge for a full turn, else in the middle of the gap the
+/// face leaves (`gap_middle`), else opposite the face's middle.
 /// Sheets under faces merged from pieces always; a lone piece when its edges are circles square
 /// to the axis and rulings. (Faces replaced by merges stay in the exchange unreferenced: only the
 /// shells' faces count.)
@@ -1292,12 +1293,13 @@ fn analytic_sheets(ex: &p21::Exchange, merged_surfaces: &std::collections::HashS
             let p = ex.get(edge)?.params();
             [p.get(1), p.get(2)].into_iter().filter_map(|v| vertex_point(ex, v?.as_ref_id()?)).find_map(|q| unit(radial(q)))
         });
-        let x = match seam_dir {
+        let x = match seam_dir.or_else(|| gap_middle(ex, &loops, z, &radial)) {
             Some(x) => Some(x),
             None => {
-                // Points along the edges, not just their ends: the corners of a half turn (a
-                // slot's round end) point both ways and cancel out (#1198). The sheet's own
-                // middle where even those do.
+                // No single gap to find (a half turn has two corners, a tube cut in pieces
+                // several). Points along the edges, not just their ends: the corners of a half
+                // turn (a slot's round end) point both ways and cancel out (#1198). The sheet's
+                // own middle where even those do.
                 let mean = loops
                     .iter()
                     .flatten()
@@ -1316,6 +1318,51 @@ fn analytic_sheets(ex: &p21::Exchange, merged_surfaces: &std::collections::HashS
     out
 }
 
+/// The middle of the one angular gap that a face on a cylinder or cone leaves around its axis
+/// `z`. The face's boundary vertices are sorted by angle; each two neighbours are joined by one
+/// of its edges except across the gap (a 270° face split at 0°, 90°, 180° and 270° leaves
+/// 270°–360°). None unless there are three angles or more and exactly one gap: two vertices can
+/// be joined either way round.
+fn gap_middle(ex: &p21::Exchange, loops: &[Vec<(u64, bool)>], z: V3, radial: &impl Fn(V3) -> V3) -> Option<V3> {
+    use std::f64::consts::TAU;
+    let mut edges: Vec<(V3, V3)> = Vec::new();
+    for (e, _) in loops.iter().flatten() {
+        let p = ex.get(*e)?.params();
+        let (a, b) = (vertex_point(ex, p.get(1)?.as_ref_id()?)?, vertex_point(ex, p.get(2)?.as_ref_id()?)?);
+        edges.push((unit(radial(a))?, unit(radial(b))?));
+    }
+    let e1 = edges.first()?.0;
+    let e2 = unit(cross(z, e1))?;
+    let angle = |d: V3| dot(d, e2).atan2(dot(d, e1)).rem_euclid(TAU);
+    let close = |a: f64, b: f64| {
+        let d = (a - b).rem_euclid(TAU);
+        d < 1e-6 || TAU - d < 1e-6
+    };
+    let mut angles: Vec<f64> = Vec::new();
+    for (a, b) in &edges {
+        for t in [angle(*a), angle(*b)] {
+            if !angles.iter().any(|s| close(*s, t)) {
+                angles.push(t);
+            }
+        }
+    }
+    if angles.len() < 3 {
+        return None;
+    }
+    angles.sort_by(f64::total_cmp);
+    let joined = |s: f64, t: f64| {
+        edges.iter().any(|(a, b)| {
+            let (x, y) = (angle(*a), angle(*b));
+            (close(x, s) && close(y, t)) || (close(x, t) && close(y, s))
+        })
+    };
+    let n = angles.len();
+    let gaps: Vec<(f64, f64)> = (0..n).filter_map(|i| Some((*angles.get(i)?, *angles.get((i + 1) % n)?))).filter(|(s, t)| !joined(*s, *t)).collect();
+    let [(s, t)] = gaps[..] else { return None };
+    let m = s + (t - s).rem_euclid(TAU) / 2.0;
+    unit(add(mul(e1, m.cos()), mul(e2, m.sin())))
+}
+
 /// Margin left round a face when its sheet is cut down to it, as a share of the face's own
 /// parameter range on each side.
 const SHEET_MARGIN: f64 = 0.02;