From b9f0f84ff879157330394f81595bb4a8bd6de01f Mon Sep 17 00:00:00 2001 From: Alex <270190108+CouchRiv@users.noreply.github.com> Date: Thu, 15 Oct 2026 02:28:01 -0400 Subject: [PATCH] fix(a11y): bookmarks from structure keep the words of glyph-by-glyph headings (#1899) * fix(a11y): bookmarks from structure keep the words of glyph-by-glyph headings Chromium (Skia) draws text one glyph at a time, each moved into place with its own Td. The marked text of a heading added a space at every Td, TD, T* and Tm, so New Bookmarks from Structure titled them "C h a p t e r T w o", and the real spaces between words were lost. On a 20-page tagged guide printed by Chromium, 25 of 61 titles came out spaced. mcid_texts now follows the text matrix (and the CTM, q/Q, Tc, Tw, Tz, TL) and decides word breaks from where the glyphs are: a glyph that starts on another line, more than 0.2 em after the previous glyph of the same marked content, or back over more than an em, starts a new word. Kerning and glyph-by-glyph placement no longer split words. Co-Authored-By: Claude Opus 5.5 * fix(a11y): headings set with a negative font size keep their words The em that decides word breaks was the font size times |Tz|, negative with a negative font size: every glyph then started a new word, and a heading in one Tj read "C h a p t e r O n e". The em is now |size x Tz|, and "ahead" follows the way glyphs advance in text space, backwards when the size or Tz is negative, so a TJ word space still splits words there as on main. Co-Authored-By: Claude Opus 5.5 * fix(a11y): keep wrapped headings separate under horizontal scaling Co-Authored-By: OpenAI Codex --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: OpenAI Codex --- crates/a11y/src/headings.rs | 153 +++++++++++++++++++++++++++++------ crates/a11y/src/tests.rs | 107 ++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 3 files changed, 238 insertions(+), 26 deletions(-) diff --git a/crates/a11y/src/headings.rs b/crates/a11y/src/headings.rs index 7a35deb469..96f99b9a9f 100644 --- a/crates/a11y/src/headings.rs +++ b/crates/a11y/src/headings.rs @@ -3,6 +3,7 @@ use std::collections::{HashMap, HashSet}; +use pdfcraft_content::Matrix; use pdfcraft_cos::{Dict, Document, ObjRef, Object}; use pdfcraft_fonts::pdf::Metrics; use pdfcraft_model::Page; @@ -171,13 +172,24 @@ fn sub(doc: &Document, d: &Dict, key: &[u8]) -> Dict { } /// The text each marked-content id shows on a page (text drawn directly in the page content). +/// +/// Word breaks come from where the glyphs are, as on the page: a glyph that starts on another +/// line, more than 0.2 em past the end of the previous glyph of the same id, or back over more +/// than an em from it, starts a new word (the em is the font size times |`Tz`|; see [`new_word`]). +/// Text-positioning operators alone don't break words: producers that place every glyph with its +/// own `Td` (Chromium) would otherwise read as "C h a p t e r". 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 font: Option> = None; let mut stack: Vec> = Vec::new(); let mut out: HashMap = HashMap::new(); + // Where each id's last glyph ended, in user space. + let mut ends: HashMap = HashMap::new(); + let mut gs = TextState::default(); + let mut saved: Vec = Vec::new(); + let mut unsaved = 0usize; + let (mut tm, mut tlm) = (Matrix::IDENTITY, Matrix::IDENTITY); for op in pdfcraft_content::parse(&page_content(doc, page)).ops { let id = stack.iter().rev().find_map(|m| *m); match op.op.as_slice() { @@ -192,8 +204,22 @@ fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { b"EMC" => { stack.pop(); } + b"q" if saved.len() < MAX_SAVED => saved.push(gs.clone()), + b"q" => unsaved += 1, + b"Q" if unsaved > 0 => unsaved -= 1, + b"Q" => { + if let Some(saved) = saved.pop() { + gs = saved; + } + } + b"cm" => { + if let Some(m) = op.nums::<6>() { + gs.ctm = Matrix(m).then(&gs.ctm); + } + } b"Tf" => { - font = op.name(0).map(<[u8]>::to_vec); + gs.size = op.num(1).unwrap_or(gs.size); + 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()) @@ -201,34 +227,113 @@ fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { metrics.insert(name.to_vec(), Metrics::from_dict(doc, &f)); } } - _ => {} - } - let (Some(id), Some(m)) = (id, font.as_ref().and_then(|f| metrics.get(f))) else { continue }; - let text = out.entry(id).or_default(); - if text.len() >= MAX_TITLE * 4 { - continue; - } - match op.op.as_slice() { - b"Tj" | b"'" | b"\"" => { - if op.op != b"Tj" { - text.push(' '); - } - if let Some(s) = op.operands.iter().rev().find_map(Object::as_string) { - text.push_str(&m.decode(&s.bytes)); + b"Tc" => gs.char_spacing = op.num(0).unwrap_or(0.0), + b"Tw" => gs.word_spacing = op.num(0).unwrap_or(0.0), + b"Tz" => gs.scale = op.num(0).unwrap_or(100.0) / 100.0, + b"TL" => gs.leading = op.num(0).unwrap_or(0.0), + b"BT" => (tm, tlm) = (Matrix::IDENTITY, Matrix::IDENTITY), + b"Td" | b"TD" => { + if let Some([x, y]) = op.nums::<2>() { + if op.op == b"TD" { + gs.leading = -y; + } + tlm = Matrix::translate(x, y).then(&tlm); + tm = tlm; } } - b"TJ" => { - for item in op.operands.first().and_then(Object::as_array).into_iter().flatten() { - match item { - Object::String(s) => text.push_str(&m.decode(&s.bytes)), - n if n.as_f64().is_some_and(|n| n < -200.0) => text.push(' '), - _ => {} + b"Tm" => { + if let Some(m) = op.nums::<6>() { + (tm, tlm) = (Matrix(m), Matrix(m)); + } + } + b"T*" => { + tlm = Matrix::translate(0.0, -gs.leading).then(&tlm); + tm = tlm; + } + _ => {} + } + let shows = matches!(op.op.as_slice(), b"Tj" | b"TJ" | b"'" | b"\""); + let Some(m) = gs.font.as_ref().and_then(|f| metrics.get(f)).filter(|_| shows) else { continue }; + if op.op != b"Tj" && op.op != b"TJ" { + if op.op == b"\"" { + gs.word_spacing = op.num(0).unwrap_or(gs.word_spacing); + gs.char_spacing = op.num(1).unwrap_or(gs.char_spacing); + } + tlm = Matrix::translate(0.0, -gs.leading).then(&tlm); + tm = tlm; + } + let pieces: Vec<&Object> = match op.op.as_slice() { + b"TJ" => op.operands.first().and_then(Object::as_array).map(|a| a.iter().collect()).unwrap_or_default(), + _ => op.operands.last().into_iter().collect(), + }; + for piece in pieces { + let s = match piece { + Object::String(s) => s, + other => { + // A TJ adjustment moves the next glyph; whether it breaks a word is decided there. + let dx = -other.as_f64().unwrap_or(0.0) / 1000.0 * gs.size * gs.scale; + tm = Matrix::translate(dx, 0.0).then(&tm); + continue; + } + }; + for (code, len) in m.codes(&s.bytes) { + let trm = tm.then(&gs.ctm); + if let Some(id) = id + && let Some(glyph) = m.text_of(code) + { + let text = out.entry(id).or_default(); + if text.len() < MAX_TITLE * 4 { + let breaks = ends.get(&id).is_some_and(|end| new_word(&trm, *end, gs.size, gs.scale)); + if breaks && !text.is_empty() && !text.ends_with(' ') && !glyph.starts_with(' ') { + text.push(' '); + } + text.push_str(&glyph); } } + let advance = m.width(code) * gs.size + gs.char_spacing + if m.is_space(code, len) { gs.word_spacing } else { 0.0 }; + tm = Matrix::translate(advance * gs.scale, 0.0).then(&tm); + if let Some(id) = id { + ends.insert(id, tm.then(&gs.ctm).apply(0.0, 0.0)); + } } - b"Td" | b"TD" | b"T*" | b"Tm" => text.push(' '), - _ => {} } } out } + +/// How deep `q` nests before the text state stops being saved (a hostile stream can nest millions). +const MAX_SAVED: usize = 256; + +/// The graphics state that places glyphs: the CTM, the font and the text state parameters. +#[derive(Clone)] +struct TextState { + ctm: Matrix, + font: Option>, + size: f64, + char_spacing: f64, + word_spacing: f64, + scale: f64, + leading: f64, +} + +impl Default for TextState { + fn default() -> Self { + TextState { ctm: Matrix::IDENTITY, font: None, size: 0.0, char_spacing: 0.0, word_spacing: 0.0, scale: 1.0, leading: 0.0 } + } +} + +/// Does a glyph drawn with text rendering matrix `trm` (text space → user space) start a new word +/// after a glyph that ended at `end` (user space)? Yes on another line (more than half the absolute +/// font size up or down), more than 0.2 horizontal em ahead, or back over more than a horizontal em. +/// The horizontal em is the absolute value of font size times horizontal scale (`Tz`); "ahead" follows +/// the glyph advance, backwards when the size or `Tz` is negative. `Tz` never scales the vertical threshold. +fn new_word(trm: &Matrix, end: (f64, f64), font_size: f64, horizontal_scale: f64) -> bool { + let advance = font_size * horizontal_scale; + let em = advance.abs(); + let start = trm.apply(0.0, 0.0); + let [a, b, c, d, _, _] = trm.0; + let Some(back) = Matrix([a, b, c, d, 0.0, 0.0]).invert() else { return true }; + let (dx, dy) = back.apply(start.0 - end.0, start.1 - end.1); + let ahead = if advance < 0.0 { -dx } else { dx }; + !(ahead.is_finite() && dy.is_finite()) || dy.abs() > font_size.abs() * 0.5 || ahead > em * 0.2 || ahead < -em +} diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index 6e23c5c6a0..b0ace2c034 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -673,6 +673,105 @@ fn headings_are_found_with_their_levels_titles_and_pages() { ); } +/// Helvetica advances (standard 14 metrics, per 1000 em) of the characters the tests below draw. +fn helvetica_width(c: char) -> f64 { + match c { + 'C' | 'w' => 722.0, + 'P' => 667.0, + 'T' | 'F' => 611.0, + 'v' => 500.0, + 'r' => 333.0, + 't' | ' ' => 278.0, + 'i' => 222.0, + _ => 556.0, // a e h o p + } +} + +/// `text` drawn one glyph at a time from (72, y), each glyph moved into place with its own `Td`, +/// as Chromium writes text. With `spaces`, the space is drawn too; without, the next glyph is +/// just moved past it. +fn glyph_by_glyph(text: &str, size: f64, y: f64, spaces: bool) -> String { + let mut ops = format!("BT /F1 {size} Tf 72 {y} Td"); + let mut step = 0.0; + for c in text.chars() { + if c == ' ' && !spaces { + step += helvetica_width(c) / 1000.0 * size; + continue; + } + ops.push_str(&format!(" {step:.3} 0 Td ({c}) Tj")); + step = helvetica_width(c) / 1000.0 * size; + } + ops + " ET" +} + +/// `text` drawn one glyph at a time with `/F1 1 Tf` and a `Tm` per glyph scaled to `size`; the +/// space is skipped over, not drawn. +fn glyph_by_tm(text: &str, size: f64, y: f64) -> String { + let mut ops = "BT /F1 1 Tf".to_string(); + let mut x = 72.0; + for c in text.chars() { + if c != ' ' { + ops.push_str(&format!(" {size} 0 0 {size} {x:.3} {y} Tm ({c}) Tj")); + } + x += helvetica_width(c) / 1000.0 * size; + } + ops + " ET" +} + +/// One tagged page whose H1 headings show `contents` (one marked-content sequence each). +fn heading_page(contents: &[String]) -> Document { + let marked: Vec = contents.iter().enumerate().map(|(i, c)| format!("/H1 << /MCID {i} >> BDC {c} EMC")).collect(); + let kids: Vec = (0..contents.len()).map(|i| format!("{} 0 R", 7 + i)).collect(); + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /StructTreeRoot 5 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R /Resources << /Font << /F1 6 0 R >> >> >>".into(), + stream(&marked.join("\n")), + format!("<< /Type /StructTreeRoot /K [{}] >>", kids.join(" ")), + FONT.into(), + ]; + objs.extend((0..contents.len()).map(|i| format!("<< /S /H1 /Pg 3 0 R /K {i} >>"))); + pdf(&objs, "") +} + +/// Chromium (Skia) places each glyph with its own `Td`: a heading must read as its words, not as +/// "C h a p t e r T w o", whether the space is drawn or only skipped over. A heading set with a +/// negative font size or `Tz` keeps its words too. +#[test] +fn headings_drawn_glyph_by_glyph_keep_their_words() { + let doc = heading_page(&[ + "BT /F1 20 Tf 72 700 Td (Chapter One) Tj ET".into(), + glyph_by_glyph("Chapter Two", 20.0, 650.0, true), + glyph_by_glyph("Chapter Three", 20.0, 600.0, false), + // Each glyph placed with its own Tm, at font size 1 scaled by the matrix. + glyph_by_tm("Part Five", 18.0, 550.0), + // Kerning inside a word, then a word space as a TJ adjustment, then a wrapped line. + "BT /F1 16 Tf 72 450 Td [(Ch) 40 (apter) -350 (Four)] TJ 0 -20 Td (continued) Tj ET".into(), + // A negative font size runs the glyphs backwards in text space: alone, in a mirrored + // text matrix (upright on the page), with a word space as a TJ adjustment, and the same + // with a negative horizontal scale instead (reset after, as Tz outlives the text object). + "BT /F1 -20 Tf 72 380 Td (Chapter Six) Tj ET".into(), + "BT /F1 -20 Tf -1 0 0 -1 300 330 Tm (Chapter Seven) Tj ET".into(), + "BT /F1 -20 Tf 72 280 Td [(Chapter) -300 (Eight)] TJ ET".into(), + "BT /F1 20 Tf -100 Tz 300 230 Td [(Chapter) -300 (Nine)] TJ ET 100 Tz".into(), + ]); + let titles: Vec = headings(&doc).into_iter().map(|h| h.title).collect(); + assert_eq!( + titles, + [ + "Chapter One", + "Chapter Two", + "Chapter Three", + "Part Five", + "Chapter Four continued", + "Chapter Six", + "Chapter Seven", + "Chapter Eight", + "Chapter Nine" + ] + ); +} + #[test] fn an_untagged_document_has_no_headings() { let mut objs = headings_doc(); @@ -769,3 +868,11 @@ fn a_nested_finding_says_itself() { // Both wordings are translated, each on its own, and the document's words are left alone. assert_eq!(f.said(&|w: &str| format!("«{w}»")), "«Table element on page 3: «rows have between 1 and 2 columns»»"); } + +/// Horizontal stretching must not raise the vertical line-break threshold. +#[test] +fn horizontally_scaled_wrapped_heading_keeps_a_space_between_lines() { + let doc = heading_page(&["BT /F1 20 Tf 400 Tz 72 700 Td (I) Tj 0 -30 Td (Overview) Tj ET".into()]); + let titles: Vec = headings(&doc).into_iter().map(|h| h.title).collect(); + assert_eq!(titles, ["I Overview"]); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 2864e37e0a..4900ffc77b 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3328,8 +3328,8 @@ milestone = "M4" status = "shipped" commands = ["bookmark.from_structure"] tools = ["bookmark_from_structure"] -evidence = ["crates/a11y/src/tests.rs::headings_are_found_with_their_levels_titles_and_pages", "crates/organize/src/tests.rs::a_bookmark_tree_nests_entries_by_level_under_a_new_first_bookmark", "crates/engine/src/tests.rs::bookmarks_from_structure_nest_the_headings_and_undo", "crates/automation/tests/automation.rs::bookmarks_from_structure_through_tools", "crates/ui-egui/tests/bookmarks.rs::new_bookmarks_from_structure_nest_the_tagged_headings"] -notes = "Bookmark options ▸ New bookmarks from structure: tagged headings (H1–H6 through the role map; a plain H takes its Sect/Part/Art nesting) become bookmarks nested by level under a new first \"Untitled\" bookmark, each linked to its element (/SE). Titles come from /ActualText, the marked text drawn directly on the page (not inside Form XObjects), /Alt or /T. Not yet: Acrobat's choice of element types (only headings), and destinations go to the heading's page rather than its position on it" +evidence = ["crates/a11y/src/tests.rs::headings_are_found_with_their_levels_titles_and_pages", "crates/a11y/src/tests.rs::headings_drawn_glyph_by_glyph_keep_their_words", "crates/a11y/src/tests.rs::horizontally_scaled_wrapped_heading_keeps_a_space_between_lines", "crates/organize/src/tests.rs::a_bookmark_tree_nests_entries_by_level_under_a_new_first_bookmark", "crates/engine/src/tests.rs::bookmarks_from_structure_nest_the_headings_and_undo", "crates/automation/tests/automation.rs::bookmarks_from_structure_through_tools", "crates/ui-egui/tests/bookmarks.rs::new_bookmarks_from_structure_nest_the_tagged_headings"] +notes = "Bookmark options ▸ New bookmarks from structure: tagged headings (H1–H6 through the role map; a plain H takes its Sect/Part/Art nesting) become bookmarks nested by level under a new first \"Untitled\" bookmark, each linked to its element (/SE). Titles come from /ActualText, the marked text drawn directly on the page (not inside Form XObjects; words split where the glyphs are apart, so text placed glyph by glyph, as Chromium writes it, keeps its words; vertical line breaks use the absolute font size independently of Tz), /Alt or /T. Not yet: Acrobat's choice of element types (only headings), and destinations go to the heading's page rather than its position on it" [[feature]] id = "organize.attachments-add"