From 92997e631a8a8df3855dc0737f03ac5b55c31ab3 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 13 Mar 2024 12:56:47 -0400 Subject: [PATCH] [fontbe] Add a 'PendingLookup' type This will let us pre-compute and store the lookupflags and any mark filtering set. --- fontbe/src/features.rs | 10 +++++- fontbe/src/features/common.rs | 26 +++++++++++++++ fontbe/src/features/kern.rs | 61 ++++++++++++++++++++++------------- fontbe/src/orchestration.rs | 4 +-- 4 files changed, 75 insertions(+), 26 deletions(-) create mode 100644 fontbe/src/features/common.rs diff --git a/fontbe/src/features.rs b/fontbe/src/features.rs index 7a41dda97..8e4592e21 100644 --- a/fontbe/src/features.rs +++ b/fontbe/src/features.rs @@ -39,11 +39,13 @@ use crate::{ orchestration::{AnyWorkId, BeWork, Context, FeaAst, FeaRsKerns, FeaRsMarks, WorkId}, }; +mod common; mod kern; mod marks; mod ot_tags; mod properties; +pub(crate) use common::PendingLookup; pub use kern::{create_gather_ir_kerning_work, create_kern_segment_work, create_kerns_work}; pub use marks::create_mark_work; @@ -219,7 +221,13 @@ impl<'a> FeatureWriter<'a> { .kerning .lookups .iter() - .map(|lookups| builder.add_lookup(LookupFlag::empty(), None, lookups.to_owned())) + .map(|lookup| { + builder.add_lookup( + lookup.flags, + lookup.mark_filter_set.clone(), + lookup.subtables.clone(), + ) + }) .collect::>(); for (feature, ids) in &self.kerning.features { diff --git a/fontbe/src/features/common.rs b/fontbe/src/features/common.rs new file mode 100644 index 000000000..630081ab5 --- /dev/null +++ b/fontbe/src/features/common.rs @@ -0,0 +1,26 @@ +use fea_rs::GlyphSet; +use write_fonts::tables::layout::LookupFlag; + +/// A lookup generated outside of user FEA +/// +/// This will be merged into any user-provided features during compilation. +#[derive(Debug, Default, Clone, PartialEq, serde::Serialize, serde::Deserialize)] +pub struct PendingLookup { + pub(crate) subtables: Vec, + pub(crate) flags: LookupFlag, + pub(crate) mark_filter_set: Option, +} + +impl PendingLookup { + pub(crate) fn new( + subtables: Vec, + flags: LookupFlag, + mark_filter_set: Option, + ) -> Self { + Self { + subtables, + flags, + mark_filter_set, + } + } +} diff --git a/fontbe/src/features/kern.rs b/fontbe/src/features/kern.rs index d58f44536..71e981568 100644 --- a/fontbe/src/features/kern.rs +++ b/fontbe/src/features/kern.rs @@ -27,7 +27,7 @@ use log::debug; use ordered_float::OrderedFloat; use write_fonts::{ read::{tables::gsub::Gsub, FontRead, ReadError}, - tables::gdef::GlyphClassDef, + tables::{gdef::GlyphClassDef, layout::LookupFlag}, types::{GlyphId, Tag}, }; @@ -43,7 +43,7 @@ use crate::{ }, }; -use super::properties::CharMap; +use super::{properties::CharMap, PendingLookup}; /// On Linux it took ~0.01 ms per loop, try to get enough to make fan out worthwhile /// based on empirical testing @@ -463,11 +463,14 @@ impl KerningGatherWork { /// fn assign_lookups_to_scripts( &self, - lookups: BTreeMap, Vec>, + lookups: BTreeMap, Vec>>, ast: &ParseTree, // one of 'kern' or 'dist' current_feature: Tag, - ) -> (Vec>, BTreeMap>) { + ) -> ( + Vec>, + BTreeMap>, + ) { let dflt_langs = vec![DFLT_LANG]; let is_kern_feature = current_feature == KERN; @@ -482,13 +485,15 @@ impl KerningGatherWork { // in python this part happens earlier, as part of splitKerning. for (scripts, lookups) in lookups { - let idx = ordered_lookups.len(); - ordered_lookups.push(lookups); - for script in scripts { - lookups_by_script - .entry(script) - .or_insert(Vec::new()) - .push(idx); + for lookup in lookups { + let idx = ordered_lookups.len(); + ordered_lookups.push(lookup); + for script in &scripts { + lookups_by_script + .entry(script.to_owned()) + .or_insert(Vec::new()) + .push(idx); + } } } @@ -568,10 +573,10 @@ impl KerningGatherWork { fn debug_ordered_lookups( features: &BTreeMap>, - lookups: &[Vec], + lookups: &[PendingLookup], ) { - for (i, subtables) in lookups.iter().enumerate() { - let total_rules = subtables.iter().map(|x| x.len()).sum::(); + for (i, lookup) in lookups.iter().enumerate() { + let total_rules = lookup.subtables.iter().map(|x| x.len()).sum::(); log::trace!("lookup {i}, {total_rules} rules"); } @@ -641,7 +646,7 @@ impl KernSplitContext { fn make_lookups( &self, pairs: &[&PairPosEntry], - ) -> BTreeMap, Vec> { + ) -> BTreeMap, Vec>> { if !self.opts.ignore_marks { let pairs = pairs.iter().map(|x| Cow::Borrowed(*x)).collect::>(); return self.make_split_script_kern_lookups(&pairs, false); @@ -665,21 +670,18 @@ impl KernSplitContext { /// Returns a map of scripts: `[lookup]`, where 'scripts' is a set of scripts /// referencing the lookups. /// - /// Although this method always/only returns a single lookup per unique set - /// of scripts, we wrap it in a vec because that is what we need at the - /// call site. // fn make_split_script_kern_lookups( &self, pairs: &[Cow], //TODO: handle marks _are_marks: bool, - ) -> BTreeMap, Vec> { + ) -> BTreeMap, Vec>> { let mut lookups_by_script = BTreeMap::new(); let kerning_per_script = self.split_kerns(pairs); let mut bidi_buf = HashSet::new(); // we can reuse this for each pair for (scripts, pairs) in kerning_per_script { - let mut lookup = PairPosBuilder::default(); + let mut builder = PairPosBuilder::default(); for mut pair in pairs { bidi_buf.clear(); for (direction, glyphs) in &self.bidi_glyphs { @@ -706,8 +708,9 @@ impl KernSplitContext { if pair_is_rtl { pair.make_rtl_compatible(); } - pair.add_to(&mut lookup); + pair.add_to(&mut builder); } + let lookup = PendingLookup::new(vec![builder], LookupFlag::empty(), None); lookups_by_script.insert(scripts, vec![lookup]); } lookups_by_script @@ -1092,9 +1095,21 @@ mod tests { .collect(); let cyr_rules = result.get(&cyrillic).unwrap(); - assert_eq!(cyr_rules.iter().map(|x| x.len()).sum::(), 1); + assert_eq!( + cyr_rules + .iter() + .flat_map(|x| x.subtables.iter().map(|sub| sub.len())) + .sum::(), + 1 + ); let latn_rules = result.get(&latn).unwrap(); - assert_eq!(latn_rules.iter().map(|x| x.len()).sum::(), 2); + assert_eq!( + latn_rules + .iter() + .flat_map(|x| x.subtables.iter().map(|sub| sub.len())) + .sum::(), + 2 + ); } } diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index a92d2d441..d1e0195fe 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -64,7 +64,7 @@ use write_fonts::{ FontWrite, }; -use crate::{error::Error, paths::Paths}; +use crate::{error::Error, features::PendingLookup, paths::Paths}; /// What exactly is being assembled from glyphs? #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -375,7 +375,7 @@ impl Persistable for FeaRsMarks { #[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq)] pub struct FeaRsKerns { /// ordered! - pub lookups: Vec>, + pub lookups: Vec>, /// each value is a set of lookups, referenced by their order in array above pub features: BTreeMap>, }