From b99d4ddde9fe0353f960240bca4a2e68bac557ca Mon Sep 17 00:00:00 2001 From: semmelrinde <124386289+semmelrinde@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:44:48 +0000 Subject: [PATCH] Sketch Coincident: a selected line midpoint is a point it takes (#10311) (#10315) * Sketch Coincident: a selected line midpoint is a point it takes (#10311) A selected midpoint goes to the command as {midpoint: line}, as Horizontal/Vertical's alignment takes it, but Coincident wanted a point id and refused it with "b must be a point or curve". The command now puts a point at the line's middle, tied there by a Midpoint constraint (or uses the one already tied there), and makes that point coincident or puts it on the other curve, in one undo step. The toolbar button and the context menu send a selected midpoint that way. Co-Authored-By: Claude Opus 5.5 * Sketch Coincident: test a line midpoint in the command, the toolbar and the context menu (#10311) Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/sketch.rs | 33 +++++++++-- .../src/cmd/sketch_constraints_tests.rs | 55 +++++++++++++++++++ crates/ui-egui/src/context_menu.rs | 5 +- crates/ui-egui/src/preselect_tests.rs | 31 +++++++++++ crates/ui-egui/src/selection.rs | 13 +++++ crates/ui-egui/src/tools.rs | 9 +-- 6 files changed, 131 insertions(+), 15 deletions(-) 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 }],