From cab698bf122e420bb91818b7a2d4f52b0319faee Mon Sep 17 00:00:00 2001 From: lucas-aguilar89 Date: Sun, 11 Oct 2026 23:53:52 -0300 Subject: [PATCH] Offset Plane from a planar body face (#167) construct.plane.offset now takes a planar face as its base ({"face": [x,y,z], body?, index?, path?}), captured with the same exact face attachment a sketch on that face uses. The Offset Plane dialog accepts planar faces as well as origin and construction planes. A construction plane on a face is resolved through its face anchor when the timeline is evaluated, so it follows the face when upstream features change (and after reopening), instead of keeping the plane as it was when picked. Tests: engine (top and side faces, follows a box resize, survives reopen, hostile picks are refused without adding a feature) and a UI scenario that picks a face in the dialog and builds a pad on the new plane. Co-Authored-By: Claude Opus 5.5 --- crates/doc/src/eval.rs | 9 +- crates/engine/src/cmd/features.rs | 25 +++++- .../engine/src/cmd/offset_plane_face_tests.rs | 82 +++++++++++++++++++ crates/ui-egui/src/dialogs.rs | 11 ++- .../scenarios/offset_plane_from_face.json | 14 ++++ 5 files changed, 134 insertions(+), 7 deletions(-) create mode 100644 crates/engine/src/cmd/offset_plane_face_tests.rs create mode 100644 crates/ui-egui/tests/scenarios/offset_plane_from_face.json 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]}}} +]