fix(svg): first element of an id, shared linked file reads, bounded blank lines

crates/svg/src/import/depth.rs

@@ -220,7 +220,14 @@ fn prescan_with(svg: &str, most: usize) -> Prescan<'_> {
 /// `most`. Without recursion.
 pub(super) fn height(xml: &roxmltree::Document, most: usize) -> Option<usize> {
     let elements = || xml.descendants().filter(|n| n.is_element());
-    let ids: HashMap<&str, XNode> = elements().filter_map(|n| Some((n.attribute("id")?, n))).collect();
+    // The first element of an id, as usvg resolves references: a later one of the same id would
+    // hide a deep target behind a shallow one.
+    let mut ids: HashMap<&str, XNode> = HashMap::new();
+    for n in elements() {
+        if let Some(id) = n.attribute("id") {
+            ids.entry(id).or_insert(n);
+        }
+    }
     let css = Styles::new(xml);
     // How deep each element's own tree goes, itself included; 0 while it is being worked out: a
     // reference back to it then is a loop, which usvg leaves out.
@@ -394,6 +401,16 @@ mod tests {
         assert!(!deep(&svg(&format!("<style>{rules}</style><defs>{defs}</defs>{art}"))));
     }
 
+    #[test]
+    fn a_reference_follows_the_first_element_of_its_id() {
+        // A chain of `<use>`s as deep as the cap, each link's id given again later to an empty group:
+        // usvg follows the first, so the chain is deep.
+        let chain: String = (1..MAX_ELEMENT_DEPTH).map(|i| format!("<g id='a{i}'><use href='#a{}'/></g>", i - 1)).collect();
+        let shallow: String = (1..MAX_ELEMENT_DEPTH).map(|i| format!("<g id='a{i}'/>")).collect();
+        let top = format!("<use href='#a{}'/>", MAX_ELEMENT_DEPTH - 1);
+        assert!(deep(&svg(&format!("<defs><g id='a0'><rect width='1' height='1'/></g>{chain}{shallow}</defs>{top}"))));
+    }
+
     #[test]
     fn a_loop_of_references_is_not_deep() {
         assert!(!deep(&svg("<g id='a'><use href='#b'/></g><g id='b'><use href='#a'/></g><clipPath id='c' clip-path='url(#c)'/>")));

crates/svg/src/import/files.rs

@@ -48,11 +48,12 @@ pub(in crate::import) struct Files {
 /// How usvg holds the bytes of a raster of one format (`ImageKind::PNG`…).
 type RasterKind = fn(Arc<Vec<u8>>) -> ImageKind;
 
+#[derive(Clone)]
 enum Source {
     /// A raster file of the kind usvg reads it as.
     Raster(RasterKind, Arc<Vec<u8>>),
     /// An SVG file's text (unpacked when it is SVGZ), parsed by usvg as an image.
-    Svg(String),
+    Svg(Arc<str>),
 }
 
 /// A linked raster image (or the placeholder of a missing one).
@@ -148,11 +149,21 @@ impl Files {
     pub(in crate::import) fn read(svg: &str, xml: &roxmltree::Document, opts: &ImportOptions, edits: &mut Edits, warnings: &mut Vec<String>) -> Self {
         let mut files = Files::default();
         let mut placeholder = None;
+        // What each file read gave, by the file: hrefs spelled differently (`a.png`, `./a.png`,
+        // `%61.png`) share one read and one copy of its bytes.
+        let mut by_file: HashMap<String, Source> = HashMap::new();
         for n in xml.descendants().filter(|n| n.is_element() && n.tag_name().name() == "image") {
             let Some(h) = href(n).filter(|h| !h.trim_start().starts_with("data:")) else { continue };
             if !files.sources.contains_key(h) {
                 let (path, file) = target(h, opts.folder);
-                let read = file.and_then(|file| opts.read.and_then(|read| read(&file)));
+                if let Some(read) = file.as_ref().and_then(|f| by_file.get(f)) {
+                    files.sources.insert(h.to_string(), read.clone());
+                    if files.placeholder(h) {
+                        edits.set(svg, n, "preserveAspectRatio", "none");
+                    }
+                    continue;
+                }
+                let read = file.clone().and_then(|file| opts.read.and_then(|read| read(&file)));
                 let source = match read {
                     Some((bytes, link)) => files.source(bytes, link).ok_or("is not an image VectorCraft reads"),
                     None => Err("not found"),
@@ -164,6 +175,9 @@ impl Files {
                     Ok::<_, &str>(Source::Raster(ImageKind::PNG, bytes))
                 });
                 let Ok(source) = source else { continue };
+                if let Some(file) = file {
+                    by_file.insert(file, source.clone());
+                }
                 files.sources.insert(h.to_string(), source);
             }
             if files.placeholder(h) {
@@ -179,7 +193,7 @@ impl Files {
         let Ok(format) = image::guess_format(&bytes) else {
             // Unpacked once, here, within what the SVGZ images may unpack to.
             let text = crate::text_within(&bytes, &self.unpacked).ok()?;
-            return text.contains("<svg").then(|| Source::Svg(text.into_owned()));
+            return text.contains("<svg").then(|| Source::Svg(Arc::from(text.as_ref())));
         };
         let Some(kind) = raster(format) else {
             let img = image::load_from_memory_with_format(&bytes, format).ok()?;

crates/svg/src/import/text.rs

@@ -44,6 +44,10 @@ const SVG_NS: &str = "http://www.w3.org/2000/svg";
 /// Most line breaks one absolutely positioned character adds before itself.
 const MAX_LINE_GAP: f64 = 10_000.0;
 
+/// Most line breaks the absolutely positioned characters of one text add in all: past it, a gap is
+/// one break as wide as the gap.
+const MAX_LINE_BREAKS: usize = 10_000;
+
 /// A whitespace/comma separated list.
 fn list(v: &str) -> impl Iterator<Item = &str> {
     v.split(|c: char| c.is_whitespace() || c == ',').filter(|t| !t.is_empty())
@@ -906,6 +910,7 @@ fn lines(
     // Line starts (first character, anchor x) and absolutely placed characters (index, x).
     let mut lines = vec![(0, x0)];
     let mut targets: Vec<(usize, f64)> = vec![];
+    let mut added = 0usize;
     for i in 1..n {
         let ny = y[i].unwrap_or(cur) + dy[i];
         let lead = cells[i].style.effective_leading();
@@ -914,7 +919,8 @@ fn lines(
             // Whole multiples of the auto leading add blank lines (not without limit: a far-off y
             // mustn't ask for billions); other spacing is one break.
             let count = (gap / lead).round().clamp(1.0, MAX_LINE_GAP);
-            let count = if (gap / count - lead).abs() <= lead * 0.05 { count } else { 1.0 };
+            let count = if (gap / count - lead).abs() <= lead * 0.05 && added + count as usize <= MAX_LINE_BREAKS { count } else { 1.0 };
+            added += count as usize;
             breaks.push(Break { at: i, count: count as usize, leading: gap / count });
             lines.push((i, ax + dx[i]));
             (base, cur) = (ny, ny);

crates/svg/tests/it/import_details.rs

@@ -88,6 +88,23 @@ fn a_linked_png_resolves_against_the_svg_folder_and_stays_linked() {
     assert!((b.x0 - 10.0).abs() < 1e-6 && (b.y0 - 20.0).abs() < 1e-6 && (b.width() - 80.0).abs() < 1e-6, "{b:?}");
 }
 
+#[test]
+fn hrefs_spelling_one_file_differently_read_it_once() {
+    let dir = folder("spellings");
+    std::fs::write(dir.join("img").join("a.png"), png(4, 4)).unwrap();
+    let reads = std::sync::atomic::AtomicUsize::new(0);
+    let counting = |path: &str| {
+        reads.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+        read(path)
+    };
+    let folder = dir.to_string_lossy();
+    let body = r#"<image href="img/a.png" width="4" height="4"/><image href="./img/a.png" width="4" height="4"/><image href="img/%61.png" width="4" height="4"/>"#;
+    let (d, w) = import_with(&svg(body), &ImportOptions { folder: Some(&folder), read: Some(&counting) }).unwrap();
+    assert!(w.is_empty(), "{w:?}");
+    assert_eq!(images(&d).len(), 3);
+    assert_eq!(reads.into_inner(), 1, "one read for the three spellings");
+}
+
 #[test]
 fn a_missing_link_warns_and_keeps_a_placeholder_in_its_box() {
     let dir = folder("missing");

crates/svg/tests/it/roundtrip.rs

@@ -789,6 +789,22 @@ fn import_tspan_far_below_is_bounded() {
     assert!(breaks <= 10_002, "{breaks} line breaks");
 }
 
+#[test]
+fn import_many_tspans_far_apart_are_bounded_together() {
+    // Each tspan 9,999 lines (auto leading 14.4 pt) below the last: each within the limit of one gap,
+    // two hundred of them were two million line breaks.
+    let spans: String = (1..=200).map(|i| format!(r#"<tspan x="5" y="{}">{i}</tspan>"#, 20.0 + 14.4 * 9_999.0 * i as f64)).collect();
+    let d = import_art(&format!(
+        r#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><text x="5" y="20" font-size="12">a{spans}</text></svg>"#
+    ));
+    let a = art(&d);
+    let NodeKind::Text(t) = &a[0].kind else { panic!() };
+    let text = t.plain_text();
+    assert!(text.ends_with("200"), "{}", &text[text.len().saturating_sub(20)..]);
+    let breaks = text.matches('\n').count();
+    assert!(breaks <= 10_200, "{breaks} line breaks");
+}
+
 #[test]
 fn import_tspan_on_the_same_baseline_stays_on_the_line() {
     // Styled runs positioned on the line's own baseline (as Illustrator writes them, with x and y on