From ec26529820a6e0c0eaac6a285e79c2c7dce8d499 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 12 Feb 2025 17:59:10 -0500 Subject: [PATCH 1/2] [fea-rs] Respect markers when adding external lookups That is, we want to respect the '# Automatic Code' comments that are used in glyphs sources. Previously we would always just append any generated looups at the back of the list; now if these markers are present we will insert the lookups at that position. --- fea-rs/src/compile/compile_ctx.rs | 52 ++++- fea-rs/src/compile/features.rs | 27 ++- fea-rs/src/compile/lookups.rs | 229 +++++++++++++++++++++-- fea-rs/src/compile/lookups/contextual.rs | 38 +++- fea-rs/src/compile/validate.rs | 1 + fea-rs/src/token_tree/typed.rs | 14 +- resources/scripts/ttx_diff.py | 1 + 7 files changed, 338 insertions(+), 24 deletions(-) diff --git a/fea-rs/src/compile/compile_ctx.rs b/fea-rs/src/compile/compile_ctx.rs index 267f33e2d..f8b23b98f 100644 --- a/fea-rs/src/compile/compile_ctx.rs +++ b/fea-rs/src/compile/compile_ctx.rs @@ -92,6 +92,12 @@ pub struct CompilationCtx<'a, F: FeatureProvider, V: VariationInfo> { conditionset_defs: ConditionSetMap, mark_attach_class_id: HashMap, mark_filter_sets: HashMap, + // feature blocks can include `# Automatic Code` comments that instruct us + // on where we should merge in code generated from an external provider. + // when we encounter these we record what lookup id would be logically next, + // and we will use that for the generated lookups. + // We also store the start pos of the comment, to break ties. + insert_markers: HashMap, } impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> { @@ -125,6 +131,7 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> { mark_attach_class_id: Default::default(), mark_filter_sets: Default::default(), opts, + insert_markers: Default::default(), } } @@ -277,12 +284,14 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> { writer.add_features(&mut builder); // now we need to merge in the newly generated features. - let id_map = self.lookups.merge_external_lookups(builder.lookups); - builder - .features - .values_mut() - .for_each(|feat| feat.base.iter_mut().for_each(|id| *id = id_map.get(*id))); + let id_map = self.lookups.merge_external_lookups( + builder.lookups, + &builder.features, + &self.insert_markers, + ); self.features.merge_external_features(builder.features); + self.features.remap_ids(&id_map); + self.lookups.remap_ids(&id_map); builder.lig_carets } @@ -1888,6 +1897,39 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> { } } else if item.kind() == Kind::Semi { // continue + } else if item.kind() == Kind::Comment { + assert!(item + .token_text() + .unwrap_or_default() + .contains("# Automatic Code")); + if self.active_feature.is_none() { + self.warning( + item.range(), + "Insertion marker outside feature block will be ignored", + ); + return; + }; + + // make sure we finish any active lookup before assigning the lookupid + if let Some((id, _name)) = self.lookups.finish_current() { + if _name.is_some() { + self.error( + item.range(), + "insertion marker cannot be inside named lookup block", + ); + } + self.add_lookup_to_current_feature_if_present(id); + } + let current_feature = self.active_feature.as_ref().unwrap().tag; + + // we can have multiple markers in a row without any lookups between, + // but we care about the order; so along with the lookup id we also + // store the comment position, which breaks ties. + let comment_start = item.range().start; + self.insert_markers.insert( + current_feature, + (self.lookups.next_gpos_id(), comment_start), + ); } else { let span = match item { NodeOrToken::Token(t) => t.range(), diff --git a/fea-rs/src/compile/features.rs b/fea-rs/src/compile/features.rs index 9e13c7d42..723b222d5 100644 --- a/fea-rs/src/compile/features.rs +++ b/fea-rs/src/compile/features.rs @@ -10,7 +10,7 @@ use write_fonts::{ use super::{ language_system::{DefaultLanguageSystems, LanguageSystem}, - lookups::{AllLookups, FeatureKey, LookupId}, + lookups::{AllLookups, FeatureKey, LookupId, LookupIdMap}, tables::{NameBuilder, NameSpec}, tags, }; @@ -41,7 +41,7 @@ pub(crate) struct AllFeatures { /// and script, and add any lookups as appropriate; this struct handles that /// logic. pub(crate) struct ActiveFeature { - tag: Tag, + pub(crate) tag: Tag, condition_set: Option, default_systems: DefaultLanguageSystems, current_lang_sys: Option, @@ -124,6 +124,12 @@ impl AllFeatures { self.features.iter() } + pub(crate) fn remap_ids(&mut self, id_map: &LookupIdMap) { + self.features + .values_mut() + .for_each(|lookups| lookups.remap_ids(id_map)); + } + pub(crate) fn finalize_aalt( &mut self, all_lookups: &mut AllLookups, @@ -189,7 +195,8 @@ impl AllFeatures { let mut seen = HashSet::new(); for lookup in self.features.values_mut() { seen.clear(); - lookup.base.retain(|id| seen.insert(*id)) + lookup.base.retain(|id| seen.insert(*id)); + lookup.base.sort(); } } @@ -254,6 +261,20 @@ impl FeatureLookups { .for_each(|id| id.adjust_if_gsub(delta)); } + pub(crate) fn remap_ids(&mut self, id_map: &LookupIdMap) { + self.base + .iter_mut() + .chain(self.variations.values_mut().flat_map(|x| x.iter_mut())) + .for_each(|id| *id = id_map.get(*id)); + } + + pub(crate) fn iter_ids(&self) -> impl Iterator + use<'_> { + self.base + .iter() + .chain(self.variations.values().flat_map(|v| v.iter())) + .copied() + } + // split lookups into gpos/gsub pub(crate) fn split_base_lookups(&self) -> (Vec, Vec) { split_lookups(&self.base) diff --git a/fea-rs/src/compile/lookups.rs b/fea-rs/src/compile/lookups.rs index 5e226aff1..e2722fb82 100644 --- a/fea-rs/src/compile/lookups.rs +++ b/fea-rs/src/compile/lookups.rs @@ -35,7 +35,11 @@ use crate::{ Kind, Opts, }; -use super::{features::AllFeatures, metrics::Anchor, tags}; +use super::{ + features::{AllFeatures, FeatureLookups}, + metrics::Anchor, + tags, +}; use contextual::{ ContextualLookupBuilder, PosChainContextBuilder, PosContextBuilder, ReverseChainBuilder, @@ -86,7 +90,7 @@ pub(crate) struct AllLookups { named: HashMap, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub(crate) struct LookupBuilder { flags: LookupFlag, mark_set: Option, @@ -258,7 +262,40 @@ impl LookupBuilder { } } +trait RemapIds { + fn remap_ids(&mut self, id_map: &LookupIdMap); +} + +impl RemapIds for LookupBuilder { + fn remap_ids(&mut self, id_map: &LookupIdMap) { + for lookup in &mut self.subtables { + lookup.remap_ids(id_map); + } + } +} + impl PositionLookup { + fn remap_ids(&mut self, id_map: &LookupIdMap) { + match self { + PositionLookup::Contextual(lookup) => lookup.remap_ids(id_map), + PositionLookup::ChainedContextual(lookup) => lookup.remap_ids(id_map), + _ => (), + } + } + + fn kind(&self) -> Kind { + match self { + PositionLookup::Single(_) => Kind::GposType1, + PositionLookup::Pair(_) => Kind::GposType2, + PositionLookup::Cursive(_) => Kind::GposType3, + PositionLookup::MarkToBase(_) => Kind::GposType4, + PositionLookup::MarkToLig(_) => Kind::GposType5, + PositionLookup::MarkToMark(_) => Kind::GposType6, + PositionLookup::Contextual(_) => Kind::GposType7, + PositionLookup::ChainedContextual(_) => Kind::GposType8, + } + } + fn force_subtable_break(&mut self) { match self { PositionLookup::Single(lookup) => lookup.force_subtable_break(), @@ -274,6 +311,14 @@ impl PositionLookup { } impl SubstitutionLookup { + fn remap_ids(&mut self, id_map: &LookupIdMap) { + match self { + SubstitutionLookup::Contextual(lookup) => lookup.remap_ids(id_map), + SubstitutionLookup::ChainedContextual(lookup) => lookup.remap_ids(id_map), + _ => (), + } + } + fn force_subtable_break(&mut self) { match self { SubstitutionLookup::Single(lookup) => lookup.force_subtable_break(), @@ -429,6 +474,10 @@ impl AllLookups { self.current.is_some() } + pub(crate) fn next_gpos_id(&self) -> LookupId { + LookupId::Gpos(self.gpos.len()) + } + /// Returns `true` if there is an active lookup of this kind pub(crate) fn has_current_kind(&self, kind: Kind) -> bool { self.current.as_ref().map(SomeLookup::kind) == Some(kind) @@ -595,6 +644,15 @@ impl AllLookups { } } + pub(crate) fn remap_ids(&mut self, ids: &LookupIdMap) { + self.gpos + .iter_mut() + .for_each(|lookup| lookup.remap_ids(ids)); + self.gsub + .iter_mut() + .for_each(|lookup| lookup.remap_ids(ids)); + } + pub(crate) fn insert_aalt_lookups( &mut self, all_alts: HashMap>, @@ -653,8 +711,86 @@ impl AllLookups { pub(crate) fn merge_external_lookups( &mut self, lookups: Vec<(LookupId, PositionLookup)>, + features: &BTreeMap, + insert_markers: &HashMap, ) -> LookupIdMap { + // okay so this is a bit complicated, so a brief outline might be useful: + // - 'lookups' and 'features' are being passed in from the outside + // - `insert_markers` are optional locations at which the lookups + // for a given feature should be inserted + // - if a feature has no marker, its lookups are appended at the end. + // + // NOTE: inserting lookups into the middle of the lookup list means that + // all subsequent lookup ids become invalid. As a consequence, we need + // to figure out the transform from old->new for any affected lookups, + // and remap those ids at the end. + let lookup_to_feature = features + .iter() + .filter(|(feat, _)| insert_markers.contains_key(&feat.feature)) + .flat_map(|(feat, lookups)| lookups.iter_ids().map(|id| (id, feat.feature))) + .collect::>(); + + // split off the lookups with an insert marker, which we will handle first + let (lookups_with_marker, lookups): (Vec<_>, Vec<_>) = lookups + .into_iter() + .partition(|lk| lookup_to_feature.contains_key(&lk.0)); + + // now we want to process the lookups that had an insert marker, + // and we want to do it by grouping them by feature tag. + let mut marked_lookups_by_feature = HashMap::new(); + for lk in lookups_with_marker { + let feature = lookup_to_feature.get(&lk.0).unwrap(); + marked_lookups_by_feature + .entry(*feature) + .or_insert(Vec::new()) + .push(lk); + } + + // except we want to assign the ids respecting the order of the markers + // in the source file, so convert to a vec and sort. + let mut marked_lookups_by_feature = + marked_lookups_by_feature.into_iter().collect::>(); + marked_lookups_by_feature.sort_by_key(|(tag, _)| insert_markers.get(tag).unwrap()); + let mut map = LookupIdMap::default(); + let mut inserted_so_far = 0; + + // 'adjustments' stores the state we need to remap existing ids, if needed. + let mut adjustments = Vec::new(); + + for (tag, lookups) in marked_lookups_by_feature { + let first_id = insert_markers.get(&tag).unwrap().0.to_raw(); + // first update the ids + for (i, (temp_id, _)) in lookups.iter().enumerate() { + let final_id = LookupId::Gpos(first_id + inserted_so_far + i); + map.insert(*temp_id, final_id); + } + // then insert the lookups into the correct position + let insert_at = first_id + inserted_so_far; + inserted_so_far += lookups.len(); + self.gpos + .splice(insert_at..insert_at, lookups.into_iter().map(|(_, lk)| lk)); + adjustments.push((first_id, inserted_so_far)); + } + + // now based on our recorded adjustments, figure out the remapping + // for the existing ids. each entry in adjustment is an (index, delta) + // pair, where the delta applies from adjustment[n] to adjustment[n +1] + if !adjustments.is_empty() { + // add the end of the last range + adjustments.push((self.gpos.len(), inserted_so_far)); + } + let (mut range_start, mut adjust) = (0, 0); + let mut adjustments = adjustments.as_slice(); + while let Some(((next_start, next_adjust), remaining)) = adjustments.split_first() { + if adjust > 0 { + for old_id in range_start..*next_start { + map.insert(LookupId::Gpos(old_id), LookupId::Gpos(old_id + adjust)); + } + } + (range_start, adjust, adjustments) = (*next_start, *next_adjust, remaining); + } + for (temp_id, lookup) in lookups { let final_id = self.push(SomeLookup::GposLookup(lookup)); map.insert(temp_id, final_id); @@ -829,16 +965,7 @@ impl SomeLookup { SubstitutionLookup::Reverse(_) => Kind::GsubType8, _ => panic!("unhandled table kind"), }, - SomeLookup::GposLookup(gpos) => match gpos { - PositionLookup::Single(_) => Kind::GposType1, - PositionLookup::Pair(_) => Kind::GposType2, - PositionLookup::Cursive(_) => Kind::GposType3, - PositionLookup::MarkToBase(_) => Kind::GposType4, - PositionLookup::MarkToLig(_) => Kind::GposType5, - PositionLookup::MarkToMark(_) => Kind::GposType6, - PositionLookup::Contextual(_) => Kind::GposType7, - PositionLookup::ChainedContextual(_) => Kind::GposType8, - }, + SomeLookup::GposLookup(gpos) => gpos.kind(), } } @@ -1229,3 +1356,81 @@ fn is_gpos_rule(kind: Kind) -> bool { | Kind::GposType8 ) } + +#[cfg(test)] +mod tests { + use crate::compile::tags::{LANG_DFLT, SCRIPT_DFLT}; + + use super::*; + + fn make_all_lookups() -> AllLookups { + AllLookups { + gpos: (0..8) + .map(|_| PositionLookup::Single(Default::default())) + .collect(), + ..Default::default() + } + } + + #[test] + fn merge_external_lookups_before() { + const KERN: Tag = Tag::new(b"kern"); + const WHAT: Tag = Tag::new(b"what"); + const DERP: Tag = Tag::new(b"derp"); + let mut all = make_all_lookups(); + + let lookups = (0..6) + .map(|id| { + ( + LookupId::External(id), + PositionLookup::Pair(Default::default()), + ) + }) + .collect(); + let features: BTreeMap<_, _> = + [(KERN, [0].as_slice()), (WHAT, &[1, 2]), (DERP, &[3, 4, 5])] + .iter() + .map(|(tag, ids)| { + let mut features = FeatureLookups::default(); + features.base = ids.iter().copied().map(LookupId::External).collect(); + (FeatureKey::new(*tag, LANG_DFLT, SCRIPT_DFLT), features) + }) + .collect(); + + // kern is 'after', derp is 'before', and what has no marker (goes at the end) + let markers = HashMap::from([ + (KERN, (LookupId::Gpos(3), 100)), + (DERP, (LookupId::Gpos(5), 200)), + ]); + + let id_map = all.merge_external_lookups(lookups, &features, &markers); + let final_lookups = all.gpos.iter().map(|lk| lk.kind()).collect::>(); + assert_eq!( + final_lookups, + [ + Kind::GposType1, + Kind::GposType1, + Kind::GposType1, + Kind::GposType2, // one kern lookup at 3 + Kind::GposType1, + Kind::GposType1, + Kind::GposType2, // 3 derp lookups at 5 + Kind::GposType2, + Kind::GposType2, + Kind::GposType1, + Kind::GposType1, + Kind::GposType1, + Kind::GposType2, // 2 what lookups at the end + Kind::GposType2 + ] + ); + let remapped = (0..8) + .map(|id| id_map.get(LookupId::Gpos(id)).to_gpos_id_or_die()) + .collect::>(); + assert_eq!(remapped, [0, 1, 2, 4, 5, 9, 10, 11]); + let inserted = (0..6) + .map(|id| id_map.get(LookupId::External(id)).to_gpos_id_or_die()) + .collect::>(); + assert_eq!(inserted, [3, 12, 13, 6, 7, 8]); + } +} diff --git a/fea-rs/src/compile/lookups/contextual.rs b/fea-rs/src/compile/lookups/contextual.rs index 4e88a9a2e..e5b2d59ac 100644 --- a/fea-rs/src/compile/lookups/contextual.rs +++ b/fea-rs/src/compile/lookups/contextual.rs @@ -7,8 +7,7 @@ use std::{ use write_fonts::{ tables::{ - gsub as write_gsub, - gsub::ReverseChainSingleSubstFormat1, + gsub::{self as write_gsub, ReverseChainSingleSubstFormat1}, layout::{self as write_layout, CoverageTableBuilder, LookupFlag}, variations::ivs_builder::VariationStoreBuilder, }, @@ -20,7 +19,7 @@ use write_fonts::{ use crate::{common::GlyphOrClass, compile::metrics::ValueRecord}; use super::{ - Builder, ClassDefBuilder2, FilterSetId, LookupBuilder, LookupId, PositionLookup, + Builder, ClassDefBuilder2, FilterSetId, LookupBuilder, LookupId, PositionLookup, RemapIds, SubstitutionLookup, }; @@ -449,6 +448,28 @@ impl Builder for SubContextBuilder { } } +impl RemapIds for PosContextBuilder { + fn remap_ids(&mut self, id_map: &super::LookupIdMap) { + self.0.remap_ids(id_map); + } +} + +impl RemapIds for SubContextBuilder { + fn remap_ids(&mut self, id_map: &super::LookupIdMap) { + self.0.remap_ids(id_map); + } +} + +impl RemapIds for ContextBuilder { + fn remap_ids(&mut self, id_map: &super::LookupIdMap) { + for rule in &mut self.rules { + for (_, lookups) in &mut rule.context { + lookups.iter_mut().for_each(|id| *id = id_map.get(*id)); + } + } + } +} + impl ContextBuilder { fn build( self, @@ -687,6 +708,17 @@ impl Builder for SubChainContextBuilder { } } +impl RemapIds for PosChainContextBuilder { + fn remap_ids(&mut self, id_map: &super::LookupIdMap) { + self.0 .0.remap_ids(id_map); + } +} + +impl RemapIds for SubChainContextBuilder { + fn remap_ids(&mut self, id_map: &super::LookupIdMap) { + self.0 .0.remap_ids(id_map); + } +} // invariant: at least one item must be Some fn pick_best_format(subtables: [Option>; 3]) -> Vec { // first see if there's only one table present, in which case we can exit early: diff --git a/fea-rs/src/compile/validate.rs b/fea-rs/src/compile/validate.rs index 09298aa43..f528e288e 100644 --- a/fea-rs/src/compile/validate.rs +++ b/fea-rs/src/compile/validate.rs @@ -605,6 +605,7 @@ impl<'a, V: VariationInfo> ValidationCtx<'a, V> { || item.kind() == Kind::LanguageNode || item.kind() == Kind::SubtableNode || item.kind() == Kind::Semi + || item.kind() == Kind::Comment { // lgtm } else if let Some(node) = typed::CvParameters::cast(item) { diff --git a/fea-rs/src/token_tree/typed.rs b/fea-rs/src/token_tree/typed.rs index 8343dda73..86ea5edc4 100644 --- a/fea-rs/src/token_tree/typed.rs +++ b/fea-rs/src/token_tree/typed.rs @@ -672,10 +672,22 @@ impl Feature { } pub(crate) fn statements(&self) -> impl Iterator { + fn filter_trivia_except_for_magic_insertion_comments(item: &&NodeOrToken) -> bool { + match item.kind() { + Kind::Comment => item + .token_text() + .unwrap_or_default() + .trim_start() + //https://github.com/googlefonts/ufo2ft/blob/5a606b7884bb6da5/Lib/ufo2ft/featureWriters/baseFeatureWriter.py#L18 + .starts_with("# Automatic Code"), + other => !other.is_trivia(), + } + } + self.iter() .skip_while(|t| t.kind() != Kind::LBrace) .skip(1) - .filter(|t| !t.kind().is_trivia()) + .filter(filter_trivia_except_for_magic_insertion_comments) .take_while(|t| t.kind() != Kind::RBrace) } } diff --git a/resources/scripts/ttx_diff.py b/resources/scripts/ttx_diff.py index 03c959857..75045f5a0 100755 --- a/resources/scripts/ttx_diff.py +++ b/resources/scripts/ttx_diff.py @@ -228,6 +228,7 @@ def build_fontc(source: Path, fontc_bin: Path, build_dir: Path): "-o", out_file.name, source, + "--emit-debug", ] build(cmd, build_dir) From a2baa71388942036cc808f4acb79a814c76f02d3 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Wed, 12 Feb 2025 19:26:34 -0500 Subject: [PATCH 2/2] [fea-rs] Add ttx tests for external feature merging --- fea-rs/src/util/ttx.rs | 59 +++++- .../good/provider_basic_insert_marker.fea | 15 ++ .../good/provider_basic_insert_marker.ttx | 174 ++++++++++++++++++ .../good/provider_insert_between_rules.fea | 10 + .../good/provider_insert_between_rules.ttx | 130 +++++++++++++ .../good/provider_remap_contextual_ids.fea | 8 + .../good/provider_remap_contextual_ids.ttx | 141 ++++++++++++++ .../good/provider_respect_ordering.fea | 12 ++ .../good/provider_respect_ordering.ttx | 104 +++++++++++ 9 files changed, 651 insertions(+), 2 deletions(-) create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/provider_basic_insert_marker.fea create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/provider_basic_insert_marker.ttx create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/provider_insert_between_rules.fea create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/provider_insert_between_rules.ttx create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/provider_remap_contextual_ids.fea create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/provider_remap_contextual_ids.ttx create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/provider_respect_ordering.fea create mode 100644 fea-rs/test-data/compile-tests/mini-latin/good/provider_respect_ordering.ttx diff --git a/fea-rs/src/util/ttx.rs b/fea-rs/src/util/ttx.rs index ac73c0f20..3ba71baae 100644 --- a/fea-rs/src/util/ttx.rs +++ b/fea-rs/src/util/ttx.rs @@ -10,13 +10,21 @@ use std::{ }; use crate::{ - compile::{error::CompilerError, Compiler, MockVariationInfo, NopFeatureProvider, Opts}, + compile::{ + error::CompilerError, Anchor, Compiler, FeatureProvider, MarkToBaseBuilder, + MockVariationInfo, Opts, PairPosBuilder, PendingLookup, ValueRecord, + }, DiagnosticSet, GlyphIdent, GlyphMap, ParseTree, }; use ansi_term::Color; use rayon::prelude::*; use serde::{Deserialize, Serialize}; +use smol_str::SmolStr; +use write_fonts::{ + tables::layout::LookupFlag, + types::{GlyphId16, Tag}, +}; static IGNORED_TESTS: &[&str] = &[ // ## tests with unofficial syntax extensiosn we haven't implemented yet ## // @@ -236,6 +244,11 @@ pub(crate) fn is_variable(test_path: &Path) -> bool { as_str.contains("variable") || as_str.contains("variation") } +pub(crate) fn needs_feature_provider(test_path: &Path) -> bool { + let as_str = test_path.to_str().expect("paths are utf8"); + as_str.contains("provider") +} + /// Run the test case at the provided path. /// /// the provided fvar table will be passed through only if the path contains @@ -246,13 +259,16 @@ pub(crate) fn run_test( fvar: &MockVariationInfo, ) -> Result { let run_result = std::panic::catch_unwind(|| { - let mut compiler: Compiler<'_, NopFeatureProvider, MockVariationInfo> = + let mut compiler: Compiler<'_, TestFeatureProvider, MockVariationInfo> = Compiler::new(path.clone(), glyph_map) .print_warnings(std::env::var(super::VERBOSE).is_ok()) .with_opts(Opts::new().make_post_table(true)); if is_variable(&path) { compiler = compiler.with_variable_info(fvar); } + if needs_feature_provider(&path) { + compiler = compiler.with_feature_writer(&TestFeatureProvider) + } match compiler.compile_binary() { // this means we have a test case that doesn't exist or something weird @@ -547,6 +563,45 @@ pub(crate) fn make_var_info() -> MockVariationInfo { MockVariationInfo::new(&[("wght", 200, 200, 1000), ("wdth", 100, 100, 200)]) } +struct TestFeatureProvider; + +impl FeatureProvider for TestFeatureProvider { + // we always add one lookup each for 'kern' and 'mark', which + // seems like enough for our purposes + fn add_features(&self, builder: &mut crate::compile::FeatureBuilder) { + let mut kern = PairPosBuilder::default(); + kern.insert_pair( + GlyphId16::new(20), + ValueRecord::new().with_x_advance(5), + GlyphId16::new(21), + ValueRecord::new(), + ); + let kern_id = builder.add_lookup(PendingLookup::new(vec![kern], LookupFlag::empty(), None)); + + let mut mark = MarkToBaseBuilder::default(); + mark.insert_mark( + GlyphId16::new(116), + SmolStr::new("derp"), + Anchor::new(101, 102), + ) + .unwrap(); + + mark.insert_base( + GlyphId16::new(36), + &SmolStr::new("derp"), + Anchor::new(11, 13), + ); + let mark_id = builder.add_lookup(PendingLookup::new( + vec![mark], + LookupFlag::IGNORE_MARKS, + None, + )); + + builder.add_to_default_language_systems(Tag::new(b"kern"), &[kern_id]); + builder.add_to_default_language_systems(Tag::new(b"mark"), &[mark_id]); + } +} + impl Report { /// Returns `true` if any tests have failed. pub fn has_failures(&self) -> bool { diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/provider_basic_insert_marker.fea b/fea-rs/test-data/compile-tests/mini-latin/good/provider_basic_insert_marker.fea new file mode 100644 index 000000000..d4b01ad07 --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/provider_basic_insert_marker.fea @@ -0,0 +1,15 @@ +# tests beginning with 'provider' get compiled with an external Feature Provider + +feature kern { + pos a b 10; + # Automatic Code +} kern; + +feature noop { + pos j k 404; +} noop; + +feature mark { +# Automatic Code +pos x y 5; +} mark; diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/provider_basic_insert_marker.ttx b/fea-rs/test-data/compile-tests/mini-latin/good/provider_basic_insert_marker.ttx new file mode 100644 index 000000000..01602baa7 --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/provider_basic_insert_marker.ttx @@ -0,0 +1,174 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/provider_insert_between_rules.fea b/fea-rs/test-data/compile-tests/mini-latin/good/provider_insert_between_rules.fea new file mode 100644 index 000000000..4255fce71 --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/provider_insert_between_rules.fea @@ -0,0 +1,10 @@ +# tests beginning with 'provider' get compiled with an external Feature Provider + +feature kern { + # these two statements should be split into separate lookups, with the + # external lookup between them. + pos a 5; + # Automatic Code + pos x 7; + +} kern; diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/provider_insert_between_rules.ttx b/fea-rs/test-data/compile-tests/mini-latin/good/provider_insert_between_rules.ttx new file mode 100644 index 000000000..894c9457e --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/provider_insert_between_rules.ttx @@ -0,0 +1,130 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/provider_remap_contextual_ids.fea b/fea-rs/test-data/compile-tests/mini-latin/good/provider_remap_contextual_ids.fea new file mode 100644 index 000000000..efef03556 --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/provider_remap_contextual_ids.fea @@ -0,0 +1,8 @@ +# tests beginning with 'provider' get compiled with an external Feature Provider + +feature kern { + # Automatic Code + # we need to ensure that the anonymous singlepos lookup here gets + # adjusted after the lookup is inserted ahead of it. + pos a a' 50 z; +} kern; diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/provider_remap_contextual_ids.ttx b/fea-rs/test-data/compile-tests/mini-latin/good/provider_remap_contextual_ids.ttx new file mode 100644 index 000000000..ad53069a6 --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/provider_remap_contextual_ids.ttx @@ -0,0 +1,141 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/provider_respect_ordering.fea b/fea-rs/test-data/compile-tests/mini-latin/good/provider_respect_ordering.fea new file mode 100644 index 000000000..b9a8c9a71 --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/provider_respect_ordering.fea @@ -0,0 +1,12 @@ +# tests beginning with 'provider' get compiled with an external Feature Provider + +feature kern { + # Automatic Code +} kern; + +feature mark { +# Automatic Code +} mark; + +# even though no lookups are created between 'kern' and 'mark', the external +# kern lookups should always precede the mark diff --git a/fea-rs/test-data/compile-tests/mini-latin/good/provider_respect_ordering.ttx b/fea-rs/test-data/compile-tests/mini-latin/good/provider_respect_ordering.ttx new file mode 100644 index 000000000..5331b9ce2 --- /dev/null +++ b/fea-rs/test-data/compile-tests/mini-latin/good/provider_respect_ordering.ttx @@ -0,0 +1,104 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +