Extrude: a symmetric distance is the total thickness (#1260)

A symmetric solid extrude of 10 mm is now 10 mm thick, 5 mm each side of the sketch plane, and switching from One Side to Symmetric keeps the thickness entered. Under format 21 the extent records its convention; designs saved without it keep the per-side reading, so their geometry and parameters don't change. Fixes #1236.

Co-authored-by: Karl-VonRichter <Karl-VonRichter@users.noreply.github.com>

crates/engine/src/cmd/edit.rs

@@ -626,6 +626,13 @@ fn edit_feature(s: &mut Session, p: &Value) -> Result<Value> {
     merge(&mut v, set, 0);
     let mut nf: Feature = serde_json::from_value(v).map_err(|e| bad(cmd, format!("invalid feature: {e}")))?;
     nf.id = f.id;
+    if let (FeatureKind::Extrude { extent: old, .. }, FeatureKind::Extrude { extent: new, .. }) = (&f.kind, &mut nf.kind)
+        && old.direction != solidcraft_doc::Direction::Symmetric
+        && new.direction == solidcraft_doc::Direction::Symmetric
+        && set.get("extent").and_then(|e| e.get("convention")).is_none()
+    {
+        new.convention = solidcraft_doc::ExtrudeConvention::TotalDistance;
+    }
     if std::mem::discriminant(&nf.kind) != std::mem::discriminant(&f.kind) {
         return Err(bad(cmd, "the feature type cannot change"));
     }
@@ -862,6 +869,17 @@ fn redefine(s: &mut Session, p: &Value) -> Result<Value> {
     let mut params = p.get("params").cloned().unwrap_or_else(|| json!({}));
     let idx = s.doc.feature_index(id).ok_or_else(|| bad(cmd, "feature"))?;
     let old = s.doc.feature(id).cloned().ok_or_else(|| bad(cmd, "feature"))?;
+    if let FeatureKind::Extrude { extent, .. } | FeatureKind::SurfaceExtrude { extent, .. } = &old.kind
+        && matches!(cid, "solid.extrude" | "surface.extrude")
+        && let Some(parameters) = params.as_object_mut()
+    {
+        let convention = if cid == "surface.extrude" || extent.direction == solidcraft_doc::Direction::Symmetric {
+            extent.convention
+        } else {
+            solidcraft_doc::ExtrudeConvention::TotalDistance
+        };
+        parameters.entry("convention").or_insert_with(|| json!(convention));
+    }
     if let FeatureKind::Revolve { convention, .. } = &old.kind
         && cid == "solid.revolve"
         && let Some(parameters) = params.as_object_mut()

crates/engine/src/cmd/extrude_convention_tests.rs

@@ -0,0 +1,143 @@
+#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
+
+use crate::Session;
+use serde_json::{Value, json};
+use solidcraft_doc::{Document, ExtrudeConvention, FeatureKind};
+
+fn run(s: &mut Session, cmd: &str, params: Value) {
+    s.execute(cmd, &params).unwrap_or_else(|e| panic!("{cmd}: {e}"));
+}
+
+fn fixture(settings: Value) -> Session {
+    let mut s = Session::default();
+    run(&mut s, "sketch.create", json!({"plane":"XY","name":"Square"}));
+    run(&mut s, "sketch.rectangle.two_point", json!({"p0":[0,0],"p1":[10,10]}));
+    run(&mut s, "sketch.finish", json!({}));
+    let mut p = json!({"sketch":"Square","distance":"10 mm","direction":"symmetric","operation":"new","name":"Depth","body_name":"Block"});
+    p.as_object_mut().unwrap().extend(settings.as_object().unwrap().clone());
+    run(&mut s, "solid.extrude", p);
+    s
+}
+
+fn check(s: &Session, lo: f64, hi: f64) {
+    assert!(s.model.results.iter().all(|r| r.error.is_none()));
+    let st = s.model.state();
+    let body = st.body("Block").unwrap();
+    let bounds = body.mesh().bounds();
+    assert!((bounds.min.z - lo).abs() < 1e-7 && (bounds.max.z - hi).abs() < 1e-7, "{bounds:?}");
+    let m = solidcraft_kernel::measure(&body.body).unwrap();
+    assert!((m.volume - 100.0 * (hi - lo)).abs() < 1e-7, "{m:?}");
+    assert!((m.centroid.z - (lo + hi) * 0.5).abs() < 1e-7);
+}
+
+#[test]
+fn symmetric_extrude_total_distance_is_centered_and_follows_parameters() {
+    for distance in ["10 mm", "-10 mm"] {
+        let mut s = fixture(json!({"distance":distance,"from":[0,0,2],"start_offset":"3 mm"}));
+        check(&s, 0.0, 10.0);
+        let FeatureKind::Extrude { extent, .. } = &s.doc.find_feature("Depth").unwrap().kind else { panic!("extrude") };
+        assert_eq!(extent.convention, ExtrudeConvention::TotalDistance);
+        let name = s.doc.find_feature("Depth").unwrap().param_names[0].clone();
+        run(&mut s, "parameters.change", json!({"name":name,"expression":"20 mm"}));
+        check(&s, -5.0, 15.0);
+        run(&mut s, "edit.undo", json!({}));
+        check(&s, 0.0, 10.0);
+        check(&Session::new(Document::from_json(&s.doc.to_json()).unwrap()), 0.0, 10.0);
+    }
+}
+
+#[test]
+fn symmetric_extrude_switch_from_one_side_keeps_total_thickness() {
+    let mut s = fixture(json!({"direction":"positive","convention":"legacy_per_side"}));
+    check(&s, 0.0, 10.0);
+    run(&mut s, "timeline.edit", json!({"feature":"Depth","set":{"extent":{"direction":"symmetric"}}}));
+    check(&s, -5.0, 5.0);
+    run(&mut s, "edit.undo", json!({}));
+    check(&s, 0.0, 10.0);
+    run(
+        &mut s,
+        "timeline.redefine",
+        json!({"feature":"Depth","command":"solid.extrude","params":{"sketch":"Square","distance":"10 mm","direction":"symmetric","body_name":"Block"}}),
+    );
+    check(&s, -5.0, 5.0);
+}
+
+#[test]
+fn symmetric_extrude_legacy_missing_marker_preserves_geometry_and_edits() {
+    let s = fixture(json!({"convention":"legacy_per_side"}));
+    let mut saved: Value = serde_json::from_str(&s.doc.to_json()).unwrap();
+    saved["format"] = json!("solidcraft/21");
+    for f in saved["features"].as_array_mut().unwrap() {
+        if f["type"] == "extrude" {
+            f["extent"].as_object_mut().unwrap().remove("convention");
+        }
+    }
+    let mut s = Session::new(Document::from_json(&saved.to_string()).unwrap());
+    check(&s, -10.0, 10.0);
+    run(&mut s, "timeline.edit", json!({"feature":"Depth","set":{"extent":{"distance":"12 mm"}}}));
+    check(&s, -12.0, 12.0);
+    run(&mut s, "edit.undo", json!({}));
+    run(
+        &mut s,
+        "timeline.redefine",
+        json!({"feature":"Depth","command":"solid.extrude","params":{"sketch":"Square","distance":"10 mm","direction":"symmetric","body_name":"Block"}}),
+    );
+    check(&s, -10.0, 10.0);
+    check(&Session::new(Document::from_json(&s.doc.to_json()).unwrap()), -10.0, 10.0);
+}
+
+#[test]
+fn symmetric_extrude_through_all_keeps_reach_and_two_sides_keep_independent_distances() {
+    let mut s = fixture(json!({"distance2":"3 mm"}));
+    check(&s, -3.0, 10.0);
+    run(&mut s, "solid.box", json!({"length":10,"width":10,"height":10,"corner":[0,0,100],"body_name":"FarAbove"}));
+    run(&mut s, "solid.box", json!({"length":10,"width":10,"height":10,"corner":[0,0,-110],"body_name":"FarBelow"}));
+    run(
+        &mut s,
+        "solid.extrude",
+        json!({"sketch":"Square","direction":"symmetric","through_all":true,"operation":"cut","targets":["FarAbove","FarBelow"]}),
+    );
+    assert!(s.model.state().body("FarAbove").is_none());
+    assert!(s.model.state().body("FarBelow").is_none());
+}
+
+#[test]
+fn symmetric_extrude_invalid_convention_is_atomic() {
+    let mut s = fixture(json!({}));
+    let before = s.doc.to_json();
+    for convention in [json!("unknown"), json!(7), Value::Null] {
+        assert!(s.execute("solid.extrude", &json!({"sketch":"Square","distance":10,"direction":"symmetric","convention":convention})).is_err());
+        assert_eq!(s.doc.to_json(), before);
+    }
+}
+
+#[test]
+fn symmetric_extrude_thin_wall_uses_total_distance_and_to_object_remains_one_sided() {
+    let s = fixture(json!({"thin":true,"thickness":"2 mm"}));
+    let st = s.model.state();
+    let body = st.body("Block").unwrap();
+    let b = body.mesh().bounds();
+    assert!((b.min.z + 5.0).abs() < 1e-7 && (b.max.z - 5.0).abs() < 1e-7);
+    // A centred 2 mm wall around a 10 mm square: (12² - 8²) × 10.
+    assert!((solidcraft_kernel::measure(&body.body).unwrap().volume - 800.0).abs() < 1e-7);
+    check(&fixture(json!({"to":[0,0,10]})), 0.0, 10.0);
+}
+
+#[test]
+fn surface_extrude_shared_extent_retains_its_existing_per_side_semantics() {
+    let mut s = fixture(json!({}));
+    run(&mut s, "surface.extrude", json!({"sketch":"Square","distance":"10 mm","direction":"symmetric","name":"Sheet","body_name":"Sheet"}));
+    let check_sheet = |s: &Session| {
+        let st = s.model.state();
+        let bounds = st.body("Sheet").unwrap().mesh().bounds();
+        assert!((bounds.min.z + 10.0).abs() < 1e-7 && (bounds.max.z - 10.0).abs() < 1e-7);
+    };
+    check_sheet(&s);
+    run(
+        &mut s,
+        "timeline.redefine",
+        json!({"feature":"Sheet","command":"surface.extrude","params":{"sketch":"Square","distance":"10 mm","direction":"symmetric","body_name":"Sheet"}}),
+    );
+    check_sheet(&s);
+    check_sheet(&Session::new(Document::from_json(&s.doc.to_json()).unwrap()));
+}

crates/engine/src/cmd/features.rs

@@ -13,7 +13,7 @@ pub static COMMANDS: &[CommandSpec] = &[
         .at("SOLID", "CREATE")
         .icon("extrude")
         .key("E")
-        .params("distance: expr (or through_all: true); taper?: angle expr; sketch?: id|name (default: active or last sketch); profiles?: all | [index] | [{curves: [ids]}] (the profile whose outer loop is made of exactly these curves, or pieces of them; found again on every recompute, so it follows parameter changes; islands inside are its holes; an error when no profile or several match) | [{point:[x,y]}] (index and point picks are stored by their curves when that finds the same profile); face?: [x,y,z] (extrude a planar body face instead of a sketch profile); face_body?: name (whose face, where faces of several bodies meet at the point); direction?: positive|negative|symmetric; distance2?; start_offset?: expr (along the sketch normal, also with direction: negative); to?: [x,y,z] (To Object: a face, vertex or point; to_offset?: expr past it); from?: [x,y,z] (From Object: start at that face or point); operation?: new|join|cut|intersect|auto (cut into a body, otherwise join existing solids; new in an empty component); targets?: [body]; name?; body_name?; thin?: true (Thin: a wall along sketch curves instead of the profiles) with curves?: [curve ids] (open chains and closed loops, joined where they meet end to end; default: every drawn curve of the sketch), thickness: expr, side?: center|inside|outside|two_sides (a closed loop's inside is the area it encloses, an open chain's the side it bends toward, a straight line's the left of its first curve as drawn; default center), thickness2?: expr (two_sides: the thickness outside); corners are mitred, crossing walls are one"),
+        .params("distance: expr (or through_all: true); taper?: angle expr; sketch?: id|name (default: active or last sketch); profiles?: all | [index] | [{curves: [ids]}] (the profile whose outer loop is made of exactly these curves, or pieces of them; found again on every recompute, so it follows parameter changes; islands inside are its holes; an error when no profile or several match) | [{point:[x,y]}] (index and point picks are stored by their curves when that finds the same profile); face?: [x,y,z] (extrude a planar body face instead of a sketch profile); face_body?: name (whose face, where faces of several bodies meet at the point); direction?: positive|negative|symmetric (distance is total thickness, split evenly around the start plane); convention?: total_distance|legacy_per_side (default total_distance; legacy preserves older symmetric extents); distance2?; start_offset?: expr (along the sketch normal, also with direction: negative); to?: [x,y,z] (To Object: a face, vertex or point; to_offset?: expr past it); from?: [x,y,z] (From Object: start at that face or point); operation?: new|join|cut|intersect|auto (cut into a body, otherwise join existing solids; new in an empty component); targets?: [body]; name?; body_name?; thin?: true (Thin: a wall along sketch curves instead of the profiles) with curves?: [curve ids] (open chains and closed loops, joined where they meet end to end; default: every drawn curve of the sketch), thickness: expr, side?: center|inside|outside|two_sides (a closed loop's inside is the area it encloses, an open chain's the side it bends toward, a straight line's the left of its first curve as drawn; default center), thickness2?: expr (two_sides: the thickness outside); corners are mitred, crossing walls are one"),
     CommandSpec::new("solid.revolve", "Revolve", revolve)
         .at("SOLID", "CREATE")
         .icon("revolve")
@@ -632,8 +632,15 @@ pub(super) fn extent_param(s: &Session, p: &Value, cmd: &str) -> Result<Extent>
     if let Some(d) = &start_offset {
         check_expr(s, d, Kind::Length, cmd, "start_offset")?;
     }
+    let convention = match p.get("convention") {
+        // Surface Extrude shares Extent but retains its existing per-side distance semantics.
+        None if cmd == "surface.extrude" => solidcraft_doc::ExtrudeConvention::LegacyPerSide,
+        None => solidcraft_doc::ExtrudeConvention::TotalDistance,
+        Some(value) => serde_json::from_value(value.clone()).map_err(|_| bad(cmd, "`convention` must be total_distance or legacy_per_side"))?,
+    };
     Ok(Extent {
         distance,
+        convention,
         direction,
         distance2,
         start_offset,
@@ -1780,3 +1787,7 @@ mod move_in_sketch_tests;
 #[cfg(test)]
 #[path = "thin_extrude_tests.rs"]
 mod thin_extrude_tests;
+
+#[cfg(test)]
+#[path = "extrude_convention_tests.rs"]
+mod extrude_convention_tests;

crates/engine/src/cmd/patterns_tests.rs

@@ -367,7 +367,8 @@ fn sawtooth_bar(teeth: usize, pattern: bool) -> (Session, Option<u64>) {
         run(&mut s, "sketch.line", json!({"points": pts, "closed": true}));
     }
     run(&mut s, "sketch.finish", json!({}));
-    run(&mut s, "solid.extrude", json!({"sketch": "Tooth", "distance": 8, "direction": "symmetric", "operation": "cut", "name": "Tooth cut"}));
+    // Total thickness 16 mm keeps the sawtooth cut spanning 8 mm on each side of YZ.
+    run(&mut s, "solid.extrude", json!({"sketch": "Tooth", "distance": 16, "direction": "symmetric", "operation": "cut", "name": "Tooth cut"}));
     let id = pattern.then(|| {
         let r = run(&mut s, "solid.pattern.rectangular", json!({"features": ["Tooth cut"], "dir1": [0, 1, 0], "count1": teeth, "spacing1": 4}));
         r["feature"].as_u64().unwrap()

crates/engine/src/cmd/pbr_appearance_tests.rs

@@ -18,7 +18,7 @@ fn format20_scalar_looks_upgrade_and_format21_setups_keep_their_meaning() {
     legacy["format"] = json!("solidcraft/20");
     legacy.as_object_mut().unwrap().remove("render");
     let upgraded = crate::doc::Document::from_json(&legacy.to_string()).unwrap();
-    assert_eq!(upgraded.format, "solidcraft/21");
+    assert_eq!(upgraded.format, crate::doc::format::format_string(crate::doc::format::FORMAT));
     assert_eq!(serde_json::to_value(&upgraded.appearances).unwrap(), before);
     assert!(upgraded.render.setups.is_empty());
     s.execute("render.settings.set", &json!({"name":"Studio","set":{"width":320,"height":240}})).unwrap();
@@ -29,7 +29,7 @@ fn format20_scalar_looks_upgrade_and_format21_setups_keep_their_meaning() {
     assert_eq!(serde_json::to_value(&d.appearances).unwrap(), before);
     assert!(d.appearances.bodies["Specimen"].pbr.is_none());
     assert_eq!(d.render.setups["Studio"].width, 320);
-    for version in [22, 23] {
+    for version in [crate::doc::format::FORMAT + 1, crate::doc::format::FORMAT + 2] {
         saved["format"] = json!(format!("solidcraft/{version}"));
         assert!(crate::doc::Document::from_json(&saved.to_string()).is_err());
     }
@@ -52,7 +52,7 @@ fn opaque_response_preserves_geometry_mass_undo_and_cold_reopening() {
     assert!(after.meshes[0].surface.is_some());
     assert_eq!(after.required_features(), vec![crate::render::RenderFeature::Pbr]);
     let saved = s.doc.to_json();
-    assert_eq!(serde_json::from_str::<serde_json::Value>(&saved).unwrap()["format"], "solidcraft/21");
+    assert_eq!(serde_json::from_str::<serde_json::Value>(&saved).unwrap()["format"], crate::doc::format::format_string(crate::doc::format::FORMAT));
     let look = &s.doc.appearances.bodies["Specimen"];
     assert_eq!(crate::doc::config::appearance_cell(&serde_json::to_string(look).unwrap()).unwrap().as_ref(), Some(look));
     let list = s.execute("appearance.list", &json!({})).unwrap();

crates/engine/src/cmd/sketch_modify.rs

@@ -46,7 +46,7 @@ pub static COMMANDS: &[CommandSpec] = &[
         .enabled(in_sketch)
         .params(concat!(
             "curves: [ids], distance: number in mm or length expression (creates an editable dimension; positive = left of the chain, or toward `side`), side?: [x,y], chain?: bool (default true: the connected chain); ",
-            "direction?: one_side|symmetric (default one_side) or symmetric?: bool (the same distance each way, as for `solid.extrude`; no `side`, whose job it takes over), ",
+            "direction?: one_side|symmetric (default one_side) or symmetric?: bool (the same distance each way; no `side`, whose job it takes over), ",
             "distance2?: positive length expression (Two Sides: an independent distance the other way, as for `solid.extrude`). Each side is its own offset association, listed in `sides`; flip?: bool reverses the primary side; `param`/`param2` name the driving dimensions",
             created_doc!()
         )),
@@ -992,7 +992,7 @@ struct Side {
 }
 
 /// The signed distances an offset runs at. One side is the distance as given. `symmetric` is the
-/// same distance each way, as `direction: symmetric` means for `solid.extrude`. `distance2` is an
+/// same distance each way. `distance2` is an
 /// independent distance the other way, as `distance2` means for `solid.extrude`. The primary side
 /// comes first, so a one-sided caller reads the same first entry it always did.
 fn offset_runs(d: f64, symmetric: bool, distance2: Option<f64>) -> Vec<f64> {
@@ -1259,7 +1259,7 @@ fn offset(s: &mut Session, p: &Value) -> Result<Value> {
     let d0 = if bool_(p, "flip").unwrap_or(false) { -value } else { value };
     let side = p.get("side").and_then(vec2);
     let use_chain = bool_(p, "chain").unwrap_or(true);
-    // Both sides at once. The words keep the meaning `solid.extrude` gives them: `symmetric` is
+    // Both sides at once: `symmetric` is
     // one distance applied each way, `distance2` is an independent distance the other way.
     let symmetric = match str_(p, "direction").map(str::to_ascii_lowercase).as_deref() {
         None | Some("positive") | Some("one_side") => bool_(p, "symmetric").unwrap_or(false),