From 5eae8d6e90c5af9dfdc54c73357b274b75fc76a2 Mon Sep 17 00:00:00 2001 From: Aman Mehtar Date: Tue, 20 Oct 2026 21:41:00 +0530 Subject: [PATCH 1/3] Edit text: keep a line together across subsets of the same font (#3661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Word and other writers split one font into several subsets, each a font resource of its own with its own six-letter tag: "Two-line" can be drawn as /F1 (Two) /F2 (-l) /F1 (ine) with F1 and F2 both Helvetica-Bold. `text_lines` ended a line wherever the resource name changed, so Edit text showed three lines and Word/HTML export wrote three paragraphs -- out of order when the subsets' boxes differ by a fraction of a point. A run now continues a line when its `/BaseFont` names the same font as the line's, ignoring the subset tag (ISO 32000-2 §9.6.4), as well as when it is the same resource or another subset of our Japanese/Hebrew fallback face. Block grouping in `text_regions` uses the same comparison, so the exporters see one block. Tests: the issue's two-resource case and Word's two-subset case read as one line and one block; two different fonts still split. Before the fix the first gave ["Hel", "lo"] and the second ["ab", "ca", "b"]. --- crates/edit/src/tests.rs | 37 +++++++++++++++++++++++++++++++-- crates/edit/src/text.rs | 27 ++++++++++++++++++------ crates/edit/src/text_regions.rs | 4 ++-- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 879ebc6c66..efde39a7f3 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -877,17 +877,23 @@ fn odd_arabic_text_never_panics() { } } -/// One page with Helvetica (WinAnsi) and a subset font that has only the glyphs it uses. +/// One page with Helvetica (WinAnsi) and subsets of Arial: `/F2` and `/F3` are two subsets of the +/// same font, as Word writes a font split across several resources, and `/F4` repeats Helvetica. fn text_page(content: &str) -> Document { let objs: Vec = vec![ "<< /Type /Catalog /Pages 2 0 R >>".into(), "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), - "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font << /F1 5 0 R /F2 6 0 R >> >> >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font << /F1 5 0 R /F2 6 0 R /F3 7 0 R /F4 8 0 R >> >> >>".into(), format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), // Subset: glyphs for a (97) and b (98) only. "<< /Type /Font /Subtype /TrueType /BaseFont /ABCDEF+Arial /FirstChar 97 /LastChar 99 /Widths [500 520 0] /Encoding /WinAnsiEncoding >>" .into(), + // Another subset of the same Arial, with a subset tag of its own. + "<< /Type /Font /Subtype /TrueType /BaseFont /GHIJKL+Arial /FirstChar 97 /LastChar 99 /Widths [500 520 0] /Encoding /WinAnsiEncoding >>" + .into(), + // Helvetica again, in a second resource. + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), ]; let mut out = b"%PDF-1.7\n".to_vec(); let mut offs = Vec::new(); @@ -1727,6 +1733,33 @@ fn word_boxes_span_whole_words_when_glyphs_run_backwards() { } } +/// PDF writers split one font into several subsets, each a font resource of its own: a word drawn +/// through two of them is still one visual line (#3661). +#[test] +fn one_line_continues_across_two_resources_of_the_same_font() { + for (content, text) in [ + // The issue's case: two resources for the same Helvetica. + ("BT /F1 12 Tf 72 700 Td (Hel) Tj /F4 12 Tf (lo) Tj ET", "Hello"), + // Word's case: two tagged subsets of one font, "abcab" through /F2, /F3, /F2. + ("BT /F2 12 Tf 72 700 Td (ab) Tj /F3 12 Tf (ca) Tj /F2 12 Tf (b) Tj ET", "abcab"), + ] { + let doc = text_page(content); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.len(), 1, "{content}: {:?}", lines.iter().map(|l| &l.text).collect::>()); + assert_eq!(lines[0].text, text, "{content}"); + let blocks = text::text_blocks(&doc, 0).unwrap(); + assert_eq!(blocks.len(), 1, "{content}"); + assert_eq!(blocks[0].text, text, "{content}"); + } +} + +/// Two genuinely different fonts still start a new line: the subset tag is all that may differ. +#[test] +fn different_fonts_start_a_new_line() { + let doc = text_page("BT /F1 12 Tf 72 700 Td (Hel) Tj /F2 12 Tf (lo) Tj ET"); + assert_eq!(text::text_lines(&doc, 0).unwrap().len(), 2); +} + #[test] fn text_lines_are_found_and_replaced_in_place() { let mut doc = diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 2672592178..1aa3e5840d 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -1125,7 +1125,7 @@ pub fn text_lines(doc: &Document, page: usize) -> Result, EditErro piece == si && lines.last().is_some_and(|l| l.scope == s.scope && l.logical_order == s.logical_order) && (bt == s.bt || lines.last().is_some_and(|l| compatible_word(l, &s))) - && (s.text.trim().is_empty() || lines.last().is_some_and(|l| same_font(l, &s.font, s.face.as_deref()))) + && (s.text.trim().is_empty() || lines.last().is_some_and(|l| same_font(l, &s.font, &s.base_font, s.face.as_deref()))) && (s.text.trim().is_empty() || lines.last().is_some_and(|l| { (s.size - l.size).abs() < 0.01 && s.bold == l.bold && s.italic == l.italic && fill_color(&s.state.fill) == l.color @@ -1220,7 +1220,7 @@ fn add_shown(lines: &mut Vec, last: &mut Option, s: Shown, stream let bt = pen.bt; lines.last().is_some_and(|l| l.scope == s.scope && l.logical_order == s.logical_order) && (bt == s.bt || lines.last().is_some_and(|l| compatible_word(l, &s))) - && (s.text.trim().is_empty() || lines.last().is_some_and(|l| same_font(l, &s.font, s.face.as_deref()))) + && (s.text.trim().is_empty() || lines.last().is_some_and(|l| same_font(l, &s.font, &s.base_font, s.face.as_deref()))) && (s.text.trim().is_empty() || lines.last().is_some_and(|l| { (s.size - l.size).abs() < 0.01 && s.bold == l.bold && s.italic == l.italic && fill_color(&s.state.fill) == l.color @@ -1456,10 +1456,25 @@ fn is_type3_fallback(font: &Dict) -> bool { && font.name(b"Name").is_some_and(|n| n == JAPANESE_FONT_NAME.as_bytes() || n == HEBREW_FONT_NAME.as_bytes()) } -/// Whether a run shown in `font` (a resource name, and the Japanese or Hebrew fallback face it subsets, if -/// any) continues `line`: the same font resource, or another subset of the same face. -fn same_font(line: &TextLine, font: &[u8], face: Option<&str>) -> bool { - line.font.as_bytes() == font || (line.face.is_some() && line.face.as_deref() == face) +/// Whether a run shown in `font` (a resource name), with `/BaseFont` `base_font` and the Japanese +/// or Hebrew fallback face it subsets, if any, continues `line`: the same font resource, another +/// resource for the same font (a writer may split one font into several subsets, #3661), or +/// another subset of the same face. +fn same_font(line: &TextLine, font: &[u8], base_font: &str, face: Option<&str>) -> bool { + line.font.as_bytes() == font || same_base_font(&line.base_font, base_font) || (line.face.is_some() && line.face.as_deref() == face) +} + +/// Whether two `/BaseFont` names name the same font: a subset's six-letter tag and `+` prefix do +/// not change the font (ISO 32000-2 §9.6.4), so `ABCDEF+Arial` and `GHIJKL+Arial` are one font. +fn same_base_font(a: &str, b: &str) -> bool { + !a.is_empty() && (a == b || strip_subset_tag(a) == strip_subset_tag(b)) +} + +/// `ABCDEF+Arial` → `Arial`. A name without a subset tag is returned unchanged. +fn strip_subset_tag(name: &str) -> &str { + let bytes = name.as_bytes(); + let is_tag = bytes.get(6) == Some(&b'+') && bytes.get(..6).is_some_and(|tag| tag.iter().all(u8::is_ascii_uppercase)); + if is_tag { name.get(7..).unwrap_or(name) } else { name } } fn source_family(base_font: &str) -> crate::added::Family { diff --git a/crates/edit/src/text_regions.rs b/crates/edit/src/text_regions.rs index e08068d9f5..cd3a258875 100644 --- a/crates/edit/src/text_regions.rs +++ b/crates/edit/src/text_regions.rs @@ -51,8 +51,8 @@ fn invisible(line: &TextLine) -> bool { } fn same_style(a: &TextLine, b: &TextLine) -> bool { - same_font(a, b.font.as_bytes(), b.face.as_deref()) - && a.base_font == b.base_font + same_font(a, b.font.as_bytes(), &b.base_font, b.face.as_deref()) + && same_base_font(&a.base_font, &b.base_font) && a.bold == b.bold && a.italic == b.italic && a.color == b.color From 4a618e8dcb6f941e6239e2494be4ba714c0b2ea5 Mon Sep 17 00:00:00 2001 From: Aman Mehtar Date: Tue, 20 Oct 2026 22:28:08 +0530 Subject: [PATCH 2/3] Edit text: do not group or join by an empty font name (#3661) Review follow-up. `Metrics::from_dict` leaves `base_font` empty when a font has no `/BaseFont`, and a bare subset tag such as `/ABCDEF+` left an empty name after stripping. Two consequences: - `same_style` split one paragraph into separate reading blocks for the exporters when both names were empty, though they grouped before subset joining; - `/ABCDEF+` and `/GHIJKL+` compared equal after stripping, so unrelated malformed fonts could join into one line. `same_base_font` now strips both tags first and requires a non-empty, equal name, so a missing name or a bare tag matches nothing. `same_style` keeps grouping lines whose fonts both lack a name, since the resource or fallback face `same_font` already matched is their whole identity. Tests: a page in a font without `/BaseFont` still reads as one block (it split into two before this change), and a bare tag matches no other name. --- crates/edit/src/tests.rs | 20 +++++++++++++++++++- crates/edit/src/text.rs | 16 +++++++++++++++- crates/edit/src/text_regions.rs | 5 ++++- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index efde39a7f3..65292daa3f 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -883,7 +883,7 @@ fn text_page(content: &str) -> Document { let objs: Vec = vec![ "<< /Type /Catalog /Pages 2 0 R >>".into(), "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), - "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font << /F1 5 0 R /F2 6 0 R /F3 7 0 R /F4 8 0 R >> >> >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font << /F1 5 0 R /F2 6 0 R /F3 7 0 R /F4 8 0 R /F5 9 0 R >> >> >>".into(), format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), // Subset: glyphs for a (97) and b (98) only. @@ -894,6 +894,8 @@ fn text_page(content: &str) -> Document { .into(), // Helvetica again, in a second resource. "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + // No `/BaseFont`: `Metrics::from_dict` leaves its base-font name empty. + "<< /Type /Font /Subtype /Type1 /Encoding /WinAnsiEncoding >>".into(), ]; let mut out = b"%PDF-1.7\n".to_vec(); let mut offs = Vec::new(); @@ -1760,6 +1762,22 @@ fn different_fonts_start_a_new_line() { assert_eq!(text::text_lines(&doc, 0).unwrap().len(), 2); } +/// A font without `/BaseFont` has no name to compare. Its lines still group by resource, as they +/// did before subset joining: an empty name must not split one paragraph into two blocks. +#[test] +fn lines_in_a_font_without_a_base_font_stay_one_block() { + let doc = text_page("BT /F5 12 Tf 72 700 Td (First) Tj 0 -15 Td (second) Tj ET"); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.len(), 2); + assert!(lines.iter().all(|l| l.base_font.is_empty()), "{:?}", lines.iter().map(|l| &l.base_font).collect::>()); + let blocks = text::text_blocks(&doc, 0).unwrap(); + assert_eq!(blocks.len(), 1); + assert_eq!(blocks[0].text, "First second"); + let reading = text::reading_blocks(&doc, 0).unwrap(); + assert_eq!(reading.len(), 1, "{:?}", reading.iter().map(|b| &b.text).collect::>()); + assert_eq!(reading[0].text, "First second"); +} + #[test] fn text_lines_are_found_and_replaced_in_place() { let mut doc = diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 1aa3e5840d..c7adfdb365 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -1466,8 +1466,10 @@ fn same_font(line: &TextLine, font: &[u8], base_font: &str, face: Option<&str>) /// Whether two `/BaseFont` names name the same font: a subset's six-letter tag and `+` prefix do /// not change the font (ISO 32000-2 §9.6.4), so `ABCDEF+Arial` and `GHIJKL+Arial` are one font. +/// A name that is missing, or only a tag (`ABCDEF+`), names no font and matches nothing. fn same_base_font(a: &str, b: &str) -> bool { - !a.is_empty() && (a == b || strip_subset_tag(a) == strip_subset_tag(b)) + let (a, b) = (strip_subset_tag(a), strip_subset_tag(b)); + !a.is_empty() && a == b } /// `ABCDEF+Arial` → `Arial`. A name without a subset tag is returned unchanged. @@ -2804,6 +2806,18 @@ pub(crate) fn translate_blocks(doc: &mut Document, page: usize, indexes: &[usize mod tests { use super::*; + #[test] + fn a_subset_tag_is_the_only_difference_between_subsets_of_one_font() { + assert!(same_base_font("ABCDEF+Arial", "GHIJKL+Arial")); + assert!(same_base_font("Arial", "ABCDEF+Arial")); + assert!(same_base_font("Arial", "Arial")); + assert!(!same_base_font("ABCDEF+Arial", "GHIJKL+Helvetica")); + // A tag with no name after it, or no name at all, names no font. + assert!(!same_base_font("ABCDEF+", "GHIJKL+")); + assert!(!same_base_font("", "")); + assert!(!same_base_font("Arial", "")); + } + #[test] fn paragraph_wrapping_keeps_ideographic_spaces() { let width = |s: &str| s.chars().count() as f64; diff --git a/crates/edit/src/text_regions.rs b/crates/edit/src/text_regions.rs index cd3a258875..a00e42f154 100644 --- a/crates/edit/src/text_regions.rs +++ b/crates/edit/src/text_regions.rs @@ -52,7 +52,10 @@ fn invisible(line: &TextLine) -> bool { fn same_style(a: &TextLine, b: &TextLine) -> bool { same_font(a, b.font.as_bytes(), &b.base_font, b.face.as_deref()) - && same_base_font(&a.base_font, &b.base_font) + // With no `/BaseFont` (or only a subset tag) there is no name to compare: the resource or + // fallback face `same_font` matched is the whole identity, so keep the lines together. + && (same_base_font(&a.base_font, &b.base_font) + || (strip_subset_tag(&a.base_font).is_empty() && strip_subset_tag(&b.base_font).is_empty())) && a.bold == b.bold && a.italic == b.italic && a.color == b.color From 1d76fcbc3b76d4fcebdc7edab83e046c2f968d92 Mon Sep 17 00:00:00 2001 From: Aman Mehtar Date: Wed, 21 Oct 2026 00:46:01 +0530 Subject: [PATCH 3/3] Edit text: tests for re-encoded subsets and five-letter prefixes (#3661) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From @mtlcl410's independent run of this PR: - a second subset of the same font can be re-encoded with `/Differences`, so "-l" has different codes from the first subset's; the line still joins, reads in coding order, and editing it replaces every run, including the re-encoded subset's; - `ABCDE+Helvetica-Bold` has a five-letter prefix, not the six-letter subset tag of ISO 32000-2 §9.6.4, so it stays a different font from `Helvetica-Bold` and the runs stay apart. The first case (and its edit) fails before the subset fix; the second guards the tag-length rule against stripping any `+` prefix. --- crates/edit/src/tests.rs | 45 ++++++++++++++++++++++++++++++++++++++++ crates/edit/src/text.rs | 2 ++ 2 files changed, 47 insertions(+) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 65292daa3f..46cfa4eec7 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -1778,6 +1778,51 @@ fn lines_in_a_font_without_a_base_font_stay_one_block() { assert_eq!(reading[0].text, "First second"); } +/// Two subsets of one font can carry different encodings: here the second names `-` and `l` +/// through `/Differences` (codes 33 and 34), so the same letters have different codes. The visual +/// line still joins and reads in coding order (#3661). +#[test] +fn a_reencoded_subset_of_the_same_font_stays_one_line() { + let content = "BT /F1 16 Tf 72 700 Td (Two) Tj /F2 16 Tf (!\") Tj /F1 16 Tf (ine heading) Tj ET"; + let mut doc = build(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font << /F1 5 0 R /F2 6 0 R >> >> >>", + &stream("", content), + "<< /Type /Font /Subtype /Type1 /BaseFont /ABCDEF+Helvetica-Bold >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /GHIJKL+Helvetica-Bold /Encoding << /Type /Encoding /BaseEncoding /WinAnsiEncoding /Differences [33 /hyphen /l] >> >>", + ]); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.len(), 1, "{:?}", lines.iter().map(|l| &l.text).collect::>()); + assert_eq!(lines[0].text, "Two-line heading"); + let reading = text::reading_blocks(&doc, 0).unwrap(); + assert_eq!(reading.len(), 1); + assert_eq!(reading[0].text, "Two-line heading"); + // Editing the one line replaces every run, including the re-encoded subset's. + text::replace_line(&mut doc, 0, 0, "Two-line heading done").unwrap(); + let reopened = reopen(&doc); + let lines = text::text_lines(&reopened, 0).unwrap(); + assert_eq!(lines.len(), 1, "{:?}", lines.iter().map(|l| &l.text).collect::>()); + assert_eq!(lines[0].text, "Two-line heading done"); +} + +/// `ABCDE+` is five letters, not the six-letter subset tag of §9.6.4, so it stays part of the +/// name: `ABCDE+Helvetica-Bold` and `Helvetica-Bold` are different fonts and their runs stay apart. +#[test] +fn a_five_letter_prefix_is_not_a_subset_tag() { + let content = "BT /F1 16 Tf 72 700 Td (Two) Tj /F2 16 Tf (-l) Tj /F1 16 Tf (ine heading) Tj ET"; + let doc = build(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 600 800] /Contents 4 0 R /Resources << /Font << /F1 5 0 R /F2 6 0 R >> >> >>", + &stream("", content), + "<< /Type /Font /Subtype /Type1 /BaseFont /ABCDE+Helvetica-Bold >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica-Bold >>", + ]); + let lines = text::text_lines(&doc, 0).unwrap(); + assert_eq!(lines.len(), 3, "{:?}", lines.iter().map(|l| &l.text).collect::>()); +} + #[test] fn text_lines_are_found_and_replaced_in_place() { let mut doc = diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index c7adfdb365..5f8866bf40 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -2812,6 +2812,8 @@ mod tests { assert!(same_base_font("Arial", "ABCDEF+Arial")); assert!(same_base_font("Arial", "Arial")); assert!(!same_base_font("ABCDEF+Arial", "GHIJKL+Helvetica")); + // Five letters are part of the name, not a subset tag. + assert!(!same_base_font("ABCDE+Helvetica-Bold", "Helvetica-Bold")); // A tag with no name after it, or no name at all, names no font. assert!(!same_base_font("ABCDEF+", "GHIJKL+")); assert!(!same_base_font("", ""));