From 8cdc172171f0873f3989904026f724a37e8e4f7b Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:39:09 +0100 Subject: [PATCH 1/3] Lay out the paragraph after a kept heading once A paragraph with keep with next measures the paragraph after it before it is placed (next_first_line). That measurement called layout_para directly, so every paragraph following a heading was laid out twice in each layout pass: once to measure it and once, through the cache, when it was placed. The measurement now goes through the layout cache under the key para_labelled uses (Ctx::para_ahead) when its inputs are the same: no proofing, no typed word, not inside a text box, no inline text boxes that resize, the body's tracked-deletion view. Otherwise it is laid out on its own as before. It counts nothing (list labels, equation numbers and the page-total flag are restored), so pagination is unchanged. Adds crates/engine/examples/export_bench.rs, which saves a long generated document (the sample document 200 times, 259 pages) through io::save_bytes and prints wall times: cargo run --release -p wordcraft-engine --example export_bench -- pdf 200 9 PDF export on that document, interleaved with main (Ryzen, 16 threads): median 286 ms -> 256 ms (-10.6 %). The PDF bytes are identical. Co-Authored-By: Claude Opus 5.5 --- crates/engine/examples/export_bench.rs | 64 ++++++++++++++++++++++++++ crates/layout/src/ctx.rs | 30 ++++++++++++ crates/layout/src/flow.rs | 43 +++++++++-------- crates/layout/src/tests.rs | 24 ++++++++++ 4 files changed, 142 insertions(+), 19 deletions(-) create mode 100644 crates/engine/examples/export_bench.rs diff --git a/crates/engine/examples/export_bench.rs b/crates/engine/examples/export_bench.rs new file mode 100644 index 000000000..318576a4b --- /dev/null +++ b/crates/engine/examples/export_bench.rs @@ -0,0 +1,64 @@ +//! Export timing: saves a long generated document (the built-in sample document repeated) through +//! the real save path, the way `file.save` / `file.export` write it, and prints wall times. +//! +//! `cargo run --release -p wordcraft-engine --example export_bench -- [ext] [copies] [runs] [out]` +//! +//! Defaults: `pdf`, 40 copies (about 100 pages), 7 runs. With `out` the last run's bytes are +//! written there, to compare the output of two builds. + +use std::sync::Arc; +use std::time::Instant; + +use wordcraft_engine::doc::props::CharProps; +use wordcraft_engine::doc::{Block, Document}; +use wordcraft_engine::{io, sample}; + +/// The sample document's body `copies` times, each paragraph ending in its copy number so no two +/// copies share text (as the chapters of a real long document don't). +fn long_document(copies: usize) -> Document { + let mut d = sample::sample_document(); + // A fixed creation date: the PDF's dates (and the ID derived from them) stay the same from run to run. + d.core.created = "2026-01-01T00:00:00Z".into(); + let one: Vec = d.body.iter().map(|b| (**b).clone()).collect(); + d.body.clear(); + for i in 0..copies { + for b in &one { + let mut b = b.clone(); + if let Block::Para(p) = &mut b { + let at = p.len(); + let _ = p.insert_text(at, &format!(" {i}"), &CharProps::default()); + } + d.body.push(Arc::new(b)); + } + } + d +} + +fn main() -> Result<(), String> { + let args: Vec = std::env::args().skip(1).collect(); + let ext = args.first().map_or("pdf", String::as_str); + let copies = args.get(1).and_then(|s| s.parse().ok()).unwrap_or(40); + let runs = args.get(2).and_then(|s| s.parse().ok()).unwrap_or(7usize).max(1); + let doc = long_document(copies); + let name = format!("bench.{ext}"); + let mut ms = Vec::with_capacity(runs); + let mut bytes = Vec::new(); + for _ in 0..runs { + let t = Instant::now(); + bytes = io::save_bytes(&name, &doc)?; + ms.push(t.elapsed().as_secs_f64() * 1000.0); + } + let mut sorted = ms.clone(); + sorted.sort_by(f64::total_cmp); + let median = sorted.get(sorted.len() / 2).copied().unwrap_or_default(); + let min = sorted.first().copied().unwrap_or_default(); + println!( + "{ext}: {copies} copies, {} bytes, runs {:?} ms, median {median:.1} ms, min {min:.1} ms", + bytes.len(), + ms.iter().map(|m| m.round()).collect::>() + ); + if let Some(out) = args.get(3) { + std::fs::write(out, &bytes).map_err(|e| e.to_string())?; + } + Ok(()) +} diff --git a/crates/layout/src/ctx.rs b/crates/layout/src/ctx.rs index b17d68277..62b99b5d9 100644 --- a/crates/layout/src/ctx.rs +++ b/crates/layout/src/ctx.rs @@ -226,6 +226,36 @@ impl Ctx<'_> { pl } + /// `p` laid out on its own, as the paragraph after a kept one is measured before it is placed + /// (no wrap areas, proofing, typed word or resized text boxes), through the cache when + /// [`Ctx::para_labelled`] would lay it out with those same inputs: it is then not laid out a + /// second time when it is placed. Counts nothing (list labels, equation numbers, page totals). + /// `None` when the inputs differ. + pub(crate) fn para_ahead(&mut self, p: &Paragraph, width: f32, label: Option<(String, Level)>) -> Option> { + let resized = p + .objects + .iter() + .any(|o| matches!(o, InlineObject::Shape { story: Some(_), float, .. } if float.wrap == Wrap::Inline && float.rot == 0.0)); + if self.opts.proofing || self.boxes.inside() || resized || self.hide_deleted != self.opts.hides_deleted(StoryRef::Body) { + return None; + } + let kept = (self.eq_count, self.uses_totals); + let (key, eq_number) = self.para_key(p, width, None, &[], label.as_ref(), &[]); + (self.eq_count, self.uses_totals) = kept; + if key.typing.is_some() { + return None; + } + self.cache.used.insert(key.clone()); + if let Some(pl) = self.cache.paras.get(&key) { + self.cache.hits += 1; + return Some(pl.clone()); + } + self.cache.misses += 1; + let pl = Arc::new(para::layout_para(p, &self.para_env(width, label, None, &[], eq_number, None, &[]))); + self.cache.paras.insert(key, pl.clone()); + Some(pl) + } + /// What `measure` reads from `p` laid out as [`Ctx::para_labelled`] would, remembering only /// the result: a table's columns measure every cell once at an unbounded width, and keeping /// those layouts would double what a long table holds (and walk them all again on each pass). diff --git a/crates/layout/src/flow.rs b/crates/layout/src/flow.rs index f52cd5ccd..e7919fd54 100644 --- a/crates/layout/src/flow.rs +++ b/crates/layout/src/flow.rs @@ -628,26 +628,31 @@ fn next_first_line(ctx: &mut Ctx, block: usize, width: f32, depth: usize, counte let rp = ctx.doc.styles.resolve_para(&p.props); let label = p.props.numbering.or(rp.numbering).filter(|n| n.num != 0).and_then(|n| counters.next_label(&ctx.doc.numbering, n.num, n.level)); - let env = para::ParaEnv { - doc: ctx.doc, - width, - label, - fields: &ctx.fields, - show_hidden: ctx.opts.show_hidden, - field_codes: ctx.opts.field_codes, - hide_deleted: ctx.opts.hides_deleted(StoryRef::Body), - table: None, - proofing: None, - typing_word: None, - numeral: ctx.opts.numeral, - exclusions: &[], - eq_number: ctx.eq_count, - read_line_spacing: if ctx.opts.view == ViewMode::Read { ctx.opts.read_line_spacing } else { None }, - grid: ctx.grid, - fit_objects: false, - fitted: &[], + let pl = match ctx.para_ahead(p, width, label.clone()) { + Some(pl) => pl, + None => { + let env = para::ParaEnv { + doc: ctx.doc, + width, + label, + fields: &ctx.fields, + show_hidden: ctx.opts.show_hidden, + field_codes: ctx.opts.field_codes, + hide_deleted: ctx.opts.hides_deleted(StoryRef::Body), + table: None, + proofing: None, + typing_word: None, + numeral: ctx.opts.numeral, + exclusions: &[], + eq_number: ctx.eq_count, + read_line_spacing: if ctx.opts.view == ViewMode::Read { ctx.opts.read_line_spacing } else { None }, + grid: ctx.grid, + fit_objects: false, + fitted: &[], + }; + Arc::new(para::layout_para(p, &env)) + } }; - let pl = para::layout_para(p, &env); // Its border room, unless it joins the paragraph before (or after) in one box. let own = Edges::own(&pl, 0, pl.lines.len()); let top = if joined(ctx.doc, &ctx.doc.body, block + 1, Some(block), None) { 0.0 } else { own.top }; diff --git a/crates/layout/src/tests.rs b/crates/layout/src/tests.rs index e38133a41..c993a9d27 100644 --- a/crates/layout/src/tests.rs +++ b/crates/layout/src/tests.rs @@ -2463,6 +2463,30 @@ fn cache_reuses_unchanged_paragraphs() { assert_eq!(c.misses, misses + 1); } +/// The paragraph after a heading kept with it is measured before the heading is placed. That +/// layout goes into the cache under the key it is placed with, so it is laid out once, and the +/// pages are those of the uncached measurement (proofing on measures without the cache). +#[test] +fn kept_headings_measure_the_next_paragraph_through_the_cache() { + let mut d = Document::new(); + let kept = ParaProps { keep_next: Some(true), ..Default::default() }; + for i in 0..40 { + let mut heading = wordcraft_doc::Paragraph::with_text(&format!("Heading {i}"), Default::default()); + heading.props = kept.clone(); + d.body.push(wordcraft_doc::para_block(heading)); + let body = format!("Body text {i} under its heading, long enough to wrap onto a second line in the column. ").repeat(3); + d.body.push(wordcraft_doc::para_block(wordcraft_doc::Paragraph::with_text(&body, Default::default()))); + } + let mut c = LayoutCache::new(); + let l = layout(&d, &mut c, &LayoutOptions::default()); + assert!(l.pages.len() > 2, "{}", l.pages.len()); + assert_eq!(c.len(), d.body.len(), "one layout per paragraph"); + assert_eq!(c.misses, d.body.len() as u64, "each paragraph laid out once"); + let proofed = layout(&d, &mut LayoutCache::new(), &LayoutOptions { proofing: true, ..Default::default() }); + let starts = |l: &DocLayout| (0..d.body.len()).map(|i| l.caret(&Pos::body(i, 0)).map(|c| (c.page, c.top.to_bits()))).collect::>(); + assert_eq!(starts(&l), starts(&proofed)); +} + /// Autofit measures every cell at an unbounded width; the cache keeps what it measured, not /// those layouts, so a long table holds one layout per cell paragraph. Laying the document out /// again after an edit elsewhere measures nothing again. From ffa127c615ffe8477187f36f0c2bdbc7de7f6e80 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:39:09 +0100 Subject: [PATCH 2/3] Skip per-character table lookups for plain text in layout and PDF export Shortcuts that give the same answers as the lookups they skip: - is_rtl: nothing before U+0590 is right to left, so Latin, Greek and Cyrillic text skips the bidi class search. - strong_rtl and is_mark: ASCII letters are the only strong ASCII characters, and no ASCII character is a mark. - cluster_starts: in ASCII text every character starts a grapheme except LF after CR, and no ASCII character joins a cluster. - display lines: a glyph run's vectors and text are sized up front, and the symbol-font check is made once per run instead of once per cluster. - PDF glyph runs: the baseline (the y most glyphs share) is only worked out for italic runs, the only ones that use it, and a run on one baseline returns it without counting. Tests check each shortcut against the full lookup (every code point for the bidi classes, every ASCII pair for the clusters). PDF export of the export_bench document, interleaved with main: median 293 ms -> 241 ms (-18 % with the previous commit). The PDF bytes are identical. Co-Authored-By: Claude Opus 5.5 --- crates/doc/src/cluster.rs | 21 +++++++++++++++++++++ crates/fonts/src/lib.rs | 13 ++++++++++++- crates/layout/src/display/lines.rs | 16 ++++++++++++---- crates/layout/src/para.rs | 16 ++++++++++++++++ crates/layout/src/para/bidi.rs | 4 ++++ crates/layout/src/para/chars.rs | 3 ++- crates/pdf/src/exporter/text.rs | 4 ++-- crates/pdf/src/glyphs.rs | 4 ++++ crates/pdf/src/lib.rs | 2 +- crates/pdf/src/tests.rs | 12 ++++++++++++ 10 files changed, 86 insertions(+), 9 deletions(-) diff --git a/crates/doc/src/cluster.rs b/crates/doc/src/cluster.rs index b41a8fcc4..186b79f9d 100644 --- a/crates/doc/src/cluster.rs +++ b/crates/doc/src/cluster.rs @@ -64,6 +64,11 @@ pub fn is_cluster_boundary(text: &str, off: usize) -> bool { /// Byte offsets where clusters start (graphemes, joined per [`joins_cluster`]), in order; the /// first is 0 for non-empty text. pub fn cluster_starts(text: &str) -> Vec { + // ASCII: every character starts a grapheme except LF after CR, and none joins (UAX #29). + if text.is_ascii() { + let b = text.as_bytes(); + return (0..b.len()).filter(|&i| !(i > 0 && b.get(i - 1) == Some(&b'\r') && b.get(i) == Some(&b'\n'))).collect(); + } let mut out: Vec = Vec::new(); let mut prev: Option = None; for (i, g) in text.grapheme_indices(true) { @@ -188,6 +193,22 @@ mod tests { assert_eq!(split(kinzi, &cluster_starts(kinzi)), vec!["အ", "င်္ဂ", "လိ", "ပ်"]); } + /// The ASCII shortcut gives the grapheme rules' answer. + #[test] + fn ascii_cluster_starts_match_the_graphemes() { + let graphemes = |t: &str| -> Vec { t.grapheme_indices(true).map(|(i, _)| i).collect() }; + let all: String = (0u8..128).map(char::from).collect(); + for t in ["", "a", "Hello, world.", "\r\n", "a\r\nb\n\rc\r\r\n\n", "\t \u{7f}x\u{0}", all.as_str()] { + assert_eq!(cluster_starts(t), graphemes(t), "{t:?}"); + } + for a in 0u8..128 { + for b in 0u8..128 { + let t = format!("{}{}", char::from(a), char::from(b)); + assert_eq!(cluster_starts(&t), graphemes(&t), "{t:?}"); + } + } + } + #[test] fn khmer_subscripts_and_indic_conjuncts_are_one_cluster() { // ខ្មែរ: ខ + coeng + ម + ែ, then រ. diff --git a/crates/fonts/src/lib.rs b/crates/fonts/src/lib.rs index d8e6acf7d..59d6295aa 100644 --- a/crates/fonts/src/lib.rs +++ b/crates/fonts/src/lib.rs @@ -133,7 +133,9 @@ pub fn feature_value(tag: &[u8; 4], value: u32) -> Feature { /// A strong right-to-left character (Hebrew, Arabic, …)? pub fn is_rtl(c: char) -> bool { use unicode_bidi::BidiClass::{AL, R}; - matches!(unicode_bidi::bidi_class(c), R | AL) + // Nothing before the Hebrew block is right to left: Latin, Greek and Cyrillic text skips the + // table search. + c >= '\u{590}' && matches!(unicode_bidi::bidi_class(c), R | AL) } /// Maximal runs of one direction (neutrals join the run they're in): (byte range, right-to-left). @@ -254,6 +256,15 @@ pub fn first_glyph(face: &FontFace, chars: &[char]) -> u32 { mod tests { use super::*; + /// The shortcut before the Hebrew block gives the bidi class's answer for every character. + #[test] + fn is_rtl_matches_the_bidi_class() { + use unicode_bidi::BidiClass::{AL, R}; + for c in (0..=0x10ffff).filter_map(char::from_u32) { + assert_eq!(is_rtl(c), matches!(unicode_bidi::bidi_class(c), R | AL), "{:04x}", c as u32); + } + } + #[test] fn last_resort_face_parses() { let f = fontdb::last_resort_face(); diff --git a/crates/layout/src/display/lines.rs b/crates/layout/src/display/lines.rs index b64899985..0d253f920 100644 --- a/crates/layout/src/display/lines.rs +++ b/crates/layout/src/display/lines.rs @@ -156,9 +156,17 @@ pub(super) fn lines( k += 1; continue; }; - let mut glyphs = Vec::new(); - let mut text = String::new(); - let mut ranges: Vec> = Vec::new(); + // Sized for the run's text clusters up front: no growing per glyph. + let run = pl.clusters.get(k..line.c1.min(pl.clusters.len())).unwrap_or_default(); + let (n_glyphs, n_bytes) = run + .iter() + .take_while(|c| c.style == si) + .filter(|c| matches!(c.kind, ClKind::Text | ClKind::Space)) + .fold((0usize, 0usize), |(g, b), c| (g + c.g1.saturating_sub(c.g0) as usize, b + c.end.saturating_sub(c.start))); + let mut glyphs = Vec::with_capacity(n_glyphs); + let mut text = String::with_capacity(n_bytes); + let mut ranges: Vec> = Vec::with_capacity(n_glyphs); + let symbol_font = wordcraft_doc::symbol_fonts::is_symbol_font(&st.rc.font); let run_start = k; while k < line.c1 { let Some(c) = pl.clusters.get(k) else { break }; @@ -184,7 +192,7 @@ pub(super) fn lines( } else if let Some(p) = para { let t = p.text.get(c.start..c.end).unwrap_or(""); // Symbol-font characters (Wingdings…) read as the Unicode symbols drawn. - if wordcraft_doc::symbol_fonts::is_symbol_font(&st.rc.font) { + if symbol_font { text.push_str(&wordcraft_doc::symbol_fonts::map_text(&st.rc.font, t)); } else if st.rc.caps || st.rc.small_caps { // All Caps and Small Caps read as the capitals drawn (#1473), mapped diff --git a/crates/layout/src/para.rs b/crates/layout/src/para.rs index 24db10314..25dd940a6 100644 --- a/crates/layout/src/para.rs +++ b/crates/layout/src/para.rs @@ -423,6 +423,22 @@ mod label_spacing_tests { mod tests { use super::*; + /// The ASCII shortcuts give the bidi class's answer for every character. + #[test] + fn strong_and_mark_shortcuts_match_the_bidi_class() { + use unicode_bidi::BidiClass::{AL, BN, L, NSM, R}; + for c in (0..=0x10ffff).filter_map(char::from_u32) { + let class = unicode_bidi::bidi_class(c); + let strong = match class { + R | AL => Some(true), + L => Some(false), + _ => None, + }; + assert_eq!(bidi::strong_rtl(c), strong, "{:04x}", c as u32); + assert_eq!(chars::is_mark(c), matches!(class, NSM | BN) && !c.is_control(), "{:04x}", c as u32); + } + } + #[test] fn symbols_ignore_roman_hint() { let mut d = Document::new(); diff --git a/crates/layout/src/para/bidi.rs b/crates/layout/src/para/bidi.rs index d62af4e7f..62bee16d3 100644 --- a/crates/layout/src/para/bidi.rs +++ b/crates/layout/src/para/bidi.rs @@ -15,6 +15,10 @@ pub(super) enum Dir<'l> { /// The direction of `c` if it is a strong letter (UAX #9 P2): `Some(true)` right to left. pub(super) fn strong_rtl(c: char) -> Option { use unicode_bidi::BidiClass::{AL, L, R}; + // In ASCII only the letters are strong (L). + if c.is_ascii() { + return c.is_ascii_alphabetic().then_some(false); + } match unicode_bidi::bidi_class(c) { R | AL => Some(true), L => Some(false), diff --git a/crates/layout/src/para/chars.rs b/crates/layout/src/para/chars.rs index 5302f44cc..ba039850a 100644 --- a/crates/layout/src/para/chars.rs +++ b/crates/layout/src/para/chars.rs @@ -58,7 +58,8 @@ pub(super) fn caps_expands(c: char) -> bool { /// character before them: same font, same shaping run, so Arabic joining isn't broken. pub(super) fn is_mark(c: char) -> bool { use unicode_bidi::BidiClass::{BN, NSM}; - matches!(unicode_bidi::bidi_class(c), NSM | BN) && !c.is_control() + // No ASCII character is a mark (its BN characters are controls). + !c.is_ascii() && matches!(unicode_bidi::bidi_class(c), NSM | BN) && !c.is_control() } /// A digit Word's Numeral modes substitute: ASCII or Arabic-Indic (U+0660–U+0669). diff --git a/crates/pdf/src/exporter/text.rs b/crates/pdf/src/exporter/text.rs index d92d4891c..fff476667 100644 --- a/crates/pdf/src/exporter/text.rs +++ b/crates/pdf/src/exporter/text.rs @@ -110,8 +110,6 @@ impl Exporter<'_> { let placed: Vec<(u32, f32, f32, std::ops::Range)> = order.iter().filter_map(|&i| Some((glyphs.get(i).copied()?, ranges.get(i).cloned()?))).map(|((g, x, y), r)| (g, x, y, r)).collect(); let Some(&(_, x0, y0, _)) = placed.first() else { return }; - // The run's baseline: the y most of its glyphs share (marks sit higher or lower). - let base = baseline(glyphs).unwrap_or(y0); let kg: Vec = placed .iter() .enumerate() @@ -125,6 +123,8 @@ impl Exporter<'_> { .collect(); let pushed = italic; if italic { + // The run's baseline: the y most of its glyphs share (marks sit higher or lower). + let base = baseline(glyphs).unwrap_or(y0); s.push_transform(&Transform::from_row(1.0, 0.0, -0.21, 1.0, 0.21 * base, 0.0)); } s.set_fill(Some(fill(color, alpha))); diff --git a/crates/pdf/src/glyphs.rs b/crates/pdf/src/glyphs.rs index ea6264156..bad9a1b21 100644 --- a/crates/pdf/src/glyphs.rs +++ b/crates/pdf/src/glyphs.rs @@ -116,6 +116,10 @@ pub(crate) fn visual_order( /// The y most of a run's glyphs share: its baseline, which raised or lowered marks leave. pub(crate) fn baseline(glyphs: &[(u32, f32, f32)]) -> Option { + let &(_, _, first) = glyphs.first()?; + if glyphs.iter().all(|g| g.2.to_bits() == first.to_bits()) { + return Some(first); + } let mut counts: HashMap = HashMap::new(); for (_, _, y) in glyphs { *counts.entry(y.to_bits()).or_insert(0) += 1; diff --git a/crates/pdf/src/lib.rs b/crates/pdf/src/lib.rs index 0363fc139..78a27d35e 100644 --- a/crates/pdf/src/lib.rs +++ b/crates/pdf/src/lib.rs @@ -65,7 +65,7 @@ pub use export::{export, export_layout}; #[cfg(test)] use exporter::Exporter; #[cfg(test)] -use glyphs::{cluster_ends, left_to_right, split_clusters, visual_order}; +use glyphs::{baseline, cluster_ends, left_to_right, split_clusters, visual_order}; #[cfg(test)] use metadata::{civil, parse_iso}; pub use options::{PdfError, PdfOptions}; diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index f6548d9c7..c8f1bec54 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -2450,3 +2450,15 @@ fn cluster_ends_match_the_next_larger_cluster_in_linear_time() { assert!(ends[..=n].iter().all(|e| *e == 2 * n + 2)); assert_eq!(ends.last(), Some(&(2 * n + 4))); } + +/// A run's baseline is the y most of its glyphs share, ties to the lower bits; a run on one +/// baseline (the common case) gives its y without counting. +#[test] +fn baseline_is_the_most_shared_y() { + assert_eq!(baseline(&[]), None); + assert_eq!(baseline(&[(1, 0.0, 7.5)]), Some(7.5)); + assert_eq!(baseline(&[(1, 0.0, 7.5), (2, 3.0, 7.5), (3, 6.0, 7.5)]), Some(7.5)); + assert_eq!(baseline(&[(1, 0.0, 7.5), (2, 3.0, 4.0), (3, 6.0, 7.5)]), Some(7.5)); + assert_eq!(baseline(&[(1, 0.0, 9.0), (2, 3.0, 4.0)]), Some(4.0)); + assert_eq!(baseline(&[(1, 0.0, -0.0), (2, 3.0, 0.0)]), Some(0.0)); +} From de3bfaf049796abf00ab36e39e14952049170733 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:39:10 +0100 Subject: [PATCH 3/3] Keep harfrust shape plans per face Each shaping call let harfrust compile a new shape plan (the face's lookups for the run's script, direction, language and features), which took about as long as shaping a short run. Each thread now keeps up to 64 plans (the oldest dropped to make room), found by face and harfrust's ShapePlanKey::matches, and passes the plan to the shaper. A kept plan is only used for the face it was made for: the face id (from one process-wide counter, so no two faces share it) together with the font bytes' address, length and face index. The plan is the one the shaper would have made for the buffer, so the glyphs are the same; tests shape with kept and fresh plans and compare, and check the per-thread bound. PDF export of the export_bench document (259 pages), interleaved with main (A/A within 1 %): median 421 ms -> 319 ms (-24.2 %), measured on a loaded machine; on a quieter run earlier 294 ms -> 224 ms (-23.8 %). PNG export of page 1 (which lays out the whole document): about -34 %. The PDF and PNG bytes are identical to main's. Co-Authored-By: Claude Opus 5.5 --- crates/fonts/src/lib.rs | 124 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 122 insertions(+), 2 deletions(-) diff --git a/crates/fonts/src/lib.rs b/crates/fonts/src/lib.rs index 59d6295aa..b7655439b 100644 --- a/crates/fonts/src/lib.rs +++ b/crates/fonts/src/lib.rs @@ -19,7 +19,7 @@ pub use fontdb::{ base_style, bundled, decode_font_file, is_font_file_name, name_language_matches, sniff, system_font_dirs, }; pub use harfrust::Feature; -use harfrust::{Direction, ShapeOptions, Tag, UnicodeBuffer}; +use harfrust::{Direction, ShapeOptions, ShapePlan, ShapePlanKey, Shaper, Tag, UnicodeBuffer}; pub use kurbo::BezPath; pub use script::{fallback_rank, is_emoji, needs_layout_tables, script_fallbacks}; use skrifa::MetadataProvider; @@ -221,7 +221,8 @@ fn shape_dir(face: &FontFace, text: &str, features: &[Feature], map: &impl Fn(ch } buf.guess_segment_properties(); buf.set_direction(if rtl { Direction::RightToLeft } else { Direction::LeftToRight }); - let gb = shaper.shape(buf, ShapeOptions::new().features(features)); + let plan = shape_plan(face, &shaper, &buf, features); + let gb = shaper.shape(buf, ShapeOptions::new().plan(plan.as_deref()).features(features)); for (info, pos) in gb.glyph_infos().iter().zip(gb.glyph_positions()) { out.push(ShapedGlyph { gid: info.glyph_id, @@ -247,6 +248,69 @@ fn shape_dir(face: &FontFace, text: &str, features: &[Feature], map: &impl Fn(ch out } +/// How many shape plans each thread keeps: the oldest is dropped to make room, so a thread never +/// holds more than this many, whatever fonts, scripts and feature sets it shapes. +const MAX_SHAPE_PLANS: usize = 64; + +/// The face a kept shape plan was made for. Face ids come from one process-wide counter +/// (`NEXT_ID`, taken once when a face is built), so no two faces share one; the font bytes' +/// address and length and the face index are kept too, so a plan is only ever reused for the +/// font data it was made from, even if the counter were to wrap after 2^32 faces. +#[derive(Clone, Copy, PartialEq, Eq)] +struct PlanFace { + id: u32, + data: usize, + len: usize, + index: u32, +} + +impl PlanFace { + fn of(face: &FontFace) -> PlanFace { + let data = face.data(); + PlanFace { id: face.id(), data: data.as_ptr() as usize, len: data.len(), index: face.index() } + } +} + +thread_local! { + /// Shape plans made so far on this thread, by face: making one (the face's lookups for a + /// script, direction and feature set) can take longer than shaping the run. + static SHAPE_PLANS: std::cell::RefCell)>> = const { std::cell::RefCell::new(Vec::new()) }; +} + +/// The plan the shaper would make for `buf` in `face` with `features`, made once per thread (the +/// oldest dropped past [`MAX_SHAPE_PLANS`]). A kept plan is used only for the same face and when +/// harfrust's own [`ShapePlanKey::matches`] accepts it (script, direction, language, the face's +/// variation instance and the features). `None` leaves making it to the shaper. +fn shape_plan(face: &FontFace, shaper: &Shaper, buf: &UnicodeBuffer, features: &[Feature]) -> Option> { + // The buffer's script is unset when it reads as unknown: the guess never sets that. + let script = Some(buf.script()).filter(|s| *s != harfrust::script::UNKNOWN); + let language = buf.language(); + let direction = buf.direction(); + if direction == Direction::Invalid { + return None; + } + let key = ShapePlanKey::new(script, direction).language(language.as_ref()).instance(face.instance.as_ref()).features(features); + let of = PlanFace::of(face); + SHAPE_PLANS.with(|cell| { + let mut plans = cell.try_borrow_mut().ok()?; + if let Some((_, plan)) = plans.iter().find(|(f, plan)| *f == of && key.matches(plan)) { + return Some(plan.clone()); + } + let plan = std::rc::Rc::new(ShapePlan::new(shaper, direction, script, language.as_ref(), features)); + if plans.len() >= MAX_SHAPE_PLANS { + plans.remove(0); + } + plans.push((of, plan.clone())); + Some(plan) + }) +} + +/// How many shape plans this thread keeps. +#[cfg(test)] +fn kept_shape_plans() -> usize { + SHAPE_PLANS.with(|cell| cell.try_borrow().map_or(0, |p| p.len())) +} + /// Glyph id of the first of `chars` the face has (0 = .notdef). pub fn first_glyph(face: &FontFace, chars: &[char]) -> u32 { chars.iter().map(|c| face.glyph_for(*c)).find(|g| *g != 0).unwrap_or(0) @@ -293,6 +357,62 @@ mod tests { assert!(g.iter().all(|g| g.x_advance > 0 && g.gid != 0)); } + /// Shaping with a plan kept from an earlier run gives what the shaper's own plan gives. + #[test] + fn kept_shape_plans_shape_as_fresh_ones() { + let fresh = |face: &FontFace, text: &str, features: &[Feature], rtl: bool| -> Vec<(u32, u32, i32, i32, i32)> { + let Some(hb) = face.hb() else { return Vec::new() }; + let shaper = face.shaper.shaper(&hb).instance(face.instance.as_ref()).build(); + let mut buf = UnicodeBuffer::new(); + buf.push_str(text); + buf.guess_segment_properties(); + buf.set_direction(if rtl { Direction::RightToLeft } else { Direction::LeftToRight }); + let gb = shaper.shape(buf, ShapeOptions::new().features(features)); + gb.glyph_infos().iter().zip(gb.glyph_positions()).map(|(i, p)| (i.glyph_id, i.cluster, p.x_advance, p.x_offset, p.y_offset)).collect() + }; + let kept = |face: &FontFace, text: &str, features: &[Feature], rtl: bool| -> Vec<(u32, u32, i32, i32, i32)> { + shape_dir(face, text, features, &|c| c, rtl).iter().map(|g| (g.gid, g.cluster as u32, g.x_advance, g.x_offset, g.y_offset)).collect() + }; + let db = FontDb::global(); + let faces = [db.face(DEFAULT_FAMILY, "Regular"), db.face(DEFAULT_FAMILY, "Italic"), db.face("Source Sans 3", "Regular")]; + let features = + [vec![], vec![feature("-liga").unwrap()], vec![feature_value(b"kern", 0)], vec![feature("smcp").unwrap(), feature("-kern").unwrap()]]; + for _ in 0..2 { + for face in &faces { + for f in &features { + for (text, rtl) in [("office affine", false), ("12, 34.", false), ("AVATAR Wolf", false), ("abc", true), ("", false)] { + assert_eq!(kept(face, text, f, rtl), fresh(face, text, f, rtl), "{} {text:?} {f:?} {rtl}", face.family); + } + } + } + } + } + + /// However many feature sets and directions a thread shapes with, it keeps at most + /// [`MAX_SHAPE_PLANS`] plans, and shaping still matches a fresh plan once old ones are dropped. + #[test] + fn kept_shape_plans_are_bounded() { + let face = FontDb::global().face(DEFAULT_FAMILY, "Regular"); + let fresh = |features: &[Feature], rtl: bool| -> Vec<(u32, i32)> { + let Some(hb) = face.hb() else { return Vec::new() }; + let shaper = face.shaper.shaper(&hb).instance(face.instance.as_ref()).build(); + let mut buf = UnicodeBuffer::new(); + buf.push_str("office"); + buf.guess_segment_properties(); + buf.set_direction(if rtl { Direction::RightToLeft } else { Direction::LeftToRight }); + let gb = shaper.shape(buf, ShapeOptions::new().features(features)); + gb.glyph_infos().iter().zip(gb.glyph_positions()).map(|(i, p)| (i.glyph_id, p.x_advance)).collect() + }; + for n in 0..3 * MAX_SHAPE_PLANS as u32 { + let features = [feature_value(b"kern", n % 2), feature_value(b"liga", n)]; + let rtl = n % 3 == 0; + let kept: Vec<(u32, i32)> = shape_dir(&face, "office", &features, &|c| c, rtl).iter().map(|g| (g.gid, g.x_advance)).collect(); + assert_eq!(kept, fresh(&features, rtl), "{n}"); + assert!(kept_shape_plans() <= MAX_SHAPE_PLANS, "{}", kept_shape_plans()); + } + assert_eq!(kept_shape_plans(), MAX_SHAPE_PLANS); + } + #[test] fn ligatures_can_be_disabled() { let face = FontDb::global().face(DEFAULT_FAMILY, "Regular");