From ad8c31b3afbbadcca5141d0b98c3e532b94ed58b Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 01:15:46 +0700 Subject: [PATCH] Layers: one next_layer_name, in the document (review of #341) The engine kept its own copy of Document::next_layer_name after #353; the document's now counts sublayers too, so the engine uses it. Co-Authored-By: Claude Opus 5.5 --- crates/doc/src/lib.rs | 14 ++++---------- crates/engine/src/cmd/layer.rs | 16 +--------------- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index e8b59a3e2..c2aab92ec 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -813,20 +813,14 @@ impl Document { /// The default name of a new layer or sublayer, "Layer N". Layers and sublayers share the /// numbering: it starts after the number of layers there are and skips any layer's name. pub fn next_layer_name(&self) -> String { - let mut names = Vec::new(); + let (mut count, mut taken) = (0, std::collections::HashSet::new()); self.walk(|n| { if n.is_layer() { - names.push(n.name.as_deref().unwrap_or_default()); + count += 1; + taken.extend(n.name.as_deref()); } }); - let mut i = names.len() + 1; - loop { - let name = format!("Layer {i}"); - if !names.contains(&name.as_str()) { - return name; - } - i += 1; - } + (count + 1..).map(|i| format!("Layer {i}")).find(|name| !taken.contains(name.as_str())).unwrap_or_default() } /// Visit every node depth first in paint order (bottom to top). diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 7139d0085..9fc9353df 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -228,24 +228,10 @@ fn all_layers(d: &Document) -> Vec<&Node> { out } -/// The next unused `Layer N` name, counting sublayers too. -pub(crate) fn next_layer_name(d: &Document) -> String { - let layers = all_layers(d); - let taken: HashSet<&str> = layers.iter().filter_map(|l| l.name.as_deref()).collect(); - let mut i = layers.len() + 1; - loop { - let name = format!("Layer {i}"); - if !taken.contains(name.as_str()) { - return name; - } - i += 1; - } -} - /// A new layer named `name` (default: the next `Layer N`) in the next layer colour. pub(crate) fn make_layer(d: &mut Document, name: Option<&str>) -> Node { let n = all_layers(d).len(); - let name = name.map(str::to_string).unwrap_or_else(|| next_layer_name(d)); + let name = name.map(str::to_string).unwrap_or_else(|| d.next_layer_name()); let id = d.alloc_id(); Node::layer(id, &name, LayerColor::Preset((n % LAYER_COLORS.len()) as u8)) }