From 584bbe923ddb86128bb5baf71af2d5da88a14e14 Mon Sep 17 00:00:00 2001 From: Karl Von Richter Date: Fri, 16 Oct 2026 11:27:24 +1000 Subject: [PATCH] sketch(fix): preserve rectangle constraints through fillets (#1543) Thanks @Karl-VonRichter for the fix. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/sketch_corner_tests.rs | 61 ++++++++++++++++++++ crates/engine/src/cmd/sketch_modify.rs | 45 ++++++++++++++- crates/ui-egui/src/sketch_corner_ui_tests.rs | 28 +++++++++ crates/ui-egui/src/sketch_tools.rs | 19 +++--- 4 files changed, 143 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/sketch_corner_tests.rs b/crates/engine/src/cmd/sketch_corner_tests.rs index aca73784..a6e6214e 100644 --- a/crates/engine/src/cmd/sketch_corner_tests.rs +++ b/crates/engine/src/cmd/sketch_corner_tests.rs @@ -102,6 +102,67 @@ fn driven_by(s: &Session, name: &str) -> Vec { sk(s).constraints.iter().filter(|c| c.param.as_deref() == Some(name)).map(|c| c.id.clone()).collect() } +#[test] +fn fully_defined_rectangles_keep_their_size_when_all_corners_are_filleted() { + for center in [false, true] { + for square in [false, true] { + let mut s = new_sketch(); + let height = if square { 40.0 } else { 20.0 }; + let r = if center { + run(&mut s, "sketch.rectangle.center", json!({"center": [0, 0], "corner": [20, height / 2.0]})) + } else { + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [40, height]})) + }; + let anchor = if center { "p5" } else { "p1" }; + run(&mut s, "sketch.constraint.coincident", json!({"a": anchor, "b": "origin"})); + let width = run(&mut s, "sketch.dimension", json!({"entities": ["l1"], "value": 40})); + if square { + run(&mut s, "sketch.constraint.equal", json!({"a": "l1", "b": "l2"})); + } else { + run(&mut s, "sketch.dimension", json!({"entities": ["l2"], "value": height})); + } + let info = run(&mut s, "sketch.inspect", json!({})); + assert_eq!(info["dof"], 0, "{info}"); + let points: Vec = ["bottom_left", "bottom_right", "top_right", "top_left"].iter().map(|name| r["corners"][name].clone()).collect(); + for (i, point) in points.iter().enumerate() { + let f = run(&mut s, "sketch.fillet", json!({"point": point, "radius": 3})); + assert_eq!(f["sketch"]["solved"], true, "center={center}, square={square}: {f}"); + assert_eq!(f["sketch"]["dof"], 0, "center={center}, square={square}: {f}"); + let expected = 40.0 * height - (i + 1) as f64 * (9.0 - std::f64::consts::PI * 9.0 / 4.0); + assert!((area(&s) - expected).abs() < 1e-5, "corner={i}, center={center}, square={square}: {} != {expected}", area(&s)); + } + let expected = 40.0 * height - 4.0 * (9.0 - std::f64::consts::PI * 9.0 / 4.0); + assert!((area(&s) - expected).abs() < 1e-5, "center={center}, square={square}: {} != {expected}", area(&s)); + run(&mut s, "parameters.change", json!({"name": width["param"], "expression": "50 mm"})); + assert!(solved(&s)); + let expected = 50.0 * if square { 50.0 } else { height } - 4.0 * (9.0 - std::f64::consts::PI * 9.0 / 4.0); + assert!((area(&s) - expected).abs() < 1e-5, "center={center}, square={square}: {} != {expected}", area(&s)); + let id = s.active_sketch.unwrap(); + let saved = solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap(); + assert_eq!(saved.sketch(id).unwrap(), s.doc.sketch(id).unwrap()); + run(&mut s, "edit.undo", json!({})); + run(&mut s, "edit.redo", json!({})); + assert!((area(&s) - expected).abs() < 1e-5); + } + } +} + +#[test] +fn filleting_a_side_keeps_its_original_midpoint() { + let mut s = new_sketch(); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [-20, 0], "p1": [20, 30]})); + run(&mut s, "sketch.constraint.midpoint", json!({"point": "origin", "line": "l1"})); + run(&mut s, "sketch.dimension", json!({"entities": ["l1"], "value": 40})); + run(&mut s, "sketch.dimension", json!({"entities": ["l2"], "value": 30})); + for point in ["p1", "p2"] { + let f = run(&mut s, "sketch.fillet", json!({"point": point, "radius": 3})); + assert_eq!(f["sketch"]["solved"], true, "{f}"); + assert_eq!(f["sketch"]["dof"], 0, "{f}"); + let (lo, hi) = sk(&s).bounds().unwrap(); + assert!((lo.x + 20.0).abs() < 1e-6 && (hi.x - 20.0).abs() < 1e-6, "{lo:?} {hi:?}"); + } +} + /// The spout of a basin: a vertical riser and a horizontal top, the top's length dimensioned /// to the corner and the corner's height dimensioned from the origin. Filleting the corner used /// to drop the height and leave the length on the shortened straight part, so the top came out diff --git a/crates/engine/src/cmd/sketch_modify.rs b/crates/engine/src/cmd/sketch_modify.rs index 0f321bbf..65e6325d 100644 --- a/crates/engine/src/cmd/sketch_modify.rs +++ b/crates/engine/src/cmd/sketch_modify.rs @@ -569,6 +569,44 @@ fn reattach(sk: &mut Sketch, l: usize, corner: usize, np: usize) { } } +/// Equal lengths and midpoints describe the original side, not its shortened tangent +/// segment. Keep that side as construction geometry sharing the original corner points. +/// Later corner cuts retain those points through `keep_corner`, so this also works when +/// the other end is rounded in a separate command. +fn keep_side_relations(sk: &mut Sketch, line: usize) -> Result> { + let affected = |k: &ConstraintKind| { + matches!(*k, ConstraintKind::Equal { a, b } if a == line || b == line) || matches!(*k, ConstraintKind::Midpoint { l, .. } if l == line) + }; + if !sk.constraints.iter().any(|k| affected(&k.kind)) { + return Ok(Vec::new()); + } + let Some(CurveKind::Line { a, b }) = sk.curves.get(line).map(|c| c.kind.clone()) else { return Ok(Vec::new()) }; + let support = sk.add_line_pts(a, b, None)?; + if let Some(c) = sk.curves.get_mut(support) { + c.construction = true; + } + let mut moved = Vec::new(); + for k in &mut sk.constraints { + if !affected(&k.kind) { + continue; + } + match &mut k.kind { + ConstraintKind::Equal { a, b } => { + if *a == line { + *a = support; + } + if *b == line { + *b = support; + } + } + ConstraintKind::Midpoint { l, .. } => *l = support, + _ => {} + } + moved.push(k.id.clone()); + } + Ok(moved) +} + /// What became of a rounded or cut corner's point: kept as a virtual sharp corner (its id), /// and the constraints and dimensions that now measure to it. #[derive(Default)] @@ -642,9 +680,14 @@ fn cut_corner( let v = (pb - pc).normalized().ok_or_else(|| bad(cmd, "degenerate line"))?; let ta = sk.add_point(pc + u * da, None)?; let tb = sk.add_point(pc + v * db, None)?; + let mut moved = keep_side_relations(sk, la)?; + moved.extend(keep_side_relations(sk, lb)?); reattach(sk, la, c, ta); reattach(sk, lb, c, tb); - let kept = keep_corner(sk, c, [(la, ta), (lb, tb)])?; + let mut kept = keep_corner(sk, c, [(la, ta), (lb, tb)])?; + if let Some(k) = &mut kept { + k.moved.extend(moved); + } let removed = kept.is_none(); if removed { sk.remove_points(&[c]); diff --git a/crates/ui-egui/src/sketch_corner_ui_tests.rs b/crates/ui-egui/src/sketch_corner_ui_tests.rs index 73107da7..c07e49d2 100644 --- a/crates/ui-egui/src/sketch_corner_ui_tests.rs +++ b/crates/ui-egui/src/sketch_corner_ui_tests.rs @@ -378,6 +378,34 @@ fn a_fillet_rounds_every_picked_corner_to_one_radius() { assert!((ss.profiles[0].area - (1200.0 - 3.0 * 25.0 * (1.0 - std::f64::consts::PI / 4.0))).abs() < 1e-6, "{}", ss.profiles[0].area); } +#[test] +fn fully_constrained_center_rectangle_corners_can_be_picked_and_filleted() { + let mut h = crate::scenario::Harness::new(); + h.app.run("sketch.create", json!({"plane":"XY"})).unwrap(); + h.app.run("sketch.rectangle.center", json!({"center":[0,0],"corner":[20,15]})).unwrap(); + h.app.run("sketch.constraint.coincident", json!({"a":"p5","b":"origin"})).unwrap(); + h.app.run("sketch.dimension", json!({"entities":["l1"],"value":40})).unwrap(); + h.app.run("sketch.dimension", json!({"entities":["l2"],"value":30})).unwrap(); + h.frames(2); + h.step(&json!({"start":"sketch.fillet"})).unwrap(); + h.frames(2); + for corner in [[-20, -15, 0], [20, -15, 0], [20, 15, 0], [-20, 15, 0]] { + h.step(&json!({"click":{"world":corner}})).unwrap(); + h.frames(2); + } + assert_eq!(h.app.dialog.as_ref().map(|d| d.inputs[0].items.len()), Some(4)); + set_size(&mut h.app, "3"); + apply(&mut h); + assert!(h.app.dialog.is_none(), "{:?}", h.app.status); + let info = h.app.run("sketch.inspect", json!({})).unwrap(); + assert_eq!(info["dof"], 0, "{info}"); + assert_eq!(info["status"], "solved", "{info}"); + let st = h.app.session.world_state(); + let ss = st.sketch(h.app.session.active_sketch.unwrap()).unwrap(); + assert_eq!(ss.sketch.curves.iter().filter(|c| matches!(c.kind, CurveKind::Arc { .. })).count(), 4); + assert!((ss.profiles[0].area - (1200.0 - 36.0 * (1.0 - std::f64::consts::PI / 4.0))).abs() < 1e-6); +} + #[test] fn ordinary_drawing_keeps_origin_snap_and_real_origin_corners_work() { let mut h = crate::scenario::Harness::new(); diff --git a/crates/ui-egui/src/sketch_tools.rs b/crates/ui-egui/src/sketch_tools.rs index 4ee4ea9b..cc93ae56 100644 --- a/crates/ui-egui/src/sketch_tools.rs +++ b/crates/ui-egui/src/sketch_tools.rs @@ -460,17 +460,18 @@ fn sketch_entity(app: &SolveApp, hits: &[Hit]) -> Option<(String, bool)> { } /// The two editable curves that meet at sketch point `pi`, in curve order, which is the order -/// the corner commands resolve them in from a `point` parameter. None unless exactly two meet -/// there and each is one the command can work on: a line always, an arc only for a fillet. +/// the corner commands resolve them in from a `point` parameter. Construction curves are +/// ignored when more than two meet. Each side must be a line, or an arc for a fillet. pub(crate) fn corner_curves(sk: &solidcraft_engine::sketch::Sketch, pi: usize, arcs_ok: bool) -> Option<(usize, usize)> { use solidcraft_engine::sketch::CurveKind; - let curves: Vec<_> = sk - .curves - .iter() - .enumerate() - .filter(|(ci, c)| !sk.is_linked_curve(*ci) && c.kind.ends().is_some_and(|(a, b)| a == pi || b == pi)) - .take(3) - .collect(); + let candidates = + sk.curves.iter().enumerate().filter(|(ci, c)| !sk.is_linked_curve(*ci) && c.kind.ends().is_some_and(|(a, b)| a == pi || b == pi)); + let mut curves: Vec<_> = candidates.clone().take(3).collect(); + // Center rectangles have construction diagonals at their corners. Match the engine's + // corner selection: when more than two curves meet, use the profile sides. + if curves.len() > 2 { + curves = candidates.filter(|(_, c)| !c.construction).take(3).collect(); + } if curves.iter().any(|(_, c)| !matches!(c.kind, CurveKind::Line { .. }) && !(arcs_ok && matches!(c.kind, CurveKind::Arc { .. }))) { return None; }