From 9ba72e118b74e6ebf343b51ea8e77cc50fbf6425 Mon Sep 17 00:00:00 2001 From: bherbruck Date: Tue, 13 Oct 2026 10:12:55 -0400 Subject: [PATCH] Sketch: accept redundant but consistent constraints (#687) (#699) A clover of same-size rectangles drawn around a shared corner was refused as over-constrained. Constraints that remove no degree of freedom but still hold (zero residual) are now kept and shown dimmed as redundant. Conflicting ones are still refused. A redundant driving dimension is refused with "already determined by ...". Typed sizes become driven (reference) dimensions. Drawing no longer adds Horizontal/Vertical to a side that is already implied through two coincident corners. The solver reports redundant constraints using a Jacobian row-dependence test. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/sketch.rs | 193 +++++++++++++- .../engine/src/cmd/sketch_axis_ref_tests.rs | 3 +- crates/engine/src/cmd/sketch_constraints.rs | 29 +- .../src/cmd/sketch_constraints_tests.rs | 10 +- crates/engine/src/cmd/sketch_dimensioned.rs | 4 +- .../engine/src/cmd/sketch_redundant_tests.rs | 252 ++++++++++++++++++ crates/sketch/src/lib.rs | 4 +- crates/sketch/src/redundancy.rs | 171 ++++++++++++ crates/sketch/src/solver.rs | 120 ++++++--- crates/ui-egui/src/dim_view.rs | 2 +- crates/ui-egui/src/inference.rs | 6 +- crates/ui-egui/src/sketch_clover_ui_tests.rs | 73 +++++ crates/ui-egui/src/sketch_dims.rs | 4 + crates/ui-egui/src/sketch_tools.rs | 10 +- 14 files changed, 804 insertions(+), 77 deletions(-) create mode 100644 crates/engine/src/cmd/sketch_redundant_tests.rs create mode 100644 crates/sketch/src/redundancy.rs create mode 100644 crates/ui-egui/src/sketch_clover_ui_tests.rs diff --git a/crates/engine/src/cmd/sketch.rs b/crates/engine/src/cmd/sketch.rs index 45fc0e1b..33906216 100644 --- a/crates/engine/src/cmd/sketch.rs +++ b/crates/engine/src/cmd/sketch.rs @@ -106,7 +106,7 @@ pub static COMMANDS: &[CommandSpec] = &[ .params(concat!("p0, p1: overall ends, width", connect_doc!(), created_doc!())), CommandSpec::new("sketch.point", "Point", draw_point).at("SKETCH", "CREATE").icon("point").enabled(in_sketch).params(concat!("point: [x,y], id?", connect_doc!(), created_doc!())), CommandSpec::new("sketch.dimension", "Sketch Dimension", dimension).at("SKETCH", "CREATE").icon("dimension").key("D").enabled(in_sketch).params( - "entities: [refs], type?: auto|distance|horizontal|vertical|length|radius|diameter|angle, value?: number or expression (default: current; a bare number is in the design's units, e.g. 2 means 2 in in an inch design; give a unit such as \"50 mm\" to override); text_at?: [x, y]. Two points or a line off the axes: with auto, text_at across from them is the distance between them, above or below them horizontal, beside them vertical. Two lines: the angle between them (acute, or 180° − it when the value is nearer that) unless text_at picks a sector. Returns measured in unit (mm or deg)", + "entities: [refs], type?: auto|distance|horizontal|vertical|length|radius|diameter|angle, value?: number or expression (default: current; a bare number is in the design's units, e.g. 2 means 2 in in an inch design; give a unit such as \"50 mm\" to override); text_at?: [x, y]. Two points or a line off the axes: with auto, text_at across from them is the distance between them, above or below them horizontal, beside them vertical. Two lines: the angle between them (acute, or 180° − it when the value is nearer that) unless text_at picks a sector. A dimension the sketch already determines is refused (naming what determines it); if_redundant?: \"driven\" places it as a driven (reference) dimension instead. Returns measured in unit (mm or deg)", ), CommandSpec::new("sketch.constraint.horizontal_vertical", "Horizontal/Vertical", c_horizontal_vertical) .at("SKETCH", "CONSTRAINTS") @@ -401,6 +401,7 @@ pub(super) fn draw( let first = sk.points.len(); let (curves, mut cons) = f(sk, doc)?; cons.extend(connect_points(sk, first, p)?); + drop_implied_axes(sk, &mut cons); Ok((curves, cons)) }) } @@ -821,6 +822,74 @@ fn rect_constraints(sk: &mut Sketch, l: &[usize]) -> Result> { Ok(cons) } +/// Whether line `l` being horizontal (`horizontal`) or vertical already follows from another +/// constraint than `own`: its ends coincide with the ends of a line constrained the same way (a +/// side two rectangles share), or with two points constrained level (or plumb). +fn axis_implied(sk: &Sketch, l: usize, horizontal: bool, own: &str) -> bool { + let Some(CurveKind::Line { a, b }) = sk.curves.get(l).map(|c| &c.kind) else { return false }; + let (a, b) = (*a, *b); + // Coincident points as one (union-find over the coincident constraints). + let mut parent: Vec = (0..sk.points.len()).collect(); + let root = |parent: &[usize], mut i: usize| { + for _ in 0..parent.len() { + match parent.get(i) { + Some(&up) if up != i => i = up, + _ => break, + } + } + i + }; + for c in &sk.constraints { + if let ConstraintKind::Coincident { p, q } = c.kind { + let (rp, rq) = (root(&parent, p), root(&parent, q)); + if rp != rq + && let Some(slot) = parent.get_mut(rp) + { + *slot = rq; + } + } + } + let (ra, rb) = (root(&parent, a), root(&parent, b)); + sk.constraints.iter().filter(|c| c.id != own).any(|c| { + let ends = match (&c.kind, horizontal) { + (ConstraintKind::Horizontal { l: m }, true) | (ConstraintKind::Vertical { l: m }, false) if *m != l => { + match sk.curves.get(*m).map(|c| &c.kind) { + Some(CurveKind::Line { a, b }) => Some((*a, *b)), + _ => None, + } + } + (ConstraintKind::HorizontalPoints { p, q }, true) | (ConstraintKind::VerticalPoints { p, q }, false) => Some((*p, *q)), + _ => None, + }; + ends.is_some_and(|(p, q)| { + let (rp, rq) = (root(&parent, p), root(&parent, q)); + rp != rq && ((rp, rq) == (ra, rb) || (rp, rq) == (rb, ra)) + }) + }) +} + +/// Drawing adds horizontal/vertical only where it is not already implied: a rectangle drawn +/// onto another's side shares that side's direction through the two coincident corners, so a +/// second "vertical" there would only be redundant. Drops such constraints among `cons` (the +/// drawing's own) from the sketch and the list. +fn drop_implied_axes(sk: &mut Sketch, cons: &mut Vec) { + let mut dropped = Vec::new(); + for id in cons.iter() { + let axis = sk.constraints.iter().find(|c| &c.id == id).and_then(|c| match c.kind { + ConstraintKind::Horizontal { l } => Some((l, true)), + ConstraintKind::Vertical { l } => Some((l, false)), + _ => None, + }); + if let Some((l, horizontal)) = axis + && axis_implied(sk, l, horizontal, id) + { + sk.constraints.retain(|c| &c.id != id); + dropped.push(id.clone()); + } + } + cons.retain(|id| !dropped.contains(id)); +} + /// Which side and corner of an axis-aligned rectangle is which, from its lines and corner /// points as `polygon_lines` made them (counter-clockwise from the bottom-left corner), so a /// caller can dimension it without inspecting the sketch first. @@ -855,6 +924,7 @@ fn rect_two_point(s: &mut Session, p: &Value) -> Result { let mut cons = rect_constraints(sk, &l)?; mark_construction(sk, &l, p); cons.extend(connect_points(sk, first, p)?); + drop_implied_axes(sk, &mut cons); Ok(((ids_of(sk, &l), cons), rect_names(sk, &l, &pts))) })?; Ok(rect_result(out, names, info)) @@ -887,6 +957,7 @@ fn rect_center(s: &mut Session, p: &Value) -> Result { } mark_construction(sk, &l, p); cons.extend(connect_points(sk, first, p)?); + drop_implied_axes(sk, &mut cons); Ok(((ids_of(sk, &l), cons), rect_names(sk, &l, &pts))) })?; Ok(rect_result(out, names, info)) @@ -1174,19 +1245,24 @@ fn draw_point(s: &mut Session, p: &Value) -> Result { fn constrain(s: &mut Session, p: &Value, cmd: &str, f: impl FnOnce(&mut Sketch) -> Result) -> Result { let mut before = None; + let mut residual = None; let (id, info) = edit(s, p, cmd, true, |sk, _| { let k = f(sk)?; let rep = solve(&mut sk.clone()); before = Some(rep.dof); let order = entity_order(sk, p); let id = add_c(sk, k)?; - if let Some((first, second)) = order { + residual = residual_of(sk, &id); + // A constraint that already holds needs nothing moved (even when both entities are + // fully determined: it is then redundant, see `redundancy`). + let holds = residual.is_some_and(|r| r <= solidcraft_sketch::CONSISTENT); + if let Some((first, second)) = order.filter(|_| !holds) { apply_in_order(sk, &rep, &[first], second, cmd)?; } Ok(id) })?; - reject_redundant(before, &info, cmd)?; - Ok(json!({"constraint": id, "sketch": info})) + let by = redundancy(s, p, cmd, before, &info, residual, &id)?; + constraint_result(p, cmd, id, info, by, Value::Null) } /// A sketch entity a constraint is applied to. @@ -1330,18 +1406,79 @@ pub(super) fn dof_now(sk: &Sketch) -> usize { solve(&mut sk.clone()).dof } -/// A constraint that removes no degree of freedom over-constrains the sketch (Fusion refuses -/// it); the caller's edit is rolled back by returning the error. -pub(super) fn reject_redundant(before: Option, info: &Value, cmd: &str) -> Result<()> { +/// A constraint just added that removes no degree of freedom (`before` → the edit's `info`) is +/// redundant or a conflict. Redundant when it held where the sketch was before it was added +/// (`residual`, see [`solidcraft_sketch::constraint_residual`]): what it asks for is already +/// implied by other constraints, and the solver satisfies it with them. Returns the +/// constraints that determine it (as "id (Name)"), or `None` when it removes a degree of +/// freedom. A conflict is refused; the caller's edit is rolled back by returning the error. +pub(super) fn redundancy( + s: &Session, + p: &Value, + cmd: &str, + before: Option, + info: &Value, + residual: Option, + cid: &str, +) -> Result>> { let after = info.get("dof").and_then(Value::as_u64).map(|d| d as usize); - if let (Some(b), Some(a)) = (before, after) - && a >= b - { - return Err(bad(cmd, "that would over-constrain the sketch (it is already determined by other constraints)")); + let (Some(b), Some(a)) = (before, after) else { return Ok(None) }; + if a < b { + return Ok(None); + } + if !residual.is_some_and(|r| r <= solidcraft_sketch::CONSISTENT) { + return Err(bad(cmd, "that would over-constrain the sketch (it conflicts with what the other constraints already determine)")); + } + Ok(Some(determined_by(s, p, cmd, cid))) +} + +/// The constraints that already determine constraint `cid` of the target sketch, as +/// "id (Name)". +fn determined_by(s: &Session, p: &Value, cmd: &str, cid: &str) -> Vec { + let Some(sk) = target_sketch(s, p, cmd).ok().and_then(|id| s.doc.sketch(id).ok()) else { return Vec::new() }; + let Some(ci) = sk.constraints.iter().position(|c| c.id == cid) else { return Vec::new() }; + solidcraft_sketch::determined_by(sk, ci) + .into_iter() + .filter_map(|k| sk.constraints.get(k)) + .map(|c| format!("{} ({})", c.id, c.kind.name())) + .collect() +} + +/// "k3 (Vertical), k9 (Vertical)", or "other constraints" when they are not known. +pub(super) fn listed(by: &[String]) -> String { + if by.is_empty() { "other constraints".to_string() } else { by.join(", ") } +} + +/// A redundant constraint is kept (and shown as redundant) unless the caller asked for +/// `if_redundant: "refuse"` (drawing-time inferences, which only add what is not already there). +pub(super) fn keep_redundant(p: &Value, cmd: &str, by: &[String]) -> Result<()> { + if str_(p, "if_redundant").is_some_and(|m| m.eq_ignore_ascii_case("refuse")) { + return Err(bad(cmd, format!("that would over-constrain the sketch (it is already determined by {})", listed(by)))); } Ok(()) } +/// The result of a constraint command: the constraint, and whether it is redundant (and what +/// determines it). +pub(super) fn constraint_result(p: &Value, cmd: &str, id: String, info: Value, by: Option>, extra: Value) -> Result { + let mut out = json!({"constraint": id, "sketch": info}); + if let Some(by) = by { + keep_redundant(p, cmd, &by)?; + out["redundant"] = json!(true); + out["determined_by"] = json!(by); + } + if let (Some(o), Value::Object(e)) = (out.as_object_mut(), extra) { + o.extend(e); + } + Ok(out) +} + +/// The residual of the constraint `id` where the sketch is now (see +/// [`solidcraft_sketch::constraint_residual`]). +pub(super) fn residual_of(sk: &Sketch, id: &str) -> Option { + sk.constraints.iter().position(|c| c.id == id).and_then(|ci| solidcraft_sketch::constraint_residual(sk, ci)) +} + fn c_horizontal_vertical(s: &mut Session, p: &Value) -> Result { let cmd = "sketch.constraint.horizontal_vertical"; let mode = str_(p, "mode").map(str::to_ascii_lowercase); @@ -1610,6 +1747,8 @@ fn dimension(s: &mut Session, p: &Value) -> Result { let text_at = p.get("text_at").and_then(vec2); let driven = bool_(p, "driven").unwrap_or(false); let mut before = None; + let (mut residual, mut new_id) = (None, String::new()); + let start = s.doc.clone(); let ((param, kind_name, current), info) = edit(s, p, cmd, true, |sk, doc| { before = Some(dof_now(sk)); let ent = |r: &str| -> Result<(Option, Option)> { @@ -1766,8 +1905,13 @@ fn dimension(s: &mut Session, p: &Value) -> Result { { (k, cur) = (ka, ca); } + // How far the value is from what the geometry measures now: zero for a dimension that + // only restates what the sketch already has. + let want = vals.get(&pname).and_then(|v| v.to_kind(if is_angle { Kind::Angle } else { Kind::Length }).ok()); + residual = want.map(|w| (w - cur).abs()).filter(|r| r.is_finite()); let name = k.name(); let id = sk.add_constraint(k, Some(pname.clone()))?; + new_id = id.clone(); place_text(sk, &id, text_at); Ok((pname, name, measured(is_angle, cur))) })?; @@ -1776,8 +1920,28 @@ fn dimension(s: &mut Session, p: &Value) -> Result { if driven { return Ok(json!({"param": Value::Null, "driven": true, "type": kind_name, "measured": current.0, "unit": current.1, "sketch": info})); } - if reject_redundant(before, &info, cmd).is_err() { - return Err(bad(cmd, "that dimension would over-constrain the sketch; add it as a driven (reference) dimension with driven: true")); + // A dimension the sketch already determines cannot drive it: it is refused, or with + // `if_redundant: "driven"` placed as a driven (reference) dimension instead. + let by = redundancy(s, p, cmd, before, &info, residual, &new_id) + .map_err(|_| bad(cmd, "that dimension conflicts with the sketch: other constraints already determine it with a different value"))?; + if let Some(by) = by { + if str_(p, "if_redundant").is_some_and(|m| m.eq_ignore_ascii_case("driven")) { + s.doc = start; + let mut q = p.clone(); + q["driven"] = json!(true); + if let Some(o) = q.as_object_mut() { + o.remove("if_redundant"); + o.remove("value"); + } + let mut out = dimension(s, &q)?; + out["redundant"] = json!(true); + out["determined_by"] = json!(by); + return Ok(out); + } + return Err(bad( + cmd, + format!("that dimension is already determined by {}; add it as a driven (reference) dimension with driven: true", listed(&by)), + )); } let v = s.doc.param(¶m).map(|p| p.expr.clone()).unwrap_or_default(); Ok(json!({"param": param, "type": kind_name, "expression": v, "measured": current.0, "unit": current.1, "sketch": info})) @@ -2156,3 +2320,6 @@ mod drag_live_tests; #[cfg(test)] #[path = "sketch_ids_tests.rs"] mod ids_tests; +#[cfg(test)] +#[path = "sketch_redundant_tests.rs"] +mod redundant_tests; diff --git a/crates/engine/src/cmd/sketch_axis_ref_tests.rs b/crates/engine/src/cmd/sketch_axis_ref_tests.rs index 2f239792..828b44ee 100644 --- a/crates/engine/src/cmd/sketch_axis_ref_tests.rs +++ b/crates/engine/src/cmd/sketch_axis_ref_tests.rs @@ -232,7 +232,8 @@ fn bad_line_references_are_errors_and_change_nothing() { // A constraint the sketch refuses rolls the axis line back with it. run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": "b", "mode": "horizontal"})); let before_redundant = s.doc.to_json(); - assert!(s.execute("sketch.constraint.parallel", &json!({"a": "b", "b": "x"})).is_err(), "already horizontal"); + // (A redundant one is kept unless refused: refused here.) + assert!(s.execute("sketch.constraint.parallel", &json!({"a": "b", "b": "x", "if_redundant": "refuse"})).is_err(), "already horizontal"); assert_eq!(s.doc.to_json(), before_redundant); assert!(axis_lines(&sketch(&s)).is_empty()); } diff --git a/crates/engine/src/cmd/sketch_constraints.rs b/crates/engine/src/cmd/sketch_constraints.rs index 7acd97f9..32e5fb7c 100644 --- a/crates/engine/src/cmd/sketch_constraints.rs +++ b/crates/engine/src/cmd/sketch_constraints.rs @@ -5,7 +5,7 @@ use serde_json::{Value, json}; use solidcraft_geom::Vec2; use solidcraft_sketch::{ConstraintKind, CurveKind, Sketch}; -use super::sketch::{add_c, dof_now, edit, reject_redundant}; +use super::sketch::{add_c, constraint_result, dof_now, edit, listed, redundancy, residual_of}; use super::{CommandSpec, in_sketch}; use crate::params::{bad, num, req_vec2, str_, string_list, vec2}; use crate::{EngineError, Result, Session}; @@ -56,15 +56,17 @@ pub static COMMANDS: &[CommandSpec] = &[ fn c_smooth(s: &mut Session, p: &Value) -> Result { let cmd = "sketch.constraint.curvature"; - let mut before = None; + let (mut before, mut residual) = (None, None); let (id, info) = edit(s, p, cmd, true, |sk, _| { let a = curve(sk, p.get("a"), cmd)?; let b = curve(sk, p.get("b"), cmd)?; before = Some(dof_now(sk)); - add_c(sk, ConstraintKind::Smooth { a, b }) + let id = add_c(sk, ConstraintKind::Smooth { a, b })?; + residual = residual_of(sk, &id); + Ok(id) })?; - reject_redundant(before, &info, cmd)?; - Ok(json!({"constraint": id, "sketch": info})) + let by = redundancy(s, p, cmd, before, &info, residual, &id)?; + constraint_result(p, cmd, id, info, by, Value::Null) } fn curve(sk: &Sketch, v: Option<&Value>, cmd: &str) -> Result { @@ -349,16 +351,18 @@ fn guess(sk: &Sketch, ents: &[String], cmd: &str) -> Result { fn smart_constrain(s: &mut Session, p: &Value) -> Result { let cmd = "sketch.constrainer"; let ents = string_list(p, "entities"); - let mut before = None; + let (mut before, mut residual) = (None, None); let mut name = ""; let (id, info) = edit(s, p, cmd, true, |sk, _| { let k = guess(sk, &ents, cmd)?; name = k.name(); before = Some(dof_now(sk)); - add_c(sk, k) + let id = add_c(sk, k)?; + residual = residual_of(sk, &id); + Ok(id) })?; - reject_redundant(before, &info, cmd)?; - Ok(json!({"constraint": id, "type": name, "sketch": info})) + let by = redundancy(s, p, cmd, before, &info, residual, &id)?; + constraint_result(p, cmd, id, info, by, json!({"type": name})) } fn toggle_driven(s: &mut Session, p: &Value) -> Result { @@ -397,7 +401,10 @@ fn toggle_driven(s: &mut Session, p: &Value) -> Result { c.driven = true; } before = Some(dof_now(&sk)); - reject_redundant(before, &info, cmd)?; + // It holds (its parameter takes the value it measures), so it can only be redundant. + if let Some(by) = redundancy(s, p, cmd, before, &info, Some(0.0), &cid)? { + return Err(bad(cmd, format!("that dimension is already determined by {}; it stays driven", listed(&by)))); + } } s.doc_mut().prune_model_params(); Ok(json!({"constraint": cid, "driven": driven, "sketch": info})) @@ -468,6 +475,8 @@ fn glyphs(s: &mut Session, p: &Value) -> Result { "param": c.param, "at": anchor(&ss.sketch, &c.kind), "failing": failing.contains(&c.id), + // Implied by the others (it removes no freedom) but holding: shown dimmed. + "redundant": ss.report.redundant.contains(&c.id), }) }) .collect(); diff --git a/crates/engine/src/cmd/sketch_constraints_tests.rs b/crates/engine/src/cmd/sketch_constraints_tests.rs index 5070d57c..4e3efc03 100644 --- a/crates/engine/src/cmd/sketch_constraints_tests.rs +++ b/crates/engine/src/cmd/sketch_constraints_tests.rs @@ -25,17 +25,17 @@ fn sketch(s: &Session) -> solidcraft_sketch::Sketch { } #[test] -fn redundant_constraints_and_dimensions_are_refused_driven_ones_measure() { +fn redundant_dimensions_are_refused_driven_ones_measure() { let mut s = new_sketch(); let l = ids(&run(&mut s, "sketch.line", json!({"points": [[0, 0], [10, 0]], "infer": true}))["curves"])[0].clone(); - // Already horizontal (inferred). - let e = s.execute("sketch.constraint.horizontal_vertical", &json!({"line": l})).unwrap_err().to_string(); - assert!(e.contains("over-constrain"), "{e}"); + // Already horizontal (inferred): kept as redundant, unless asked to refuse it. + let e = s.execute("sketch.constraint.horizontal_vertical", &json!({"line": l, "if_redundant": "refuse"})).unwrap_err().to_string(); + assert!(e.contains("over-constrain") && e.contains("already determined by"), "{e}"); run(&mut s, "sketch.constraint.coincident", json!({"a": format!("{l}.start"), "b": "origin"})); run(&mut s, "sketch.dimension", json!({"entities": [l], "value": 10})); let si = run(&mut s, "sketch.inspect", json!({})); assert_eq!(si["dof"], 0, "{si}"); - // A second length on it over-constrains: refused, unless driven. + // A second length on it is already determined: refused, unless driven. assert!(s.execute("sketch.dimension", &json!({"entities": [format!("{l}.start"), format!("{l}.end")]})).is_err()); let d = run(&mut s, "sketch.dimension", json!({"entities": [format!("{l}.start"), format!("{l}.end")], "driven": true})); assert_eq!(d["driven"], true); diff --git a/crates/engine/src/cmd/sketch_dimensioned.rs b/crates/engine/src/cmd/sketch_dimensioned.rs index 7f04d883..168ace7e 100644 --- a/crates/engine/src/cmd/sketch_dimensioned.rs +++ b/crates/engine/src/cmd/sketch_dimensioned.rs @@ -95,7 +95,9 @@ pub(super) fn draw(s: &mut Session, p: &Value) -> Result { } _ => return Err(bad(cmd, "unsupported dimension role")), }; - let mut dimension = json!({"entities": entities, "type": ty, "value": value}); + // A typed size the sketch already has (a rectangle drawn onto another's side, the same + // height) is placed as a driven (reference) dimension rather than refused. + let mut dimension = json!({"entities": entities, "type": ty, "value": value, "if_redundant": "driven"}); if let Some(sketch) = params.get("sketch") { dimension["sketch"] = sketch.clone(); } diff --git a/crates/engine/src/cmd/sketch_redundant_tests.rs b/crates/engine/src/cmd/sketch_redundant_tests.rs new file mode 100644 index 00000000..c3205b59 --- /dev/null +++ b/crates/engine/src/cmd/sketch_redundant_tests.rs @@ -0,0 +1,252 @@ +//! Redundant but consistent constraints (#687): rectangles of one size drawn around a shared +//! corner (a "clover"), or onto each other's sides, imply some of their constraints twice. Those +//! are kept (and reported as redundant) or not added at all; only constraints that conflict +//! with the sketch are refused. + +use serde_json::{Value, json}; +use solidcraft_geom::Vec2; + +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 new_sketch() -> Session { + let mut s = Session::default(); + run(&mut s, "sketch.create", json!({"plane": "XY"})); + s +} + +fn sketch(s: &Session) -> solidcraft_sketch::Sketch { + s.model.state().sketch(s.active_sketch.unwrap()).unwrap().sketch.clone() +} + +fn report(s: &Session) -> solidcraft_sketch::SolveReport { + s.model.state().sketch(s.active_sketch.unwrap()).unwrap().report.clone() +} + +fn glyphs(s: &mut Session) -> Value { + run(s, "sketch.glyphs", json!({})) +} + +/// The four quadrants around `c`, each `w` square: bottom-left, bottom-right, top-right, +/// top-left (each next one shares a side with the one before; the last shares two). +fn quadrants(c: [f64; 2], w: f64) -> [([f64; 2], [f64; 2]); 4] { + let [x, y] = c; + [([x - w, y - w], [x, y]), ([x, y - w], [x + w, y]), ([x, y], [x + w, y + w]), ([x - w, y], [x, y + w])] +} + +fn horizontal_vertical(sk: &solidcraft_sketch::Sketch) -> usize { + sk.constraints + .iter() + .filter(|c| matches!(c.kind, solidcraft_sketch::ConstraintKind::Horizontal { .. } | solidcraft_sketch::ConstraintKind::Vertical { .. })) + .count() +} + +/// The owner's steps: rectangles of one size (typed 10 × 10) drawn snapped onto a shared +/// corner, 2, 3 and 4 of them. Every one is accepted; a size the sketch already has becomes a +/// driven (reference) dimension, and the sketch ends fully constrained. +#[test] +fn a_clover_of_same_size_rectangles_drawn_by_snapping_is_accepted() { + for n in 2..=4 { + let mut s = new_sketch(); + for (i, (a, b)) in quadrants([0.0, 0.0], 10.0).iter().take(n).enumerate() { + let r = s.execute( + "sketch.draw.dimensioned", + &json!({"command": "sketch.rectangle.two_point", "params": {"p0": a, "p1": b}, "dimensions": {"width": 10, "height": 10}}), + ); + let r = r.unwrap_or_else(|e| panic!("rectangle {i} of {n}: {e}")); + assert_eq!(r["sketch"]["solved"], true, "{r}"); + } + let g = glyphs(&mut s); + assert_eq!(g["fully_constrained"], true, "{n} rectangles: {g}"); + assert_eq!(g["conflicts"], json!([]), "{g}"); + let sk = sketch(&s); + assert_eq!(sk.curves.len(), 4 * n); + // Shared sides carry one horizontal/vertical, not two: 4 for the first rectangle, 3 for + // each with one shared side, 2 for the last of four (two shared sides). + let expected = [0, 4, 7, 10, 12][n]; + assert_eq!(horizontal_vertical(&sk), expected, "{n} rectangles"); + // Each later rectangle's size along a shared side is a reference dimension. + let driven = sk.constraints.iter().filter(|c| c.driven).count(); + assert_eq!(driven, [0, 0, 1, 2, 4][n], "{n} rectangles"); + for (a, b) in quadrants([0.0, 0.0], 10.0).iter().take(n) { + for corner in [Vec2::new(a[0], a[1]), Vec2::new(b[0], b[1])] { + assert!(sk.points.iter().any(|p| p.pos.dist(corner) < 1e-9), "{n} rectangles: corner {corner:?} moved"); + } + } + } +} + +/// Undimensioned, the same clover is drawn without a single redundant constraint: shared +/// sides are not constrained twice, and the freedom left is the two column widths and the two +/// row heights. +#[test] +fn rectangles_sharing_sides_do_not_constrain_a_side_twice() { + let mut s = new_sketch(); + for (a, b) in quadrants([50.0, 50.0], 10.0) { + run(&mut s, "sketch.rectangle.two_point", json!({"p0": a, "p1": b})); + } + let rep = report(&s); + assert!(rep.ok(), "{rep:?}"); + assert_eq!(rep.redundant, Vec::::new()); + // Free: the shared corner (2), two column widths, two row heights. + assert_eq!(rep.dof, 6); + assert_eq!(horizontal_vertical(&sketch(&s)), 12); +} + +/// Two rectangles on one side: the second only adds its width. +#[test] +fn shared_edge_rectangles() { + let mut s = new_sketch(); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [10, 10]})); + let r = run(&mut s, "sketch.rectangle.two_point", json!({"p0": [10, 0], "p1": [25, 10]})); + // Three of its sides' directions and its two corners on the first one. + assert_eq!(r["constraints"].as_array().map(Vec::len), Some(5), "{r}"); + let rep = report(&s); + assert_eq!(rep.dof, 3, "the first one's width and height, the second one's width"); + assert!(rep.redundant.is_empty()); + // Sized and placed, all of it is fixed. + run(&mut s, "sketch.dimension", json!({"entities": ["l1"], "value": 10})); + run(&mut s, "sketch.dimension", json!({"entities": ["l2"], "value": 10})); + run(&mut s, "sketch.dimension", json!({"entities": ["l5"], "value": 15})); + assert_eq!(glyphs(&mut s)["fully_constrained"], true); +} + +/// Two separately drawn 10 × 10 squares snapped corner to corner: the second snap is implied by +/// the first and the equal sizes. It is kept, reported as redundant along with what implies +/// it, and the degrees of freedom and the fully constrained state are unchanged. +#[test] +fn snapping_a_second_corner_of_same_size_rectangles_is_redundant_not_refused() { + let mut s = new_sketch(); + for (a, b) in [([0.0, 0.0], [10.0, 10.0]), ([20.0, 0.0], [30.0, 10.0])] { + run( + &mut s, + "sketch.draw.dimensioned", + json!({"command": "sketch.rectangle.two_point", "params": {"p0": a, "p1": b}, "dimensions": {"width": 10, "height": 10}}), + ); + } + // The second square's bottom-left corner onto the first one's bottom-right. + run(&mut s, "sketch.constraint.coincident", json!({"a": "l5.start", "b": "l1.end"})); + let dof = report(&s).dof; + assert_eq!(dof, 0, "the first square is on the origin, the second one placed by its corner"); + // Its top-left corner is already on the first one's top-right. + let r = run(&mut s, "sketch.constraint.coincident", json!({"a": "l7.end", "b": "l2.end"})); + assert_eq!(r["redundant"], true, "{r}"); + assert!(!r["determined_by"].as_array().is_none_or(Vec::is_empty), "{r}"); + let rep = report(&s); + assert_eq!((rep.ok(), rep.dof), (true, 0)); + assert_eq!(rep.redundant, vec![r["constraint"].as_str().unwrap().to_string()]); + let g = glyphs(&mut s); + assert_eq!(g["fully_constrained"], true); + let flagged: Vec<&Value> = g["glyphs"].as_array().unwrap().iter().filter(|x| x["redundant"] == true).collect(); + assert_eq!(flagged.len(), 1, "{g}"); + // Drawing-time inferences ask for redundant ones to be left out. + let e = s.execute("sketch.constraint.horizontal_vertical", &json!({"line": "l8", "if_redundant": "refuse"})).unwrap_err().to_string(); + assert!(e.contains("already determined by"), "{e}"); +} + +/// A size the sketch already has, dimensioned again: refused as already determined (naming +/// what determines it), or placed as a driven dimension; an equal constraint the sketch already +/// implies is kept as redundant. +#[test] +fn redundant_equal_dimensions() { + let mut s = new_sketch(); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [10, 10]})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [10, 0], "p1": [20, 10]})); + let height = run(&mut s, "sketch.dimension", json!({"entities": ["l2"], "value": 10})); + let k = sketch(&s).constraints.iter().find(|c| c.param.as_deref() == height["param"].as_str()).map(|c| c.id.clone()).unwrap(); + // The second one's right side is as tall as the shared side. + let e = s.execute("sketch.dimension", &json!({"entities": ["l6"], "value": 10})).unwrap_err().to_string(); + assert!(e.contains("already determined by") && e.contains(&k), "{e}"); + let d = run(&mut s, "sketch.dimension", json!({"entities": ["l6"], "value": 10, "if_redundant": "driven"})); + assert_eq!((d["driven"].clone(), d["redundant"].clone()), (json!(true), json!(true)), "{d}"); + // Equal widths, twice: the second is implied by the first. + run(&mut s, "sketch.constraint.equal", json!({"a": "l1", "b": "l5"})); + let before = report(&s).dof; + let r = run(&mut s, "sketch.constraint.equal", json!({"a": "l3", "b": "l7"})); + assert_eq!(r["redundant"], true, "{r}"); + assert_eq!(report(&s).dof, before); + // Its driving value changes still solve with the redundant constraints present. + let pname = height["param"].as_str().unwrap().to_string(); + run(&mut s, "parameters.change", json!({"name": pname, "expression": "14 mm"})); + let sk = sketch(&s); + assert!(report(&s).ok()); + let len = |id: &str| { + let c = sk.curve_index(id).unwrap(); + let solidcraft_sketch::CurveKind::Line { a, b } = sk.curves[c].kind else { panic!() }; + sk.points[a].pos.dist(sk.points[b].pos) + }; + assert!((len("l6") - 14.0).abs() < 1e-7 && (len("l4") - 14.0).abs() < 1e-7); +} + +/// A constraint that does not hold where the others put the geometry still conflicts. +#[test] +fn a_conflicting_constraint_is_still_refused() { + let mut s = new_sketch(); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [10, 10]})); + run(&mut s, "sketch.rectangle.two_point", json!({"p0": [10, 0], "p1": [20, 10]})); + run(&mut s, "sketch.dimension", json!({"entities": ["l2"], "value": 10})); + let before = sketch(&s).constraints.len(); + // The second one's right side cannot be 12 while the shared side is 10. + let e = s.execute("sketch.dimension", &json!({"entities": ["l6"], "value": 12})).unwrap_err().to_string(); + assert!(e.contains("over-constrain") || e.contains("conflicts"), "{e}"); + let e = s.execute("sketch.dimension", &json!({"entities": ["l6"], "value": 12, "if_redundant": "driven"})).unwrap_err().to_string(); + assert!(e.contains("over-constrain") || e.contains("conflicts"), "{e}"); + // A vertical side cannot be horizontal. + let e = s.execute("sketch.constraint.horizontal_vertical", &json!({"line": "l2", "mode": "horizontal"})).unwrap_err().to_string(); + assert!(e.contains("over-constrain"), "{e}"); + assert_eq!(sketch(&s).constraints.len(), before); + assert!(report(&s).redundant.is_empty()); +} + +/// Dragging the shared corner of a clover moves all four rectangles with it, redundant +/// constraints and all. +#[test] +fn dragging_the_shared_corner_moves_every_rectangle() { + let mut s = new_sketch(); + for (a, b) in quadrants([50.0, 50.0], 10.0) { + run(&mut s, "sketch.rectangle.two_point", json!({"p0": a, "p1": b})); + } + // A redundant equal and a redundant vertical, so the drag has some to carry. + run(&mut s, "sketch.constraint.equal", json!({"a": "l1", "b": "l13"})); + let r = run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": "l4"})); + assert_eq!(r["redundant"], true, "{r}"); + assert!(!report(&s).redundant.is_empty()); + let centre = Vec2::new(50.0, 50.0); + let id = sketch(&s).points.iter().find(|p| p.pos.dist(centre) < 1e-9).map(|p| p.id.clone()).unwrap(); + run(&mut s, "sketch.drag", json!({"entity": id, "from": [50, 50], "to": [56, 47]})); + let sk = sketch(&s); + let rep = report(&s); + assert!(rep.ok(), "{rep:?}"); + let to = Vec2::new(56.0, 47.0); + // Each rectangle still has a corner on the moved one and is still a rectangle. + let near: usize = sk.points.iter().filter(|p| p.pos.dist(to) < 1e-6).count(); + assert!(near >= 4, "{near} corners at the dragged spot"); + for l in &sk.curves { + let solidcraft_sketch::CurveKind::Line { a, b } = l.kind else { continue }; + let d = sk.points[b].pos - sk.points[a].pos; + assert!(d.x.abs() < 1e-7 || d.y.abs() < 1e-7, "{} is no longer axis-aligned: {d:?}", l.id); + } + assert!(sk.points.iter().all(|p| p.pos.dist(centre) > 1e-6), "nothing stayed behind at the old corner"); +} + +/// A horizontal on a line that already is (it was drawn so, inferred) is redundant, not an +/// error; the degrees of freedom do not change, and the sketch still turns fully constrained. +#[test] +fn an_explicit_redundant_constraint_keeps_the_dof_and_full_constraint() { + let mut s = new_sketch(); + let l = run(&mut s, "sketch.line", json!({"points": [[0, 0], [10, 0]], "infer": true}))["curves"][0].as_str().unwrap().to_string(); + let r = run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": l})); + assert_eq!(r["redundant"], true, "{r}"); + // Its start is on the origin (snapped): only its length is free. + assert_eq!(report(&s).dof, 1); + run(&mut s, "sketch.dimension", json!({"entities": [l], "value": 10})); + let g = glyphs(&mut s); + assert_eq!((g["dof"].clone(), g["fully_constrained"].clone()), (json!(0), json!(true)), "{g}"); + assert!(report(&s).point_determined.iter().all(|d| *d)); +} diff --git a/crates/sketch/src/lib.rs b/crates/sketch/src/lib.rs index c2313655..8d37bd55 100644 --- a/crates/sketch/src/lib.rs +++ b/crates/sketch/src/lib.rs @@ -55,8 +55,8 @@ pub use profiles::{ }; pub use shape::{Shape, intersections}; pub use solver::{ - Analysis, DrivingValues, Hold, SolveReport, SolveStatus, measure_dimension, solve, solve_after_edit, solve_after_value_edit, solve_holding, - solve_with, + Analysis, CONSISTENT, DrivingValues, Hold, SolveReport, SolveStatus, constraint_residual, determined_by, measure_dimension, solve, + solve_after_edit, solve_after_value_edit, solve_holding, solve_with, }; pub use text::{MAX_TEXT_CHARS, text_geometry, text_geometry_in}; pub use wire::{MAX_FIT_POINTS, MAX_WIRE_POINTS, Wire, WireSpace, fit_polyline}; diff --git a/crates/sketch/src/redundancy.rs b/crates/sketch/src/redundancy.rs new file mode 100644 index 00000000..efe6196e --- /dev/null +++ b/crates/sketch/src/redundancy.rs @@ -0,0 +1,171 @@ +//! Redundant constraints: ones whose equations are already implied by the others where the +//! sketch is now (two rectangles sharing a side both say the side is vertical; a coincidence +//! between two corners that equal sizes already put together). They remove no degree of freedom +//! but they do not conflict either: the solver satisfies them along with the rest. +//! +//! The test is the Jacobian's: walking the rows in order (the curves' own relations, then the +//! constraints in the order they were added), a row that adds no rank to the rows before it is +//! dependent on them. A constraint none of whose rows add rank is redundant. Whether a +//! redundant constraint is consistent is a question of its residual (see +//! [`constraint_residual`]): zero where the other constraints hold means it is implied, not in +//! conflict. + +use super::{Block, Hold, Layout, State, block_residuals, build_blocks, columns, initial_x, layout, local_system, max_abs, points_on_curves}; +use crate::linalg::{Mat, solve as lin_solve}; +use crate::model::Sketch; +use crate::union_find::{find, union}; + +/// A row is dependent on the rows before it when what is left of it, once their directions are +/// taken out, is below this share of its length (the rank analysis' pivot tolerance is 1e-7 on +/// rows of a comparable scale). +const DEPENDENT: f64 = 1e-6; +/// Variables in one component past which its constraints are not checked for redundancy: the +/// check is dense, rows × rank × variables. +pub(super) const MAX_REDUNDANCY_VARS: usize = 400; +/// A constraint's residual (mm, or radians for angles) below which it holds. +pub const CONSISTENT: f64 = 1e-6; + +/// The constraint a block stands for. +pub(super) fn block_constraint(b: &Block) -> Option { + match b { + Block::User(ci, _) | Block::TangentAt(ci, _, _) => Some(*ci), + Block::Closure { .. } | Block::Arc { .. } => None, + } +} + +fn dot(a: &[f64], b: &[f64]) -> f64 { + a.iter().zip(b).map(|(x, y)| x * y).sum() +} + +/// Per block of `blocks` (in order, over the variables `vars`): whether every one of its rows +/// is a combination of the rows before it. With `support`, also for the block at that index +/// (when redundant) the earlier blocks its rows combine. +pub(super) fn dependent_blocks( + sk: &Sketch, + lay: &Layout, + blocks: &[(Block, Vec)], + vars: &[usize], + x: &mut [f64], + support: Option, +) -> (Vec, Vec) { + let mut col = vec![usize::MAX; lay.n]; + columns(&mut col, vars, vars); + let (_, j, owner) = local_system(sk, lay, blocks, x, &col, vars.len()); + let n = j.cols; + let row = |k: usize| -> Vec { j.data.get(k * n..(k + 1) * n).map(<[f64]>::to_vec).unwrap_or_default() }; + // Orthonormal directions of the independent rows so far, and those rows with their block. + let mut q: Vec> = Vec::new(); + let mut basis: Vec<(Vec, usize)> = Vec::new(); + let mut has_rows = vec![false; blocks.len()]; + let mut adds_rank = vec![false; blocks.len()]; + let mut target_rows: Vec> = Vec::new(); + for k in 0..j.rows { + let Some(&b) = owner.get(k) else { continue }; + let v = row(k); + let norm = dot(&v, &v).sqrt(); + let mut w = v.clone(); + // Twice, for the orthogonality lost to rounding. + for _ in 0..2 { + for qi in &q { + let d = dot(qi, &w); + for (wk, qk) in w.iter_mut().zip(qi) { + *wk -= d * qk; + } + } + } + let left = dot(&w, &w).sqrt(); + if let Some(h) = has_rows.get_mut(b) { + *h = true; + } + if norm.is_finite() && left.is_finite() && norm > 0.0 && left > DEPENDENT * norm { + q.push(w.iter().map(|wk| wk / left).collect()); + basis.push((v, b)); + if let Some(a) = adds_rank.get_mut(b) { + *a = true; + } + } else if Some(b) == support { + target_rows.push(v); + } + } + let dependent: Vec = has_rows.iter().zip(&adds_rank).map(|(h, a)| *h && !*a).collect(); + let mut by = Vec::new(); + if let Some(t) = support + && dependent.get(t).copied().unwrap_or(false) + && !basis.is_empty() + { + // The combination of the independent rows that gives each of the target's rows: the + // least-squares solution of Bᵀ a = v, from (B Bᵀ) a = B v (B has full row rank). + let m = basis.len(); + let mut g = Mat::zeros(m, m); + for (r, (br, _)) in basis.iter().enumerate() { + for (c, (bc, _)) in basis.iter().enumerate() { + g.set(r, c, dot(br, bc)); + } + } + for v in &target_rows { + let rhs: Vec = basis.iter().map(|(b, _)| dot(b, v)).collect(); + let Some(a) = lin_solve(g.clone(), rhs) else { continue }; + let big = a.iter().fold(0.0_f64, |m, x| m.max(x.abs())); + for (ak, (_, b)) in a.iter().zip(&basis) { + if ak.abs() > 1e-6 * big && !by.contains(b) { + by.push(*b); + } + } + } + by.sort_unstable(); + } + (dependent, by) +} + +/// How far the constraint at index `ci` is from holding where the sketch is now: its largest +/// residual (mm, or radians for angles). Zero for a constraint that holds, so a constraint that +/// removes no degree of freedom but holds is redundant, not conflicting. `None` for a driven +/// dimension (it constrains nothing) or an unknown index. +pub fn constraint_residual(sk: &Sketch, ci: usize) -> Option { + let c = sk.constraints.get(ci).filter(|c| !c.driven)?; + let fixed = Layout { pvar: vec![None; sk.points.len()], rvar: vec![None; sk.curves.len()], n: 0, points: 0 }; + let on_curve = points_on_curves(sk); + let b = match super::touch_point(sk, &c.kind, &on_curve) { + Some(p) => Block::TangentAt(ci, &c.kind, p), + None => Block::User(ci, &c.kind), + }; + let mut r = Vec::new(); + block_residuals(&State { sk, lay: &fixed, x: &[] }, &b, &mut r); + Some(max_abs(&r)) +} + +/// The constraints (indices) that already determine what the constraint at index `ci` asks +/// for, where the sketch is now: the ones whose equations its own are a combination of. Empty +/// when it is not redundant (it removes a degree of freedom), or when its component is too +/// large to check. +pub fn determined_by(sk: &Sketch, ci: usize) -> Vec { + let lay = layout(sk, &Hold::default()); + let mut x = initial_x(sk, &lay); + let on_curve = points_on_curves(sk); + let (blocks, _) = build_blocks(sk, &lay, &on_curve); + let Some(t) = blocks.iter().position(|(b, _)| block_constraint(b) == Some(ci)) else { return Vec::new() }; + let mut parent: Vec = (0..lay.n).collect(); + for (_, vars) in &blocks { + if let Some(&first) = vars.first() { + for &v in vars.iter().skip(1) { + union(&mut parent, first, v); + } + } + } + let Some(root) = blocks.get(t).and_then(|(_, v)| v.first().copied()).map(|v| find(&mut parent, v)) else { return Vec::new() }; + let vars: Vec = (0..lay.n).filter(|v| find(&mut parent, *v) == root).collect(); + if vars.len() > MAX_REDUNDANCY_VARS { + return Vec::new(); + } + // The component's other blocks in order, then the constraint asked about. + let mut order: Vec = (0..blocks.len()) + .filter(|bi| *bi != t && blocks.get(*bi).and_then(|(_, v)| v.first()).is_some_and(|v| find(&mut parent, *v) == root)) + .collect(); + order.push(t); + let sub: Vec<(Block, Vec)> = order.iter().filter_map(|bi| blocks.get(*bi).map(|(b, v)| (super::copy_block(b), v.clone()))).collect(); + let (_, by) = dependent_blocks(sk, &lay, &sub, &vars, &mut x, Some(sub.len().saturating_sub(1))); + let mut out: Vec = by.iter().filter_map(|k| sub.get(*k).and_then(|(b, _)| block_constraint(b))).collect(); + out.sort_unstable(); + out.dedup(); + out +} diff --git a/crates/sketch/src/solver.rs b/crates/sketch/src/solver.rs index 8df6972f..58c6e3e7 100644 --- a/crates/sketch/src/solver.rs +++ b/crates/sketch/src/solver.rs @@ -21,6 +21,10 @@ use crate::linalg::{Mat, determined_vars, solve as lin_solve}; use crate::model::{ConstraintKind, CurveKind, Sketch}; use crate::union_find::{find, union}; +#[path = "redundancy.rs"] +mod redundancy; +pub use redundancy::{CONSISTENT, constraint_residual, determined_by}; + const MAX_ITERS: usize = 500; /// Variables in one connected component. Each component is solved as a dense system: an n by n /// matrix for the Levenberg-Marquardt step, rows by n for the Jacobian. 4,000 variables is a @@ -65,6 +69,12 @@ pub struct SolveReport { pub point_determined: Vec, /// Per curve: all of its defining values determined. pub curve_determined: Vec, + /// Constraints that remove no degree of freedom: everything they ask for is already implied + /// by the constraints before them (a second "vertical" on a side two rectangles share). + /// They hold, so they are not a conflict; they are shown as redundant. Only from a full + /// analysis, and only for components of at most a few hundred variables. + #[serde(default)] + pub redundant: Vec, } impl SolveReport { @@ -1298,6 +1308,60 @@ impl Sketch { } } +/// The variables' values where the sketch is now. +fn initial_x(sk: &Sketch, lay: &Layout) -> Vec { + let mut x = vec![0.0; lay.n]; + for (i, p) in sk.points.iter().enumerate() { + if let Some(v) = lay.pvar.get(i).copied().flatten() { + set_point(&mut x, v, p.pos); + } + } + for (i, c) in sk.curves.iter().enumerate() { + if let (CurveKind::Circle { r, .. } | CurveKind::Ellipse { r, .. }, Some(v)) = (&c.kind, lay.rvar.get(i).copied().flatten()) + && let Some(s) = x.get_mut(v) + { + *s = *r; + } + } + x +} + +/// The solve's blocks in order (the curves' own relations, then the driving constraints in +/// constraint order) with the variables each reads, and per block whether it reads a constant +/// point. +fn build_blocks<'a>(sk: &'a Sketch, lay: &Layout, on_curve: &[(usize, usize)]) -> (Vec<(Block<'a>, Vec)>, Vec) { + let mut raw = Vec::new(); + for c in &sk.curves { + if let CurveKind::Spline { pts, periodic: true, .. } = &c.kind + && let (Some(a), Some(b)) = (pts.first(), pts.last()) + && a != b + { + raw.push(Block::Closure { a: *a, b: *b }); + } + if let CurveKind::Arc { c, a, b } = c.kind { + raw.push(Block::Arc { c, a, b }); + } + } + for (i, c) in sk.constraints.iter().enumerate() { + if c.driven { + continue; + } + raw.push(match touch_point(sk, &c.kind, on_curve) { + Some(p) => Block::TangentAt(i, &c.kind, p), + None => Block::User(i, &c.kind), + }); + } + let mut blocks: Vec<(Block, Vec)> = Vec::with_capacity(raw.len()); + let mut reads_constant = Vec::with_capacity(raw.len()); + for bl in raw { + let (vars, constant) = block_vars(sk, lay, &bl); + blocks.push((bl, vars)); + reads_constant.push(constant); + } + + (blocks, reads_constant) +} + #[cfg(not(test))] fn solve_from( sk: &mut Sketch, @@ -1721,19 +1785,7 @@ fn solve_inner( } } let lay = layout(sk, hold); - let mut x = vec![0.0; lay.n]; - for (i, p) in sk.points.iter().enumerate() { - if let Some(v) = lay.pvar.get(i).copied().flatten() { - set_point(&mut x, v, p.pos); - } - } - for (i, c) in sk.curves.iter().enumerate() { - if let (CurveKind::Circle { r, .. } | CurveKind::Ellipse { r, .. }, Some(v)) = (&c.kind, lay.rvar.get(i).copied().flatten()) - && let Some(s) = x.get_mut(v) - { - *s = *r; - } - } + let mut x = initial_x(sk, &lay); let x0 = x.clone(); let on_curve = points_on_curves(sk); // Start from the constructed positions of the points the constraints pin down (see @@ -1785,34 +1837,7 @@ fn solve_inner( nudge_line_angles(sk, &lay, &mut x); // The blocks, and per block whether it reads a constant point. - let mut raw = Vec::new(); - for c in &sk.curves { - if let CurveKind::Spline { pts, periodic: true, .. } = &c.kind - && let (Some(a), Some(b)) = (pts.first(), pts.last()) - && a != b - { - raw.push(Block::Closure { a: *a, b: *b }); - } - if let CurveKind::Arc { c, a, b } = c.kind { - raw.push(Block::Arc { c, a, b }); - } - } - for (i, c) in sk.constraints.iter().enumerate() { - if c.driven { - continue; - } - raw.push(match touch_point(sk, &c.kind, &on_curve) { - Some(p) => Block::TangentAt(i, &c.kind, p), - None => Block::User(i, &c.kind), - }); - } - let mut blocks: Vec<(Block, Vec)> = Vec::with_capacity(raw.len()); - let mut reads_constant = Vec::with_capacity(raw.len()); - for bl in raw { - let (vars, constant) = block_vars(sk, &lay, &bl); - blocks.push((bl, vars)); - reads_constant.push(constant); - } + let (blocks, reads_constant) = build_blocks(sk, &lay, &on_curve); carry_with_seeded(&mut x, &x0, &constructed, &blocks, &lay); // The plain start, for the variables whose start the seed (or carrying along it) moved. @@ -1981,6 +2006,7 @@ fn solve_inner( let mut failing: Vec = Vec::new(); let mut det = vec![false; lay.n]; let mut rank = 0; + let mut redundant: Vec = Vec::new(); // A component too large to solve is a failure, not an allocation. if comps.iter().any(|(_, vars, bis)| !bis.is_empty() && oversized(vars)) { max_residual = f64::MAX; @@ -2023,6 +2049,17 @@ fn solve_inner( let (rest_det, rest_rank) = if j.rows > 0 { determined_vars(&j, 1e-7) } else { (vec![false; rest.len()], 0) }; let comp_rank = pin_flags.iter().filter(|p| **p).count() + rest_rank; rank += comp_rank; + // More rows than rank: some constraint may be implied by the others. + if r.len() > comp_rank && vars.len() <= redundancy::MAX_REDUNDANCY_VARS { + let (dependent, _) = redundancy::dependent_blocks(sk, &lay, &sub, vars, &mut x, None); + for ((b, _), d) in sub.iter().zip(dependent) { + if d && let Some(c) = redundancy::block_constraint(b).and_then(|ci| sk.constraints.get(ci)) + && !redundant.contains(&c.id) + { + redundant.push(c.id.clone()); + } + } + } for (&v, pinned) in vars.iter().zip(&pin_flags) { let flag = match rest.binary_search(&v) { Ok(k) => rest_det.get(k).copied().unwrap_or(false), @@ -2171,6 +2208,7 @@ fn solve_inner( continuations, point_determined, curve_determined, + redundant, } } diff --git a/crates/ui-egui/src/dim_view.rs b/crates/ui-egui/src/dim_view.rs index 4086b37f..b57c77aa 100644 --- a/crates/ui-egui/src/dim_view.rs +++ b/crates/ui-egui/src/dim_view.rs @@ -128,7 +128,7 @@ pub fn begin_dimension(app: &mut SolveApp, entities: Vec, at: Vec2) -> R if draft.driven { PLACEMENT.with(|preview| *preview.borrow_mut() = None); app.run("sketch.dimension", json!({"entities": draft.entities, "text_at": [at.x, at.y], "driven": true}))?; - app.set_status("That dimension would over-constrain the sketch, so it was added as a driven (reference) dimension", false); + app.set_status("The sketch already determines that dimension, so it was added as a driven (reference) dimension", false); return Ok(()); } EDIT.with(|edit| *edit.borrow_mut() = Some(Edit { param, expr, at: None, fresh: FRESH, wait: 10, draft: Some(draft), live: None })); diff --git a/crates/ui-egui/src/inference.rs b/crates/ui-egui/src/inference.rs index e27f40b3..d9067fda 100644 --- a/crates/ui-egui/src/inference.rs +++ b/crates/ui-egui/src/inference.rs @@ -156,13 +156,15 @@ pub fn lines(sk: &Sketch, prev: Vec2, lp: Vec2, tol: f64) -> Option { pub fn commit(app: &mut SolveApp, line: &str, end: Vec2) { let Some(i) = current().filter(|i| i.at.dist(end) < 1e-9) else { return }; clear(); - let (cmd, params) = match i.kind { + let (cmd, mut params) = match i.kind { Kind::Parallel => ("sketch.constraint.parallel", json!({"a": i.curve, "b": line})), Kind::Perpendicular => ("sketch.constraint.perpendicular", json!({"a": i.curve, "b": line})), Kind::Tangent => ("sketch.constraint.tangent", json!({"a": i.curve, "b": line})), Kind::Extension => ("sketch.constraint.coincident", json!({"a": format!("{line}.end"), "b": i.curve})), }; - // An inference that would over-constrain the sketch is simply not kept. + // An inference only adds what the sketch does not already have: one that is already implied + // (redundant) or would conflict is simply not kept. + params["if_redundant"] = json!("refuse"); let _ = app.session.execute(cmd, ¶ms); } diff --git a/crates/ui-egui/src/sketch_clover_ui_tests.rs b/crates/ui-egui/src/sketch_clover_ui_tests.rs new file mode 100644 index 00000000..a393f804 --- /dev/null +++ b/crates/ui-egui/src/sketch_clover_ui_tests.rs @@ -0,0 +1,73 @@ +//! Rectangles of one size drawn by clicking around a shared corner (a "clover"): each one's +//! corner snaps onto the corners already there and its typed width and height go in. None is +//! refused as over-constraining the sketch (#687). + +use serde_json::json; + +use crate::scenario::Harness; + +fn type_value(h: &mut Harness, text: &str) { + h.step(&json!({"key": "A", "cmd": true})).unwrap(); + h.step(&json!({"text": text})).unwrap(); +} + +/// Draw a rectangle from `a` (clicked, snapping onto what is there) towards `b`, typing its +/// width and height when given, else clicking `b`. +fn rectangle(h: &mut Harness, a: [f64; 2], b: [f64; 2], typed: bool) { + h.step(&json!({"start": "sketch.rectangle.two_point"})).unwrap(); + h.step(&json!({"click": {"world": [a[0], a[1], 0]}})).unwrap(); + h.step(&json!({"move": {"world": [b[0], b[1], 0]}})).unwrap(); + if typed { + type_value(h, "10"); + h.step(&json!({"key": "Tab"})).unwrap(); + type_value(h, "10"); + h.step(&json!({"key": "Enter"})).unwrap(); + } else { + h.step(&json!({"click": {"world": [b[0], b[1], 0]}})).unwrap(); + } + h.step(&json!({"key": "Escape"})).unwrap(); + h.settle(); +} + +const CLOVER: [([f64; 2], [f64; 2]); 4] = [([0., 0.], [-10., -10.]), ([0., 0.], [10., -10.]), ([0., 0.], [10., 10.]), ([0., 0.], [-10., 10.])]; + +#[test] +fn a_clover_of_typed_10_by_10_rectangles_is_drawn_and_fully_constrained() { + let mut h = Harness::new(); + h.app.run("sketch.create", json!({"plane": "XY"})).unwrap(); + for (i, (a, b)) in CLOVER.iter().enumerate() { + rectangle(&mut h, *a, *b, true); + if let Some((message, _, true)) = &h.app.status { + panic!("rectangle {i} was refused: {message}"); + } + let state = h.app.session.world_state(); + let solved = state.sketch(h.app.session.active_sketch.unwrap()).unwrap(); + assert_eq!(solved.sketch.curves.iter().filter(|c| !c.construction).count(), 4 * (i + 1), "rectangle {i} was not added"); + assert!(solved.report.ok()); + } + let state = h.app.session.world_state(); + let solved = state.sketch(h.app.session.active_sketch.unwrap()).unwrap(); + assert_eq!(solved.profiles.len(), 4); + assert!(solved.profiles.iter().all(|p| (p.area - 100.0).abs() < 1e-6), "{:?}", solved.profiles.iter().map(|p| p.area).collect::>()); + assert!(solved.report.fully_constrained(), "the clover on the origin, every size typed: {:?}", solved.report); +} + +#[test] +fn a_clover_drawn_by_clicks_snaps_every_corner_without_redundant_constraints() { + let mut h = Harness::new(); + h.app.run("sketch.create", json!({"plane": "XY"})).unwrap(); + // Clicked, the last one goes from corner to corner of the ones already there, so both its + // clicks snap (a free click lands only as near as the pointer allows). + let mut clover = CLOVER; + clover[3] = ([0., 10.], [-10., 0.]); + for (a, b) in clover { + rectangle(&mut h, a, b, false); + } + assert!(h.app.status.as_ref().is_none_or(|(_, _, error)| !error), "{:?}", h.app.status); + let state = h.app.session.world_state(); + let solved = state.sketch(h.app.session.active_sketch.unwrap()).unwrap(); + assert_eq!(solved.profiles.len(), 4); + assert!(solved.report.ok() && solved.report.redundant.is_empty(), "{:?}", solved.report); + // On the origin: the two column widths and the two row heights are left. + assert_eq!(solved.report.dof, 4); +} diff --git a/crates/ui-egui/src/sketch_dims.rs b/crates/ui-egui/src/sketch_dims.rs index 7dee6349..24cf178a 100644 --- a/crates/ui-egui/src/sketch_dims.rs +++ b/crates/ui-egui/src/sketch_dims.rs @@ -377,3 +377,7 @@ mod tests; #[cfg(test)] #[path = "sketch_typed_intent_tests.rs"] mod typed_intent_tests; + +#[cfg(test)] +#[path = "sketch_clover_ui_tests.rs"] +mod clover_ui_tests; diff --git a/crates/ui-egui/src/sketch_tools.rs b/crates/ui-egui/src/sketch_tools.rs index 2c22921d..5a6c2555 100644 --- a/crates/ui-egui/src/sketch_tools.rs +++ b/crates/ui-egui/src/sketch_tools.rs @@ -1086,6 +1086,8 @@ fn glyphs(app: &SolveApp, painter: &egui::Painter, proj: &Proj, hover: Option