From 1ec33fcdc770e8f664ccec40c9eecbcf7ad275b3 Mon Sep 17 00:00:00 2001 From: Karl Von Richter Date: Thu, 15 Oct 2026 15:19:55 +1000 Subject: [PATCH] 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 --- apps/solidcraft-cli/src/recipe.rs | 2 +- crates/doc/src/document.rs | 23 ++- crates/doc/src/eval.rs | 6 +- crates/doc/src/format.rs | 6 + crates/doc/src/format_offsets.rs | 1 + crates/doc/src/profile_refs.rs | 2 + crates/doc/src/tests.rs | 2 + crates/doc/src/walk.rs | 1 + crates/engine/src/cmd/edit.rs | 18 +++ .../src/cmd/extrude_convention_tests.rs | 143 ++++++++++++++++++ crates/engine/src/cmd/features.rs | 13 +- crates/engine/src/cmd/patterns_tests.rs | 3 +- crates/engine/src/cmd/pbr_appearance_tests.rs | 6 +- crates/engine/src/cmd/sketch_modify.rs | 6 +- crates/ui-egui/src/canvas.rs | 24 ++- crates/ui-egui/src/dialogs.rs | 3 + crates/ui-egui/src/dialogs_surface.rs | 9 +- crates/ui-egui/src/extrude_edit_tests.rs | 66 ++++++++ docs/book/src/guide/features.md | 7 + 19 files changed, 323 insertions(+), 18 deletions(-) create mode 100644 crates/engine/src/cmd/extrude_convention_tests.rs diff --git a/apps/solidcraft-cli/src/recipe.rs b/apps/solidcraft-cli/src/recipe.rs index c3b9afc0..68d5bfdd 100644 --- a/apps/solidcraft-cli/src/recipe.rs +++ b/apps/solidcraft-cli/src/recipe.rs @@ -92,7 +92,7 @@ fn extent(f: &Value, p: &mut Value) { "symmetric" => { let full = e.get("is_full_length").and_then(Value::as_bool).unwrap_or(false); p["direction"] = json!("symmetric"); - p["distance"] = if full { json!(format!("({}) / 2", expr_text(&dist))) } else { dist }; + p["distance"] = if full { dist } else { json!(format!("({}) * 2", expr_text(&dist))) }; } "two_sides" => { p["distance"] = e.get("side1").cloned().unwrap_or(Value::Null); diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index d9b19331..b199acb9 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -134,7 +134,7 @@ pub enum Direction { #[default] Positive, Negative, - /// Both sides; `distance` per side. + /// Both sides; the extent convention determines whether `distance` is total or per side. Symmetric, } @@ -186,12 +186,33 @@ pub enum RevolveConvention { SketchNormal, } +/// Symmetric distance semantics. Missing markers preserve existing designs' thickness. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ExtrudeConvention { + #[default] + LegacyPerSide, + TotalDistance, +} + +impl ExtrudeConvention { + /// Distance from the mid-plane to either cap for a distance-limited extent. + pub fn symmetric_half_distance(self, distance: f64) -> f64 { + match self { + Self::LegacyPerSide => distance, + Self::TotalDistance => distance * 0.5, + } + } +} + /// Extent of an extrude. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, solidcraft_derive::Walk)] pub struct Extent { #[walk(length = "Distance")] pub distance: String, #[serde(default)] + pub convention: ExtrudeConvention, + #[serde(default)] pub direction: Direction, /// Optional second side distance (two-sided extrude), along the negative normal. #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index c8c2eca7..63f5ce56 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -3291,7 +3291,11 @@ pub(crate) fn extent_sides(vals: &BTreeMap, extent: &crate::Exten (Some(d2), _) => (-val(vals, d2, Kind::Length)?, d), (None, crate::Direction::Positive) => (0.0, d), (None, crate::Direction::Negative) => (-d, 0.0), - (None, crate::Direction::Symmetric) => (-d, d), + (None, crate::Direction::Symmetric) => { + // Through All is a reach computed from target bodies, not an entered total. + let half = if extent.through_all { d } else { extent.convention.symmetric_half_distance(d) }; + (-half, half) + } }) } diff --git a/crates/doc/src/format.rs b/crates/doc/src/format.rs index eced0442..8b170882 100644 --- a/crates/doc/src/format.rs +++ b/crates/doc/src/format.rs @@ -179,6 +179,11 @@ //! neighbouring edges as they are (`neighbours: false`). An Extend without them builds the //! planar extension it always did and saves as before; 0.11.0 asks for an update instead of //! dropping them and extending a plane by the distance. +//! Under the same version, an extrude's extent records how a symmetric `distance` is read +//! (`convention`: `total_distance`, split evenly around the start plane, which new solid +//! extrudes save; or `legacy_per_side`). An extent without it keeps the per-side reading +//! designs always had, so their thickness and parameters don't change; 0.11.0 asks for an +//! update instead of doubling a total-distance extrude. use serde_json::Value; @@ -1131,6 +1136,7 @@ mod tests { sketch, profiles: ProfileSel::All, extent: Extent { + convention: crate::ExtrudeConvention::LegacyPerSide, distance: "10".into(), direction: Direction::Negative, distance2: None, diff --git a/crates/doc/src/format_offsets.rs b/crates/doc/src/format_offsets.rs index 8834a08e..aa3f52f4 100644 --- a/crates/doc/src/format_offsets.rs +++ b/crates/doc/src/format_offsets.rs @@ -427,6 +427,7 @@ mod tests { sketch: if missing_sketch { 999_999 } else { id }, profiles: ProfileSel::OffsetBoundaries { associations: vec!["deleted-o1".into()] }, extent: crate::Extent { + convention: crate::ExtrudeConvention::LegacyPerSide, distance: "2".into(), direction: crate::Direction::Positive, distance2: None, diff --git a/crates/doc/src/profile_refs.rs b/crates/doc/src/profile_refs.rs index 177a3dc5..dca0678a 100644 --- a/crates/doc/src/profile_refs.rs +++ b/crates/doc/src/profile_refs.rs @@ -312,6 +312,7 @@ mod tests { sketch: feature, profiles: selection.clone(), extent: crate::Extent { + convention: crate::ExtrudeConvention::LegacyPerSide, distance: "2".into(), direction: crate::Direction::Positive, distance2: None, @@ -438,6 +439,7 @@ mod tests { sketch: feature, profiles: selection.clone(), extent: crate::Extent { + convention: crate::ExtrudeConvention::LegacyPerSide, distance: "2".into(), direction: crate::Direction::Positive, distance2: None, diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index faeaae49..a4f5aaa9 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -33,6 +33,7 @@ fn plate_doc() -> (Document, u64) { sketch: s, profiles: ProfileSel::All, extent: Extent { + convention: crate::ExtrudeConvention::LegacyPerSide, distance: "20".into(), direction: Direction::Positive, distance2: None, @@ -125,6 +126,7 @@ fn box_fillet_cut_timeline() { sketch: s2, profiles: ProfileSel::Points { points: vec![Vec2::new(20.0, 15.0)] }, extent: Extent { + convention: crate::ExtrudeConvention::LegacyPerSide, distance: "20".into(), direction: Direction::Positive, distance2: None, diff --git a/crates/doc/src/walk.rs b/crates/doc/src/walk.rs index c9e23363..34499951 100644 --- a/crates/doc/src/walk.rs +++ b/crates/doc/src/walk.rs @@ -53,6 +53,7 @@ impl Walk for crate::document::ThinSide {} impl Walk for crate::document::ExtendMode {} impl Walk for crate::document::RuledMode {} impl Walk for crate::document::RuledCorners {} +impl Walk for crate::document::ExtrudeConvention {} /// Containers walk what they hold, in order. macro_rules! walk_each { diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 72d94c9e..339b6af0 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -626,6 +626,13 @@ fn edit_feature(s: &mut Session, p: &Value) -> Result { 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 { 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() diff --git a/crates/engine/src/cmd/extrude_convention_tests.rs b/crates/engine/src/cmd/extrude_convention_tests.rs new file mode 100644 index 00000000..def42697 --- /dev/null +++ b/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, ¶ms).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())); +} diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index 5feee761..012e199a 100644 --- a/crates/engine/src/cmd/features.rs +++ b/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 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; diff --git a/crates/engine/src/cmd/patterns_tests.rs b/crates/engine/src/cmd/patterns_tests.rs index 709cc5f3..ee9e7e38 100644 --- a/crates/engine/src/cmd/patterns_tests.rs +++ b/crates/engine/src/cmd/patterns_tests.rs @@ -367,7 +367,8 @@ fn sawtooth_bar(teeth: usize, pattern: bool) -> (Session, Option) { 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() diff --git a/crates/engine/src/cmd/pbr_appearance_tests.rs b/crates/engine/src/cmd/pbr_appearance_tests.rs index 9fdcccf9..00cd3e06 100644 --- a/crates/engine/src/cmd/pbr_appearance_tests.rs +++ b/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::(&saved).unwrap()["format"], "solidcraft/21"); + assert_eq!(serde_json::from_str::(&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(); diff --git a/crates/engine/src/cmd/sketch_modify.rs b/crates/engine/src/cmd/sketch_modify.rs index b0e06c49..0f321bbf 100644 --- a/crates/engine/src/cmd/sketch_modify.rs +++ b/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) -> Vec { @@ -1259,7 +1259,7 @@ fn offset(s: &mut Session, p: &Value) -> Result { 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), diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 662cc247..93cc4f54 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -338,6 +338,16 @@ fn draw(app: &SolveApp, ui: &mut egui::Ui, painter: &egui::Painter, proj: &Proj, _ => 1.0, }; let symmetric = to_span.is_none() && matches!(d.kind, Kind::Extrude { direction: 2, .. }); + let symmetric_scale = if symmetric { + d.extra + .get("convention") + .cloned() + .and_then(|v| serde_json::from_value::(v).ok()) + .unwrap_or(solidcraft_engine::doc::ExtrudeConvention::TotalDistance) + .symmetric_half_distance(1.0) + } else { + 1.0 + }; // Extrudes and moves go either way; radii and thicknesses stay positive. let signed = matches!( d.kind, @@ -416,17 +426,19 @@ fn draw(app: &SolveApp, ui: &mut egui::Ui, painter: &egui::Painter, proj: &Proj, if let (Some(n), Some(l)) = (dir, len.filter(|_| kind == ValueKind::Length)) { let n = n * sign; let baseline = to_span.map(f64::abs).unwrap_or(0.0); - let mut shown_value = format_len(l + baseline); + // Through All already supplies a reach per side; dragging switches to an entered distance. + let half = if all_span.is_some() { l } else { l * symmetric_scale }; + let mut shown_value = format_len(half + baseline); let original_shown = shown_value.clone(); - let dragged = if to_span.is_some() { &mut shown_value } else { &mut *value }; - if let Some(ts) = drag_arrow(ui, painter, proj, base, bs, n, l + baseline, signed, egui::Id::new("sc_manipulator"), half_height, dragged) { - if to_span.is_some() + let dragged = if to_span.is_some() || symmetric_scale != 1.0 { &mut shown_value } else { &mut *value }; + if let Some(ts) = drag_arrow(ui, painter, proj, base, bs, n, half + baseline, signed, egui::Id::new("sc_manipulator"), half_height, dragged) { + if (to_span.is_some() || symmetric_scale != 1.0) && shown_value != original_shown && let Ok(v) = app.session.doc.eval(&shown_value, ValueKind::Length) { - *value = format_len(v - baseline); + *value = format_len((v - baseline) / symmetric_scale); } - if symmetric && let Some(bt) = proj.to_screen(base - n * l) { + if symmetric && let Some(bt) = proj.to_screen(base - n * half) { arrow(painter, bs, bt, t.manipulator); } painter.circle(bs, 3.5, t.manipulator, Stroke::new(1.0, Color32::WHITE)); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index d72d28e4..d9665f2b 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -2087,6 +2087,8 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) { combo(ui, "ex_dir", &["One Side", "Two Sides", "Symmetric"], &mut pos); if pos != order.iter().position(|o| o == direction).unwrap_or(0) { *direction = order.get(pos).copied().unwrap_or(0); + // Selecting Symmetric applies the current total-distance semantics. + d.extra.insert("convention".into(), json!(solidcraft_engine::doc::ExtrudeConvention::TotalDistance)); // An explicit direction ends the automatic choices (#1257). smart.manual_dir = true; *auto_op = false; @@ -4194,6 +4196,7 @@ pub fn for_feature(app: &SolveApp, id: u64, marker: Option) -> Option Option<(K Direction::Symmetric => 1, Direction::Negative => 2, }; - (Surf::Extrude { distance: extent.distance.clone(), direction }, vec![sels]) + let k = Surf::Extrude { distance: extent.distance.clone(), direction }; + let mut ins = inputs(&k); + if let Some(input) = ins.first_mut() { + input.items = sels; + } + let mut extra = Map::new(); + extra.insert("convention".into(), json!(extent.convention)); + return Some((Kind::Surface(k), ins, extra)); } FeatureKind::Patch { sketch, edges, .. } => { let mut sels: Vec = edges.iter().filter_map(edge).collect(); diff --git a/crates/ui-egui/src/extrude_edit_tests.rs b/crates/ui-egui/src/extrude_edit_tests.rs index 82e1c297..39f4de18 100644 --- a/crates/ui-egui/src/extrude_edit_tests.rs +++ b/crates/ui-egui/src/extrude_edit_tests.rs @@ -46,6 +46,7 @@ fn unchanged_actual_editor_preserves_all_extents_and_parameter_identities() { json!({"direction":"negative","distance":"10"}), json!({"direction":"negative","distance":"-10 mm"}), json!({"direction":"symmetric","distance":"5 mm"}), + json!({"direction":"symmetric","distance":"5 mm","convention":"legacy_per_side"}), json!({"distance":"10 mm","distance2":"3 mm"}), json!({"from":[0,0,5],"start_offset":"0 mm"}), json!({"to":[0,0,10],"to_offset":"3 mm"}), @@ -295,3 +296,68 @@ fn dragging_to_update_value_reselects_whole_value_box_text() { assert_eq!(distance, "15 mm"); assert_eq!(selection(&h), (0, 5), "the drag grew the text length; the selection must cover all of it"); } + +#[test] +fn symmetric_total_distance_preview_handle_drag_apply_and_reopen_agree() { + for (convention, cap, total_after_drag) in [("total_distance", 50.0, 120.0), ("legacy_per_side", 100.0, 60.0)] { + let mut h = make(json!({"direction":"symmetric","distance":"100 mm","convention":convention})); + h.call("ui.view", json!({"view":"fit"})); + h.frames(4); + open(&mut h); + let expected_tip = h.at(&json!({"world":[5,5,cap]})).unwrap(); + let handle = h.at(&json!({"handle":"arrow"})).unwrap(); + assert!((handle[0] - expected_tip[0]).hypot(handle[1] - expected_tip[1]) < 1.0, "{convention}: {handle:?} expected {expected_tip:?}"); + h.step(&json!({"drag":[{"handle":"arrow"},{"world":[5,5,60]}]})).unwrap(); + let d = h.app.dialog.as_ref().unwrap(); + let crate::dialogs::Kind::Extrude { distance, .. } = &d.kind else { panic!("extrude") }; + assert_eq!(h.app.session.doc.eval(distance, solidcraft_engine::doc::expr::Kind::Length).unwrap(), total_after_drag); + let commands = crate::dialogs::apply_commands(&h.app, d).unwrap(); + let preview = h.app.session.preview(&commands).unwrap(); + let preview_bounds = preview.after.body("Main").unwrap().mesh().bounds(); + assert!((preview_bounds.min.z + 60.0).abs() < 1e-7 && (preview_bounds.max.z - 60.0).abs() < 1e-7); + h.step(&json!({"widget":"OK"})).unwrap(); + assert_eq!(h.app.session.world_state().body("Main").unwrap().mesh().bounds(), preview_bounds); + let path = h.dir.join("symmetric.solidcraft"); + h.app.run("file.save_as", json!({"path":path})).unwrap(); + h.app.run("doc.open", json!({"path":path})).unwrap(); + assert_eq!(h.app.session.world_state().body("Main").unwrap().mesh().bounds(), preview_bounds); + open(&mut h); + h.step(&json!({"widget":"OK"})).unwrap(); + assert_eq!(h.app.session.world_state().body("Main").unwrap().mesh().bounds(), preview_bounds); + } +} + +#[test] +fn switching_one_side_to_symmetric_in_actual_editor_keeps_total_thickness() { + for convention in ["total_distance", "legacy_per_side"] { + let mut h = make(json!({"convention":convention})); + open(&mut h); + h.step(&json!({"widget":"One Side"})).unwrap(); + h.step(&json!({"widget":"Symmetric"})).unwrap(); + let commands = crate::dialogs::apply_commands(&h.app, h.app.dialog.as_ref().unwrap()).unwrap(); + let preview = h.app.session.preview(&commands).unwrap(); + let bounds = preview.after.body("Main").unwrap().mesh().bounds(); + assert!((bounds.min.z + 5.0).abs() < 1e-7 && (bounds.max.z - 5.0).abs() < 1e-7); + h.step(&json!({"widget":"OK"})).unwrap(); + assert_eq!(h.app.session.world_state().body("Main").unwrap().mesh().bounds(), bounds); + assert_eq!(extent(&h).convention, solidcraft_engine::doc::ExtrudeConvention::TotalDistance); + } +} + +#[test] +fn symmetric_through_all_handle_drag_switches_to_total_distance() { + let mut h = Harness::new(); + h.app.run("solid.box", json!({"length":10,"width":10,"height":100,"corner":[0,0,-50],"body_name":"Reference"})).unwrap(); + let mut h = make_in(h, json!({"direction":"symmetric","through_all":true})); + h.call("ui.view", json!({"view":"fit"})); + h.frames(4); + open(&mut h); + h.step(&json!({"drag":[{"handle":"arrow"},{"world":[5,5,60]}]})).unwrap(); + let d = h.app.dialog.as_ref().unwrap(); + let crate::dialogs::Kind::Extrude { distance, all, .. } = &d.kind else { panic!("extrude") }; + assert!(!all); + assert_eq!(h.app.session.doc.eval(distance, solidcraft_engine::doc::expr::Kind::Length).unwrap(), 120.0); + h.step(&json!({"widget":"OK"})).unwrap(); + let bounds = h.app.session.world_state().body("Main").unwrap().mesh().bounds(); + assert!((bounds.min.z + 60.0).abs() < 1e-7 && (bounds.max.z - 60.0).abs() < 1e-7); +} diff --git a/docs/book/src/guide/features.md b/docs/book/src/guide/features.md index 9d634ab8..e6c72b79 100644 --- a/docs/book/src/guide/features.md +++ b/docs/book/src/guide/features.md @@ -55,6 +55,13 @@ hole identities also makes it lost. Older curve-span selections retain their exa multisets. If an older feature has already lost its profile after subdivision changed, edit the feature and reselect the intended current region to capture the new identity. +New symmetric solid extrusions treat the entered distance as the total thickness and split it evenly +around the start plane: 10 mm spans -5 to +5 mm on an XY sketch. Switching from One Side to +Symmetric keeps the entered total thickness. Through All still reaches targets in both +directions, and Two Sides retains its independent distances. Symmetric extrusions saved before +this change keep their original per-side distances when reopened or edited. Selecting another direction +in the feature editor uses the current total-distance convention. + New symmetric revolves split the entered angle evenly around the sketch profile, regardless of which way the selected axis points. Two Sides measures the second angle backward and the first angle forward along the sketch-normal sweep direction. Profiles on opposite sides of