From 85b6c2a5eb4fb01484d7d3134df638d46427915d Mon Sep 17 00:00:00 2001
From: dotCooCoo <RobertLeeLW@gmail.com>
Date: Mon, 2 Nov 2026 02:59:51 -0800
Subject: [PATCH] SVG: ids made up on import never repeat an id in the file

The import gives an element an id when it needs to find it again in
usvg's tree (links, symbol uses, events, data attributes, and now
circles and ellipses). A made-up id could repeat one the file already
had, such as vectorcraft-id-0, so a <use> or paint reference to that
element showed the other one. Made-up ids now skip every id in the file.

Circles and ellipses in a file whose style sheet has an [id selector get
no made-up id and keep usvg's curves, so the selector matches the same
elements as before.
---
 crates/svg/src/import/prepass.rs    | 22 ++++++++++++++++++----
 crates/svg/tests/it/basic_shapes.rs | 20 ++++++++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/crates/svg/src/import/prepass.rs b/crates/svg/src/import/prepass.rs
index 2afa651a5..8ed825948 100644
--- a/crates/svg/src/import/prepass.rs
+++ b/crates/svg/src/import/prepass.rs
@@ -40,6 +40,8 @@ pub(crate) struct Edits {
     list: Vec<(std::ops::Range<usize>, String)>,
     /// The ids made up so far, by element.
     ids: HashMap<roxmltree::NodeId, String>,
+    /// The ids in the file and those made up, read when the first id is made up.
+    taken: Option<HashSet<String>>,
     /// Declarations added to elements' `style`s, by element: (where they go, whether the element
     /// has a `style` attribute, the declarations, XML-escaped).
     styles: HashMap<roxmltree::NodeId, (usize, bool, String)>,
@@ -50,7 +52,8 @@ impl Edits {
         self.list.push((at..at, text));
     }
 
-    /// The id of element `n` (in `svg`), made up and written in when it has none.
+    /// The id of element `n` (in `svg`), made up and written in when it has none. A made-up id is
+    /// one no element of the file has.
     pub(super) fn id(&mut self, svg: &str, n: XNode) -> String {
         if let Some(id) = n.attribute("id").filter(|id| !id.is_empty()) {
             return id.to_string();
@@ -58,7 +61,16 @@ impl Edits {
         if let Some(id) = self.ids.get(&n.id()) {
             return id.clone();
         }
-        let id = format!("{MADE_UP_ID}{}", self.ids.len());
+        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}");
+            if !taken.contains(&id) {
+                break id;
+            }
+            i += 1;
+        };
+        taken.insert(id.clone());
         self.set(svg, n, "id", &id);
         self.ids.insert(n.id(), id.clone());
         id
@@ -119,7 +131,8 @@ impl Edits {
 ///   ([`user_events`]), and the `<script>` links are recorded ([`linked_scripts`]);
 /// * so do objects with attributes in other applications' namespaces ([`foreign`]);
 /// * so do `<circle>` and `<ellipse>` elements, recorded: usvg makes their arcs into a varying
-///   number of curves, and they come back as the ellipse VectorCraft draws;
+///   number of curves, and they come back as the ellipse VectorCraft draws (not in a file whose
+///   style sheet has an `[id` selector);
 /// * linked files are read ([`files`]);
 /// * percentage lengths in a `<symbol>` with a `viewBox` become numbers ([`symbol_units`]);
 /// * what style sheet rules with a `:not()` set goes in `style` attributes ([`not_rules`]).
@@ -134,7 +147,8 @@ pub(super) fn prepass<'s>(svg: &'s str, markers: &str, xml: &roxmltree::Document
     let not = markers.contains(":not(");
     let events = SVG_EVENTS.iter().any(|e| markers.contains(e));
     let foreign_attrs = markers.contains("xmlns:") && has_foreign_attributes(xml);
-    let ellipses = ["<circle", ":circle", "<ellipse", ":ellipse"].iter().any(|t| markers.contains(t));
+    // An id would change what a style sheet's `[id]` selector matches: such files keep usvg's curves.
+    let ellipses = !markers.contains("[id") && ["<circle", ":circle", "<ellipse", ":ellipse"].iter().any(|t| markers.contains(t));
     if !css_transform
         && !not
         && !events
diff --git a/crates/svg/tests/it/basic_shapes.rs b/crates/svg/tests/it/basic_shapes.rs
index 24ef6fd8d..8d76739b4 100644
--- a/crates/svg/tests/it/basic_shapes.rs
+++ b/crates/svg/tests/it/basic_shapes.rs
@@ -183,3 +183,23 @@ fn circles_and_ellipses_import_as_the_ellipse_drawn() {
     let far = &back[3].subpaths[0].anchors;
     assert!(far.len() == 4 && far[0].p.distance(Point::new(40_000_400.0, 0.0)) < 8.0, "{far:?}");
 }
+
+#[test]
+fn circles_import_without_changing_ids_or_style_matches() {
+    // The file's own ids stay unique: the `<use>` shows the rectangle, not the circle.
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="200">
+        <circle cx="30" cy="30" r="20" fill="#0000ff"/>
+        <defs><rect id="vectorcraft-id-0" width="40" height="40" fill="#ff0000"/></defs>
+        <use xlink:href="#vectorcraft-id-0" x="120" y="120"/></svg>"##;
+    let img = render_artboard(&import(svg).unwrap());
+    assert_eq!(img.over_white(30, 30), [0, 0, 255]);
+    assert_eq!(img.over_white(140, 140), [255, 0, 0]);
+    // A style sheet's `[id]` selector still matches only the circle that has an id.
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" width="200" height="200">
+        <style>circle[id] { fill: #ff0000 }</style>
+        <circle cx="30" cy="30" r="20" fill="#0000ff"/>
+        <circle id="named" cx="130" cy="30" r="20" fill="#0000ff"/></svg>"##;
+    let img = render_artboard(&import(svg).unwrap());
+    assert_eq!(img.over_white(30, 30), [0, 0, 255]);
+    assert_eq!(img.over_white(130, 30), [255, 0, 0]);
+}