mirror of
https://github.com/bherbruck/solidcraft.git
synced 2026-11-03 10:35:58 +00:00
Surface Extrude: a symmetric distance is the total, as for a solid extrude
A symmetric surface.extrude given no convention now splits its distance evenly around the start plane (10 mm: 5 mm each side), the way solid.extrude reads it, instead of going the distance each way. Both commands take their default from one function, which a session setting (surface_extrude_symmetric_per_side) can turn back to the per-side reading for surface extrudes. timeline.edit and timeline.redefine give a one-sided surface extrude made symmetric the same default. Saved designs are unchanged: every surface extrude saved its convention (legacy_per_side), and one saved before the marker existed reads per side as it always did. The help of both commands says how the distance is read. The 3830 kernel case, recorded with the per-side reading, now says so explicitly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1047,12 +1047,16 @@ fn edit_feature(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
resolve_feature_names(&mut v, &s.doc, cmd)?;
|
||||
let mut nf: Feature = serde_json::from_value(v.clone()).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)
|
||||
// A one-sided extrude made symmetric takes the convention a new one would: its old marker
|
||||
// never read a symmetric distance.
|
||||
let maker = if matches!(f.kind, FeatureKind::SurfaceExtrude { .. }) { "surface.extrude" } else { "solid.extrude" };
|
||||
if let (FeatureKind::Extrude { extent: old, .. }, FeatureKind::Extrude { extent: new, .. })
|
||||
| (FeatureKind::SurfaceExtrude { extent: old, .. }, FeatureKind::SurfaceExtrude { 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;
|
||||
new.convention = super::features::default_extrude_convention(s, maker);
|
||||
}
|
||||
if std::mem::discriminant(&nf.kind) != std::mem::discriminant(&f.kind) {
|
||||
return Err(bad(cmd, "the feature type cannot change"));
|
||||
@@ -1410,10 +1414,10 @@ fn redefine(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
&& 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 {
|
||||
let convention = if extent.direction == solidcraft_doc::Direction::Symmetric {
|
||||
extent.convention
|
||||
} else {
|
||||
solidcraft_doc::ExtrudeConvention::TotalDistance
|
||||
super::features::default_extrude_convention(s, cid)
|
||||
};
|
||||
parameters.entry("convention").or_insert_with(|| json!(convention));
|
||||
}
|
||||
|
||||
@@ -123,23 +123,102 @@ fn symmetric_extrude_thin_wall_uses_total_distance_and_to_object_remains_one_sid
|
||||
check(&fixture(json!({"to":[0,0,10]})), 0.0, 10.0);
|
||||
}
|
||||
|
||||
/// The z range of surface body `name`.
|
||||
fn sheet_z(s: &Session, name: &str) -> (f64, f64) {
|
||||
assert!(s.model.results.iter().all(|r| r.error.is_none()));
|
||||
let st = s.model.state();
|
||||
let bounds = st.body(name).unwrap().mesh().bounds();
|
||||
(bounds.min.z, bounds.max.z)
|
||||
}
|
||||
|
||||
fn assert_z(s: &Session, name: &str, lo: f64, hi: f64) {
|
||||
let (z0, z1) = sheet_z(s, name);
|
||||
assert!((z0 - lo).abs() < 1e-7 && (z1 - hi).abs() < 1e-7, "{name}: {z0}..{z1}, want {lo}..{hi}");
|
||||
}
|
||||
|
||||
/// A symmetric surface extrude reads its distance as a solid one does: the total, split evenly
|
||||
/// around the start plane, through a redefine and a save.
|
||||
#[test]
|
||||
fn surface_extrude_shared_extent_retains_its_existing_per_side_semantics() {
|
||||
fn surface_extrude_symmetric_distance_is_the_total_like_solid_extrude() {
|
||||
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);
|
||||
assert_z(&s, "Sheet", -5.0, 5.0);
|
||||
let FeatureKind::SurfaceExtrude { extent, .. } = &s.doc.find_feature("Sheet").unwrap().kind else { panic!("surface extrude") };
|
||||
assert_eq!(extent.convention, ExtrudeConvention::TotalDistance);
|
||||
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()));
|
||||
assert_z(&s, "Sheet", -5.0, 5.0);
|
||||
assert_z(&Session::new(Document::from_json(&s.doc.to_json()).unwrap()), "Sheet", -5.0, 5.0);
|
||||
// A one-sided surface extrude made symmetric splits its distance too, as a solid one does.
|
||||
run(&mut s, "surface.extrude", json!({"sketch":"Square","distance":"10 mm","convention":"legacy_per_side","name":"Wall","body_name":"Wall"}));
|
||||
assert_z(&s, "Wall", 0.0, 10.0);
|
||||
run(&mut s, "timeline.edit", json!({"feature":"Wall","set":{"extent":{"direction":"symmetric"}}}));
|
||||
assert_z(&s, "Wall", -5.0, 5.0);
|
||||
}
|
||||
|
||||
/// Surface extrudes saved before they matched solid ones stored `legacy_per_side` (or, older
|
||||
/// still, no marker): they rebuild the distance on each side, and keep it through edits.
|
||||
#[test]
|
||||
fn older_symmetric_surface_extrudes_keep_the_distance_on_each_side() {
|
||||
let mut s = fixture(json!({}));
|
||||
run(
|
||||
&mut s,
|
||||
"surface.extrude",
|
||||
json!({"sketch":"Square","distance":"10 mm","direction":"symmetric","convention":"legacy_per_side","name":"Sheet","body_name":"Sheet"}),
|
||||
);
|
||||
assert_z(&s, "Sheet", -10.0, 10.0);
|
||||
let marked: Value = serde_json::from_str(&s.doc.to_json()).unwrap();
|
||||
let mut unmarked = marked.clone();
|
||||
unmarked["format"] = json!("solidcraft/21");
|
||||
unmarked.as_object_mut().unwrap().remove("format_pending");
|
||||
for f in unmarked["features"].as_array_mut().unwrap() {
|
||||
if let Some(extent) = f.get_mut("extent").and_then(Value::as_object_mut) {
|
||||
extent.remove("convention");
|
||||
}
|
||||
}
|
||||
for saved in [marked, unmarked] {
|
||||
let mut s = Session::new(Document::from_json(&saved.to_string()).unwrap());
|
||||
assert_z(&s, "Sheet", -10.0, 10.0);
|
||||
run(&mut s, "timeline.edit", json!({"feature":"Sheet","set":{"extent":{"distance":"12 mm"}}}));
|
||||
assert_z(&s, "Sheet", -12.0, 12.0);
|
||||
run(
|
||||
&mut s,
|
||||
"timeline.redefine",
|
||||
json!({"feature":"Sheet","command":"surface.extrude","params":{"sketch":"Square","distance":"10 mm","direction":"symmetric","body_name":"Sheet"}}),
|
||||
);
|
||||
assert_z(&s, "Sheet", -10.0, 10.0);
|
||||
}
|
||||
}
|
||||
|
||||
/// The preference brings back the per-side reading for new surface extrudes only: solid
|
||||
/// extrudes and an explicit `convention` are unchanged by it.
|
||||
#[test]
|
||||
fn the_per_side_preference_keeps_the_older_surface_extrude_reading() {
|
||||
let mut s = fixture(json!({}));
|
||||
s.surface_extrude_symmetric_per_side = true;
|
||||
run(&mut s, "surface.extrude", json!({"sketch":"Square","distance":"10 mm","direction":"symmetric","name":"Sheet","body_name":"Sheet"}));
|
||||
assert_z(&s, "Sheet", -10.0, 10.0);
|
||||
let FeatureKind::SurfaceExtrude { extent, .. } = &s.doc.find_feature("Sheet").unwrap().kind else { panic!("surface extrude") };
|
||||
assert_eq!(extent.convention, ExtrudeConvention::LegacyPerSide);
|
||||
run(
|
||||
&mut s,
|
||||
"surface.extrude",
|
||||
json!({"sketch":"Square","distance":"10 mm","direction":"symmetric","convention":"total_distance","name":"Total","body_name":"Total"}),
|
||||
);
|
||||
assert_z(&s, "Total", -5.0, 5.0);
|
||||
run(&mut s, "solid.extrude", json!({"sketch":"Square","distance":"10 mm","direction":"symmetric","operation":"new","body_name":"Solid"}));
|
||||
assert_z(&s, "Solid", -5.0, 5.0);
|
||||
// The saved marker, not the preference, decides on reopening.
|
||||
s.surface_extrude_symmetric_per_side = false;
|
||||
let reopened = Session::new(Document::from_json(&s.doc.to_json()).unwrap());
|
||||
assert_z(&reopened, "Sheet", -10.0, 10.0);
|
||||
// A design opened or started keeps the user's choice.
|
||||
s.surface_extrude_symmetric_per_side = true;
|
||||
s.replace_design(Session::default());
|
||||
assert!(s.surface_extrude_symmetric_per_side);
|
||||
}
|
||||
|
||||
/// A second distance goes the other way from the first side (#2700): along the normal when the
|
||||
|
||||
@@ -25,7 +25,7 @@ pub static COMMANDS: &[CommandSpec] = &[
|
||||
.key("E")
|
||||
.contract(&EXTRUDE_CONTRACT)
|
||||
.output(output_schema_for::<FeatureResult>)
|
||||
.params("distance: expr (or through_all: true); taper?: angle expr (positive: outward, so the far end is larger along the extrusion direction; negative: inward, so the far end is smaller); corner?: preserve_curvature|tangent|round (how a taper closes a sharp corner its offset opens: each wall continues as itself, a line's plane or an arc's cone, until it meets its neighbour; each continues straight along its end tangent; or an arc about the corner. Without it, each profile preserves curvature where every curve is a line or an arc and takes tangent where another curve has a corner, and the feature's warning says so; a corner rule given is kept, and refused where the kernel cannot build it); 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; a pick on a drawn profile that reference geometry, such as the projected outline of the face the sketch is on, cuts into regions takes the whole drawn profile); exact_regions?: bool (each index or point pick is exactly the region it names, as the dialogs send); default: every drawn profile, except that a loop inside another is its hole and a loop inside that hole is material again; face?: [x,y,z] (extrude a planar body face instead of a sketch profile); faces?: [[x,y,z],…] (coplanar faces of one body); 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?: expr (Two Sides: a second distance the other way from the first side, which may be through_all or to; with direction: negative the first side goes against the normal and distance2 along it); 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); touching?: separate|one_body (what a join makes of pieces that share no face: separate, the default, makes each piece that touches another only along an edge or at a point a body of its own and keeps pieces that touch nothing in one body; one_body keeps every piece in one body); targets?: [body] (Join from a face, or from a sketch on a body's face, defaults to that face's body; explicit targets or participants override); 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 (positive: outward, so the far end is larger along the extrusion direction; negative: inward, so the far end is smaller); corner?: preserve_curvature|tangent|round (how a taper closes a sharp corner its offset opens: each wall continues as itself, a line's plane or an arc's cone, until it meets its neighbour; each continues straight along its end tangent; or an arc about the corner. Without it, each profile preserves curvature where every curve is a line or an arc and takes tangent where another curve has a corner, and the feature's warning says so; a corner rule given is kept, and refused where the kernel cannot build it); 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; a pick on a drawn profile that reference geometry, such as the projected outline of the face the sketch is on, cuts into regions takes the whole drawn profile); exact_regions?: bool (each index or point pick is exactly the region it names, as the dialogs send); default: every drawn profile, except that a loop inside another is its hole and a loop inside that hole is material again; face?: [x,y,z] (extrude a planar body face instead of a sketch profile); faces?: [[x,y,z],…] (coplanar faces of one body); 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 (how a symmetric distance is read; default total_distance, split evenly around the start plane; legacy_per_side: the distance on each side, as older symmetric extrudes keep); distance2?: expr (Two Sides: a second distance the other way from the first side, which may be through_all or to; with direction: negative the first side goes against the normal and distance2 along it); 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); touching?: separate|one_body (what a join makes of pieces that share no face: separate, the default, makes each piece that touches another only along an edge or at a point a body of its own and keeps pieces that touch nothing in one body; one_body keeps every piece in one body); targets?: [body] (Join from a face, or from a sketch on a body's face, defaults to that face's body; explicit targets or participants override); 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")
|
||||
@@ -1120,6 +1120,18 @@ fn thin_regions(s: &Session, ss: &solidcraft_doc::SolvedSketch, thin: &ThinWall)
|
||||
solidcraft_sketch::thin_wall_regions(&curves, inside, outside).map_err(|e| format!("thin wall: {e}"))
|
||||
}
|
||||
|
||||
/// How a new extent of `cmd` (`solid.extrude` or `surface.extrude`) reads a symmetric distance
|
||||
/// when the call doesn't say: the total, split evenly around the start plane, for both. A surface
|
||||
/// extrude reads it on each side, as surface extrudes did before, while the user's preference
|
||||
/// says so ([`Session::surface_extrude_symmetric_per_side`]).
|
||||
pub(crate) fn default_extrude_convention(s: &Session, cmd: &str) -> solidcraft_doc::ExtrudeConvention {
|
||||
if cmd == "surface.extrude" && s.surface_extrude_symmetric_per_side {
|
||||
solidcraft_doc::ExtrudeConvention::LegacyPerSide
|
||||
} else {
|
||||
solidcraft_doc::ExtrudeConvention::TotalDistance
|
||||
}
|
||||
}
|
||||
|
||||
/// An extrude's extent from its parameters: distance (or through all, or to an object), the
|
||||
/// side(s), a second distance, a start offset, a taper, and from an object.
|
||||
pub(super) fn extent_param(s: &Session, p: &Value, cmd: &str) -> Result<Extent> {
|
||||
@@ -1156,9 +1168,7 @@ pub(super) fn extent_param(s: &Session, p: &Value, cmd: &str) -> Result<Extent>
|
||||
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,
|
||||
None => default_extrude_convention(s, cmd),
|
||||
Some(value) => serde_json::from_value(value.clone()).map_err(|_| bad(cmd, "`convention` must be total_distance or legacy_per_side"))?,
|
||||
};
|
||||
Ok(Extent {
|
||||
|
||||
@@ -39,7 +39,7 @@ pub static COMMANDS: &[CommandSpec] = &[
|
||||
CommandSpec::new("surface.extrude", "Surface Extrude", surface_extrude)
|
||||
.at("SURFACE", "CREATE")
|
||||
.icon("surface_extrude")
|
||||
.params("sketch?: id|name; curves?: [curve ids] (open or closed chains of curves meeting end to end) and/or profiles? (their boundaries; the default when no curves are given); distance: expr; direction?: positive|negative|symmetric; distance2?: expr; start_offset?: expr; name?, body_name? (a surface body: walls only, no caps)"),
|
||||
.params("sketch?: id|name; curves?: [curve ids] (open or closed chains of curves meeting end to end) and/or profiles? (their boundaries; the default when no curves are given); distance: expr; direction?: positive|negative|symmetric (symmetric: distance is the total, split evenly around the start plane, as solid.extrude); convention?: total_distance|legacy_per_side (as solid.extrude; default total_distance, or legacy_per_side, the distance on each side, under the user's preference for the older reading); distance2?: expr; start_offset?: expr; name?, body_name? (a surface body: walls only, no caps)"),
|
||||
CommandSpec::new("surface.revolve", "Surface Revolve", surface_revolve)
|
||||
.at("SURFACE", "CREATE")
|
||||
.icon("surface_revolve")
|
||||
@@ -797,7 +797,8 @@ mod tests {
|
||||
// The line's wall and the half circle's (radius 5): length × height.
|
||||
let want = (10.0 + std::f64::consts::PI * 5.0) * 4.0;
|
||||
assert!((area(&s, "Wall") - want).abs() < want * 1e-3, "{} vs {want}", area(&s, "Wall"));
|
||||
run(&mut s, "surface.extrude", json!({"sketch": "S", "curves": ["l1"], "distance": 2, "direction": "symmetric", "body_name": "Both"}));
|
||||
// Symmetric: the distance is the total, split evenly around the sketch plane.
|
||||
run(&mut s, "surface.extrude", json!({"sketch": "S", "curves": ["l1"], "distance": 4, "direction": "symmetric", "body_name": "Both"}));
|
||||
let bb = solidcraft_kernel::measure(&s.world_state().unwrap().body("Both").unwrap().body).unwrap().bbox;
|
||||
assert!((bb.min.z + 2.0).abs() < 1e-9 && (bb.max.z - 2.0).abs() < 1e-9, "{bb:?}");
|
||||
run(&mut s, "edit.undo", json!({}));
|
||||
|
||||
@@ -380,6 +380,10 @@ pub struct Session {
|
||||
/// A new metric tapped hole given no `diameter` drills the major diameter less the pitch,
|
||||
/// as before the tap maker's drills (a user preference); false: the tap maker's drill.
|
||||
pub metric_tap_drill_major_less_pitch: bool,
|
||||
/// A new symmetric surface extrude given no `convention` reads its distance on each side, as
|
||||
/// surface extrudes did before they matched solid ones (a user preference); false: the total,
|
||||
/// split evenly around the start plane, as a solid extrude reads it.
|
||||
pub surface_extrude_symmetric_per_side: bool,
|
||||
/// How calls that break a command's parameter contract are treated (off by default). It
|
||||
/// belongs to the session, not the design, so it survives opening and starting designs.
|
||||
pub param_check: schema::ParamCheck,
|
||||
@@ -600,6 +604,7 @@ impl Session {
|
||||
followed: None,
|
||||
auto_project: true,
|
||||
metric_tap_drill_major_less_pitch: false,
|
||||
surface_extrude_symmetric_per_side: false,
|
||||
param_check: schema::ParamCheck::Off,
|
||||
easyprint_upload: None,
|
||||
live_links: None,
|
||||
@@ -647,6 +652,7 @@ impl Session {
|
||||
pub fn replace_design(&mut self, fresh: Session) {
|
||||
let auto_project = self.auto_project;
|
||||
let metric_tap_drill_major_less_pitch = self.metric_tap_drill_major_less_pitch;
|
||||
let surface_extrude_symmetric_per_side = self.surface_extrude_symmetric_per_side;
|
||||
let param_check = self.param_check;
|
||||
let easyprint_upload = self.easyprint_upload.take();
|
||||
let render_viewport = self.render_viewport.clone();
|
||||
@@ -663,6 +669,7 @@ impl Session {
|
||||
self.command_check = command_check;
|
||||
self.auto_project = auto_project;
|
||||
self.metric_tap_drill_major_less_pitch = metric_tap_drill_major_less_pitch;
|
||||
self.surface_extrude_symmetric_per_side = surface_extrude_symmetric_per_side;
|
||||
self.param_check = param_check;
|
||||
self.easyprint_upload = easyprint_upload;
|
||||
self.render_viewport = render_viewport;
|
||||
@@ -1406,6 +1413,7 @@ impl Session {
|
||||
followed: None,
|
||||
auto_project: self.auto_project,
|
||||
metric_tap_drill_major_less_pitch: self.metric_tap_drill_major_less_pitch,
|
||||
surface_extrude_symmetric_per_side: self.surface_extrude_symmetric_per_side,
|
||||
param_check: self.param_check,
|
||||
easyprint_upload: self.easyprint_upload.clone(),
|
||||
live_links: None,
|
||||
|
||||
@@ -105,6 +105,7 @@
|
||||
],
|
||||
"distance": 30,
|
||||
"direction": "symmetric",
|
||||
"convention": "legacy_per_side",
|
||||
"body_name": "Body2"
|
||||
}
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user