From 6b0a434c3212e3fee78491bbe6c004bbb6f401f2 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 13:04:00 +0000 Subject: [PATCH] perf(svg): CSS class and imported symbol names go on from the last suffix --- crates/svg/src/css.rs | 36 +++++++++++++++++++++++++------ crates/svg/src/import.rs | 8 +++++++ crates/svg/src/import/convert.rs | 21 +++++++++++------- crates/svg/src/import/importer.rs | 11 ++++++++-- 4 files changed, 59 insertions(+), 17 deletions(-) diff --git a/crates/svg/src/css.rs b/crates/svg/src/css.rs index 747dae25b..05ab3ce55 100644 --- a/crates/svg/src/css.rs +++ b/crates/svg/src/css.rs @@ -4,7 +4,7 @@ //! here, colours are `#rrggbb` (or `rgba()` where CSS has no separate opacity), gradients take the //! stops SVG export writes, and numbers go through [`fmt_num`]. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use serde::{Deserialize, Serialize}; use vectorcraft_color::{BlendMode, Color, GradientKind, GradientPaint, Paint}; @@ -308,7 +308,7 @@ impl CssSheet { /// paragraph's alignment; shadows and glows give `box-shadow` (`text-shadow`), a Gaussian blur /// `filter: blur()`. Other art is described as its box (see [`CssOptions::rasterize`]). pub fn css_rules(doc: &Document, ids: &[NodeId], opts: &CssOptions) -> CssSheet { - let mut w = Rules { doc, opts, sheet: CssSheet::default(), seen: HashSet::new(), names: HashSet::new() }; + let mut w = Rules { doc, opts, sheet: CssSheet::default(), seen: HashSet::new(), names: HashSet::new(), next_suffix: HashMap::new() }; for n in ids.iter().filter_map(|id| doc.node(*id)) { w.object(n); } @@ -323,6 +323,8 @@ struct Rules<'a> { seen: HashSet, /// Class names given, lower case. names: HashSet, + /// The next suffix to try for each base class name, lower case. + next_suffix: HashMap, } /// The CSS of one object's art, and why it is approximate. @@ -367,14 +369,18 @@ impl Rules<'_> { /// A class name for `name` (an object's, or its kind's), unique in the sheet. fn class_name(&mut self, name: &str) -> String { let base = sanitize_id(name).replace('.', "_"); - let mut cand = base.clone(); - for i in 2.. { + if self.names.insert(base.to_lowercase()) { + return base; + } + // Each base goes on from the suffix it last gave: objects sharing a name take linear work. + let next = self.next_suffix.entry(base.to_lowercase()).or_insert(2); + loop { + let cand = format!("{base}-{next}"); + *next += 1; if self.names.insert(cand.to_lowercase()) { - break; + return cand; } - cand = format!("{base}-{i}"); } - cand } fn rule(&mut self, n: &Node) { @@ -729,4 +735,20 @@ mod tests { assert!(family("a(b").starts_with("'a(b'")); assert!(family("Arial").starts_with("Arial,")); } + + #[test] + fn class_names_sharing_a_base_take_the_next_free_suffix() { + let doc = Document::new(10.0, 10.0); + let opts = CssOptions::default(); + let mut w = + Rules { doc: &doc, opts: &opts, sheet: CssSheet::default(), seen: HashSet::new(), names: HashSet::new(), next_suffix: HashMap::new() }; + assert_eq!(w.class_name("a-3"), "a-3"); + let names: Vec = (0..4).map(|_| w.class_name("A")).collect(); + assert_eq!(names, ["A", "A-2", "A-4", "A-5"]); + // Many objects of one name: each suffix is tried once. + for _ in 0..20_000 { + w.class_name("b"); + } + assert_eq!(w.next_suffix["b"], 20_001); + } } diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index 78c12043f..11795b83f 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -154,6 +154,9 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions, max_nodes: usize) -> Resul hidden: found.hidden, uses: found.uses, symbols: HashMap::new(), + symbol_names: HashSet::new(), + symbol_suffixes: HashMap::new(), + pattern_suffixes: HashMap::new(), blends: fx::blends(&xml, &scan.markers), files, non_scaling: found.non_scaling, @@ -304,6 +307,11 @@ struct Importer { uses: HashMap, /// The symbols made so far, by `` id. symbols: HashMap, + /// The names of the symbols made so far. + symbol_names: HashSet, + /// Where the suffixes of each symbol and pattern name base go on from (`unique_name`). + symbol_suffixes: HashMap, + pattern_suffixes: HashMap, /// Blend modes of the shadows and glows our export wrote, by filter id ([`fx::blends`]). blends: HashMap, /// The files `` elements link to. diff --git a/crates/svg/src/import/convert.rs b/crates/svg/src/import/convert.rs index 2a777b342..92f5234ee 100644 --- a/crates/svg/src/import/convert.rs +++ b/crates/svg/src/import/convert.rs @@ -15,15 +15,20 @@ pub(super) fn without_ids(n: &Node) -> Node { n } -/// `base`, or `base 2`, `base 3`… : the first that isn't `taken`. -pub(super) fn unique_name(base: &str, taken: impl Fn(&str) -> bool) -> String { - let mut name = base.to_string(); - let mut k = 2; - while taken(&name) { - name = format!("{base} {k}"); - k += 1; +/// `base`, or `base 2`, `base 3`… : the first that isn't `taken`. `next` keeps where each base's +/// suffixes go on from, so names sharing a base take linear work in all. +pub(super) fn unique_name(base: &str, next: &mut HashMap, taken: impl Fn(&str) -> bool) -> String { + if !taken(base) { + return base.to_string(); + } + let k = next.entry(base.to_string()).or_insert(2); + loop { + let name = format!("{base} {k}"); + *k += 1; + if !taken(&name) { + return name; + } } - name } /// The stops our export added for midpoints (`data-vc-midpoint`), by gradient id: for each stop, diff --git a/crates/svg/src/import/importer.rs b/crates/svg/src/import/importer.rs index eeb0a4804..0a0372e3f 100644 --- a/crates/svg/src/import/importer.rs +++ b/crates/svg/src/import/importer.rs @@ -183,7 +183,10 @@ impl Importer { Some(made) if whole && made.art == without_ids(&art) && made.reuse.allows(xf) => made.name.clone(), Some(_) => return None, None => { - let name = unique_name(self.name_of(&sym), |n| self.doc.symbols.iter().any(|s| s.name == n)); + let base = self.name_of(&sym).to_string(); + let names = &self.symbol_names; + let name = unique_name(&base, &mut self.symbol_suffixes, |n| names.contains(n)); + self.symbol_names.insert(name.clone()); let reuse = Reuse::of(&self.doc, &art); let made = SymbolMade { name: name.clone(), art: without_ids(&art), reuse }; self.doc.symbols.push(vectorcraft_doc::Symbol { dynamic: false, name: name.clone(), art: Arc::new(art) }); @@ -541,7 +544,8 @@ impl Importer { let art = self.named("", NodeKind::Group { children, clip: true }); let Some(art) = art else { return Paint::None }; let base = if pt.id().is_empty() { "Pattern" } else { pt.id() }; - let name = unique_name(base, |n| self.doc.pattern(n).is_some()); + let doc = &self.doc; + let name = unique_name(base, &mut self.pattern_suffixes, |n| doc.pattern(n).is_some()); let mut def = PatternDef::new(&name, vec![Arc::new(art)]); def.tile = tile; self.doc.patterns.push(def); @@ -714,6 +718,9 @@ mod tests { hidden: HashSet::new(), uses: HashMap::new(), symbols: HashMap::new(), + symbol_names: HashSet::new(), + symbol_suffixes: HashMap::new(), + pattern_suffixes: HashMap::new(), blends: HashMap::new(), files: Files::default(), non_scaling: HashSet::new(),