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

@@ -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",

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",

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": "複数",

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": "다중",

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<SelInput> {
     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<SelInput>)> {
     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<String
             let d = signed_length(app, distance, *flip)?;
             ("sketch.offset", json!({"curves": curves, "distance": d, "chain": chain}))
         }
+        Mod::Mirror => {
+            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 {

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<SelInput> {
+    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<_>>(), 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);
+}

crates/ui-egui/src/sketch_tools.rs

@@ -362,7 +362,7 @@ fn mode(id: &str) -> Option<Mode> {
         "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<String> {
             "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<String> = 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}));

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}}
+]