diff --git a/crates/svg/src/import/depth.rs b/crates/svg/src/import/depth.rs index 2cdccc9da..0586deaf6 100644 --- a/crates/svg/src/import/depth.rs +++ b/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 { 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!("{defs}{art}")))); } + #[test] + fn a_reference_follows_the_first_element_of_its_id() { + // A chain of ``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!("", i - 1)).collect(); + let shallow: String = (1..MAX_ELEMENT_DEPTH).map(|i| format!("")).collect(); + let top = format!("", MAX_ELEMENT_DEPTH - 1); + assert!(deep(&svg(&format!("{chain}{shallow}{top}")))); + } + #[test] fn a_loop_of_references_is_not_deep() { assert!(!deep(&svg(""))); diff --git a/crates/svg/src/import/files.rs b/crates/svg/src/import/files.rs index 9de3bebcd..1a4c06f0e 100644 --- a/crates/svg/src/import/files.rs +++ b/crates/svg/src/import/files.rs @@ -49,11 +49,12 @@ pub(in crate::import) struct Files { /// How usvg holds the bytes of a raster of one format (`ImageKind::PNG`…). type RasterKind = fn(Arc>) -> ImageKind; +#[derive(Clone)] enum Source { /// A raster file of the kind usvg reads it as. Raster(RasterKind, Arc>), /// An SVG file's text (unpacked when it is SVGZ), parsed by usvg as an image. - Svg(String), + Svg(Arc), } /// A linked raster image (or the placeholder of a missing one). @@ -133,14 +134,29 @@ impl Files { pub(in crate::import) fn read(svg: &str, xml: &roxmltree::Document, opts: &ImportOptions, edits: &mut Edits, warnings: &mut Vec) -> 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> = 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))); - let source = match read { - Some((bytes, link)) => files.source(bytes, link).ok_or("is not an image VectorCraft reads"), - None => Err("not found"), + // A failed read is cached too, but each href gets its own placeholder: its own link + // path (an Arc shared between hrefs would share the first href's link) and warning. + let cached = file.as_ref().and_then(|f| by_file.get(f)).cloned(); + let source = match cached { + Some(source) => source, + None => { + 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"), + }; + if let Some(file) = file { + by_file.insert(file, source.clone()); + } + source + } }; let source = source.or_else(|why| { warnings.push(format!("linked image '{h}' {why}: a placeholder keeps its place until it is relinked")); @@ -164,7 +180,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(" impl Iterator { v.split(|c: char| c.is_whitespace() || c == ',').filter(|t| !t.is_empty()) @@ -1155,6 +1159,12 @@ fn justify_of(anchor: f64) -> Justify { } } +/// How far a line's leading may differ from its spacing and still count as it: 0.01 absolute, but +/// never more than a thousandth of the spacing itself (tiny type has tiny gaps). +fn leading_tolerance(spacing: f64) -> f64 { + 0.01_f64.min(spacing.abs() * 1e-3) +} + /// Point type: line breaks, baseline shifts and kerning from the positioning lists. Returns the /// justification, the x of the first line's left (or anchor) edge and, when no character is /// placed on its own, each line's first character and anchor x. @@ -1175,6 +1185,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(); @@ -1183,7 +1194,9 @@ 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 }; + // The first newline just ends the previous line: only the others are blank lines. + let count = if (gap / count - lead).abs() <= lead * 0.05 && added + (count as usize - 1) <= MAX_LINE_BREAKS { count } else { 1.0 }; + added += count as usize - 1; breaks.push(Break { at: i, count: count as usize, leading: gap / count }); lines.push((i, ax + dx[i])); (base, cur) = (ny, ny); @@ -1199,7 +1212,8 @@ fn lines( // A line's characters take its spacing as leading unless it is their auto leading. for (k, b) in breaks.iter().enumerate() { let end = breaks.get(k + 1).map_or(n, |nb| nb.at); - if cells[b.at..end].iter().any(|c| (c.style.effective_leading() - b.leading).abs() > 0.01) { + let tolerance = leading_tolerance(b.leading); + if cells[b.at..end].iter().any(|c| (c.style.effective_leading() - b.leading).abs() > tolerance) { for c in &mut cells[b.at..end] { c.style.leading = Some(b.leading); } @@ -1372,7 +1386,7 @@ fn assemble(cells: &[Cell], breaks: &[Break]) -> (Runs, Vec) { let key = (prev.fill, prev.stroke); push(&mut out, '\n', prev.final_style(), key); let mut blank = prev.final_style(); - if (blank.effective_leading() - b.leading).abs() > 0.01 { + if (blank.effective_leading() - b.leading).abs() > leading_tolerance(b.leading) { blank.leading = Some(b.leading); } for _ in 1..b.count { diff --git a/crates/svg/tests/it/import_details.rs b/crates/svg/tests/it/import_details.rs index 10418f344..d6983a1ce 100644 --- a/crates/svg/tests/it/import_details.rs +++ b/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#""#; + 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"); @@ -139,6 +156,26 @@ fn files_with_the_same_pixels_have_keys_of_their_own() { assert_eq!(*d.images[&b.key].bytes, bytes); } +#[test] +fn hrefs_spelling_one_missing_file_differently_each_warn_and_keep_their_own_link() { + let dir = folder("missing_twice"); + 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#""#; + let (d, w) = import_with(&svg(body), &ImportOptions { folder: Some(&folder), read: Some(&counting) }).unwrap(); + assert_eq!(w.iter().filter(|w| w.contains("gone.png") && w.contains("not found")).count(), 2, "{w:?}"); + assert_eq!(reads.into_inner(), 1, "the failed read is cached"); + let found = images(&d); + assert_eq!(found.len(), 2); + for (_, im) in found { + assert_eq!(Path::new(&im.link.as_ref().unwrap().path), dir.join("img").join("gone.png")); + } +} + #[test] fn svg_images_import_as_vector_art() { let dir = folder("svgimage"); diff --git a/crates/svg/tests/it/import_text.rs b/crates/svg/tests/it/import_text.rs index 35289a0c1..8b0d0338c 100644 --- a/crates/svg/tests/it/import_text.rs +++ b/crates/svg/tests/it/import_text.rs @@ -277,6 +277,25 @@ fn anchored_letter_spaced_lines_stay_where_a_reader_draws_them() { assert!(close(s[0], 200.0 - a[0] / 2.0, 1e-6) && close(s[1], 200.0 - a[1], 1e-6), "{s:?} {a:?}"); } +#[test] +fn a_small_line_gap_in_tiny_type_keeps_its_leading() { + // 0.0085 against an auto leading of 0.006 differs by less than 0.01 but is not a whole multiple of it. + let d = open(r#"OneTwo"#); + let t = only_text(&d); + let two = t.runs.iter().find(|r| r.text.contains("Two")).unwrap().style.leading; + assert!(two.is_some_and(|l| close(l, 0.0085, 1e-9)), "{two:?}"); +} + +#[test] +fn blank_lines_in_tiny_type_keep_their_own_spacing() { + // B sits 0.0085 below A (kept as its leading); C is three 0.006 lines below B, so the two blank + // lines between them must be 0.006 apart, not carry B's 0.0085. + let d = open(r#"ABC"#); + let t = only_text(&d); + let blank = t.runs.iter().find(|r| r.text == "\n\n").expect("a run of the two blank lines"); + assert!(blank.style.leading.is_some_and(|l| close(l, 0.006, 1e-9)), "{:?}", blank.style.leading); +} + #[test] fn tspan_lines_dx_and_baseline_shift() { let d = open( diff --git a/crates/svg/tests/it/roundtrip.rs b/crates/svg/tests/it/roundtrip.rs index b13ecf160..127e47846 100644 --- a/crates/svg/tests/it/roundtrip.rs +++ b/crates/svg/tests/it/roundtrip.rs @@ -909,6 +909,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#"{i}"#, 20.0 + 14.4 * 9_999.0 * i as f64)).collect(); + let d = import_art(&format!( + r#"a{spans}"# + )); + 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