diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index d9859394..ed6c69d8 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -2928,7 +2928,14 @@ fn eval_feature(doc: &Document, vals: &BTreeMap, f: &Feature, st: Ok(()) } FeatureKind::ConstructionPlane { plane } => { - let pl = doc.resolve_plane_in(vals, plane, 0, Some(st))?; + // On a body face: follow the face as it is now, as a sketch on that face does. + let pl = match crate::face_anchor::plane_face_anchor(plane) { + Some(anchor) => { + let fst = crate::state_in_face_anchor_frame(doc, st, anchor, f.component)?; + doc.resolve_attached_plane(vals, plane, f.component, st, &fst)? + } + None => doc.resolve_plane_in(vals, plane, 0, Some(st))?, + }; st.construct.push(crate::construct::Construct { feature: f.id, name: f.name.clone(), geom: crate::construct::ConstructGeom::Plane(pl) }); Ok(()) } diff --git a/crates/engine/src/cmd/features.rs b/crates/engine/src/cmd/features.rs index f423c476..8a373f20 100644 --- a/crates/engine/src/cmd/features.rs +++ b/crates/engine/src/cmd/features.rs @@ -87,7 +87,7 @@ pub static COMMANDS: &[CommandSpec] = &[ CommandSpec::new("construct.plane.offset", "Offset Plane", plane_offset) .at("SOLID", "CONSTRUCT") .icon("plane") - .params("base: XY|XZ|YZ|plane name; offset: expr; name?"), + .params("base: XY|XZ|YZ|plane name, or a planar face {face: [x,y,z], body?, index?, path?}; offset: expr; name?"), CommandSpec::new("construct.plane.at_angle", "Plane at Angle", plane_angle) .at("SOLID", "CONSTRUCT") .icon("plane") @@ -359,10 +359,17 @@ pub(super) fn check_expr(s: &Session, e: &str, kind: Kind, cmd: &str, what: &str /// Add a feature, evaluate, and report its result (an evaluation error fails the command). pub(super) fn add_feature(s: &mut Session, p: &Value, kind: FeatureKind) -> Result { + add_feature_in(s, p, kind, true) +} + +/// `add_feature`; `world` is false when `kind` is already in the active component's frame. +fn add_feature_in(s: &mut Session, p: &Value, kind: FeatureKind, world: bool) -> Result { let name = str_(p, "name"); // Commands speak world coordinates; the feature is authored in the active component. let mut kind = kind; - super::component::to_active_frame(s, &mut kind); + if world { + super::component::to_active_frame(s, &mut kind); + } // Bare lengths typed in an inch (or cm…) design mean that unit. if s.doc.units.trim() != "mm" { let doc = s.doc.clone(); @@ -1007,9 +1014,17 @@ pub(super) fn plane_param(s: &Session, v: Option<&Value>, cmd: &str) -> Result Result { let cmd = "construct.plane.offset"; - let base = plane_param(s, p.get("base"), cmd)?; let d = req_expr(cmd, p, "offset")?; check_expr(s, &d, Kind::Length, cmd, "offset")?; + // A planar body face: attached exactly, as a sketch on that face is, so the plane follows it. + if let Some(face) = p.get("base").filter(|b| b.get("face").is_some()) { + let base = super::sketch::face::capture(s, face, cmd)?; + super::sketch::face::capture_positions(s, &base)?; + let kind = FeatureKind::ConstructionPlane { plane: solvecraft_doc::PlaneRef::Offset { base: Box::new(base), distance: d } }; + // The capture is already in the authoring frame. + return add_feature_in(s, p, kind, false); + } + let base = plane_param(s, p.get("base"), cmd)?; add_feature(s, p, FeatureKind::ConstructionPlane { plane: solvecraft_doc::PlaneRef::Offset { base: Box::new(base), distance: d } }) } @@ -1434,3 +1449,7 @@ mod offset_identity_tests; #[cfg(test)] #[path = "offset_edit_guard_tests.rs"] mod offset_edit_guard_tests; + +#[cfg(test)] +#[path = "offset_plane_face_tests.rs"] +mod offset_plane_face_tests; diff --git a/crates/engine/src/cmd/offset_plane_face_tests.rs b/crates/engine/src/cmd/offset_plane_face_tests.rs new file mode 100644 index 00000000..148c62ff --- /dev/null +++ b/crates/engine/src/cmd/offset_plane_face_tests.rs @@ -0,0 +1,82 @@ +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +use crate::Session; +use serde_json::{Value, json}; +use solvecraft_doc::construct::ConstructGeom; +use solvecraft_doc::{Document, FeatureKind, PlaneRef}; +use solvecraft_geom::Plane; + +fn run(s: &mut Session, command: &str, params: Value) -> Value { + s.execute(command, ¶ms).unwrap_or_else(|e| panic!("{command} {params}: {e}")) +} + +fn plane(s: &Session, name: &str) -> Plane { + let state = s.model.state(); + match state.construct.iter().find(|c| c.name == name).map(|c| &c.geom) { + Some(ConstructGeom::Plane(p)) => *p, + other => panic!("construction plane `{name}`: {other:?}"), + } +} + +fn volume(s: &Session) -> f64 { + s.model.state().bodies.iter().map(|b| solvecraft_kernel::measure(&b.body).unwrap().volume).sum() +} + +#[test] +fn offset_plane_from_a_planar_face_follows_the_face() { + let mut s = Session::default(); + run(&mut s, "solid.box", json!({"length": 40, "width": 30, "height": 20, "name": "Base"})); + + // Top face (z = 20), 5 mm out along its normal. + run(&mut s, "construct.plane.offset", json!({"base": {"face": [5, 5, 20]}, "offset": 5, "name": "Above"})); + let above = plane(&s, "Above"); + assert!((above.origin.z - 25.0).abs() < 1e-9, "{above:?}"); + assert!((above.normal().z.abs() - 1.0).abs() < 1e-9, "{above:?}"); + // Stored as an exact face attachment, not a frozen copy of the plane. + let feature = s.doc.find_feature("Above").unwrap(); + let FeatureKind::ConstructionPlane { plane: PlaneRef::Offset { base, .. } } = &feature.kind else { panic!("offset plane") }; + assert!(matches!(base.as_ref(), PlaneRef::Face { anchor: Some(_), .. }), "{base:?}"); + + // Side face (x = 40), 3 mm out. + run(&mut s, "construct.plane.offset", json!({"base": {"face": [40, 15, 10]}, "offset": 3, "name": "Side"})); + let side = plane(&s, "Side"); + assert!((side.origin.x - 43.0).abs() < 1e-9, "{side:?}"); + assert!((side.normal().x.abs() - 1.0).abs() < 1e-9, "{side:?}"); + + // A 10 × 10 × 10 pad sketched on the new plane sits 5 mm above the box. + run(&mut s, "sketch.create", json!({"plane": "Above"})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [10, 10]})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"distance": 10, "operation": "new"})); + s.refresh(); + assert!(s.model.results.iter().all(|r| r.error.is_none()), "{:?}", s.model.results); + assert!((volume(&s) - (40.0 * 30.0 * 20.0 + 1000.0)).abs() < 1e-6, "{}", volume(&s)); + + // Taller and longer box: both planes move with their faces. + run(&mut s, "timeline.edit", json!({"feature": "Base", "set": {"height": "30", "length": "50"}})); + assert!((plane(&s, "Above").origin.z - 35.0).abs() < 1e-9); + assert!((plane(&s, "Side").origin.x - 53.0).abs() < 1e-9); + assert!(s.model.results.iter().all(|r| r.error.is_none()), "{:?}", s.model.results); + + // Saved and reopened, the attachment still resolves. + let reopened = Session::new(Document::from_json(&s.doc.to_json()).unwrap()); + assert!((plane(&reopened, "Above").origin.z - 35.0).abs() < 1e-9); + assert!((plane(&reopened, "Side").origin.x - 53.0).abs() < 1e-9); +} + +#[test] +fn offset_plane_rejects_bad_face_picks_without_panicking() { + let mut s = Session::default(); + run(&mut s, "solid.box", json!({"length": 40, "width": 30, "height": 20})); + for base in [ + json!({"face": [100, 100, 100]}), + json!({"face": [1e300, 0, 0]}), + json!({"face": "top"}), + json!({"face": [5, 5]}), + json!({"face": [5, 5, 20], "index": 0}), + json!({"face": [5, 5, 20], "body": 7, "index": 0}), + ] { + let features = s.doc.features.len(); + assert!(s.execute("construct.plane.offset", &json!({"base": base, "offset": 5})).is_err(), "{base}"); + assert_eq!(s.doc.features.len(), features, "{base}"); + } +} diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 63f19907..cc8a0d4a 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -382,7 +382,9 @@ impl Dialog { ), "solid.pipe" => Dialog::new(Kind::Pipe { diameter: "5 mm".into(), wall: String::new() }, vec![SelInput::new("Path", CURVES, true)]), "solid.bounding_solid" => Dialog::new(Kind::Stock { margin: "2 mm".into() }, vec![SelInput::new("Bodies (all if none)", BODIES, true)]), - "construct.plane.offset" => Dialog::new(Kind::OffsetPlane { offset: "10 mm".into() }, vec![SelInput::new("Plane", PLANES, false)]), + "construct.plane.offset" => { + Dialog::new(Kind::OffsetPlane { offset: "10 mm".into() }, vec![SelInput::new("Plane", PLANES | PLANAR_FACES, false)]) + } "construct.plane.at_angle" => Dialog::new( Kind::AnglePlane { angle: "45 deg".into() }, vec![SelInput::new("Plane", PLANES, false), SelInput::new("Axis", AXES, false)], @@ -2589,8 +2591,11 @@ fn dialog_commands(app: &SolveApp, d: &Dialog) -> Result, S Kind::OffsetPlane { offset } => { need(0, "a plane")?; let base = match sels(d, 0).first() { - Some(Sel::Plane { name }) => name.clone(), - _ => return Err("pick an origin or construction plane".into()), + Some(Sel::Plane { name }) => json!(name), + Some(face @ Sel::Face { .. }) => { + crate::sketch_attachment_ui::face_plane(app, face).ok_or("the picked face is no longer in the model")? + } + _ => return Err("pick an origin or construction plane, or a planar face".into()), }; ("construct.plane.offset", json!({"base": base, "offset": offset})) } diff --git a/crates/ui-egui/tests/scenarios/offset_plane_from_face.json b/crates/ui-egui/tests/scenarios/offset_plane_from_face.json new file mode 100644 index 00000000..dae079bf --- /dev/null +++ b/crates/ui-egui/tests/scenarios/offset_plane_from_face.json @@ -0,0 +1,14 @@ +[ + {"note": "#167: Offset Plane takes a planar face of a body, not only the origin and construction planes."}, + {"call": "engine.execute", "params": {"command": "solid.box", "params": {"length": 40, "width": 30, "height": 20, "name": "Base"}}}, + {"start": "construct.plane.offset"}, + {"expect": {"dialog": "OffsetPlane"}}, + {"click": {"world": [20, 15, 20]}}, + {"widget": "OK"}, + {"expect": {"errors": 0, "features": 2}}, + {"call": "engine.execute", "params": {"command": "sketch.create", "params": {"plane": "Plane1"}}}, + {"call": "engine.execute", "params": {"command": "sketch.rectangle.two_point", "params": {"p0": [0, 0], "p1": [10, 10]}}}, + {"call": "engine.execute", "params": {"command": "sketch.finish", "params": {}}}, + {"call": "engine.execute", "params": {"command": "solid.extrude", "params": {"distance": 10, "operation": "new", "body_name": "Pad"}}}, + {"expect": {"errors": 0, "bodies": 2, "bbox": {"body": "Pad", "min": [null, null, 30], "max": [null, null, 40]}}} +]