collective
Browse / github.com/storytold/pdfcraft / patch
fix(edit, a11y): `Q` inside a text object restores the text matrices, as the renderer does
fix(edit, a11y): `Q` inside a text object restores the text matrices, as the renderer does ## What this changes Follow-up offered in #4831, same bug class. Inside a text object, `q`/`Q` save and restore the text matrix and the text line matrix as well (ISO 32000-2 §9.4.1, referenced from §8.4.2, as corrected by the PDF Association's erratum, pdf-association/pdf-issues #368). The renderer already does this: hayro's saved state holds both matrices. Two more interpreters carried the matrices across `Q` and so disagreed with what is drawn: - **Text editing** (`crates/edit/src/text/interp.rs`). Text after `Q` was listed at the position set inside `q … Q`, and editing it moved it ther
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- KenM76 (imported from pull request #4906, open)
- Base
1dd2737b47a9c1c9562f0af563053d1bf510e4bf- Size
- +48 −8 in 4 files
- Review
- pending
- Shared
- 2026-10-28
- Id
1bb036f10d6734cfc0e92dbc
How it went
No other operator has reported using it yet.
Diff
raw · crates/a11y/src/headings.rs crates/a11y/src/tests.rs crates/edit/src/tests.rs crates/edit/src/text/interp.rs
@@ -188,7 +188,10 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap<i64, String> { // Where each id's last glyph ended, in user space. let mut ends: HashMap<i64, (f64, f64)> = HashMap::new(); let mut gs = TextState::default();- let mut saved: Vec<TextState> = Vec::new();+ // With each saved state, the text and text line matrices: inside a text object `q`/`Q`+ // save and restore them too (ISO 32000-2 §9.4.1, as corrected by the PDF Association's+ // errata, issue 368), as the renderer does.+ let mut saved: Vec<(TextState, Matrix, Matrix)> = 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 {@@ -209,12 +212,12 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap<i64, String> { b"EMC" => { stack.pop(); }- b"q" if saved.len() < MAX_SAVED => saved.push(gs.clone()),+ b"q" if saved.len() < MAX_SAVED => saved.push((gs.clone(), tm, tlm)), b"q" => unsaved += 1, b"Q" if unsaved > 0 => unsaved -= 1, b"Q" => { if let Some(saved) = saved.pop() {- gs = saved;+ (gs, tm, tlm) = saved; } } b"cm" => {@@ -1235,6 +1235,19 @@ fn headings_drawn_glyph_by_glyph_keep_their_words() { ); } +/// Inside a text object `q`/`Q` save and restore the text matrices (ISO 32000-2 §9.4.1, as+/// corrected by the PDF Association's errata, issue 368), as the renderer does: a word whose+/// glyphs continue after a `Q` stays one word.+#[test]+fn q_inside_a_text_object_restores_the_text_matrices() {+ let doc = heading_page(&[+ "BT /F1 20 Tf 72 700 Td (Chap) Tj q 0 -100 Td Q (ter One) Tj ET".into(),+ "BT /F1 20 Tf 72 650 Td (Chap) Tj q 1 0 0 1 300 300 Tm Q (ter Two) Tj ET".into(),+ ]);+ let titles: Vec<String> = headings(&doc).into_iter().map(|h| h.title).collect();+ assert_eq!(titles, ["Chapter One", "Chapter Two"]);+}+ #[test] fn an_untagged_document_has_no_headings() { let mut objs = headings_doc();@@ -7457,3 +7457,24 @@ fn a_run_of_spaces_in_a_subset_without_a_space_glyph_keeps_its_width() { let content = String::from_utf8_lossy(&page_content_bytes(&reopen(&doc), 0)).into_owned(); assert_eq!(content.matches("[-330] TJ").count(), 5, "{content}"); }++/// Inside a text object `q`/`Q` save and restore the text and text line matrices (ISO 32000-2+/// §9.4.1, as corrected by the PDF Association's errata, issue 368), as the renderer does: the+/// text after `Q` is found, and stays, where it is drawn.+#[test]+fn q_inside_a_text_object_restores_the_text_matrices() {+ for (content, text, baseline) in [+ ("BT /F1 12 Tf 72 700 Td q 0 -100 Td Q (After) Tj ET", "After", [72.0, 700.0]),+ ("BT /F1 12 Tf 72 700 Td q 1 0 0 1 300 300 Tm Q (After) Tj ET", "After", [72.0, 700.0]),+ // `T*` moves from the restored line, not the one set inside q/Q.+ ("BT /F1 12 Tf 14 TL 72 700 Td q 0 -100 Td Q T* (Next) Tj ET", "Next", [72.0, 686.0]),+ ] {+ let mut doc = text_page(content);+ let lines = text::text_lines(&doc, 0).unwrap();+ assert_eq!((lines[0].text.as_str(), lines[0].baseline()), (text, baseline), "{content}");+ text::replace_line(&mut doc, 0, 0, "Edited").unwrap();+ let doc = reopen(&doc);+ let lines = text::text_lines(&doc, 0).unwrap();+ assert_eq!((lines[0].text.as_str(), lines[0].baseline()), ("Edited", baseline), "{content} after editing");+ }+}@@ -425,7 +425,10 @@ fn positioned_tracking(shown: &mut [Shown]) { /// stream), an unbalanced `q`, the font and the colour still apply in the streams after it. pub(super) struct Carry { pub(super) ts: Ts,- pub(super) stack: Vec<Ts>,+ /// Each saved state with the text and text line matrices: inside a text object `q`/`Q`+ /// save and restore them too (ISO 32000-2 §9.4.1, as corrected by the PDF Association's+ /// errata, issue 368), as the renderer does.+ pub(super) stack: Vec<(Ts, Matrix, Matrix)>, pub(super) marked: Vec<Option<usize>>, /// For each open marked-content sequence, its optional content group when it is `/OC`. pub(super) layers: Vec<Option<Vec<u8>>>,@@ -487,7 +490,7 @@ pub(super) fn interpret( let mut out: Vec<Shown> = Vec::new(); let mut ts = carry.ts.clone(); let mut at_bt = (0usize, ts.clone());- let mut stack: Vec<Ts> = std::mem::take(&mut carry.stack);+ let mut stack: Vec<(Ts, Matrix, Matrix)> = std::mem::take(&mut carry.stack); let (mut tm, mut tlm) = (Matrix::IDENTITY, Matrix::IDENTITY); let mut bt = 0usize; let mut marked = std::mem::take(&mut carry.marked);@@ -551,10 +554,10 @@ pub(super) fn interpret( } } }- b"q" => stack.push(ts.clone()),+ b"q" => stack.push((ts.clone(), tm, tlm)), b"Q" => {- if let Some(s) = stack.pop() {- ts = s;+ if let Some(saved) = stack.pop() {+ (ts, tm, tlm) = saved; } } b"cm" => {