From a6dc759a02f5ffcba5d915e0372a9e241a62c39c Mon Sep 17 00:00:00 2001 From: bherbruck Date: Thu, 8 Oct 2026 02:46:35 +0000 Subject: [PATCH] Fix: rectangles over-constrained after an edit (dragged sides collapsed) Parallel and perpendicular residuals were the cross/dot product over the sum of the line lengths, so the solver could satisfy them by shrinking a line to nothing. Dragging a corner of a rotated (3-point) rectangle collapsed a side to zero; every later dimension then conflicted. The residuals are now the sine or cosine of the angle (over the product of the lengths), which shrinking can't improve. Tests drag, dimension, pin and square center, 2-point and 3-point rectangles and check their degrees of freedom; oracle unchanged at 53/66. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AS5Tn3xieewUSg3uxrMVdL --- crates/engine/src/cmd/sketch.rs | 4 + crates/engine/src/cmd/sketch_rect_tests.rs | 103 +++++++++++++++++++++ crates/sketch/src/solver.rs | 5 +- 3 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 crates/engine/src/cmd/sketch_rect_tests.rs diff --git a/crates/engine/src/cmd/sketch.rs b/crates/engine/src/cmd/sketch.rs index c62321ef..d1ac4bab 100644 --- a/crates/engine/src/cmd/sketch.rs +++ b/crates/engine/src/cmd/sketch.rs @@ -1289,3 +1289,7 @@ fn solve_cmd(s: &mut Session, p: &Value) -> Result { let ss = st.sketch(id).ok_or_else(|| EngineError::Other("the sketch has not been evaluated".into()))?; Ok(json!({"status": ss.report.status, "dof": ss.report.dof, "failing": ss.report.failing, "profiles": ss.profiles.len()})) } + +#[cfg(test)] +#[path = "sketch_rect_tests.rs"] +mod rect_tests; diff --git a/crates/engine/src/cmd/sketch_rect_tests.rs b/crates/engine/src/cmd/sketch_rect_tests.rs new file mode 100644 index 00000000..d8d85324 --- /dev/null +++ b/crates/engine/src/cmd/sketch_rect_tests.rs @@ -0,0 +1,103 @@ +//! Regression tests: freshly drawn rectangles take every ordinary edit without being reported +//! over-constrained (dragging, dimensioning, pinning the centre, making them square). + +use serde_json::{Value, json}; + +use crate::Session; + +fn run(s: &mut Session, id: &str, p: Value) -> Value { + match s.execute(id, &p) { + Ok(v) => v, + Err(e) => panic!("{id} {p}: {e}"), + } +} + +fn ids(v: &Value) -> Vec { + v.as_array().map(|a| a.iter().filter_map(|x| x.as_str().map(str::to_string)).collect()).unwrap_or_default() +} + +/// A new sketch with one rectangle of the given kind; returns its four side ids. +fn rect(kind: &str) -> (Session, Vec) { + let mut s = Session::default(); + run(&mut s, "SketchCreate", json!({"plane": "XY"})); + let r = match kind { + "center" => run(&mut s, "ShapeRectangleCenter", json!({"center": [3, 2], "corner": [13, 9]})), + "two" => run(&mut s, "ShapeRectangleTwoPoint", json!({"p0": [1, 1], "p1": [21, 15]})), + _ => run(&mut s, "ShapeRectangleThreePoint", json!({"p0": [0, 0], "p1": [20, 5], "p2": [18, 15]})), + }; + let sides = ids(&r["curves"]); + assert_eq!(sides.len(), 4, "{r}"); + (s, sides) +} + +fn healthy(s: &mut Session, what: &str) -> i64 { + let si = run(s, "sketch.inspect", json!({})); + assert_ne!(si["status"], "conflict", "{what}: {si}"); + assert!(si["status"].as_str().is_none_or(|t| !t.contains("over")), "{what}: {si}"); + si["dof"].as_i64().unwrap_or(-1) +} + +const KINDS: [&str; 3] = ["center", "two", "three"]; + +#[test] +fn fresh_rectangles_have_the_expected_freedom() { + for k in KINDS { + let (mut s, _) = rect(k); + let dof = healthy(&mut s, k); + // Centre/two-point: x, y, width, height. Three-point adds the rotation. + let want = if k == "three" { 5 } else { 4 }; + assert_eq!(dof, want, "{k}"); + } +} + +#[test] +fn rectangles_take_dimensions_without_conflict() { + for k in KINDS { + let (mut s, l) = rect(k); + run(&mut s, "SketchDimension", json!({"entities": [l[0]], "value": 30})); + healthy(&mut s, &format!("{k} width")); + run(&mut s, "SketchDimension", json!({"entities": [l[1]], "value": 12})); + let dof = healthy(&mut s, &format!("{k} width+height")); + assert_eq!(dof, if k == "three" { 3 } else { 2 }, "{k}"); + } +} + +#[test] +fn rectangles_can_be_dragged_pinned_and_made_square() { + for k in KINDS { + // Drag a corner, then the middle of an edge. + let (mut s, l) = rect(k); + run(&mut s, "sketch.move_point", json!({"point": format!("{}.start", l[0]), "to": [-4, -3]})); + healthy(&mut s, &format!("{k} drag corner")); + run(&mut s, "sketch.move_point", json!({"point": format!("{}.end", l[2]), "to": [25, 22]})); + healthy(&mut s, &format!("{k} drag other corner")); + // Pin a corner to the origin. + run(&mut s, "ConstraintCoincident", json!({"a": format!("{}.start", l[0]), "b": "origin"})); + healthy(&mut s, &format!("{k} corner on origin")); + // Square: equal sides. + run(&mut s, "ConstraintEqual", json!({"a": l[0], "b": l[1]})); + healthy(&mut s, &format!("{k} square")); + run(&mut s, "SketchDimension", json!({"entities": [l[0]], "value": 10})); + let dof = healthy(&mut s, &format!("{k} square sized")); + assert_eq!(dof, if k == "three" { 1 } else { 0 }, "{k}"); + } +} + +#[test] +fn dragging_corners_never_collapses_a_side() { + for k in KINDS { + let (mut s, l) = rect(k); + for (i, to) in [[-4.0, -3.0], [25.0, 22.0], [30.0, -5.0], [-8.0, 18.0]].iter().enumerate() { + let pt = format!("{}.start", l[i % 4]); + run(&mut s, "sketch.move_point", json!({"point": pt, "to": to})); + let sk = s.model.state().sketch(s.active_sketch.unwrap()).unwrap().sketch.clone(); + for c in &l { + let ci = sk.curve_index(c).unwrap(); + let pi = sk.curves[ci].kind.point_ids(); + let len = sk.point(pi[0]).unwrap().dist(sk.point(pi[1]).unwrap()); + assert!(len > 0.5, "{k}: side {c} collapsed to {len} after dragging {pt}"); + } + healthy(&mut s, &format!("{k} drag {i}")); + } + } +} diff --git a/crates/sketch/src/solver.rs b/crates/sketch/src/solver.rs index be3dff9f..620e0359 100644 --- a/crates/sketch/src/solver.rs +++ b/crates/sketch/src/solver.rs @@ -274,7 +274,10 @@ fn residuals(s: &State, k: &ConstraintKind, out: &mut Vec) { let (a0, a1) = s.line(a); let (b0, b1) = s.line(b); let (da, db) = (a1 - a0, b1 - b0); - let scale = (da.len() + db.len()).max(1e-12); + // The sine (parallel) or cosine (perpendicular) of the angle between the lines: + // independent of their lengths, so the solver can't satisfy it by shrinking a line + // to nothing (scaling by the summed lengths did, collapsing dragged rectangles). + let scale = (da.len() * db.len()).max(1e-12); let v = if matches!(k, Parallel { .. }) { da.cross(db) } else { da.dot(db) }; out.push(v / scale); }