diff --git a/fea-rs/src/common.rs b/fea-rs/src/common.rs index 4a55b354c..b17255518 100644 --- a/fea-rs/src/common.rs +++ b/fea-rs/src/common.rs @@ -3,7 +3,6 @@ use std::fmt::{Display, Formatter}; use smol_str::SmolStr; -use write_fonts::tables::gpos::AnchorTable; pub use write_fonts::types::GlyphId; mod glyph_class; @@ -14,6 +13,8 @@ pub(crate) use glyph_class::GlyphClass; pub use glyph_class::GlyphSet; pub use glyph_map::GlyphMap; +use crate::compile::Anchor; + /// A glyph name pub type GlyphName = SmolStr; @@ -41,7 +42,7 @@ pub enum GlyphIdent { #[derive(Clone, Debug, Default)] pub(crate) struct MarkClass { - pub(crate) members: Vec<(GlyphClass, Option)>, + pub(crate) members: Vec<(GlyphClass, Option)>, } impl> From for GlyphIdent { diff --git a/fea-rs/src/compile.rs b/fea-rs/src/compile.rs index cd1477b12..1134f80cc 100644 --- a/fea-rs/src/compile.rs +++ b/fea-rs/src/compile.rs @@ -19,6 +19,7 @@ pub use lookups::{ FeatureKey, LookupId, MarkToBaseBuilder, MarkToMarkBuilder, PairPosBuilder, PreviouslyAssignedClass, }; +pub use metrics::{Anchor, ValueRecord}; pub use opts::Opts; pub use output::Compilation; pub use variations::{AxisLocation, VariationInfo}; @@ -34,12 +35,12 @@ mod features; mod glyph_range; mod language_system; mod lookups; +mod metrics; mod opts; mod output; mod tables; mod tags; mod validate; -mod valuerecordext; mod variations; /// Run the validation pass, returning any diagnostics. diff --git a/fea-rs/src/compile/compile_ctx.rs b/fea-rs/src/compile/compile_ctx.rs index 430f52e10..2701e105d 100644 --- a/fea-rs/src/compile/compile_ctx.rs +++ b/fea-rs/src/compile/compile_ctx.rs @@ -14,10 +14,9 @@ use write_fonts::{ tables::{ self, gdef::CaretValue, - gpos::{AnchorTable, ValueFormat, ValueRecord}, - layout::{ - ConditionFormat1, ConditionSet, FeatureVariations, LookupFlag, PendingVariationIndex, - }, + gpos::ValueFormat, + layout::{ConditionFormat1, ConditionSet, FeatureVariations, LookupFlag}, + variations::ivs_builder::{RemapVariationIndices, VariationStoreBuilder}, }, types::{F2Dot14, NameId, Tag}, }; @@ -44,11 +43,10 @@ use super::{ lookups::{ AllLookups, FilterSetId, LookupFlagInfo, LookupId, PreviouslyAssignedClass, SomeLookup, }, + metrics::{Anchor, DeviceOrDeltas, Metric, ValueRecord}, output::Compilation, tables::{ClassId, ScriptRecord, Tables}, - tags, - valuerecordext::ValueRecordExt, - VariationInfo, + tags, VariationInfo, }; /// Context that manages state for a compilation. @@ -88,7 +86,7 @@ pub struct CompilationCtx<'a> { script: Option, glyph_class_defs: HashMap, mark_classes: HashMap, - anchor_defs: HashMap, + anchor_defs: HashMap, value_record_defs: HashMap, conditionset_defs: ConditionSetMap, mark_attach_class_id: HashMap, @@ -184,18 +182,22 @@ impl<'a> CompilationCtx<'a> { .as_ref() .map(|raw| raw.build(&mut name_builder)); + // the var store builder is required so that variable metrics/anchors + // in the GPOS table can be collected into an ItemVariationStore + let mut ivs = VariationStoreBuilder::new(); + + let (mut gsub, mut gpos) = self.lookups.build(&self.features, &mut ivs); + if !ivs.is_empty() { + self.tables + .gdef + .get_or_insert_with(Default::default) + .var_store = Some(ivs); + } + let (gdef, key_map) = match self.tables.gdef.as_ref().map(|raw| raw.build()) { Some((gdef, key_map)) => (Some(gdef), key_map), None => (None, None), }; - // all VariationIndex tables (in value records and anchors) currently have - // temporary indices; now that we've built the ItemVariationStore we need - // to go and update them all. - if let Some(key_map) = key_map { - self.lookups.update_variation_index_tables(&key_map); - } - - let (mut gsub, mut gpos) = self.lookups.build(&self.features); let feature_params = self.features.build_feature_params(&mut name_builder); @@ -212,6 +214,12 @@ impl<'a> CompilationCtx<'a> { } } if let Some(gpos) = gpos.as_mut() { + if let Some(key_map) = key_map { + // all VariationIndex tables (in value records and anchors) + // currently have temporary indices; now that we've built the + // ItemVariationStore we need to go and update them all. + gpos.remap_variation_indices(&key_map); + } if let Some(variations) = gpos.feature_variations.as_mut() { sort_feature_variations(variations, |condset| { self.conditionset_defs.sort_order(condset) @@ -1084,15 +1092,15 @@ impl<'a> CompilationCtx<'a> { } if let Some(adv) = record.advance() { - let (adv, var_idx) = self.resolve_metric(&adv); + let adv = self.resolve_metric(&adv); if self.vertical_feature.in_eligible_vertical_feature() { return ValueRecord::new() - .with_y_advance(adv) - .with_device(var_idx, ValueFormat::Y_ADVANCE_DEVICE); + .with_y_advance(adv.default) + .with_y_advance_device(adv.device_or_deltas); } else { return ValueRecord::new() - .with_x_advance(adv) - .with_device(var_idx, ValueFormat::X_ADVANCE_DEVICE); + .with_x_advance(adv.default) + .with_x_advance_device(adv.device_or_deltas); } } @@ -1101,20 +1109,16 @@ impl<'a> CompilationCtx<'a> { return Default::default(); }; - let (x_place, x_place_var) = self.resolve_metric(&x_place); - let (y_place, y_place_var) = self.resolve_metric(&y_place); - let (x_adv, x_adv_var) = self.resolve_metric(&x_adv); - let (y_adv, y_adv_var) = self.resolve_metric(&y_adv); - - let result = ValueRecord::new() - .with_x_placement(x_place) - .with_y_placement(y_place) - .with_x_advance(x_adv) - .with_y_advance(y_adv) - .with_device(x_place_var, ValueFormat::X_PLACEMENT_DEVICE) - .with_device(y_place_var, ValueFormat::Y_PLACEMENT_DEVICE) - .with_device(x_adv_var, ValueFormat::X_ADVANCE_DEVICE) - .with_device(y_adv_var, ValueFormat::Y_ADVANCE_DEVICE); + let x_placement = self.resolve_metric(&x_place); + let y_placement = self.resolve_metric(&y_place); + let x_advance = self.resolve_metric(&x_adv); + let y_advance = self.resolve_metric(&y_adv); + let mut result = ValueRecord { + x_placement: Some(x_placement), + y_placement: Some(y_placement), + x_advance: Some(x_advance), + y_advance: Some(y_advance), + }; if let Some([x_place_dev, y_place_dev, x_adv_dev, y_adv_dev]) = record.device() { // if we have an explicit device we must not also be variable @@ -1127,32 +1131,28 @@ impl<'a> CompilationCtx<'a> { ), "checked during parsing" ); - return result - .with_device(x_place_dev.compile(), ValueFormat::X_PLACEMENT_DEVICE) - .with_device(y_place_dev.compile(), ValueFormat::Y_PLACEMENT_DEVICE) - .with_device(x_adv_dev.compile(), ValueFormat::X_ADVANCE_DEVICE) - .with_device(y_adv_dev.compile(), ValueFormat::Y_ADVANCE_DEVICE); + result.x_advance.as_mut().unwrap().device_or_deltas = x_adv_dev.compile().into(); + result.y_advance.as_mut().unwrap().device_or_deltas = y_adv_dev.compile().into(); + result.x_placement.as_mut().unwrap().device_or_deltas = x_place_dev.compile().into(); + result.y_placement.as_mut().unwrap().device_or_deltas = y_place_dev.compile().into(); } result } - fn resolve_metric(&mut self, metric: &typed::Metric) -> (i16, Option) { + fn resolve_metric(&mut self, metric: &typed::Metric) -> Metric { match metric { - typed::Metric::Scalar(value) => (value.parse_signed(), None), + typed::Metric::Scalar(value) => value.parse_signed().into(), typed::Metric::Variable(variable) => self.resolve_variable_metric(variable), } } - fn resolve_variable_metric( - &mut self, - metric: &typed::VariableMetric, - ) -> (i16, Option) { + fn resolve_variable_metric(&mut self, metric: &typed::VariableMetric) -> Metric { let Some(var_info) = self.variation_info else { self.error( metric.range(), "variable metric only valid when compiling variable font", ); - return (0, None); + return Default::default(); }; let mut locations = HashMap::new(); @@ -1176,14 +1176,13 @@ impl<'a> CompilationCtx<'a> { locations.insert(pos, metric_loc.value().parse_signed()); } match var_info.resolve_variable_metric(&locations) { - Ok((default, deltas)) => { - let temp_idx = self.tables.var_store().add_deltas(deltas); - let var_idx = PendingVariationIndex::new(temp_idx); - (default, Some(var_idx)) - } + Ok((default, deltas)) => Metric { + default, + device_or_deltas: DeviceOrDeltas::Deltas(deltas), + }, Err(e) => { self.error(metric.range(), format!("failed to compute deltas: '{e}'")); - (0, None) + Default::default() } } } @@ -1797,15 +1796,15 @@ impl<'a> CompilationCtx<'a> { let anchor_block = anchor_def.anchor(); let name = anchor_def.name(); let anchor = match self.resolve_anchor(&anchor_block) { - Some(a @ AnchorTable::Format1(_) | a @ AnchorTable::Format2(_)) => a, - Some(_) => { + Some(a) if !a.x.has_device_or_deltas() && !a.y.has_device_or_deltas() => a, + _ => { return self.error( anchor_block.range(), "named anchor definition can only be in format A or B", ) } - None => return, }; + if let Some(_prev) = self .anchor_defs .insert(name.text.clone(), (anchor, anchor_def.range().start)) @@ -1814,7 +1813,7 @@ impl<'a> CompilationCtx<'a> { } } - fn resolve_anchor(&mut self, item: &typed::Anchor) -> Option { + fn resolve_anchor(&mut self, item: &typed::Anchor) -> Option { if item.null().is_some() { return None; } @@ -1834,33 +1833,24 @@ impl<'a> CompilationCtx<'a> { return None; }; - let (x, x_var) = self.resolve_metric(&x); - let (y, y_var) = self.resolve_metric(&y); - - if x_var.is_some() || y_var.is_some() { - return Some(AnchorTable::format_3( - x, - y, - x_var.map(Into::into), - y_var.map(Into::into), - )); - } + let x = self.resolve_metric(&x); + let y = self.resolve_metric(&y); + let mut anchor = Anchor { + x, + y, + contourpoint: None, + }; if let Some(point) = item.contourpoint() { match point.parse_unsigned() { - Some(point) => Some(AnchorTable::format_2(x, y, point)), + Some(point) => anchor.contourpoint = Some(point), None => panic!("negative contourpoint, go fix your parser"), } - } else if let Some((x_coord, y_coord)) = item.devices() { - Some(AnchorTable::format_3( - x, - y, - x_coord.compile().map(Into::into), - y_coord.compile().map(Into::into), - )) - } else { - Some(AnchorTable::format_1(x, y)) + } else if let Some((x_dev, y_dev)) = item.devices() { + anchor.x.device_or_deltas = x_dev.compile().into(); + anchor.y.device_or_deltas = y_dev.compile().into(); } + Some(anchor) } fn define_condition_set(&mut self, node: typed::ConditionSet) { diff --git a/fea-rs/src/compile/feature_writer.rs b/fea-rs/src/compile/feature_writer.rs index 579322bb1..65f2288db 100644 --- a/fea-rs/src/compile/feature_writer.rs +++ b/fea-rs/src/compile/feature_writer.rs @@ -2,13 +2,7 @@ use std::collections::{BTreeMap, HashMap}; -use write_fonts::{ - tables::{ - layout::{LookupFlag, PendingVariationIndex}, - variations::VariationRegion, - }, - types::Tag, -}; +use write_fonts::{tables::layout::LookupFlag, types::Tag}; use crate::GlyphSet; @@ -92,19 +86,6 @@ impl<'a> FeatureBuilder<'a> { next_id } - /// Add a set of deltas to the `ItemVariationStore`. - /// - /// Returns a `PendingVariationIndex` which should be stored whereever a - /// `VariationIndex` table would be expected (it will be remapped during - /// compilation). - pub fn add_deltas>( - &mut self, - deltas: Vec<(VariationRegion, T)>, - ) -> PendingVariationIndex { - let delta_set_id = self.tables.var_store().add_deltas(deltas); - PendingVariationIndex { delta_set_id } - } - /// Add lookups to every default language system. /// /// Convenience method for recurring pattern. diff --git a/fea-rs/src/compile/lookups.rs b/fea-rs/src/compile/lookups.rs index b34b092de..3556544c2 100644 --- a/fea-rs/src/compile/lookups.rs +++ b/fea-rs/src/compile/lookups.rs @@ -14,26 +14,26 @@ use smol_str::SmolStr; use write_fonts::{ tables::{ - gpos::{self as write_gpos, AnchorTable, ValueRecord}, + gpos::{self as write_gpos}, gsub as write_gsub, layout::{ - ConditionSet as RawConditionSet, DeviceOrVariationIndex, Feature, FeatureList, - FeatureRecord, FeatureTableSubstitution, FeatureTableSubstitutionRecord, - FeatureVariationRecord, FeatureVariations, LangSys, LangSysRecord, Lookup as RawLookup, - LookupFlag, LookupList, Script, ScriptList, ScriptRecord, + ConditionSet as RawConditionSet, Feature, FeatureList, FeatureRecord, + FeatureTableSubstitution, FeatureTableSubstitutionRecord, FeatureVariationRecord, + FeatureVariations, LangSys, LangSysRecord, Lookup as RawLookup, LookupFlag, LookupList, + Script, ScriptList, ScriptRecord, }, - variations::ivs_builder::VariationIndexRemapping, + variations::ivs_builder::VariationStoreBuilder, }, types::Tag, }; use crate::{ common::{GlyphId, GlyphOrClass, GlyphSet}, - compile::lookups::contextual::ChainOrNot, + compile::{lookups::contextual::ChainOrNot, metrics::ValueRecord}, Kind, }; -use super::{features::AllFeatures, tables::ClassId, tags}; +use super::{features::AllFeatures, metrics::Anchor, tables::ClassId, tags}; use contextual::{ ContextualLookupBuilder, PosChainContextBuilder, PosContextBuilder, ReverseChainBuilder, @@ -49,9 +49,19 @@ use gsub_builders::{ }; pub(crate) use helpers::ClassDefBuilder2; -pub trait Builder { +// A simple trait for building lookups +// This exists because we use it to implement `LookupBuilder` +pub(crate) trait Builder { type Output; - fn build(self) -> Self::Output; + // the var_store is only used in GPOS, but we pass it everywhere. + // This is annoying but feels like the lesser of two evils. It's easy to + // ignore this argument where it isn't used, and this makes the logic + // in LookupBuilder simpler, since it is identical for GPOS/GSUB. + // + // It would be nice if this could then be Option<&mut T>, but that type is + // annoying to work with, as Option<&mut _> doesn't impl Copy, so you need + // to do a dance anytime you use it. + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output; } pub(crate) type FilterSetId = u16; @@ -183,14 +193,6 @@ pub(crate) struct PosSubBuilder { variations: HashMap>>, } -/// A trait for position lookups which might contain VariationIndex tables -/// -/// These tables contain indicies that are not known until compilaiton time, -/// so we need to go and update them all then. -pub(crate) trait VariationIndexContainingLookup { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping); -} - impl LookupBuilder { fn new(flags: LookupFlag, mark_set: Option) -> Self { LookupBuilder { @@ -226,14 +228,6 @@ impl LookupBuilder { } } -impl LookupBuilder { - fn update_variation_index_tables(&mut self, key_map: &VariationIndexRemapping) { - for table in &mut self.subtables { - table.remap_variation_indices(key_map); - } - } -} - impl LookupBuilder { /// A helper method for converting from (say) ContextBuilder to PosContextBuilder fn convert>(self) -> LookupBuilder { @@ -263,18 +257,6 @@ impl PositionLookup { PositionLookup::ChainedContextual(lookup) => lookup.force_subtable_break(), } } - - fn update_variation_index_tables(&mut self, key_map: &VariationIndexRemapping) { - match self { - PositionLookup::Single(lookup) => lookup.update_variation_index_tables(key_map), - PositionLookup::Pair(lookup) => lookup.update_variation_index_tables(key_map), - PositionLookup::Cursive(lookup) => lookup.update_variation_index_tables(key_map), - PositionLookup::MarkToBase(lookup) => lookup.update_variation_index_tables(key_map), - PositionLookup::MarkToLig(lookup) => lookup.update_variation_index_tables(key_map), - PositionLookup::MarkToMark(lookup) => lookup.update_variation_index_tables(key_map), - PositionLookup::Contextual(_) | PositionLookup::ChainedContextual(_) => (), - } - } } impl SubstitutionLookup { @@ -298,11 +280,11 @@ where { type Output = RawLookup; - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { let subtables = self .subtables .into_iter() - .flat_map(|b| b.build().into_iter()) + .flat_map(|b| b.build(var_store).into_iter()) .collect(); RawLookup::new(self.flags, subtables, self.mark_set.unwrap_or_default()) } @@ -311,25 +293,31 @@ where impl Builder for PositionLookup { type Output = write_gpos::PositionLookup; - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { match self { - PositionLookup::Single(lookup) => write_gpos::PositionLookup::Single(lookup.build()), - PositionLookup::Pair(lookup) => write_gpos::PositionLookup::Pair(lookup.build()), - PositionLookup::Cursive(lookup) => write_gpos::PositionLookup::Cursive(lookup.build()), + PositionLookup::Single(lookup) => { + write_gpos::PositionLookup::Single(lookup.build(var_store)) + } + PositionLookup::Pair(lookup) => { + write_gpos::PositionLookup::Pair(lookup.build(var_store)) + } + PositionLookup::Cursive(lookup) => { + write_gpos::PositionLookup::Cursive(lookup.build(var_store)) + } PositionLookup::MarkToBase(lookup) => { - write_gpos::PositionLookup::MarkToBase(lookup.build()) + write_gpos::PositionLookup::MarkToBase(lookup.build(var_store)) } PositionLookup::MarkToLig(lookup) => { - write_gpos::PositionLookup::MarkToLig(lookup.build()) + write_gpos::PositionLookup::MarkToLig(lookup.build(var_store)) } PositionLookup::MarkToMark(lookup) => { - write_gpos::PositionLookup::MarkToMark(lookup.build()) + write_gpos::PositionLookup::MarkToMark(lookup.build(var_store)) } PositionLookup::Contextual(lookup) => { - write_gpos::PositionLookup::Contextual(lookup.build().into_concrete()) + write_gpos::PositionLookup::Contextual(lookup.build(var_store).into_concrete()) } PositionLookup::ChainedContextual(lookup) => { - write_gpos::PositionLookup::ChainContextual(lookup.build().into_concrete()) + write_gpos::PositionLookup::ChainContextual(lookup.build(var_store).into_concrete()) } } } @@ -338,28 +326,30 @@ impl Builder for PositionLookup { impl Builder for SubstitutionLookup { type Output = write_gsub::SubstitutionLookup; - fn build(self) -> Self::Output { + fn build(self, _var_store: &mut VariationStoreBuilder) -> Self::Output { match self { SubstitutionLookup::Single(lookup) => { - write_gsub::SubstitutionLookup::Single(lookup.build()) + write_gsub::SubstitutionLookup::Single(lookup.build(_var_store)) } SubstitutionLookup::Multiple(lookup) => { - write_gsub::SubstitutionLookup::Multiple(lookup.build()) + write_gsub::SubstitutionLookup::Multiple(lookup.build(_var_store)) } SubstitutionLookup::Alternate(lookup) => { - write_gsub::SubstitutionLookup::Alternate(lookup.build()) + write_gsub::SubstitutionLookup::Alternate(lookup.build(_var_store)) } SubstitutionLookup::Ligature(lookup) => { - write_gsub::SubstitutionLookup::Ligature(lookup.build()) + write_gsub::SubstitutionLookup::Ligature(lookup.build(_var_store)) } SubstitutionLookup::Contextual(lookup) => { - write_gsub::SubstitutionLookup::Contextual(lookup.build().into_concrete()) + write_gsub::SubstitutionLookup::Contextual(lookup.build(_var_store).into_concrete()) } SubstitutionLookup::ChainedContextual(lookup) => { - write_gsub::SubstitutionLookup::ChainContextual(lookup.build().into_concrete()) + write_gsub::SubstitutionLookup::ChainContextual( + lookup.build(_var_store).into_concrete(), + ) } SubstitutionLookup::Reverse(lookup) => { - write_gsub::SubstitutionLookup::Reverse(lookup.build()) + write_gsub::SubstitutionLookup::Reverse(lookup.build(_var_store)) } } } @@ -630,12 +620,6 @@ impl AllLookups { lookup_ids } - pub(crate) fn update_variation_index_tables(&mut self, key_map: &VariationIndexRemapping) { - for lookup in self.gpos.iter_mut() { - lookup.update_variation_index_tables(key_map); - } - } - /// Returns a map that must be used to remap the ids in any features where /// they were used. pub(crate) fn merge_external_lookups( @@ -653,6 +637,7 @@ impl AllLookups { pub(crate) fn build( &self, features: &AllFeatures, + var_store: &mut VariationStoreBuilder, ) -> (Option, Option) { let mut gpos_builder = PosSubBuilder::new(self.gpos.clone()); let mut gsub_builder = PosSubBuilder::new(self.gsub.clone()); @@ -702,7 +687,7 @@ impl AllLookups { } } - (gsub_builder.build(), gpos_builder.build()) + (gsub_builder.build(var_store), gpos_builder.build(var_store)) } } @@ -869,8 +854,8 @@ impl SomeLookup { pub(crate) fn add_gpos_type_3( &mut self, id: GlyphId, - entry: Option, - exit: Option, + entry: Option, + exit: Option, ) { if let SomeLookup::GposLookup(PositionLookup::Cursive(table)) = self { let subtable = table.last_mut().unwrap(); @@ -1050,6 +1035,7 @@ where #[allow(clippy::type_complexity)] // i love my big dumb tuple fn build_raw( self, + var_store: &mut VariationStoreBuilder, ) -> Option<( LookupList, ScriptList, @@ -1084,7 +1070,11 @@ where }) .collect::>(); - let lookups = self.lookups.into_iter().map(|x| x.build()).collect(); + let lookups = self + .lookups + .into_iter() + .map(|x| x.build(var_store)) + .collect(); let variations = if self.variations.is_empty() { None @@ -1126,8 +1116,8 @@ where impl Builder for PosSubBuilder { type Output = Option; - fn build(self) -> Self::Output { - self.build_raw() + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { + self.build_raw(var_store) .map(|(lookups, scripts, features, variations)| { let mut gpos = write_gpos::Gpos::new(scripts, features, lookups); gpos.feature_variations = variations.into(); @@ -1139,8 +1129,8 @@ impl Builder for PosSubBuilder { impl Builder for PosSubBuilder { type Output = Option; - fn build(self) -> Self::Output { - self.build_raw() + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { + self.build_raw(var_store) .map(|(lookups, scripts, features, variations)| { let mut gsub = write_gsub::Gsub::new(scripts, features, lookups); gsub.feature_variations = variations.into(); @@ -1163,43 +1153,6 @@ impl FeatureKey { } } -impl VariationIndexContainingLookup for ValueRecord { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - for table in [ - self.x_placement_device.as_mut(), - self.y_placement_device.as_mut(), - self.x_advance_device.as_mut(), - self.y_advance_device.as_mut(), - ] - .into_iter() - .flatten() - { - table.remap_variation_indices(key_map) - } - } -} - -impl VariationIndexContainingLookup for DeviceOrVariationIndex { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - if let DeviceOrVariationIndex::PendingVariationIndex(table) = self { - *self = key_map.get(table.delta_set_id).unwrap().into(); - } - } -} - -impl VariationIndexContainingLookup for AnchorTable { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - if let AnchorTable::Format3(table) = self { - table - .x_device - .as_mut() - .into_iter() - .chain(table.y_device.as_mut()) - .for_each(|x| x.remap_variation_indices(key_map)) - } - } -} - fn is_gpos_rule(kind: Kind) -> bool { matches!( kind, diff --git a/fea-rs/src/compile/lookups/contextual.rs b/fea-rs/src/compile/lookups/contextual.rs index 743c744fe..d8e8b7b0a 100644 --- a/fea-rs/src/compile/lookups/contextual.rs +++ b/fea-rs/src/compile/lookups/contextual.rs @@ -7,17 +7,17 @@ use std::{ use write_fonts::{ tables::{ - gpos::ValueRecord, gsub as write_gsub, gsub::ReverseChainSingleSubstFormat1, layout::{self as write_layout, CoverageTableBuilder, LookupFlag}, + variations::ivs_builder::VariationStoreBuilder, }, types::GlyphId, validate::Validate, FontWrite, }; -use crate::common::GlyphOrClass; +use crate::{common::GlyphOrClass, compile::metrics::ValueRecord}; use super::{ Builder, ClassDefBuilder2, FilterSetId, LookupBuilder, LookupId, PositionLookup, @@ -440,21 +440,25 @@ impl ContextRule { impl Builder for PosContextBuilder { type Output = Vec; - fn build(self) -> Self::Output { - self.0.build(true) + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { + self.0.build(Some(var_store)) } } impl Builder for SubContextBuilder { type Output = Vec; - fn build(self) -> Self::Output { - self.0.build(false) + fn build(self, _var_store: &mut VariationStoreBuilder) -> Self::Output { + self.0.build(None) } } impl ContextBuilder { - fn build(self, in_gpos: bool) -> Vec { + fn build( + self, + var_store: Option<&mut VariationStoreBuilder>, + ) -> Vec { + let in_gpos = var_store.is_some(); assert!(self.rules.iter().all(|rule| !rule.is_chain_rule())); let format_1 = self.build_format_1(in_gpos); //NOTE: I'm skipping format_2 because it seems consistently larger @@ -674,7 +678,7 @@ impl SubChainContextBuilder { impl Builder for PosChainContextBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, _: &mut VariationStoreBuilder) -> Self::Output { self.0.build(true) } } @@ -682,7 +686,7 @@ impl Builder for PosChainContextBuilder { impl Builder for SubChainContextBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, _: &mut VariationStoreBuilder) -> Self::Output { self.0.build(false) } } @@ -733,7 +737,7 @@ impl ReverseChainBuilder { impl Builder for ReverseChainBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, _: &mut VariationStoreBuilder) -> Self::Output { self.rules .into_iter() .map(|rule| { diff --git a/fea-rs/src/compile/lookups/gpos_builders.rs b/fea-rs/src/compile/lookups/gpos_builders.rs index 36347d96f..814820f2d 100644 --- a/fea-rs/src/compile/lookups/gpos_builders.rs +++ b/fea-rs/src/compile/lookups/gpos_builders.rs @@ -5,16 +5,19 @@ use std::collections::{BTreeMap, HashMap}; use smol_str::SmolStr; use write_fonts::{ tables::{ - gpos::{self as write_gpos, AnchorTable, MarkRecord, ValueFormat, ValueRecord}, + gpos::{self as write_gpos, MarkRecord, ValueFormat, ValueRecord as RawValueRecord}, layout::CoverageTable, - variations::ivs_builder::VariationIndexRemapping, + variations::ivs_builder::VariationStoreBuilder, }, types::GlyphId, }; -use crate::common::GlyphSet; +use crate::{ + common::GlyphSet, + compile::metrics::{Anchor, ValueRecord}, +}; -use super::{Builder, ClassDefBuilder2, VariationIndexContainingLookup}; +use super::{Builder, ClassDefBuilder2}; #[derive(Clone, Debug, Default)] pub struct SinglePosBuilder { @@ -35,19 +38,11 @@ impl SinglePosBuilder { } } -impl VariationIndexContainingLookup for SinglePosBuilder { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - self.items - .values_mut() - .for_each(|x| x.remap_variation_indices(key_map)) - } -} - impl Builder for SinglePosBuilder { type Output = Vec; - fn build(self) -> Self::Output { - fn build_subtable(items: BTreeMap) -> write_gpos::SinglePos { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { + fn build_subtable(items: BTreeMap) -> write_gpos::SinglePos { let first = *items.values().next().unwrap(); let use_format_1 = first.format().is_empty() || items.values().all(|val| val == &first); let coverage: CoverageTable = items.keys().copied().collect(); @@ -58,21 +53,26 @@ impl Builder for SinglePosBuilder { } } const NEW_SUBTABLE_COST: usize = 10; + let items = self + .items + .into_iter() + .map(|(glyph, anchor)| (glyph, anchor.build(var_store))) + .collect::>(); // list of sets of glyph ids which will end up in their own subtables let mut subtables = Vec::new(); - let mut group_by_record: HashMap<&ValueRecord, BTreeMap> = + let mut group_by_record: HashMap<&RawValueRecord, BTreeMap> = Default::default(); // first group by specific record; glyphs that share a record can use // the more efficient format-1 subtable type - for (gid, value) in &self.items { + for (gid, value) in &items { group_by_record .entry(value) .or_default() .insert(*gid, value); } - let mut group_by_format: HashMap> = + let mut group_by_format: HashMap> = Default::default(); for (value, glyphs) in group_by_record { // if this saves us size, use format 1 @@ -144,23 +144,6 @@ impl Default for ClassPairPosSubtable { #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] struct ClassPairPosBuilder(BTreeMap<(ValueFormat, ValueFormat), Vec>); -impl VariationIndexContainingLookup for PairPosBuilder { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - self.pairs - .0 - .values_mut() - .flat_map(|x| x.values_mut()) - .chain(self.classes.0.values_mut().flat_map(|x| { - x.iter_mut() - .flat_map(|x| x.items.values_mut().flat_map(|x| x.values_mut())) - })) - .for_each(|v| { - v.0.remap_variation_indices(key_map); - v.1.remap_variation_indices(key_map); - }); - } -} - impl ClassPairPosBuilder { fn insert( &mut self, @@ -249,9 +232,9 @@ impl PairPosBuilder { impl Builder for PairPosBuilder { type Output = Vec; - fn build(self) -> Self::Output { - let mut out = self.pairs.build(); - out.extend(self.classes.build()); + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { + let mut out = self.pairs.build(var_store); + out.extend(self.classes.build(var_store)); out } } @@ -259,7 +242,7 @@ impl Builder for PairPosBuilder { impl Builder for GlyphPairPosBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { let mut split_by_format = BTreeMap::<_, BTreeMap<_, Vec<_>>>::default(); for (g1, map) in self.0 { for (g2, (v1, v2)) in map { @@ -268,7 +251,11 @@ impl Builder for GlyphPairPosBuilder { .or_default() .entry(g1) .or_default() - .push(write_gpos::PairValueRecord::new(g2, v1, v2)); + .push(write_gpos::PairValueRecord::new( + g2, + v1.build(var_store), + v2.build(var_store), + )); } } @@ -286,18 +273,20 @@ impl Builder for GlyphPairPosBuilder { impl Builder for ClassPairPosBuilder { type Output = Vec; - fn build(self) -> Self::Output { - self.0 - .into_values() - .flat_map(|subs| subs.into_iter().map(Builder::build)) - .collect() + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { + // not an iterator because we have funky lifetime issues + let mut result = Vec::new(); + for sub in self.0.into_values().flat_map(|subs| subs.into_iter()) { + result.push(sub.build(var_store)); + } + result } } impl Builder for ClassPairPosSubtable { type Output = write_gpos::PairPos; - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { assert!(!self.items.is_empty(), "filter before here"); // we have a set of classes/values with a single valueformat @@ -309,8 +298,8 @@ impl Builder for ClassPairPosSubtable { .and_then(|val| val.values().next()) .map(|(v1, v2)| { write_gpos::Class2Record::new( - ValueRecord::new().with_explicit_value_format(v1.format()), - ValueRecord::new().with_explicit_value_format(v2.format()), + RawValueRecord::new().with_explicit_value_format(v1.format()), + RawValueRecord::new().with_explicit_value_format(v2.format()), ) }) .unwrap(); @@ -326,7 +315,8 @@ impl Builder for ClassPairPosSubtable { let mut records = vec![empty_record.clone(); class2map.len() + 1]; for (class, (v1, v2)) in stuff { let idx = class2map.get(&class).unwrap(); - records[*idx as usize] = write_gpos::Class2Record::new(v1.clone(), v2.clone()); + records[*idx as usize] = + write_gpos::Class2Record::new(v1.build(var_store), v2.build(var_store)); } out[*idx as usize] = write_gpos::Class1Record::new(records); } @@ -336,42 +326,31 @@ impl Builder for ClassPairPosSubtable { #[derive(Clone, Debug, Default)] pub struct CursivePosBuilder { - items: BTreeMap, -} - -impl VariationIndexContainingLookup for CursivePosBuilder { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - self.items - .values_mut() - .flat_map(|record| { - record - .entry_anchor - .as_mut() - .into_iter() - .chain(record.exit_anchor.as_mut()) - }) - .for_each(|anchor| anchor.remap_variation_indices(key_map)) - } + // (entry, exit) + items: BTreeMap, Option)>, } impl CursivePosBuilder { - pub fn insert( - &mut self, - glyph: GlyphId, - entry: Option, - exit: Option, - ) { - let record = write_gpos::EntryExitRecord::new(entry, exit); - self.items.insert(glyph, record); + pub fn insert(&mut self, glyph: GlyphId, entry: Option, exit: Option) { + self.items.insert(glyph, (entry, exit)); } } impl Builder for CursivePosBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { let coverage = self.items.keys().copied().collect(); - let records = self.items.into_values().collect(); + let records = self + .items + .into_values() + .map(|(entry, exit)| { + write_gpos::EntryExitRecord::new( + entry.map(|x| x.build(var_store)), + exit.map(|x| x.build(var_store)), + ) + }) + .collect(); vec![write_gpos::CursivePosFormat1::new(coverage, records)] } } @@ -379,19 +358,12 @@ impl Builder for CursivePosBuilder { // shared between several tables #[derive(Clone, Debug, Default)] struct MarkList { - glyphs: BTreeMap, + // (class id, anchor) + glyphs: BTreeMap, // map class names to their idx for this table classes: HashMap, } -impl VariationIndexContainingLookup for MarkList { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - self.glyphs - .values_mut() - .for_each(|mark| mark.mark_anchor.remap_variation_indices(key_map)) - } -} - impl MarkList { /// If this glyph is already part of another class, return the previous class name /// @@ -400,19 +372,20 @@ impl MarkList { &mut self, glyph: GlyphId, class: SmolStr, - anchor: AnchorTable, + anchor: Anchor, ) -> Result { let next_id = self.classes.len().try_into().unwrap(); let id = *self.classes.entry(class).or_insert(next_id); if let Some(prev) = self .glyphs - .insert(glyph, MarkRecord::new(id, anchor)) - .filter(|prev| prev.mark_class != id) + //.insert(glyph, MarkRecord::new(id, anchor)) + .insert(glyph, (id, anchor)) + .filter(|prev| prev.0 != id) { let class = self .classes .iter() - .find_map(|(name, idx)| (*idx == prev.mark_class).then(|| name.clone())) + .find_map(|(name, idx)| (*idx == prev.0).then(|| name.clone())) .unwrap(); return Err(PreviouslyAssignedClass { @@ -438,9 +411,14 @@ impl MarkList { impl Builder for MarkList { type Output = (CoverageTable, write_gpos::MarkArray); - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { let coverage = self.glyphs().collect(); - let array = write_gpos::MarkArray::new(self.glyphs.into_values().collect()); + let array = write_gpos::MarkArray::new( + self.glyphs + .into_values() + .map(|(class, anchor)| MarkRecord::new(class, anchor.build(var_store))) + .collect(), + ); (coverage, array) } } @@ -449,17 +427,7 @@ impl Builder for MarkList { #[derive(Clone, Debug, Default)] pub struct MarkToBaseBuilder { marks: MarkList, - bases: BTreeMap>, -} - -impl VariationIndexContainingLookup for MarkToBaseBuilder { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - self.marks.remap_variation_indices(key_map); - self.bases - .values_mut() - .flat_map(|vals| vals.iter_mut().map(|(_, anchor)| anchor)) - .for_each(|anchor| anchor.remap_variation_indices(key_map)) - } + bases: BTreeMap>, } /// An error indicating a given glyph has been assigned to multiple mark classes @@ -480,13 +448,13 @@ impl MarkToBaseBuilder { &mut self, glyph: GlyphId, class: SmolStr, - anchor: AnchorTable, + anchor: Anchor, ) -> Result { self.marks.insert(glyph, class, anchor) } /// Insert a new base glyph. - pub fn insert_base(&mut self, glyph: GlyphId, class: &SmolStr, anchor: AnchorTable) { + pub fn insert_base(&mut self, glyph: GlyphId, class: &SmolStr, anchor: Anchor) { let class = self.marks.get_class(class); self.bases.entry(glyph).or_default().push((class, anchor)) } @@ -505,18 +473,18 @@ impl MarkToBaseBuilder { impl Builder for MarkToBaseBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { let MarkToBaseBuilder { marks, bases } = self; let n_classes = marks.classes.len(); - let (mark_coverage, mark_array) = marks.build(); + let (mark_coverage, mark_array) = marks.build(var_store); let base_coverage = bases.keys().copied().collect(); let base_records = bases .into_values() .map(|anchors| { let mut anchor_offsets = vec![None; n_classes]; for (class, anchor) in anchors { - anchor_offsets[class as usize] = Some(anchor); + anchor_offsets[class as usize] = Some(anchor.build(var_store)); } write_gpos::BaseRecord::new(anchor_offsets) }) @@ -534,7 +502,7 @@ impl Builder for MarkToBaseBuilder { #[derive(Clone, Debug, Default)] pub struct MarkToLigBuilder { marks: MarkList, - ligatures: BTreeMap>>, + ligatures: BTreeMap>>, } impl MarkToLigBuilder { @@ -542,12 +510,12 @@ impl MarkToLigBuilder { &mut self, glyph: GlyphId, class: SmolStr, - anchor: AnchorTable, + anchor: Anchor, ) -> Result { self.marks.insert(glyph, class, anchor) } - pub fn add_lig(&mut self, glyph: GlyphId, components: Vec>) { + pub fn add_lig(&mut self, glyph: GlyphId, components: Vec>) { self.ligatures.insert(glyph, components); } @@ -560,20 +528,10 @@ impl MarkToLigBuilder { } } -impl VariationIndexContainingLookup for MarkToLigBuilder { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - self.marks.remap_variation_indices(key_map); - self.ligatures - .values_mut() - .flat_map(|vals| vals.iter_mut().flat_map(|tree| tree.values_mut())) - .for_each(|anchor| anchor.remap_variation_indices(key_map)) - } -} - impl Builder for MarkToLigBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { let MarkToLigBuilder { marks, ligatures } = self; let n_classes = marks.classes.len(); @@ -591,7 +549,7 @@ impl Builder for MarkToLigBuilder { let mut anchor_offsets = vec![None; n_classes]; for (class, anchor) in anchors { let class_idx = marks.get_class(&class); - anchor_offsets[class_idx as usize] = Some(anchor); + anchor_offsets[class_idx as usize] = Some(anchor.build(var_store)); } write_gpos::ComponentRecord::new(anchor_offsets) }) @@ -600,7 +558,7 @@ impl Builder for MarkToLigBuilder { }) .collect(); let ligature_array = write_gpos::LigatureArray::new(ligature_array); - let (mark_coverage, mark_array) = marks.build(); + let (mark_coverage, mark_array) = marks.build(var_store); vec![write_gpos::MarkLigPosFormat1::new( mark_coverage, ligature_coverage, @@ -614,7 +572,7 @@ impl Builder for MarkToLigBuilder { #[derive(Clone, Debug, Default)] pub struct MarkToMarkBuilder { attaching_marks: MarkList, - base_marks: BTreeMap>, + base_marks: BTreeMap>, } impl MarkToMarkBuilder { @@ -626,13 +584,13 @@ impl MarkToMarkBuilder { &mut self, glyph: GlyphId, class: SmolStr, - anchor: AnchorTable, + anchor: Anchor, ) -> Result { self.attaching_marks.insert(glyph, class, anchor) } /// Insert a new mark2 (base) glyph - pub fn insert_mark2(&mut self, glyph: GlyphId, class: &SmolStr, anchor: AnchorTable) { + pub fn insert_mark2(&mut self, glyph: GlyphId, class: &SmolStr, anchor: Anchor) { let id = self.attaching_marks.get_class(class); self.base_marks.entry(glyph).or_default().push((id, anchor)) } @@ -648,34 +606,24 @@ impl MarkToMarkBuilder { } } -impl VariationIndexContainingLookup for MarkToMarkBuilder { - fn remap_variation_indices(&mut self, key_map: &VariationIndexRemapping) { - self.attaching_marks.remap_variation_indices(key_map); - self.base_marks - .values_mut() - .flat_map(|vals| vals.iter_mut().map(|(_, anchor)| anchor)) - .for_each(|anchor| anchor.remap_variation_indices(key_map)) - } -} - impl Builder for MarkToMarkBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, var_store: &mut VariationStoreBuilder) -> Self::Output { let MarkToMarkBuilder { attaching_marks, base_marks, } = self; let n_classes = attaching_marks.classes.len(); - let (mark_coverage, mark_array) = attaching_marks.build(); + let (mark_coverage, mark_array) = attaching_marks.build(var_store); let mark2_coverage = base_marks.keys().copied().collect(); let mark2_records = base_marks .into_values() .map(|anchors| { let mut anchor_offsets = vec![None; n_classes]; for (class, anchor) in anchors { - anchor_offsets[class as usize] = Some(anchor); + anchor_offsets[class as usize] = Some(anchor.build(var_store)); } write_gpos::Mark2Record::new(anchor_offsets) }) diff --git a/fea-rs/src/compile/lookups/gsub_builders.rs b/fea-rs/src/compile/lookups/gsub_builders.rs index 853b1b8d7..3fd839c53 100644 --- a/fea-rs/src/compile/lookups/gsub_builders.rs +++ b/fea-rs/src/compile/lookups/gsub_builders.rs @@ -3,7 +3,7 @@ use std::{collections::BTreeMap, convert::TryFrom}; use write_fonts::{ - tables::gsub as write_gsub, + tables::{gsub as write_gsub, variations::ivs_builder::VariationStoreBuilder}, types::{FixedSize, GlyphId}, }; @@ -59,7 +59,7 @@ impl SingleSubBuilder { impl Builder for SingleSubBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, _: &mut VariationStoreBuilder) -> Self::Output { const COST_OF_EXTRA_SUB1F1_SUBTABLE: usize = 2 + // extra offset 2 + 2 + 2 + // format1 table itself 2 + 2; // extra coverage table @@ -147,7 +147,7 @@ pub struct MultipleSubBuilder { impl Builder for MultipleSubBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, _: &mut VariationStoreBuilder) -> Self::Output { let coverage = self.items.keys().copied().collect(); let seq_tables = self .items @@ -193,7 +193,7 @@ impl AlternateSubBuilder { impl Builder for AlternateSubBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, _: &mut VariationStoreBuilder) -> Self::Output { let coverage = self.items.keys().copied().collect(); let seq_tables = self .items @@ -230,7 +230,7 @@ impl LigatureSubBuilder { impl Builder for LigatureSubBuilder { type Output = Vec; - fn build(self) -> Self::Output { + fn build(self, _: &mut VariationStoreBuilder) -> Self::Output { let coverage = self.items.keys().copied().collect(); let lig_sets = self .items diff --git a/fea-rs/src/compile/metrics.rs b/fea-rs/src/compile/metrics.rs new file mode 100644 index 000000000..5f79c8874 --- /dev/null +++ b/fea-rs/src/compile/metrics.rs @@ -0,0 +1,342 @@ +//! Variable-first metrics, ValueRecords & Anchors + +use write_fonts::tables::{ + gpos::{AnchorTable, ValueFormat}, + layout::{Device, DeviceOrVariationIndex, PendingVariationIndex}, + variations::{ivs_builder::VariationStoreBuilder, VariationRegion}, +}; + +/// A value record, possibly containing raw deltas or device tables +#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct ValueRecord { + /// The x advance, plus a possible device table or set of deltas + pub(crate) x_advance: Option, + /// The y advance, plus a possible device table or set of deltas + pub(crate) y_advance: Option, + /// The x placement, plus a possible device table or set of deltas + pub(crate) x_placement: Option, + /// The y placement, plus a possible device table or set of deltas + pub(crate) y_placement: Option, +} + +/// An Anchor table, possibly containing deltas or a devices +#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct Anchor { + /// The x coordinate, plus a possible device table or set of deltas + pub(crate) x: Metric, + /// The y coordinate, plus a possible device table or set of deltas + pub(crate) y: Metric, + /// The countourpoint, in a format 2 anchor. + /// + /// This is a rarely used format. + pub(crate) contourpoint: Option, +} + +/// Either a `Device` table or a set of deltas +#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[allow(missing_docs)] +pub enum DeviceOrDeltas { + Device(Device), + Deltas(Vec<(VariationRegion, i16)>), + #[default] + None, +} + +/// A metric with optional device or variation information +#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub(crate) struct Metric { + /// The value at the default location + pub default: i16, + /// An optional device table or delta set + pub device_or_deltas: DeviceOrDeltas, +} + +impl ValueRecord { + /// Create a new all-zeros ValueRecord + pub fn new() -> Self { + Default::default() + } + + // these methods just match the existing builder methods on `ValueRecord` + /// Builder style method to set the default x_placement value + pub fn with_x_placement(mut self, val: i16) -> Self { + self.x_placement + .get_or_insert_with(Default::default) + .default = val; + self + } + + /// Builder style method to set the default y_placement value + pub fn with_y_placement(mut self, val: i16) -> Self { + self.y_placement + .get_or_insert_with(Default::default) + .default = val; + self + } + + /// Builder style method to set the default x_placement value + pub fn with_x_advance(mut self, val: i16) -> Self { + self.x_advance.get_or_insert_with(Default::default).default = val; + self + } + + /// Builder style method to set the default y_placement value + pub fn with_y_advance(mut self, val: i16) -> Self { + self.y_advance.get_or_insert_with(Default::default).default = val; + self + } + + /// Builder style method to set the device or deltas for x_placement + /// + /// The argument can be a `Device` table or a `Vec<(VariationRegion, i16)>` + pub fn with_x_placement_device(mut self, val: impl Into) -> Self { + self.x_placement + .get_or_insert_with(Default::default) + .device_or_deltas = val.into(); + self + } + + /// Builder style method to set the device or deltas for y_placement + /// + /// The argument can be a `Device` table or a `Vec<(VariationRegion, i16)>` + pub fn with_y_placement_device(mut self, val: impl Into) -> Self { + self.y_placement + .get_or_insert_with(Default::default) + .device_or_deltas = val.into(); + self + } + + /// Builder style method to set the device or deltas for x_advance + /// + /// The argument can be a `Device` table or a `Vec<(VariationRegion, i16)>` + pub fn with_x_advance_device(mut self, val: impl Into) -> Self { + self.x_advance + .get_or_insert_with(Default::default) + .device_or_deltas = val.into(); + self + } + + /// Builder style method to set the device or deltas for y_advance + /// + /// The argument can be a `Device` table or a `Vec<(VariationRegion, i16)>` + pub fn with_y_advance_device(mut self, val: impl Into) -> Self { + self.y_advance + .get_or_insert_with(Default::default) + .device_or_deltas = val.into(); + self + } + + pub(crate) fn clear_zeros(mut self) -> Self { + self.x_advance = self.x_advance.filter(|m| !m.is_zero()); + self.y_advance = self.y_advance.filter(|m| !m.is_zero()); + self.x_placement = self.x_placement.filter(|m| !m.is_zero()); + self.y_placement = self.y_placement.filter(|m| !m.is_zero()); + self + } + + pub(crate) fn format(&self) -> ValueFormat { + const EMPTY: ValueFormat = ValueFormat::empty(); + use ValueFormat as VF; + + let get_flags = |field: &Option, def_flag, dev_flag| { + let field = field.as_ref(); + let def_flag = if field.is_some() { def_flag } else { EMPTY }; + let dev_flag = field + .and_then(|fld| (!fld.device_or_deltas.is_none()).then_some(dev_flag)) + .unwrap_or(EMPTY); + (def_flag, dev_flag) + }; + + let (x_adv, x_adv_dev) = get_flags(&self.x_advance, VF::X_ADVANCE, VF::X_ADVANCE_DEVICE); + let (y_adv, y_adv_dev) = get_flags(&self.y_advance, VF::Y_ADVANCE, VF::Y_ADVANCE_DEVICE); + let (x_place, x_place_dev) = + get_flags(&self.x_placement, VF::X_PLACEMENT, VF::X_PLACEMENT_DEVICE); + let (y_place, y_place_dev) = + get_flags(&self.y_placement, VF::Y_PLACEMENT, VF::Y_PLACEMENT_DEVICE); + x_adv | y_adv | x_place | y_place | x_adv_dev | y_adv_dev | x_place_dev | y_place_dev + } + + /// `true` if we are not null, but our set values are all 0 + fn is_all_zeros(&self) -> bool { + let device_mask = ValueFormat::X_PLACEMENT_DEVICE + | ValueFormat::Y_PLACEMENT_DEVICE + | ValueFormat::X_ADVANCE_DEVICE + | ValueFormat::Y_ADVANCE_DEVICE; + + let format = self.format(); + if format.is_empty() || format.intersects(device_mask) { + return false; + } + let all_values = [ + &self.x_placement, + &self.y_placement, + &self.x_advance, + &self.y_advance, + ]; + all_values + .iter() + .all(|v| v.as_ref().map(|v| v.is_zero()).unwrap_or(true)) + } + + // Modify this value record for the special requirements of pairpos lookups + // + // In pair pos tables, if a value record is all zeros (but not null) then + // we interpret it as a having a single zero advance in the x/y direction, + // depending on context. + pub(crate) fn for_pair_pos(self, in_vert_feature: bool) -> Self { + if !self.is_all_zeros() { + return self.clear_zeros(); + } + let mut out = self.clear_zeros(); + if in_vert_feature { + out.y_advance = Some(0.into()); + } else { + out.x_advance = Some(0.into()); + } + out + } + + pub(crate) fn build( + self, + var_store: &mut VariationStoreBuilder, + ) -> write_fonts::tables::gpos::ValueRecord { + let mut result = write_fonts::tables::gpos::ValueRecord::new(); + result.x_advance = self.x_advance.as_ref().map(|val| val.default); + result.y_advance = self.y_advance.as_ref().map(|val| val.default); + result.x_placement = self.x_placement.as_ref().map(|val| val.default); + result.y_placement = self.y_placement.as_ref().map(|val| val.default); + result.x_advance_device = self + .x_advance + .and_then(|val| val.device_or_deltas.build(var_store)) + .into(); + result.y_advance_device = self + .y_advance + .and_then(|val| val.device_or_deltas.build(var_store)) + .into(); + result.x_placement_device = self + .x_placement + .and_then(|val| val.device_or_deltas.build(var_store)) + .into(); + result.y_placement_device = self + .y_placement + .and_then(|val| val.device_or_deltas.build(var_store)) + .into(); + + result + } +} + +impl Anchor { + /// Create a new anchor. + pub fn new(x: i16, y: i16) -> Self { + Anchor { + x: x.into(), + y: y.into(), + contourpoint: None, + } + } + + /// Builder style method to set the device or deltas for the x value + /// + /// The argument can be a `Device` table or a `Vec<(VariationRegion, i16)>` + pub fn with_x_device(mut self, val: impl Into) -> Self { + self.x.device_or_deltas = val.into(); + self + } + + /// Builder style method to set the device or deltas for the y value + /// + /// The argument can be a `Device` table or a `Vec<(VariationRegion, i16)>` + pub fn with_y_device(mut self, val: impl Into) -> Self { + self.y.device_or_deltas = val.into(); + self + } + + /// Builder-style method to set the contourpoint. + /// + /// This is for the little-used format2 AnchorTable; it will be ignored + /// if any device or deltas have been set. + pub fn with_contourpoint(mut self, idx: u16) -> Self { + self.contourpoint = Some(idx); + self + } + + pub(crate) fn build(self, var_store: &mut VariationStoreBuilder) -> AnchorTable { + let x = self.x.default; + let y = self.y.default; + let x_dev = self.x.device_or_deltas.build(var_store); + let y_dev = self.y.device_or_deltas.build(var_store); + if x_dev.is_some() || y_dev.is_some() { + AnchorTable::format_3(x, y, x_dev, y_dev) + } else if let Some(point) = self.contourpoint { + AnchorTable::format_2(x, y, point) + } else { + AnchorTable::format_1(x, y) + } + } +} + +impl Metric { + fn is_zero(&self) -> bool { + self.default == 0 && !self.has_device_or_deltas() + } + + /// `true` if this metric has either a device table or deltas + pub fn has_device_or_deltas(&self) -> bool { + !self.device_or_deltas.is_none() + } +} + +impl DeviceOrDeltas { + fn is_none(&self) -> bool { + *self == DeviceOrDeltas::None + } + + fn build(self, var_store: &mut VariationStoreBuilder) -> Option { + match self { + DeviceOrDeltas::Device(dev) => Some(DeviceOrVariationIndex::Device(dev)), + DeviceOrDeltas::Deltas(deltas) => { + let temp_id = var_store.add_deltas(deltas); + Some(DeviceOrVariationIndex::PendingVariationIndex( + PendingVariationIndex::new(temp_id), + )) + } + DeviceOrDeltas::None => None, + } + } +} + +impl From for Metric { + fn from(src: i16) -> Metric { + Metric { + default: src, + device_or_deltas: DeviceOrDeltas::None, + } + } +} + +impl From> for DeviceOrDeltas { + fn from(src: Option) -> DeviceOrDeltas { + src.map(DeviceOrDeltas::Device).unwrap_or_default() + } +} + +impl From for DeviceOrDeltas { + fn from(value: Device) -> Self { + DeviceOrDeltas::Device(value) + } +} + +impl From> for DeviceOrDeltas { + fn from(src: Vec<(VariationRegion, i16)>) -> DeviceOrDeltas { + if src.is_empty() { + DeviceOrDeltas::None + } else { + DeviceOrDeltas::Deltas(src) + } + } +} diff --git a/fea-rs/src/compile/tables.rs b/fea-rs/src/compile/tables.rs index a9b5854db..29c955a84 100644 --- a/fea-rs/src/compile/tables.rs +++ b/fea-rs/src/compile/tables.rs @@ -57,16 +57,6 @@ pub(crate) struct VmtxBuilder { pub advances_y: Vec<(GlyphId, i16)>, } -impl Tables { - // convenience method to access the varstore, creating it if it doesn't exist - pub(crate) fn var_store(&mut self) -> &mut VariationStoreBuilder { - self.gdef - .get_or_insert_with(Default::default) - .var_store - .get_or_insert_with(Default::default) - } -} - // this is the value used in python fonttools when writing this table const DATE_2011_12_13_H11_M22_S33: LongDateTime = LongDateTime::new(1323780153); diff --git a/fea-rs/src/compile/valuerecordext.rs b/fea-rs/src/compile/valuerecordext.rs deleted file mode 100644 index 6c13e6649..000000000 --- a/fea-rs/src/compile/valuerecordext.rs +++ /dev/null @@ -1,112 +0,0 @@ -//! Extra helper methods on ValueRecord - -use write_fonts::tables::{ - gpos::{ValueFormat, ValueRecord}, - layout::DeviceOrVariationIndex, -}; - -pub(crate) trait ValueRecordExt { - fn clear_zeros(self) -> Self; - fn for_pair_pos(self, in_vert_feature: bool) -> Self; - fn is_all_zeros(&self) -> bool; - /// Set a device for the field indicated by the provided format - fn with_device>( - self, - val: Option, - field: ValueFormat, - ) -> Self; -} - -impl ValueRecordExt for ValueRecord { - fn clear_zeros(mut self) -> Self { - if self.x_placement == Some(0) && self.x_placement_device.is_none() { - self.x_placement = None; - } - - if self.y_placement == Some(0) && self.y_placement_device.is_none() { - self.y_placement = None; - } - - if self.x_advance == Some(0) && self.x_advance_device.is_none() { - self.x_advance = None; - } - - if self.y_advance == Some(0) && self.y_advance_device.is_none() { - self.y_advance = None; - } - - self - } - - /// `true` if we are not null, but our set values are all 0 - fn is_all_zeros(&self) -> bool { - let device_mask = ValueFormat::X_PLACEMENT_DEVICE - | ValueFormat::Y_PLACEMENT_DEVICE - | ValueFormat::X_ADVANCE_DEVICE - | ValueFormat::Y_ADVANCE_DEVICE; - - let format = self.format(); - if format.is_empty() || format.intersects(device_mask) { - return false; - } - let all_values = [ - self.x_placement, - self.y_placement, - self.x_advance, - self.y_advance, - ]; - all_values.iter().all(|v| v.unwrap_or_default() == 0) - } - - // Modify this value record for the special requirements of pairpos lookups - // - // In pair pos tables, if a value record is all zeros (but not null) then - // we interpret it as a having a single zero advance in the x/y direction, - // depending on context. - fn for_pair_pos(self, in_vert_feature: bool) -> Self { - if !self.is_all_zeros() { - return self.clear_zeros(); - } - let mut out = self.clear_zeros(); - if in_vert_feature { - out.y_advance = Some(0); - } else { - out.x_advance = Some(0); - } - out - } - - fn with_device>( - mut self, - val: Option, - field: ValueFormat, - ) -> Self { - let val = val.map(Into::into); - match field { - ValueFormat::X_PLACEMENT_DEVICE => self.x_placement_device = val.into(), - ValueFormat::Y_PLACEMENT_DEVICE => self.y_placement_device = val.into(), - ValueFormat::X_ADVANCE_DEVICE => self.x_advance_device = val.into(), - ValueFormat::Y_ADVANCE_DEVICE => self.y_advance_device = val.into(), - other => panic!("how did '{other:?}' get in here?"), - }; - self - } -} - -#[cfg(test)] -mod tests { - use write_fonts::tables::layout::VariationIndex; - - use super::*; - - #[test] - fn leave_zero_if_device_exists() { - let record = ValueRecord::new().with_x_advance(0).clear_zeros(); - assert!(record.x_advance.is_none()); - let record = ValueRecord::new() - .with_x_advance(0) - .with_x_advance_device(VariationIndex::new(420, 0xbeef)) - .clear_zeros(); - assert_eq!(record.x_advance, Some(0)); - } -} diff --git a/fontbe/src/features.rs b/fontbe/src/features.rs index 4b00518c9..f73ae3d27 100644 --- a/fontbe/src/features.rs +++ b/fontbe/src/features.rs @@ -199,12 +199,8 @@ impl<'a> FeatureWriter<'a> { } let mut ppos_subtables = PairPosBuilder::default(); - let mut var_indices = HashMap::new(); - for (idx, deltas) in self.kerning.deltas().enumerate() { - var_indices.insert(idx, builder.add_deltas(deltas.clone())); - } for kern in self.kerning.kerns() { - kern.insert_into(&var_indices, &mut ppos_subtables); + kern.insert_into(&mut ppos_subtables); } // now we have a builder for the pairpos subtables, so we can make @@ -252,7 +248,7 @@ impl<'a> FeatureWriter<'a> { .insert_mark( mark.gid, mark_base.class.clone(), - mark.create_anchor_table(builder)?, + mark.create_anchor_table(), ) .map_err(Error::PreviouslyAssignedClass)?; } @@ -261,7 +257,7 @@ impl<'a> FeatureWriter<'a> { mark_base_builder.insert_base( base.gid, &mark_base.class, - base.create_anchor_table(builder)?, + base.create_anchor_table(), ) } @@ -282,7 +278,7 @@ impl<'a> FeatureWriter<'a> { .insert_mark1( mark.gid, mark_mark.class.clone(), - mark.create_anchor_table(builder)?, + mark.create_anchor_table(), ) .map_err(Error::PreviouslyAssignedClass)?; } @@ -291,7 +287,7 @@ impl<'a> FeatureWriter<'a> { mark_mark_builder.insert_mark2( base.gid, &mark_mark.class, - base.create_anchor_table(builder)?, + base.create_anchor_table(), ); } mark_mark_lookups.push(builder.add_lookup( diff --git a/fontbe/src/kern.rs b/fontbe/src/kern.rs index 6b689bc37..c893e84cd 100644 --- a/fontbe/src/kern.rs +++ b/fontbe/src/kern.rs @@ -1,17 +1,17 @@ //! Generates a [Kerning] datastructure to be fed to fea-rs -use std::collections::{BTreeMap, HashMap, HashSet}; +use std::collections::{BTreeMap, HashSet}; -use fea_rs::GlyphSet; +use fea_rs::{compile::ValueRecord as ValueRecordBuilder, GlyphSet}; use fontdrasil::orchestration::{Access, Work}; -use write_fonts::{tables::gpos::ValueRecord, types::GlyphId}; +use fontir::{ir::KernParticipant, orchestration::WorkId as FeWorkId}; +use write_fonts::types::GlyphId; use crate::{ error::Error, features::resolve_variable_metric, orchestration::{AnyWorkId, BeWork, Context, Kerning, WorkId}, }; -use fontir::{ir::KernParticipant, orchestration::WorkId as FeWorkId}; #[derive(Debug)] struct KerningWork {} @@ -62,25 +62,15 @@ impl Work for KerningWork { let mut kerning = Kerning::default(); // now for each kerning entry, directly add a rule to the builder: - let mut delta_indices = HashMap::new(); for ((left, right), values) in &ir_kerning.kerns { let (default_value, deltas) = resolve_variable_metric(&static_metadata, values)?; - let delta_idx = if !deltas.is_empty() { - let mut current = delta_indices.get(&deltas).cloned(); - if current.is_none() { - let idx = kerning.add_deltas(deltas.clone()); - delta_indices.insert(deltas, idx); - current = Some(idx); - } - current - } else { - None - }; - let x_adv_record = ValueRecord::new().with_x_advance(default_value); + let x_adv_record = ValueRecordBuilder::new() + .with_x_advance(default_value) + .with_x_advance_device(deltas); match (left, right) { (KernParticipant::Glyph(left), KernParticipant::Glyph(right)) => { - kerning.add_pair(gid(left)?, x_adv_record.clone(), gid(right)?, delta_idx); + kerning.add_pair(gid(left)?, x_adv_record.clone(), gid(right)?); } (KernParticipant::Group(left), KernParticipant::Group(right)) => { let left = glyph_classes @@ -91,7 +81,7 @@ impl Work for KerningWork { .get(right) .ok_or_else(|| Error::MissingGlyphId(right.clone()))? .clone(); - kerning.add_class(left, x_adv_record.clone(), right, delta_idx); + kerning.add_class(left, x_adv_record.clone(), right); } // if groups are mixed with glyphs then we enumerate the group (KernParticipant::Glyph(left), KernParticipant::Group(right)) => { @@ -105,7 +95,7 @@ impl Work for KerningWork { .get(&right) .ok_or_else(|| Error::MissingGlyphId(right.clone()))?; for gid1 in right.iter() { - kerning.add_pair(gid0, x_adv_record.clone(), gid1, delta_idx); + kerning.add_pair(gid0, x_adv_record.clone(), gid1); } } (KernParticipant::Group(left), KernParticipant::Glyph(right)) => { @@ -114,7 +104,7 @@ impl Work for KerningWork { .ok_or_else(|| Error::MissingGlyphId(left.clone()))?; let gid1 = gid(right)?; for gid0 in left.iter() { - kerning.add_pair(gid0, x_adv_record.clone(), gid1, delta_idx); + kerning.add_pair(gid0, x_adv_record.clone(), gid1); } } } diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index 58672480a..a27beaab7 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -1,7 +1,6 @@ //! Helps coordinate the graph execution for BE use std::{ - collections::HashMap, fs::File, io::{self, BufReader, BufWriter, Read, Write}, path::{Path, PathBuf}, @@ -9,7 +8,7 @@ use std::{ }; use fea_rs::{ - compile::{FeatureBuilder, PairPosBuilder}, + compile::{PairPosBuilder, ValueRecord as ValueRecordBuilder}, GlyphMap, GlyphSet, }; use fontdrasil::{ @@ -38,13 +37,12 @@ use write_fonts::{ fvar::Fvar, gdef::Gdef, glyf::Glyph as RawGlyph, - gpos::{AnchorTable, Gpos, ValueRecord}, + gpos::Gpos, gsub::Gsub, gvar::{GlyphDelta, GlyphDeltas}, head::Head, hhea::Hhea, hvar::Hvar, - layout::PendingVariationIndex, loca::LocaFormat, maxp::Maxp, name::Name, @@ -275,56 +273,32 @@ pub enum Kern { Pair { glyph0: GlyphId, glyph1: GlyphId, - x_advance: ValueRecord, - delta_idx: Option, + x_advance: ValueRecordBuilder, }, Class { glyphs0: GlyphSet, glyphs1: GlyphSet, - x_advance: ValueRecord, - delta_idx: Option, + x_advance: ValueRecordBuilder, }, } impl Kern { - pub fn insert_into( - &self, - var_indices: &HashMap, - ppos_subtables: &mut PairPosBuilder, - ) { - let with_deltas = |x_advance: &ValueRecord, delta_idx: Option| { - if let Some(delta_idx) = delta_idx { - x_advance.clone().with_x_advance_device( - var_indices - .get(&delta_idx) - .unwrap_or_else(|| panic!("No entry for {delta_idx} in {var_indices:?}")) - .clone(), - ) - } else { - x_advance.clone() - } - }; - + pub fn insert_into(&self, ppos_subtables: &mut PairPosBuilder) { match self { Kern::Pair { glyph0, glyph1, x_advance, - delta_idx, - } => ppos_subtables.insert_pair( - *glyph0, - with_deltas(x_advance, *delta_idx), - *glyph1, - Default::default(), - ), + } => { + ppos_subtables.insert_pair(*glyph0, x_advance.clone(), *glyph1, Default::default()) + } Kern::Class { glyphs0, glyphs1, x_advance, - delta_idx, } => ppos_subtables.insert_classes( glyphs0.clone(), - with_deltas(x_advance, *delta_idx), + x_advance.clone(), glyphs1.clone(), Default::default(), ), @@ -381,24 +355,10 @@ impl MarkEntry { }) } - pub(crate) fn create_anchor_table( - &self, - builder: &mut FeatureBuilder, - ) -> Result { - let x_var_idx = - (!self.x_deltas.is_empty()).then(|| builder.add_deltas(self.x_deltas.clone())); - let y_var_idx = - (!self.y_deltas.is_empty()).then(|| builder.add_deltas(self.y_deltas.clone())); - if x_var_idx.is_some() || y_var_idx.is_some() { - Ok(AnchorTable::format_3( - self.x_default, - self.y_default, - x_var_idx.map(Into::into), - y_var_idx.map(Into::into), - )) - } else { - Ok(AnchorTable::format_1(self.x_default, self.y_default)) - } + pub(crate) fn create_anchor_table(&self) -> fea_rs::compile::Anchor { + fea_rs::compile::Anchor::new(self.x_default, self.y_default) + .with_x_device(self.x_deltas.clone()) + .with_y_device(self.y_deltas.clone()) } } @@ -500,33 +460,24 @@ impl Kerning { self.deltas.len() - 1 } - pub fn add_pair( - &mut self, - glyph0: GlyphId, - x_advance: ValueRecord, - glyph1: GlyphId, - delta_idx: Option, - ) { + pub fn add_pair(&mut self, glyph0: GlyphId, x_advance: ValueRecordBuilder, glyph1: GlyphId) { self.kerns.push(Kern::Pair { glyph0, glyph1, x_advance, - delta_idx, }) } pub fn add_class( &mut self, glyphs0: GlyphSet, - x_advance: ValueRecord, + x_advance: ValueRecordBuilder, glyphs1: GlyphSet, - delta_idx: Option, ) { self.kerns.push(Kern::Class { glyphs0, glyphs1, x_advance, - delta_idx, }) } }