fix(ui): Box, Cylinder, Sphere, Torus and Coil dialogs show the unit of their sizes (#630) (#709)

The Box, Cylinder, Sphere, Torus and Coil dialogs now start their sizes in the design's units with the unit written out ("20 mm", "20 in"), like the other dialogs. OK makes the same sizes as before. Fixes #630.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
semmelrindeandClaude Opus 5.5 authored and GitHub committed 2026-10-13 10:44:10 -04:00
1 parent 831fa34752
commit ce5762d873
5 files changed
+294 -19

No files matched your search

+55 -15
View File
@@ -469,27 +469,43 @@ impl Dialog {
"solid.box" => Dialog::new(
Kind::Primitive {
cmd: "solid.box",
fields: vec![("length", "20".into()), ("width", "20".into()), ("height", "20".into())],
fields: vec![("length", length_default(s, "20")), ("width", length_default(s, "20")), ("height", length_default(s, "20"))],
operation: 0,
at: None,
},
vec![],
),
"solid.cylinder" => Dialog::new(
Kind::Primitive { cmd: "solid.cylinder", fields: vec![("diameter", "20".into()), ("height", "20".into())], operation: 0, at: None },
Kind::Primitive {
cmd: "solid.cylinder",
fields: vec![("diameter", length_default(s, "20")), ("height", length_default(s, "20"))],
operation: 0,
at: None,
},
vec![],
),
"solid.sphere" => Dialog::new(
Kind::Primitive { cmd: "solid.sphere", fields: vec![("diameter", length_default(s, "20"))], operation: 0, at: None },
vec![],
),
"solid.sphere" => {
Dialog::new(Kind::Primitive { cmd: "solid.sphere", fields: vec![("diameter", "20".into())], operation: 0, at: None }, vec![])
}
"solid.torus" => Dialog::new(
Kind::Primitive { cmd: "solid.torus", fields: vec![("major", "20".into()), ("minor", "5".into())], operation: 0, at: None },
Kind::Primitive {
cmd: "solid.torus",
fields: vec![("major", length_default(s, "20")), ("minor", length_default(s, "5"))],
operation: 0,
at: None,
},
vec![],
),
"solid.coil" => Dialog::new(
Kind::Primitive {
cmd: "solid.coil",
fields: vec![("diameter", "40".into()), ("revolutions", "4".into()), ("pitch", "8".into()), ("section_size", "3".into())],
fields: vec![
("diameter", length_default(s, "40")),
("revolutions", "4".into()),
("pitch", length_default(s, "8")),
("section_size", length_default(s, "3")),
],
operation: 0,
at: None,
},
@@ -2371,6 +2387,24 @@ fn fillet_kind(size: &str, chamfer: bool) -> Kind {
Kind::Fillet { radius: size.into(), chamfer, chain: true, ctype: 0, distance2: size.into(), angle: "45 deg".into(), flip: false }
}
/// A length field's first text: `number` in the design's units, with the unit written out so the
/// field shows it as the other dialogs show theirs ("20 mm", "20 in"). OK makes the size the bare
/// number would.
fn length_default(s: &Session, number: &str) -> String {
match s.doc.with_design_unit(number, ValueKind::Length) {
e if e != number => e,
// A millimetre design (or one whose units are not a length): bare numbers are mm.
_ => format!("{} mm", number.trim()),
}
}
/// A feature's length as its edit field shows it. A bare number is kept in millimetres: where the
/// design reads bare numbers in another unit, the field says "mm" (as the configuration table
/// does), so OK keeps the size. Anything else shows as stored.
fn stored_length(s: &Session, e: &str) -> String {
if s.doc.with_design_unit(e, ValueKind::Length) == e { e.to_string() } else { format!("{} mm", e.trim()) }
}
fn hole_defaults() -> Kind {
Kind::Hole {
diameter: "5 mm".into(),
@@ -3136,10 +3170,15 @@ fn box_corner(s: &Session, fields: &[(&'static str, String)], p: Vec3, normal: O
Some(_) => 1.0,
None => 0.5,
};
match size.trim().parse::<f64>() {
Ok(v) => json!(c - back * v),
Err(_) if back < 1.0 => json!(format!("{c} mm - ({size}) / 2")),
Err(_) => json!(format!("{c} mm - ({size})")),
// A plain length ("20 mm", "2 in"; a bare number has the design's unit by now) gives a
// plain number (mm).
let plain = solidcraft_engine::doc::expr::is_literal(&size)
.then(|| solidcraft_engine::doc::Document::eval_in(&Default::default(), &size, ValueKind::Length).ok())
.flatten();
match plain.filter(|v| v.is_finite()) {
Some(v) => json!(c - back * v),
None if back < 1.0 => json!(format!("{c} mm - ({size}) / 2")),
None => json!(format!("{c} mm - ({size})")),
}
};
json!([coord(p.x, size("length"), Vec3::X), coord(p.y, size("width"), Vec3::Y), coord(p.z, size("height"), Vec3::Z)])
@@ -3616,7 +3655,7 @@ pub fn for_feature(app: &SolveApp, id: u64, marker: Option<usize>) -> Option<Dia
let mut d = start("solid.box")?;
d.kind = Kind::Primitive {
cmd: "solid.box",
fields: vec![("length", length.clone()), ("width", width.clone()), ("height", height.clone())],
fields: vec![("length", stored_length(s, length)), ("width", stored_length(s, width)), ("height", stored_length(s, height))],
operation: op_index(operation),
at: None,
};
@@ -3627,7 +3666,7 @@ pub fn for_feature(app: &SolveApp, id: u64, marker: Option<usize>) -> Option<Dia
let mut d = start("solid.cylinder")?;
d.kind = Kind::Primitive {
cmd: "solid.cylinder",
fields: vec![("radius", radius.clone()), ("height", height.clone())],
fields: vec![("radius", stored_length(s, radius)), ("height", stored_length(s, height))],
operation: op_index(operation),
at: None,
};
@@ -3637,7 +3676,8 @@ pub fn for_feature(app: &SolveApp, id: u64, marker: Option<usize>) -> Option<Dia
}
FeatureKind::Sphere { center, radius, operation } => {
let mut d = start("solid.sphere")?;
d.kind = Kind::Primitive { cmd: "solid.sphere", fields: vec![("radius", radius.clone())], operation: op_index(operation), at: None };
d.kind =
Kind::Primitive { cmd: "solid.sphere", fields: vec![("radius", stored_length(s, radius))], operation: op_index(operation), at: None };
d.extra.insert("center".into(), pt_expr(center));
d
}
@@ -3645,7 +3685,7 @@ pub fn for_feature(app: &SolveApp, id: u64, marker: Option<usize>) -> Option<Dia
let mut d = start("solid.torus")?;
d.kind = Kind::Primitive {
cmd: "solid.torus",
fields: vec![("major", major.clone()), ("minor", minor.clone())],
fields: vec![("major", stored_length(s, major)), ("minor", stored_length(s, minor))],
operation: op_index(operation),
at: None,
};
+2
View File
@@ -76,6 +76,8 @@ pub mod prefs;
#[cfg(test)]
mod preselect_tests;
pub mod preview;
#[cfg(test)]
mod primitive_units_tests;
pub mod recovery_ui;
pub mod ref_images;
#[cfg(test)]
+4 -2
View File
@@ -879,8 +879,10 @@ fn primitives_go_on_the_picked_face() {
assert_eq!(p["axis"], json!([0.0, 0.0, 1.0]), "standing on the top face: {p}");
assert!(near(c, Vec3::new(8.0, 10.0, 30.0)), "{c:?}");
// A box stands on the face, centred on the picked point across it.
let (_, c) = primitive_on(&mut app, vec![top.clone()], "solid.box", |_| {});
// A box stands on the face, centred on the picked point across it. Sizes that are plain
// lengths ("20 mm") give a plain corner.
let (p, c) = primitive_on(&mut app, vec![top.clone()], "solid.box", |_| {});
assert_eq!(p["corner"], json!([-2.0, 0.0, 20.0]), "{p}");
assert!(near(c, Vec3::new(8.0, 10.0, 30.0)), "{c:?}");
let (p, c) = primitive_on(&mut app, vec![top], "solid.torus", |_| {});
+230
View File
@@ -0,0 +1,230 @@
//! #630: the Box, Cylinder, Sphere, Torus and Coil dialogs show the unit of their sizes, as the
//! other dialogs do ("10 mm"), in the design's units. OK makes what the bare numbers made before,
//! and a bare number typed into a field still means the design's units.
use serde_json::{Value, json};
use solidcraft_engine::Session;
use crate::dialogs::{Kind, apply_commands};
use crate::scenario::Harness;
/// The primitives with their sizes as a bare number reads them: the command and fields.
const PRIMITIVES: [(&str, &[(&str, &str)]); 5] = [
("solid.box", &[("length", "20"), ("width", "20"), ("height", "20")]),
("solid.cylinder", &[("diameter", "20"), ("height", "20")]),
("solid.sphere", &[("diameter", "20")]),
("solid.torus", &[("major", "20"), ("minor", "5")]),
("solid.coil", &[("diameter", "40"), ("revolutions", "4"), ("pitch", "8"), ("section_size", "3")]),
];
/// A design in `units`, with nothing in it.
fn design(units: &str) -> Harness {
let mut h = Harness::new();
h.app.home.open = false;
if units != "mm" {
h.app.run("document.units", json!({ "units": units })).unwrap();
}
h.frames(2);
h
}
/// The open dialog's fields.
fn fields(h: &Harness) -> Vec<(&'static str, String)> {
match h.app.dialog.as_ref().map(|d| &d.kind) {
Some(Kind::Primitive { fields, .. }) => fields.clone(),
other => panic!("no primitive dialog: {other:?}"),
}
}
/// Every field shows its text in its row, as a person sees it.
fn assert_shown(h: &Harness, fields: &[(&str, String)]) {
for (label, text) in fields {
assert!(h.widget_near(text, label).is_some(), "the {label} row shows {text:?}");
}
}
/// Replace a field's text by typing, as a person does.
fn type_into(h: &mut Harness, label: &str, text: &str) {
h.step(&json!({"click": {"handle": format!("dialog:{label}")}})).unwrap();
h.step(&json!({"key": "A", "cmd": true})).unwrap();
h.step(&json!({"text": text})).unwrap();
}
fn ok(h: &mut Harness) {
h.step(&json!({"click": {"handle": "dialog:OK"}})).unwrap();
h.frames(2);
assert!(h.app.dialog.is_none(), "OK closes the dialog: {:?}", h.app.dialog.as_ref().and_then(|d| d.error.clone()));
}
/// The volume of everything in the design (mm³).
fn volume(s: &mut Session) -> f64 {
s.execute("inspect.measure", &json!({})).unwrap()["total"]["volume_mm3"].as_f64().unwrap()
}
/// The size of the design's only body (mm).
fn size(s: &mut Session) -> [f64; 3] {
let r = s.execute("inspect.measure", &json!({})).unwrap();
let b = &r["bodies"][0]["bbox"];
std::array::from_fn(|i| b["max"][i].as_f64().unwrap() - b["min"][i].as_f64().unwrap())
}
fn assert_size(s: &mut Session, want: [f64; 3]) {
let got = size(s);
assert!(got.iter().zip(want).all(|(g, w)| (g - w).abs() < 1e-6), "{got:?}, want {want:?}");
}
/// The volume the command makes from the bare numbers in a design in `units` (what the dialog
/// made before its fields showed units).
fn bare_volume(units: &str, cmd: &str, sizes: &[(&str, &str)]) -> f64 {
let mut s = Session::default();
s.execute("document.units", &json!({ "units": units })).unwrap();
let params: serde_json::Map<String, Value> = sizes.iter().map(|(k, v)| ((*k).to_string(), json!(v))).collect();
s.execute(cmd, &Value::Object(params)).unwrap();
volume(&mut s)
}
#[test]
fn primitive_dialogs_show_millimetres_and_make_the_same_sizes() {
for (cmd, sizes) in PRIMITIVES {
let mut h = design("mm");
h.app.start(cmd);
h.frames(3);
let shown = fields(&h);
let want: Vec<(&str, String)> =
sizes.iter().map(|(k, v)| (*k, if *k == "revolutions" { (*v).to_string() } else { format!("{v} mm") })).collect();
assert_eq!(shown, want, "{cmd}");
assert_shown(&h, &shown);
// OK sends the fields' texts, units and all.
let (id, params) = apply_commands(&h.app, h.app.dialog.as_ref().unwrap()).unwrap().remove(0);
assert_eq!(id, cmd);
for (k, v) in &want {
assert_eq!(params[*k], json!(v), "{cmd} {k}");
}
ok(&mut h);
let made = volume(&mut h.app.session);
let before = bare_volume("mm", cmd, sizes);
assert!(made > 0.0 && (made - before).abs() < 1e-6 * before, "{cmd}: {made} mm³, the bare numbers made {before} mm³");
}
let mut h = design("mm");
h.app.start("solid.box");
h.frames(3);
ok(&mut h);
assert_size(&mut h.app.session, [20.0, 20.0, 20.0]);
}
#[test]
fn in_an_inch_design_primitive_dialogs_show_inches_and_read_bare_numbers_as_inches() {
for (cmd, sizes) in PRIMITIVES {
let mut h = design("in");
h.app.start(cmd);
h.frames(3);
let shown = fields(&h);
let want: Vec<(&str, String)> =
sizes.iter().map(|(k, v)| (*k, if *k == "revolutions" { (*v).to_string() } else { format!("{v} in") })).collect();
assert_eq!(shown, want, "{cmd}");
assert_shown(&h, &shown);
ok(&mut h);
// The same size the bare numbers made in an inch design.
let made = volume(&mut h.app.session);
let before = bare_volume("in", cmd, sizes);
assert!(made > 0.0 && (made - before).abs() < 1e-6 * before, "{cmd}: {made} mm³, the bare numbers made {before} mm³");
}
// A bare number typed into a field is in the design's units; a typed unit is honoured.
let mut h = design("in");
h.app.start("solid.box");
h.frames(3);
type_into(&mut h, "width", "2");
type_into(&mut h, "height", "10 mm");
h.frames(2);
let (_, params) = apply_commands(&h.app, h.app.dialog.as_ref().unwrap()).unwrap().remove(0);
assert_eq!((&params["length"], &params["width"], &params["height"]), (&json!("20 in"), &json!("2"), &json!("10 mm")));
ok(&mut h);
assert_size(&mut h.app.session, [508.0, 50.8, 10.0]);
}
#[test]
fn a_centimetre_design_shows_centimetres() {
let mut h = design("cm");
h.app.start("solid.sphere");
h.frames(3);
assert_eq!(fields(&h), vec![("diameter", "20 cm".to_string())]);
ok(&mut h);
assert_size(&mut h.app.session, [200.0, 200.0, 200.0]);
}
/// The sizes a primitive feature keeps, as stored.
fn stored(h: &Harness, id: u64) -> Vec<String> {
use solidcraft_engine::doc::FeatureKind;
match h.app.session.doc.feature(id).map(|f| &f.kind) {
Some(FeatureKind::Box { length, width, height, .. }) => vec![length.clone(), width.clone(), height.clone()],
Some(FeatureKind::Cylinder { radius, height, .. }) => vec![radius.clone(), height.clone()],
Some(FeatureKind::Sphere { radius, .. }) => vec![radius.clone()],
Some(FeatureKind::Torus { major, minor, .. }) => vec![major.clone(), minor.clone()],
other => panic!("not a primitive: {other:?}"),
}
}
#[test]
fn editing_a_primitive_after_the_design_goes_to_inches_keeps_its_millimetres() {
// Made by a script in a millimetre design, then the design goes to inches: bare sizes are
// still millimetres, and Edit shows them so; OK without a change keeps the part. Sizes with
// a unit, expressions and parameters show as they are kept.
let mut h = design("mm");
h.app.run("parameters.add", json!({"name": "w", "expression": "6 mm"})).unwrap();
h.app.run("solid.box", json!({"length": 40, "width": "w", "height": 20})).unwrap();
h.app.run("solid.cylinder", json!({"diameter": "10 mm", "height": 12, "base": [60, 0, 0], "operation": "new"})).unwrap();
h.app.run("solid.sphere", json!({"radius": 4, "center": [0, 60, 0], "operation": "new"})).unwrap();
h.app.run("solid.torus", json!({"major": 10, "minor": "2 mm", "center": [60, 60, 0], "operation": "new"})).unwrap();
h.app.run("document.units", json!({"units": "in"})).unwrap();
h.frames(2);
let before = volume(&mut h.app.session);
let ids: Vec<u64> = h.app.session.doc.features.iter().map(|f| f.id).collect();
let want: [&[(&str, &str)]; 4] = [
&[("length", "40 mm"), ("width", "w"), ("height", "20 mm")],
&[("radius", "(10 mm) / 2"), ("height", "12 mm")],
&[("radius", "4 mm")],
&[("major", "10 mm"), ("minor", "2 mm")],
];
assert_eq!(ids.len(), want.len());
for (id, want) in ids.into_iter().zip(want) {
h.app.edit_feature(id);
h.frames(3);
let shown = fields(&h);
assert_eq!(shown, want.iter().map(|(k, v)| (*k, (*v).to_string())).collect::<Vec<_>>());
assert_shown(&h, &shown);
ok(&mut h);
assert_eq!(stored(&h, id), want.iter().map(|(_, v)| (*v).to_string()).collect::<Vec<_>>());
let after = volume(&mut h.app.session);
assert!((after - before).abs() < 1e-6 * before, "the part is unchanged: {after} mm³, was {before} mm³");
}
// A size typed bare in the edit is in the design's units, as anywhere else.
let first = h.app.session.doc.features.first().map(|f| f.id).unwrap();
h.app.edit_feature(first);
h.frames(3);
type_into(&mut h, "height", "1");
h.frames(2);
ok(&mut h);
let r = h.app.session.execute("inspect.measure", &json!({})).unwrap();
let height = r["bodies"].as_array().unwrap().iter().map(|b| b["bbox"]["max"][2].as_f64().unwrap()).fold(f64::MIN, f64::max);
assert!((height - 25.4).abs() < 1e-6, "1 in tall: {height}");
}
#[test]
fn editing_in_a_millimetre_design_keeps_the_sizes_as_they_are_kept() {
// A bare number already means millimetres here: Edit shows and sends it as kept, so OK
// without a change leaves the feature (and the active configuration's match) alone.
let mut h = design("mm");
h.app.run("solid.box", json!({"length": 40, "width": 30, "height": 20})).unwrap();
let (id, length) = h.app.session.doc.features.first().map(|f| (f.id, f.param_names[0].clone())).unwrap();
h.app.run("config.configure", json!({ "params": [length] })).unwrap();
h.app.run("config.activate", json!({"row": "Default"})).unwrap();
assert!(h.app.session.doc.configuration_matches());
h.app.edit_feature(id);
h.frames(3);
let shown = fields(&h);
assert_eq!(shown, vec![("length", "40".to_string()), ("width", "30".to_string()), ("height", "20".to_string())]);
ok(&mut h);
assert_eq!(stored(&h, id), ["40", "30", "20"]);
assert!(h.app.session.doc.configuration_matches(), "an edit without a change keeps the configuration matched");
assert_size(&mut h.app.session, [40.0, 30.0, 20.0]);
}
+3 -2
View File
@@ -23,8 +23,9 @@ parameter updates every expression, dimension and feature input that uses it, an
in use can't be deleted (the error lists its users).
The design's units (Document Settings in the browser: mm, cm, m, in or ft) set how a bare number
typed into a feature is read. In an inch design `2` means 2 in. Changing the units never moves
existing geometry.
typed into a feature is read. In an inch design `2` means 2 in. The Box, Cylinder, Sphere, Torus
and Coil dialogs start with their sizes in the design's units, unit shown (`20 mm`, `20 in`).
Changing the units never moves existing geometry.
Parameters export to and import from CSV or JSON (`parameters.export`, `parameters.import`).
**Configurations** (`config.*`) keep a table of rows, each with its own parameter values and