diff --git a/fontbe/src/features.rs b/fontbe/src/features.rs index 877065c67..8094cd4eb 100644 --- a/fontbe/src/features.rs +++ b/fontbe/src/features.rs @@ -15,13 +15,13 @@ use log::{debug, error, trace, warn}; use ordered_float::OrderedFloat; use fea_rs::{ - compile::{Compilation, FeatureBuilder, FeatureProvider, VariationInfo}, - parse::{SourceLoadError, SourceResolver}, - Compiler, + compile::{error::CompilerError, Compilation, FeatureBuilder, FeatureProvider, VariationInfo}, + parse::{FileSystemResolver, SourceLoadError, SourceResolver}, + DiagnosticSet, ParseTree, }; use fontir::{ - ir::{FeaturesSource, StaticMetadata}, + ir::{FeaturesSource, GlyphOrder, StaticMetadata}, orchestration::{Flags, WorkId as FeWorkId}, }; @@ -36,11 +36,14 @@ use write_fonts::{ use crate::{ error::Error, - orchestration::{AnyWorkId, BeWork, Context, FeaRsKerns, FeaRsMarks, WorkId}, + orchestration::{AnyWorkId, BeWork, Context, FeaAst, FeaRsKerns, FeaRsMarks, WorkId}, }; #[derive(Debug)] -pub struct FeatureWork {} +pub struct FeatureParsingWork {} + +#[derive(Debug)] +pub struct FeatureCompilationWork {} // I did not want to make a struct // I did not want to clone the content @@ -54,6 +57,16 @@ struct InMemoryResolver { include_dir: Option, } +impl InMemoryResolver { + fn empty() -> Self { + InMemoryResolver { + content_path: Default::default(), + content: "".into(), + include_dir: None, + } + } +} + impl SourceResolver for InMemoryResolver { fn get_contents(&self, rel_path: &OsStr) -> Result, SourceLoadError> { if rel_path == &*self.content_path { @@ -342,61 +355,41 @@ impl<'a> VariationInfo for FeaVariationInfo<'a> { } } -impl FeatureWork { +impl FeatureCompilationWork { pub fn create() -> Box { - Box::new(FeatureWork {}) + Box::new(FeatureCompilationWork {}) } fn compile( &self, static_metadata: &StaticMetadata, - features: &FeaturesSource, + ast: &FeaAst, kerns: &FeaRsKerns, marks: &FeaRsMarks, ) -> Result { let var_info = FeaVariationInfo::new(static_metadata); let feature_writer = FeatureWriter::new(kerns, marks); - match features { - FeaturesSource::File { - fea_file, - include_dir, - } => { - let mut compiler = Compiler::new(OsString::from(fea_file), &marks.glyphmap); - if let Some(include_dir) = include_dir { - compiler = compiler.with_project_root(include_dir) - } - compiler - } - FeaturesSource::Memory { - fea_content, - include_dir, - } => { - let root = OsString::new(); - let mut compiler = - Compiler::new(root.clone(), &marks.glyphmap).with_resolver(InMemoryResolver { - content_path: root, - content: Arc::from(fea_content.as_str()), - include_dir: include_dir.clone(), - }); - if let Some(include_dir) = include_dir { - compiler = compiler.with_project_root(include_dir) - } - compiler - } - FeaturesSource::Empty => { - // There is no user feature file but we could still generate kerning, marks, etc - let root = OsString::new(); - Compiler::new(root.clone(), &marks.glyphmap).with_resolver(InMemoryResolver { - content_path: root, - content: Arc::from(""), - include_dir: None, - }) + // first do the validation pass + let diagnostics = fea_rs::compile::validate(&ast.ast, &marks.glyphmap, Some(&var_info)); + if diagnostics.has_errors() { + return Err(CompilerError::ValidationFail(diagnostics).into()); + } + log_fea_warnings("validation", &diagnostics); + // then we do the actual compilation pass: + match fea_rs::compile::compile( + &ast.ast, + &marks.glyphmap, + Some(&var_info), + Some(&feature_writer), + ) { + Ok((result, warnings)) => { + log_fea_warnings("compilation", &warnings); + Ok(result) } + Err(errors) => Err(Error::FeaCompileError(CompilerError::CompilationFail( + errors, + ))), } - .with_variable_info(&var_info) - .with_feature_writer(&feature_writer) - .compile() - .map_err(Error::FeaCompileError) } } @@ -420,9 +413,9 @@ fn write_debug_fea(context: &Context, is_error: bool, why: &str, fea_content: &s }; } -impl Work for FeatureWork { +impl Work for FeatureParsingWork { fn id(&self) -> AnyWorkId { - WorkId::Features.into() + WorkId::FeaturesAst.into() } fn read_access(&self) -> Access { @@ -430,6 +423,82 @@ impl Work for FeatureWork { .variant(FeWorkId::GlyphOrder) .variant(FeWorkId::StaticMetadata) .variant(FeWorkId::Features) + .build() + } + + fn exec(&self, context: &Context) -> Result<(), Error> { + let features = context.ir.features.get(); + let glyph_order = context.ir.glyph_order.get(); + let result = self.parse(&features, &glyph_order); + + if let FeaturesSource::Memory { fea_content, .. } = features.as_ref() { + write_debug_fea(context, result.is_err(), "compile failed", fea_content); + } + context.fea_ast.set(result?.into()); + Ok(()) + } +} + +impl FeatureParsingWork { + pub fn create() -> Box { + Box::new(Self {}) + } + + fn parse( + &self, + features: &FeaturesSource, + glyph_order: &GlyphOrder, + ) -> Result { + let (resolver, root_path) = get_resolver_and_root_path(features); + let glyph_map = glyph_order.iter().map(|g| g.clone().into_inner()).collect(); + let (tree, diagnostics) = fea_rs::parse::parse_root(root_path, Some(&glyph_map), resolver) + .map_err(CompilerError::SourceLoad)?; + if diagnostics.has_errors() { + return Err(CompilerError::ParseFail(diagnostics).into()); + } + log_fea_warnings("parsing", &diagnostics); + Ok(tree) + } +} + +fn get_resolver_and_root_path(features: &FeaturesSource) -> (Box, OsString) { + match features { + FeaturesSource::File { + fea_file, + include_dir, + } => { + let project_root = include_dir + .clone() + .or_else(|| fea_file.parent().map(PathBuf::from)) + .unwrap_or_default(); + ( + Box::new(FileSystemResolver::new(project_root)), + fea_file.to_owned().into_os_string(), + ) + } + FeaturesSource::Memory { + fea_content, + include_dir, + } => ( + Box::new(InMemoryResolver { + include_dir: include_dir.to_owned(), + content_path: OsString::new(), + content: fea_content.as_str().into(), + }), + OsString::new(), + ), + FeaturesSource::Empty => (Box::new(InMemoryResolver::empty()), Default::default()), + } +} + +impl Work for FeatureCompilationWork { + fn id(&self) -> AnyWorkId { + WorkId::Features.into() + } + + fn read_access(&self) -> Access { + AccessBuilder::new() + .variant(WorkId::FeaturesAst) .variant(WorkId::GatherBeKerning) .variant(WorkId::Marks) .build() @@ -445,22 +514,11 @@ impl Work for FeatureWork { fn exec(&self, context: &Context) -> Result<(), Error> { let static_metadata = context.ir.static_metadata.get(); - let features = context.ir.features.get(); + let ast = context.fea_ast.get(); let kerns = context.fea_rs_kerns.get(); let marks = context.fea_rs_marks.get(); - let result = self.compile( - &static_metadata, - features.as_ref(), - kerns.as_ref(), - marks.as_ref(), - ); - if result.is_err() || context.flags.contains(Flags::EMIT_DEBUG) { - if let FeaturesSource::Memory { fea_content, .. } = features.as_ref() { - write_debug_fea(context, result.is_err(), "compile failed", fea_content); - } - } - let result = result?; + let result = self.compile(&static_metadata, &ast, kerns.as_ref(), marks.as_ref())?; debug!( "Built features, gpos? {} gsub? {} gdef? {}", @@ -493,6 +551,17 @@ impl Work for FeatureWork { } } +fn log_fea_warnings(stage: &str, warnings: &DiagnosticSet) { + assert!(!warnings.has_errors(), "of course we checked this already"); + if !warnings.is_empty() { + log::debug!( + "FEA {stage} produced {} warnings:\n{}", + warnings.len(), + warnings.display() + ); + } +} + #[cfg(test)] mod tests { use std::collections::{HashMap, HashSet}; diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index 20c313a2d..bdd3e128f 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -12,7 +12,7 @@ use fea_rs::{ compile::{ MarkToBaseBuilder, MarkToMarkBuilder, PairPosBuilder, ValueRecord as ValueRecordBuilder, }, - GlyphMap, GlyphSet, + GlyphMap, GlyphSet, ParseTree, }; use fontdrasil::{ coords::NormalizedLocation, @@ -81,6 +81,7 @@ type KernBlock = usize; #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] pub enum WorkId { Features, + FeaturesAst, Avar, Cmap, Font, @@ -114,6 +115,7 @@ impl Identifier for WorkId { fn discriminant(&self) -> IdentifierDiscriminant { match self { WorkId::Features => "BeFeatures", + WorkId::FeaturesAst => "BeFeaturesAst", WorkId::Avar => "BeAvar", WorkId::Cmap => "BeCmap", WorkId::Font => "BeFont", @@ -373,6 +375,20 @@ pub struct FeaRsKerns { pub lookups: Vec, } +/// The Ast for any user features. +/// +/// This is a newtype so that we can implement `Persistable` +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +pub struct FeaAst { + pub ast: ParseTree, +} + +impl From for FeaAst { + fn from(ast: ParseTree) -> FeaAst { + FeaAst { ast } + } +} + impl FeaRsKerns { pub fn is_empty(&self) -> bool { self.lookups.is_empty() @@ -389,6 +405,16 @@ impl Persistable for FeaRsKerns { } } +impl Persistable for FeaAst { + fn read(from: &mut dyn Read) -> Self { + bincode::deserialize_from(from).unwrap() + } + + fn write(&self, to: &mut dyn Write) { + bincode::serialize_into(to, self).unwrap() + } +} + /// Kerning adjustments at various locations pub type KernAdjustments = BTreeMap>; @@ -608,6 +634,7 @@ pub struct Context { pub mvar: BeContextItem>, pub all_kerning_pairs: BeContextItem, pub kern_fragments: BeContextMap, + pub fea_ast: BeContextItem, pub fea_rs_kerns: BeContextItem, pub fea_rs_marks: BeContextItem, pub stat: BeContextItem>, @@ -647,6 +674,7 @@ impl Context { fea_rs_kerns: self.fea_rs_kerns.clone_with_acl(acl.clone()), fea_rs_marks: self.fea_rs_marks.clone_with_acl(acl.clone()), stat: self.stat.clone_with_acl(acl.clone()), + fea_ast: self.fea_ast.clone_with_acl(acl.clone()), font: self.font.clone_with_acl(acl), } } @@ -702,6 +730,11 @@ impl Context { acl.clone(), persistent_storage.clone(), ), + fea_ast: ContextItem::new( + WorkId::FeaturesAst.into(), + acl.clone(), + persistent_storage.clone(), + ), stat: ContextItem::new(WorkId::Stat.into(), acl.clone(), persistent_storage.clone()), font: ContextItem::new(WorkId::Font.into(), acl, persistent_storage), } diff --git a/fontbe/src/paths.rs b/fontbe/src/paths.rs index debea9744..9ea6d0730 100644 --- a/fontbe/src/paths.rs +++ b/fontbe/src/paths.rs @@ -65,6 +65,7 @@ impl Paths { pub fn target_file(&self, id: &WorkId) -> PathBuf { match id { WorkId::Features => self.build_dir.join("features.marker"), + WorkId::FeaturesAst => self.build_dir.join("features_ast.marker"), WorkId::GlyfFragment(name) => self.glyph_glyf_file(name.as_str()), WorkId::GvarFragment(name) => self.glyph_gvar_file(name.as_str()), WorkId::Avar => self.build_dir.join("avar.table"), diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs index f183e0b6c..f7f7ebfc4 100644 --- a/fontc/src/lib.rs +++ b/fontc/src/lib.rs @@ -24,7 +24,7 @@ use std::{ use fontbe::{ avar::create_avar_work, cmap::create_cmap_work, - features::FeatureWork, + features::{FeatureCompilationWork, FeatureParsingWork}, font::create_font_work, fvar::create_fvar_work, glyphs::{create_glyf_loca_work, create_glyf_work}, @@ -128,8 +128,30 @@ fn add_feature_ir_job(workload: &mut Workload) -> Result<(), Error> { Ok(()) } -fn add_feature_be_job(workload: &mut Workload) -> Result<(), Error> { - let work = FeatureWork::create(); +fn add_feature_parse_be_job(workload: &mut Workload) -> Result<(), Error> { + let work = FeatureParsingWork::create(); + + // copied from below + if workload.change_detector.feature_be_change() { + if workload.change_detector.glyph_name_filter().is_some() { + warn!("Not processing BE Features because a glyph name filter is active"); + } + if workload.change_detector.should_skip_features() { + debug!("Not processing BE Features because FEA compilation is disabled"); + } + } + + workload.add( + work.into(), + workload.change_detector.feature_be_change() + && workload.change_detector.glyph_name_filter().is_none() + && !workload.change_detector.should_skip_features(), + ); + Ok(()) +} + +fn add_feature_comp_be_job(workload: &mut Workload) -> Result<(), Error> { + let work = FeatureCompilationWork::create(); // Features are extremely prone to not making sense when glyphs are filtered if workload.change_detector.feature_be_change() { if workload.change_detector.glyph_name_filter().is_some() { @@ -410,7 +432,9 @@ pub fn create_workload( add_glyph_order_ir_job(&mut workload)?; // BE: f(IR, maybe other BE work) => binary - add_feature_be_job(&mut workload)?; + + add_feature_parse_be_job(&mut workload)?; + add_feature_comp_be_job(&mut workload)?; add_glyph_be_jobs(&mut workload)?; add_glyf_loca_be_job(&mut workload)?; add_avar_be_job(&mut workload)?; @@ -698,6 +722,7 @@ mod tests { FeWorkIdentifier::KernInstance(NormalizedLocation::for_pos(&[("wght", 0.0)])).into(), FeWorkIdentifier::KernInstance(NormalizedLocation::for_pos(&[("wght", 1.0)])).into(), BeWorkIdentifier::Features.into(), + BeWorkIdentifier::FeaturesAst.into(), BeWorkIdentifier::Avar.into(), BeWorkIdentifier::Cmap.into(), BeWorkIdentifier::Font.into(), @@ -809,6 +834,7 @@ mod tests { AnyWorkId::Fe(FeWorkIdentifier::Glyph("bar".into())), FeWorkIdentifier::Anchor("bar".into()).into(), BeWorkIdentifier::Features.into(), + BeWorkIdentifier::FeaturesAst.into(), BeWorkIdentifier::Cmap.into(), BeWorkIdentifier::Font.into(), BeWorkIdentifier::Glyf.into(), @@ -850,6 +876,7 @@ mod tests { FeWorkIdentifier::KernInstance(NormalizedLocation::for_pos(&[("wght", 1.0)])) .into(), BeWorkIdentifier::Features.into(), + BeWorkIdentifier::FeaturesAst.into(), BeWorkIdentifier::Font.into(), BeWorkIdentifier::Gpos.into(), BeWorkIdentifier::Gsub.into(), @@ -963,6 +990,7 @@ mod tests { &[("wght", 1.0)] ))), BeWorkIdentifier::Features.into(), + BeWorkIdentifier::FeaturesAst.into(), BeWorkIdentifier::Font.into(), BeWorkIdentifier::Gpos.into(), BeWorkIdentifier::Gsub.into(), diff --git a/fontc/src/timing.rs b/fontc/src/timing.rs index 79f1e2f88..eb6d00c91 100644 --- a/fontc/src/timing.rs +++ b/fontc/src/timing.rs @@ -135,6 +135,7 @@ fn short_name(id: &AnyWorkId) -> &'static str { AnyWorkId::Be(BeWorkIdentifier::Avar) => "avar", AnyWorkId::Be(BeWorkIdentifier::Cmap) => "cmap", AnyWorkId::Be(BeWorkIdentifier::Features) => "fea", + AnyWorkId::Be(BeWorkIdentifier::FeaturesAst) => "fea.ast", AnyWorkId::Be(BeWorkIdentifier::Font) => "font", AnyWorkId::Be(BeWorkIdentifier::Fvar) => "fvar", AnyWorkId::Be(BeWorkIdentifier::Gdef) => "GDEF", @@ -179,6 +180,7 @@ fn color(id: &AnyWorkId) -> &'static str { AnyWorkId::Be(BeWorkIdentifier::Avar) => "gray", AnyWorkId::Be(BeWorkIdentifier::Cmap) => "gray", AnyWorkId::Be(BeWorkIdentifier::Features) => "gray", + AnyWorkId::Be(BeWorkIdentifier::FeaturesAst) => "gray", AnyWorkId::Be(BeWorkIdentifier::Font) => "gray", AnyWorkId::Be(BeWorkIdentifier::Fvar) => "gray", AnyWorkId::Be(BeWorkIdentifier::Gdef) => "gray",