From c684db5428f1f521824ee5f3533eef14f5af7d0f Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 16:17:42 -0500 Subject: [PATCH] Tools: selection bounds in one walk, bounded smart spacing and parent snapping (#2260) * tools: selection bounds in one walk, caps on smart-spacing boxes and parent occurrences * Apply review fixes - crates/tools/src/lib.rs * address review: iterative selection walk; stop collecting spacing boxes at the cap --------- Co-authored-by: autocraft-bot[bot] <354215646+autocraft-bot[bot]@users.noreply.github.com> --- crates/tools/src/lib.rs | 50 ++++++++++++++++++++++++++++++++++++---- crates/tools/src/snap.rs | 26 +++++++++++++++++---- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index 73afc4d5d..d4a11b582 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -314,11 +314,54 @@ impl ToolContext<'_> { self.doc.item(id) } + /// The selected items with the transform from their own space to the canvas, found in one walk + /// of the document (a search per item made a big selection quadratic). Same first match per + /// id as `Document::find`, in selection order. + fn selected_with_canvas_xf(&self) -> Vec<(&Item, Affine)> { + let mut want: std::collections::HashSet = self.selection.items.iter().copied().collect(); + let mut found: std::collections::HashMap = std::collections::HashMap::new(); + // Pre-order with an explicit stack: a deep group tree must not exhaust the call stack. + fn walk<'a>( + items: &'a [std::sync::Arc], + want: &mut std::collections::HashSet, + found: &mut std::collections::HashMap, + ) { + let mut stack = vec![(items.iter(), Affine::IDENTITY)]; + while let Some((kids, xf)) = stack.last_mut() { + if want.is_empty() { + return; + } + let Some(it) = kids.next() else { + stack.pop(); + continue; + }; + let xf = *xf; + if want.remove(&it.id) { + found.insert(it.id, (it, xf)); + } + stack.push((it.children().iter(), xf * it.xf)); + } + } + for r in self.doc.spread_refs() { + if want.is_empty() { + break; + } + if let Some(sp) = self.doc.spread(r) { + let mut here = std::collections::HashMap::new(); + walk(&sp.items, &mut want, &mut here); + if !here.is_empty() { + let canvas = self.layout.xf(r); + found.extend(here.into_iter().map(|(id, (it, xf))| (id, (it, canvas * xf)))); + } + } + } + self.selection.items.iter().filter_map(|id| found.get(id).copied()).collect() + } + /// Canvas bounds of the selected items' paths. Handles and resize use this geometric box. pub fn selection_bounds(&self) -> Option { let mut r: Option = None; - for id in &self.selection.items { - let (Some(it), Some(xf)) = (self.doc.item(*id), self.item_canvas_xf(*id)) else { continue }; + for (it, xf) in self.selected_with_canvas_xf() { let b = xf.transform_rect_bbox(it.bounds()); r = Some(r.map_or(b, |r| r.union(b))); } @@ -328,8 +371,7 @@ impl ToolContext<'_> { /// Canvas union of the selection's visible bounds. A group uses the alignment box. pub fn selection_visible_bounds(&self) -> Option { let mut acc: Option = None; - for id in &self.selection.items { - let (Some(it), Some(xf)) = (self.doc.item(*id), self.item_canvas_xf(*id)) else { continue }; + for (it, xf) in self.selected_with_canvas_xf() { let Some(local) = crate::snap::moving_bounds(it) else { continue }; let b = xf.transform_rect_bbox(local); if !(b.x0.is_finite() && b.y0.is_finite() && b.x1.is_finite() && b.y1.is_finite()) { diff --git a/crates/tools/src/snap.rs b/crates/tools/src/snap.rs index c76a0bd82..97b94a64b 100644 --- a/crates/tools/src/snap.rs +++ b/crates/tools/src/snap.rs @@ -525,7 +525,8 @@ fn pass_spacing(cx: &ToolContext, req: &SnapRequest<'_>, axis: Axis, tol: f64) - return None; } let mover = interval_on(req.rect, axis)?; - let boxes = spacing_intervals(cx, req, axis, &mover); + // Every box is compared with every other (quadratic on each pointer move): none past the cap. + let boxes = spacing_intervals(cx, req, axis, &mover)?; let stationary = stationary_gaps(&boxes); let low = nearest_side(&mover, &boxes, None, true); let high = nearest_side(&mover, &boxes, None, false); @@ -561,20 +562,27 @@ fn pass_spacing(cx: &ToolContext, req: &SnapRequest<'_>, axis: Axis, tol: f64) - /// Top-level visible boxes that overlap `mover` on the other axis. /// -/// The moving rect is not one of them. Parents are not walked. -fn spacing_intervals(cx: &ToolContext, req: &SnapRequest<'_>, axis: Axis, mover: &Interval) -> Vec { +/// The moving rect is not one of them. Parents are not walked. `None` as soon as there would be +/// more than [`MAX_SPACING_BOXES`], before any more are collected. +fn spacing_intervals(cx: &ToolContext, req: &SnapRequest<'_>, axis: Axis, mover: &Interval) -> Option> { let mut out = Vec::new(); - let Some(sp) = cx.doc.spread(req.spread) else { return out }; + let Some(sp) = cx.doc.spread(req.spread) else { return Some(out) }; for item in &sp.items { let Some(rect) = item_target_rect(cx.selection, req, item) else { continue }; let Some(span) = interval_on(rect, axis) else { continue }; if ranges_overlap(mover.perp_lo, mover.perp_hi, span.perp_lo, span.perp_hi) { + if out.len() == MAX_SPACING_BOXES { + return None; + } out.push(span); } } - out + Some(out) } +/// Most boxes on the mover's band that smart spacing compares; a spread with more has no spacing guides. +const MAX_SPACING_BOXES: usize = 2048; + struct Interval { lo: f64, hi: f64, @@ -1213,6 +1221,8 @@ fn offer_parent_items(best: &mut Option, cx: &ToolContext, req: &SnapReque fn shown_parent_items(doc: &Document, spread: SpreadRef) -> Vec<(&Item, f64)> { let Some(sp) = doc.spread(spread) else { return Vec::new() }; let mut out = Vec::new(); + // Pages times parent items can be many times what the file stores: no more than this are listed. + let room = MAX_PARENT_OCCURRENCES; for (pi, page) in sp.pages.iter().enumerate() { if !page.show_parent_items { continue; @@ -1233,6 +1243,9 @@ fn shown_parent_items(doc: &Document, spread: SpreadRef) -> Vec<(&Item, f64)> { if parent.pages.len() > 1 && parent.page_at_x(item.bounds().center().x) != Some(ppage) { continue; } + if out.len() >= room { + return out; + } out.push((item.as_ref(), dx)); } } @@ -1240,6 +1253,9 @@ fn shown_parent_items(doc: &Document, spread: SpreadRef) -> Vec<(&Item, f64)> { out } +/// Parent item occurrences snapping considers on one spread. +const MAX_PARENT_OCCURRENCES: usize = 100_000; + fn shift_x(r: Rect, dx: f64) -> Rect { Rect::new(r.x0 + dx, r.y0, r.x1 + dx, r.y1) }