From b4d3ae41bfd6e64620f930fa35eec01a410c7895 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 2 Nov 2026 03:04:04 -0800 Subject: [PATCH] SVG: made-up ids skip the file's taken ids once Each id made up on import starts where the last one stopped instead of at the number of ids made so far, so a file that already holds many of the made-up ids does not make every new id try them all again. The ids made up are the same. --- crates/svg/src/import/prepass.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/svg/src/import/prepass.rs b/crates/svg/src/import/prepass.rs index 8ed825948..a635b88d5 100644 --- a/crates/svg/src/import/prepass.rs +++ b/crates/svg/src/import/prepass.rs @@ -42,6 +42,8 @@ pub(crate) struct Edits { ids: HashMap, /// The ids in the file and those made up, read when the first id is made up. taken: Option>, + /// The number the next made-up id is tried with: the numbers below it are made up or taken. + next: usize, /// Declarations added to elements' `style`s, by element: (where they go, whether the element /// has a `style` attribute, the declarations, XML-escaped). styles: HashMap, @@ -62,13 +64,12 @@ impl Edits { return id.clone(); } let taken = self.taken.get_or_insert_with(|| n.document().descendants().filter_map(|e| e.attribute("id")).map(str::to_string).collect()); - let mut i = self.ids.len(); let id = loop { - let id = format!("{MADE_UP_ID}{i}"); + let id = format!("{MADE_UP_ID}{}", self.next); + self.next += 1; if !taken.contains(&id) { break id; } - i += 1; }; taken.insert(id.clone()); self.set(svg, n, "id", &id); @@ -415,3 +416,25 @@ pub(super) fn labels(xml: &roxmltree::Document) -> HashMap { }) .collect() } + +#[cfg(test)] +mod tests { + use super::*; + + /// A file that already holds the made-up ids does not make each new id try them all again: + /// 5,000 ids made up past 50,000 taken ones take a moment, not 250 million tries. + #[test] + fn made_up_ids_skip_the_taken_ones_once() { + let taken: String = (0..50_000).map(|i| format!("")).collect(); + let bare = "".repeat(5_000); + let svg = format!("{taken}{bare}"); + let doc = roxmltree::Document::parse(&svg).unwrap(); + let rects: Vec = doc.descendants().filter(|n| n.has_tag_name("rect")).collect(); + let mut edits = Edits::default(); + let start = std::time::Instant::now(); + let ids: Vec = rects.iter().map(|n| edits.id(&svg, *n)).collect(); + assert!(start.elapsed() < std::time::Duration::from_secs(1), "{:?}", start.elapsed()); + assert_eq!(ids.first().map(String::as_str), Some(format!("{MADE_UP_ID}50000").as_str())); + assert_eq!(ids.iter().collect::>().len(), ids.len()); + } +}