From f6a6ded2ccdb6bfbb980e7d0aceed06db3ad8965 Mon Sep 17 00:00:00 2001 From: Valentin Klein Date: Sat, 10 Oct 2026 19:55:28 +0200 Subject: [PATCH] Tests no longer depend on the installed fonts; PDF text of field results with ligatures (#261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a Linux machine with Carlito and Noto Naskh Arabic installed, four layout and PDF tests failed on main: - field_results_are_extractable_text found a real bug: a field result is laid out as one cluster, and the display list gave each of its glyphs one character. A ligature (Carlito, the Calibri substitute, joins "ti") took only its first letter and shifted the rest, so the PDF text read "Sectio 3.01 fir details3". The layout now keeps each glyph's text range from shaping, so a ligature carries all its letters. A new test pins this with bundled Source Sans 3 ("fi", "ffi"), whatever fonts are installed. - fragmented_hidden_text_lays_out_like_the_visible_text compared visible text in alternating bold runs with an all-regular paragraph, so it only passed when the default font's bold happened to break lines the same. The plain paragraph now has the same bold runs. - persian_letters_join_and_brackets_mirror counted glyphs, but Noto Naskh Arabic draws a dotted letter as two glyphs (the dotless form and the dot). It now compares letters by shaping cluster. - glyph_runs_carry_their_text_for_pdf_export expected لا as one ligature glyph; Noto Naskh Arabic draws the lam and the alef separately. The ligature check now applies when the font forms the ligature. Co-authored-by: Claude Opus 5.5 --- crates/layout/src/display.rs | 30 ++++++++++++++++-------- crates/layout/src/para.rs | 21 +++++++++++++---- crates/layout/src/tests.rs | 11 ++++++--- crates/layout/src/tests_bidi.rs | 41 ++++++++++++++++++++++++--------- crates/pdf/src/tests.rs | 17 ++++++++++++++ 5 files changed, 91 insertions(+), 29 deletions(-) diff --git a/crates/layout/src/display.rs b/crates/layout/src/display.rs index c9e19dcb3..801c0fbf4 100644 --- a/crates/layout/src/display.rs +++ b/crates/layout/src/display.rs @@ -343,26 +343,36 @@ fn lines( if matches!(c.kind, ClKind::Text | ClKind::Space) { let a = text.len(); // An object's cluster (a field, a note number) stands for the text it shows. - let mut shown = false; - if let Ok(i) = pl.shown.binary_search_by_key(&k, |(i, _)| *i) - && let Some((_, s)) = pl.shown.get(i) + let mut shown = None; + if let Ok(i) = pl.shown.binary_search_by_key(&k, |(i, _, _)| *i) + && let Some((_, s, r)) = pl.shown.get(i) { text.push_str(s); - shown = true; + shown = Some(r); } else if let Some(p) = para { text.push_str(p.text.get(c.start..c.end).unwrap_or("")); } let b = text.len(); let cg = pl.glyphs.get(c.g0 as usize..c.g1 as usize).unwrap_or(&[]); - if shown && cg.len() > 1 { - // A whole field result is one cluster of many glyphs: give them a character - // each (the last takes any remainder), not the whole text as one ligature. + if let Some(shown) = shown + && cg.len() > 1 + { + // A whole field result is one cluster of many glyphs: each glyph gets the + // text it was shaped from (a ligature its letters), not the whole text as + // one ligature. Without that, a character each (the last takes any remainder). let bounds: Vec = text.get(a..b).unwrap_or("").char_indices().map(|(i, _)| a + i).collect(); + let known = shown.len() == cg.len(); for (n, g) in cg.iter().enumerate() { - let from = bounds.get(n).or(bounds.last()).copied().unwrap_or(a); - let to = if n + 1 == cg.len() { b } else { bounds.get(n + 1).copied().unwrap_or(b) }; + let r = match shown.get(n).filter(|_| known) { + Some(r) => (a + r.start).min(b)..(a + r.end).min(b), + None => { + let from = bounds.get(n).or(bounds.last()).copied().unwrap_or(a); + let to = if n + 1 == cg.len() { b } else { bounds.get(n + 1).copied().unwrap_or(b) }; + from..to + } + }; glyphs.push((g.gid, cx + g.dx, base - st.shift - g.dy)); - ranges.push(from..to.max(from)); + ranges.push(r.start..r.end.max(r.start)); } k += 1; continue; diff --git a/crates/layout/src/para.rs b/crates/layout/src/para.rs index 9d30c2240..995a04e5b 100644 --- a/crates/layout/src/para.rs +++ b/crates/layout/src/para.rs @@ -248,8 +248,9 @@ pub struct ParaLayout { /// tracked deletions in the final text. Resolved as laid out, table formatting included. pub left_out: Vec>, /// The text drawn by clusters that stand for an object (field results, note numbers, - /// equations) rather than for the paragraph's own text, by cluster index, sorted. - pub shown: Vec<(usize, String)>, + /// equations) rather than for the paragraph's own text, by cluster index, sorted, with each of + /// the cluster's glyphs' byte range in that text (a ligature's glyph spans its letters). + pub shown: Vec<(usize, String, Vec>)>, } /// What a table style gives the text of a cell: its paragraph and run formatting, with the @@ -302,7 +303,7 @@ struct Builder<'a> { /// Bidi level per byte of the paragraph text (empty: all left to right). levels: Vec, /// Text drawn by clusters that stand for an object, by cluster index (see `ParaLayout::shown`). - shown: Vec<(usize, String)>, + shown: Vec<(usize, String, Vec>)>, } /// What a piece of text is shaped as: one face, case, script formatting and direction. @@ -564,18 +565,28 @@ impl<'a> Builder<'a> { }); return; }; - // Merge into one cluster: rebase glyph dx onto the first cluster. + // Merge into one cluster: rebase glyph dx onto the first cluster, and keep each glyph's + // text (a ligature's glyph takes the letters of the clusters after it that have none). let mut x = 0.0; + let mut ranges: Vec> = Vec::new(); for c in &added { for g in self.glyphs.get_mut(c.g0 as usize..c.g1 as usize).into_iter().flatten() { g.dx += x; } x += c.adv; + let (a, b) = (c.start.saturating_sub(start), c.end.saturating_sub(start)); + if c.g1 <= c.g0 { + let from = ranges.last().map(|r| r.start); + for r in ranges.iter_mut().rev().take_while(|r| Some(r.start) == from) { + r.end = b; + } + } + ranges.extend((c.g0..c.g1).map(|_| a..b)); } let g0 = first.g0; let style = first.style; let g1 = added.last().map(|c| c.g1).unwrap_or(g0); - self.shown.push((self.clusters.len(), text.to_string())); + self.shown.push((self.clusters.len(), text.to_string(), ranges)); self.clusters.push(Cluster { start, end, adv: x, kind: ClKind::Text, style, g0, g1, break_after: false, obj_h: 0.0, obj_d: 0.0, dot: false }); } } diff --git a/crates/layout/src/tests.rs b/crates/layout/src/tests.rs index c84f1182a..ede55d3c1 100644 --- a/crates/layout/src/tests.rs +++ b/crates/layout/src/tests.rs @@ -260,10 +260,14 @@ fn many_hidden_float_anchors_lay_out_in_linear_time() { #[test] fn fragmented_hidden_text_lays_out_like_the_visible_text() { // Visible words with a hidden multi-byte char after every visible one, formatted in alternating - // runs (bold on and off) so no two left-out runs are adjacent or merge. + // runs (bold on and off) so no two left-out runs are adjacent or merge. The plain paragraph has + // the same visible runs without the hidden chars, so both lay out the same widths whatever font + // the default font resolves to. let words = "Hyphenation wraps international words across narrow columns of text ".repeat(6); let mut d = Document::from_text(""); d.settings.auto_hyphenation = true; + let mut plain = Document::from_text(""); + plain.settings.auto_hyphenation = true; let mut text = String::new(); let mut kept = Vec::new(); for (i, c) in words.chars().enumerate() { @@ -276,10 +280,11 @@ fn fragmented_hidden_text_lays_out_like_the_visible_text() { d.insert_text(&Pos::body(0, n), &c.to_string(), &bold).unwrap(); let hidden = wordcraft_doc::CharProps { hidden: Some(true), bold: Some(i % 2 == 1), ..Default::default() }; d.insert_text(&Pos::body(0, at), "é", &hidden).unwrap(); + let n = plain.para(StoryRef::Body, &Path::top(0)).unwrap().len(); + plain.insert_text(&Pos::body(0, n), &c.to_string(), &bold).unwrap(); } assert_eq!(d.para(StoryRef::Body, &Path::top(0)).unwrap().text, text); - let mut plain = Document::from_text(&words); - plain.settings.auto_hyphenation = true; + assert_eq!(plain.para(StoryRef::Body, &Path::top(0)).unwrap().text, words); for doc in [&mut d, &mut plain] { doc.format_paragraphs(&Pos::body(0, 0), &Pos::body(0, 0), &|p| p.indent_right = Some(468.0 - 90.0)).unwrap(); } diff --git a/crates/layout/src/tests_bidi.rs b/crates/layout/src/tests_bidi.rs index 6a3545aa4..da738e535 100644 --- a/crates/layout/src/tests_bidi.rs +++ b/crates/layout/src/tests_bidi.rs @@ -303,24 +303,38 @@ fn arabic_face() -> Option { wordcraft_fonts::FontDb::global().fallback_for('ب', 0).map(|f| wordcraft_fonts::FaceRef::of(&f)).filter(|f| f.covers('پ')) } +/// The glyph ids of each cluster, in order. A font may draw a letter as several glyphs (Noto +/// Naskh Arabic draws ب as its dotless form plus the dot), so letters are compared by cluster. +fn cluster_glyphs(glyphs: &[wordcraft_fonts::ShapedGlyph]) -> Vec> { + let mut out: Vec<(usize, Vec)> = Vec::new(); + for g in glyphs { + match out.last_mut() { + Some((c, ids)) if *c == g.cluster => ids.push(g.gid), + _ => out.push((g.cluster, vec![g.gid])), + } + } + out.into_iter().map(|(_, ids)| ids).collect() +} + #[test] fn persian_letters_join_and_brackets_mirror() { let Some(face) = arabic_face() else { eprintln!("skipped: no font with Persian letters installed"); return; }; - let isolated = wordcraft_fonts::shape_run(&face, "ب", &[], |c| c, true); - let joined = wordcraft_fonts::shape_run(&face, "ببب", &[], |c| c, true); - assert_eq!(joined.len(), 3); - assert!(joined.iter().all(|g| g.gid != 0), "the font has the letters"); + let isolated = cluster_glyphs(&wordcraft_fonts::shape_run(&face, "ب", &[], |c| c, true)); + assert_eq!(isolated.len(), 1, "{isolated:?}"); + let shaped = wordcraft_fonts::shape_run(&face, "ببب", &[], |c| c, true); + let joined = cluster_glyphs(&shaped); + assert_eq!(joined.len(), 3, "one cluster per letter: {joined:?}"); + assert!(shaped.iter().all(|g| g.gid != 0), "the font has the letters"); // Initial, medial and final forms differ from the isolated letter. - assert!(joined.iter().any(|g| g.gid != isolated[0].gid), "cursive joining picks contextual forms"); + assert!(joined.iter().any(|g| *g != isolated[0]), "cursive joining picks contextual forms: {joined:?} {isolated:?}"); // Clusters come back in logical order. - assert!(joined.windows(2).all(|w| w[0].cluster <= w[1].cluster)); + assert!(shaped.windows(2).all(|w| w[0].cluster <= w[1].cluster)); // ZWNJ (نیم‌فاصله) breaks the join: "می‌خواهم" keeps می separate. - let with_zwnj = wordcraft_fonts::shape_run(&face, "ب\u{200C}ب", &[], |c| c, true); - let first = with_zwnj.first().map(|g| g.gid); - assert_eq!(first, Some(isolated[0].gid), "a letter before ZWNJ takes its isolated form"); + let with_zwnj = cluster_glyphs(&wordcraft_fonts::shape_run(&face, "ب\u{200C}ب", &[], |c| c, true)); + assert_eq!(with_zwnj.first(), isolated.first(), "a letter before ZWNJ takes its isolated form"); // In right-to-left text, "(" is drawn with the mirrored glyph. let open = wordcraft_fonts::shape_run(&face, "(", &[], |c| c, true); let close = wordcraft_fonts::shape_run(&face, ")", &[], |c| c, false); @@ -354,6 +368,10 @@ fn glyph_runs_carry_their_text_for_pdf_export() { let s = "سلام WordCraft می‌خواهم"; let d = doc_of(&[(s, true)]); let l = lay(&d); + let lam_alef_ligature = arabic_face().is_some_and(|f| { + let g = wordcraft_fonts::shape_run(&f, "لا", &[], |c| c, true); + !g.is_empty() && g.iter().all(|g| g.cluster == 0) + }); let mut all = String::new(); for it in display::page_display(&d, &l.pages[0], &Default::default()) { if let display::Draw::Glyphs { glyphs, text, ranges, .. } = it { @@ -361,8 +379,9 @@ fn glyph_runs_carry_their_text_for_pdf_export() { for r in &ranges { assert!(text.get(r.clone()).is_some_and(|t| !t.is_empty()), "{r:?} in {text:?}"); } - // A ligature glyph (لا) shows both its letters (when a Persian font draws it). - for r in ranges.iter().filter(|_| arabic_face().is_some()) { + // A ligature glyph (لا) shows both its letters (when the Persian font draws one: some, + // such as Noto Naskh Arabic, draw the lam and the alef as glyphs of their own). + for r in ranges.iter().filter(|_| lam_alef_ligature) { if text.get(r.clone()) == Some("ل") { panic!("the lam of لا is drawn as one glyph with the alef: {text:?} {ranges:?}"); } diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index 9c5fdfbfe..bd1c2be3a 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -539,6 +539,23 @@ fn field_results_are_extractable_text() { assert!(squash(&text).contains("See Section 3.01 for details."), "{text:?}"); } +#[test] +fn field_result_ligatures_extract_all_their_letters() { + // A field result is shaped as one cluster; a ligature in it (Carlito, the Calibri substitute, + // joins "ti") used to get only its first letter, shifting the rest: "Sectio 3.01". Bundled + // Source Sans 3 joins "fi" and "ffi", so this doesn't depend on the installed fonts. + let font = CharProps { font: Some("Source Sans 3".into()), ..Default::default() }; + let result = "the first office"; + let face = wordcraft_fonts::FontDb::global().face("Source Sans 3", "Regular"); + assert!(wordcraft_fonts::shape(&face, result, &[], |c| c).len() < result.chars().count(), "the result has ligatures"); + let mut d = Document::new(); + let mut p = Paragraph::with_text("See here.", font.clone()); + p.insert_object(4, InlineObject::Field { instr: " REF _RefTarget \\h ".into(), result: result.into(), locked: false }, &font).unwrap(); + d.body = vec![para_block(p)]; + let text = extract_text(&export(&d, &PdfOptions::default()).unwrap()).concat(); + assert!(squash(&text).contains("See the first office here."), "{text:?}"); +} + #[test] fn synthetic_bold_text_is_extracted_once() { // JetBrains Mono ships without a bold face, so its bold is filled and stroked: the text