From be5c7103edb6c60beab24dabca375346b06007ba Mon Sep 17 00:00:00 2001 From: SofiaAFigueredo Date: Tue, 27 Oct 2026 10:45:50 -0300 Subject: [PATCH] Fix #4312: review fixes for the ruler edge guides An artboard guide now pulls a rectangle only where it overlaps the guide's extent (no reach added to the ends), and a Shift move slides along its line to a ruler guide only within twice the snap reach, so a line nearly parallel to the guide doesn't carry the selection far away. Co-Authored-By: Claude Sonnet 5.5 --- crates/tools/src/guides.rs | 6 +++--- crates/tools/src/guides/targets.rs | 12 +++++++++--- crates/tools/src/guides/tests.rs | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/crates/tools/src/guides.rs b/crates/tools/src/guides.rs index 112f732baa..ee38a86c0b 100644 --- a/crates/tools/src/guides.rs +++ b/crates/tools/src/guides.rs @@ -84,10 +84,10 @@ impl Ruler { /// The line it pulls a rectangle into line with, whose extent along the guide is `lo..=hi`: /// as [`Self::line_at`], but it starts at the middle of that extent, so the guide marked /// there spans the rectangle rather than the canvas. None where the guide doesn't run along - /// the rectangle. - fn line_over(&self, (lo, hi): (f64, f64), tol: f64) -> Option<(f64, Point, Kind)> { + /// the rectangle: one that spans an artboard needs the rectangle to overlap it, however near. + fn line_over(&self, (lo, hi): (f64, f64)) -> Option<(f64, Point, Kind)> { if let Some((a, b)) = self.span - && (hi < a - tol || lo > b + tol) + && (hi < a || lo > b) { return None; } diff --git a/crates/tools/src/guides/targets.rs b/crates/tools/src/guides/targets.rs index 54f2bd21da..1f3c958279 100644 --- a/crates/tools/src/guides/targets.rs +++ b/crates/tools/src/guides/targets.rs @@ -795,11 +795,17 @@ impl Targets { let lines = if x { &self.xs } else { &self.ys }; for r in rects { let vs = if x { [r.x0, r.center().x, r.x1] } else { [r.y0, r.center().y, r.y1] }; - let rulers = self.edge_rulers.iter().filter(|g| g.vertical == x).filter_map(|g| g.line_over(span(r, !x), tol)); + let rulers = self.edge_rulers.iter().filter(|g| g.vertical == x).filter_map(|g| g.line_over(span(r, !x))); let rulers: Vec<_> = rulers.collect(); for v in vs { - for (t, from, _) in lines_near(lines, v, tol).iter().chain(&rulers).filter(|(t, ..)| (t - v).abs() <= tol) { + let art = lines_near(lines, v, tol).iter().map(|l| (l, false)); + for ((t, from, _), ruler) in art.chain(rulers.iter().map(|l| (l, true))).filter(|((t, ..), _)| (t - v).abs() <= tol) { let s = (t - v) / c; + // A ruler guide runs the whole canvas, so a line nearly parallel to it would slide + // the selection far to meet it: it pulls within the reach of a point's slide only. + if ruler && s.abs() > 2.0 * tol { + continue; + } if best.is_none_or(|b| s.abs() < b.0.abs()) { best = Some((s, *t, *from, x)); } @@ -936,7 +942,7 @@ impl Targets { .flat_map(|r| along(r).into_iter().map(move |v| (v, *r))) .flat_map(|(v, r)| { // The ruler guides across this axis that run along the rectangle. - let rulers = self.edge_rulers.iter().filter(move |g| g.vertical == x).filter_map(move |g| g.line_over(span(&r, !x), tol)); + let rulers = self.edge_rulers.iter().filter(move |g| g.vertical == x).filter_map(move |g| g.line_over(span(&r, !x))); lines_near(targets, v, tol).iter().copied().chain(rulers).map(move |(t, from, _)| (t - v, from, v, r)) }) .filter(|(d, ..)| d.abs() <= tol) diff --git a/crates/tools/src/guides/tests.rs b/crates/tools/src/guides/tests.rs index 2d6d4c1771..ad7f6f36c1 100644 --- a/crates/tools/src/guides/tests.rs +++ b/crates/tools/src/guides/tests.rs @@ -36,11 +36,28 @@ fn rect_snap_aligns_edges_with_ruler_guides_where_they_run() { // The guide of an artboard above it doesn't run along it; the horizontal one still pulls its bottom. let (dv, _) = t.snap_rect(Rect::new(248.0, 60.0, 298.0, 88.0), 4.0); assert_eq!(dv, Vec2::new(0.0, 2.0)); + // The guide of an artboard needs the rectangle to overlap it, not only be within reach of its end. + let (dv, _) = t.snap_rect(Rect::new(299.0, 51.0, 349.0, 80.0), 4.0); + assert_eq!(dv.x, 0.0, "{dv:?}"); // Without them as edge guides, nothing pulls: `rulers` is for points. let points_only = Targets { rulers: vec![Ruler { vertical: true, pos: 300.0, span: None }], ..Targets::default() }; assert_eq!(points_only.snap_rect(Rect::new(248.0, 10.0, 298.0, 40.0), 4.0).0, Vec2::ZERO); } +/// #4312: a Shift move along a line almost parallel to a ruler guide doesn't slide far to meet it. +#[test] +fn a_slide_along_a_nearly_parallel_line_ignores_a_ruler_guide_beyond_its_reach() { + let t = Targets { edge_rulers: vec![Ruler { vertical: true, pos: 300.0, span: None }], ..Targets::default() }; + let r = Rect::new(248.0, 10.0, 298.0, 40.0); + // Level: the right edge is 2 short of the guide, which is the way along the line. + assert_eq!(t.snap_rects_along(&[r], Vec2::new(1.0, 0.0), 4.0).0, Vec2::new(2.0, 0.0)); + // On the diagonal it is 2.8 along. + let (d, _) = t.snap_rects_along(&[r], Vec2::new(1.0, 1.0), 4.0); + assert!((d - Vec2::new(2.0, 2.0)).hypot() < 1e-9, "{d:?}"); + // Nearly upright, it would take about 118 along the line to meet the guide. + assert_eq!(t.snap_rects_along(&[r], Vec2::new(0.017, 1.0), 4.0).0, Vec2::ZERO); +} + #[test] fn rect_snap_aligns_edges() { let (d, id) = doc_with_rect();