solid.cylinder, box, sphere, torus and coil place by expressions: base, corner and center take numbers or length expressions per coordinate ("basin_width/2", "10 mm") and follow parameter changes, renames and component frames; before, a point that was not three plain numbers (or an unreadable axis) was silently replaced by the origin (or Z). The features now store the point as expressions (plain numbers still saved as numbers, so existing designs read and write the same) and evaluate it on every recompute; an unreadable or out-of-range point or axis is refused with an error. Tests: primitive points take expressions, follow parameters, renames and a moved component, and bad points are refused

crates/engine/src/cmd/features.rs

@@ -1,7 +1,7 @@
 //! Solid feature commands: extrude, revolve, fillet, chamfer, primitives, combine, move.
 
 use serde_json::{Value, json};
-use solvecraft_doc::{AxisRef, Direction, Extent, FeatureKind, Operation, ProfileSel, expr::Kind};
+use solvecraft_doc::{AxisRef, Direction, Extent, FeatureKind, Operation, PointExpr, ProfileSel, expr::Kind, point_expr};
 use solvecraft_geom::Vec3;
 
 use super::CommandSpec;
@@ -23,10 +23,10 @@ pub static COMMANDS: &[CommandSpec] = &[
         .icon("sweep")
         .params("sketch: profile sketch; profiles?; path_sketch: sketch; path: [curve ids in order]; operation?"),
     CommandSpec::new("solid.loft", "Loft", loft).at("SOLID", "CREATE").icon("loft").params("sections: [{sketch, profiles?} | {sketch, point: id} (an apex)] (2 or more, in order); operation?"),
-    CommandSpec::new("solid.box", "Box", prim_box).at("SOLID", "CREATE").icon("box").params("length, width, height: expr; corner?: [x,y,z] | center?: [x,y,z]; operation?"),
-    CommandSpec::new("solid.cylinder", "Cylinder", prim_cylinder).at("SOLID", "CREATE").icon("cylinder").params("radius | diameter, height: expr; base?: [x,y,z]; axis?: [x,y,z]; operation?"),
-    CommandSpec::new("solid.sphere", "Sphere", prim_sphere).at("SOLID", "CREATE").icon("sphere").params("radius | diameter: expr; center?: [x,y,z]; operation?"),
-    CommandSpec::new("solid.torus", "Torus", prim_torus).at("SOLID", "CREATE").icon("torus").params("major, minor: expr (radii); center?; operation?"),
+    CommandSpec::new("solid.box", "Box", prim_box).at("SOLID", "CREATE").icon("box").params("length, width, height: expr; corner?: [x,y,z] | center?: [x,y,z] (numbers in mm or length expressions; default: the origin); operation?"),
+    CommandSpec::new("solid.cylinder", "Cylinder", prim_cylinder).at("SOLID", "CREATE").icon("cylinder").params("radius | diameter, height: expr; base?: [x,y,z] (centre of the bottom: numbers in mm or length expressions; default: the origin); axis?: X|Y|Z|[x,y,z] (default Z); operation?"),
+    CommandSpec::new("solid.sphere", "Sphere", prim_sphere).at("SOLID", "CREATE").icon("sphere").params("radius | diameter: expr; center?: [x,y,z] (numbers in mm or length expressions; default: the origin); operation?"),
+    CommandSpec::new("solid.torus", "Torus", prim_torus).at("SOLID", "CREATE").icon("torus").params("major, minor: expr (radii); center?: [x,y,z] (numbers in mm or length expressions; default: the origin); operation?"),
     CommandSpec::new("solid.fillet", "Fillet", fillet)
         .at("SOLID", "MODIFY")
         .icon("fillet")
@@ -484,19 +484,59 @@ fn radius_expr(p: &Value, cmd: &str) -> Result<String> {
     }
 }
 
+/// A placement point: `[x, y, z]` (or `{x, y, z}`) of numbers (mm) or length expressions;
+/// the origin when it is not given. Anything else is refused (never read as the origin).
+pub(super) fn point_param(s: &Session, p: &Value, key: &str, cmd: &str) -> Result<Option<PointExpr>> {
+    let Some(v) = p.get(key) else { return Ok(None) };
+    let want = || bad(cmd, format!("`{key}` must be a point [x, y, z] of numbers or length expressions"));
+    let coords: Vec<&Value> = match v {
+        Value::Array(a) if a.len() == 3 => a.iter().collect(),
+        Value::Object(o) => ["x", "y", "z"].iter().map(|k| o.get(*k)).collect::<Option<Vec<_>>>().ok_or_else(want)?,
+        _ => return Err(want()),
+    };
+    let mut out = PointExpr::default();
+    for (slot, (c, axis)) in out.iter_mut().zip(coords.into_iter().zip(["x", "y", "z"])) {
+        let e = expr(&json!({ "v": c }), "v").ok_or_else(want)?;
+        let x = s.doc.eval(&e, Kind::Length).map_err(|err| bad(cmd, format!("{key} {axis}: {err}")))?;
+        if !(x.is_finite() && x.abs() < 1e9) {
+            return Err(bad(cmd, format!("{key} {axis} is out of range")));
+        }
+        *slot = e;
+    }
+    Ok(Some(out))
+}
+
+/// An axis direction: X|Y|Z or a non-zero `[x, y, z]`; `Z` when it is not given.
+pub(super) fn axis_dir_param(p: &Value, cmd: &str) -> Result<Vec3> {
+    match p.get("axis") {
+        None => Ok(Vec3::Z),
+        Some(Value::String(a)) => match a.trim().to_ascii_uppercase().as_str() {
+            "X" => Ok(Vec3::X),
+            "Y" => Ok(Vec3::Y),
+            "Z" => Ok(Vec3::Z),
+            _ => Err(bad(cmd, "`axis` must be X, Y, Z or [x, y, z]")),
+        },
+        Some(v) => vec3(v).and_then(|a| a.normalized()).ok_or_else(|| bad(cmd, "`axis` must be X, Y, Z or a non-zero [x, y, z]")),
+    }
+}
+
 fn prim_box(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "solid.box";
     let (l, w, h) = (req_expr(cmd, p, "length")?, req_expr(cmd, p, "width")?, req_expr(cmd, p, "height")?);
     for (e, n) in [(&l, "length"), (&w, "width"), (&h, "height")] {
         check_expr(s, e, Kind::Length, cmd, n)?;
     }
-    let corner = match (p.get("corner").and_then(vec3), p.get("center").and_then(vec3)) {
+    let corner = match (point_param(s, p, "corner", cmd)?, point_param(s, p, "center", cmd)?) {
         (Some(c), _) => c,
-        (None, Some(c)) => {
-            let v = |e: &str| s.doc.eval(e, Kind::Length).unwrap_or(0.0);
-            c - Vec3::new(v(&l), v(&w), v(&h)) * 0.5
+        // The corner half a size back from the centre (numbers stay numbers).
+        (None, Some([cx, cy, cz])) => {
+            let back = |c: String, size: &String| match (c.parse::<f64>(), size.parse::<f64>()) {
+                (Ok(a), Ok(b)) => format!("{}", a - b / 2.0),
+                _ => format!("({c}) - ({size}) / 2"),
+            };
+            [back(cx, &l), back(cy, &w), back(cz, &h)]
         }
-        _ => Vec3::ZERO,
+        _ => point_expr(Vec3::ZERO),
     };
     add_feature(s, p, FeatureKind::Box { corner, length: l, width: w, height: h, operation: operation(p, cmd)? })
 }
@@ -507,8 +547,8 @@ fn prim_cylinder(s: &mut Session, p: &Value) -> Result<Value> {
     let height = req_expr(cmd, p, "height")?;
     check_expr(s, &radius, Kind::Length, cmd, "radius")?;
     check_expr(s, &height, Kind::Length, cmd, "height")?;
-    let base = p.get("base").and_then(vec3).unwrap_or(Vec3::ZERO);
-    let axis = p.get("axis").and_then(vec3).unwrap_or(Vec3::Z);
+    let base = point_param(s, p, "base", cmd)?.unwrap_or_else(|| point_expr(Vec3::ZERO));
+    let axis = axis_dir_param(p, cmd)?;
     add_feature(s, p, FeatureKind::Cylinder { base, axis, radius, height, operation: operation(p, cmd)? })
 }
 
@@ -516,7 +556,7 @@ fn prim_sphere(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "solid.sphere";
     let radius = radius_expr(p, cmd)?;
     check_expr(s, &radius, Kind::Length, cmd, "radius")?;
-    let center = p.get("center").and_then(vec3).unwrap_or(Vec3::ZERO);
+    let center = point_param(s, p, "center", cmd)?.unwrap_or_else(|| point_expr(Vec3::ZERO));
     add_feature(s, p, FeatureKind::Sphere { center, radius, operation: operation(p, cmd)? })
 }
 
@@ -525,7 +565,7 @@ fn prim_torus(s: &mut Session, p: &Value) -> Result<Value> {
     let (major, minor) = (req_expr(cmd, p, "major")?, req_expr(cmd, p, "minor")?);
     check_expr(s, &major, Kind::Length, cmd, "major")?;
     check_expr(s, &minor, Kind::Length, cmd, "minor")?;
-    let center = p.get("center").and_then(vec3).unwrap_or(Vec3::ZERO);
+    let center = point_param(s, p, "center", cmd)?.unwrap_or_else(|| point_expr(Vec3::ZERO));
     add_feature(s, p, FeatureKind::Torus { center, major, minor, operation: operation(p, cmd)? })
 }
 

crates/engine/src/cmd/features_more.rs

@@ -6,7 +6,7 @@ use solvecraft_doc::expr::Kind;
 use solvecraft_geom::Vec3;
 
 use super::CommandSpec;
-use super::features::{add_feature, check_expr, feature_sketch, plane_param, profiles};
+use super::features::{add_feature, axis_dir_param, check_expr, feature_sketch, plane_param, point_param, profiles};
 use crate::params::{bad, bool_, expr, req_expr, str_, string_list, vec3};
 use crate::{Result, Session};
 
@@ -16,7 +16,7 @@ pub static COMMANDS: &[CommandSpec] = &[
     ),
     CommandSpec::new("solid.coil", "Coil", coil).at("SOLID", "CREATE").icon("coil").params(
         "diameter; two of revolutions (or turns), height, pitch; section_size; section?: circular|square; section_position?: inside|center|outside; \
-         base?: [x,y,z]; axis?: X|Y|Z|[x,y,z] (default Z); start_angle?; clockwise?: bool; operation?, targets?, name?, body_name?",
+         base?: [x,y,z] (numbers in mm or length expressions; default: the origin); axis?: X|Y|Z|[x,y,z] (default Z); start_angle?; clockwise?: bool; operation?, targets?, name?, body_name?",
     ),
     CommandSpec::new("solid.rib", "Rib", rib)
         .at("SOLID", "CREATE")
@@ -43,17 +43,8 @@ pub static COMMANDS: &[CommandSpec] = &[
 
 fn coil(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "solid.coil";
-    let base = p.get("base").and_then(vec3).unwrap_or(Vec3::ZERO);
-    let axis = match p.get("axis") {
-        None => Vec3::Z,
-        Some(Value::String(a)) => match a.to_ascii_uppercase().as_str() {
-            "X" => Vec3::X,
-            "Y" => Vec3::Y,
-            "Z" => Vec3::Z,
-            _ => return Err(bad(cmd, "`axis` must be X, Y, Z or [x, y, z]")),
-        },
-        Some(v) => vec3(v).and_then(|a| a.normalized()).ok_or_else(|| bad(cmd, "`axis` must be X, Y, Z or a non-zero [x, y, z]"))?,
-    };
+    let base = point_param(s, p, "base", cmd)?.unwrap_or_else(|| solvecraft_doc::point_expr(Vec3::ZERO));
+    let axis = axis_dir_param(p, cmd)?;
     let diameter = req_expr(cmd, p, "diameter")?;
     check_expr(s, &diameter, Kind::Length, cmd, "diameter")?;
     let turns = expr(p, "revolutions").or_else(|| expr(p, "turns"));

crates/engine/src/frames.rs

@@ -10,7 +10,7 @@
 //! without moved components).
 
 use serde_json::{Map, Value};
-use solvecraft_doc::{Mat, apply_point, apply_vector, is_identity, mat_inverse};
+use solvecraft_doc::{Mat, apply_point, apply_vector, is_identity, map_point_expr, mat_inverse};
 use solvecraft_geom::Vec3;
 
 use crate::Session;
@@ -203,11 +203,29 @@ impl Mapper<'_> {
     }
 
     fn make(&mut self, v: &Value) -> Option<Value> {
-        let p = vec3(v)?;
         let m = self.f.active;
+        let Some(p) = vec3(v) else { return self.make_expr(&m, v) };
         self.local_point(&m, p)
     }
 
+    /// A point with expression coordinates (`["w / 2", 0, 0]`): mapped as expressions.
+    fn make_expr(&mut self, m: &Mat, v: &Value) -> Option<Value> {
+        let a = v.as_array().filter(|a| a.len() == 3)?;
+        let c = |i: usize| match a.get(i)? {
+            Value::Number(n) => n.as_f64().filter(|x| x.is_finite()).map(|x| format!("{x}")),
+            Value::String(e) => Some(e.clone()),
+            _ => None,
+        };
+        let mut p = [c(0)?, c(1)?, c(2)?];
+        if is_identity(m) {
+            return None;
+        }
+        let inv = mat_inverse(m)?;
+        map_point_expr(&inv, &mut p);
+        self.changed = true;
+        Some(serde_json::json!(p))
+    }
+
     fn dir(&mut self, v: &Value) -> Option<Value> {
         let d = vec3(v)?;
         let m = self.first.unwrap_or(self.f.active);

crates/engine/src/tests.rs

@@ -36,6 +36,7 @@ fn join_extrude_from_a_chamfered_top_face() {
 }
 
 mod component_frames;
+mod primitive_points;
 mod robustness;
 
 /// Sketch a 40 x 30 rectangle with dimensions, extrude 20, fillet a vertical edge, cut a hole.

crates/engine/src/tests/primitive_points.rs

@@ -0,0 +1,121 @@
+//! Primitive placement points (`base`, `corner`, `center`) take expressions like their sizes do,
+//! follow parameter changes, and are refused when they cannot be read (user report: a cylinder
+//! with `base: ["basin_width/2", …]` silently landed at the origin).
+
+use serde_json::{Value, json};
+
+use super::run;
+use crate::*;
+
+fn bbox(s: &Session, body: &str) -> ([f64; 3], [f64; 3]) {
+    let b = s.model.state().body(body).map(|b| b.body.clone()).unwrap();
+    let m = solvecraft_kernel::measure(&b).unwrap();
+    ([m.bbox.min.x, m.bbox.min.y, m.bbox.min.z], [m.bbox.max.x, m.bbox.max.y, m.bbox.max.z])
+}
+
+fn near(a: [f64; 3], b: [f64; 3]) -> bool {
+    a.iter().zip(b).all(|(x, y)| (x - y).abs() < 1e-3)
+}
+
+#[test]
+fn cylinder_base_expressions_follow_parameters() {
+    let mut s = Session::default();
+    run(&mut s, "parameters.change", json!({"name": "basin_width", "expression": "100 mm"}));
+    run(&mut s, "solid.cylinder", json!({"radius": 5, "height": 10, "base": ["basin_width/2", "10 mm", "-2"], "body_name": "Pin"}));
+    let (lo, hi) = bbox(&s, "Pin");
+    assert!(near(lo, [45.0, 5.0, -2.0]) && near(hi, [55.0, 15.0, 8.0]), "{lo:?} {hi:?}");
+    // A parameter change moves it on recompute.
+    run(&mut s, "parameters.change", json!({"name": "basin_width", "expression": "60 mm"}));
+    let (lo, _) = bbox(&s, "Pin");
+    assert!(near(lo, [25.0, 5.0, -2.0]), "{lo:?}");
+    // And it survives a save and reopen.
+    let text = s.doc.to_json();
+    assert!(text.contains("basin_width/2"), "{text}");
+    let s2 = Session::new(solvecraft_doc::Document::from_json(&text).unwrap());
+    let (lo, _) = bbox(&s2, "Pin");
+    assert!(near(lo, [25.0, 5.0, -2.0]), "{lo:?}");
+}
+
+#[test]
+fn primitive_points_take_expressions() {
+    let mut s = Session::default();
+    run(&mut s, "parameters.change", json!({"name": "w", "expression": "40 mm"}));
+    run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10, "corner": ["w", 0, 0], "body_name": "A"}));
+    run(&mut s, "solid.box", json!({"length": "w", "width": 10, "height": 10, "center": [0, "w", 0], "body_name": "B"}));
+    run(&mut s, "solid.sphere", json!({"radius": 2, "center": ["w * 2", 0, 0], "body_name": "C"}));
+    run(&mut s, "solid.torus", json!({"major": 5, "minor": 1, "center": [0, 0, "w"], "body_name": "D"}));
+    run(&mut s, "solid.coil", json!({"diameter": 10, "revolutions": 2, "pitch": 5, "section_size": 1, "base": ["-w", 0, 0], "body_name": "E"}));
+    assert!(near(bbox(&s, "A").0, [40.0, 0.0, 0.0]));
+    assert!(near(bbox(&s, "B").0, [-20.0, 35.0, -5.0]), "{:?}", bbox(&s, "B"));
+    assert!(near(bbox(&s, "C").0, [78.0, -2.0, -2.0]));
+    assert!(near(bbox(&s, "D").0, [-6.0, -6.0, 39.0]));
+    assert!((bbox(&s, "E").0[0] + 45.5).abs() < 0.2, "{:?}", bbox(&s, "E"));
+    run(&mut s, "parameters.change", json!({"name": "w", "expression": "20 mm"}));
+    assert!(near(bbox(&s, "A").0, [20.0, 0.0, 0.0]));
+    assert!(near(bbox(&s, "B").0, [-10.0, 15.0, -5.0]), "{:?}", bbox(&s, "B"));
+    assert!(near(bbox(&s, "C").0, [38.0, -2.0, -2.0]));
+    assert!(near(bbox(&s, "D").0, [-6.0, -6.0, 19.0]));
+    assert!((bbox(&s, "E").0[0] + 25.5).abs() < 0.2, "{:?}", bbox(&s, "E"));
+}
+
+/// Points and axes that cannot be read are refused, never silently put at the origin.
+#[test]
+fn unreadable_primitive_points_are_refused() {
+    let mut s = Session::default();
+    let cases: Vec<(&str, Value)> = vec![
+        ("solid.cylinder", json!({"radius": 5, "height": 10, "base": ["nope/2", 0, 0]})),
+        ("solid.cylinder", json!({"radius": 5, "height": 10, "base": [1, 2]})),
+        ("solid.cylinder", json!({"radius": 5, "height": 10, "base": "XY"})),
+        ("solid.cylinder", json!({"radius": 5, "height": 10, "base": [1, 2, "5 deg"]})),
+        ("solid.cylinder", json!({"radius": 5, "height": 10, "axis": ["x", 0, 1]})),
+        ("solid.cylinder", json!({"radius": 5, "height": 10, "axis": [0, 0, 0]})),
+        ("solid.box", json!({"length": 1, "width": 1, "height": 1, "corner": [0, null, 0]})),
+        ("solid.box", json!({"length": 1, "width": 1, "height": 1, "center": {"x": 1}})),
+        ("solid.sphere", json!({"radius": 1, "center": ["a +", 0, 0]})),
+        ("solid.torus", json!({"major": 5, "minor": 1, "center": [1e300, 0, 0]})),
+        ("solid.coil", json!({"diameter": 10, "revolutions": 2, "pitch": 5, "section_size": 1, "base": [true, 0, 0]})),
+    ];
+    for (cmd, p) in cases {
+        let before = s.doc.features.len();
+        assert!(s.execute(cmd, &p).is_err(), "{cmd} {p} should be refused");
+        assert_eq!(s.doc.features.len(), before, "{cmd} {p} added a feature");
+    }
+}
+
+/// Renaming a parameter follows into placement points; deleting it is refused while a point
+/// uses it. Plain-number points are stored as numbers, as before.
+#[test]
+fn point_parameters_rename_and_store() {
+    let mut s = Session::default();
+    run(&mut s, "parameters.change", json!({"name": "off", "expression": "30 mm"}));
+    run(&mut s, "solid.sphere", json!({"radius": 2, "center": ["off", 0, 0], "body_name": "S"}));
+    run(&mut s, "solid.box", json!({"length": 1, "width": 1, "height": 1, "corner": [1.5, -2, 0], "body_name": "B"}));
+    assert!(s.execute("parameters.delete", &json!({"name": "off"})).is_err());
+    run(&mut s, "parameters.rename", json!({"name": "off", "new_name": "shift"}));
+    run(&mut s, "parameters.change", json!({"name": "shift", "expression": "10 mm"}));
+    assert!(near(bbox(&s, "S").0, [8.0, -2.0, -2.0]), "{:?}", bbox(&s, "S"));
+    let v: Value = serde_json::from_str(&s.doc.to_json()).unwrap();
+    let feats = v["features"].as_array().unwrap();
+    assert_eq!(feats[0]["center"], json!(["shift", 0.0, 0.0]), "{v}");
+    assert_eq!(feats[1]["corner"], json!([1.5, -2.0, 0.0]), "{v}");
+}
+
+/// A primitive placed by an expression in a moved component lands in that component's frame
+/// and keeps following its parameter.
+#[test]
+fn point_expressions_in_a_moved_component() {
+    let mut s = Session::default();
+    run(&mut s, "parameters.change", json!({"name": "x0", "expression": "10 mm"}));
+    let c = run(&mut s, "component.create", json!({"name": "Arm"}));
+    let (cid, occ) = (c["component"].as_u64().unwrap(), c["occurrence"].as_u64().unwrap());
+    run(&mut s, "component.activate", json!({"component": 0}));
+    run(&mut s, "occurrence.move", json!({"occurrence": occ, "translate": [0, 50, 0]}));
+    run(&mut s, "component.activate", json!({"component": cid}));
+    run(&mut s, "solid.cylinder", json!({"radius": 1, "height": 2, "base": ["x0", 60, 0], "body_name": "Pin"}));
+    let wlo = |s: &Session| s.world_state().body("Pin").unwrap().mesh().bounds().min;
+    let lo = wlo(&s);
+    assert!((lo.x - 9.0).abs() < 1e-3 && (lo.y - 59.0).abs() < 1e-3, "{lo:?}");
+    run(&mut s, "parameters.change", json!({"name": "x0", "expression": "20 mm"}));
+    let lo = wlo(&s);
+    assert!((lo.x - 19.0).abs() < 1e-3 && (lo.y - 59.0).abs() < 1e-3, "{lo:?}");
+}