STEP export: shelled revolved bodies export instead of panicking (#1496)

A scale across a revolution's axis keeps the face a surface of revolution and is written as one; any other refusal from the writer is an export error, not a panic.

crates/engine/tests/step_export_regressions.rs

@@ -0,0 +1,68 @@
+//! STEP export regressions found by round-tripping the robustness designs: a body the app built
+//! must export, and read 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 19, shrunk: a three-quarter turn of a rectangle, a sphere cut, a 1 mm shell.
+/// The shell moves revolved faces by a scale across their axis; export refused them and the
+/// refusal surfaced as a panic ("a Display implementation returned an error").
+#[test]
+fn shelled_revolve_with_a_sphere_cut_exports() {
+    let mut s = Session::default();
+    run(
+        &mut s,
+        json!([
+            {"command":"sketch.create","params":{"plane":"XZ"}},
+            {"command":"sketch.rectangle.two_point","params":{"p0":[5.0,0],"p1":[25.0,5.0]}},
+            {"command":"sketch.finish","params":{}},
+            {"command":"solid.revolve","params":{"angle":"270 deg","axis":"Z"}},
+            {"command":"solid.sphere","params":{"center":[-2.5,5.0,-2.5],"operation":"cut","radius":10.0}},
+            {"command":"solid.shell","params":{"faces":[[-2.7554552980815448e-15,-15.0,-2.5000000000000004]],"thickness":1.0}}
+        ]),
+    );
+    round_trips(&s);
+}
+
+/// Robustness seed 24, shrunk: a three-quarter turn of a rectangle, a chamfer of two edges, a
+/// 1.5 mm shell. The same refusal as seed 19's, reached through a chamfer.
+#[test]
+fn shelled_chamfered_revolve_exports() {
+    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":[27.5,22.5]}},
+            {"command":"sketch.finish","params":{}},
+            {"command":"solid.revolve","params":{"angle":"270 deg","axis":"Z"}},
+            {"command":"solid.chamfer","params":{"distance":2.0,"edges":[[7.0710678118654755,7.0710678118654755,0.0],[-19.445436482630054,19.44543648263006,0.0]]}},
+            {"command":"solid.shell","params":{"faces":[[18.75,0.0,-11.358616634594824]],"thickness":1.5}}
+        ]),
+    );
+    round_trips(&s);
+}

crates/kernel/src/step.rs

@@ -74,10 +74,25 @@ pub(crate) fn truck_step(bodies: &[&Body], system: &str) -> Result<String> {
         let cs: Vec<_> = bodies.iter().map(|b| plain_curves(b).map(|solid| solid.compress())).collect::<Result<_>>()?;
         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())
+        written(&out::CompleteStepDisplay::new(models, header))
     })
 }
 
+/// The text of truck's STEP writer, or an error. The writer refuses some geometry by returning
+/// `fmt::Error` after a line saying why; `to_string` would turn that into a panic (caught by
+/// `guard` on the desktop, but the browser build aborts on any panic).
+fn written(display: &impl std::fmt::Display) -> Result<String> {
+    use std::fmt::Write;
+    let mut text = String::new();
+    match write!(text, "{display}") {
+        Ok(()) => Ok(text),
+        Err(_) => {
+            let why = text.lines().last().map(str::trim).filter(|l| !l.is_empty() && !l.starts_with('#')).unwrap_or("unsupported geometry");
+            Err(KernelError::Failed(format!("not supported yet: STEP export of this body ({why})")))
+        }
+    }
+}
+
 /// STEP (ISO 10303-21, AP242) text for the given bodies as one part (`Body1`…, with the
 /// bodies' colours).
 pub fn step_export(bodies: &[&Body], system: &str) -> Result<String> {

crates/kernel/src/step_in/tests.rs

@@ -1202,3 +1202,42 @@ fn large_imports_are_freed_with_their_last_user() {
     drop((first, again));
     assert!(weak.upgrade().is_none(), "the cache must not keep a large import alive after its design is gone");
 }
+
+/// A three-quarter turn of a rectangle about Z: planar end caps, revolved cylinders and annuli.
+fn three_quarter_ring() -> Body {
+    let rect =
+        Region2 { outer: Loop2::polygon(&[Vec2::new(5.0, 0.0), Vec2::new(25.0, 0.0), Vec2::new(25.0, 5.0), Vec2::new(5.0, 5.0)]), holes: vec![] };
+    revolve(&Plane::XZ, &[rect], Vec2::ZERO, Vec2::Y, 1.5 * std::f64::consts::PI).unwrap().pop().unwrap()
+}
+
+fn scaled(b: &Body, x: f64, y: f64, z: f64) -> Body {
+    crate::transform_matrix(b, [[x, 0.0, 0.0, 0.0], [0.0, y, 0.0, 0.0], [0.0, 0.0, z, 0.0], [0.0, 0.0, 0.0, 1.0]]).unwrap()
+}
+
+/// A scale across the revolution axis that differs from the scale along it (how a shell or an
+/// offset moves a revolved face) keeps the faces surfaces of revolution. The writer refused it
+/// ("non-uniform scale"), and the refusal surfaced as a panic in export (robustness seeds 19, 24).
+#[test]
+fn revolved_faces_scaled_across_the_axis_round_trip() {
+    let ring = three_quarter_ring();
+    let v0 = measure(&ring).unwrap().volume;
+    let b = scaled(&ring, 1.5, 1.5, 0.5);
+    assert!(rel(measure(&b).unwrap().volume, v0 * 1.5 * 1.5 * 0.5) < 1e-3);
+    round_trip(&b);
+}
+
+/// A transform that does not keep the faces surfaces of revolution: export may refuse the body,
+/// but with an error, never a panic (the browser build aborts on any panic), and anything it does
+/// write must read back as the same solid.
+#[test]
+fn revolved_faces_skewed_never_panic_on_export() {
+    let b = scaled(&three_quarter_ring(), 1.5, 1.0, 1.0);
+    let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| step_export(&[&b], "t")));
+    let Ok(r) = r else { panic!("STEP export panicked") };
+    match r {
+        Err(e) => assert!(e.to_string().contains("not supported yet"), "{e}"),
+        Ok(_) => {
+            round_trip(&b);
+        }
+    }
+}

vendor/README.md

@@ -53,6 +53,12 @@ Changes:
   mirrored revolutions (ISO 10303-42 parameterises them by angle then profile, so their normal
   is the opposite of truck's; a mirroring transform reverses it again), and the modeling-surface
   wrapper passes the revolution's sense through instead of always `true`.
+- `out/geometry.rs`: a surface of revolution under a transform that commutes with turning about
+  its axis (a scale across the axis that differs from the scale along it, as a shell or offset
+  makes of a revolved face) is written as the transformed profile revolved about the
+  transformed axis. Only uniform scales were taken. Other transforms are still refused: the
+  writer stops with a formatting error, which SolidCraft's `step.rs` now reports as an export
+  error (`to_string` panicked on it).
 - `lib.rs`: compiler warnings no longer fail the build. Example and test targets are dropped
   (their sources are not vendored).
 

vendor/truck-stepio/src/out/geometry.rs

@@ -748,22 +748,42 @@ impl<C> StepSurface for RevolutedCurve<C> {
 impl<C> DisplayByStep for Processor<RevolutedCurve<C>, Matrix4>
 where C: StepLength + Transformed<Matrix4> + DisplayByStep
 {
+    // SolidCraft: a transform that commutes with turning about the axis keeps the surface one of
+    // revolution: the transformed profile revolved about the transformed axis. That holds for
+    // uniform scales (the only case taken before) and also for a scale across the axis that
+    // differs from the scale along it, which is how a shell or an offset moves a revolved
+    // cylinder or cone face. Any other transform is still refused.
     fn fmt(&self, idx: usize, f: &mut Formatter<'_>) -> Result {
         let surface = self.entity();
-        let transform = self.transform();
-        let (k, a, _) = match transform.iwasawa_decomposition() {
-            Some(x) => x,
-            None => {
+        let transform = *self.transform();
+        if transform.determinant().so_small() {
             f.write_str("Transform is not regular")?;
             return ERR;
         }
-        };
-        if !a[0][0].near(&a[1][1]) || !a[1][1].near(&a[2][2]) {
-            f.write_str("Transform contains non-uniform scale.")?;
+        let a = surface.axis().normalize();
+        let la = transform.transform_vector(a);
+        if la.so_small() {
+            f.write_str("Transform is not regular")?;
             return ERR;
         }
-        let curve = surface.entity_curve().transformed(*transform);
-        let axis = k.transform_vector(surface.axis());
+        let axis = la.normalize();
+        // Two directions across the axis, and their images.
+        let seed = if a.x.abs() < 0.9 { Vector3::unit_x() } else { Vector3::unit_y() };
+        let u = (seed - a * a.dot(seed)).normalize();
+        let v = a.cross(u);
+        let (lu, lv) = (transform.transform_vector(u), transform.transform_vector(v));
+        let s = lu.magnitude();
+        // Across the axis: a similarity onto the plane across the image axis.
+        let commutes = !s.so_small()
+            && (lu.dot(axis) / s).so_small()
+            && (lv.dot(axis) / s).so_small()
+            && (lu.dot(lv) / (s * s)).so_small()
+            && (lv.magnitude() / s).near(&1.0);
+        if !commutes {
+            f.write_str("Transform does not keep the surface one of revolution.")?;
+            return ERR;
+        }
+        let curve = surface.entity_curve().transformed(transform);
         let origin = transform.transform_point(surface.origin());
         let surface = RevolutedCurve::by_revolution(curve, origin, axis);
         DisplayByStep::fmt(&surface, idx, f)