diff --git a/fea-rs/src/compile/feature_writer.rs b/fea-rs/src/compile/feature_writer.rs index 8a2858e32..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,18 +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 { - todo!("remove me!") - } - /// Add lookups to every default language system. /// /// Convenience method for recurring pattern. 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, }) } }