diff --git a/fontbe/src/features.rs b/fontbe/src/features.rs index 5424204b..0d055a2d 100644 --- a/fontbe/src/features.rs +++ b/fontbe/src/features.rs @@ -16,8 +16,8 @@ use ordered_float::OrderedFloat; use fea_rs::{ compile::{ - error::CompilerError, Compilation, FeatureBuilder, FeatureProvider, PendingLookup, - VariationInfo, + error::CompilerError, Compilation, FeatureBuilder, FeatureProvider, NopFeatureProvider, + PendingLookup, VariationInfo, }, parse::{FileSystemResolver, SourceLoadError, SourceResolver}, DiagnosticSet, GlyphMap, Opts, ParseTree, @@ -35,15 +35,16 @@ use fontdrasil::{ types::Axis, }; use write_fonts::{ - tables::{layout::ClassDef, variations::VariationRegion}, - types::{NameId, Tag}, + tables::{gdef::GlyphClassDef, layout::ClassDef, variations::VariationRegion}, + types::{GlyphId16, NameId, Tag}, OtRound, }; use crate::{ error::Error, orchestration::{ - AnyWorkId, BeWork, Context, ExtraFeaTables, FeaAst, FeaRsKerns, FeaRsMarks, WorkId, + AnyWorkId, BeWork, Context, ExtraFeaTables, FeaFirstPassOutput, FeaRsKerns, FeaRsMarks, + WorkId, }, }; @@ -59,7 +60,7 @@ const DFLT_SCRIPT: Tag = Tag::new(b"DFLT"); const DFLT_LANG: Tag = Tag::new(b"dflt"); #[derive(Debug)] -pub struct FeatureParsingWork {} +pub struct FeatureFirstPassWork {} #[derive(Debug)] pub struct FeatureCompilationWork {} @@ -151,6 +152,31 @@ impl<'a> FeaVariationInfo<'a> { } } +/// Return GDEF classes. +/// +/// If the source is one where we prefer classes declared explicitly in FEA, +/// and those exist, return those; otherwise return computed classes (from +/// public.openTypeCatgories or from Glyphs.xml, depending on the source type) +pub(crate) fn get_gdef_classes( + meta: &StaticMetadata, + ast: &FeaFirstPassOutput, + glyph_order: &GlyphOrder, +) -> HashMap { + ast.gdef_classes + .as_ref() + .filter(|_| meta.gdef_categories.prefer_gdef_categories_in_fea) + .cloned() + .unwrap_or_else(|| { + meta.gdef_categories + .categories + .iter() + .filter_map(|(name, category)| { + glyph_order.glyph_id(name).map(|gid| (gid, *category)) + }) + .collect() + }) +} + //NOTE: this is basically identical to the same method on FeaVariationInfo, //except they have slightly different inputs? pub(crate) fn resolve_variable_metric<'a>( @@ -376,7 +402,7 @@ impl FeatureCompilationWork { fn compile( &self, static_metadata: &StaticMetadata, - ast: &FeaAst, + ast: &FeaFirstPassOutput, kerns: &FeaRsKerns, marks: &FeaRsMarks, ) -> Result { @@ -426,7 +452,7 @@ fn write_debug_fea(context: &Context, is_error: bool, why: &str, fea_content: &s }; } -impl Work for FeatureParsingWork { +impl Work for FeatureFirstPassWork { fn id(&self) -> AnyWorkId { WorkId::FeaturesAst.into() } @@ -458,13 +484,26 @@ impl Work for FeatureParsingWork { // after parsing we validate; we only need to do this once, and future // work can trust the AST. self.validate(&ast, &glyph_map, &static_metadata)?; + let var_info = FeaVariationInfo::new(&static_metadata); - context.fea_ast.set(ast.into()); + let (compilation, _) = fea_rs::compile::compile::<_, NopFeatureProvider>( + &ast, + &glyph_map, + Some(&var_info), + None, + Opts::new().compile_gpos(false), + ) + .map_err(|err| { + Error::FeaCompileError(fea_rs::compile::error::CompilerError::CompilationFail(err)) + })?; + context + .fea_ast + .set(FeaFirstPassOutput::new(ast, compilation)?); Ok(()) } } -impl FeatureParsingWork { +impl FeatureFirstPassWork { pub fn create() -> Box { Box::new(Self {}) } diff --git a/fontbe/src/features/kern.rs b/fontbe/src/features/kern.rs index 70828152..7bfcb99a 100644 --- a/fontbe/src/features/kern.rs +++ b/fontbe/src/features/kern.rs @@ -6,12 +6,9 @@ use std::{ }; use fea_rs::{ - compile::{ - FeatureKey, FeatureProvider, NopFeatureProvider, NopVariationInfo, PairPosBuilder, - ValueRecord as ValueRecordBuilder, - }, + compile::{FeatureKey, FeatureProvider, PairPosBuilder, ValueRecord as ValueRecordBuilder}, typed::{AstNode, LanguageSystem}, - GlyphSet, Opts, ParseTree, + GlyphSet, ParseTree, }; use fontdrasil::{ coords::NormalizedLocation, @@ -26,7 +23,7 @@ use icu_properties::props::BidiClass; use log::debug; use ordered_float::OrderedFloat; use write_fonts::{ - read::{tables::gsub::Gsub, FontRead, ReadError}, + read::{tables::gsub::Gsub, ReadError}, tables::{gdef::GlyphClassDef, layout::LookupFlag}, types::{GlyphId16, Tag}, }; @@ -38,8 +35,8 @@ use crate::{ resolve_variable_metric, }, orchestration::{ - AllKerningPairs, AnyWorkId, BeWork, Context, FeaRsKerns, KernAdjustments, KernFragment, - KernPair, KernSide, WorkId, + AllKerningPairs, AnyWorkId, BeWork, Context, FeaFirstPassOutput, FeaRsKerns, + KernAdjustments, KernFragment, KernPair, KernSide, WorkId, }, }; @@ -410,7 +407,7 @@ impl Work for KerningGatherWork { let lookups = finalize_kerning( &pairs, - &ast.ast, + &ast, &meta, &glyph_order, char_map, @@ -426,51 +423,14 @@ impl Work for KerningGatherWork { // This includes much of the logic from the ufo2ft KernFeatureWriter fn finalize_kerning( pairs: &[&KernPair], - ast: &ParseTree, + ast: &FeaFirstPassOutput, meta: &StaticMetadata, glyph_order: &GlyphOrder, char_map: HashMap, non_spacing_glyphs: HashSet, ) -> Result { - let glyph_map = glyph_order.names().cloned().collect(); - // ignore diagnostics, they'll get logged during actual GSUB compilation - let (compilation, _) = fea_rs::compile::compile::( - ast, - &glyph_map, - None, - None, - Opts::new().compile_gpos(false), - ) - .map_err(|err| { - Error::FeaCompileError(fea_rs::compile::error::CompilerError::CompilationFail(err)) - })?; - - let gsub = compilation - .gsub - .as_ref() - .map(write_fonts::dump_table) - .transpose() - .expect( - "if this doesn't compile we will already panic when we try to add it to the context", - ); - let gsub = gsub - .as_ref() - .map(|data| write_fonts::read::tables::gsub::Gsub::read(data.as_slice().into())) - .transpose()?; - - let glyph_classes = compilation - .gdef_classes - .filter(|_| meta.gdef_categories.prefer_gdef_categories_in_fea) - .unwrap_or_else(|| { - meta.gdef_categories - .categories - .iter() - .filter_map(|(name, category)| { - glyph_order.glyph_id(name).map(|gid| (gid, *category)) - }) - .collect() - }); - let known_scripts = guess_font_scripts(ast, &char_map); + let known_scripts = guess_font_scripts(&ast.ast, &char_map); + let glyph_classes = super::get_gdef_classes(meta, ast, glyph_order); let mark_glyphs = glyph_order .iter() @@ -487,13 +447,13 @@ fn finalize_kerning( }) .collect(); - let split_ctx = KernSplitContext::new(&char_map, &known_scripts, gsub, mark_glyphs)?; + let split_ctx = KernSplitContext::new(&char_map, &known_scripts, ast.gsub(), mark_glyphs)?; let lookups = split_ctx.make_lookups(pairs); let (lookups_by_script, lookups) = split_lookups_by_script(lookups); - let kern_features = assign_lookups_to_scripts(lookups_by_script.clone(), ast, KERN); - let dist_features = assign_lookups_to_scripts(lookups_by_script, ast, DIST); + let kern_features = assign_lookups_to_scripts(lookups_by_script.clone(), &ast.ast, KERN); + let dist_features = assign_lookups_to_scripts(lookups_by_script, &ast.ast, DIST); let features = kern_features.into_iter().chain(dist_features).collect(); debug_ordered_lookups(&features, &lookups); Ok(FeaRsKerns { lookups, features }) @@ -1144,7 +1104,9 @@ fn merge_scripts( mod tests { use std::{path::Path, sync::Arc}; + use fea_rs::compile::NopVariationInfo; use fontir::ir::GdefCategories; + use write_fonts::read::FontRead; use super::*; @@ -1313,6 +1275,7 @@ mod tests { }), ) .unwrap(); + let fea_first_pass = FeaFirstPassOutput::for_test(ast, &glyph_map).unwrap(); let fake_meta = StaticMetadata::new( 1000, Default::default(), @@ -1330,7 +1293,7 @@ mod tests { .unwrap(); let kerns = finalize_kerning( &pairs, - &ast, + &fea_first_pass, &fake_meta, &self.glyph_order, self.charmap, @@ -1339,7 +1302,7 @@ mod tests { .unwrap(); let (comp, _) = fea_rs::compile::compile::( - &ast, + &fea_first_pass.ast, &glyph_map, None, Some(&kerns), diff --git a/fontbe/src/features/marks.rs b/fontbe/src/features/marks.rs index f484b444..bb4d3e9c 100644 --- a/fontbe/src/features/marks.rs +++ b/fontbe/src/features/marks.rs @@ -5,10 +5,10 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use fea_rs::{ compile::{ Anchor as FeaAnchor, CaretValue, CursivePosBuilder, FeatureProvider, MarkToBaseBuilder, - MarkToLigBuilder, MarkToMarkBuilder, NopFeatureProvider, NopVariationInfo, PendingLookup, + MarkToLigBuilder, MarkToMarkBuilder, PendingLookup, }, typed::{AstNode, LanguageSystem}, - GlyphSet, Opts, ParseTree, + GlyphSet, ParseTree, }; use fontdrasil::{ orchestration::{Access, AccessBuilder, Work}, @@ -24,11 +24,12 @@ use write_fonts::{ use crate::{ error::Error, - orchestration::{AnyWorkId, BeWork, Context, FeaRsMarks, WorkId}, + orchestration::{AnyWorkId, BeWork, Context, FeaFirstPassOutput, FeaRsMarks, WorkId}, }; use fontir::{ ir::{self, Anchor, AnchorKind, GlyphAnchors, GlyphOrder, StaticMetadata}, orchestration::WorkId as FeWorkId, + variations::DeltaError, }; use super::DFLT_SCRIPT; @@ -45,14 +46,14 @@ type GroupName = SmolStr; struct MarkLookupBuilder<'a> { // extracted from public.openTypeCatgories/GlyphData.xml or FEA - gdef_classes: BTreeMap, + gdef_classes: HashMap, // pruned, only the anchors we are using - anchor_lists: BTreeMap>, + anchor_lists: BTreeMap>, glyph_order: &'a GlyphOrder, static_metadata: &'a StaticMetadata, // we don't currently use this, because just adding lookups to all scripts works? _fea_scripts: HashSet, - mark_glyphs: BTreeSet, + mark_glyphs: BTreeSet, lig_carets: BTreeMap>, } @@ -66,24 +67,20 @@ enum BaseOrLigAnchors { /// The bases and marks in a particular group, e.g. "top" or "bottom" #[derive(Default, Debug, Clone, PartialEq)] struct MarkGroup<'a> { - bases: Vec<(GlyphName, BaseOrLigAnchors<&'a ir::Anchor>)>, - marks: Vec<(GlyphName, &'a ir::Anchor)>, + bases: Vec<(GlyphId16, BaseOrLigAnchors<&'a ir::Anchor>)>, + marks: Vec<(GlyphId16, &'a ir::Anchor)>, // if `true`, we will make a mark filter set from the marks in this group // (only true for mkmk) filter_glyphs: bool, } impl MarkGroup<'_> { - fn make_filter_glyph_set(&self, glyph_order: &GlyphOrder) -> Option { + fn make_filter_glyph_set(&self) -> Option { self.filter_glyphs.then(|| { self.marks .iter() - .map(|(name, _)| glyph_order.glyph_id(name).unwrap()) - .chain( - self.bases - .iter() - .map(|(name, _)| glyph_order.glyph_id(name).unwrap()), - ) + .map(|(gid, _)| *gid) + .chain(self.bases.iter().map(|(gid, _)| *gid)) .collect() }) } @@ -146,10 +143,10 @@ impl<'a> MarkLookupBuilder<'a> { anchors: Vec<&'a GlyphAnchors>, glyph_order: &'a GlyphOrder, static_metadata: &'a StaticMetadata, - ast: &ParseTree, + ast: &FeaFirstPassOutput, ) -> Result { - let gdef_classes = get_gdef_classes(static_metadata, ast, glyph_order); - let fea_scripts = get_fea_scripts(ast); + let gdef_classes = super::get_gdef_classes(static_metadata, ast, glyph_order); + let fea_scripts = get_fea_scripts(&ast.ast); let lig_carets = get_ligature_carets(glyph_order, static_metadata, &anchors)?; // first we want to narrow our input down to only anchors that are participating. // in pythonland this is https://github.com/googlefonts/ufo2ft/blob/8e9e6eb66a/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L380 @@ -171,8 +168,9 @@ impl<'a> MarkLookupBuilder<'a> { if !glyph_order.contains(&anchors.glyph_name) { continue; } + let gid = glyph_order.glyph_id(&anchors.glyph_name).unwrap(); // skip glyphs that are not mark/lig/base, if we have any defined categories - if !include.is_empty() && !include.contains(&anchors.glyph_name) { + if !include.is_empty() && !include.contains(&gid) { continue; } for anchor in &anchors.anchors { @@ -192,10 +190,7 @@ impl<'a> MarkLookupBuilder<'a> { | AnchorKind::Caret(_) | AnchorKind::VCaret(_) => (), } - pruned - .entry(anchors.glyph_name.clone()) - .or_insert(Vec::new()) - .push(anchor); + pruned.entry(gid).or_insert(Vec::new()).push(anchor); } } @@ -256,22 +251,21 @@ impl<'a> MarkLookupBuilder<'a> { .filter(|(_, group)| !(group.bases.is_empty() || group.marks.is_empty())) .map(|(group_name, group)| { let mut builder = T::default(); - let filter_set = group.make_filter_glyph_set(self.glyph_order); + let filter_set = group.make_filter_glyph_set(); let mut flags = LookupFlag::empty(); if filter_set.is_some() { flags |= LookupFlag::USE_MARK_FILTERING_SET; } - for (mark_name, anchor) in group.marks { - // we already filtered to only things in glyph order - let gid = self.glyph_order.glyph_id(&mark_name).unwrap(); - let anchor = resolve_anchor_once(anchor, self.static_metadata, &mark_name)?; - builder.add_mark(gid, &group_name, anchor); + for (mark_gid, anchor) in &group.marks { + let anchor = resolve_anchor_once(anchor, self.static_metadata) + .map_err(|e| self.convert_delta_error(e, *mark_gid))?; + builder.add_mark(*mark_gid, &group_name, anchor); } - for (base_name, anchor) in group.bases { - let gid = self.glyph_order.glyph_id(&base_name).unwrap(); - let anchor = resolve_anchor(anchor, self.static_metadata, &base_name)?; - builder.add_base(gid, &group_name, anchor); + for (mark_gid, anchor) in group.bases { + let anchor = resolve_anchor(anchor, self.static_metadata) + .map_err(|e| self.convert_delta_error(e, mark_gid))?; + builder.add_base(mark_gid, &group_name, anchor); } Ok(PendingLookup::new(vec![builder], flags, filter_set)) }) @@ -280,11 +274,11 @@ impl<'a> MarkLookupBuilder<'a> { fn make_mark_to_base_groups(&self) -> BTreeMap> { let mut groups = BTreeMap::<_, MarkGroup>::new(); - for (glyph_name, anchors) in &self.anchor_lists { - let is_mark = self.mark_glyphs.contains(glyph_name); + for (gid, anchors) in &self.anchor_lists { + let is_mark = self.mark_glyphs.contains(gid); // if we have explicit gdef classes and this is not an expilcit base let is_not_base = !self.gdef_classes.is_empty() - && (self.gdef_classes.get(glyph_name)) != Some(&GlyphClassDef::Base); + && (self.gdef_classes.get(gid)) != Some(&GlyphClassDef::Base); let treat_as_base = !(is_mark | is_not_base); for anchor in anchors { @@ -293,12 +287,12 @@ impl<'a> MarkLookupBuilder<'a> { .entry(group.clone()) .or_default() .bases - .push((glyph_name.clone(), BaseOrLigAnchors::Base(anchor))), + .push((*gid, BaseOrLigAnchors::Base(anchor))), ir::AnchorKind::Mark(group) if is_mark => groups .entry(group.clone()) .or_default() .marks - .push((glyph_name.clone(), anchor)), + .push((*gid, anchor)), _ => continue, } } @@ -326,8 +320,8 @@ impl<'a> MarkLookupBuilder<'a> { // then iterate again, looking for glyphs that we have identified as marks, // but which also have participating base anchors let mut result = BTreeMap::::new(); - for (glyph, glyph_anchors) in &self.anchor_lists { - if !mark_glyphs.contains(glyph) { + for (gid, glyph_anchors) in &self.anchor_lists { + if !mark_glyphs.contains(gid) { continue; } @@ -337,9 +331,7 @@ impl<'a> MarkLookupBuilder<'a> { if mark_anchors.contains(group_name) { let group = result.entry(group_name.clone()).or_default(); group.filter_glyphs = true; - group - .bases - .push((glyph.clone(), BaseOrLigAnchors::Base(anchor))) + group.bases.push((*gid, BaseOrLigAnchors::Base(anchor))) } } } @@ -352,7 +344,7 @@ impl<'a> MarkLookupBuilder<'a> { if let AnchorKind::Mark(group) = &anchor.kind { // only add mark glyph if there is at least one base if let Some(group) = result.get_mut(group) { - group.marks.push((mark.clone(), anchor)) + group.marks.push((*mark, anchor)) } } } @@ -369,10 +361,10 @@ impl<'a> MarkLookupBuilder<'a> { // first do a pass to build up the ligature anchors and track the set // of mark classes used - for (glyph_name, anchors) in &self.anchor_lists { + for (gid, anchors) in &self.anchor_lists { // skip anything that is definitely not a ligature glyph let might_be_liga = self.gdef_classes.is_empty() - || (self.gdef_classes.get(glyph_name) == Some(&GlyphClassDef::Ligature)); + || (self.gdef_classes.get(gid) == Some(&GlyphClassDef::Ligature)); if !might_be_liga { continue; @@ -399,13 +391,13 @@ impl<'a> MarkLookupBuilder<'a> { .entry(group_name) .or_default() .bases - .push((glyph_name.clone(), BaseOrLigAnchors::Ligature(anchors))); + .push((*gid, BaseOrLigAnchors::Ligature(anchors))); } } // then we do another pass to add the marks in the used classes - for (glyph_name, anchors) in &self.anchor_lists { - if !self.mark_glyphs.contains(glyph_name) { + for (gid, anchors) in &self.anchor_lists { + if !self.mark_glyphs.contains(gid) { continue; } for anchor in anchors { @@ -415,7 +407,7 @@ impl<'a> MarkLookupBuilder<'a> { .entry(group.to_owned()) .or_default() .marks - .push((glyph_name.clone(), anchor)); + .push((*gid, anchor)); } } } @@ -430,16 +422,16 @@ impl<'a> MarkLookupBuilder<'a> { let mut affected_glyphs = BTreeSet::new(); let mut exits = BTreeMap::new(); - for (glyph_name, anchors) in &self.anchor_lists { + for (gid, anchors) in &self.anchor_lists { for anchor in anchors { match anchor.kind { AnchorKind::CursiveEntry => { - entries.insert(glyph_name, anchor); - affected_glyphs.insert(glyph_name); + entries.insert(*gid, anchor); + affected_glyphs.insert(*gid); } AnchorKind::CursiveExit => { - exits.insert(glyph_name, anchor); - affected_glyphs.insert(glyph_name); + exits.insert(*gid, anchor); + affected_glyphs.insert(*gid); } _ => {} } @@ -448,22 +440,28 @@ impl<'a> MarkLookupBuilder<'a> { if affected_glyphs.is_empty() { return Ok(vec![]); } - for glyph_name in affected_glyphs { - let gid = self.glyph_order.glyph_id(glyph_name).unwrap(); + for gid in affected_glyphs { let entry_anchor = entries - .get(glyph_name) - .map(|anchor| resolve_anchor_once(anchor, self.static_metadata, glyph_name)) - .transpose()?; + .get(&gid) + .map(|anchor| resolve_anchor_once(anchor, self.static_metadata)) + .transpose() + .map_err(|e| self.convert_delta_error(e, gid))?; let exit_anchor = exits - .get(glyph_name) - .map(|anchor| resolve_anchor_once(anchor, self.static_metadata, glyph_name)) - .transpose()?; + .get(&gid) + .map(|anchor| resolve_anchor_once(anchor, self.static_metadata)) + .transpose() + .map_err(|e| self.convert_delta_error(e, gid))?; builder.insert(gid, entry_anchor, exit_anchor); } // In the future we might to do an LTR/RTL split, but for now return a // vector with one element. Ok(vec![PendingLookup::new(vec![builder], flags, None)]) } + + fn convert_delta_error(&self, err: DeltaError, gid: GlyphId16) -> Error { + let name = self.glyph_order.glyph_name(gid.into()).cloned().unwrap(); + Error::AnchorDeltaError(name, err) + } } impl Work for MarkWork { @@ -485,7 +483,7 @@ impl Work for MarkWork { let static_metadata = context.ir.static_metadata.get(); let glyph_order = context.ir.glyph_order.get(); let raw_anchors = context.ir.anchors.all(); - let ast = context.fea_ast.get(); + let fea_first_pass = context.fea_ast.get(); let anchors = raw_anchors .iter() @@ -494,7 +492,7 @@ impl Work for MarkWork { // this code is roughly equivalent to what in pythonland happens in // setContext: https://github.com/googlefonts/ufo2ft/blob/8e9e6eb66a/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L322 - let ctx = MarkLookupBuilder::new(anchors, &glyph_order, &static_metadata, &ast.ast)?; + let ctx = MarkLookupBuilder::new(anchors, &glyph_order, &static_metadata, &fea_first_pass)?; let all_marks = ctx.build()?; context.fea_rs_marks.set(all_marks); @@ -506,9 +504,9 @@ impl Work for MarkWork { // in py this is set during _groupMarkGlyphsByAnchor; we try to match that logic // https://github.com/googlefonts/ufo2ft/blob/8e9e6eb66/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L412 fn find_mark_glyphs( - anchors: &BTreeMap>, - gdef_classes: &BTreeMap, -) -> BTreeSet { + anchors: &BTreeMap>, + gdef_classes: &HashMap, +) -> BTreeSet { anchors .iter() .filter(|(name, anchors)| { @@ -534,55 +532,18 @@ fn get_fea_scripts(ast: &ParseTree) -> HashSet { .collect() } -fn get_gdef_classes( - meta: &StaticMetadata, - ast: &ParseTree, - glyph_order: &GlyphOrder, -) -> BTreeMap { - let glyph_map = glyph_order.names().cloned().collect(); - // if we prefer classes defined in fea, compile the fea and see if we have any - if meta.gdef_categories.prefer_gdef_categories_in_fea { - if let Some(gdef_classes) = - fea_rs::compile::compile::( - ast, - &glyph_map, - None, - None, - Opts::new().compile_gpos(false), - ) - .ok() - .and_then(|mut comp| comp.0.gdef_classes.take()) - { - return gdef_classes - .into_iter() - .filter_map(|(g, cls)| { - glyph_order - .glyph_name(g.to_u16() as _) - .cloned() - .map(|name| (name, cls)) - }) - .collect(); - } - } - // otherwise (we don't care about FEA or nothing is defined) we use the - // classes in StaticMetadata (which are from public.openTypeCategories or - // GlyphData.xml) - meta.gdef_categories.categories.clone() -} - fn resolve_anchor( anchor: BaseOrLigAnchors<&ir::Anchor>, static_metadata: &StaticMetadata, - glyph_name: &GlyphName, -) -> Result, Error> { +) -> Result, DeltaError> { match anchor { BaseOrLigAnchors::Base(anchor) => { - resolve_anchor_once(anchor, static_metadata, glyph_name).map(BaseOrLigAnchors::Base) + resolve_anchor_once(anchor, static_metadata).map(BaseOrLigAnchors::Base) } BaseOrLigAnchors::Ligature(anchors) => anchors .into_iter() .map(|a| { - a.map(|a| resolve_anchor_once(a, static_metadata, glyph_name)) + a.map(|a| resolve_anchor_once(a, static_metadata)) .transpose() }) .collect::>() @@ -593,8 +554,7 @@ fn resolve_anchor( fn resolve_anchor_once( anchor: &ir::Anchor, static_metadata: &StaticMetadata, - glyph_name: &GlyphName, // just used for error reporting -) -> Result { +) -> Result { let (x_values, y_values): (Vec<_>, Vec<_>) = anchor .positions .iter() @@ -613,13 +573,11 @@ fn resolve_anchor_once( // clippy complains about the seemingly no-op identity map: // https://rust-lang.github.io/rust-clippy/master/index.html#/map_identity x_values.iter().map(|item| (&item.0, &item.1)), - ) - .map_err(|err| Error::AnchorDeltaError(glyph_name.to_owned(), err))?; + )?; let (y_default, y_deltas) = crate::features::resolve_variable_metric( static_metadata, y_values.iter().map(|item| (&item.0, &item.1)), - ) - .map_err(|err| Error::AnchorDeltaError(glyph_name.to_owned(), err))?; + )?; let mut anchor = FeaAnchor::new(x_default, y_default); if x_deltas.iter().any(|v| v.1 != 0) { @@ -944,7 +902,7 @@ mod tests { fn run_marks_writer( &self, static_metadata: &StaticMetadata, - ast: &ParseTree, + ast: &FeaFirstPassOutput, ) -> FeaRsMarks { let anchors = self .anchors @@ -978,12 +936,13 @@ mod tests { ) .unwrap(); + let first_pass_fea = FeaFirstPassOutput::for_test(ast, &glyph_map).unwrap(); // then run the marks code in this module: - let marks = self.run_marks_writer(&static_metadata, &ast); + let marks = self.run_marks_writer(&static_metadata, &first_pass_fea); let var_info = FeaVariationInfo::new(&static_metadata); // then compile with fea-rs, passing in our generated marks: let (result, _) = fea_rs::compile::compile( - &ast, + &first_pass_fea.ast, &glyph_map, Some(&var_info), Some(&marks), diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index 8987bd13..984abe45 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -1,7 +1,7 @@ //! Helps coordinate the graph execution for BE use std::{ - collections::{BTreeMap, HashSet}, + collections::{BTreeMap, HashMap, HashSet}, fmt::Display, fs::File, io::{self, BufReader, BufWriter, Read, Write}, @@ -36,13 +36,13 @@ use serde::{Deserialize, Serialize}; use write_fonts::{ dump_table, - read::FontRead, + read::{tables::gsub::Gsub as ReadGsub, FontRead}, tables::{ base::Base, cmap::Cmap, fvar::Fvar, gasp::Gasp, - gdef::Gdef, + gdef::{Gdef, GlyphClassDef}, glyf::Glyph as RawGlyph, gpos::Gpos, gsub::Gsub, @@ -443,19 +443,68 @@ pub struct FeaRsKerns { pub features: BTreeMap>, } -/// The abstract syntax tree of any user FEA. +/// "Stage one" state from fea compilation, which is needed as input later. +/// +/// To generate marks & kerns we need access to things like the AST and a +/// compiled GSUB table; to avoid needing to compile GSUB multiple times we +/// do it once up front, and then that gets shared by later jobs. /// /// Before storing the AST, ensure that it has been validated (via [`fea_rs::compile::validate`]). /// This does not include features that are generated, such as for kerning or marks. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] -pub struct FeaAst { +pub struct FeaFirstPassOutput { /// A validated abstract syntax tree. pub ast: ParseTree, + // bytes of the compiled gsub table + gsub_bytes: Option>, + /// GDEF classes, if they were declared explicitly in the fea + pub(crate) gdef_classes: Option>, } -impl From for FeaAst { - fn from(ast: ParseTree) -> FeaAst { - FeaAst { ast } +impl FeaFirstPassOutput { + pub fn new(ast: ParseTree, compilation: Compilation) -> Result { + let gsub_bytes = compilation + .gsub + .as_ref() + .map(write_fonts::dump_table) + .transpose() + .map_err(|e| Error::DumpTableError { + e, + context: "GSUB".into(), + })?; + Ok(Self { + ast, + gsub_bytes, + gdef_classes: compilation.gdef_classes, + }) + } + + #[cfg(test)] + pub(crate) fn for_test(ast: ParseTree, glyph_map: &GlyphMap) -> Result { + use fea_rs::{ + compile::{NopFeatureProvider, NopVariationInfo}, + Opts, + }; + + let (compilation, _) = fea_rs::compile::compile::( + &ast, + glyph_map, + None, + None, + Opts::new().compile_gpos(false), + ) + .map_err(|err| { + Error::FeaCompileError(fea_rs::compile::error::CompilerError::CompilationFail(err)) + })?; + + Self::new(ast, compilation) + } + + /// Return a read-fonts GSUB table (for computing glyph closure for mark/kern) + pub fn gsub(&self) -> Option { + self.gsub_bytes + .as_ref() + .and_then(|bytes| ReadGsub::read(bytes.as_slice().into()).ok()) } } @@ -488,7 +537,7 @@ impl Persistable for FeaRsKerns { } } -impl Persistable for FeaAst { +impl Persistable for FeaFirstPassOutput { fn read(from: &mut dyn Read) -> Self { bincode::deserialize_from(from).unwrap() } @@ -827,7 +876,7 @@ pub struct Context { pub mvar: BeContextItem, pub all_kerning_pairs: BeContextItem, pub kern_fragments: BeContextMap, - pub fea_ast: BeContextItem, + pub fea_ast: BeContextItem, pub fea_rs_kerns: BeContextItem, pub fea_rs_marks: BeContextItem, pub extra_fea_tables: BeContextItem, diff --git a/fontc/src/workload.rs b/fontc/src/workload.rs index 0b3eb2fa..8d07fe4c 100644 --- a/fontc/src/workload.rs +++ b/fontc/src/workload.rs @@ -15,7 +15,7 @@ use fontbe::{ cmap::create_cmap_work, features::{ create_gather_ir_kerning_work, create_kern_segment_work, create_kerns_work, - create_mark_work, FeatureCompilationWork, FeatureParsingWork, + create_mark_work, FeatureCompilationWork, FeatureFirstPassWork, }, font::create_font_work, fvar::create_fvar_work, @@ -156,7 +156,7 @@ impl Workload { workload.add(create_glyph_order_work()); // BE: f(IR, maybe other BE work) => binary - workload.add_skippable_feature_work(FeatureParsingWork::create()); + workload.add_skippable_feature_work(FeatureFirstPassWork::create()); workload.add_skippable_feature_work(FeatureCompilationWork::create()); workload.add(create_gasp_work()); let ir_glyphs = workload