From d4fd760b0d8038f34e765d22b7e723555fe17450 Mon Sep 17 00:00:00 2001 From: William Dunn Date: Tue, 13 Oct 2026 13:29:26 -0400 Subject: [PATCH] A Mirror window for sketch entities, holding both picks until OK (#734) Sketch Mirror now opens a tool window with Entities and Mirror line inputs, so both picks stay visible and editable until OK, replacing the old click-then-right-click flow. --- crates/ui-egui/src/locales/de.json | 1 + crates/ui-egui/src/locales/en.json | 1 + crates/ui-egui/src/locales/ja.json | 1 + crates/ui-egui/src/locales/ko.json | 1 + crates/ui-egui/src/sketch_modify_ui.rs | 45 +++++-- crates/ui-egui/src/sketch_modify_ui_tests.rs | 121 +++++++++++++++++- crates/ui-egui/src/sketch_tools.rs | 9 +- .../tests/scenarios/sketch_mirror_window.json | 45 +++++++ 8 files changed, 205 insertions(+), 19 deletions(-) create mode 100644 crates/ui-egui/tests/scenarios/sketch_mirror_window.json diff --git a/crates/ui-egui/src/locales/de.json b/crates/ui-egui/src/locales/de.json index 38d097c4..d5fbf341 100644 --- a/crates/ui-egui/src/locales/de.json +++ b/crates/ui-egui/src/locales/de.json @@ -196,6 +196,7 @@ "ui.measure": "Messen", "ui.midpoint": "Mittelpunkt", "ui.mirror": "Spiegeln", + "ui.mirror_line": "Spiegelachse", "ui.modify": "Ändern", "ui.move": "Verschieben", "ui.multiple": "Mehrfach", diff --git a/crates/ui-egui/src/locales/en.json b/crates/ui-egui/src/locales/en.json index 34f3eb96..ed5e65c0 100644 --- a/crates/ui-egui/src/locales/en.json +++ b/crates/ui-egui/src/locales/en.json @@ -471,6 +471,7 @@ "ui.measure": "Measure", "ui.midpoint": "Midpoint", "ui.mirror": "Mirror", + "ui.mirror_line": "Mirror line", "ui.modify": "Modify", "ui.move": "Move", "ui.multiple": "Multiple", diff --git a/crates/ui-egui/src/locales/ja.json b/crates/ui-egui/src/locales/ja.json index d4348ca2..41991a4d 100644 --- a/crates/ui-egui/src/locales/ja.json +++ b/crates/ui-egui/src/locales/ja.json @@ -471,6 +471,7 @@ "ui.measure": "計測", "ui.midpoint": "中点", "ui.mirror": "ミラー", + "ui.mirror_line": "ミラー線", "ui.modify": "修正", "ui.move": "移動", "ui.multiple": "複数", diff --git a/crates/ui-egui/src/locales/ko.json b/crates/ui-egui/src/locales/ko.json index 0cdf9d5c..fd835f9b 100644 --- a/crates/ui-egui/src/locales/ko.json +++ b/crates/ui-egui/src/locales/ko.json @@ -196,6 +196,7 @@ "ui.measure": "측정", "ui.midpoint": "중간점", "ui.mirror": "대칭 복사", + "ui.mirror_line": "대칭선", "ui.modify": "수정", "ui.move": "이동", "ui.multiple": "다중", diff --git a/crates/ui-egui/src/sketch_modify_ui.rs b/crates/ui-egui/src/sketch_modify_ui.rs index 957ae895..cdb1a104 100644 --- a/crates/ui-egui/src/sketch_modify_ui.rs +++ b/crates/ui-egui/src/sketch_modify_ui.rs @@ -4,9 +4,10 @@ //! reached no tool and no dialog at all, so a press ran `sketch.scale` with no parameters and //! the engine answered "`factor` must be a positive number". Offset joins it here (upstream //! issue #162 item 1: "Offset does not create a constraint, the size of the offset seems -//! random"). `sketch.fillet`, the three `sketch.chamfer.*` commands, the sketch patterns, -//! `sketch.mirror` and `sketch.move` are the same gap and join this module later, each its own -//! change. +//! random"). Mirror joins them here, which ran as a bare click flow: entities until a +//! right-click, then one more click for the line, with nothing on screen saying what was +//! picked. `sketch.fillet`, the three `sketch.chamfer.*` commands, the sketch patterns and +//! `sketch.move` are the same gap and join this module later, each its own change. use serde_json::{Map, Value, json}; use solidcraft_engine::Sel; @@ -14,7 +15,7 @@ use solidcraft_engine::doc::expr::Kind as ValueKind; use crate::SolveApp; use crate::dialogs::{Kind, check, field, row_label}; -use crate::selection::{CURVES, POINTS, SelInput}; +use crate::selection::{AXES, CURVES, POINTS, SelInput}; /// A sketch modify tool window's values. #[derive(Clone, Debug, PartialEq)] @@ -27,6 +28,10 @@ pub enum Mod { /// chain follows. A positive distance is left of the chain, as the command defines it; /// `flip` reverses whatever sign the field holds. Offset { distance: String, chain: bool, flip: bool }, + /// `sketch.mirror`: the entities and the line they reflect in. The command has no number of + /// its own, so the window's whole job is to show what is picked and let a pick be changed + /// before anything is committed. + Mirror, } impl Mod { @@ -34,15 +39,17 @@ impl Mod { match self { Mod::Scale { .. } => "SKETCH SCALE", Mod::Offset { .. } => "OFFSET", + Mod::Mirror => "MIRROR", } } /// The value on the canvas. pub fn primary(&mut self) -> Option<(&'static str, ValueKind, &mut String)> { - Some(match self { - Mod::Scale { factor } => ("Factor", ValueKind::Unitless, factor), - Mod::Offset { distance, .. } => ("Distance", ValueKind::Length, distance), - }) + match self { + Mod::Scale { factor } => Some(("Factor", ValueKind::Unitless, factor)), + Mod::Offset { distance, .. } => Some(("Distance", ValueKind::Length, distance)), + Mod::Mirror => None, + } } } @@ -50,6 +57,8 @@ fn inputs(k: &Mod) -> Vec { match k { Mod::Scale { .. } => vec![SelInput::new("Entities", CURVES | POINTS, true), SelInput::new("Base point", POINTS, false)], Mod::Offset { .. } => vec![SelInput::new("Curves", CURVES, true)], + // AXES is the mask that takes a straight sketch curve (`selection::SelInput::accepts`). + Mod::Mirror => vec![SelInput::new("Entities", CURVES | POINTS, true), SelInput::new("Mirror line", AXES, false)], } } @@ -58,6 +67,7 @@ pub fn start(_app: &SolveApp, id: &str) -> Option<(Kind, Vec)> { let k = match id { "sketch.scale" => Mod::Scale { factor: "2".into() }, "sketch.offset" => Mod::Offset { distance: "2 mm".into(), chain: true, flip: false }, + "sketch.mirror" => Mod::Mirror, _ => return None, }; let i = inputs(&k); @@ -84,6 +94,8 @@ pub fn rows(ui: &mut egui::Ui, k: &mut Mod) -> bool { check(ui, chain); ui.end_row(); } + // Mirror has no value of its own: its inputs are the whole window. + Mod::Mirror => {} } enter } @@ -167,6 +179,23 @@ pub fn commands(app: &SolveApp, k: &Mod, inputs: &[SelInput], extra: &Map { + let entities = entity_ids(inputs, 0); + if entities.is_empty() { + return Err("Select the sketch curves or points to mirror first".into()); + } + // The AXES mask also takes an origin axis, which is not a curve of this sketch and + // cannot be a mirror line. + let line = match items(inputs, 1).first() { + Some(Sel::SketchCurve { id }) => id.clone(), + Some(_) => return Err("The mirror line must be a straight curve of this sketch, not an origin axis".into()), + None => return Err("Select the mirror line".into()), + }; + if entities.contains(&line) { + return Err("The mirror line cannot also be mirrored".into()); + } + ("sketch.mirror", json!({"entities": entities, "line": line})) + } }; if let Value::Object(m) = &mut params { for (key, value) in extra { diff --git a/crates/ui-egui/src/sketch_modify_ui_tests.rs b/crates/ui-egui/src/sketch_modify_ui_tests.rs index f8671600..2e5e592a 100644 --- a/crates/ui-egui/src/sketch_modify_ui_tests.rs +++ b/crates/ui-egui/src/sketch_modify_ui_tests.rs @@ -1,12 +1,14 @@ -//! The Sketch Scale tool window: the toolbar button used to reach no tool and no dialog, so a -//! press ran `sketch.scale` with no parameters and the engine refused it. +//! The sketch modify tool windows. Sketch Scale's toolbar button used to reach no tool and no +//! dialog, so a press ran `sketch.scale` with no parameters and the engine refused it. Mirror +//! ran as a click flow that collected entities until a right-click, took the next click as the +//! mirror line and committed at once, with nothing on screen saying what was picked. use serde_json::json; use solidcraft_engine::Sel; use crate::dialogs::Kind; use crate::scenario::Harness; -use crate::selection::SelInput; +use crate::selection::{AXES, CURVES, POINTS, SelInput}; use crate::sketch_modify_ui::{Mod, commands}; /// A sketch on XY holding one 20 by 10 rectangle at the origin. @@ -124,3 +126,116 @@ fn the_window_scales_the_picked_geometry_and_leaves_the_rest_alone() { assert!(xs.iter().any(|x| (x - 60.0).abs() < 1e-6), "{xs:?}"); assert_eq!(sk.curves.len(), 5); } + +/// A sketch on XY holding a vertical line on the Y axis and a circle to its right. +fn line_and_circle() -> Harness { + let mut h = Harness::new(); + h.app.run("sketch.create", json!({"plane": "XY"})).unwrap(); + h.app.run("sketch.line", json!({"points": [[0, -20], [0, 20]]})).unwrap(); + h.app.run("sketch.circle.center", json!({"center": [10, 0], "radius": 3})).unwrap(); + h.settle(); + h +} + +fn mirror_inputs() -> Vec { + vec![SelInput::new("Entities", CURVES | POINTS, true), SelInput::new("Mirror line", AXES, false)] +} + +#[test] +fn the_toolbar_button_opens_a_window_with_both_picks_visible() { + let mut h = line_and_circle(); + let before = curve_ids(&h).len(); + h.step(&json!({"start": "sketch.mirror"})).unwrap(); + let d = h.app.dialog.as_ref().expect("sketch.mirror opens a tool window"); + assert!(matches!(d.kind, Kind::SketchModify(Mod::Mirror)), "{:?}", d.kind); + assert_eq!(d.inputs.iter().map(|i| i.label).collect::>(), vec!["Entities", "Mirror line"]); + assert!(h.app.tool.is_none(), "the dialog replaces the old click tool"); + // Opening the window commits nothing. + assert_eq!(curve_ids(&h).len(), before); +} + +#[test] +fn the_two_picks_become_the_command() { + let h = line_and_circle(); + let ids = curve_ids(&h); + let (line, circle) = (ids[0].clone(), ids[1].clone()); + let mut inputs = mirror_inputs(); + inputs[0].items = vec![Sel::SketchCurve { id: circle.clone() }]; + inputs[1].items = vec![Sel::SketchCurve { id: line.clone() }]; + let out = commands(&h.app, &Mod::Mirror, &inputs, &serde_json::Map::new()).unwrap(); + assert_eq!(out.len(), 1); + assert_eq!(out[0].0, "sketch.mirror"); + assert_eq!(out[0].1, json!({"entities": [circle], "line": line})); +} + +#[test] +fn a_half_filled_window_is_an_error_instead_of_a_command() { + let h = line_and_circle(); + let ids = curve_ids(&h); + let mut only_entities = mirror_inputs(); + only_entities[0].items = vec![Sel::SketchCurve { id: ids[1].clone() }]; + assert!(commands(&h.app, &Mod::Mirror, &only_entities, &serde_json::Map::new()).is_err()); + let mut only_line = mirror_inputs(); + only_line[1].items = vec![Sel::SketchCurve { id: ids[0].clone() }]; + assert!(commands(&h.app, &Mod::Mirror, &only_line, &serde_json::Map::new()).is_err()); + // The AXES mask also takes an origin axis, which is no curve of this sketch. + let mut origin_axis = mirror_inputs(); + origin_axis[0].items = vec![Sel::SketchCurve { id: ids[1].clone() }]; + origin_axis[1].items = vec![Sel::Axis { name: "Y".into() }]; + assert!(commands(&h.app, &Mod::Mirror, &origin_axis, &serde_json::Map::new()).is_err()); + // The line itself must not be in the entities: the old click flow could not express that + // either, and the engine silently dropped it. + let mut line_in_both = mirror_inputs(); + line_in_both[0].items = vec![Sel::SketchCurve { id: ids[0].clone() }]; + line_in_both[1].items = vec![Sel::SketchCurve { id: ids[0].clone() }]; + assert!(commands(&h.app, &Mod::Mirror, &line_in_both, &serde_json::Map::new()).is_err()); +} + +#[test] +fn the_window_reflects_the_picked_circle_across_the_line() { + let mut h = line_and_circle(); + let sid = h.app.session.active_sketch.unwrap(); + let ids = curve_ids(&h); + h.step(&json!({"start": "sketch.mirror"})).unwrap(); + { + let d = h.app.dialog.as_mut().unwrap(); + d.inputs[0].items = vec![Sel::SketchCurve { id: ids[1].clone() }]; + d.inputs[1].items = vec![Sel::SketchCurve { id: ids[0].clone() }]; + } + // A frame with the picks in place: the dialog reads them when it next draws. + h.frames(2); + h.step(&json!({"key": "Enter"})).unwrap(); + h.settle(); + assert!(h.app.dialog.is_none(), "Enter applies and closes the window"); + let st = h.app.session.model.state(); + let sk = &st.sketch(sid).unwrap().sketch; + assert_eq!(sk.curves.len(), 3, "{:?}", h.app.status); + let made = sk.curves.last().unwrap(); + // `CurveKind` is not Copy: match the field behind its reference. + let solidcraft_engine::sketch::CurveKind::Circle { c, r } = &made.kind else { panic!("{:?}", made.kind) }; + let at = sk.point(*c).unwrap(); + assert!((at.x + 10.0).abs() < 1e-6 && at.y.abs() < 1e-6, "{at:?}"); + assert!((*r - 3.0).abs() < 1e-6, "{r}"); +} + +#[test] +fn escape_leaves_the_sketch_exactly_as_it_was() { + let mut h = line_and_circle(); + let sid = h.app.session.active_sketch.unwrap(); + let undo = h.app.session.undo.len(); + let before = curve_ids(&h); + let constraints = h.app.session.model.state().sketch(sid).unwrap().sketch.constraints.len(); + h.step(&json!({"start": "sketch.mirror"})).unwrap(); + { + let d = h.app.dialog.as_mut().unwrap(); + d.inputs[0].items = vec![Sel::SketchCurve { id: before[1].clone() }]; + d.inputs[1].items = vec![Sel::SketchCurve { id: before[0].clone() }]; + } + h.frames(2); + h.step(&json!({"key": "Escape"})).unwrap(); + h.settle(); + assert!(h.app.dialog.is_none()); + assert_eq!(curve_ids(&h), before); + assert_eq!(h.app.session.undo.len(), undo); + assert_eq!(h.app.session.model.state().sketch(sid).unwrap().sketch.constraints.len(), constraints); +} diff --git a/crates/ui-egui/src/sketch_tools.rs b/crates/ui-egui/src/sketch_tools.rs index fc13937c..382944c8 100644 --- a/crates/ui-egui/src/sketch_tools.rs +++ b/crates/ui-egui/src/sketch_tools.rs @@ -362,7 +362,7 @@ fn mode(id: &str) -> Option { "sketch.circle.three_tangent" => Mode::Entities(3), "sketch.constrainer" | "sketch.constraint.polygon" => Mode::Entities(0), "sketch.centerline" => Mode::Entities(1), - "sketch.mirror" | "sketch.pattern.circular" | "sketch.pattern.rectangular" => Mode::EntitiesThen, + "sketch.pattern.circular" | "sketch.pattern.rectangular" => Mode::EntitiesThen, "sketch.project_to_surface" | "sketch.intersection_curve" | "sketch.spun_profile" => Mode::Steps, _ => return None, }) @@ -424,7 +424,6 @@ pub fn hint(id: &str) -> Option { "sketch.constrainer" => "Constrain: click one or two entities; right-click to apply", "sketch.constraint.polygon" => "Polygon: click the lines of a closed chain; right-click to apply", "sketch.centerline" => "Centerline: click lines", - "sketch.mirror" => "Mirror: click entities, right-click, then click the mirror line", "sketch.pattern.circular" => "Circular pattern: click entities, right-click, then click the centre", "sketch.pattern.rectangular" => "Rectangular pattern: click entities, right-click, then click two points (direction and spacing)", "sketch.project_to_surface" => "Project to surface: click curves of other sketches, right-click, then click the face", @@ -742,12 +741,6 @@ pub fn on_click(app: &mut SolveApp, tool: &mut Tool, proj: &Proj, pos: Pos2) { if tool.picks.last().is_some_and(|x| x == "|") { let ents: Vec = tool.picks.iter().filter(|x| *x != "|").cloned().collect(); match cmd { - "sketch.mirror" => { - if let Some((line, false)) = sketch_entity(app, &hits) { - let _ = app.run(cmd, json!({"entities": ents, "line": line})); - tool.picks.clear(); - } - } "sketch.pattern.circular" => { if let Some(p) = at { let _ = app.run(cmd, json!({"entities": ents, "center": arg(&p), "count": 6})); diff --git a/crates/ui-egui/tests/scenarios/sketch_mirror_window.json b/crates/ui-egui/tests/scenarios/sketch_mirror_window.json new file mode 100644 index 00000000..6b4ff956 --- /dev/null +++ b/crates/ui-egui/tests/scenarios/sketch_mirror_window.json @@ -0,0 +1,45 @@ +[ + {"note": "sketch.mirror ran as a bare click flow: entities until a right-click, then one more click taken as the mirror line and committed at once. Nothing on screen said what was picked. It now opens a tool window carrying Entities and a Mirror line."}, + {"start": "sketch.create"}, + {"click": {"plane": "XY"}}, + {"expect": {"sketching": true}}, + {"start": "sketch.line"}, + {"click": {"sketch": [0, -20]}}, + {"click": {"sketch": [0, 20]}}, + {"key": "Escape"}, + {"start": "sketch.circle.center"}, + {"click": {"sketch": [10, 0]}}, + {"click": {"sketch": [13, 0]}}, + {"key": "Escape"}, + {"expect": {"sketch_curves": {"own": 2}, "errors": 0}}, + + {"note": "The press opens a window and commits nothing. Each pick is visible in its own input and nothing is created yet."}, + {"start": "sketch.mirror"}, + {"expect": {"dialog": "SketchModify(Mirror", "sketch_curves": {"own": 2}, "errors": 0}}, + {"click": {"sketch": [13, 0]}}, + {"click": {"handle": "dialog:Mirror line"}}, + {"click": {"sketch": [0, 10]}}, + {"expect": {"dialog": "SketchModify(Mirror", "sketch_curves": {"own": 2}, "errors": 0}}, + + {"note": "Escape leaves the sketch exactly as it was: the old click flow had already committed by this point."}, + {"key": "Escape"}, + {"expect": {"dialog": null, "errors": 0, "sketch_curves": {"own": 2}}}, + + {"note": "OK reflects the circle in the line: a 3 mm circle at x = 10 becomes a 3 mm circle at x = -10."}, + {"start": "sketch.mirror"}, + {"click": {"sketch": [13, 0]}}, + {"click": {"handle": "dialog:Mirror line"}}, + {"click": {"sketch": [0, 10]}}, + {"click": {"handle": "dialog:OK"}}, + {"expect": {"dialog": null, "errors": 0, "sketch_curves": {"own": 3}}}, + {"expect": {"query": {"command": "sketch.inspect", "path": ["curves", 2, "radius"], "value": 3}}}, + {"expect": {"query": {"command": "sketch.inspect", "path": ["curves", 2, "center_at", 0], "value": -10}}}, + {"expect": {"query": {"command": "sketch.inspect", "path": ["curves", 2, "center_at", 1], "value": 0}}}, + {"shot": "sketch_mirror_window"}, + + {"note": "Starting the command again and pressing Escape leaves the document and the undo history untouched."}, + {"start": "sketch.mirror"}, + {"expect": {"dialog": "SketchModify(Mirror"}}, + {"key": "Escape"}, + {"expect": {"dialog": null, "errors": 0}} +]