M4.29: object ids and names survive the SVG round trip

Layer Names writes readable ids plus data-name with the original name
whenever the id differs; Minimal writes only the ids definitions need;
Unique prefixes every id and CSS class with a content hash, so SVGs
inlined in one page never collide. Import names objects and layers from
data-name, inkscape:label, serif:id or aria-label.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/svg/src/export.rs

@@ -283,11 +283,17 @@ impl Writer<'_> {
             self.names.insert(id, cand);
         }
     }
+    /// ` id="…"` for a named object, plus ` data-name="…"` with the name itself when the id had to
+    /// differ from it (spaces, punctuation, duplicates, the unique prefix).
     fn id_attr(&self, n: &Node) -> String {
         if self.anonymous {
             return String::new();
         }
-        self.names.get(&n.id).map(|id| format!(" id=\"{}\"", xml_escape(id))).unwrap_or_default()
+        let Some(id) = self.names.get(&n.id) else { return String::new() };
+        match n.name.as_deref() {
+            Some(name) if name != id => format!(" id=\"{}\" data-name=\"{}\"", xml_escape(id), xml_escape(name)),
+            _ => format!(" id=\"{}\"", xml_escape(id)),
+        }
     }
     fn matrix(&self, m: Affine) -> String {
         let c = m.as_coeffs();

crates/svg/src/import.rs

@@ -33,7 +33,16 @@ pub(crate) fn import(svg: &str) -> Result<(Document, Vec<String>), SvgError> {
     let (kx, ky) = units.k;
     let mut doc = Document::new(size.width() as f64 * kx, size.height() as f64 * ky);
     doc.units = units.unit;
-    let mut im = Importer { doc, warnings, mask_flags: mask_flags(&xml), links, slots, patterns: HashMap::new(), midpoints: midpoint_stops(&xml) };
+    let mut im = Importer {
+        doc,
+        warnings,
+        mask_flags: mask_flags(&xml),
+        links,
+        slots,
+        patterns: HashMap::new(),
+        midpoints: midpoint_stops(&xml),
+        labels: labels(&xml),
+    };
 
     // usvg wraps everything in an id-less group carrying the viewBox transform when needed.
     let mut top = tree.root();
@@ -69,7 +78,8 @@ pub(crate) fn import(svg: &str) -> Result<(Document, Vec<String>), SvgError> {
             children.extend(im.children(g, base * aff(g.transform())));
             let id = im.doc.alloc_id();
             let preset = (im.doc.layers.len() % vectorcraft_doc::LAYER_COLORS.len()) as u8;
-            let mut l = Node::layer(id, g.id(), LayerColor::Preset(preset));
+            let name = im.name_of(g.id()).to_string();
+            let mut l = Node::layer(id, &name, LayerColor::Preset(preset));
             if let Some(ch) = l.children_mut() {
                 *ch = children;
             }
@@ -142,6 +152,8 @@ struct Importer {
     patterns: HashMap<usize, String>,
     /// Midpoint stops by gradient id (see [`midpoint_stops`]).
     midpoints: HashMap<String, Vec<Option<f32>>>,
+    /// Object names by element id (see [`labels`]).
+    labels: HashMap<String, String>,
 }
 
 /// The stops our export added for midpoints (`data-vc-midpoint`), by gradient id: for each stop,
@@ -246,6 +258,24 @@ fn link(n: &mut Node, url: &str) {
     }
 }
 
+/// The names apps keep beside element ids, by id: `data-name`, `inkscape:label`, `serif:id` or
+/// `aria-label` (the first one present).
+fn labels(xml: &roxmltree::Document) -> HashMap<String, String> {
+    const INKSCAPE: &str = "http://www.inkscape.org/namespaces/inkscape";
+    const SERIF: &str = "http://www.serif.com/";
+    xml.descendants()
+        .filter_map(|n| {
+            let id = n.attribute("id").filter(|id| !id.is_empty())?;
+            let name = n
+                .attribute("data-name")
+                .or_else(|| n.attribute((INKSCAPE, "label")))
+                .or_else(|| n.attribute((SERIF, "id")))
+                .or_else(|| n.attribute("aria-label"))?;
+            Some((id.to_string(), name.to_string()))
+        })
+        .collect()
+}
+
 fn aff(t: usvg::Transform) -> Affine {
     Affine::new([t.sx as f64, t.ky as f64, t.kx as f64, t.sy as f64, t.tx as f64, t.ty as f64])
 }
@@ -325,11 +355,16 @@ impl Importer {
         gradient_stops(g, self.midpoints.get(g.id()).map(Vec::as_slice))
     }
 
+    /// The object name for an element id: its label (see [`labels`]), else the id itself.
+    fn name_of<'a>(&'a self, id: &'a str) -> &'a str {
+        self.labels.get(id).map_or(id, String::as_str)
+    }
+
     fn named(&mut self, id: &str, kind: NodeKind) -> Node {
         let nid = self.doc.alloc_id();
         let mut n = Node::new(nid, kind);
         if !id.is_empty() {
-            n.name = Some(id.to_string());
+            n.name = Some(self.name_of(id).to_string());
         }
         n
     }

crates/svg/tests/ids.rs

@@ -0,0 +1,104 @@
+//! Object IDs: names survive the round trip (Layer Names), Minimal writes only referenced ids,
+//! Unique ids never collide, and other apps' label attributes become object names.
+// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md › Robustness).
+#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
+
+use std::collections::HashSet;
+
+use vectorcraft_color::{Color, Gradient, GradientPaint, Paint};
+use vectorcraft_doc::{Appearance, Document, Node};
+use vectorcraft_geom::{Rect, shapes};
+use vectorcraft_svg::{ExportOptions, ObjectIds, Styling, export, import};
+
+/// A document with a named rectangle, an unnamed gradient-filled one and a second layer.
+fn doc(title: &str, shift: f64) -> Document {
+    let mut d = Document::new(200.0, 200.0);
+    d.title = title.into();
+    let l = d.layers[0].id;
+    let mut a = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(10.0 + shift, 10.0, 60.0, 60.0)), Appearance::default_art());
+    a.name = Some("Logo mark (v2)".into());
+    d.insert(Some(l), usize::MAX, a).unwrap();
+    let g = Paint::Gradient(Box::new(GradientPaint::new(Gradient::default())));
+    let b = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(80.0, 10.0, 120.0, 60.0)), Appearance::basic(g, Paint::None, 0.0));
+    d.insert(Some(l), usize::MAX, b).unwrap();
+    d.add_layer(Some("Top & Front"));
+    let top = d.layers[1].id;
+    let c = Node::path(
+        d.alloc_id(),
+        shapes::ellipse(Rect::new(0.0, 100.0, 50.0, 150.0)),
+        Appearance::basic(Paint::solid(Color::BLACK), Paint::None, 0.0),
+    );
+    d.insert(Some(top), usize::MAX, c).unwrap();
+    d
+}
+
+/// Every `id="…"` value.
+fn ids(svg: &str) -> Vec<String> {
+    svg.split(" id=\"").skip(1).map(|s| s[..s.find('"').unwrap()].to_string()).collect()
+}
+
+fn names(d: &Document) -> Vec<String> {
+    let mut v = vec![];
+    d.walk(|n| v.extend(n.name.clone()));
+    v
+}
+
+#[test]
+fn layer_names_round_trip_exactly() {
+    let d = doc("a", 0.0);
+    let s = export(&d, &ExportOptions::default());
+    assert!(s.contains("id=\"Layer_1\" data-name=\"Layer 1\""), "{s}");
+    assert!(s.contains("data-name=\"Top &amp; Front\""), "{s}");
+    let back = import(&s).unwrap();
+    assert_eq!(back.layers.iter().map(|l| l.name.clone().unwrap()).collect::<Vec<_>>(), ["Layer 1", "Top & Front"]);
+    assert!(names(&back).contains(&"Logo mark (v2)".to_string()), "{:?}", names(&back));
+    // An id that is the name already gets no data-name.
+    let mut d = Document::new(10.0, 10.0);
+    std::sync::Arc::make_mut(&mut d.layers[0]).name = Some("Layer_1".into());
+    let s = export(&d, &ExportOptions::default());
+    assert!(s.contains("<g id=\"Layer_1\">"), "{s}");
+}
+
+#[test]
+fn minimal_writes_only_referenced_ids() {
+    let s = export(&doc("a", 0.0), &ExportOptions { object_ids: ObjectIds::Minimal, ..Default::default() });
+    let ids = ids(&s);
+    assert!(!ids.is_empty(), "the gradient keeps its id");
+    for id in &ids {
+        assert!(s.contains(&format!("#{id})")) || s.contains(&format!("\"#{id}\"")), "`{id}` is never referenced:\n{s}");
+    }
+    assert!(!s.contains("data-name"));
+}
+
+#[test]
+fn unique_ids_never_collide() {
+    let opts = ExportOptions { object_ids: ObjectIds::Unique, styling: Styling::InternalCss, ..Default::default() };
+    let a = export(&doc("a", 0.0), &opts);
+    let b = export(&doc("b", 5.0), &opts);
+    let (ia, ib) = (ids(&a), ids(&b));
+    assert_eq!(ia.len(), ia.iter().collect::<HashSet<_>>().len(), "unique within a document");
+    assert!(ia.iter().all(|i| !ib.contains(i)), "{ia:?} vs {ib:?}");
+    // CSS class names are page-global too.
+    let classes = |s: &str| s.split("class=\"").skip(1).map(|s| s[..s.find('"').unwrap()].to_string()).collect::<HashSet<_>>();
+    assert!(classes(&a).is_disjoint(&classes(&b)));
+    // The same document gets the same ids, and the names still come back.
+    assert_eq!(a, export(&doc("a", 0.0), &opts));
+    let back = import(&a).unwrap();
+    assert_eq!(back.layers[0].name.as_deref(), Some("Layer 1"));
+    assert!(names(&back).contains(&"Logo mark (v2)".to_string()));
+}
+
+#[test]
+fn label_attributes_name_objects() {
+    let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape" xmlns:serif="http://www.serif.com/" width="100" height="100">
+        <g id="layer1" inkscape:label="Background">
+            <rect id="r1" data-name="Sky box" x="0" y="0" width="10" height="10"/>
+            <rect id="r2" serif:id="Sun disc" x="20" y="0" width="10" height="10"/>
+            <rect id="r3" aria-label="Badge" x="40" y="0" width="10" height="10"/>
+            <rect id="plain" x="60" y="0" width="10" height="10"/>
+        </g>
+    </svg>"##;
+    let d = import(svg).unwrap();
+    assert_eq!(d.layers[0].name.as_deref(), Some("Background"));
+    assert_eq!(names(&d), ["Background", "Sky box", "Sun disc", "Badge", "plain"]);
+}

crates/svg/tests/options.rs

@@ -0,0 +1,77 @@
+//! SVG Options across each other: every styling, id mode and tspan mode reads back, and linked
+//! images are named after their bytes.
+// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md › Robustness).
+#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)]
+
+use std::sync::Arc;
+
+use vectorcraft_color::{Color, Paint};
+use vectorcraft_doc::{Appearance, AppearanceItem, CharStyle, Document, ImageBlob, ImageObject, Node, NodeKind, TextKind, TextObject, TextRun};
+use vectorcraft_geom::{Affine, Point, Rect, shapes};
+use vectorcraft_svg::{ExportOptions, ImageMode, ObjectIds, Styling, export, export_full, import};
+
+/// A red rectangle with an awkward name, point type and area type of two paragraphs each.
+fn doc() -> Document {
+    let mut d = Document::new(300.0, 200.0);
+    let l = d.layers[0].id;
+    let red = Appearance::basic(Paint::solid(Color::rgb8(255, 0, 0)), Paint::solid(Color::BLACK), 2.0);
+    let mut a = Node::path(d.alloc_id(), shapes::rectangle(Rect::new(10.0, 10.0, 60.0, 60.0)), red);
+    a.name = Some("A \"quoted\" & <odd> 100%".into());
+    d.insert(Some(l), usize::MAX, a).unwrap();
+    let st = CharStyle { size: 12.0, font_family: "Source Sans 3".into(), fill: Paint::solid(Color::BLACK), ..CharStyle::default() };
+    let mut t = TextObject::point(Point::new(20.0, 120.0), "", st.clone());
+    t.runs = vec![TextRun { text: "one two\nthree".into(), style: st.clone() }];
+    let i = d.alloc_id();
+    d.insert(Some(l), usize::MAX, Node::new(i, NodeKind::Text(Box::new(t.clone())))).unwrap();
+    t.kind = TextKind::Area { frame: shapes::rectangle(Rect::new(0.0, 0.0, 200.0, 60.0)) };
+    t.xf = Affine::translate((20.0, 140.0));
+    t.runs = vec![TextRun { text: "para one\npara two".into(), style: st }];
+    let i = d.alloc_id();
+    d.insert(Some(l), usize::MAX, Node::new(i, NodeKind::Text(Box::new(t)))).unwrap();
+    d
+}
+
+#[test]
+fn every_combination_reads_back() {
+    let d = doc();
+    for styling in [Styling::PresentationAttributes, Styling::InlineStyle, Styling::StyleEntities, Styling::InternalCss] {
+        for object_ids in [ObjectIds::LayerNames, ObjectIds::Minimal, ObjectIds::Unique] {
+            for (fewer_tspans, minify) in [(false, false), (true, true)] {
+                let o = ExportOptions { styling, object_ids, fewer_tspans, minify, ..Default::default() };
+                let s = export(&d, &o);
+                let back = import(&s).unwrap_or_else(|e| panic!("{o:?}: {e}\n{s}"));
+                let mut red = false;
+                back.walk(|n| {
+                    red |= n.appearance.items.iter().any(|i| matches!(i, AppearanceItem::Fill(f) if f.paint == Paint::solid(Color::rgb8(255, 0, 0))));
+                });
+                assert!(red, "{o:?}: the fill came back\n{s}");
+                // Paragraph breaks are new lines, never characters in a line.
+                assert!(!s.contains("\n</tspan>") && !s.contains("two\n"), "{o:?}\n{s}");
+            }
+        }
+    }
+}
+
+/// A document with one embedded image under `key`.
+fn image_doc(key: &str, bytes: Vec<u8>) -> Document {
+    let mut d = Document::new(100.0, 100.0);
+    d.images.insert(key.into(), ImageBlob { mime: "image/png".into(), bytes: Arc::new(bytes) });
+    let l = d.layers[0].id;
+    let im = ImageObject { key: key.into(), width: 2, height: 2, xf: Affine::IDENTITY, link: None };
+    let n = Node::new(d.alloc_id(), NodeKind::Image(im));
+    d.insert(Some(l), usize::MAX, n).unwrap();
+    d
+}
+
+#[test]
+fn linked_images_are_named_after_their_bytes() {
+    let link = ExportOptions { images: ImageMode::Link, ..Default::default() };
+    // Keys such as "raster-1" repeat across documents: two exports into one folder must not clash.
+    let a = export_full(&image_doc("raster-1", vec![1, 2, 3]), &link, None);
+    let b = export_full(&image_doc("raster-1", vec![4, 5, 6]), &link, None);
+    assert_eq!((a.linked.len(), b.linked.len()), (1, 1));
+    assert_ne!(a.linked[0].name, b.linked[0].name);
+    assert!(a.linked[0].name.ends_with(".png") && a.svg.contains(&format!("href=\"{}\"", a.linked[0].name)), "{}", a.svg);
+    // The same bytes under another key are the same file.
+    assert_eq!(export_full(&image_doc("other", vec![1, 2, 3]), &link, None).linked[0].name, a.linked[0].name);
+}

crates/svg/tests/roundtrip.rs

@@ -91,10 +91,11 @@ fn groups_and_names_roundtrip() {
     assert!(s.contains("id=\"My_Group\"") && s.contains("id=\"Box_B\"") && s.contains("id=\"Layer_1\""), "{s}");
     let r = roundtrip(&d);
     assert_eq!(r.node_count(), d.node_count());
-    assert_eq!(r.layers[0].name.as_deref(), Some("Layer_1"));
+    // data-name carries the names back exactly.
+    assert_eq!(r.layers[0].name.as_deref(), Some("Layer 1"));
     let a = art(&r);
     assert!(matches!(a[0].kind, NodeKind::Group { clip: false, .. }));
-    assert_eq!(a[0].name.as_deref(), Some("My_Group"));
+    assert_eq!(a[0].name.as_deref(), Some("My Group"));
     assert!(close_rect(a[0].geometric_bounds().unwrap(), Rect::new(0.0, 0.0, 30.0, 10.0), 0.01));
 }