From 7f1764973146e3b6a0e62ee8e95130c41a32d394 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 14 Nov 2026 04:34:05 -0800 Subject: [PATCH] Guides: document and layer guide edits share one set of one-pass helpers Document::guides_to_saved, remove_edit_layer, retain_guides and the artboard guide moves now call the layer helpers in node/ops.rs (held_guides, strip_held, map_held), which Node::strip_ruler_guides and Node::map_ruler_guide_positions already use. The copies in document_guides.rs (picked_in_layers, drop_picked, edit_picked) are removed. Picked stays for the walks through objects and opacity-mask art. Results, guide order, copies kept for undo and the number of nodes each walk looks at are unchanged. --- crates/doc/src/document_guides.rs | 76 ++++++------------------------- crates/doc/src/node.rs | 1 + crates/doc/src/node/ops.rs | 8 ++-- 3 files changed, 18 insertions(+), 67 deletions(-) diff --git a/crates/doc/src/document_guides.rs b/crates/doc/src/document_guides.rs index df336fe09e..ba6e4dc428 100644 --- a/crates/doc/src/document_guides.rs +++ b/crates/doc/src/document_guides.rs @@ -1,6 +1,7 @@ use super::document_ext::free_id; use super::guides::Guide; use super::limits::MAX_NEXT_ID; +use super::node::{held_guides, map_held, strip_held}; use super::{Affine, Arc, Document, Node, NodeId, NodeKind, PatternDef, Point, Rect, Selection, SymbolBoxes, Vec2, artboard_holds}; use std::collections::{BTreeMap, HashSet}; @@ -272,8 +273,8 @@ impl Document { return; } for l in &mut self.layers { - if let Some(held) = picked_in_layers(l, &Node::is_ruler_guide) { - drop_picked(l, held); + if let Some(held) = held_guides(l, &Node::is_ruler_guide) { + strip_held(Arc::make_mut(l), held); } } self.saved_guides.extend(guides); @@ -347,14 +348,14 @@ impl Document { /// saved guide whose layer is gone opens. Returns the removed node, without the guides. pub fn remove_edit_layer(&mut self, id: NodeId) -> Result, crate::DocError> { let mut layer = self.remove(id)?; - let Some(held) = picked_in_layers(&layer, &Node::is_ruler_guide) else { return Ok(layer) }; + let Some(held) = held_guides(&layer, &Node::is_ruler_guide) else { return Ok(layer) }; let mut guides = vec![]; layer.walk(&mut |c| { if c.is_ruler_guide() { guides.push(Arc::new(c.clone())); } }); - drop_picked(&mut layer, held); + strip_held(Arc::make_mut(&mut layer), held); let home = self.default_layer().unwrap_or_else(|| self.add_layer(None)); self.children_mut(Some(home))?.extend(guides); Ok(layer) @@ -418,8 +419,8 @@ impl Document { } let pick = |n: &Node| n.is_ruler_guide() && gone.contains(&n.id); for l in &mut self.layers { - if let Some(held) = picked_in_layers(l, &pick) { - drop_picked(l, held); + if let Some(held) = held_guides(l, &pick) { + strip_held(Arc::make_mut(l), held); } } sel.guides.retain(|id| !gone.contains(id)); @@ -434,8 +435,8 @@ impl Document { fn edit_artboard_guides(&mut self, id: u32, f: &mut impl FnMut(bool, &mut f64)) { let pick = |n: &Node| matches!(n.kind, NodeKind::RulerGuide { artboard: Some(a), .. } if a == id); for l in &mut self.layers { - if let Some(held) = picked_in_layers(l, &pick) { - edit_picked(l, held, f); + if let Some(held) = held_guides(l, &pick) { + map_held(Arc::make_mut(l), held, f); } } } @@ -569,57 +570,6 @@ impl Picked { } } -/// The children of `n`, if it is a layer, that `pick` picks (with nothing inside), and its -/// sublayers holding one (with what they hold), or `None` when it holds none. `pick` looks at -/// each child of the layers once. -fn picked_in_layers(n: &Node, pick: &impl Fn(&Node) -> bool) -> Option { - if !n.is_layer() { - return None; - } - let children = n - .children() - .into_iter() - .flatten() - .enumerate() - .filter_map(|(i, c)| if pick(c) { Some((i, Picked::default())) } else { picked_in_layers(c, pick).map(|p| (i, p)) }) - .collect(); - Picked { children, mask: None }.some() -} - -/// Remove the children of `layer` and of its sublayers that [`picked_in_layers`] picked (`held`). -/// Only the layers holding one are copied, and only those are visited. -fn drop_picked(layer: &mut Arc, held: Picked) { - let Some(ch) = Arc::make_mut(layer).children_mut() else { return }; - let mut held = held.children.into_iter().peekable(); - let mut next = 0; - ch.retain_mut(|c| { - let at = next; - next += 1; - let Some((_, inner)) = held.next_if(|(i, _)| *i == at) else { return true }; - // A sublayer holds something; a child picked itself has nothing inside. - if inner.is_empty() { - return false; - } - drop_picked(c, inner); - true - }); -} - -/// Change with `f` (whether it is vertical, its position) the line of each ruler guide of `layer` -/// and of its sublayers that [`picked_in_layers`] picked (`held`), in the order they lie in. Only -/// the layers holding one, and the guides, are copied, and only those layers are visited. -fn edit_picked(layer: &mut Arc, held: Picked, f: &mut impl FnMut(bool, &mut f64)) { - let Some(ch) = Arc::make_mut(layer).children_mut() else { return }; - for (i, inner) in held.children { - let Some(c) = ch.get_mut(i) else { continue }; - if !inner.is_empty() { - edit_picked(c, inner, f); - } else if let NodeKind::RulerGuide { vertical, pos, .. } = &mut Arc::make_mut(c).kind { - f(*vertical, pos); - } - } -} - /// `n` picked itself when it is a ruler guide by `is_guide`, else the ruler guides below it /// ([`guides_below`]). fn guide_or_below(n: &Node, is_guide: &impl Fn(&Node) -> bool) -> Option { @@ -872,9 +822,9 @@ mod tests { looked.set(looked.get() + 1); on_artboard_7(n) }; - let held = picked_in_layers(&d.layers[0], &pick).unwrap(); + let held = held_guides(&d.layers[0], &pick).unwrap(); assert_eq!(looked.get(), NESTED_CHILDREN, "each child of the layers is looked at once"); - drop_picked(&mut d.layers[0], held); + strip_held(Arc::make_mut(&mut d.layers[0]), held); assert_eq!(looked.get(), NESTED_CHILDREN, "removing looks at nothing again"); assert!(d.layers[0].children().is_some_and(|c| Arc::ptr_eq(&c[1], &still)), "a sublayer with nothing to remove is not copied"); @@ -932,10 +882,10 @@ mod tests { looked.set(looked.get() + 1); on_artboard_7(n) }; - let held = picked_in_layers(&d.layers[0], &pick).unwrap(); + let held = held_guides(&d.layers[0], &pick).unwrap(); assert_eq!(looked.get(), NESTED_CHILDREN, "each child of the layers is looked at once"); let mut seen = vec![]; - edit_picked(&mut d.layers[0], held, &mut |vertical, pos: &mut f64| { + map_held(Arc::make_mut(&mut d.layers[0]), held, &mut |vertical, pos: &mut f64| { seen.push((vertical, *pos)); *pos += 9.0; }); diff --git a/crates/doc/src/node.rs b/crates/doc/src/node.rs index eeec6ab162..79ce594212 100644 --- a/crates/doc/src/node.rs +++ b/crates/doc/src/node.rs @@ -32,6 +32,7 @@ pub use kind::{ImageObject, MAX_BUFFERED_VALUES, NodeKind, with_buffered_value_b pub use live_shape::{LiveShape, Scaling}; pub use opacity::{Knockout, OpacityMask, whiten}; pub use ops::{flattens, keeps_gradient_fit}; +pub(crate) use ops::{held_guides, map_held, strip_held}; use wire::NodeWire; pub(crate) use wire::counted_list; pub use wire::{MAX_READ_NODES, with_node_budget}; diff --git a/crates/doc/src/node/ops.rs b/crates/doc/src/node/ops.rs index fb6e8de8d9..efb006afed 100644 --- a/crates/doc/src/node/ops.rs +++ b/crates/doc/src/node/ops.rs @@ -823,11 +823,11 @@ impl Node { /// The ruler guides a layer holds: its children that are one, by index in order (each with /// nothing inside), and its sublayers holding one (with what they hold). #[derive(Debug, Default)] -pub(super) struct HeldGuides(Vec<(usize, HeldGuides)>); +pub(crate) struct HeldGuides(Vec<(usize, HeldGuides)>); /// The ruler guides, by `is_guide`, that `n` holds if it is a layer, or `None` when it holds /// none. `is_guide` looks at each child of `n` and of its sublayers once. -pub(super) fn held_guides(n: &Node, is_guide: &impl Fn(&Node) -> bool) -> Option { +pub(crate) fn held_guides(n: &Node, is_guide: &impl Fn(&Node) -> bool) -> Option { if !n.is_layer() { return None; } @@ -843,7 +843,7 @@ pub(super) fn held_guides(n: &Node, is_guide: &impl Fn(&Node) -> bool) -> Option /// Remove the ruler guides of `layer` and of its sublayers that [`held_guides`] found (`held`). /// Only the sublayers holding one are copied, and only those are visited. -pub(super) fn strip_held(layer: &mut Node, held: HeldGuides) { +pub(crate) fn strip_held(layer: &mut Node, held: HeldGuides) { let Some(ch) = layer.children_mut() else { return }; let mut held = held.0.into_iter().peekable(); let mut next = 0; @@ -863,7 +863,7 @@ pub(super) fn strip_held(layer: &mut Node, held: HeldGuides) { /// Call `f` (whether it is vertical, its position) on each ruler guide of `layer` and of its /// sublayers that [`held_guides`] found (`held`), in the order they lie in. Only the sublayers /// holding one, and the guides, are copied, and only those sublayers are visited. -pub(super) fn map_held(layer: &mut Node, held: HeldGuides, f: &mut impl FnMut(bool, &mut f64)) { +pub(crate) fn map_held(layer: &mut Node, held: HeldGuides, f: &mut impl FnMut(bool, &mut f64)) { let Some(ch) = layer.children_mut() else { return }; for (i, inner) in held.0 { let Some(c) = ch.get_mut(i) else { continue };