fix(snap): keep an arc's sweep for TAN and PER snaps (#1698)

* fix(snap): keep an arc's sweep for TAN, PER and deferred snaps

An arc was snapped as its full circle: TAN and PER solutions on the circle
that fall outside the arc's sweep were offered as if they touched the arc,
and a deferred tangent lost the sweep when it was stored.

- SnapCurve gains an Arc variant carrying start and end, so a deferred snap
  keeps the sweep and resolves onto the arc.
- TAN and PER filter their solutions with Arc::contains_angle; PER clamps to
  the arc with Arc::closest instead of the circle's closest point.
- Tangents and perpendiculars between two deferred snaps are filtered on
  both ends.
- The viewport projection maps the new variant.
- Tests: a tangent in the gap is not offered, the perpendicular clamps to an
  end, and a deferred tangent keeps its sweep and solves onto the arc.

Fixes #1697

* fix(snap): correct direct PER, support Arc in deferred pairs

Addresses the review on #1698:
- Direct PER on an arc used `arc.closest(bp)`, which clamps a gap solution to an
  end and misses the valid opposite antipode; both antipodes are now offered,
  filtered to the arc's sweep.
- `deferred_pairs` matched only circles and lines, so any deferred pair with an
  arc operand fell through to None. An arc is now matched as its carrier circle
  for the math, with the sweep filter still applied to the solutions.
- Tests: the direct PER case now expects the antipode on the arc, and a
  TAN-TAN pair between an arc and a circle only keeps the tangent on the arc.

* test(snap): use a margin over the boundary in the arc PER test

Addresses the review on #1698: the cursor is 5 units from the antipode, so the
aperture is 5.5 rather than 5.0 to keep the distance comparison off the
boundary; the comment now describes the antipode rather than the nearest end.

* cargo fmt

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
william-wwh
2026-10-24 01:48:57 -04:00
committed by GitHub
co-authored by Claude Opus 5.5 Brandon Thomas
parent 76f7cea83f
commit 6ee6e652fd
2 changed files with 174 additions and 18 deletions
+167 -18
View File
@@ -78,12 +78,53 @@ impl SnapHit {
#[derive(Clone, Copy, Debug, PartialEq, Serialize)]
#[serde(tag = "kind", rename_all = "camelCase")]
pub enum SnapCurve {
/// A circle, or the circle an arc lies on.
/// A circle, or the circle an arc lies on; an arc's sweep is carried by [`SnapCurve::Arc`].
Circle { center: Vec2, radius: f64 },
/// An arc: the points of `Circle { center, radius }` swept CCW from `start` to `end` (radians).
Arc { center: Vec2, radius: f64, start: f64, end: f64 },
/// The infinite line through `a` and `b` (a line, xline or ray).
Line { a: Vec2, b: Vec2 },
}
impl SnapCurve {
/// Whether `p` lies on the drawn curve: always on a circle or a line, and only within the
/// sweep on an arc. Tangent and perpendicular solutions on an arc's circle that fall off the
/// arc are filtered out with this, so they never snap into the gap.
pub fn contains(&self, p: Vec2) -> bool {
match self {
// A zero sweep is how a full circle is stored (see `fix_circle`): the whole circle.
SnapCurve::Arc { center, radius, start, end } => {
(*start - *end).abs() < 1e-12 || cadcraft_geom::Arc::new(*center, *radius, *start, *end).contains_angle(center.angle_to(p))
}
_ => true,
}
}
/// Points on the curve whose tangent passes through `p` (a circle's or an arc's).
fn tangents(&self, p: Vec2) -> Vec<Vec2> {
match self {
SnapCurve::Circle { center, radius } | SnapCurve::Arc { center, radius, .. } => {
Circle::new(*center, *radius).tangent_points(p).into_iter().filter(|t| self.contains(*t)).collect()
}
SnapCurve::Line { .. } => Vec::new(),
}
}
/// Points on the curve whose perpendicular passes through `p`; `picked` is where the curve
/// was picked, kept when `p` is the center of a circle or arc (every radius is perpendicular).
fn perpendiculars(&self, p: Vec2, picked: Vec2) -> Vec<Vec2> {
match self {
SnapCurve::Circle { center, radius } | SnapCurve::Arc { center, radius, .. } => {
let u = (p - *center).normalized();
if u == Vec2::ZERO {
vec![Circle::new(*center, *radius).closest(picked)]
} else {
[*center + u * *radius, *center - u * *radius].into_iter().filter(|q| self.contains(*q)).collect()
}
}
SnapCurve::Line { a, b } => line_dir(*a, *b).map(|_| vec![cadcraft_geom::Line::new(*a, *b).project(p)]).unwrap_or_default(),
}
}
}
/// A deferred tangent ([`mode::TAN`]) or perpendicular ([`mode::PER`]) snap, picked while there
/// is no base point yet (the first point of a line). The real point depends on the other end of
/// the line, so it is resolved later with [`resolve_deferred`].
@@ -107,6 +148,18 @@ pub fn tangent_point_in_direction(curve: &SnapCurve, dir: f64, hint: Vec2) -> Op
let c2 = *center - Vec2::new(n.cos(), n.sin()) * *radius;
Some(if c1.dist(hint) < c2.dist(hint) { c1 } else { c2 })
}
SnapCurve::Arc { center, radius, start, end } => {
let n = dir + std::f64::consts::FRAC_PI_2;
let c1 = *center + Vec2::new(n.cos(), n.sin()) * *radius;
let c2 = *center - Vec2::new(n.cos(), n.sin()) * *radius;
let on = cadcraft_geom::Arc::new(*center, *radius, *start, *end);
let ok = |c: Vec2| on.contains_angle(center.angle_to(c));
let (c1, c2) = (ok(c1).then_some(c1), ok(c2).then_some(c2));
match (c1, c2) {
(Some(a), Some(b)) => Some(if a.dist(hint) < b.dist(hint) { a } else { b }),
(a, b) => a.or(b),
}
}
_ => None,
}
}
@@ -423,7 +476,12 @@ fn finish(
if osmode & mode::PER != 0 {
match p {
Prim::Seg(Segment::Line(l)) => cands.push((mode::PER, l.project(bp))),
Prim::Seg(Segment::Arc { arc, .. }) => cands.push((mode::PER, Circle::new(arc.center, arc.radius).closest(bp))),
// Both antipodes, filtered to the arc's sweep: a perpendicular in the gap is
// not offered, and the far side is still found.
Prim::Seg(Segment::Arc { arc, .. }) => {
let curve = SnapCurve::Arc { center: arc.center, radius: arc.radius, start: arc.start, end: arc.end };
cands.extend(curve.perpendiculars(bp, bp).into_iter().map(|q| (mode::PER, q)));
}
Prim::Circle(c) => cands.push((mode::PER, c.closest(bp))),
Prim::Infinite { base: b0, dir, .. } => cands.push((mode::PER, *b0 + *dir * (bp - *b0).dot(*dir))),
Prim::Ellipse(e) => cands.extend(curves::ellipse_perpendiculars(e, bp).into_iter().map(|q| (mode::PER, q))),
@@ -433,7 +491,12 @@ fn finish(
}
if osmode & mode::TAN != 0 {
let pts = match p {
Prim::Seg(Segment::Arc { arc, .. }) => Circle::new(arc.center, arc.radius).tangent_points(bp),
// Only the tangent points that lie on the arc itself (not its circle).
Prim::Seg(Segment::Arc { arc, .. }) => Circle::new(arc.center, arc.radius)
.tangent_points(bp)
.into_iter()
.filter(|t| arc.contains_angle(arc.center.angle_to(*t)))
.collect(),
Prim::Circle(c) => c.tangent_points(bp),
Prim::Ellipse(e) => curves::ellipse_tangents(e, bp),
Prim::Spline(s) => curves::spline_tangents(s, bp),
@@ -808,7 +871,7 @@ fn deferred_hit(prims: &[Prim], cursor: Vec2, aperture: f64, osmode: u32, clip:
for p in prims {
let curve = match p {
Prim::Circle(c) => SnapCurve::Circle { center: c.center, radius: c.radius },
Prim::Seg(Segment::Arc { arc, .. }) => SnapCurve::Circle { center: arc.center, radius: arc.radius },
Prim::Seg(Segment::Arc { arc, .. }) => SnapCurve::Arc { center: arc.center, radius: arc.radius, start: arc.start, end: arc.end },
Prim::Seg(Segment::Line(l)) => SnapCurve::Line { a: l.a, b: l.b },
Prim::Infinite { base, dir, .. } => SnapCurve::Line { a: *base, b: *base + *dir },
_ => continue,
@@ -820,6 +883,12 @@ fn deferred_hit(prims: &[Prim], cursor: Vec2, aperture: f64, osmode: u32, clip:
SnapCurve::Circle { center, radius } if osmode & mode::PER != 0 => {
(mode::PER, "Deferred Perpendicular", Circle::new(center, radius).closest(cursor))
}
SnapCurve::Arc { center, radius, start, end } if osmode & mode::TAN != 0 => {
(mode::TAN, "Deferred Tangent", cadcraft_geom::Arc::new(center, radius, start, end).closest(cursor))
}
SnapCurve::Arc { center, radius, start, end } if osmode & mode::PER != 0 => {
(mode::PER, "Deferred Perpendicular", cadcraft_geom::Arc::new(center, radius, start, end).closest(cursor))
}
SnapCurve::Line { a, b } if osmode & mode::PER != 0 => {
// Pick on the drawn extent: a segment's own span, an xline/ray anywhere.
let l = cadcraft_geom::Line::new(a, b);
@@ -848,18 +917,9 @@ pub fn resolve_deferred(a: LineEnd, b: LineEnd) -> Option<(Vec2, Vec2)> {
if !p.is_finite() {
return None;
}
let cands: Vec<Vec2> = match (d.mode, d.curve) {
(mode::TAN, SnapCurve::Circle { center, radius }) => Circle::new(center, radius).tangent_points(p),
(mode::PER, SnapCurve::Circle { center, radius }) => {
let u = (p - center).normalized();
if u == Vec2::ZERO {
// Every radius is perpendicular: keep the picked point.
vec![Circle::new(center, radius).closest(d.at)]
} else {
vec![center + u * radius, center - u * radius]
}
}
(mode::PER, SnapCurve::Line { a, b }) => line_dir(a, b).map(|_| vec![cadcraft_geom::Line::new(a, b).project(p)]).unwrap_or_default(),
let cands: Vec<Vec2> = match d.mode {
mode::TAN => d.curve.tangents(p),
mode::PER => d.curve.perpendiculars(p, d.at),
_ => Vec::new(),
};
pick_best(cands.into_iter().map(|t| (t, p)), d.at, None)
@@ -875,7 +935,12 @@ pub fn resolve_deferred(a: LineEnd, b: LineEnd) -> Option<(Vec2, Vec2)> {
/// caller retries swapped).
fn deferred_pairs(d1: &Deferred, d2: &Deferred) -> Option<Vec<(Vec2, Vec2)>> {
use SnapCurve::{Circle as C, Line as L};
Some(match ((d1.mode, d1.curve), (d2.mode, d2.curve)) {
// The math below is on the circle an arc lies on; the sweep is checked afterwards.
let circle_of = |c: SnapCurve| match c {
SnapCurve::Circle { center, radius } | SnapCurve::Arc { center, radius, .. } => C { center, radius },
other => other,
};
let pairs = match ((d1.mode, circle_of(d1.curve)), (d2.mode, circle_of(d2.curve))) {
((mode::TAN, C { center: c1, radius: r1 }), (mode::TAN, C { center: c2, radius: r2 })) => {
Circle::new(c1, r1).common_tangents(&Circle::new(c2, r2))
}
@@ -923,7 +988,10 @@ fn deferred_pairs(d1: &Deferred, d2: &Deferred) -> Option<Vec<(Vec2, Vec2)>> {
vec![(p, cadcraft_geom::Line::new(a2, b2).project(p))]
}
_ => return None,
})
};
// A solution on an arc's circle that falls off the arc is no solution: the line would touch
// the circle, not the arc that was picked.
Some(pairs.into_iter().filter(|(p, q)| d1.curve.contains(*p) && d2.curve.contains(*q)).collect())
}
/// Unit direction of the line through `a` and `b`, if it has one.
@@ -1185,4 +1253,85 @@ mod tests {
let curve = SnapCurve::Line { a: Vec2::ZERO, b: Vec2::X };
assert!(resolve_deferred(LineEnd::Deferred(Deferred { mode: mode::TAN, curve, at: Vec2::ZERO }), pt(3.0, 3.0)).is_none());
}
/// A drawing with one arc: center (20,0), radius 2, swept CCW from 0° to 90° (the first
/// quadrant only, so the rest of its circle is the gap).
fn arc_drawing() -> Drawing {
let mut d = Drawing::new_imperial();
d.add(
&Space::Model,
Common::default(),
EntityKind::Arc(cadcraft_doc::Arc { center: Vec3::new(20.0, 0.0, 0.0), radius: 2.0, start: 0.0, end: std::f64::consts::FRAC_PI_2 }),
)
.unwrap();
d
}
fn on_arc(p: Vec2) -> bool {
let a = Vec2::new(20.0, 0.0).angle_to(p);
cadcraft_geom::angle_in_sweep(a, 0.0, std::f64::consts::FRAC_PI_2) && (p.dist(Vec2::new(20.0, 0.0)) - 2.0).abs() < 1e-9
}
#[test]
fn arc_tangent_only_on_the_arc_not_its_circle() {
let d = arc_drawing();
// From (20,5), the circle's tangents touch at about ±23.6° from the vertical through the
// center: one lands at 90+23.6 = 113.6° (the gap), the other at 90-23.6 = 66.4° (on the
// arc). Only the one on the arc may snap.
let base = Some(Vec2::new(20.0, 5.0));
let t = osnap(&d, &Space::Model, Vec2::new(20.0 + 2.0, 0.0), 8.0, mode::TAN, base, false).expect("the arc's tangent");
assert!(on_arc(t.point), "tangent must lie on the arc, got {:?}", t.point);
// The other solution is on the circle but off the arc, so it is not offered.
let off = Vec2::polar(Vec2::new(20.0, 0.0), 2.0, 113.6f64.to_radians());
assert!(!on_arc(off));
let none = osnap(&d, &Space::Model, off, 0.05, mode::TAN, base, false);
assert!(none.is_none(), "a tangent in the arc's gap must not snap, got {none:?}");
}
#[test]
fn arc_perpendicular_clamps_to_the_arc() {
let d = arc_drawing();
// The foot of the perpendicular from below lands at 270°, in the gap. The opposite
// antipode at (20,2) is on the arc. The cursor is 5 units away from it, so the aperture
// is a little larger to keep the distance test off the boundary.
let h = osnap(&d, &Space::Model, Vec2::new(20.0, -3.0), 5.5, mode::PER, Some(Vec2::new(20.0, -6.0)), false)
.expect("the perpendicular on the arc");
assert!(on_arc(h.point), "perpendicular must stay on the arc, got {:?}", h.point);
assert!(h.point.near(Vec2::new(20.0, 2.0), 1e-9), "the opposite antipode on the arc, got {:?}", h.point);
}
#[test]
fn arc_deferred_tangent_keeps_its_sweep() {
let d = arc_drawing();
let at = Vec2::polar(Vec2::new(20.0, 0.0), 2.0, 45f64.to_radians());
let h = osnap(&d, &Space::Model, at, 0.5, mode::TAN, None, true).expect("deferred tangent on the arc");
assert_eq!((h.mode, h.name), (mode::TAN, "Deferred Tangent"));
let df = h.deferred.unwrap();
// The sweep travels with the curve, so a later solve stays on the arc.
assert!(matches!(df.curve, SnapCurve::Arc { .. }), "the arc's sweep must be kept: {:?}", df.curve);
// A deferred tangent solves to a point on the arc, not into its gap.
let solved = resolve_deferred(LineEnd::Deferred(df), pt(10.0, 10.0)).expect("a tangent from (10,10)");
// "In the order given": the deferred end solves first.
assert!(on_arc(solved.0), "resolved tangent must lie on the arc, got {:?}", solved.0);
}
#[test]
fn deferred_tangent_pairs_with_arcs() {
// The arc is the top half of a circle at (0,0) r=5.
let arc = SnapCurve::Arc { center: Vec2::new(0.0, 0.0), radius: 5.0, start: 0.0, end: std::f64::consts::PI };
// The circle is at (10,0) r=5.
let circle = SnapCurve::Circle { center: Vec2::new(10.0, 0.0), radius: 5.0 };
let d_arc = Deferred { mode: mode::TAN, curve: arc, at: Vec2::new(0.0, 5.0) };
let d_circle = Deferred { mode: mode::TAN, curve: circle, at: Vec2::new(10.0, 5.0) };
// Common tangents of the carrier circles: two outer (y=5 and y=-5) and two inner.
// The y=-5 tangent touches the arc's circle at (0,-5), which is in the gap.
// The y=5 tangent touches at (0,5), on the arc.
// The inner tangents touch the left circle at angles that are also not on the top half.
// Only the y=5 solution survives the arc's sweep.
let solved = resolve_deferred(LineEnd::Deferred(d_arc), LineEnd::Deferred(d_circle)).unwrap();
assert!(solved.0.near(Vec2::new(0.0, 5.0), 1e-9), "expected the belt tangent on the top arc, got {:?}", solved.0);
assert!(solved.1.near(Vec2::new(10.0, 5.0), 1e-9));
}
}
+7
View File
@@ -71,6 +71,13 @@ impl<'a> Projection<'a> {
}
SnapCurve::Circle { center, radius }
}
SnapCurve::Arc { center, radius, start, end } => {
let (center, radius) = (self.to_paper(center), radius * self.scale);
if !(center.is_finite() && radius.is_finite() && radius > 0.0) {
return None;
}
SnapCurve::Arc { center, radius, start, end }
}
SnapCurve::Line { a, b } => {
let (a, b) = (self.to_paper(a), self.to_paper(b));
if !(a.is_finite() && b.is_finite()) {