From 26bf1af93b9215cd47ea8be2426d571e5f3077a4 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Fri, 16 Oct 2026 03:00:24 -0400 Subject: [PATCH] Sketch on an origin plane faces the side you clicked (#1587) Fixes #1586. --- crates/doc/src/assembly.rs | 2 +- crates/doc/src/clipboard.rs | 6 +- crates/doc/src/construct.rs | 2 +- crates/doc/src/document.rs | 9 ++ crates/doc/src/eval.rs | 3 + crates/doc/src/face_anchor.rs | 8 +- .../format/pending/flipped-sketch-plane.rs | 5 + crates/doc/src/sheet.rs | 8 +- crates/doc/src/walk.rs | 4 +- crates/engine/src/cmd/clipboard.rs | 2 +- crates/engine/src/cmd/sketch.rs | 34 ++++- crates/engine/src/cmd/sketch_face.rs | 4 +- crates/engine/src/cmd/sketch_flip_tests.rs | 136 ++++++++++++++++++ crates/engine/src/occurrences.rs | 2 +- crates/geom/src/plane.rs | 6 + crates/ui-egui/src/dialogs.rs | 48 ++++++- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/sketch_side_ui_tests.rs | 87 +++++++++++ .../scenarios/s07_components_joint_drive.json | 2 +- .../s20_section_with_joint_drive.json | 2 +- .../scenarios/s26_drive_joint_typed.json | 2 +- .../s32_cut_in_active_component.json | 2 +- .../tests/scenarios/s37_browser_chevrons.json | 2 +- 23 files changed, 351 insertions(+), 27 deletions(-) create mode 100644 crates/doc/src/format/pending/flipped-sketch-plane.rs create mode 100644 crates/engine/src/cmd/sketch_flip_tests.rs create mode 100644 crates/ui-egui/src/sketch_side_ui_tests.rs diff --git a/crates/doc/src/assembly.rs b/crates/doc/src/assembly.rs index 66870784..a52580e3 100644 --- a/crates/doc/src/assembly.rs +++ b/crates/doc/src/assembly.rs @@ -145,7 +145,7 @@ fn map_plane_ref(m: &Mat, p: &mut PlaneRef) { *plane = map_plane(m, plane); *at = apply_point(m, *at); } - PlaneRef::Offset { base, .. } => map_plane_ref(m, base), + PlaneRef::Offset { base, .. } | PlaneRef::Flipped { base, .. } => map_plane_ref(m, base), PlaneRef::AtAngle { base, axis_origin, axis_dir, .. } => { map_plane_ref(m, base); *axis_origin = apply_point(m, *axis_origin); diff --git a/crates/doc/src/clipboard.rs b/crates/doc/src/clipboard.rs index b8679b44..5581892c 100644 --- a/crates/doc/src/clipboard.rs +++ b/crates/doc/src/clipboard.rs @@ -113,7 +113,9 @@ fn remap_plane( return Err(crate::DocError::Invalid("clone plane reference depth exceeded".into())); } match p { - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => remap_plane(base, ids, names, bodies, depth + 1)?, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => { + remap_plane(base, ids, names, bodies, depth + 1)? + } PlaneRef::Construction { name } => mapped_name(&mut name.0, names), PlaneRef::Face { name, anchor, .. } => { if let Some(name) = name { @@ -283,7 +285,7 @@ fn checked_plane(p: &PlaneRef, depth: usize) -> crate::Result<()> { return Err(crate::DocError::Invalid("copy reference nesting exceeds 64".into())); } match p { - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => checked_plane(base, depth + 1)?, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => checked_plane(base, depth + 1)?, PlaneRef::Midplane { a, b } | PlaneRef::TwoEdges { a, b } => { checked_geo(a, depth + 1)?; checked_geo(b, depth + 1)?; diff --git a/crates/doc/src/construct.rs b/crates/doc/src/construct.rs index da42a77e..5940eab9 100644 --- a/crates/doc/src/construct.rs +++ b/crates/doc/src/construct.rs @@ -633,7 +633,7 @@ impl PlaneRef { PlaneRef::ThreePoints { a, b, c } => vec![a.as_mut(), b.as_mut(), c.as_mut()], PlaneRef::Tangent { face, .. } => vec![face.as_mut()], PlaneRef::AlongPath { path, .. } => vec![path.as_mut()], - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => base.refs_mut(), + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => base.refs_mut(), _ => Vec::new(), } } diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index 81e79c06..c7c142f9 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -58,6 +58,14 @@ pub enum PlaneRef { AtAngle { base: Box, axis_origin: Vec3, axis_dir: Vec3, angle: Expr }, /// A construction plane feature (by name). Construction { name: FeatureName }, + /// `base` seen from its other side: the normal reversed by a half turn about the base's + /// own x axis (y reversed), or about its y axis (x reversed) when `about_y`. A sketch + /// started from behind a plane faces the viewer this way. + Flipped { + base: Box, + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + about_y: bool, + }, /// Halfway between two planes (or planar faces); bisecting them when they meet at an angle. Midplane { a: Box, b: Box }, /// Through two straight edges (lines) that lie in one plane. @@ -2884,6 +2892,7 @@ impl Document { let o = *axis_origin + rot(b.origin - *axis_origin); Plane::new(o, rot(b.x), rot(b.y)).ok_or_else(|| DocError::Invalid("degenerate plane".into())) } + PlaneRef::Flipped { base, about_y } => Ok(self.resolve_plane_in(vals, base, depth + 1, st)?.flipped(*about_y)), PlaneRef::Construction { name } => { // Evaluated already: as it came out. if let Some(c) = st.and_then(|st| st.construct.iter().find(|c| &c.name == name)) diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index 8b42a922..10fecf97 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -3998,6 +3998,9 @@ pub(crate) fn sheet_face_plane_in_frame(doc: &Document, st: &ModelState, comp: u axis_dir: *axis_dir, angle: angle.clone(), }, + PlaneRef::Flipped { base, about_y } => { + PlaneRef::Flipped { base: Box::new(sheet_face_plane_in_frame(doc, st, comp, base)?), about_y: *about_y } + } other => other.clone(), }) } diff --git a/crates/doc/src/face_anchor.rs b/crates/doc/src/face_anchor.rs index a6e1df44..8d32d926 100644 --- a/crates/doc/src/face_anchor.rs +++ b/crates/doc/src/face_anchor.rs @@ -82,7 +82,7 @@ impl FaceAnchor { pub fn plane_face_anchor(plane: &PlaneRef) -> Option<&FaceAnchor> { match plane { PlaneRef::Face { anchor, .. } => anchor.as_deref(), - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => plane_face_anchor(base), + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => plane_face_anchor(base), _ => None, } } @@ -90,7 +90,7 @@ pub fn plane_face_anchor(plane: &PlaneRef) -> Option<&FaceAnchor> { fn has_face_reference(plane: &PlaneRef) -> bool { match plane { PlaneRef::Face { .. } => true, - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => has_face_reference(base), + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => has_face_reference(base), _ => false, } } @@ -110,7 +110,9 @@ impl Document { } match plane { PlaneRef::Face { anchor: Some(anchor), .. } => *plane = PlaneRef::Custom { plane: doc.resolve_face_anchor(anchor, author, state)? }, - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => replace(doc, base, author, state, depth + 1)?, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => { + replace(doc, base, author, state, depth + 1)? + } _ => {} } Ok(()) diff --git a/crates/doc/src/format/pending/flipped-sketch-plane.rs b/crates/doc/src/format/pending/flipped-sketch-plane.rs new file mode 100644 index 00000000..973f13f6 --- /dev/null +++ b/crates/doc/src/format/pending/flipped-sketch-plane.rs @@ -0,0 +1,5 @@ +//! A sketch can sit on a plane seen from its other side: `{"type": "flipped", "base": }` +//! (with `"about_y": true` when it turns about the plane's y axis rather than its x axis). New +//! sketches started from behind an origin or construction plane save this, so they face the +//! viewer. Older designs have no flipped planes and open unchanged; the last release refuses a +//! design that has one (it asks for an update) instead of putting the sketch on the wrong side. diff --git a/crates/doc/src/sheet.rs b/crates/doc/src/sheet.rs index 6538b3bb..b8dcc0ec 100644 --- a/crates/doc/src/sheet.rs +++ b/crates/doc/src/sheet.rs @@ -4257,7 +4257,9 @@ pub fn rename_plane_sheet_body(plane: &mut crate::PlaneRef, old: &str, new: &str *name = renamed; } } - crate::PlaneRef::Offset { base, .. } | crate::PlaneRef::AtAngle { base, .. } => rename_plane_sheet_body(base, old, new), + crate::PlaneRef::Offset { base, .. } | crate::PlaneRef::AtAngle { base, .. } | crate::PlaneRef::Flipped { base, .. } => { + rename_plane_sheet_body(base, old, new) + } _ => {} } } @@ -4267,7 +4269,9 @@ pub fn rename_plane_sheet_body(plane: &mut crate::PlaneRef, old: &str, new: &str pub fn plane_sheet_face(plane: &crate::PlaneRef) -> Option<&str> { match plane { crate::PlaneRef::Face { name: Some(name), anchor: None, .. } if split_sheet_face_name(name).is_some() => Some(name), - crate::PlaneRef::Offset { base, .. } | crate::PlaneRef::AtAngle { base, .. } => plane_sheet_face(base), + crate::PlaneRef::Offset { base, .. } | crate::PlaneRef::AtAngle { base, .. } | crate::PlaneRef::Flipped { base, .. } => { + plane_sheet_face(base) + } _ => None, } } diff --git a/crates/doc/src/walk.rs b/crates/doc/src/walk.rs index d7c54c8f..64726064 100644 --- a/crates/doc/src/walk.rs +++ b/crates/doc/src/walk.rs @@ -364,7 +364,7 @@ impl Walk for PlaneRef { let mut p = self; for _ in 0..64 { match p { - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => p = base, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => p = base, PlaneRef::Construction { name } => return f(Ref::Feature(name)), _ => return, } @@ -374,7 +374,7 @@ impl Walk for PlaneRef { let mut p = self; for _ in 0..64 { match p { - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => p = base, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => p = base, PlaneRef::Construction { name } => return f(Ref::Feature(name)), _ => return, } diff --git a/crates/engine/src/cmd/clipboard.rs b/crates/engine/src/cmd/clipboard.rs index 4b37b39a..eff00a90 100644 --- a/crates/engine/src/cmd/clipboard.rs +++ b/crates/engine/src/cmd/clipboard.rs @@ -45,7 +45,7 @@ fn explicit_planes(p: &mut PlaneRef) { *p = PlaneRef::Custom { plane }; } } - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => explicit_planes(base), + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => explicit_planes(base), _ => {} } } diff --git a/crates/engine/src/cmd/sketch.rs b/crates/engine/src/cmd/sketch.rs index 6cc69a3d..4cf5e4b0 100644 --- a/crates/engine/src/cmd/sketch.rs +++ b/crates/engine/src/cmd/sketch.rs @@ -29,7 +29,7 @@ pub static COMMANDS: &[CommandSpec] = &[ CommandSpec::new("sketch.create", "Create Sketch", create_sketch) .at("SOLID", "CREATE") .icon("sketch") - .params("plane: XY|XZ|YZ | construction plane name or id | {origin, x_dir, y_dir} | {face: [world x,y,z], body?: exact world body, index?: face index, path?: complete occurrence ids, fixed?: bool}; new face attachments require a unique certified source (a flat face of a sheet metal body attaches to its panel instead, in the flat pattern's coordinates), fixed:true explicitly freezes the selected plane; offset?: expr, project_edges?: bool, name?"), + .params("plane: XY|XZ|YZ | construction plane name or id | {origin, x_dir, y_dir} | {face: [world x,y,z], body?: exact world body, index?: face index, path?: complete occurrence ids, fixed?: bool}; new face attachments require a unique certified source (a flat face of a sheet metal body attaches to its panel instead, in the flat pattern's coordinates), fixed:true explicitly freezes the selected plane; offset?: expr, flip?: bool | \"x\" | \"y\" (not on a face: sketch the plane from its other side, so the normal and the default extrude direction are reversed; a half turn about the plane's own x or y axis, true picks the one keeping the sketch's y axis upward; offset still moves along the unflipped normal), project_edges?: bool, name?"), CommandSpec::new("sketch.edit", "Edit Sketch", edit_sketch).noundo().icon("sketch").params("sketch: id or name"), CommandSpec::new("sketch.finish", "Finish Sketch", finish_sketch).at("SKETCH", "FINISH SKETCH").icon("finish").enabled(in_sketch).noundo(), CommandSpec::new("sketch.line", "Line", draw_line) @@ -543,7 +543,7 @@ pub(crate) fn author_path_for_rebind(s: &Session, feature: &solidcraft_doc::Feat let mut plane = plane; loop { match plane { - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => plane = base, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => plane = base, PlaneRef::Face { anchor: Some(anchor), .. } => { saved = Some(anchor.author_path.clone()); break; @@ -635,7 +635,31 @@ fn create_sketch(s: &mut Session, p: &Value) -> Result { return Err(bad(cmd, format!("`{key}` belongs inside `plane`: {example}"))); } } - create_sketch_with_plane(s, p, plane_ref(s, p, cmd)?, None) + let mut plane = plane_ref(s, p, cmd)?; + if let Some(about_y) = flip_param(s, p, &plane, cmd)? { + plane = PlaneRef::Flipped { base: Box::new(plane), about_y }; + } + create_sketch_with_plane(s, p, plane, None) +} + +/// `flip` of `sketch.create`: `None` keeps the plane's own side, otherwise the axis to turn +/// about (`true`: about y when the plane's y axis points upward, so it stays up, else about x). +fn flip_param(s: &Session, p: &Value, plane: &PlaneRef, cmd: &str) -> Result> { + let about_y = match p.get("flip") { + None | Some(Value::Null) | Some(Value::Bool(false)) => return Ok(None), + Some(Value::String(a)) if a.eq_ignore_ascii_case("x") => false, + Some(Value::String(a)) if a.eq_ignore_ascii_case("y") => true, + Some(Value::Bool(true)) => { + let (vals, _) = s.doc.param_values(); + let resolved = s.doc.resolve_plane_in(&vals, plane, 0, Some(&s.model.state()))?; + resolved.y.z > 1e-9 + } + Some(_) => return Err(bad(cmd, "`flip` must be true, false, \"x\" or \"y\"")), + }; + if face::is_face(p) { + return Err(bad(cmd, "`flip` is for origin, construction and explicit planes; a face already faces outward")); + } + Ok(Some(about_y)) } /// Used only by face Extrude/Revolve/PressPull, preserving pre-certificate linked profiles. @@ -2493,6 +2517,10 @@ fn solve_cmd(s: &mut Session, p: &Value) -> Result { Ok(json!({"status": ss.report.status, "dof": ss.report.dof, "failing": ss.report.failing, "profiles": ss.profiles.len()})) } +#[cfg(test)] +#[path = "sketch_flip_tests.rs"] +mod flip_tests; + #[cfg(test)] #[path = "sketch_rect_tests.rs"] mod rect_tests; diff --git a/crates/engine/src/cmd/sketch_face.rs b/crates/engine/src/cmd/sketch_face.rs index 985cdf0c..35d30953 100644 --- a/crates/engine/src/cmd/sketch_face.rs +++ b/crates/engine/src/cmd/sketch_face.rs @@ -206,7 +206,7 @@ fn capture_impl(s: &Session, value: &Value, cmd: &str, internal: bool) -> Result pub(in crate::cmd) fn anchor(mut plane: &PlaneRef) -> Option<&solidcraft_doc::FaceAnchor> { loop { match plane { - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => plane = base, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => plane = base, PlaneRef::Face { anchor, .. } => return anchor.as_deref(), _ => return None, } @@ -216,7 +216,7 @@ pub(in crate::cmd) fn anchor(mut plane: &PlaneRef) -> Option<&solidcraft_doc::Fa pub(in crate::cmd) fn anchor_mut(mut plane: &mut PlaneRef) -> Option<&mut solidcraft_doc::FaceAnchor> { loop { match plane { - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => plane = base, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => plane = base, PlaneRef::Face { anchor, .. } => return anchor.as_deref_mut(), _ => return None, } diff --git a/crates/engine/src/cmd/sketch_flip_tests.rs b/crates/engine/src/cmd/sketch_flip_tests.rs new file mode 100644 index 00000000..2385cb74 --- /dev/null +++ b/crates/engine/src/cmd/sketch_flip_tests.rs @@ -0,0 +1,136 @@ +//! `sketch.create` `flip`: a sketch on a plane seen from its other side (#1586). Without it a +//! sketch keeps the plane's own side, and older designs open, rebuild and save as before. + +use serde_json::{Value, json}; +use solidcraft_doc::{FeatureKind, PlaneRef}; +use solidcraft_geom::{Plane, Vec3}; + +use crate::Session; + +fn run(s: &mut Session, id: &str, p: Value) -> Value { + match s.execute(id, &p) { + Ok(v) => v, + Err(e) => panic!("{id} {p}: {e}"), + } +} + +fn sketch_plane(s: &Session, id: u64) -> Plane { + s.model.state().sketch(id).map(|ss| ss.plane).expect("an evaluated sketch") +} + +fn measured(s: &mut Session) -> Value { + run(s, "inspect.measure", json!({}))["bodies"].clone() +} + +fn close(a: Vec3, b: Vec3) -> bool { + a.dist(b) < 1e-9 +} + +#[test] +fn without_flip_a_sketch_keeps_the_planes_own_side_and_saves_as_before() { + for name in ["XY", "XZ", "YZ"] { + let mut s = Session::default(); + let id = run(&mut s, "sketch.create", json!({"plane": name}))["sketch"].as_u64().unwrap(); + let saved = serde_json::to_value(&s.doc.feature(id).unwrap().kind).unwrap(); + assert_eq!(saved["plane"], json!({"type": "origin", "name": name}), "{name}"); + assert_eq!(sketch_plane(&s, id), Plane::named(name).unwrap(), "{name}"); + } +} + +#[test] +fn flip_turns_the_plane_about_the_axis_asked_for() { + // (plane, flip, expected x, expected y) + let cases = [ + ("XZ", json!(true), Vec3::X, Vec3::Z), + ("XZ", json!("y"), -Vec3::X, -Vec3::Z), + ("YZ", json!(true), -Vec3::Y, Vec3::Z), + ("YZ", json!("x"), Vec3::Y, -Vec3::Z), + ("XY", json!(true), Vec3::X, -Vec3::Y), + ("XY", json!("Y"), -Vec3::X, Vec3::Y), + ]; + for (name, flip, x, y) in cases { + let mut s = Session::default(); + let id = run(&mut s, "sketch.create", json!({"plane": name, "flip": flip}))["sketch"].as_u64().unwrap(); + let p = sketch_plane(&s, id); + assert!(close(p.x, x) && close(p.y, y), "{name} flip {flip}: {p:?}"); + assert!(close(p.normal(), -Plane::named(name).unwrap().normal()), "{name} flip {flip}"); + assert!(matches!(&s.doc.feature(id).unwrap().kind, FeatureKind::Sketch { plane: PlaneRef::Flipped { .. }, .. })); + } + let mut s = Session::default(); + let id = run(&mut s, "sketch.create", json!({"plane": "XZ", "flip": false}))["sketch"].as_u64().unwrap(); + assert!(matches!(&s.doc.feature(id).unwrap().kind, FeatureKind::Sketch { plane: PlaneRef::Origin { .. }, .. })); + assert!(s.execute("sketch.create", &json!({"plane": "XZ", "flip": "z"})).is_err()); +} + +#[test] +fn a_flipped_sketch_extrudes_the_other_way_and_offsets_along_the_planes_own_normal() { + let mut s = Session::default(); + let id = run(&mut s, "sketch.create", json!({"plane": "XZ", "flip": true, "offset": "5 mm"}))["sketch"].as_u64().unwrap(); + assert!(close(sketch_plane(&s, id).origin, Vec3::new(0.0, 5.0, 0.0))); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [10, 4]})); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "solid.extrude", json!({"sketch": id, "distance": 3, "operation": "new_body", "body_name": "B"})); + let bounds = s.model.state().body("B").map(|b| b.mesh().bounds()).unwrap(); + // Sketch y is world +Z; extruded along -Y, from y = 5 to y = 2. + assert!((bounds.min.y - 2.0).abs() < 1e-9 && (bounds.max.y - 5.0).abs() < 1e-9, "{bounds:?}"); + assert!((bounds.min.z).abs() < 1e-9 && (bounds.max.z - 4.0).abs() < 1e-9, "{bounds:?}"); + // Saved and opened again, it is the same. + let path = std::env::temp_dir().join(format!("solidcraft-flip-{}.solidcraft", std::process::id())); + run(&mut s, "file.save", json!({"path": path})); + let mut reopened = Session::default(); + run(&mut reopened, "doc.open", json!({"path": path})); + assert_eq!(*reopened.doc, *s.doc); + assert_eq!(measured(&mut reopened), measured(&mut s)); + std::fs::remove_file(path).unwrap(); +} + +#[test] +fn a_construction_plane_flips_too_and_follows_its_parameter() { + let mut s = Session::default(); + run(&mut s, "parameters.add", json!({"name": "h", "expression": "7 mm"})); + run(&mut s, "construct.plane.offset", json!({"base": "XY", "offset": "h", "name": "P"})); + let id = run(&mut s, "sketch.create", json!({"plane": "P", "flip": true}))["sketch"].as_u64().unwrap(); + let p = sketch_plane(&s, id); + assert!(close(p.origin, Vec3::new(0.0, 0.0, 7.0)) && close(p.normal(), -Vec3::Z), "{p:?}"); + run(&mut s, "sketch.finish", json!({})); + run(&mut s, "parameters.change", json!({"name": "h", "expression": "9 mm"})); + let p = sketch_plane(&s, id); + assert!(close(p.origin, Vec3::new(0.0, 0.0, 9.0)) && close(p.normal(), -Vec3::Z), "{p:?}"); +} + +#[test] +fn flip_is_refused_on_a_face() { + let mut s = Session::default(); + run(&mut s, "solid.box", json!({"length": 10, "width": 10, "height": 10})); + assert!(s.execute("sketch.create", &json!({"plane": {"face": [5, 5, 10]}, "flip": true})).is_err()); +} + +/// Designs from before `flip` open with every sketch where it was, never flipped, rebuild the +/// same bodies and save and reopen unchanged. +#[test] +fn older_designs_open_rebuild_and_save_unchanged() { + let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../apps/solidcraft-cli/tests/corpus"); + let mut files: Vec<_> = + std::fs::read_dir(&dir).unwrap().flatten().map(|e| e.path()).filter(|p| p.extension().is_some_and(|x| x == "solidcraft")).collect(); + files.sort(); + assert!(files.len() >= 7, "corpus designs in {}", dir.display()); + for old in files { + let mut s = Session::default(); + run(&mut s, "doc.open", json!({"path": old})); + assert!(!serde_json::to_string(&*s.doc).unwrap().contains("\"flipped\""), "{}", old.display()); + // The oldest one's XZ sketch: sketch y along +Z, as it was drawn. + if old.ends_with("m0-revolve-primitives.solidcraft") { + let id = s.doc.features.iter().find(|f| matches!(f.kind, FeatureKind::Sketch { .. })).map(|f| f.id).unwrap(); + let p = sketch_plane(&s, id); + assert!(close(p.origin, Vec3::ZERO) && close(p.x, Vec3::X) && close(p.y, Vec3::Z), "{p:?}"); + } + let bodies = measured(&mut s); + let path = std::env::temp_dir().join(format!("solidcraft-old-{}.solidcraft", std::process::id())); + run(&mut s, "file.save", json!({"path": path})); + let mut reopened = Session::default(); + run(&mut reopened, "doc.open", json!({"path": path})); + assert_eq!(*reopened.doc, *s.doc, "{}", old.display()); + assert_eq!(measured(&mut reopened), bodies, "{}", old.display()); + std::fs::remove_file(path).unwrap(); + } +} diff --git a/crates/engine/src/occurrences.rs b/crates/engine/src/occurrences.rs index daf3ea8d..0fd615cf 100644 --- a/crates/engine/src/occurrences.rs +++ b/crates/engine/src/occurrences.rs @@ -240,7 +240,7 @@ impl Session { let mut plane = plane; loop { match plane { - PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } => plane = base, + PlaneRef::Offset { base, .. } | PlaneRef::AtAngle { base, .. } | PlaneRef::Flipped { base, .. } => plane = base, // A face attachment that names no exact source: which body it sits on is unknown, // so none is taken from the view. PlaneRef::Face { anchor: None, .. } => return none, diff --git a/crates/geom/src/plane.rs b/crates/geom/src/plane.rs index ee51a010..1d7d882e 100644 --- a/crates/geom/src/plane.rs +++ b/crates/geom/src/plane.rs @@ -39,6 +39,12 @@ impl Plane { Plane::new(origin, x, n.cross(x)) } + /// The same plane seen from its other side: a half turn about the sketch x axis (y + /// reversed), or about the y axis (x reversed) when `about_y`. Still right-handed. + pub fn flipped(&self, about_y: bool) -> Plane { + if about_y { Plane { x: -self.x, ..*self } } else { Plane { y: -self.y, ..*self } } + } + /// Named origin plane: `XY`, `XZ`, `YZ` (case-insensitive). pub fn named(name: &str) -> Option { match name.to_ascii_uppercase().as_str() { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 2f6911c6..2d4ab062 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -3327,13 +3327,18 @@ pub(crate) fn change_param(app: &mut SolveApp, name: &str, expression: &str) -> /// Start a sketch on a picked plane or planar face and turn the view to face it. pub fn start_sketch(app: &mut SolveApp, sel: &Sel) -> Result<(), String> { - let plane = match sel { - Sel::Plane { name } => json!(name), - Sel::Face { .. } => crate::sketch_attachment_ui::face_plane(app, sel).ok_or("Selected face placement no longer exists")?, + let mut params = match sel { + Sel::Plane { name } => json!({ "plane": name }), + Sel::Face { .. } => json!({ "plane": crate::sketch_attachment_ui::face_plane(app, sel).ok_or("Selected face placement no longer exists")? }), _ => return Err("Select an origin plane, construction plane or planar body face".into()), }; + if let Sel::Plane { name } = sel + && let Some(flip) = flip_toward_viewer(app, name) + { + params["flip"] = json!(flip); + } let before = app.cam; - app.run("sketch.create", json!({ "plane": plane })).map_err(|error| crate::friendly_error("sketch.create", &error))?; + app.run("sketch.create", params).map_err(|error| crate::friendly_error("sketch.create", &error))?; app.pre_sketch_cam = Some(before); look_at_sketch(app); // On a face: look at the picked point, not the plane's origin (a corner of the face), @@ -3347,6 +3352,40 @@ pub fn start_sketch(app: &mut SolveApp, sel: &Sel) -> Result<(), String> { Ok(()) } +/// A new sketch on origin or construction plane `name` faces the viewer: when the camera is +/// behind the plane's normal, the in-plane axis to turn the plane about (`"x"` or `"y"`), chosen +/// so the sketch's y axis stays as upright on screen as it can (x then runs to the right). +fn flip_toward_viewer(app: &SolveApp, name: &str) -> Option<&'static str> { + let plane = plane_by_name(app, name)?; + let n = plane.normal(); + // Where the eye is: in perspective its position, in orthographic only its direction counts. + let side = if app.cam.fov > 0.0 { (app.cam.eye() - plane.origin).dot(n) } else { app.cam.basis().2.dot(n) }; + if side >= -1e-9 { + return None; + } + let up = app.cam.up.dir(); + Some(if plane.y.dot(up) > 1e-9 { "y" } else { "x" }) +} + +/// The world plane of an origin plane or a construction plane by name. +fn plane_by_name(app: &SolveApp, name: &str) -> Option { + if let Some(p) = solidcraft_engine::geom::Plane::named(name) { + // In an active component its origin planes are the component's. + let m = app.session.active_component_frame(); + let (o, x, y) = ( + solidcraft_engine::doc::apply_point(&m, p.origin), + solidcraft_engine::doc::apply_vector(&m, p.x), + solidcraft_engine::doc::apply_vector(&m, p.y), + ); + return solidcraft_engine::geom::Plane::new(o, x, y); + } + let st = app.session.world_state(); + st.construct.iter().find(|c| c.name == name).and_then(|c| match &c.geom { + solidcraft_engine::doc::construct::ConstructGeom::Plane(pl) => Some(*pl), + _ => None, + }) +} + /// Animate the camera to look straight at the active sketch plane. pub fn look_at_sketch(app: &mut SolveApp) { let st = app.session.world_state(); @@ -4352,6 +4391,7 @@ pub(crate) fn plane_sel(s: &Session, pl: &PlaneRef) -> Option { PlaneRef::Origin { name } => Some(Sel::Plane { name: name.clone() }), PlaneRef::Construction { name } => Some(Sel::Plane { name: name.to_string() }), PlaneRef::Custom { plane } | PlaneRef::Face { plane, .. } => face_sel(s, plane.origin), + PlaneRef::Flipped { base, .. } => plane_sel(s, base), _ => None, } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index f09b2e48..8e5a2a43 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -149,6 +149,8 @@ pub mod sketch_modify_ui; mod sketch_modify_ui_tests; pub mod sketch_palette; mod sketch_pattern_tree; +#[cfg(test)] +mod sketch_side_ui_tests; pub(crate) mod sketch_text; mod sketch_text_handles; mod sketch_text_move; diff --git a/crates/ui-egui/src/sketch_side_ui_tests.rs b/crates/ui-egui/src/sketch_side_ui_tests.rs new file mode 100644 index 00000000..bb62fa61 --- /dev/null +++ b/crates/ui-egui/src/sketch_side_ui_tests.rs @@ -0,0 +1,87 @@ +//! Sketch on an origin plane from either side: the new sketch faces the viewer and the view +//! turns to it from where it was, never round to the far side (#1586). + +use crate::scenario::Harness; +use serde_json::json; +use solidcraft_engine::doc::{FeatureKind, PlaneRef}; +use solidcraft_engine::geom::{Plane, Vec3}; + +fn sketch_from(yaw: f64, pitch: f64, name: &str) -> (Harness, Vec3, Plane) { + let mut h = Harness::new(); + h.app.preferences.look_at_anim = false; + h.app.cam.yaw = yaw; + h.app.cam.pitch = pitch; + let eye = h.app.cam.eye(); + h.step(&json!({"start":"sketch.create"})).unwrap(); + h.step(&json!({"click":{"plane":name}})).unwrap(); + h.settle(); + let plane = h.app.session.world_state().sketches.first().map(|s| s.plane).expect("a sketch on the plane"); + (h, eye, plane) +} + +#[test] +fn a_new_sketch_on_an_origin_plane_faces_the_side_it_was_started_from() { + // (yaw, pitch, plane, the plane's own normal); each plane from both of its sides. + let cases = [ + (0.4, 0.4, "XZ", Vec3::Y), + (std::f64::consts::PI - 0.4, 0.4, "XZ", Vec3::Y), + (-1.2, 0.4, "YZ", Vec3::X), + (1.2, 0.4, "YZ", Vec3::X), + (-0.6, 0.5, "XY", Vec3::Z), + (-0.6, -0.5, "XY", Vec3::Z), + ]; + for (yaw, pitch, name, own) in cases { + let (h, eye, plane) = sketch_from(yaw, pitch, name); + let n = plane.normal(); + let what = format!("{name} from yaw {yaw} pitch {pitch}"); + assert!(n.dot(own).abs() > 0.999, "{what}: sketched on {n:?}"); + assert!(eye.dot(n) > 0.0, "{what}: the sketch normal {n:?} points away from the eye {eye:?}"); + // The view looks straight at the sketch from the side it was looking from. + let after = h.app.cam.eye(); + assert!(after.dot(own * eye.dot(own).signum()) > 0.0, "{what}: the view turned to the far side {after:?}"); + let (right, up, back) = h.app.cam.basis(); + assert!(back.dot(n) > 0.999, "{what}: not looking straight at the sketch ({back:?})"); + // Only a sketch from behind the plane saves the flip, turned so its x runs right and + // its y up on screen (a plane's own side keeps its own axes, as before). + let sketch = h.app.session.active_sketch.unwrap(); + let FeatureKind::Sketch { plane: saved, .. } = &h.app.session.doc.feature(sketch).unwrap().kind else { panic!() }; + let behind = eye.dot(own) < 0.0; + if behind { + assert!(right.dot(plane.x) > 0.999 && up.dot(plane.y) > 0.999, "{what}: sketch axes {:?} {:?} on screen", plane.x, plane.y); + } + assert_eq!(matches!(saved, PlaneRef::Flipped { .. }), behind, "{what}: saved {saved:?}"); + } +} + +#[test] +fn a_flipped_sketch_extrudes_toward_the_viewer_and_reopens_the_same() { + let (mut h, _, plane) = sketch_from(0.4, 0.4, "XZ"); + assert!(plane.normal().dot(Vec3::Y) < -0.999); + h.app.run("sketch.rectangle.two_point", json!({"p0":[0,0],"p1":[10,5]})).unwrap(); + let sketch = h.app.session.active_sketch.unwrap(); + h.app.finish_sketch(); + h.app.run("solid.extrude", json!({"sketch":sketch,"distance":4,"operation":"new_body","body_name":"Block"})).unwrap(); + let bounds = |h: &Harness| h.app.session.world_state().body("Block").map(|b| b.mesh().bounds()).expect("an extruded body"); + let before = bounds(&h); + assert!(before.min.y < -3.9 && before.max.y < 1e-6, "extruded away from the viewer: {before:?}"); + let path = h.dir.join("flipped-sketch.solidcraft"); + h.app.run("file.save", json!({"path":path})).unwrap(); + h.app.run("doc.open", json!({"path":path})).unwrap(); + assert_eq!(bounds(&h), before); +} + +#[test] +fn a_construction_plane_seen_from_below_gets_a_sketch_facing_down() { + let mut h = Harness::new(); + h.app.preferences.look_at_anim = false; + h.app.run("construct.plane.offset", json!({"base":"XY","offset":"5 mm","name":"P"})).unwrap(); + h.app.cam.yaw = -0.6; + h.app.cam.pitch = -0.5; + h.app.session.selection = vec![solidcraft_engine::Sel::Plane { name: "P".into() }]; + h.step(&json!({"key":"L"})).unwrap(); + h.settle(); + let sketch = h.app.session.active_sketch.expect("a sketch on P"); + let plane = h.app.session.world_state().sketch(sketch).map(|s| s.plane).unwrap(); + assert!(plane.normal().z < -0.999 && (plane.origin.z - 5.0).abs() < 1e-9, "{plane:?}"); + assert!(h.app.cam.eye().z < 5.0, "the view stays below the plane"); +} diff --git a/crates/ui-egui/tests/scenarios/s07_components_joint_drive.json b/crates/ui-egui/tests/scenarios/s07_components_joint_drive.json index 8d4e318c..34cc697c 100644 --- a/crates/ui-egui/tests/scenarios/s07_components_joint_drive.json +++ b/crates/ui-egui/tests/scenarios/s07_components_joint_drive.json @@ -113,7 +113,7 @@ "click": { "sketch": [ 50, - -10 + 10 ] } }, diff --git a/crates/ui-egui/tests/scenarios/s20_section_with_joint_drive.json b/crates/ui-egui/tests/scenarios/s20_section_with_joint_drive.json index aaefdc34..845f4ea5 100644 --- a/crates/ui-egui/tests/scenarios/s20_section_with_joint_drive.json +++ b/crates/ui-egui/tests/scenarios/s20_section_with_joint_drive.json @@ -113,7 +113,7 @@ "click": { "sketch": [ 50, - -10 + 10 ] } }, diff --git a/crates/ui-egui/tests/scenarios/s26_drive_joint_typed.json b/crates/ui-egui/tests/scenarios/s26_drive_joint_typed.json index 1785b799..18381075 100644 --- a/crates/ui-egui/tests/scenarios/s26_drive_joint_typed.json +++ b/crates/ui-egui/tests/scenarios/s26_drive_joint_typed.json @@ -113,7 +113,7 @@ "click": { "sketch": [ 50, - -10 + 10 ] } }, diff --git a/crates/ui-egui/tests/scenarios/s32_cut_in_active_component.json b/crates/ui-egui/tests/scenarios/s32_cut_in_active_component.json index a02cef5c..5141ca64 100644 --- a/crates/ui-egui/tests/scenarios/s32_cut_in_active_component.json +++ b/crates/ui-egui/tests/scenarios/s32_cut_in_active_component.json @@ -102,7 +102,7 @@ "click": { "sketch": [ 52, - -10 + 10 ] } }, diff --git a/crates/ui-egui/tests/scenarios/s37_browser_chevrons.json b/crates/ui-egui/tests/scenarios/s37_browser_chevrons.json index 124b89f4..cff63767 100644 --- a/crates/ui-egui/tests/scenarios/s37_browser_chevrons.json +++ b/crates/ui-egui/tests/scenarios/s37_browser_chevrons.json @@ -116,7 +116,7 @@ "click": { "sketch": [ 50, - -10 + 10 ] } },