diff --git a/crates/engine/src/cmd/sketch.rs b/crates/engine/src/cmd/sketch.rs index 9d2c48f228..0f06b8aba2 100644 --- a/crates/engine/src/cmd/sketch.rs +++ b/crates/engine/src/cmd/sketch.rs @@ -139,7 +139,7 @@ pub static COMMANDS: &[CommandSpec] = &[ .at("SKETCH", "CONSTRAINTS") .icon("c_coincident") .enabled(in_sketch) - .params(concat!("a, b: points, a point and a curve or sketch axis (x / y: the sketch's own axes, unless a curve has that id; an axis becomes a linked construction line on it, reported in created), or two distinct lines (joins their closest endpoints), or two circles/arcs (the second lies on the first: concentric and equal, `constraints` lists both)", model_operand_doc!())), + .params(concat!("a, b: points (or a line's middle as {midpoint: line}, which becomes a point tied there), a point and a curve or sketch axis (x / y: the sketch's own axes, unless a curve has that id; an axis becomes a linked construction line on it, reported in created), or two distinct lines (joins their closest endpoints), or two circles/arcs (the second lies on the first: concentric and equal, `constraints` lists both)", model_operand_doc!())), CommandSpec::new("sketch.constraint.tangent", "Tangent", c_tangent) .at("SKETCH", "CONSTRAINTS") .icon("c_tangent") @@ -2250,17 +2250,38 @@ fn c_coincident(s: &mut Session, p: &Value) -> Result { if let Some((a, b)) = line_points { return Ok(ConstraintKind::Coincident { p: a, q: b }); } + // A line's middle, `{midpoint: line}` as a selected midpoint is sent (#10311): the point + // tied there by a Midpoint constraint stands for it, made as Horizontal/Vertical makes it. + let middle = |sk: &mut Sketch, key: &str| -> Result> { + let Some(line) = p.get(key).and_then(|v| v.get("midpoint")) else { return Ok(None) }; + let l = curve_ref(sk, Some(line), cmd, "midpoint")?; + AlignmentTarget::Midpoint(l).materialize(sk, cmd).map(Some) + }; + let (a_mid, b_mid) = (middle(sk, "a")?, middle(sk, "b")?); let aref = p.get("a").and_then(Value::as_str).unwrap_or_default(); - let bref = p.get("b").and_then(Value::as_str).ok_or_else(|| bad(cmd, "`b` must be a point or curve"))?; + let bref = p.get("b").and_then(Value::as_str); + let b_point = match (b_mid, bref) { + (Some(b), _) => Some(b), + (None, Some(r)) => sk.resolve_point(r), + (None, None) => return Err(bad(cmd, "`b` must be a point or curve")), + }; + let bref = bref.unwrap_or_default(); // A curve (or sketch axis) picked first and a point second: the point goes onto it. - if sk.resolve_point(aref).is_none() - && let Some(q) = sk.resolve_point(bref) + if a_mid.is_none() + && sk.resolve_point(aref).is_none() + && let Some(q) = b_point && let Some(c) = curve_or_axis(sk, aref)? { return Ok(ConstraintKind::PointOnCurve { p: q, c }); } - let a = point_ref(sk, p.get("a"), cmd, "a")?; - if let Some(b) = sk.resolve_point(bref) { + let a = match a_mid { + Some(a) => a, + None => point_ref(sk, p.get("a"), cmd, "a")?, + }; + if let Some(b) = b_point { + if a == b && (a_mid.is_some() || b_mid.is_some()) { + return Err(bad(cmd, "that point is already the line's midpoint")); + } return Ok(ConstraintKind::Coincident { p: a, q: b }); } let c = curve_or_axis(sk, bref)?.ok_or_else(|| bad(cmd, format!("unknown entity `{bref}`")))?; diff --git a/crates/engine/src/cmd/sketch_constraints_tests.rs b/crates/engine/src/cmd/sketch_constraints_tests.rs index 3c136dd178..c9f8eda30a 100644 --- a/crates/engine/src/cmd/sketch_constraints_tests.rs +++ b/crates/engine/src/cmd/sketch_constraints_tests.rs @@ -803,6 +803,61 @@ fn midpoint_alignment_rejects_invalid_or_conflicting_references_atomically() { } } +/// #10311: Coincident takes a line's middle as `{midpoint: line}`, first or second. With another +/// line the middle lies on it, with a point it lands on the point; a point tied to the middle by +/// a Midpoint constraint stands for it, in one undo step. +#[test] +fn coincident_takes_a_line_midpoint_either_way_round() { + for other in ["l2", "target"] { + for reverse in [false, true] { + let mut s = new_sketch(); + run(&mut s, "sketch.line", json!({"points":[[10,10],[30,20]],"infer":false,"connect":false})); + run(&mut s, "sketch.line", json!({"points":[[0,40],[60,40]],"infer":false,"connect":false})); + run(&mut s, "sketch.point", json!({"point":[24,50],"id":"target"})); + run(&mut s, "sketch.constraint.fix", json!({"entities":["l2","target"]})); + let before = s.doc.to_json(); + let undo = s.undo.len(); + let middle = json!({"midpoint":"l1"}); + let (a, b) = if reverse { (json!(other), middle) } else { (middle, json!(other)) }; + let result = run(&mut s, "sketch.constraint.coincident", json!({"a":a,"b":b})); + assert_eq!(result["created"]["points"].as_array().unwrap().len(), 1, "{other} {reverse}: {result}"); + assert_eq!(s.undo.len(), undo + 1); + let sk = sketch(&s); + let mid = line_midpoint(&sk, "l1"); + if other == "l2" { + assert!((mid.y - 40.0).abs() < 1e-6, "{other} {reverse}: middle at {mid:?}"); + } else { + assert!(mid.dist(Vec2::new(24.0, 50.0)) < 1e-6, "{other} {reverse}: middle at {mid:?}"); + } + assert!(sk.constraints.iter().any(|c| matches!(c.kind, ConstraintKind::Midpoint { .. }))); + run(&mut s, "edit.undo", json!({})); + assert_eq!(s.doc.to_json(), before); + } + } +} + +/// A middle Coincident can't take (not a line, an unknown curve, the same middle twice, or the +/// point already tied there) is refused and leaves the sketch as it was. +#[test] +fn coincident_refuses_a_midpoint_it_cannot_take_atomically() { + let mut s = new_sketch(); + run(&mut s, "sketch.line", json!({"points":[[10,10],[30,20]],"infer":false,"connect":false})); + run(&mut s, "sketch.circle.center", json!({"center":[40,50],"radius":3})); + let tied = run(&mut s, "sketch.point", json!({"point":"mid:l1"}))["point"].as_str().unwrap().to_owned(); + for (a, b) in [ + (json!({"midpoint":"c1"}), json!("origin")), + (json!({"midpoint":"missing"}), json!("origin")), + (json!({"midpoint":"l1"}), json!({"midpoint":"l1"})), + (json!({"midpoint":"l1"}), json!(tied)), + ] { + let before = s.doc.to_json(); + let undo = s.undo.len(); + assert!(s.execute("sketch.constraint.coincident", &json!({"a":a,"b":b})).is_err(), "{a} {b}"); + assert_eq!(s.doc.to_json(), before, "{a} {b}"); + assert_eq!(s.undo.len(), undo); + } +} + fn line_midpoint(sk: &solidcraft_sketch::Sketch, id: &str) -> Vec2 { // Read the curve's current endpoints: after a corner cut an original endpoint // id can still name the retained construction side's corner. diff --git a/crates/ui-egui/src/context_menu.rs b/crates/ui-egui/src/context_menu.rs index 872fbaa89e..e67b00fda7 100644 --- a/crates/ui-egui/src/context_menu.rs +++ b/crates/ui-egui/src/context_menu.rs @@ -1119,7 +1119,10 @@ fn sketch_entity_items(app: &SolveApp) -> Vec { let st = app.session.world_state(); let Some(ss) = st.sketch(sid) else { return Vec::new() }; if app.session.selection.iter().any(|sel| matches!(sel, Sel::SketchMidpoint { .. })) { - return alignment_items(&app.session.selection); + // A midpoint with a point or curve can also be made coincident with it (#10311). + let coincident = crate::selection::coincident_operands(&app.session.selection) + .map(|operands| cmd(app, "sketch.constraint.coincident", "Coincident").with(operands)); + return coincident.into_iter().chain(alignment_items(&app.session.selection)).collect(); } let mut lines = Vec::new(); let mut rounds = Vec::new(); diff --git a/crates/ui-egui/src/preselect_tests.rs b/crates/ui-egui/src/preselect_tests.rs index df85f2323e..f06f2bf28f 100644 --- a/crates/ui-egui/src/preselect_tests.rs +++ b/crates/ui-egui/src/preselect_tests.rs @@ -2990,3 +2990,34 @@ fn extrude_takes_every_profile_of_a_sketch_clicked_in_the_browser() { "both rectangles of the clicked sketch, not the last sketch's circle" ); } + +/// #10311: a selected line midpoint and another line, either way round, go to Coincident from the +/// toolbar and from the context menu: the line's middle lands on the other line. +#[test] +fn coincident_takes_a_selected_line_midpoint() { + use crate::context_menu::{self, Target}; + let middle_y = |h: &crate::scenario::Harness| { + let sk = h.app.session.doc.sketch(h.app.session.active_sketch.unwrap()).unwrap(); + let line = sk.curve_index("l1").unwrap(); + crate::selection::line_midpoint(sk, line).unwrap().y + }; + for menu in [false, true] { + for reverse in [false, true] { + let mut h = crate::scenario::Harness::new(); + h.app.run("sketch.create", json!({"plane": "XY"})).unwrap(); + h.app.run("sketch.line", json!({"points": [[10, 10], [30, 20]], "infer": false, "connect": false})).unwrap(); + h.app.run("sketch.line", json!({"points": [[0, 40], [60, 40]], "infer": false, "connect": false})).unwrap(); + h.app.run("sketch.constraint.fix", json!({"entities": ["l2"]})).unwrap(); + let (middle, line) = (Sel::SketchMidpoint { id: "l1".into() }, Sel::SketchCurve { id: "l2".into(), sketch: None }); + h.app.session.selection = if reverse { vec![line, middle] } else { vec![middle, line] }; + if menu { + let items = context_menu::items(&h.app, &Target::Viewport); + let item = items.iter().find(|i| i.label == "Coincident").expect("the menu offers Coincident"); + context_menu::run_item(&mut h.app, item, egui::Pos2::ZERO); + } else { + h.app.start("sketch.constraint.coincident"); + } + assert!((middle_y(&h) - 40.0).abs() < 1e-6, "menu {menu}, reverse {reverse}: middle at y {} ({:?})", middle_y(&h), h.app.status); + } + } +} diff --git a/crates/ui-egui/src/selection.rs b/crates/ui-egui/src/selection.rs index f8ce333988..6f3cc770d5 100644 --- a/crates/ui-egui/src/selection.rs +++ b/crates/ui-egui/src/selection.rs @@ -29,6 +29,19 @@ pub(crate) fn alignment_points(selection: &[Sel]) -> Option<[serde_json::Value; (a != b).then_some([a, b]) } +/// Coincident's two operands for a two-item selection: points and curves by id, a line's +/// midpoint as `{midpoint: line}`, which the command ties a point to (#10311). +pub(crate) fn coincident_operands(selection: &[Sel]) -> Option { + let [a, b] = selection else { return None }; + let operand = |s: &Sel| match s { + Sel::SketchPoint { id } | Sel::SketchCurve { id, .. } => Some(serde_json::json!(id)), + Sel::SketchMidpoint { id } => Some(serde_json::json!({"midpoint": id})), + _ => None, + }; + let (a, b) = (operand(a)?, operand(b)?); + (a != b).then(|| serde_json::json!({"a": a, "b": b})) +} + /// The current middle of a finite straight sketch edge. pub(crate) fn line_midpoint(sk: &solidcraft_engine::sketch::Sketch, line: usize) -> Option { let curve = sk.curves.get(line)?; diff --git a/crates/ui-egui/src/tools.rs b/crates/ui-egui/src/tools.rs index 74d0378819..3758653f12 100644 --- a/crates/ui-egui/src/tools.rs +++ b/crates/ui-egui/src/tools.rs @@ -739,14 +739,7 @@ pub(crate) fn constraint_preselection( point_line_alignment(sketch?, &json!(id), line) } ("sketch.constraint.horizontal_vertical", _) => crate::selection::alignment_points(selection).map(|points| json!({"points": points})), - ("sketch.constraint.coincident", [a, b]) => { - let entity = |s: &solidcraft_engine::Sel| match s { - SketchPoint { id } | SketchCurve { id, .. } => Some(id.clone()), - _ => None, - }; - let (a, b) = (entity(a)?, entity(b)?); - (a != b).then(|| json!({"a":a,"b":b})) - } + ("sketch.constraint.coincident", [_, _]) => crate::selection::coincident_operands(selection), ( "sketch.constraint.midpoint", [SketchPoint { id: point }, SketchCurve { id: line, .. }] | [SketchCurve { id: line, .. }, SketchPoint { id: point }],