From ce5762d87370eb08ca150f79cc9fb27e5537a9fe Mon Sep 17 00:00:00 2001 From: semmelrinde <124386289+semmelrinde@users.noreply.github.com> Date: Tue, 13 Oct 2026 16:44:10 +0200 Subject: [PATCH] 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 --- crates/ui-egui/src/dialogs.rs | 70 ++++-- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/preselect_tests.rs | 6 +- crates/ui-egui/src/primitive_units_tests.rs | 230 ++++++++++++++++++++ docs/book/src/guide/parameters.md | 5 +- 5 files changed, 294 insertions(+), 19 deletions(-) create mode 100644 crates/ui-egui/src/primitive_units_tests.rs diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 75ed51da..35479eb4 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -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::() { - 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) -> Option) -> Option) -> Option { 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) -> Option 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 = 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!((¶ms["length"], ¶ms["width"], ¶ms["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 { + 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 = 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::>()); + assert_shown(&h, &shown); + ok(&mut h); + assert_eq!(stored(&h, id), want.iter().map(|(_, v)| (*v).to_string()).collect::>()); + 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]); +} diff --git a/docs/book/src/guide/parameters.md b/docs/book/src/guide/parameters.md index 965e0c56..28e0cb6d 100644 --- a/docs/book/src/guide/parameters.md +++ b/docs/book/src/guide/parameters.md @@ -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