From 28c30d6d9713523ebb177689bdbb78884c81c2be Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 13:46:12 -0800 Subject: [PATCH 01/10] Count the subtable cache rank once per lookup application LazySubtables::has_cache counted the subtables that parse before an index by scanning from 0 on every call, and the walk asks for it at every class-based context subtable past the eighth for every glyph, so a lookup of N such subtables cost O(N^2) per glyph. A GSUB of 32,000 offsets to one ChainContextFormat2 took 7 s to shape "aaaa". The count now advances from where the last call left it and stops at the eighth subtable that parses, which fixes the cutoff for the rest of the application: each subtable is counted at most once per lookup application. A unit test counts subtable reads per visit. The walk also reads each subtable's slot once per visit (the accelerator check now goes through LazySubtables::get_admitted), a subtable that fails to parse takes no room in the parsed list, the list reserves room for every subtable once a walk has parsed eight (as parsing them all up front did, instead of copying at each doubling), and parsing goes through a Subtable trait instead of function pointers. --- src/shape/gpos.rs | 22 ++-- src/shape/gsub_parsed.rs | 20 ++-- src/shape/lazy.rs | 251 ++++++++++++++++++++++++++++++++------- 3 files changed, 229 insertions(+), 64 deletions(-) diff --git a/src/shape/gpos.rs b/src/shape/gpos.rs index 0330870..d1ec86a 100644 --- a/src/shape/gpos.rs +++ b/src/shape/gpos.rs @@ -32,7 +32,7 @@ use alloc::vec::Vec; use super::attach::{self, Attach, AttachSubtable, LookupCx}; use super::glyph_flags::FlagCx; -use super::lazy::LazySubtables; +use super::lazy::{LazySubtables, Subtable}; use super::{ feature_disabled, filter_for_lookup, resolve_extension, Feature, LookupBudget, VarCtx, MAX_NESTED_DEPTH, @@ -203,9 +203,11 @@ enum PosSubtable<'a> { Chain(ChainContextPos<'a>), } -impl PosSubtable<'_> { - /// True for a class-based context subtable, the one kind whose - /// matching reads [`MatchContext::rule_set_digests`]. +impl<'a> Subtable<'a> for PosSubtable<'a> { + fn parse(lookup: &Lookup<'a>, index: u16) -> Option { + parse_subtable(lookup, index) + } + fn reads_rule_set_digests(&self) -> bool { matches!( self, @@ -274,7 +276,7 @@ fn apply_lookup( if !accel.may_apply(run.iter().map(|g| g.id)) { return; } - let mut subtables = LazySubtables::new(lookup, parse_subtable); + let mut subtables = LazySubtables::new(lookup); let state = LookupState::new(&lookup, cx.gdef, stage.joiners, stage.index); // Each lookup starts with no remembered mark base, as in HarfBuzz. att.reset_base_cache(); @@ -338,7 +340,7 @@ fn apply_lookup_at( if !accel.may_have_cheaply(glyphs[at].glyph_id as u16) { return; } - let mut subtables = LazySubtables::new(lookup, parse_subtable); + let mut subtables = LazySubtables::new(lookup); let state = LookupState::new(&lookup, cx.gdef, joiners, lookup_index); let (state, accel) = (&state, &accel); apply_subtables_at( @@ -376,12 +378,8 @@ fn apply_subtables_at( let horizontal = att.direction.is_horizontal(); let id = glyphs.get(at)?.glyph_id as u16; for index in 0..subtables.len() { - if !accel.subtable_may_start(subtables.is_parsed(index), usize::from(index), id) { - continue; - } - let Some((sub, digests)) = - subtables.get_with_digests(index, PosSubtable::reads_rule_set_digests) - else { + let admits = |parsed| accel.subtable_may_start(parsed, usize::from(index), id); + let Some((sub, digests)) = subtables.get_admitted(index, admits) else { continue; }; let mcx = &state.mcx.with_rule_set_digests(digests); diff --git a/src/shape/gsub_parsed.rs b/src/shape/gsub_parsed.rs index fa1b775..a5949fe 100644 --- a/src/shape/gsub_parsed.rs +++ b/src/shape/gsub_parsed.rs @@ -5,7 +5,7 @@ use super::gsub::{apply_gsub_chain_context_at, apply_gsub_context_at, GsubCx}; use super::gsub_buffer::GsubBuffer; -use super::lazy::LazySubtables; +use super::lazy::{LazySubtables, Subtable}; use super::{lig, resolve_extension, LookupBudget}; use crate::tables::gdef::Gdef; use crate::tables::gsub::{ @@ -33,9 +33,11 @@ pub(super) enum ParsedGsubSubtable<'a> { ReverseChained(ReverseChain<'a>), } -impl ParsedGsubSubtable<'_> { - /// True for a class-based context subtable, the one kind whose - /// matching reads [`MatchContext::rule_set_digests`]. +impl<'a> Subtable<'a> for ParsedGsubSubtable<'a> { + fn parse(lookup: &Lookup<'a>, index: u16) -> Option { + parse_subtable(lookup, index) + } + fn reads_rule_set_digests(&self) -> bool { matches!( self, @@ -50,7 +52,7 @@ pub(super) type GsubSubtables<'a> = LazySubtables<'a, ParsedGsubSubtable<'a>>; /// The subtables of `lookup`, none parsed yet. pub(super) fn lazy_subtables(lookup: Lookup<'_>) -> GsubSubtables<'_> { - LazySubtables::new(lookup, parse_subtable) + LazySubtables::new(lookup) } /// Parses subtable `sub_idx` of `lookup`, looking through an Extension @@ -123,12 +125,8 @@ pub(super) fn apply_parsed_lookup_at( let id = cur.glyph_id as u16; let at = buf.cursor(); for index in 0..subtables.len() { - if !accel.subtable_may_start(subtables.is_parsed(index), usize::from(index), id) { - continue; - } - let Some((sub, digests)) = - subtables.get_with_digests(index, ParsedGsubSubtable::reads_rule_set_digests) - else { + let admits = |parsed| accel.subtable_may_start(parsed, usize::from(index), id); + let Some((sub, digests)) = subtables.get_admitted(index, admits) else { continue; }; let mcx = &mcx.with_rule_set_digests(digests); diff --git a/src/shape/lazy.rs b/src/shape/lazy.rs index 2b261fd..672f832 100644 --- a/src/shape/lazy.rs +++ b/src/shape/lazy.rs @@ -16,37 +16,64 @@ use crate::tables::layout::Lookup; /// small lookup is applied without allocating them. const INLINE_SLOTS: usize = 8; -/// The subtables of one lookup, parsed on demand with `parse`. +/// The slot of a subtable not parsed yet. +const UNPARSED: u32 = 0; +/// The slot of a subtable that does not parse (the shaper drops those). +const FAILED: u32 = u32::MAX; + +/// A kind of lookup subtable that [`LazySubtables`] parses on demand. +pub(super) trait Subtable<'a>: Sized { + /// Subtable `index` of `lookup`, `None` when it does not parse. + fn parse(lookup: &Lookup<'a>, index: u16) -> Option; + + /// True for a class-based context subtable, the one kind whose + /// matching reads + /// [`crate::tables::layout::MatchContext::rule_set_digests`]. + fn reads_rule_set_digests(&self) -> bool; +} + +/// The subtables of one lookup, each parsed the first time it is asked +/// for. pub(super) struct LazySubtables<'a, T> { lookup: Lookup<'a>, - /// For each of the first [`INLINE_SLOTS`] subtable indices, 0 - /// before it is parsed, else one more than its position in - /// `parsed`. - first: [u16; INLINE_SLOTS], + /// For each of the first [`INLINE_SLOTS`] subtable indices, + /// [`UNPARSED`], [`FAILED`], or one more than the subtable's + /// position in `parsed`. + first: [u32; INLINE_SLOTS], /// The same for the other subtables. - rest: Vec, - /// The subtables parsed so far, `None` for one that failed to - /// parse (the shaper drops those). - parsed: Vec>, - parse: fn(&Lookup<'a>, u16) -> Option, + rest: Vec, + /// The subtables that parsed so far. + parsed: Vec, + /// [`Self::has_cache`] has counted the subtables before this index, + /// stopping one past the [`SUBTABLE_CACHES`]th that parses. + counted: u16, + /// How many of the subtables before `counted` parse: at most + /// [`SUBTABLE_CACHES`]. + counted_parsing: usize, + /// Subtable reads, for the test that bounds the work per glyph. + #[cfg(test)] + reads: usize, } -impl<'a, T> LazySubtables<'a, T> { +impl<'a, T: Subtable<'a>> LazySubtables<'a, T> { /// The subtables of `lookup`, none parsed yet. - pub(super) fn new(lookup: Lookup<'a>, parse: fn(&Lookup<'a>, u16) -> Option) -> Self { + pub(super) fn new(lookup: Lookup<'a>) -> Self { let count = usize::from(lookup.subtable_count()); Self { - first: [0; INLINE_SLOTS], - rest: alloc::vec![0; count.saturating_sub(INLINE_SLOTS)], + first: [UNPARSED; INLINE_SLOTS], + rest: alloc::vec![UNPARSED; count.saturating_sub(INLINE_SLOTS)], lookup, parsed: Vec::new(), - parse, + counted: 0, + counted_parsing: 0, + #[cfg(test)] + reads: 0, } } /// The slot of subtable `index`, `None` past the last subtable. #[inline] - fn slot(&self, index: u16) -> Option { + fn slot(&self, index: u16) -> Option { if index >= self.len() { return None; } @@ -57,7 +84,7 @@ impl<'a, T> LazySubtables<'a, T> { } } - fn set_slot(&mut self, index: u16, slot: u16) { + fn set_slot(&mut self, index: u16, slot: u32) { let i = usize::from(index); let cell = match i.checked_sub(INLINE_SLOTS) { None => self.first.get_mut(i), @@ -77,44 +104,83 @@ impl<'a, T> LazySubtables<'a, T> { /// parsed). #[inline] pub(super) fn is_parsed(&self, index: u16) -> bool { - self.slot(index).is_some_and(|s| s != 0) + self.slot(index).is_some_and(|s| s != UNPARSED) } /// Subtable `index`, parsed now if it was not yet; `None` when it /// does not parse. #[inline] pub(super) fn get(&mut self, index: u16) -> Option<&T> { + #[cfg(test)] + { + self.reads += 1; + } let slot = self.slot(index)?; - let at = if slot == 0 { - let parsed = (self.parse)(&self.lookup, index); - self.parsed.push(parsed); - // At most one entry per subtable, so the position fits. - let at = self.parsed.len() - 1; - self.set_slot(index, (at + 1) as u16); - at - } else { - usize::from(slot - 1) + let at = self.position(index, slot)?; + self.parsed.get(at) + } + + /// Where subtable `index`, whose slot is `slot`, is in `parsed`, + /// parsing it now if it was not yet; `None` when it does not parse. + #[inline] + fn position(&mut self, index: u16, slot: u32) -> Option { + match slot { + UNPARSED => self.parse(index), + FAILED => None, + slot => Some(slot as usize - 1), + } + } + + /// Parses subtable `index`, which is not parsed yet, and returns + /// where it is in `parsed`, `None` when it does not parse. Only the + /// subtables that parse take room in `parsed`. + fn parse(&mut self, index: u16) -> Option { + let Some(parsed) = T::parse(&self.lookup, index) else { + self.set_slot(index, FAILED); + return None; }; - self.parsed.get(at)?.as_ref() + if self.parsed.len() == INLINE_SLOTS { + // A walk that has parsed this many subtables tends to parse + // most of them: room for all of them at once, as parsing + // every subtable up front took, costs less than copying the + // parsed ones at each doubling. + self.parsed + .reserve_exact(usize::from(self.len()) - INLINE_SLOTS); + } + self.parsed.push(parsed); + // At most one entry per subtable, and there are at most + // u16::MAX subtables, so the slot is neither UNPARSED nor FAILED. + let at = self.parsed.len() - 1; + self.set_slot(index, at as u32 + 1); + Some(at) } - /// Subtable `index`, parsed now if it was not yet, and whether - /// HarfBuzz gives it a cache, as [`Self::has_cache`] tells for a - /// subtable `reads_digests` says reads one; `None` when it does not - /// parse. + /// Subtable `index` if `admits` lets it through, told whether the + /// subtable is parsed already, parsed now if it was not yet, and + /// whether HarfBuzz gives it a cache, as [`Self::has_cache`] tells + /// for a subtable that reads one. `None` when `admits` turns it + /// away, which parses nothing, or when it does not parse. The walk + /// over a lookup's subtables asks this of each at every glyph, with + /// one look at the subtable's slot. #[inline] - pub(super) fn get_with_digests( + pub(super) fn get_admitted( &mut self, index: u16, - reads_digests: fn(&T) -> bool, + admits: impl FnOnce(bool) -> bool, ) -> Option<(&T, bool)> { - if usize::from(index) >= SUBTABLE_CACHES - && reads_digests(self.get(index)?) - && !self.has_cache(index) + #[cfg(test)] { - return self.get(index).map(|t| (t, false)); + self.reads += 1; } - self.get(index).map(|t| (t, true)) + let slot = self.slot(index)?; + if !admits(slot != UNPARSED) { + return None; + } + let at = self.position(index, slot)?; + let digests = usize::from(index) < SUBTABLE_CACHES + || !self.parsed.get(at)?.reads_rule_set_digests() + || self.has_cache(index); + self.parsed.get(at).map(|t| (t, digests)) } /// Whether subtable `index`, one that parses, is among the first @@ -122,11 +188,114 @@ impl<'a, T> LazySubtables<'a, T> { /// HarfBuzz gives a cache (see /// [`crate::tables::layout::MatchContext::with_rule_set_digests`]). /// Parses the subtables before it when that takes counting. + /// + /// The count is kept across calls, so over one application of the + /// lookup each subtable is counted at most once, however many + /// glyphs reach however many subtables: once the + /// [`SUBTABLE_CACHES`]th subtable that parses is found, every later + /// one is known to have no cache. pub(super) fn has_cache(&mut self, index: u16) -> bool { if usize::from(index) < SUBTABLE_CACHES { return true; } - let failed = (0..index).filter(|&i| self.get(i).is_none()).count(); - usize::from(index) - failed < SUBTABLE_CACHES + while self.counted < index && self.counted_parsing < SUBTABLE_CACHES { + if self.get(self.counted).is_some() { + self.counted_parsing += 1; + } + self.counted += 1; + } + // Either fewer than SUBTABLE_CACHES of the subtables before + // `counted`, which has reached `index`, parse, or `counted` is + // one past the SUBTABLE_CACHES-th that parses and only the + // subtables before it have a cache. + self.counted_parsing < SUBTABLE_CACHES || index < self.counted + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tables::layout::LookupList; + use alloc::vec; + + /// A LookupList of one lookup of `count` one-byte subtables, of + /// which every third fails to parse. + fn lookup_list(count: u16) -> Vec { + let mut out = vec![]; + for v in [1, 4, 1, 0, count] { + out.extend_from_slice(&u16::to_be_bytes(v)); + } + let body = 6 + 2 * usize::from(count); + for i in 0..usize::from(count) { + out.extend_from_slice(&u16::try_from(body + i).unwrap().to_be_bytes()); + } + out.extend((0..count).map(|i| u8::from(i % 3 != 0))); + out + } + + /// A one-byte subtable that parses when its byte is 1, and reads + /// rule set digests. + struct Byte; + + impl<'a> Subtable<'a> for Byte { + fn parse(lookup: &Lookup<'a>, index: u16) -> Option { + let byte = lookup.subtable_bytes(index)?.first().copied(); + (byte == Some(1)).then_some(Byte) + } + + fn reads_rule_set_digests(&self) -> bool { + true + } + } + + #[test] + fn the_cache_rank_counts_the_subtables_that_parse_in_any_order() { + let count = 40; + let data = lookup_list(count); + let list = LookupList::parse(&data).unwrap(); + // Whether fewer than SUBTABLE_CACHES subtables before `index` + // parse, counted afresh. + let scan = |index: u16| (0..index).filter(|i| i % 3 != 0).count() < SUBTABLE_CACHES; + let orders: [Vec; 3] = [ + (0..count).collect(), + (0..count).rev().collect(), + (0..count).map(|i| (i * 17) % count).collect(), + ]; + for order in orders { + let mut subtables = LazySubtables::::new(list.get(0).unwrap()); + for &i in order.iter().chain(&order) { + let got = subtables + .get_admitted(i, |_| true) + .map(|(_, digests)| digests); + assert_eq!(got, (i % 3 != 0).then(|| scan(i)), "subtable {i}"); + } + } + } + + #[test] + fn the_cache_rank_costs_constant_work_per_subtable_visit() { + // Every glyph reaches every subtable of a lookup of many + // class-based context subtables, and each visit past the first + // SUBTABLE_CACHES asks for the cache rank. Counting the + // subtables before each one made a glyph's walk quadratic in + // their number. + let count = 6000; + let data = lookup_list(count); + let list = LookupList::parse(&data).unwrap(); + let mut subtables = LazySubtables::::new(list.get(0).unwrap()); + let glyphs = 4; + for _ in 0..glyphs { + for i in 0..count { + let _ = subtables.get_admitted(i, |_| true); + } + } + let visits = glyphs * usize::from(count); + // One read a visit, plus counting each subtable at most once. + let bound = visits + usize::from(count); + assert!( + subtables.reads <= bound, + "{} subtable reads for {visits} visits, at most {bound}", + subtables.reads + ); } } From d3157eba2ef69927e26ff9f366154c574b20e840 Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 13:46:45 -0800 Subject: [PATCH 02/10] Bound the lookup accelerators by the table's size LookupAccel::build kept a 24-byte digest per subtable for every lookup index a run applied, even after both reading budgets were spent, so what a Font kept grew with lookup indices times subtables rather than with the table: a 66 KB GSUB of 500 indices naming one lookup of 32,000 subtables kept 375 MB, and lookups that overlap in the table can make a 96 KB GSUB keep 1.5 GB. - Each digest kept costs one entry of the table's budget (twice the table's length plus 64 Ki), taken before any coverage is read. A lookup the budget cannot pay for keeps no digest and admits every glyph, which only means less skipping. - Lookup indices with the same LookupList offset share one accelerator. Fonts compilers write list each lookup once at increasing offsets, which needs no sorting to tell. - A lookup whose subtables all have the lookup's digest (one subtable, or many sharing a coverage) keeps only that: the walk checks the lookup's digest before any subtable, so theirs would rule nothing more out. tests/hostile_layout_caches.rs builds such tables on Open Sans and checks the bytes a used Font keeps and that it shapes as a fresh Font. --- src/font/cache.rs | 30 +-- src/ot/layout_select.rs | 18 +- src/shape/pipeline.rs | 4 +- src/tables/layout/accel.rs | 242 ++++++++++++++++++++---- src/tables/layout/lookup_list.rs | 11 +- tests/hostile_layout_caches.rs | 310 +++++++++++++++++++++++++++++++ 6 files changed, 549 insertions(+), 66 deletions(-) create mode 100644 tests/hostile_layout_caches.rs diff --git a/src/font/cache.rs b/src/font/cache.rs index fa4a2b6..ab47bfa 100644 --- a/src/font/cache.rs +++ b/src/font/cache.rs @@ -31,11 +31,13 @@ //! output does not depend on what is cached. //! //! Memory is bounded per font. For each of GSUB and GPOS: one pointer -//! per lookup, 24 bytes per subtable of each lookup a run reached, at -//! most 16 resolved language systems of at most 1024 features and 16384 -//! lookup indices each, and at most 64 stage plans of at most 4096 -//! lookups each. For the glyph caches: at most [`GLYPH_CACHE_MAX`] -//! four-byte entries for each of the three, whatever the glyph count. +//! per lookup; 24 bytes per subtable of each lookup a run reached, for +//! at most twice as many subtables as the table has bytes plus 64 Ki +//! (lookup indices that name one lookup share its digests); at most 16 +//! resolved language systems of at most 1024 features and 16384 lookup +//! indices each; and at most 64 stage plans of at most 4096 lookups +//! each. For the glyph caches: at most [`GLYPH_CACHE_MAX`] four-byte +//! entries for each of the three, whatever the glyph count. use alloc::boxed::Box; use alloc::sync::Arc; @@ -44,7 +46,7 @@ use core::sync::atomic::{AtomicBool, AtomicU32, Ordering}; use crate::sync::OnceBox; use crate::tables::layout::accel::LayoutCache; -use crate::tables::layout::LayoutTable; +use crate::tables::layout::{LayoutTable, LookupList}; /// The caches of one [`Font`](super::Font). pub(crate) struct FontCaches { @@ -148,18 +150,18 @@ impl FaceCache { } } - /// What the font keeps for its GSUB, which has `lookup_count` - /// lookups in `table_len` bytes. - pub(crate) fn gsub(&self, lookup_count: u16, table_len: usize) -> &LayoutCache { + /// What the font keeps for its GSUB, whose LookupList is `lookups`, + /// in `table_len` bytes. + pub(crate) fn gsub(&self, lookups: &LookupList<'_>, table_len: usize) -> &LayoutCache { self.gsub - .get_or_init(|| LayoutCache::new(LayoutTable::Gsub, lookup_count, table_len)) + .get_or_init(|| LayoutCache::new(LayoutTable::Gsub, lookups, table_len)) } - /// What the font keeps for its GPOS, which has `lookup_count` - /// lookups in `table_len` bytes. - pub(crate) fn gpos(&self, lookup_count: u16, table_len: usize) -> &LayoutCache { + /// What the font keeps for its GPOS, whose LookupList is `lookups`, + /// in `table_len` bytes. + pub(crate) fn gpos(&self, lookups: &LookupList<'_>, table_len: usize) -> &LayoutCache { self.gpos - .get_or_init(|| LayoutCache::new(LayoutTable::Gpos, lookup_count, table_len)) + .get_or_init(|| LayoutCache::new(LayoutTable::Gpos, lookups, table_len)) } fn heap_bytes(&self) -> usize { diff --git a/src/ot/layout_select.rs b/src/ot/layout_select.rs index aa6f5f0..217f33d 100644 --- a/src/ot/layout_select.rs +++ b/src/ot/layout_select.rs @@ -1197,28 +1197,18 @@ mod tests { for data in fonts { let face = Face::parse_bytes(data, 0).unwrap(); for gsub in [true, false] { - let (script_list, list, variations, count) = if gsub { + let (script_list, list, variations, lookups) = if gsub { let Some(t) = face.gsub().unwrap() else { continue; }; let v = t.feature_variations().ok().flatten(); - ( - *t.script_list(), - *t.feature_list(), - v, - t.lookup_list().len(), - ) + (*t.script_list(), *t.feature_list(), v, *t.lookup_list()) } else { let Some(t) = face.gpos().unwrap() else { continue; }; let v = t.feature_variations().ok().flatten(); - ( - *t.script_list(), - *t.feature_list(), - v, - t.lookup_list().len(), - ) + (*t.script_list(), *t.feature_list(), v, *t.lookup_list()) }; let table = if gsub { LayoutTable::Gsub @@ -1236,7 +1226,7 @@ mod tests { tags.extend(unknown); for record in records { let features = ActiveFeatures::new(list, record); - let cache = LayoutCache::new(table, count, 1 << 20); + let cache = LayoutCache::new(table, &lookups, 1 << 20); for languages in languages { let walk = LayoutView { script_list, diff --git a/src/shape/pipeline.rs b/src/shape/pipeline.rs index 748b4c2..5a876cb 100644 --- a/src/shape/pipeline.rs +++ b/src/shape/pipeline.rs @@ -262,7 +262,7 @@ pub fn shape(font: &Font<'_>, buffer: &Buffer, features: &[Feature]) -> Result, buffer: &Buffer, features: &[Feature]) -> Result Self { + Self { + digest: Digest::FULL, + subtables: Box::default(), + } + } + /// Builds the accelerator of `lookup`, a lookup of `table`, reading /// at most what the lookup's budget and `table_budget`, which the /// table's other lookups share, leave. + /// + /// Each subtable digest the accelerator keeps costs one entry of + /// `table_budget` too, taken before any coverage is read, so what a + /// table's accelerators keep is bounded by the table's size however + /// many lookups apply. A lookup the budget cannot pay for keeps no + /// digest and admits every glyph. + /// + /// A lookup whose subtables all have the lookup's own digest (a + /// single subtable, or many that share one coverage) keeps only + /// that: every walk checks a glyph against the lookup's digest + /// before it tries a subtable, so theirs would rule nothing more + /// out. pub(crate) fn build( table: LayoutTable, lookup: &Lookup<'_>, table_budget: &AtomicUsize, ) -> Self { let count = usize::from(lookup.subtable_count()); + if !spend(table_budget, count) { + return Self::admit_all(); + } let mut budget = MIN_COVERAGE_BUDGET.max(count.saturating_mul(COVERAGE_BUDGET_PER_SUBTABLE)); let mut digest = Digest::EMPTY; @@ -170,10 +202,12 @@ impl LookupAccel { d }) .collect(); - Self { - digest, - subtables: subtables.into_boxed_slice(), - } + let subtables = if subtables.iter().all(|d| *d == digest) { + Box::default() + } else { + subtables.into_boxed_slice() + }; + Self { digest, subtables } } /// The digest of every glyph a subtable of the lookup can start at. @@ -320,25 +354,37 @@ impl Accel<'_, '_> { pub(crate) struct LookupAccels { table: LayoutTable, lookups: Box<[OnceBox]>, - /// Coverage entries the accelerators may still read. + /// For a table whose LookupList names one lookup by several indices: + /// for each index, the first index with the same offset, whose + /// accelerator it shares. Empty when every index has its own offset. + shared: Box<[u16]>, + /// Coverage entries the accelerators may still read, and digests + /// they may still keep. budget: AtomicUsize, } impl LookupAccels { - /// Room for the `lookup_count` lookups of a `table` table of - /// `table_len` bytes, none built yet. - pub(crate) fn new(table: LayoutTable, lookup_count: u16, table_len: usize) -> Self { + /// Room for the lookups of `lookups`, the LookupList of a `table` + /// table of `table_len` bytes, none built yet. + pub(crate) fn new(table: LayoutTable, lookups: &LookupList<'_>, table_len: usize) -> Self { Self { table, - lookups: (0..lookup_count).map(|_| OnceBox::new()).collect(), + lookups: (0..lookups.len()).map(|_| OnceBox::new()).collect(), + shared: shared_lookups(lookups), budget: AtomicUsize::new(table_coverage_budget(table_len)), } } /// The accelerator of `lookup`, lookup `index` of the table, built - /// now if this is the first time it is asked for. Past the table's - /// lookups, its coverages are read for this one use. + /// now if this is the first time it or another index of the same + /// lookup is asked for. Past the table's lookups, its coverages are + /// read for this one use. pub(crate) fn get<'a>(&self, index: u16, lookup: &Lookup<'a>) -> Accel<'_, 'a> { + let index = self + .shared + .get(usize::from(index)) + .copied() + .unwrap_or(index); match self.lookups.get(usize::from(index)) { Some(slot) => Accel::Built( slot.get_or_init(|| LookupAccel::build(self.table, lookup, &self.budget)), @@ -355,7 +401,8 @@ impl LookupAccels { /// Heap bytes the accelerators hold. pub(crate) fn heap_bytes(&self) -> usize { - let slots = self.lookups.len() * core::mem::size_of::>(); + let slots = self.lookups.len() * core::mem::size_of::>() + + self.shared.len() * 2; let built: usize = self .lookups .iter() @@ -389,11 +436,11 @@ pub(crate) struct LayoutCache { } impl LayoutCache { - /// An empty cache for a `table` table of `lookup_count` lookups and - /// `table_len` bytes. - pub(crate) fn new(table: LayoutTable, lookup_count: u16, table_len: usize) -> Self { + /// An empty cache for a `table` table of `table_len` bytes whose + /// LookupList is `lookups`. + pub(crate) fn new(table: LayoutTable, lookups: &LookupList<'_>, table_len: usize) -> Self { Self { - accels: LookupAccels::new(table, lookup_count, table_len), + accels: LookupAccels::new(table, lookups, table_len), maps: FeatureMaps::new(), plans: StagePlans::new(), } @@ -429,17 +476,8 @@ fn coverage_digest( ) -> Option { let entries = usize::from(cov.len()); let left = budget.checked_sub(entries)?; - // Spend from the shared budget with a compare-and-swap loop, so two - // threads building accelerators of one table cannot both take its - // last entries. - let mut shared = table_budget.load(Ordering::Relaxed); - loop { - let rest = shared.checked_sub(entries)?; - match table_budget.compare_exchange_weak(shared, rest, Ordering::Relaxed, Ordering::Relaxed) - { - Ok(_) => break, - Err(now) => shared = now, - } + if !spend(table_budget, entries) { + return None; } *budget = left; let mut d = Digest::EMPTY; @@ -447,6 +485,50 @@ fn coverage_digest( Some(d) } +/// Takes `amount` from the shared `budget`, or nothing and false when +/// less is left. A compare-and-swap loop, so two threads building +/// accelerators of one table cannot both take its last entries. +fn spend(budget: &AtomicUsize, amount: usize) -> bool { + let mut left = budget.load(Ordering::Relaxed); + loop { + let Some(rest) = left.checked_sub(amount) else { + return false; + }; + match budget.compare_exchange_weak(left, rest, Ordering::Relaxed, Ordering::Relaxed) { + Ok(_) => return true, + Err(now) => left = now, + } + } +} + +/// For a LookupList that names one lookup by several indices, the +/// first index of each index's offset; empty when every index has its +/// own offset, as in fonts that compilers write. Lookups are usually +/// listed at increasing offsets, which needs no sorting to tell. +fn shared_lookups(lookups: &LookupList<'_>) -> Box<[u16]> { + let offset = |index: u16| lookups.lookup_offset(index).unwrap_or(0); + let count = lookups.len(); + if (1..count).all(|i| offset(i - 1) < offset(i)) { + return Box::default(); + } + let mut by_offset: Vec<(u16, u16)> = (0..count).map(|i| (offset(i), i)).collect(); + by_offset.sort_unstable(); + let mut first: Vec = (0..count).collect(); + for pair in by_offset.windows(2) { + if let [(a, earlier), (b, later)] = *pair { + if a == b { + // Sorted by index within one offset, so `earlier` + // already points at the first index of the offset. + first[usize::from(later)] = first[usize::from(earlier)]; + } + } + } + if first.iter().enumerate().all(|(i, &f)| usize::from(f) == i) { + return Box::default(); + } + first.into_boxed_slice() +} + /// The coverage the subtable at `bytes`, of lookup type `lookup_type` /// in `table`, looks the cursor glyph up in before anything else, or /// `None` when it cannot be read or the subtable has none sigilbuzz @@ -516,7 +598,6 @@ fn primary_coverage(table: LayoutTable, lookup_type: u16, bytes: &[u8]) -> Optio #[cfg(test)] mod tests { use super::*; - use crate::tables::layout::LookupList; use alloc::vec; #[test] @@ -630,6 +711,22 @@ mod tests { assert!(!accel.digest().may_have(5000)); } + #[test] + fn subtables_with_the_lookup_digest_keep_none_of_their_own() { + for subs in [vec![single(&[10, 20])], vec![single(&[10, 20]); 3]] { + let data = lookup_list(1, &subs); + let list = LookupList::parse(&data).unwrap(); + let accel = LookupAccel::build( + LayoutTable::Gsub, + &list.get(0).unwrap(), + &AtomicUsize::new(usize::MAX), + ); + assert!(accel.subtables.is_empty()); + assert_eq!(*accel.digest(), Digest::of([10, 20])); + assert!(accel.subtable_may_have(subs.len() - 1, 20)); + } + } + #[test] fn a_shared_huge_coverage_admits_everything_past_the_budget() { // Many subtables share one big coverage: past the budget the @@ -660,19 +757,96 @@ mod tests { let first = LookupAccel::build(LayoutTable::Gsub, &lookup, &budget); let second = LookupAccel::build(LayoutTable::Gsub, &lookup, &budget); let third = LookupAccel::build(LayoutTable::Gsub, &lookup, &budget); - assert!(!first.subtable_may_have(0, 65_001)); - assert!(!second.subtable_may_have(0, 65_001)); - assert!(third.subtable_may_have(0, 65_001)); - assert_eq!(budget.load(Ordering::Relaxed), 2000); + // One subtable each, so each keeps only the lookup's digest. + assert!(!first.digest().may_have(65_001)); + assert!(!second.digest().may_have(65_001)); + assert!(third.digest().may_have(65_001)); + // Three digests kept, two coverages read. + assert_eq!(budget.load(Ordering::Relaxed), 2000 - 3); assert_eq!(table_coverage_budget(1000), 2000 + (1 << 16)); } + #[test] + fn kept_digests_are_paid_from_the_table_budget() { + // 500 subtables of one-glyph coverages: keeping their digests + // costs 500 entries, and reading their coverages 500 more. + let subs: Vec> = (0..500).map(|g| single(&[g])).collect(); + let data = lookup_list(1, &subs); + let list = LookupList::parse(&data).unwrap(); + let lookup = list.get(0).unwrap(); + let budget = AtomicUsize::new(1499); + let kept = LookupAccel::build(LayoutTable::Gsub, &lookup, &budget); + assert_eq!(kept.subtables.len(), 500); + assert!(kept.subtable_may_have(7, 7)); + assert!(!kept.subtable_may_have(7, 8)); + assert_eq!(budget.load(Ordering::Relaxed), 499); + // What is left cannot pay for 500 digests: the next build keeps + // none, reads nothing, and admits every glyph at every subtable. + let none = LookupAccel::build(LayoutTable::Gsub, &lookup, &budget); + assert!(none.subtables.is_empty()); + assert_eq!(*none.digest(), Digest::FULL); + assert!(none.subtable_may_have(7, 8)); + assert_eq!(budget.load(Ordering::Relaxed), 499); + } + + /// A LookupList whose index `i` names lookup `targets[i]` of + /// `lookups`, each a lookup of single substitutions `subtables`. + fn list_of(targets: &[usize], lookups: &[&[Vec]]) -> Vec { + // Each lookup as `lookup_list` lays it out, past the list's + // count and offset. + let bodies: Vec> = lookups + .iter() + .map(|subtables| lookup_list(1, subtables)[4..].to_vec()) + .collect(); + let mut at = 2 + 2 * targets.len(); + let mut starts = vec![]; + for body in &bodies { + starts.push(at); + at += body.len(); + } + let mut out = vec![]; + push16(&mut out, targets.len() as u16); + for &t in targets { + push16(&mut out, starts[t] as u16); + } + for body in &bodies { + out.extend_from_slice(body); + } + out + } + + #[test] + fn indices_of_one_lookup_share_its_accelerator() { + let a = [single(&[10])]; + let b = [single(&[20]), single(&[30])]; + // Indices 0, 2 and 3 name lookup a, index 1 names lookup b. + let data = list_of(&[0, 1, 0, 0], &[&a, &b]); + let list = LookupList::parse(&data).unwrap(); + assert_eq!(&*shared_lookups(&list), &[0, 1, 0, 0]); + let accels = LookupAccels::new(LayoutTable::Gsub, &list, data.len()); + let built = |i: u16| match accels.get(i, &list.get(i).unwrap()) { + Accel::Built(a) => core::ptr::from_ref(a), + Accel::Direct(_) => panic!("lookup {i} has no slot"), + }; + let first = built(2); + assert_eq!(built(0), first); + assert_eq!(built(3), first); + assert_ne!(built(1), first); + assert_eq!(accels.built(), 2); + // Lists that name each lookup once share nothing and keep no + // map, at increasing offsets or not. + for targets in [[0, 1], [1, 0]] { + let data = list_of(&targets, &[&a, &b]); + assert!(shared_lookups(&LookupList::parse(&data).unwrap()).is_empty()); + } + } + #[test] fn accelerators_are_built_once_and_agree_with_the_coverages() { let data = lookup_list(1, &[single(&[10]), single(&[3000])]); let list = LookupList::parse(&data).unwrap(); let lookup = list.get(0).unwrap(); - let accels = LookupAccels::new(LayoutTable::Gsub, 3, data.len()); + let accels = LookupAccels::new(LayoutTable::Gsub, &list, data.len()); assert_eq!(accels.built(), 0); let first = accel_for(None, LayoutTable::Gsub, 0, &lookup); assert!(matches!(first, Accel::Direct(_))); diff --git a/src/tables/layout/lookup_list.rs b/src/tables/layout/lookup_list.rs index 69bd2c3..0e445a6 100644 --- a/src/tables/layout/lookup_list.rs +++ b/src/tables/layout/lookup_list.rs @@ -65,12 +65,19 @@ impl<'a> LookupList<'a> { /// Returns the lookup at `index`, or `None` when out of range. #[must_use] pub fn get(&self, index: u16) -> Option> { + Lookup::parse_at(self.data, self.lookup_offset(index)?).ok() + } + + /// The offset of lookup `index` from the start of the list, or + /// `None` when out of range. Lookup indices with the same offset + /// name the same lookup. + #[must_use] + pub(crate) fn lookup_offset(&self, index: u16) -> Option { if index >= self.lookup_count { return None; } let off = self.offsets_off + index as usize * 2; - let lookup_off = u16::from_be_bytes([self.data[off], self.data[off + 1]]); - Lookup::parse_at(self.data, lookup_off).ok() + Some(u16::from_be_bytes([self.data[off], self.data[off + 1]])) } } diff --git a/tests/hostile_layout_caches.rs b/tests/hostile_layout_caches.rs new file mode 100644 index 0000000..a8961a7 --- /dev/null +++ b/tests/hostile_layout_caches.rs @@ -0,0 +1,310 @@ +//! What a `Font` keeps for its GSUB and GPOS is bounded by the tables' +//! size, and keeping it never changes the output, on tables built to +//! make the caches work hard: one lookup listed under many indices, +//! lookups that overlap in the table, and one subtable repeated +//! thousands of times in a lookup. +//! +//! Each test swaps Open Sans's GSUB or GPOS for a synthetic one, shapes +//! with one `Font` three times (its caches are built from the second +//! call on) and with a fresh `Font`, and counts the heap the used font +//! keeps with a per-thread counting allocator. + +use std::alloc::{GlobalAlloc, Layout, System}; +use std::cell::Cell; + +use sigilbuzz::{shape, Blob, Buffer, Face, Font}; + +const OPEN_SANS: &[u8] = include_bytes!("fixtures/opensans_regular.ttf"); + +/// Counts the bytes each thread has live, so tests running at once do +/// not see each other's allocations. +struct Counting; + +thread_local! { + static LIVE: Cell = const { Cell::new(0) }; +} + +fn add(bytes: isize) { + let _ = LIVE.try_with(|live| live.set(live.get() + bytes)); +} + +// SAFETY: every method forwards to `System` with the same arguments; the +// counter only reads the layouts. +unsafe impl GlobalAlloc for Counting { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + add(layout.size() as isize); + // SAFETY: forwarded as is. + unsafe { System.alloc(layout) } + } + + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + add(layout.size() as isize); + // SAFETY: forwarded as is. + unsafe { System.alloc_zeroed(layout) } + } + + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + add(-(layout.size() as isize)); + // SAFETY: forwarded as is. + unsafe { System.dealloc(ptr, layout) } + } + + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + add(new_size as isize - layout.size() as isize); + // SAFETY: forwarded as is. + unsafe { System.realloc(ptr, layout, new_size) } + } +} + +#[global_allocator] +static ALLOCATOR: Counting = Counting; + +fn live() -> isize { + LIVE.with(Cell::get) +} + +fn push16(out: &mut Vec, v: u16) { + out.extend_from_slice(&v.to_be_bytes()); +} + +fn u16s(values: &[u16]) -> Vec { + let mut out = Vec::with_capacity(2 * values.len()); + for &v in values { + push16(&mut out, v); + } + out +} + +fn offset16(v: usize) -> u16 { + u16::try_from(v).expect("offset fits in 16 bits") +} + +/// `font` with table `tag` replaced by `table`. +fn with_table(font: &[u8], tag: [u8; 4], table: &[u8]) -> Vec { + let be16 = |at: usize| usize::from(u16::from_be_bytes([font[at], font[at + 1]])); + let be32 = |at: usize| { + u32::from_be_bytes([font[at], font[at + 1], font[at + 2], font[at + 3]]) as usize + }; + let mut tables: Vec<([u8; 4], &[u8])> = (0..be16(4)) + .map(|i| 12 + 16 * i) + .map(|at| { + let t = [font[at], font[at + 1], font[at + 2], font[at + 3]]; + (t, &font[be32(at + 8)..be32(at + 8) + be32(at + 12)]) + }) + .filter(|(t, _)| *t != tag) + .collect(); + tables.push((tag, table)); + tables.sort_by_key(|(t, _)| *t); + let count = tables.len() as u16; + let selector = 15 - count.leading_zeros() as u16; + let range = (1u16 << selector) * 16; + let mut out = font[..4].to_vec(); + for v in [count, range, selector, count * 16 - range] { + push16(&mut out, v); + } + let mut data = Vec::new(); + let data_at = 12 + 16 * tables.len(); + for (t, bytes) in &tables { + out.extend_from_slice(t); + out.extend_from_slice(&0u32.to_be_bytes()); + out.extend_from_slice(&((data_at + data.len()) as u32).to_be_bytes()); + out.extend_from_slice(&(bytes.len() as u32).to_be_bytes()); + data.extend_from_slice(bytes); + while data.len() % 4 != 0 { + data.push(0); + } + } + out.extend(data); + out +} + +/// A GSUB or GPOS whose DFLT and latn default language systems list one +/// feature `tag` with `lookup_indices`, over `lookup_list`. +fn layout_table(tag: [u8; 4], lookup_indices: &[u16], lookup_list: &[u8]) -> Vec { + let mut script_list = u16s(&[2]); + script_list.extend_from_slice(b"DFLT"); + push16(&mut script_list, 14); + script_list.extend_from_slice(b"latn"); + push16(&mut script_list, 14); + // Script: default LangSys at 4, no others; LangSys: no required + // feature, feature 0. + script_list.extend(u16s(&[4, 0, 0, 0xFFFF, 1, 0])); + let mut feature_list = u16s(&[1]); + feature_list.extend_from_slice(&tag); + push16(&mut feature_list, 8); + feature_list.extend(u16s(&[0, offset16(lookup_indices.len())])); + feature_list.extend(u16s(lookup_indices)); + let script_at: usize = 10; + let feature_at = script_at + script_list.len(); + let lookups_at = feature_at + feature_list.len(); + let mut out = u16s(&[ + 1, + 0, + offset16(script_at), + offset16(feature_at), + offset16(lookups_at), + ]); + out.extend(script_list); + out.extend(feature_list); + out.extend_from_slice(lookup_list); + out +} + +/// A lookup of `lookup_type` whose `count` subtable offsets all point +/// at `subtable`. +fn repeated_lookup(lookup_type: u16, count: u16, subtable: &[u8]) -> Vec { + let body = 6 + 2 * usize::from(count); + let mut out = u16s(&[lookup_type, 0, count]); + out.extend(u16s(&vec![offset16(body); usize::from(count)])); + out.extend_from_slice(subtable); + out +} + +/// A LookupList of `indices` indices that all name `lookup`. +fn shared_lookup_list(indices: u16, lookup: &[u8]) -> Vec { + let at = offset16(2 + 2 * usize::from(indices)); + let mut out = u16s(&[indices]); + out.extend(u16s(&vec![at; usize::from(indices)])); + out.extend_from_slice(lookup); + out +} + +/// A chained context format 3 subtable whose input is `first` then +/// glyph 1, which never follows it: it reads its coverages and never +/// matches. +fn chain_context3(first: u16) -> Vec { + // No backtrack, two input coverages (at 14 and 20), no lookahead, + // no records. + let mut out = u16s(&[3, 0, 2, 14, 20, 0, 0]); + out.extend(u16s(&[1, 1, first])); + out.extend(u16s(&[1, 1, 1])); + out +} + +/// A chained context format 2 subtable covering `first`, with no class +/// sets: class-based, so its matching reads the subtable cache rank. +fn chain_context2(first: u16) -> Vec { + let mut out = u16s(&[2, 12, 18, 18, 18, 0]); + out.extend(u16s(&[1, 1, first])); + // ClassDef format 2 with no ranges. + out.extend(u16s(&[2, 0])); + out +} + +fn glyph(font: &[u8], c: char) -> u16 { + let face = Face::parse_bytes(font, 0).unwrap(); + face.cmap().unwrap().glyph_id(c).unwrap() +} + +type Shaped = Vec<(u32, u32, i32, i32, i32, i32)>; + +fn run(font: &Font<'_>, text: &str) -> Shaped { + let mut buffer = Buffer::new(); + buffer.push_str(text); + buffer.guess_segment_properties(); + shape(font, &buffer, &[]) + .unwrap() + .glyphs + .iter() + .map(|g| { + ( + g.glyph_id, + g.cluster, + g.x_advance, + g.y_advance, + g.x_offset, + g.y_offset, + ) + }) + .collect() +} + +/// Shapes `text` three times with one font of `data`, checks each +/// result against a fresh font's, and returns the heap the used font +/// keeps. +fn kept_bytes(data: &[u8], text: &str) -> usize { + let blob = Blob::new(data); + let fresh = run(&Font::new(Face::parse(&blob, 0).unwrap(), 1000.0), text); + let before = live(); + let font = Font::new(Face::parse(&blob, 0).unwrap(), 1000.0); + for call in 0..3 { + assert_eq!(run(&font, text), fresh, "call {call}"); + } + let kept = live() - before; + drop(font); + usize::try_from(kept).unwrap_or(0) +} + +#[test] +fn indices_of_one_lookup_keep_its_digests_once() { + // 64 lookup indices name one lookup of 2,000 subtables. Keeping + // 24 bytes per subtable for each index took 3 MB. + let a = glyph(OPEN_SANS, 'a'); + let lookup = repeated_lookup(6, 2000, &chain_context3(a)); + let indices: Vec = (0..64).collect(); + let gsub = layout_table(*b"ccmp", &indices, &shared_lookup_list(64, &lookup)); + let font = with_table(OPEN_SANS, *b"GSUB", &gsub); + let kept = kept_bytes(&font, "a"); + assert!( + kept < 64 << 10, + "{kept} bytes kept for a {}-byte GSUB", + gsub.len() + ); + + // The same in GPOS, with chained context positioning. + let lookup = repeated_lookup(8, 2000, &chain_context3(a)); + let gpos = layout_table(*b"kern", &indices, &shared_lookup_list(64, &lookup)); + let font = with_table(OPEN_SANS, *b"GPOS", &gpos); + let kept = kept_bytes(&font, "a"); + assert!( + kept < 64 << 10, + "{kept} bytes kept for a {}-byte GPOS", + gpos.len() + ); +} + +#[test] +fn overlapping_lookups_keep_what_the_table_size_allows() { + // 400 lookups at distinct offsets that overlap: the words repeat + // [1, 0, N], so a lookup at every third word reads type 1 (single + // substitution), flag 0, and N subtables at offsets [1, 0, N, ...] + // from it, some of which parse (with an empty coverage) and some + // of which do not. Index sharing cannot help; keeping every + // subtable's digest took 19 MB for this 7 KB table. + let (lookups, n) = (400usize, 2000u16); + let words = 3 * lookups + usize::from(n) + 3; + let pattern: Vec = [1, 0, n].into_iter().cycle().take(words).collect(); + let first = 2 + 2 * lookups; + let mut lookup_list = u16s(&[offset16(lookups)]); + lookup_list.extend(u16s( + &(0..lookups) + .map(|k| offset16(first + 6 * k)) + .collect::>(), + )); + lookup_list.extend(u16s(&pattern)); + let indices: Vec = (0..offset16(lookups)).collect(); + let gsub = layout_table(*b"ccmp", &indices, &lookup_list); + let font = with_table(OPEN_SANS, *b"GSUB", &gsub); + let kept = kept_bytes(&font, "a"); + // 24 bytes a digest for at most twice the table's length plus + // 64 Ki digests, and a few dozen bytes per lookup. + let bound = 24 * (2 * gsub.len() + (1 << 16)) + 64 * lookups; + assert!(kept < bound, "{kept} bytes kept, bound {bound}"); +} + +#[test] +fn many_class_based_subtables_shape_as_a_fresh_font_does() { + // One lookup of 3,000 offsets to a class-based chained context: + // every subtable past the eighth asks for its cache rank at every + // glyph. The cache rank of `lazy.rs` has a unit test that counts + // the work; this one checks the output on real tables. + let a = glyph(OPEN_SANS, 'a'); + let lookup = repeated_lookup(6, 3000, &chain_context2(a)); + let gsub = layout_table(*b"ccmp", &[0], &shared_lookup_list(1, &lookup)); + let font = with_table(OPEN_SANS, *b"GSUB", &gsub); + kept_bytes(&font, "aaaa"); + let lookup = repeated_lookup(8, 3000, &chain_context2(a)); + let gpos = layout_table(*b"kern", &[0], &shared_lookup_list(1, &lookup)); + let font = with_table(OPEN_SANS, *b"GPOS", &gpos); + kept_bytes(&font, "aaaa"); +} From 3b3862eb921610202fbfbc821707020e916ceb5e Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 13:46:46 -0800 Subject: [PATCH 03/10] Walk the language system past the kept feature maps Once a table's 16 map slots held other (script, language, FeatureVariations record) keys, every feature query resolved the whole language system into a map and dropped it, more work per query than the walk the cache replaces, and a font shaped in many languages did that for most of its queries. Past the slots, a query now walks the language system, as it does without a cache. The first 16 keys a font is shaped with stay kept; a map and the walk answer alike, so which keys those are never shows in the output. The walk-equivalence test now gives each language its own cache, so every key it checks is a kept map. --- src/ot/layout_select.rs | 68 ++++++++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/src/ot/layout_select.rs b/src/ot/layout_select.rs index 217f33d..95f0ff4 100644 --- a/src/ot/layout_select.rs +++ b/src/ot/layout_select.rs @@ -264,7 +264,7 @@ impl<'a> LayoutView<'a> { tag: [u8; 4], script_priority: &[[u8; 4]], ) -> Option> { - match self.map(script_priority).as_deref() { + match self.map(script_priority) { Some(map) => map.feature_lookups(tag), None => feature_lookup_indices( &self.script_list, @@ -278,7 +278,7 @@ impl<'a> LayoutView<'a> { /// [`lists_feature`] for this view. pub(crate) fn lists(&self, tag: [u8; 4], script_priority: &[[u8; 4]]) -> bool { - match self.map(script_priority).as_deref() { + match self.map(script_priority) { Some(map) => map.lists(tag), None => lists_feature( &self.script_list, @@ -292,7 +292,7 @@ impl<'a> LayoutView<'a> { /// [`listed_feature_lookups`] for this view. pub(crate) fn listed_lookups(&self, tag: [u8; 4], script_priority: &[[u8; 4]]) -> Vec { - match self.map(script_priority).as_deref() { + match self.map(script_priority) { Some(map) => map.listed_lookups(tag), None => listed_feature_lookups( &self.script_list, @@ -306,7 +306,7 @@ impl<'a> LayoutView<'a> { /// [`required_feature`] for this view. pub(crate) fn required(&self, script_priority: &[[u8; 4]]) -> Option<([u8; 4], Vec)> { - match self.map(script_priority).as_deref() { + match self.map(script_priority) { Some(map) => map.required.clone(), None => required_feature( &self.script_list, @@ -319,8 +319,9 @@ impl<'a> LayoutView<'a> { /// The resolved language system of a run whose candidate script /// tags are `script_priority`, from the font's cache: `None` - /// without one, or when the language system is too large to keep. - fn map(&self, script_priority: &[[u8; 4]]) -> Option> { + /// without one, when the language system is too large to keep, or + /// when the cache already keeps as many as it may. + fn map(&self, script_priority: &[[u8; 4]]) -> Option<&'a FeatureMap> { let maps = self.maps?; let key = MapKey { script_priority, @@ -531,9 +532,10 @@ impl KeyedMap { } /// The language systems one table has resolved, kept by a -/// [`crate::Font`] across shaping calls: at most [`MAP_SLOTS`] of them. -/// Past that, a run's language system is walked each time, as it is -/// without a cache. +/// [`crate::Font`] across shaping calls: the first [`MAP_SLOTS`] a font +/// is shaped with. A run with any other key walks its language system +/// per query, as it does without a cache, rather than resolving all of +/// it for each query. pub(crate) struct FeatureMaps { slots: [OnceBox; MAP_SLOTS], } @@ -547,12 +549,8 @@ impl FeatureMaps { } /// The map for `key`, built with `build` the first time, or `None` - /// when it is too large to keep. - fn get( - &self, - key: &MapKey<'_>, - build: impl Fn() -> Option, - ) -> Option> { + /// when it is too large to keep or every slot keeps another key. + fn get(&self, key: &MapKey<'_>, build: impl Fn() -> Option) -> Option<&FeatureMap> { for slot in &self.slots { let keyed = match slot.get() { Some(keyed) => keyed, @@ -564,10 +562,10 @@ impl FeatureMaps { }), }; if keyed.matches(key) { - return keyed.map.as_ref().map(Cow::Borrowed); + return keyed.map.as_ref(); } } - build().map(Cow::Owned) + None } /// Heap bytes the kept maps hold. @@ -1226,8 +1224,10 @@ mod tests { tags.extend(unknown); for record in records { let features = ActiveFeatures::new(list, record); - let cache = LayoutCache::new(table, &lookups, 1 << 20); for languages in languages { + // One cache per language keeps every priority's + // language system within the cache's slots. + let cache = LayoutCache::new(table, &lookups, 1 << 20); let walk = LayoutView { script_list, features, @@ -1264,6 +1264,38 @@ mod tests { const LATN: &[[u8; 4]] = &[*b"latn"]; + #[test] + fn language_systems_past_the_slots_are_walked_not_resolved() { + let maps = FeatureMaps::new(); + let built = core::cell::Cell::new(0); + let build = || { + built.set(built.get() + 1); + Some(FeatureMap { + lang_sys: false, + entries: Vec::new(), + required: None, + global_vert: None, + }) + }; + let languages: Vec<[[u8; 4]; 1]> = (0..MAP_SLOTS as u8 + 8) + .map(|i| [[b'L', b'0' + i / 10, b'0' + i % 10, b' ']]) + .collect(); + let key = |i: usize| MapKey { + script_priority: LATN, + language_tags: &languages[i], + record: None, + }; + for round in 0..3 { + for i in 0..languages.len() { + let kept = maps.get(&key(i), build).is_some(); + assert_eq!(kept, i < MAP_SLOTS, "language {i}, round {round}"); + } + } + // Each kept key was resolved once, and the others never: their + // queries walk the language system instead. + assert_eq!(built.get(), MAP_SLOTS); + } + #[test] fn stage_plans_are_kept_per_key() { let plans = StagePlans::new(); From b82d191fd8da022475b3b2301e42dd45fb10e301 Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 13:47:20 -0800 Subject: [PATCH 04/10] Document the const Font::coords change and the atomics requirement STABILITY.md says Font::coords is no longer a const fn, next to the note that its slice now borrows the font. The crate docs, the README and a new "Supported targets" section of STABILITY.md say the core crate needs pointer-width atomics with compare-and-swap: the Font caches are built through them, and OwnedFace and Language already needed them through alloc::sync::Arc, so thumbv6m-none-eabi did not build before 0.24.0 either. --- README.md | 3 ++- docs/STABILITY.md | 13 ++++++++++++- src/lib.rs | 6 ++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 91a24c9..a60827f 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,8 @@ A few things you will likely need next: writing our own made no sense (Brotli for WOFF2, zlib for WOFF1 and PNG, clap for the CLI). [docs/deps.md](docs/deps.md) explains each one. - `no_std` support. The default build uses `std`, but the shaping path runs under - `no_std` with `alloc`. + `no_std` with `alloc`. The target needs pointer-width atomics with compare-and-swap, + which every target with `alloc::sync::Arc` has (`thumbv6m-none-eabi` does not). - Deterministic output. The same font, text, features, and direction produce the same glyphs, byte for byte. That matters for replays, lockstep networking, and golden-file tests. diff --git a/docs/STABILITY.md b/docs/STABILITY.md index 349ef89..d881e47 100644 --- a/docs/STABILITY.md +++ b/docs/STABILITY.md @@ -15,6 +15,16 @@ with, for example, `sigilbuzz-render = "0.10"` and `sigilbuzz-paint = "0.3"`. The target is 1.0 in 2026, with a stable shaping API and a documented path for renderers built on top of it. +## Supported targets + +The core crate builds on stable Rust 1.81 or later, with or without `std`, on any +target that has pointer-width atomics with compare-and-swap +(`cfg(target_has_atomic = "ptr")`). A `Font` builds its shaping caches through atomic +compare-and-swap, and `OwnedFace` shares its bytes through `alloc::sync::Arc`, which +needs the same. Targets without them, such as `thumbv6m-none-eabi` (Cortex-M0 and +M0+), do not build; this was already true before 0.24.0, through `alloc::sync`. +`wasm32-unknown-unknown` and the usual desktop and mobile targets have them. + ## Three tiers ### Tier 1: stable public API @@ -92,7 +102,8 @@ signatures only change in a major version. - Added in 0.24.0: - `Font::with_variations`: bind a variable instance from user-space axis values, as HarfBuzz's `hb_font_set_variations` does. `Font::coords` now returns a slice that - borrows the font, since such a font owns its coords. + borrows the font, since such a font owns its coords, and is no longer a `const fn`, + so it cannot be called in a const context. `Font::new` is still `const`. - `Face::glyph_outlines` and `GlyphOutlines::{outline, draw}`: draw many glyphs at one instance with the outline tables read once. diff --git a/src/lib.rs b/src/lib.rs index 7d344b9..c48d417 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -51,6 +51,12 @@ //! The crate compiles with `--no-default-features` on stable Rust. The //! default `std` feature adds filesystem helpers such as `Blob::from_path`. //! Everything on the shaping path works without it. +//! +//! The target needs pointer-width atomics with compare-and-swap +//! (`cfg(target_has_atomic = "ptr")`): a [`Font`] builds its shaping +//! caches through them without a lock, and [`OwnedFace`] shares its bytes +//! through `alloc::sync::Arc`, which needs them too. Targets without +//! them, such as `thumbv6m-none-eabi`, do not build. #![cfg_attr(not(feature = "std"), no_std)] #![forbid(unsafe_op_in_unsafe_fn)] From 6e8b971ebe1464df711a6f34d515d96b99c2484c Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 13:47:20 -0800 Subject: [PATCH 05/10] Check a used Font against a fresh one in the shape fuzz helper shape_samples shapes every sample text with one Font, whose caches are built from its second call on. Each text is now shaped with a fresh Font too, run by run for bidi text, and the two outputs must be equal, so a fuzzed font that makes the caches change the output crashes the shape and shape_text targets. --- fuzz/src/lib.rs | 47 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 856fd51..05bd95b 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -1,7 +1,8 @@ //! Shared helpers for the sigilbuzz fuzz targets. //! //! The helpers never panic on their own, so any crash a target reports comes -//! from sigilbuzz itself. +//! from sigilbuzz itself. The one assertion, in [`shape_samples`], checks +//! sigilbuzz's own promise that a `Font`'s caches never change its output. use sigilbuzz::{ shape, BidiParagraph, Buffer, BufferFlags, ClusterLevel, Direction, Face, Feature, Font, @@ -101,25 +102,43 @@ pub fn axis_coords(face: &Face<'_>, knobs: &mut Knobs<'_>) -> Vec { (0..axes.min(16) + extra).map(|_| knobs.coord()).collect() } +/// A font of `face` at `size`, at `variations` (user-space values) when +/// given, else at the normalized `coords`. +fn bind<'a>( + face: Face<'a>, + size: f32, + coords: &'a [f32], + variations: Option<&[([u8; 4], f32)]>, +) -> Font<'a> { + let font = Font::new(face, size); + match variations { + Some(variations) => font.with_variations(variations), + None => font.with_coords(coords), + } +} + /// Shapes every sample text (plus `extra`) against `face` with the given /// knobs choosing direction, features, size, and coordinates. +/// +/// One font shapes every text, so from its second call on it shapes with +/// the caches it keeps. Each text is shaped with a fresh font too, which +/// builds none, and the two must agree glyph for glyph. pub fn shape_samples(face: Face<'_>, knobs: &mut Knobs<'_>, extra: Option<&str>) { let coords = axis_coords(&face, knobs); let size = f32::from(knobs.byte()) + 1.0; // Half the time the instance comes from user-space values. - let font = if knobs.byte() & 1 == 1 { - let variations: Vec<([u8; 4], f32)> = match face.fvar() { + let variations: Option> = + (knobs.byte() & 1 == 1).then(|| match face.fvar() { Ok(Some(fvar)) => fvar .axes() .iter() .map(|a| (a.tag, a.default_value + knobs.coord() * 500.0)) .collect(), _ => Vec::new(), - }; - Font::new(face, size).with_variations(&variations) - } else { - Font::new(face, size).with_coords(&coords) - }; + }); + let variations = variations.as_deref(); + let font = bind(face.clone(), size, &coords, variations); + let fresh = || bind(face.clone(), size, &coords, variations); let mut features = Vec::new(); for _ in 0..(knobs.byte() % 6) { let i = usize::from(knobs.byte()) % FEATURE_TAGS.len(); @@ -147,7 +166,13 @@ pub fn shape_samples(face: Face<'_>, knobs: &mut Knobs<'_>, extra: Option<&str>) // Bidi text shapes run by run, each run in its own // direction, with `buffer` as the settings template. let paragraph = BidiParagraph::new(text, None); - let _ = paragraph.shape(&font, &buffer, &features); + let warm = paragraph + .shape(&font, &buffer, &features) + .map(|run| run.glyphs); + let cold = paragraph + .shape(&fresh(), &buffer, &features) + .map(|run| run.glyphs); + assert_eq!(warm, cold, "a used Font shaped {text:?} unlike a fresh one"); buffer.set_text(text); } } @@ -158,6 +183,8 @@ pub fn shape_samples(face: Face<'_>, knobs: &mut Knobs<'_>, extra: Option<&str>) _ => Direction::Btt, }); let _ = buffer.script_runs(); - let _ = shape(&font, &buffer, &features); + let warm = shape(&font, &buffer, &features).map(|run| run.glyphs); + let cold = shape(&fresh(), &buffer, &features).map(|run| run.glyphs); + assert_eq!(warm, cold, "a used Font shaped {text:?} unlike a fresh one"); } } From 306d53f894ffab52b053b1f5ab00ad650803e98a Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 14:16:26 -0800 Subject: [PATCH 06/10] Keep varied HVAR and VVAR advances in the font's instance cache A vertical run moves every glyph by half its horizontal advance, and at non-default coordinates that advance comes from HVAR, whose delta walks the variation store's regions per glyph. That walk ran for every glyph of every call: about half of a 100-character Hahmlet[wght] TTB call at wght 700, which made such runs slower than 0.23.0 although the vertical origins themselves were cached. The varied horizontal and vertical advances, from HVAR and VVAR as from phantom points, now share the font's two advance caches (a font has HVAR or not, so one instance only ever stores one kind), as HarfBuzz keeps an advance cache per font. The vertical origins and phantom-point advances now look in the font's cache before the call's own maps, so a warm call adds no map entries. Horizontal runs of fonts with HVAR get the cached advances too. The glyph caches stay at three of at most 4096 entries each. --- src/font/cache.rs | 26 ++++---- src/shape/position.rs | 143 +++++++++++++++++++++++++++++++----------- 2 files changed, 121 insertions(+), 48 deletions(-) diff --git a/src/font/cache.rs b/src/font/cache.rs index ab47bfa..7da8c49 100644 --- a/src/font/cache.rs +++ b/src/font/cache.rs @@ -13,9 +13,10 @@ //! [`Font::with_coords`](super::Font::with_coords) and //! [`Font::with_size`](super::Font::with_size) keep it. //! - [`InstanceCache`], which depends on the variation coordinates too: -//! the vertical origins and the advances that come from phantom -//! points, which cost an outline walk per glyph. `with_size` keeps it -//! and `with_coords` starts a new one. +//! the vertical origins and the varied advances, which cost an outline +//! walk (phantom points) or a walk of a variation store's regions +//! (`HVAR`, `VVAR`) per glyph. `with_size` keeps it and `with_coords` +//! starts a new one. //! //! A font's first shaping call builds none of this: a font shaped once, //! which many callers build per run, would spend more building it than @@ -175,7 +176,7 @@ impl FaceCache { } /// What a font keeps that depends on its coordinates too: per-glyph -/// values whose computation walks an outline. +/// values whose computation walks an outline or a variation store. pub(crate) struct InstanceCache { v_origins: OnceBox, v_advances: OnceBox, @@ -187,12 +188,13 @@ pub(crate) struct InstanceCache { pub(crate) enum GlyphValue { /// The y of the vertical origin. VOrigin, - /// The vertical advance from varied phantom points, if they could - /// be computed. - VPhantomAdvance, - /// The horizontal advance from varied phantom points, if they - /// could be computed. - HPhantomAdvance, + /// The varied vertical advance: from `VVAR`, or from varied phantom + /// points, if they could be computed. A font has `VVAR` or not, so + /// one instance only ever stores one kind. + VAdvance, + /// The varied horizontal advance: from `HVAR`, or from varied + /// phantom points, if they could be computed. + HAdvance, } impl InstanceCache { @@ -207,8 +209,8 @@ impl InstanceCache { fn cache(&self, which: GlyphValue) -> &OnceBox { match which { GlyphValue::VOrigin => &self.v_origins, - GlyphValue::VPhantomAdvance => &self.v_advances, - GlyphValue::HPhantomAdvance => &self.h_advances, + GlyphValue::VAdvance => &self.v_advances, + GlyphValue::HAdvance => &self.h_advances, } } diff --git a/src/shape/position.rs b/src/shape/position.rs index 7194973..cadee84 100644 --- a/src/shape/position.rs +++ b/src/shape/position.rs @@ -374,9 +374,11 @@ fn zero_mark_widths(glyphs: &mut [Glyph], gdef: Option<&Gdef<'_>>, adjust_offset /// /// The pipeline builds one per shaping call and hands it to the /// vertical origins, the fallback spaces, and the `stch` stretch. The -/// values that walk an outline (vertical origins, and advances from -/// phantom points) are also kept in the font's [`InstanceCache`] for -/// later calls, as HarfBuzz keeps them for a font. +/// values that walk an outline or a variation store (vertical origins, +/// and every varied advance, from `HVAR`, `VVAR` or phantom points) are +/// also kept in the font's [`InstanceCache`] for later calls, as +/// HarfBuzz keeps them for a font, and a value found there is not +/// looked up in the call's own maps. pub(super) struct FontAdvances<'a, 'c> { face: &'c Face<'a>, coords: &'c [f32], @@ -491,6 +493,31 @@ impl<'a, 'c> FontAdvances<'a, 'c> { }) } + /// The font's cache of `which`, if this call shares it. + fn cache_for(&self, which: GlyphValue) -> Option<&'c InstanceCache> { + // The vertical values come from `vmtx`, `VORG` and `VVAR`, which + // only a vertical run reads, so only a vertical run shares them. + let vertical_value = matches!(which, GlyphValue::VOrigin | GlyphValue::VAdvance); + self.cache.filter(|_| self.vertical || !vertical_value) + } + + /// `which` of glyph `id`, if the font's cache holds it. + #[inline] + fn cached_value(&self, which: GlyphValue, id: u16) -> Option { + self.cache_for(which)?.get(which, id) + } + + /// Adds `which` of glyph `id` to the font's cache, if this call + /// shares it. + fn store(&self, which: GlyphValue, id: u16, value: Option) { + if let Some(cache) = self.cache_for(which) { + let num_glyphs = *self + .num_glyphs + .get_or_init(|| self.face.maxp().map_or(u16::MAX, |maxp| maxp.num_glyphs)); + cache.set(which, id, num_glyphs, value); + } + } + /// `which` of glyph `id` from the font's cache, else computed with /// `compute` and added to the cache. fn cached( @@ -499,20 +526,11 @@ impl<'a, 'c> FontAdvances<'a, 'c> { id: u16, compute: impl FnOnce() -> Option, ) -> Option { - // The vertical values come from `vmtx`, `VORG` and `VVAR`, which - // only a vertical run reads, so only a vertical run shares them. - let vertical_value = matches!(which, GlyphValue::VOrigin | GlyphValue::VPhantomAdvance); - let Some(cache) = self.cache.filter(|_| self.vertical || !vertical_value) else { - return compute(); - }; - if let Some(Known(value)) = cache.get(which, id) { + if let Some(Known(value)) = self.cached_value(which, id) { return value; } let value = compute(); - let num_glyphs = *self - .num_glyphs - .get_or_init(|| self.face.maxp().map_or(u16::MAX, |maxp| maxp.num_glyphs)); - cache.set(which, id, num_glyphs, value); + self.store(which, id, value); value } @@ -534,9 +552,16 @@ impl<'a, 'c> FontAdvances<'a, 'c> { return base; } if let Some(hvar) = &self.hvar { - let delta = hvar.advance_delta(id, self.coords); - // HarfBuzz's `hb_max (0.0f, advance + roundf (delta))`. - return base.saturating_add(hb_round(delta)).max(0); + // An `HVAR` delta walks the variation store's regions for + // every glyph, so the font's cache keeps the varied advance, + // as HarfBuzz's advance cache does. + return self + .cached(GlyphValue::HAdvance, id, || { + let delta = hvar.advance_delta(id, self.coords); + // HarfBuzz's `hb_max (0.0f, advance + roundf (delta))`. + Some(base.saturating_add(hb_round(delta)).max(0)) + }) + .unwrap_or(base); } self.h_phantom_advance(id).unwrap_or(base) } @@ -554,14 +579,19 @@ impl<'a, 'c> FontAdvances<'a, 'c> { return base; } if let Some(vvar) = &self.vvar { - let delta = vvar.advance_height_delta(id, self.coords); - return base.saturating_add(hb_round(delta)).max(0); + // Kept in the font's cache, as the `HVAR` advances are. + return self + .cached(GlyphValue::VAdvance, id, || { + let delta = vvar.advance_height_delta(id, self.coords); + Some(base.saturating_add(hb_round(delta)).max(0)) + }) + .unwrap_or(base); } // HarfBuzz takes the phantom points only from a gvar it reads. if !self.outlines().is_some_and(|t| t.gvar.is_some()) { return base; } - self.cached(GlyphValue::VPhantomAdvance, id, || { + self.cached(GlyphValue::VAdvance, id, || { self.v_phantoms(id) .map(|pp| hb_round(pp[2].1 - pp[3].1).max(0)) }) @@ -595,6 +625,11 @@ impl<'a, 'c> FontAdvances<'a, 'c> { /// outline, shapes too. pub(super) fn v_origin(&self, id: u32) -> i32 { let id = id as u16; + // The font's cache first: a hit there is one atomic load, and + // needs no entry in the call's own map. + if let Some(Known(Some(origin))) = self.cached_value(GlyphValue::VOrigin, id) { + return origin; + } // The cell is only borrowed here and in the insert below, so // the borrows always succeed. if let Some(origin) = self @@ -605,9 +640,8 @@ impl<'a, 'c> FontAdvances<'a, 'c> { { return origin; } - let origin = self - .cached(GlyphValue::VOrigin, id, || Some(self.compute_v_origin(id))) - .unwrap_or_else(|| self.compute_v_origin(id)); + let origin = self.compute_v_origin(id); + self.store(GlyphValue::VOrigin, id, Some(origin)); if let Ok(mut cache) = self.v_origins.try_borrow_mut() { cache.insert(id, origin); } @@ -668,25 +702,28 @@ impl<'a, 'c> FontAdvances<'a, 'c> { /// cannot be computed; the caller then keeps the glyph's static /// advance. fn h_phantom_advance(&self, id: u16) -> Option { + // The font's cache first, as for the vertical origins. + if let Some(Known(advance)) = self.cached_value(GlyphValue::HAdvance, id) { + return advance; + } // The cells are only borrowed inside this method, which does // not call itself, so the borrows always succeed. if let Some(&advance) = self.h_phantom.try_borrow().ok()?.get(&id) { return advance; } - let advance = self.cached(GlyphValue::HPhantomAdvance, id, || { - self.outlines().and_then(|t| { - let gvar = t.gvar.as_ref()?; - let metrics = PhantomMetrics { - hmtx: &self.hmtx, - vmtx: None, - }; - let pp = t - .glyf - .phantom_points_at_coords(&t.loca, id, Some(gvar), self.coords, &metrics) - .ok()?; - Some(hb_round(pp[1].0 - pp[0].0).max(0)) - }) + let advance = self.outlines().and_then(|t| { + let gvar = t.gvar.as_ref()?; + let metrics = PhantomMetrics { + hmtx: &self.hmtx, + vmtx: None, + }; + let pp = t + .glyf + .phantom_points_at_coords(&t.loca, id, Some(gvar), self.coords, &metrics) + .ok()?; + Some(hb_round(pp[1].0 - pp[0].0).max(0)) }); + self.store(GlyphValue::HAdvance, id, advance); if let Ok(mut cache) = self.h_phantom.try_borrow_mut() { cache.insert(id, advance); } @@ -812,4 +849,38 @@ mod tests { zero_mark_widths(&mut glyphs, Some(&gdef), true); assert_eq!((glyphs[0].x_advance, glyphs[0].x_offset), (0, -300)); } + + #[test] + fn varied_advances_are_kept_in_the_font_cache() { + // Noto Sans KR's vertical subset varies both advances through + // HVAR and VVAR, whose deltas walk a variation store per glyph. + let data = include_bytes!("../../tests/fixtures/noto_sans_kr_vf_vertical_subset.otf"); + let face = Face::parse_bytes(data, 0).unwrap(); + let coords = [0.6f32]; + let caches = crate::font::FontCaches::new(); + let cache = caches.instance(); + let plain = FontAdvances::new(&face, &coords, true, None).unwrap(); + let first = FontAdvances::new(&face, &coords, true, Some(cache)).unwrap(); + for gid in 0..16 { + assert_eq!(first.h_advance(gid), plain.h_advance(gid), "{gid}"); + assert_eq!(first.v_advance(gid), plain.v_advance(gid), "{gid}"); + assert_eq!(first.v_origin(gid), plain.v_origin(gid), "{gid}"); + } + // A later call finds them in the font's cache. + let later = FontAdvances::new(&face, &coords, true, Some(cache)).unwrap(); + for gid in 0..16u16 { + let id = u32::from(gid); + let h = Some(Known(Some(plain.h_advance(id)))); + assert_eq!(cache.get(GlyphValue::HAdvance, gid), h, "{gid}"); + let v = Some(Known(Some(plain.v_advance(id)))); + assert_eq!(cache.get(GlyphValue::VAdvance, gid), v, "{gid}"); + assert_eq!(later.h_advance(id), plain.h_advance(id)); + assert_eq!(later.v_advance(id), plain.v_advance(id)); + assert_eq!(later.v_origin(id), plain.v_origin(id)); + } + // A horizontal call shares the horizontal advances only. + let horizontal = FontAdvances::new(&face, &coords, false, Some(cache)).unwrap(); + assert_eq!(horizontal.h_advance(3), plain.h_advance(3)); + assert!(horizontal.cached_value(GlyphValue::VAdvance, 3).is_none()); + } } From d8ab508490251f8e9f166569d33248fb6d8a3d7e Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 14:47:45 -0800 Subject: [PATCH 07/10] Say that each subtable is charged for the digest it may keep A lookup whose subtables all have the lookup's digest keeps none of theirs, but its subtables were charged before that was known; the docs now say so. --- src/tables/layout/accel.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tables/layout/accel.rs b/src/tables/layout/accel.rs index fba5b8e..9afee0e 100644 --- a/src/tables/layout/accel.rs +++ b/src/tables/layout/accel.rs @@ -27,12 +27,12 @@ //! than reading its table a few times. //! //! What the accelerators keep is bounded by the table's size the same -//! way: keeping a subtable's digest costs one entry of the table's -//! budget, and a lookup the budget cannot pay for keeps no digest and -//! admits every glyph. Lookup indices that name one lookup (the same -//! offset in the LookupList) share its accelerator, so a table that -//! lists one lookup of many subtables under many indices keeps its -//! digests once. +//! way: each subtable of a lookup costs one entry of the table's budget +//! for the digest it may keep, and a lookup the budget cannot pay for +//! keeps no digest and admits every glyph. Lookup indices that name one +//! lookup (the same offset in the LookupList) share its accelerator, so +//! a table that lists one lookup of many subtables under many indices +//! keeps its digests once. use alloc::boxed::Box; use alloc::vec::Vec; @@ -168,8 +168,8 @@ impl LookupAccel { /// at most what the lookup's budget and `table_budget`, which the /// table's other lookups share, leave. /// - /// Each subtable digest the accelerator keeps costs one entry of - /// `table_budget` too, taken before any coverage is read, so what a + /// Each subtable costs one entry of `table_budget` too, for the + /// digest it may keep, taken before any coverage is read, so what a /// table's accelerators keep is bounded by the table's size however /// many lookups apply. A lookup the budget cannot pay for keeps no /// digest and admits every glyph. From 37108e3819db397df3ce5d1fd0107e564cd87ebf Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 15:50:42 -0800 Subject: [PATCH 08/10] Trim the per-subtable bookkeeping of the lookup walk A lookup whose subtables keep no digests of their own (all of them have the lookup's) admits every subtable at a glyph the walk stopped at, so the walk no longer asks the accelerator per subtable there. An inline slot past the last subtable reads as a subtable that does not parse, so a slot lookup needs no bounds check besides the array's own, and the cache rank check is inlined. --- src/shape/gpos.rs | 5 ++++- src/shape/gsub_parsed.rs | 5 ++++- src/shape/lazy.rs | 11 ++++++----- src/tables/layout/accel.rs | 8 ++++++++ 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/shape/gpos.rs b/src/shape/gpos.rs index d1ec86a..b2863e8 100644 --- a/src/shape/gpos.rs +++ b/src/shape/gpos.rs @@ -377,8 +377,11 @@ fn apply_subtables_at( ) -> Option { let horizontal = att.direction.is_horizontal(); let id = glyphs.get(at)?.glyph_id as u16; + // The walk only stops at glyphs the lookup's digest admits, so a + // lookup without digests of its own subtables admits all of them. + let every = accel.subtables_rule_out_nothing_more(); for index in 0..subtables.len() { - let admits = |parsed| accel.subtable_may_start(parsed, usize::from(index), id); + let admits = |parsed| every || accel.subtable_may_start(parsed, usize::from(index), id); let Some((sub, digests)) = subtables.get_admitted(index, admits) else { continue; }; diff --git a/src/shape/gsub_parsed.rs b/src/shape/gsub_parsed.rs index a5949fe..d231da1 100644 --- a/src/shape/gsub_parsed.rs +++ b/src/shape/gsub_parsed.rs @@ -124,8 +124,11 @@ pub(super) fn apply_parsed_lookup_at( }; let id = cur.glyph_id as u16; let at = buf.cursor(); + // The walk only stops at glyphs the lookup's digest admits, so a + // lookup without digests of its own subtables admits all of them. + let every = accel.subtables_rule_out_nothing_more(); for index in 0..subtables.len() { - let admits = |parsed| accel.subtable_may_start(parsed, usize::from(index), id); + let admits = |parsed| every || accel.subtable_may_start(parsed, usize::from(index), id); let Some((sub, digests)) = subtables.get_admitted(index, admits) else { continue; }; diff --git a/src/shape/lazy.rs b/src/shape/lazy.rs index 672f832..b9f0b9d 100644 --- a/src/shape/lazy.rs +++ b/src/shape/lazy.rs @@ -60,7 +60,9 @@ impl<'a, T: Subtable<'a>> LazySubtables<'a, T> { pub(super) fn new(lookup: Lookup<'a>) -> Self { let count = usize::from(lookup.subtable_count()); Self { - first: [UNPARSED; INLINE_SLOTS], + // An inline slot past the last subtable reads as one that + // does not parse, so `slot` needs no bounds check of its own. + first: core::array::from_fn(|i| if i < count { UNPARSED } else { FAILED }), rest: alloc::vec![UNPARSED; count.saturating_sub(INLINE_SLOTS)], lookup, parsed: Vec::new(), @@ -71,12 +73,10 @@ impl<'a, T: Subtable<'a>> LazySubtables<'a, T> { } } - /// The slot of subtable `index`, `None` past the last subtable. + /// The slot of subtable `index`: [`FAILED`] or `None` past the last + /// subtable. #[inline] fn slot(&self, index: u16) -> Option { - if index >= self.len() { - return None; - } let i = usize::from(index); match i.checked_sub(INLINE_SLOTS) { None => self.first.get(i).copied(), @@ -194,6 +194,7 @@ impl<'a, T: Subtable<'a>> LazySubtables<'a, T> { /// glyphs reach however many subtables: once the /// [`SUBTABLE_CACHES`]th subtable that parses is found, every later /// one is known to have no cache. + #[inline] pub(super) fn has_cache(&mut self, index: u16) -> bool { if usize::from(index) < SUBTABLE_CACHES { return true; diff --git a/src/tables/layout/accel.rs b/src/tables/layout/accel.rs index 9afee0e..10708d4 100644 --- a/src/tables/layout/accel.rs +++ b/src/tables/layout/accel.rs @@ -334,6 +334,14 @@ impl Accel<'_, '_> { self.subtable_may_start(false, index, glyph) } + /// True when [`Self::subtable_may_start`] admits every glyph that + /// [`Self::may_have`] admits: the lookup keeps no digest per + /// subtable, its subtables all having the lookup's. + #[inline] + pub(crate) fn subtables_rule_out_nothing_more(&self) -> bool { + matches!(self, Self::Built(a) if a.subtables.is_empty()) + } + /// False only when subtable `index` cannot start at `glyph`, as a /// check ahead of applying it. Without digests, a subtable that is /// already `parsed` is let through: it looks the glyph up in its From 375ee8bc043ebd862c000e69bd98d3642d2bb2af Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 15:50:42 -0800 Subject: [PATCH 09/10] Cap the coverages a font's first call reads per lookup A Font's first call reads each applied lookup's subtable coverages up front, so a hostile lookup of 32,000 subtables read 32,000 coverages per application on top of parsing them. The first 1,024 are read now; the subtables past them admit every glyph and are parsed when a glyph reaches them, as every subtable was before the accelerators. Real lookups stay far below the cap (Noto Sans KR's ccmp has 268). --- src/tables/layout/accel.rs | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/tables/layout/accel.rs b/src/tables/layout/accel.rs index 10708d4..7e6b732 100644 --- a/src/tables/layout/accel.rs +++ b/src/tables/layout/accel.rs @@ -241,6 +241,13 @@ pub(crate) struct DirectAccel<'a> { /// Subtables whose coverages a [`DirectAccel`] keeps inline. const INLINE_COVERAGES: usize = 4; +/// Subtables whose coverages a [`DirectAccel`] reads at most. Past them +/// a lookup's subtables admit every glyph and are parsed when a glyph +/// reaches them, as every subtable was before the accelerators. Real +/// lookups stay far below it (Noto Sans KR's `ccmp` has 268), and a +/// hostile one of tens of thousands then costs an application no more +/// reading than its subtables' parsing. +const MAX_DIRECT_COVERAGES: usize = 1024; impl<'a> DirectAccel<'a> { fn new(table: LayoutTable, lookup: &Lookup<'a>) -> Self { @@ -255,13 +262,15 @@ impl<'a> DirectAccel<'a> { } let rest = (0..lookup.subtable_count()) .skip(INLINE_COVERAGES) + .take(MAX_DIRECT_COVERAGES - INLINE_COVERAGES) .map(coverage) .collect(); Self { first, rest, count } } /// Subtable `index`'s coverage: `Some(None)` for a subtable without - /// one sigilbuzz reads, `None` past the last subtable. + /// one sigilbuzz reads, `None` past the last subtable or past + /// [`MAX_DIRECT_COVERAGES`]. #[inline] fn coverage(&self, index: usize) -> Option<&Option>> { if index >= self.count { @@ -280,7 +289,7 @@ impl<'a> DirectAccel<'a> { } fn covers_any(&self, glyph: u16) -> bool { - (0..self.count).any(|i| self.covers(i, glyph)) + self.count > MAX_DIRECT_COVERAGES || (0..self.count).any(|i| self.covers(i, glyph)) } } @@ -849,6 +858,22 @@ mod tests { } } + #[test] + fn direct_reads_stop_at_their_cap() { + let subs: Vec> = (0..MAX_DIRECT_COVERAGES as u16 + 3) + .map(|g| single(&[g])) + .collect(); + let data = lookup_list(1, &subs); + let list = LookupList::parse(&data).unwrap(); + let accel = DirectAccel::new(LayoutTable::Gsub, &list.get(0).unwrap()); + assert_eq!(accel.rest.len(), MAX_DIRECT_COVERAGES - INLINE_COVERAGES); + assert!(accel.covers(7, 7)); + assert!(!accel.covers(7, 8)); + // Past the cap, every glyph is admitted. + assert!(accel.covers(MAX_DIRECT_COVERAGES + 1, 8)); + assert!(accel.covers_any(60_000)); + } + #[test] fn accelerators_are_built_once_and_agree_with_the_coverages() { let data = lookup_list(1, &[single(&[10]), single(&[3000])]); From 61cc7901bb7d76a4b79f94ba9f3414893e05d2b6 Mon Sep 17 00:00:00 2001 From: albedosehen Date: Sun, 4 Oct 2026 17:19:46 -0800 Subject: [PATCH 10/10] Cap the room a lookup's parsed subtables reserve Once eight subtables of a lookup had parsed, the list reserved room for every subtable of the lookup. Each nested lookup application holds its own list, so a lookup of 32,000 subtables that applies itself again held room for all of them at every level: 88 MiB on the #301 review's font, where the walk parsed ten per level. The reservation is now at most 56 more entries; past it the list grows by doubling. Also say in the OnceBox docs that the lookup accelerators can keep different digests depending on which thread builds them first, while shaping output does not change. --- src/shape/lazy.rs | 64 +++++++++++++++++++++++++++++++++++++++++++---- src/sync.rs | 10 +++++--- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/shape/lazy.rs b/src/shape/lazy.rs index b9f0b9d..5c73133 100644 --- a/src/shape/lazy.rs +++ b/src/shape/lazy.rs @@ -16,6 +16,13 @@ use crate::tables::layout::Lookup; /// small lookup is applied without allocating them. const INLINE_SLOTS: usize = 8; +/// Most room [`LazySubtables`] reserves at once for parsed subtables, +/// once [`INLINE_SLOTS`] of them have parsed. Past it `parsed` grows by +/// doubling. Nested lookup applications each hold their own list (up +/// to the nesting limit at once), so room for every subtable of a huge +/// lookup at each level held tens of MB for a walk that parsed ten. +const MAX_RESERVED: usize = 56; + /// The slot of a subtable not parsed yet. const UNPARSED: u32 = 0; /// The slot of a subtable that does not parse (the shaper drops those). @@ -141,11 +148,11 @@ impl<'a, T: Subtable<'a>> LazySubtables<'a, T> { }; if self.parsed.len() == INLINE_SLOTS { // A walk that has parsed this many subtables tends to parse - // most of them: room for all of them at once, as parsing - // every subtable up front took, costs less than copying the - // parsed ones at each doubling. - self.parsed - .reserve_exact(usize::from(self.len()) - INLINE_SLOTS); + // more of them: room for several more at once costs less + // than copying the parsed ones at each early doubling. The + // room is capped (see `MAX_RESERVED`). + let room = usize::from(self.len()) - INLINE_SLOTS; + self.parsed.reserve_exact(room.min(MAX_RESERVED)); } self.parsed.push(parsed); // At most one entry per subtable, and there are at most @@ -249,6 +256,20 @@ mod tests { } } + /// `Byte` with a size, as a parsed subtable has: a vector of a + /// zero-sized type reports unbounded capacity. + struct Wide([u8; 32]); + + impl<'a> Subtable<'a> for Wide { + fn parse(lookup: &Lookup<'a>, index: u16) -> Option { + Byte::parse(lookup, index).map(|_| Wide([0; 32])) + } + + fn reads_rule_set_digests(&self) -> bool { + self.0[0] == 0 + } + } + #[test] fn the_cache_rank_counts_the_subtables_that_parse_in_any_order() { let count = 40; @@ -299,4 +320,37 @@ mod tests { subtables.reads ); } + + #[test] + fn a_walk_that_parses_a_few_subtables_holds_room_for_a_few() { + // A walk that parses ten subtables of a huge lookup and then + // stops (as each level of a lookup that applies itself again + // does) held room for all of them, at every nesting level. (The + // fixture's 16-bit offsets fit 20,000 one-byte subtables.) + let count = 20_000; + let data = lookup_list(count); + let list = LookupList::parse(&data).unwrap(); + let mut subtables = LazySubtables::::new(list.get(0).unwrap()); + let mut parsed = 0; + let mut i = 0; + while parsed < 10 { + if subtables.get(i).is_some() { + parsed += 1; + } + i += 1; + } + let room = subtables.parsed.capacity(); + assert!( + room <= INLINE_SLOTS + MAX_RESERVED, + "room for {room} parsed subtables after parsing {parsed}" + ); + // Parsing every subtable still works past the reserved room. + for i in 0..count { + let _ = subtables.get(i); + } + assert_eq!( + subtables.parsed.len(), + (0..count).filter(|i| i % 3 != 0).count() + ); + } } diff --git a/src/sync.rs b/src/sync.rs index a5f2fc3..c139d06 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -5,9 +5,13 @@ //! reach. [`OnceBox`] is the lock-free alternative: the first thread to //! need the value builds it and publishes it with one compare-and-swap. //! Threads that race it may build the value too, and drop theirs when -//! they lose; every cache built here is a pure function of the font, so -//! whichever copy wins holds the same data and shaping stays -//! deterministic. +//! they lose. Shaping output never depends on which copy wins: every +//! cache built here reads the same font. Most hold the same data +//! whichever thread builds them. The lookup accelerators are the +//! exception: what they keep depends on the font's shared work budget +//! when they are built, so racing threads can keep different digests +//! (and memory and speed can follow call order), but a lookup without a +//! digest admits every glyph, so the glyphs it applies to are the same. use alloc::boxed::Box; use core::fmt;