From b978eeddcddd02225be4c4090d7247e65edcd694 Mon Sep 17 00:00:00 2001 From: Colin Faulkingham Date: Mon, 2 Nov 2026 13:45:36 +0000 Subject: [PATCH] Sketch: preserve short reference-line directions when dimensioning (#11001) (#11117) * claim: #11001 * fix(sketch): keep short infinite references aligned --- .../engine/src/cmd/sketch_infinite_tests.rs | 38 +++++++++++++++++ crates/sketch/src/degenerate_line_tests.rs | 38 +++++++++++++++++ crates/sketch/src/solver.rs | 41 ++++++++++++++++--- 3 files changed, 112 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/sketch_infinite_tests.rs b/crates/engine/src/cmd/sketch_infinite_tests.rs index 2ca806a34b..00c5dbf958 100644 --- a/crates/engine/src/cmd/sketch_infinite_tests.rs +++ b/crates/engine/src/cmd/sketch_infinite_tests.rs @@ -29,6 +29,44 @@ fn new_sketch() -> Session { s } +#[test] +fn a_short_horizontal_reference_holds_through_dimension_edits_drag_and_reopen() { + let mut s = new_sketch(); + let id = s.active_sketch.unwrap(); + // A saved infinite line may have very close defining points (#11001). + let sk = s.doc_mut().sketch_mut(id).unwrap(); + let end = sk.add_point(Vec2::new(6.36e-8, 0.0), Some("axis.end")).unwrap(); + let arm_end = sk.add_point(Vec2::new(12.0, 10.0), Some("arm.end")).unwrap(); + let axis = sk.add_line_pts(0, end, Some("axis")).unwrap(); + sk.curves[axis].infinite = true; + sk.curves[axis].construction = true; + sk.add_line_pts(0, arm_end, Some("arm")).unwrap(); + sk.add_constraint(solidcraft_sketch::ConstraintKind::Horizontal { l: axis }, None).unwrap(); + s.refresh(); + + let check = |s: &Session, degrees: f64| { + let state = s.model.state(); + let solved = state.sketch(id).unwrap(); + assert!(solved.report.ok(), "{:?}", solved.report); + let axis = solved.sketch.points[end].pos.normalized().unwrap(); + let arm = solved.sketch.points[arm_end].pos.normalized().unwrap(); + assert!(axis.dist(Vec2::X) < 1e-6, "the horizontal reference rotated: {axis:?}"); + assert!((arm.y.atan2(arm.x) - degrees.to_radians()).abs() < 1e-6, "the free line has the wrong angle: {arm:?}"); + }; + let out = run(&mut s, "sketch.dimension", json!({"entities": ["axis", "arm"], "type": "angle", "value": "30 deg"})); + check(&s, 30.0); + run(&mut s, "parameters.change", json!({"name": out["param"], "expression": "15 deg"})); + check(&s, 15.0); + let at = sketch(&s).points[arm_end].pos; + run(&mut s, "sketch.drag", json!({"entity": "arm.end", "from": [at.x, at.y], "to": [14, 10]})); + run(&mut s, "sketch.drag_end", json!({})); + check(&s, 15.0); + let doc = solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap(); + *s.doc_mut() = doc; + s.refresh(); + check(&s, 15.0); +} + fn profiles(s: &Session) -> usize { s.model.state().sketch(s.active_sketch.unwrap()).unwrap().profiles.len() } diff --git a/crates/sketch/src/degenerate_line_tests.rs b/crates/sketch/src/degenerate_line_tests.rs index 4ec991510e..065028290e 100644 --- a/crates/sketch/src/degenerate_line_tests.rs +++ b/crates/sketch/src/degenerate_line_tests.rs @@ -60,3 +60,41 @@ fn dragging_the_midpoint_line_end_keeps_the_sketch_solved() { let off = (at.y.atan2(at.x) - 50f64.to_radians()).abs(); assert!(off < 1e-3, "l1.end went to {at:?} (off its line by {off} rad)"); } + +/// An infinite reference can have very close defining points in a saved sketch. Its +/// horizontal/vertical constraint still fixes its visible direction, not just the tiny +/// distance between the defining points (#11001). +#[test] +fn an_angle_keeps_a_short_infinite_reference_horizontal_or_vertical() { + for (vertical, sign, origin) in [ + (false, 1.0, v(0.0, 0.0)), + (true, 1.0, v(0.0, 0.0)), + (false, -1.0, v(0.0, 0.0)), + (true, -1.0, v(0.0, 0.0)), + (false, 1.0, v(25.0, -17.0)), + (true, 1.0, v(25.0, -17.0)), + ] { + for length in [6.36e-8, 1.0] { + let mut sk = Sketch::new(); + sk.points[0].pos = origin; + let direction = (if vertical { v(0.0, 1.0) } else { v(1.0, 0.0) }) * sign; + let end = sk.add_point(origin + direction * length, None).unwrap(); + let arm_end = sk.add_point(origin + (if vertical { v(-10.0, 12.0) } else { v(12.0, 10.0) }) * sign, None).unwrap(); + let axis = sk.add_line_pts(0, end, Some("axis")).unwrap(); + sk.curves[axis].infinite = true; + sk.curves[axis].construction = true; + let arm = sk.add_line_pts(0, arm_end, Some("arm")).unwrap(); + let constraint = if vertical { ConstraintKind::Vertical { l: axis } } else { ConstraintKind::Horizontal { l: axis } }; + sk.add_constraint(constraint, None).unwrap(); + sk.add_constraint(ConstraintKind::Angle { a: axis, b: arm, value: 30f64.to_radians(), flip: false }, None).unwrap(); + + let report = crate::solve(&mut sk); + assert!(report.ok(), "vertical={vertical}, length={length}: {report:?}"); + let actual = (sk.points[end].pos - origin).normalized().expect("the reference retains a direction"); + assert!(actual.dist(direction) < 1e-6, "vertical={vertical}, length={length}: reference rotated to {actual:?}"); + let arm_direction = (sk.points[arm_end].pos - origin).normalized().unwrap(); + let angle = actual.cross(arm_direction).atan2(actual.dot(arm_direction)); + assert!((angle - 30f64.to_radians()).abs() < 1e-6, "the free line must satisfy the angle: {angle}"); + } + } +} diff --git a/crates/sketch/src/solver.rs b/crates/sketch/src/solver.rs index 87213765d4..109960b112 100644 --- a/crates/sketch/src/solver.rs +++ b/crates/sketch/src/solver.rs @@ -502,11 +502,16 @@ fn residuals(s: &State, k: &ConstraintKind, out: &mut Vec) { } Horizontal { l } => { let (a, b) = s.line(l); - out.push(b.y - a.y); + let scale = if s.sk.curves.get(l).is_some_and(|c| c.infinite) { (b - a).len().max(1e-12) } else { 1.0 }; + // An infinite line's defining points may be very close together. Its visible + // direction must remain horizontal even when their separation is below the + // distance tolerance, so measure direction rather than endpoint distance. + out.push((b.y - a.y) / scale); } Vertical { l } => { let (a, b) = s.line(l); - out.push(b.x - a.x); + let scale = if s.sk.curves.get(l).is_some_and(|c| c.infinite) { (b - a).len().max(1e-12) } else { 1.0 }; + out.push((b.x - a.x) / scale); } HorizontalPoints { p, q } => out.push(s.p(q).y - s.p(p).y), VerticalPoints { p, q } => out.push(s.p(q).x - s.p(p).x), @@ -1048,7 +1053,7 @@ fn system(sk: &Sketch, lay: &Layout, blocks: &[(Block, Vec)], x: &mut [f6 for &v in vars { let Some(x0) = x.get(v).copied() else { continue }; let opening = distance_opening_slope(&State { sk, lay, x }, b, v); - let h = 1e-7 * x0.abs().max(1.0); + let h = direction_difference_step(&State { sk, lay, x }, b, v, x0); if let Some(xv) = x.get_mut(v) { *xv = x0 + h; } @@ -1100,7 +1105,7 @@ fn local_system( let Some(c) = col.get(v).copied().filter(|c| *c < ncols) else { continue }; let Some(x0) = x.get(v).copied() else { continue }; let opening = distance_opening_slope(&State { sk, lay, x }, b, v); - let h = 1e-7 * x0.abs().max(1.0); + let h = direction_difference_step(&State { sk, lay, x }, b, v, x0); if let Some(xv) = x.get_mut(v) { *xv = x0 + h; } @@ -1124,6 +1129,32 @@ fn local_system( (r, j, owner) } +/// An infinite line's direction derivative must be sampled within its defining span. The +/// usual coordinate-sized step can cross its other end and measure a half-turn instead +/// of the local slope when the saved defining points are very close (#11001). +fn direction_difference_step(s: &State<'_>, block: &Block<'_>, variable: usize, value: f64) -> f64 { + let mut step = 1e-7 * value.abs().max(1.0); + let Block::User(_, kind) = block else { return step }; + let lines = match **kind { + ConstraintKind::Angle { a, b, .. } => [Some(a), Some(b)], + ConstraintKind::LineAngle { l, .. } => [Some(l), None], + ConstraintKind::Horizontal { l } | ConstraintKind::Vertical { l } if s.sk.curves.get(l).is_some_and(|c| c.infinite) => [Some(l), None], + _ => return step, + }; + for l in lines.into_iter().flatten() { + let Some(CurveKind::Line { a, b }) = s.sk.curves.get(l).filter(|c| c.infinite).map(|c| &c.kind) else { continue }; + let depends = [a, b].into_iter().any(|&p| s.lay.pvar.get(p).copied().flatten().is_some_and(|v| variable == v || variable == v + 1)); + if depends { + let length = (s.p(*b) - s.p(*a)).len(); + if length > 0.0 { + step = step.min(length * 1e-3); + } + } + } + // Keep both samples representable when the sketch is far from its origin. + step.max(value.abs() * f64::EPSILON * 8.0) +} + fn max_abs(v: &[f64]) -> f64 { v.iter().fold(0.0_f64, |m, x| if x.is_finite() { m.max(x.abs()) } else { f64::INFINITY }) } @@ -1565,7 +1596,7 @@ fn accumulate(sk: &Sketch, lay: &Layout, b: &Block, unit: &[usize], x: &mut [f64 let (mut plus, mut minus) = (Vec::new(), Vec::new()); for (&v, dk) in unit.iter().zip(&mut d) { let Some(x0) = x.get(v).copied() else { continue }; - let h = 1e-7 * x0.abs().max(1.0); + let h = direction_difference_step(&State { sk, lay, x }, b, v, x0); for (at, out) in [(x0 + h, &mut plus), (x0 - h, &mut minus)] { if let Some(xv) = x.get_mut(v) { *xv = at;