From d5509c3b6c2f50b9e49ab55200eac6b240908076 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 13 Nov 2026 15:58:43 -0800 Subject: [PATCH] Guides: stripping and mapping ruler guides look at each child of a layer once Node::strip_ruler_guides and Node::map_ruler_guide_positions asked a layer whether its whole subtree held a ruler guide, then asked each sublayer the same question again when filtering the children and again when the recursive call started. A layer nested d deep was tested 2d + 1 times, so a chain of N layers cost about N^2 layer tests. strip_ruler_guides runs when a document's objects are placed or pasted (Clipboard::from_document), when a copy of the document is saved (Document::guides_to_saved) and when a temporary editing layer is removed (Document::remove_edit_layer). map_ruler_guide_positions runs when an imported EPS is scaled. A new held_guides reads the layer once and records which children are ruler guides and which sublayers hold one; strip_held and map_held then visit and copy only those sublayers. The guides removed, the order f is called in, the art and its order, and the sharing of layers without a guide are the same as before. Neither method records an undo step. --- crates/doc/src/node/ops.rs | 81 ++++++++++---- crates/doc/src/node/tests.rs | 204 +++++++++++++++++++++++++++++++++++ 2 files changed, 265 insertions(+), 20 deletions(-) diff --git a/crates/doc/src/node/ops.rs b/crates/doc/src/node/ops.rs index 2bb655236a..fb6e8de8d9 100644 --- a/crates/doc/src/node/ops.rs +++ b/crates/doc/src/node/ops.rs @@ -763,33 +763,19 @@ impl Node { children.iter().rposition(|c| !c.is_ruler_guide()).map_or(0, |i| i + 1) } - /// Remove the ruler guides of this layer and of its sublayers. Other nodes hold none. + /// Remove the ruler guides of this layer and of its sublayers. Other nodes hold none. Only + /// layers that hold one are copied. pub fn strip_ruler_guides(&mut self) { - if !self.holds_ruler_guides() { - return; - } - if let Some(ch) = self.children_mut() { - ch.retain(|c| !c.is_ruler_guide()); - for c in ch.iter_mut().filter(|c| c.holds_ruler_guides()) { - Arc::make_mut(c).strip_ruler_guides(); - } + if let Some(held) = held_guides(self, &Self::is_ruler_guide) { + strip_held(self, held); } } /// Call `f` on the position (along the axis it crosses) of each ruler guide of this layer and /// of its sublayers, with whether the guide is vertical. Only layers that hold one are copied. pub fn map_ruler_guide_positions(&mut self, f: &mut impl FnMut(bool, &mut f64)) { - if !self.holds_ruler_guides() { - return; - } - if let Some(ch) = self.children_mut() { - for c in ch.iter_mut().filter(|c| c.is_ruler_guide() || c.holds_ruler_guides()) { - let c = Arc::make_mut(c); - match &mut c.kind { - NodeKind::RulerGuide { vertical, pos, .. } => f(*vertical, pos), - _ => c.map_ruler_guide_positions(f), - } - } + if let Some(held) = held_guides(self, &Self::is_ruler_guide) { + map_held(self, held, f); } } @@ -834,6 +820,61 @@ 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)>); + +/// 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 { + if !n.is_layer() { + return None; + } + let held: Vec<_> = n + .children() + .into_iter() + .flatten() + .enumerate() + .filter_map(|(i, c)| if is_guide(c) { Some((i, HeldGuides::default())) } else { held_guides(c, is_guide).map(|h| (i, h)) }) + .collect(); + (!held.is_empty()).then_some(HeldGuides(held)) +} + +/// 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) { + let Some(ch) = layer.children_mut() else { return }; + let mut held = held.0.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 ruler guide has nothing inside. + if inner.0.is_empty() { + return false; + } + strip_held(Arc::make_mut(c), inner); + true + }); +} + +/// 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)) { + let Some(ch) = layer.children_mut() else { return }; + for (i, inner) in held.0 { + let Some(c) = ch.get_mut(i) else { continue }; + if !inner.0.is_empty() { + map_held(Arc::make_mut(c), inner, f); + } else if let NodeKind::RulerGuide { vertical, pos, .. } = &mut Arc::make_mut(c).kind { + f(*vertical, pos); + } + } +} + impl Node { /// The Appearance panel's row for what this object's own fills and strokes paint around: /// "Contents" for a group or layer (its members) and for an image (its pixels), "Characters" diff --git a/crates/doc/src/node/tests.rs b/crates/doc/src/node/tests.rs index 9981514879..06e68a79d0 100644 --- a/crates/doc/src/node/tests.rs +++ b/crates/doc/src/node/tests.rs @@ -598,3 +598,207 @@ fn compound_stroke_path_is_its_members_in_order() { assert_eq!(bp.elements(), expected.elements()); assert!(!bp.elements().is_empty()); } + +/// The sublayers in [`guide_layers`]'s chain, each inside the one before. +const GUIDE_CHAIN: u64 = 30; + +/// How many children the layers of [`guide_layers`] hold: the layer's 6, 1 in the sublayer +/// without a guide, 2 at each of the two places the shared sublayer lies, the innermost +/// sublayer's 2 and 3 in each sublayer around it. +const GUIDE_LAYER_CHILDREN: usize = 6 + 1 + 2 * 2 + 2 + (GUIDE_CHAIN as usize - 1) * 3; + +/// How many ruler guides the layers of [`guide_layers`] hold: one in each sublayer of the chain, +/// one at each place the shared sublayer lies and the layer's own. +const GUIDE_LAYER_GUIDES: usize = GUIDE_CHAIN as usize + 2 + 1; + +/// A layer holding, in order: [`GUIDE_CHAIN`] sublayers, each inside the one before and holding an +/// object and a ruler guide; a sublayer with an object only; a sublayer with an object and a +/// guide; an object; that last sublayer again; and a guide. Guides differ in direction, artboard +/// and lock. Returns the layer, the sublayer without a guide and the one that lies twice. +fn guide_layers() -> (Node, Arc, Arc) { + let rect = |id| Arc::new(Node::path(NodeId(id), shapes::rectangle(Rect::new(0.0, 0.0, 10.0, 10.0)), Appearance::default_art())); + let guide = |id: u64| { + let kind = NodeKind::RulerGuide { vertical: id.is_multiple_of(2), pos: id as f64, artboard: id.is_multiple_of(3).then_some(7) }; + let mut g = Node::new(NodeId(id), kind); + g.locked = id.is_multiple_of(5); + Arc::new(g) + }; + let sublayer = |id, children: Vec>| { + let mut l = Node::layer(NodeId(id), "Sub", LayerColor::Preset(1)); + if let Some(ch) = l.children_mut() { + *ch = children; + } + Arc::new(l) + }; + let mut chain = sublayer(100, vec![rect(150), guide(200)]); + for depth in 1..GUIDE_CHAIN { + chain = sublayer(100 + depth, vec![rect(600 + depth), chain, guide(300 + depth)]); + } + let still = sublayer(500, vec![rect(501)]); + let twice = sublayer(510, vec![rect(511), guide(512)]); + let mut layer = Node::layer(NodeId(1), "Layer", LayerColor::Preset(0)); + if let Some(ch) = layer.children_mut() { + *ch = vec![chain, Arc::clone(&still), Arc::clone(&twice), rect(2), Arc::clone(&twice), guide(3)]; + } + (layer, still, twice) +} + +/// Whether `a` and `b` hold the same children, none of them copied. +fn same_children(a: &Node, b: &Node) -> bool { + let (a, b) = (a.children().unwrap(), b.children().unwrap()); + a.len() == b.len() && a.iter().zip(b).all(|(a, b)| Arc::ptr_eq(a, b)) +} + +/// Stripping the ruler guides of a layer looks at each child of the layer and of its sublayers +/// once, however deep the sublayers nest. The art and its order stay, a sublayer without a guide is +/// not copied, and a copy of the layer held elsewhere keeps its guides. +#[test] +fn stripping_ruler_guides_looks_at_each_child_of_nested_layers_once() { + use std::cell::Cell; + let (whole, still, twice) = guide_layers(); + let ids = |n: &Node| { + let mut out = vec![]; + n.walk(&mut |c| out.push(c.id)); + out + }; + let mut art = vec![]; + whole.walk(&mut |c| { + if !c.is_ruler_guide() { + art.push(c.id); + } + }); + assert_eq!(ids(&whole).len(), art.len() + GUIDE_LAYER_GUIDES); + + let mut n = whole.clone(); + let looked = Cell::new(0_usize); + let pick = |c: &Node| { + looked.set(looked.get() + 1); + c.is_ruler_guide() + }; + let held = super::ops::held_guides(&n, &pick).unwrap(); + assert_eq!(looked.get(), GUIDE_LAYER_CHILDREN, "each child of the layers is looked at once"); + super::ops::strip_held(&mut n, held); + assert_eq!(looked.get(), GUIDE_LAYER_CHILDREN, "removing looks at nothing again"); + assert_eq!(ids(&n), art, "the art stays, in its order"); + let ch = n.children().unwrap(); + assert!(Arc::ptr_eq(&ch[1], &still), "a sublayer without a guide is not copied"); + assert!(Arc::ptr_eq(&ch[3], &whole.children().unwrap()[3]), "an object is not copied"); + assert!(!Arc::ptr_eq(&ch[2], &twice) && !Arc::ptr_eq(&ch[4], &twice) && !Arc::ptr_eq(&ch[2], &ch[4])); + assert_eq!( + (ch[2].children().map(Vec::len), ch[4].children().map(Vec::len)), + (Some(1), Some(1)), + "a sublayer lying twice is stripped at both places" + ); + // The copies held elsewhere (an undo step's, say) keep their guides. + assert_eq!(twice.children().map(Vec::len), Some(2)); + assert_eq!(ids(&whole).len(), art.len() + GUIDE_LAYER_GUIDES); + + // The method strips the same guides. + let mut same = whole.clone(); + same.strip_ruler_guides(); + assert_eq!(same, n); + assert!(Arc::ptr_eq(&same.children().unwrap()[1], &still)); + // With no guide left there is nothing to remove, and nothing is copied. + let before = n.clone(); + n.strip_ruler_guides(); + assert!(same_children(&n, &before)); + let mut empty = Node::layer(NodeId(900), "Empty", LayerColor::Preset(0)); + empty.strip_ruler_guides(); + assert_eq!(empty.children().map(Vec::len), Some(0)); + // Only a layer holds ruler guides: a group is left as it is, on its own and in a layer. + let mut group = Node::group(NodeId(901), vec![Arc::clone(&twice)]); + assert!(super::ops::held_guides(&group, &Node::is_ruler_guide).is_none()); + group.strip_ruler_guides(); + assert!(Arc::ptr_eq(&group.children().unwrap()[0], &twice)); + let group = Arc::new(group); + let mut around = Node::layer(NodeId(902), "Around", LayerColor::Preset(0)); + around.children_mut().unwrap().push(Arc::clone(&group)); + around.strip_ruler_guides(); + assert!(Arc::ptr_eq(&around.children().unwrap()[0], &group)); +} + +/// Changing the positions of the ruler guides of a layer looks at each child of the layer and of +/// its sublayers once, however deep the sublayers nest. Each guide is changed once, in the order +/// the guides lie in, and keeps everything else about it; the art is unchanged, a sublayer without +/// a guide is not copied, and a copy of the layer held elsewhere keeps its positions. +#[test] +fn mapping_ruler_guide_positions_looks_at_each_child_of_nested_layers_once() { + use std::cell::Cell; + let (whole, still, twice) = guide_layers(); + let mut lines = vec![]; + whole.walk(&mut |c| { + if let NodeKind::RulerGuide { vertical, pos, .. } = c.kind { + lines.push((vertical, pos)); + } + }); + assert_eq!(lines.len(), GUIDE_LAYER_GUIDES); + assert!(lines.iter().any(|l| l.0) && lines.iter().any(|l| !l.0), "guides in both directions"); + + let mut n = whole.clone(); + let looked = Cell::new(0_usize); + let pick = |c: &Node| { + looked.set(looked.get() + 1); + c.is_ruler_guide() + }; + let held = super::ops::held_guides(&n, &pick).unwrap(); + assert_eq!(looked.get(), GUIDE_LAYER_CHILDREN, "each child of the layers is looked at once"); + let mut seen = vec![]; + super::ops::map_held(&mut n, held, &mut |vertical, pos| { + seen.push((vertical, *pos)); + *pos = *pos * 2.0 + 1.0; + }); + assert_eq!(looked.get(), GUIDE_LAYER_CHILDREN, "changing looks at nothing again"); + assert_eq!(seen, lines, "each guide once, in the order the guides lie in"); + + // The same nodes in the same order: a guide differs in its position alone, and the art not + // at all. + let (mut was, mut now) = (vec![], vec![]); + whole.walk(&mut |c| was.push(c)); + n.walk(&mut |c| now.push(c)); + assert_eq!(was.len(), now.len()); + for (a, b) in was.into_iter().zip(now) { + if a.is_layer() { + assert_eq!((b.id, b.is_layer(), b.children().map(Vec::len)), (a.id, true, a.children().map(Vec::len))); + continue; + } + let mut want = a.clone(); + if let NodeKind::RulerGuide { pos, .. } = &mut want.kind { + *pos = *pos * 2.0 + 1.0; + } + assert_eq!(b, &want); + } + let ch = n.children().unwrap(); + assert!(Arc::ptr_eq(&ch[1], &still), "a sublayer without a guide is not copied"); + assert!(Arc::ptr_eq(&ch[3], &whole.children().unwrap()[3]), "an object is not copied"); + assert!(!Arc::ptr_eq(&ch[2], &twice) && !Arc::ptr_eq(&ch[4], &twice) && !Arc::ptr_eq(&ch[2], &ch[4])); + // The copy held elsewhere keeps its position. + assert!(matches!(twice.children().unwrap()[1].kind, NodeKind::RulerGuide { pos, .. } if pos == 512.0)); + + // The method changes the same guides, each once. + let mut same = whole.clone(); + let mut calls = 0; + same.map_ruler_guide_positions(&mut |_, pos| { + calls += 1; + *pos = *pos * 2.0 + 1.0; + }); + assert_eq!((calls, &same), (GUIDE_LAYER_GUIDES, &n)); + assert!(Arc::ptr_eq(&same.children().unwrap()[1], &still)); + // A function that leaves the positions as they are is still called on each guide. + let mut kept = whole.clone(); + let mut calls = 0; + kept.map_ruler_guide_positions(&mut |_, _| calls += 1); + assert_eq!((calls, &kept), (GUIDE_LAYER_GUIDES, &whole)); + // A layer without a guide, an empty layer and a group: nothing to change, nothing copied. + let mut bare = whole.clone(); + bare.strip_ruler_guides(); + let before = bare.clone(); + let mut calls = 0; + bare.map_ruler_guide_positions(&mut |_, _| calls += 1); + assert!(same_children(&bare, &before)); + let mut empty = Node::layer(NodeId(900), "Empty", LayerColor::Preset(0)); + empty.map_ruler_guide_positions(&mut |_, _| calls += 1); + let mut group = Node::group(NodeId(901), vec![Arc::clone(&twice)]); + group.map_ruler_guide_positions(&mut |_, _| calls += 1); + assert!(Arc::ptr_eq(&group.children().unwrap()[0], &twice)); + assert_eq!(calls, 0); +}