From 1d7a9f9ffe45247dbc20a2047a4d2b151cb8385c Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 15:51:22 -0500 Subject: [PATCH] fix(edit,a11y): font aliases share the metrics built for their font (#5998) * fix(edit,a11y): font aliases share the metrics built for their font Text extraction and heading extraction built and kept font metrics per resource name, so many names for one font object each held a copy of its Unicode and width tables. * fmt * address review: font metrics are cached by the object at the end of the reference chain --- crates/a11y/src/headings.rs | 29 ++++++++++++++++++++++++++--- crates/edit/src/tests.rs | 33 +++++++++++++++++++++++++++++++++ crates/edit/src/text.rs | 2 ++ crates/edit/src/text/interp.rs | 22 +++++++++++++++++++++- 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/crates/a11y/src/headings.rs b/crates/a11y/src/headings.rs index 235ba4b452..a9de970b65 100644 --- a/crates/a11y/src/headings.rs +++ b/crates/a11y/src/headings.rs @@ -206,7 +206,9 @@ fn sub(doc: &Document, d: &Dict, key: &[u8]) -> Dict { pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { let res = sub(doc, page, b"Resources"); let (fonts, props) = (sub(doc, &res, b"Font"), sub(doc, &res, b"Properties")); - let mut metrics: HashMap, Metrics> = HashMap::new(); + let mut metrics: HashMap, std::rc::Rc> = HashMap::new(); + // Built once per font object, however many resource names alias it. + let mut by_font: HashMap> = HashMap::new(); // The id each open marked-content sequence is in: its own, else the enclosing one's. let mut stack: Vec> = Vec::new(); let mut out: HashMap = HashMap::new(); @@ -255,9 +257,30 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { gs.font = op.name(0).map(<[u8]>::to_vec); if let Some(name) = op.name(0) && !metrics.contains_key(name) - && let Some(f) = fonts.get(name).map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()) { - metrics.insert(name.to_vec(), Metrics::from_dict(doc, &f)); + // Keyed by the object at the end of the reference chain, so names that reach one + // font through different chains share its metrics (the hop limit is resolve's). + let font = fonts.get(name).and_then(|o| { + let (mut cur, mut last) = (o.clone(), None); + for _ in 0..16 { + let Object::Ref(r) = cur else { break }; + last = Some(r); + cur = (*doc.get(r)).clone(); + } + last + }); + let built = match font.and_then(|r| by_font.get(&r)) { + Some(m) => Some(m.clone()), + None => { + fonts.get(name).map(|f| doc.resolve(f)).and_then(|f| f.as_dict().map(|d| std::rc::Rc::new(Metrics::from_dict(doc, d)))) + } + }; + if let Some(m) = built { + if let Some(r) = font { + by_font.insert(r, m.clone()); + } + metrics.insert(name.to_vec(), m); + } } } b"Tc" => gs.char_spacing = op.num(0).unwrap_or(0.0), diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 1f2b925b46..b956cb7079 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -14042,6 +14042,39 @@ fn review_inside_scope_group_move_matches_direct_transform_under_outer_scale() { } } +/// A page that names one font under 300 resource names builds its metrics once: each alias +/// used to build (and keep) a set of its own. +#[test] +fn font_aliases_share_one_set_of_metrics() { + let fonts: String = (0..300).map(|i| format!(" /A{i} 5 0 R")).collect(); + let content: String = format!("BT 72 700 Td {} ET", (0..300).map(|i| format!("/A{i} 12 Tf (x) Tj ")).collect::()); + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font <<{fonts} >> >> >>"), + format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offs = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offs.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let x = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offs { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{x}\n%%EOF\n", objs.len() + 1).as_bytes()); + let doc = Document::open(Arc::new(out)).unwrap(); + text::METRICS_BUILT.with(|n| n.set(0)); + let lines = text::text_lines(&doc, 0).unwrap(); + assert!(lines.iter().map(|l| l.text.matches('x').count()).sum::() >= 300, "{lines:?}"); + let built = text::METRICS_BUILT.with(std::cell::Cell::get); + assert!(built < 10, "metrics built {built} times for one font"); +} + /// Each `Tj`/`TJ` moves the text matrix by the width of its text. A run shown later in the same /// text object without a `Td` or `Tm` of its own (a raised run in another colour, a column after /// a `TJ` gap) is placed by those advances. Rewriting the paragraph before it must keep them, as diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index b58c41303f..eba8834c2b 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -54,6 +54,8 @@ pub(crate) use fallback::{SOURCE_SPACE_CALLS, take_source_character_measurements #[cfg(test)] use fallback::{add_type3_font, is_type3_fallback}; pub(crate) use fallback::{glyph_path, type3_font_dict, unique_font_name}; +#[cfg(test)] +pub(crate) use interp::METRICS_BUILT; use interp::{Origin, Paint, union}; #[cfg(test)] pub(crate) use lines::dropped_advances_within; diff --git a/crates/edit/src/text/interp.rs b/crates/edit/src/text/interp.rs index 0bc9373899..e628e017c4 100644 --- a/crates/edit/src/text/interp.rs +++ b/crates/edit/src/text/interp.rs @@ -664,6 +664,13 @@ impl Carry { /// The advance is an error message when the font's writing mode can't be read. type CachedFont = (Rc, Option, Result, String>); +#[cfg(test)] +thread_local! { + /// Font metrics built by [`interpret`] on this thread, for the test that keeps font aliases + /// sharing one set. + pub(crate) static METRICS_BUILT: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// A `Do` met while interpreting: its operator index, XObject name and the state there. type Invocation = (usize, Vec, Ts); @@ -976,6 +983,7 @@ pub(super) fn interpret( let fonts_res = super::resource_dict(doc, resources, b"Font", &mut held_fonts).unwrap_or(&none); let states = super::resource_dict(doc, resources, b"ExtGState", &mut held_states).unwrap_or(&none); let mut out: Vec = Vec::new(); + let mut by_font: HashMap = HashMap::new(); let mut ts = carry.ts.clone(); let mut at_bt = (0usize, ts.clone()); let mut stack: Vec<(Ts, Matrix, Matrix)> = std::mem::take(&mut carry.stack); @@ -1105,11 +1113,23 @@ pub(super) fn interpret( ts.size = op.num(1).unwrap_or(ts.size); if let Some(name) = op.name(0) { let (m, face, vertical) = cache.entry(name.to_vec()).or_insert_with(|| { + // Names that refer to one font object share what was built for it: a + // thousand aliases of one font are one set of metrics, not a thousand. + let font = fonts_res.get(name).and_then(Object::as_ref); + if let Some(built) = font.and_then(|r| by_font.get(&r)) { + return built.clone(); + } + #[cfg(test)] + METRICS_BUILT.with(|n| n.set(n.get() + 1)); let dict = fonts_res.get(name).and_then(|f| doc.resolve(f).as_dict().cloned()); let metrics = dict.as_ref().map_or_else(Metrics::fallback, |d| Metrics::from_dict(doc, d)); let vertical = dict.as_ref().map_or(Ok(None), |d| vertical_advance(doc, d).map_err(|e| e.to_string())); let face = dict.filter(is_type3_fallback).map(|_| metrics.base_font.clone()); - (Rc::new(metrics), face, vertical) + let built: CachedFont = (Rc::new(metrics), face, vertical); + if let Some(r) = font { + by_font.insert(r, built.clone()); + } + built }); ts.font = Some((name.to_vec(), m.clone())); ts.face = face.clone();