diff --git a/fea-rs/src/common/glyph_class.rs b/fea-rs/src/common/glyph_class.rs index 314a346d0..ed8b19fa1 100644 --- a/fea-rs/src/common/glyph_class.rs +++ b/fea-rs/src/common/glyph_class.rs @@ -1,5 +1,3 @@ -use std::rc::Rc; - use write_fonts::types::GlyphId; use super::GlyphOrClass; @@ -24,7 +22,7 @@ use super::GlyphOrClass; /// /// [spec docs]: http://adobe-type-tools.github.io/afdko/OpenTypeFeatureFileSpecification.html#2g-glyph-classes #[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub(crate) struct GlyphClass(Rc<[GlyphId]>); +pub(crate) struct GlyphClass(Vec); /// A sorted set of unique glyph ids. /// @@ -34,7 +32,8 @@ pub(crate) struct GlyphClass(Rc<[GlyphId]>); /// In the former case, we want to be able to do things like compare for equality /// and stabily sort, so we ensure that these classes are sorted and deduped. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct GlyphSet(Rc<[GlyphId]>); +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct GlyphSet(Vec); impl GlyphClass { pub(crate) fn items(&self) -> &[GlyphId] { @@ -43,7 +42,7 @@ impl GlyphClass { /// Return a new, empty glyph class pub fn empty() -> Self { - Self(Rc::new([])) + Self(Default::default()) } /// Return a `GlyphSet` containing the unique glyphs in this class. @@ -89,7 +88,7 @@ impl<'a> std::iter::IntoIterator for &'a GlyphClass { impl From> for GlyphClass { fn from(src: Vec) -> GlyphClass { - GlyphClass(src.into()) + GlyphClass(src) } } @@ -104,7 +103,7 @@ impl From> for GlyphSet { fn from(mut value: Vec) -> Self { value.sort_unstable(); value.dedup(); - Self(value.into()) + Self(value) } } diff --git a/fea-rs/src/common/glyph_map.rs b/fea-rs/src/common/glyph_map.rs index 580436fc0..966c4b2df 100644 --- a/fea-rs/src/common/glyph_map.rs +++ b/fea-rs/src/common/glyph_map.rs @@ -15,7 +15,8 @@ use std::{ /// /// Currently, the only way to construct this type is by calling `collect()` /// on an iterator of cids or names. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct GlyphMap { names: HashMap, cids: HashMap, diff --git a/fea-rs/src/compile/lookups/gpos_builders.rs b/fea-rs/src/compile/lookups/gpos_builders.rs index a84ee5879..36347d96f 100644 --- a/fea-rs/src/compile/lookups/gpos_builders.rs +++ b/fea-rs/src/compile/lookups/gpos_builders.rs @@ -111,16 +111,19 @@ fn cmp_coverage_key(coverage: &CoverageTable) -> impl Ord { /// A builder for GPOS type 2 (PairPos) subtables /// /// This builder can build both glyph and class-based kerning subtables. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct PairPosBuilder { pairs: GlyphPairPosBuilder, classes: ClassPairPosBuilder, } -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] struct GlyphPairPosBuilder(BTreeMap>); -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] struct ClassPairPosSubtable { items: BTreeMap>, classdef_1: ClassDefBuilder2, @@ -137,7 +140,8 @@ impl Default for ClassPairPosSubtable { } } -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] struct ClassPairPosBuilder(BTreeMap<(ValueFormat, ValueFormat), Vec>); impl VariationIndexContainingLookup for PairPosBuilder { diff --git a/fea-rs/src/compile/lookups/helpers.rs b/fea-rs/src/compile/lookups/helpers.rs index 45b2c7af2..1880aa8f8 100644 --- a/fea-rs/src/compile/lookups/helpers.rs +++ b/fea-rs/src/compile/lookups/helpers.rs @@ -15,7 +15,8 @@ use crate::common::{GlyphId, GlyphSet}; // - to handle optionally assigning class 0 or not // // TODO: use this in other lookups? -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub(crate) struct ClassDefBuilder2 { classes: HashSet, glyphs: HashSet, diff --git a/fontbe/Cargo.toml b/fontbe/Cargo.toml index f3cd46adb..7951eaaff 100644 --- a/fontbe/Cargo.toml +++ b/fontbe/Cargo.toml @@ -13,7 +13,7 @@ categories = ["text-processing", "parsing", "graphics"] [dependencies] fontdrasil = { version = "0.0.1", path = "../fontdrasil" } fontir = { version = "0.0.1", path = "../fontir" } -fea-rs = { version = "0.18.0", path = "../fea-rs" } +fea-rs = { version = "0.18.0", path = "../fea-rs", features = ["serde"] } serde.workspace = true bincode.workspace = true diff --git a/fontbe/src/features.rs b/fontbe/src/features.rs index 60d132bfc..4146cd60a 100644 --- a/fontbe/src/features.rs +++ b/fontbe/src/features.rs @@ -1,6 +1,7 @@ //! Feature binary compilation. use std::{ + cell::RefCell, collections::{BTreeMap, HashMap, HashSet}, error::Error as StdError, ffi::{OsStr, OsString}, @@ -8,6 +9,7 @@ use std::{ fs, path::PathBuf, sync::Arc, + time::Instant, }; use fea_rs::{ @@ -16,34 +18,24 @@ use fea_rs::{ PairPosBuilder, VariationInfo, }, parse::{SourceLoadError, SourceResolver}, - Compiler, GlyphMap, GlyphName as FeaRsGlyphName, GlyphSet, + Compiler, }; -use font_types::{GlyphId, Tag}; +use font_types::Tag; use fontdrasil::{coords::NormalizedLocation, types::Axis}; use fontir::{ - ir::{Anchor, Features, GlyphAnchors, GlyphOrder, KernParticipant, Kerning, StaticMetadata}, + ir::{Features, StaticMetadata}, orchestration::{Flags, WorkId as FeWorkId}, }; use log::{debug, error, trace, warn}; use ordered_float::OrderedFloat; -use fontdrasil::{ - orchestration::{Access, Work}, - types::GlyphName, -}; -use write_fonts::{ - tables::layout::LookupFlag, - tables::{ - gpos::{AnchorTable, ValueRecord}, - variations::VariationRegion, - }, - OtRound, -}; +use fontdrasil::orchestration::{Access, Work}; +use write_fonts::{tables::layout::LookupFlag, tables::variations::VariationRegion, OtRound}; use crate::{ error::Error, - orchestration::{AnyWorkId, BeWork, Context, WorkId}, + orchestration::{AnyWorkId, BeWork, Context, Kerning, Marks, WorkId}, }; #[derive(Debug)] @@ -126,194 +118,88 @@ impl<'a> FeaVariationInfo<'a> { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -struct MarkGroupName<'a>(&'a str); +//NOTE: this is basically identical to the same method on FeaVariationInfo, +//except they have slightly different inputs? +pub(crate) fn resolve_variable_metric( + static_metadata: &StaticMetadata, + values: &BTreeMap>, +) -> Result<(i16, Vec<(VariationRegion, i16)>), Error> { + let var_model = &static_metadata.variation_model; -/// The type of an anchor, used when generating mark features -#[derive(Clone, Debug, PartialEq)] -enum AnchorInfo<'a> { - Base(MarkGroupName<'a>), - Mark(MarkGroupName<'a>), -} - -impl<'a> AnchorInfo<'a> { - fn new(name: &'a GlyphName) -> AnchorInfo<'a> { - // _ prefix means mark. This convention appears to come from FontLab and is now everywhere. - if name.as_str().starts_with('_') { - AnchorInfo::Mark(MarkGroupName(&name.as_str()[1..])) - } else { - AnchorInfo::Base(MarkGroupName(name.as_str())) - } - } - - fn group_name(&self) -> MarkGroupName<'a> { - match self { - AnchorInfo::Base(group_name) | AnchorInfo::Mark(group_name) => *group_name, - } - } -} - -#[derive(Default, Debug)] -struct MarkGroup<'a> { - bases: Vec<(GlyphName, &'a Anchor)>, - marks: Vec<(GlyphName, &'a Anchor)>, -} - -fn create_mark_groups<'a>( - anchors: &BTreeMap, - glyph_order: &GlyphOrder, -) -> BTreeMap, MarkGroup<'a>> { - let mut groups: BTreeMap, MarkGroup> = Default::default(); - for (glyph_name, glyph_anchors) in anchors.iter() { - // We assume the anchor list to be small - // considering only glyphs with anchors, - // - glyphs with *only* base anchors are bases - // - glyphs with *any* mark anchor are marks - // We ignore glyphs missing from the glyph order (e.g. no-export glyphs) - if !glyph_order.contains(glyph_name) { - continue; - } - let mut base = true; // TODO: only a base if user rules allow it - for anchor in glyph_anchors.anchors.iter() { - let anchor_info = AnchorInfo::new(&anchor.name); - if matches!(anchor_info, AnchorInfo::Mark(..)) { - base = false; - - // TODO: only if user rules allow us to be a mark - groups - .entry(anchor_info.group_name()) - .or_default() - .marks - .push((glyph_name.clone(), anchor)); - } - } + let point_seqs = values + .iter() + .map(|(pos, value)| (pos.to_owned(), vec![value.0 as f64])) + .collect(); + let raw_deltas: Vec<_> = var_model + .deltas(&point_seqs) + .expect("FIXME: MAKE OUR ERROR TYPE SUPPORT ? HERE") + .into_iter() + .map(|(region, values)| { + assert!(values.len() == 1, "{} values?!", values.len()); + (region, values[0]) + }) + .collect(); - if base { - for anchor in glyph_anchors.anchors.iter() { - let anchor_info = AnchorInfo::new(&anchor.name); - groups - .entry(anchor_info.group_name()) - .or_default() - .bases - .push((glyph_name.clone(), anchor)); + let default_value: i16 = raw_deltas + .iter() + .filter_map(|(region, value)| { + let scaler = region.scalar_at(&var_model.default).into_inner(); + match scaler { + scaler if scaler == 0.0 => None, + scaler => Some(scaler * *value as f32), } + }) + .sum::() + .ot_round(); + + let mut deltas = Vec::with_capacity(raw_deltas.len()); + for (region, value) in raw_deltas.iter().filter(|(r, _)| !r.is_default()) { + // https://learn.microsoft.com/en-us/typography/opentype/spec/otvarcommonformats#variation-regions + // Array of region axis coordinates records, in the order of axes given in the 'fvar' table. + let mut region_axes = Vec::with_capacity(static_metadata.axes.len()); + for axis in static_metadata.axes.iter() { + let Some(tent) = region.get(&axis.tag) else { + todo!("FIXME: add this error conversion!") + }; + region_axes.push(tent.to_region_axis_coords()); } + deltas.push(( + write_fonts::tables::variations::VariationRegion { region_axes }, + value.ot_round(), + )); } - groups + + Ok((default_value, deltas)) } struct FeatureWriter<'a> { kerning: &'a Kerning, - glyph_map: &'a GlyphOrder, - static_metadata: &'a StaticMetadata, - raw_anchors: &'a Vec<(FeWorkId, Arc)>, + marks: &'a Marks, + timing: RefCell>, } impl<'a> FeatureWriter<'a> { - fn new( - static_metadata: &'a StaticMetadata, - kerning: &'a Kerning, - glyph_map: &'a GlyphOrder, - raw_anchors: &'a Vec<(FeWorkId, Arc)>, - ) -> Self { + fn new(kerning: &'a Kerning, marks: &'a Marks) -> Self { FeatureWriter { + marks, kerning, - static_metadata, - glyph_map, - raw_anchors, + timing: Default::default(), } } - fn glyph_id(&self, glyph_name: &GlyphName) -> Result { - self.glyph_map - .glyph_id(glyph_name) - .map(|val| GlyphId::new(val as u16)) - .ok_or_else(|| Error::MissingGlyphId(glyph_name.clone())) - } - - //TODO: at least for kerning, we should be able to generate the lookups - //as a separate worktask, and then just add them here. + /// We did most of the work in the kerning job, take the data and populate a builder fn add_kerning_features(&self, builder: &mut FeatureBuilder) -> Result<(), Error> { if self.kerning.is_empty() { return Ok(()); } - - // convert the groups stored in the Kerning object into the glyph classes - // expected by fea-rs: - let glyph_classes = self - .kerning - .groups - .iter() - .map(|(class_name, members)| { - let glyph_class: Result = - members.iter().map(|name| self.glyph_id(name)).collect(); - glyph_class.map(|set| (class_name, set)) - }) - .collect::, Error>>()?; - let mut ppos_subtables = PairPosBuilder::default(); - // now for each kerning entry, directly add a rule to the builder: - for ((left, right), values) in &self.kerning.kerns { - let (default_value, deltas) = self - .resolve_variable_metric(values) - .expect("FIGURE OUT ERRORS"); - - let mut x_adv_record = ValueRecord::new().with_x_advance(default_value); - let empty_record = ValueRecord::default(); - - if !deltas.is_empty() { - let var_idx = builder.add_deltas(deltas); - x_adv_record = x_adv_record.with_x_advance_device(var_idx); - } - - match (left, right) { - (KernParticipant::Glyph(left), KernParticipant::Glyph(right)) => { - let gid0 = self.glyph_id(left)?; - let gid1 = self.glyph_id(right)?; - ppos_subtables.insert_pair(gid0, x_adv_record, gid1, empty_record); - } - (KernParticipant::Group(left), KernParticipant::Group(right)) => { - let left = glyph_classes - .get(left) - .ok_or_else(|| Error::MissingGlyphId(left.clone()))? - .clone(); - let right = glyph_classes - .get(right) - .ok_or_else(|| Error::MissingGlyphId(right.clone()))? - .clone(); - ppos_subtables.insert_classes(left, x_adv_record, right, empty_record); - } - // if groups are mixed with glyphs then we enumerate the group - (KernParticipant::Glyph(left), KernParticipant::Group(right)) => { - let gid0 = self.glyph_id(left)?; - let right = glyph_classes - .get(right) - .ok_or_else(|| Error::MissingGlyphId(right.clone()))?; - for gid1 in right.iter() { - ppos_subtables.insert_pair( - gid0, - x_adv_record.clone(), - gid1, - empty_record.clone(), - ); - } - } - (KernParticipant::Group(left), KernParticipant::Glyph(right)) => { - let left = glyph_classes - .get(left) - .ok_or_else(|| Error::MissingGlyphId(left.clone()))?; - let gid1 = self.glyph_id(right)?; - for gid0 in left.iter() { - ppos_subtables.insert_pair( - gid0, - x_adv_record.clone(), - gid1, - empty_record.clone(), - ); - } - } - } + 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); } // now we have a builder for the pairpos subtables, so we can make @@ -321,6 +207,11 @@ impl<'a> FeatureWriter<'a> { let lookups = vec![builder.add_lookup(LookupFlag::empty(), None, vec![ppos_subtables])]; builder.add_to_default_language_systems(Tag::new(b"kern"), &lookups); + { + self.timing + .borrow_mut() + .push(("End add kerning", Instant::now())); + } Ok(()) } @@ -337,88 +228,64 @@ impl<'a> FeatureWriter<'a> { /// * //TODO: could we generate as a separate task, and then just add here. fn add_marks(&self, builder: &mut FeatureBuilder) -> Result<(), Error> { - let mut anchors = BTreeMap::new(); - for (work_id, arc_glyph_anchors) in self.raw_anchors { - let glyph_anchors: &GlyphAnchors = arc_glyph_anchors; - let FeWorkId::Anchor(glyph_name) = work_id else { - return Err(Error::ExpectedAnchor(work_id.clone())); - }; - anchors.insert(glyph_name.clone(), glyph_anchors); + { + self.timing + .borrow_mut() + .push(("Start add marks", Instant::now())); } - - let mark_groups = create_mark_groups(&anchors, self.glyph_map); + let marks = self.marks; // Build the actual mark base and mark mark constructs using fea-rs builders let mut mark_base_lookups = Vec::new(); let mut mark_mark_lookups = Vec::new(); - for (group_name, group) in mark_groups.iter() { - // if we have bases *and* marks produce mark to base - if !group.bases.is_empty() && !group.marks.is_empty() { - let mut mark_base = MarkToBaseBuilder::default(); - - for (mark_name, mark_anchor) in group.marks.iter() { - let mark_gid = self.glyph_id(mark_name)?; - let mark_anchor_table = self.create_anchor_table(mark_anchor, builder)?; - mark_base - .insert_mark(mark_gid, group_name.0.into(), mark_anchor_table.clone()) - .map_err(Error::PreviouslyAssignedClass)?; - } - - for (base_name, base_anchor) in group.bases.iter() { - let base_gid = self.glyph_id(base_name)?; - let base_anchor_table = self.create_anchor_table(base_anchor, builder)?; - mark_base.insert_base(base_gid, &group_name.0.into(), base_anchor_table); - } + for mark_base in marks.mark_base.iter() { + let mut mark_base_builder = MarkToBaseBuilder::default(); + for mark in mark_base.marks.iter() { + mark_base_builder + .insert_mark( + mark.gid, + mark_base.class.clone(), + mark.create_anchor_table(builder)?, + ) + .map_err(Error::PreviouslyAssignedClass)?; + } - // each mark to base it's own lookup, whch differs from fontmake - mark_base_lookups.push(builder.add_lookup( - LookupFlag::default(), - None, - vec![mark_base], - )); + for base in mark_base.bases.iter() { + mark_base_builder.insert_base( + base.gid, + &mark_base.class, + base.create_anchor_table(builder)?, + ) } - // If a mark has anchors that are themselves marks what we got here is a mark to mark - - let mut mark_mark = MarkToMarkBuilder::default(); - let mut filter_set = Vec::new(); - - for (mark_name, _) in group.marks.iter() { - let Some(glyph_anchors) = anchors.get(mark_name) else { - continue; - }; - if !glyph_anchors - .anchors - .iter() - .any(|a| matches!(AnchorInfo::new(&a.name), AnchorInfo::Mark(..))) - { - continue; - } - let mark_gid = self.glyph_id(mark_name)?; - let Some(anchor_my_anchor) = glyph_anchors - .anchors - .iter() - .find(|a| a.name.as_str() == group_name.0) - else { - debug!("No anchor_my_anchor for {:?}", group_name.0); - continue; - }; - let anchor = self.create_anchor_table(anchor_my_anchor, builder)?; - - mark_mark - .insert_mark1(mark_gid, group_name.0.into(), anchor) + // each mark to base it's own lookup, whch differs from fontmake + mark_base_lookups.push(builder.add_lookup( + LookupFlag::default(), + None, + vec![mark_base_builder], + )); + } + + // If a mark has anchors that are themselves marks what we got here is a mark to mark + for mark_mark in marks.mark_mark.iter() { + let mut mark_mark_builder = MarkToMarkBuilder::default(); + + for mark in mark_mark.marks.iter() { + mark_mark_builder + .insert_mark1( + mark.gid, + mark_mark.class.clone(), + mark.create_anchor_table(builder)?, + ) .map_err(Error::PreviouslyAssignedClass)?; - filter_set.push(mark_gid); - } - if !filter_set.is_empty() { - mark_mark_lookups.push(builder.add_lookup( - LookupFlag::default(), - Some(filter_set.into()), - vec![mark_mark], - )); } + mark_mark_lookups.push(builder.add_lookup( + LookupFlag::default(), + Some(mark_mark.filter_set.clone().into()), + vec![mark_mark_builder], + )); } if !mark_base_lookups.is_empty() { @@ -428,96 +295,12 @@ impl<'a> FeatureWriter<'a> { builder.add_to_default_language_systems(Tag::new(b"mkmk"), &mark_mark_lookups); } - Ok(()) - } - - // a helper for getting the default & variations for an anchor - fn create_anchor_table( - &self, - anchor: &Anchor, - builder: &mut FeatureBuilder, - ) -> Result { - // squish everything into the shape expected by `resolve_variable_metric` - let (x_values, y_values) = anchor - .positions - .iter() - .map(|(loc, pt)| { - ( - (loc.clone(), OrderedFloat::from(pt.x as f32)), - (loc.clone(), OrderedFloat::from(pt.y as f32)), - ) - }) - .unzip(); - - let (x, x_deltas) = self.resolve_variable_metric(&x_values)?; - let (y, y_deltas) = self.resolve_variable_metric(&y_values)?; - let x_var_idx = (!x_deltas.is_empty()).then(|| builder.add_deltas(x_deltas)); - let y_var_idx = (!y_deltas.is_empty()).then(|| builder.add_deltas(y_deltas)); - - if x_var_idx.is_some() || y_var_idx.is_some() { - Ok(AnchorTable::format_3( - x, - y, - x_var_idx.map(Into::into), - y_var_idx.map(Into::into), - )) - } else { - Ok(AnchorTable::format_1(x, y)) + { + self.timing + .borrow_mut() + .push(("End add marks", Instant::now())); } - } - - //NOTE: this is basically identical to the same method on FeaVariationInfo, - //except they have slightly different inputs? - fn resolve_variable_metric( - &self, - values: &BTreeMap>, - ) -> Result<(i16, Vec<(VariationRegion, i16)>), Error> { - let var_model = &self.static_metadata.variation_model; - - let point_seqs = values - .iter() - .map(|(pos, value)| (pos.to_owned(), vec![value.0 as f64])) - .collect(); - let raw_deltas: Vec<_> = var_model - .deltas(&point_seqs) - .expect("FIXME: MAKE OUR ERROR TYPE SUPPORT ? HERE") - .into_iter() - .map(|(region, values)| { - assert!(values.len() == 1, "{} values?!", values.len()); - (region, values[0]) - }) - .collect(); - - let default_value: i16 = raw_deltas - .iter() - .filter_map(|(region, value)| { - let scaler = region.scalar_at(&var_model.default).into_inner(); - match scaler { - scaler if scaler == 0.0 => None, - scaler => Some(scaler * *value as f32), - } - }) - .sum::() - .ot_round(); - - let mut deltas = Vec::with_capacity(raw_deltas.len()); - for (region, value) in raw_deltas.iter().filter(|(r, _)| !r.is_default()) { - // https://learn.microsoft.com/en-us/typography/opentype/spec/otvarcommonformats#variation-regions - // Array of region axis coordinates records, in the order of axes given in the 'fvar' table. - let mut region_axes = Vec::with_capacity(self.static_metadata.axes.len()); - for axis in self.static_metadata.axes.iter() { - let Some(tent) = region.get(&axis.tag) else { - todo!("FIXME: add this error conversion!") - }; - region_axes.push(tent.to_region_axis_coords()); - } - deltas.push(( - write_fonts::tables::variations::VariationRegion { region_axes }, - value.ot_round(), - )); - } - - Ok((default_value, deltas)) + Ok(()) } } @@ -616,19 +399,17 @@ impl FeatureWork { &self, static_metadata: &StaticMetadata, features: &Features, - glyph_order: &GlyphOrder, kerning: &Kerning, - raw_anchors: &Vec<(FeWorkId, Arc)>, + marks: &Marks, ) -> Result { let var_info = FeaVariationInfo::new(static_metadata); - let feature_writer = FeatureWriter::new(static_metadata, kerning, glyph_order, raw_anchors); - let fears_glyph_map = create_glyphmap(glyph_order); - let compiler = match features { + let feature_writer = FeatureWriter::new(kerning, marks); + match features { Features::File { fea_file, include_dir, } => { - let mut compiler = Compiler::new(OsString::from(fea_file), &fears_glyph_map); + let mut compiler = Compiler::new(OsString::from(fea_file), &marks.glyphmap); if let Some(include_dir) = include_dir { compiler = compiler.with_project_root(include_dir) } @@ -640,7 +421,7 @@ impl FeatureWork { } => { let root = OsString::new(); let mut compiler = - Compiler::new(root.clone(), &fears_glyph_map).with_resolver(InMemoryResolver { + Compiler::new(root.clone(), &marks.glyphmap).with_resolver(InMemoryResolver { content_path: root, content: Arc::from(fea_content.as_str()), include_dir: include_dir.clone(), @@ -653,7 +434,7 @@ impl FeatureWork { Features::Empty => { // There is no user feature file but we could still generate kerning, marks, etc let root = OsString::new(); - Compiler::new(root.clone(), &fears_glyph_map).with_resolver(InMemoryResolver { + Compiler::new(root.clone(), &marks.glyphmap).with_resolver(InMemoryResolver { content_path: root, content: Arc::from(""), include_dir: None, @@ -661,8 +442,9 @@ impl FeatureWork { } } .with_variable_info(&var_info) - .with_feature_writer(&feature_writer); - compiler.compile().map_err(Error::FeaCompileError) + .with_feature_writer(&feature_writer) + .compile() + .map_err(Error::FeaCompileError) } } @@ -686,16 +468,6 @@ fn write_debug_fea(context: &Context, is_error: bool, why: &str, fea_content: &s }; } -fn create_glyphmap(glyph_order: &GlyphOrder) -> GlyphMap { - if glyph_order.is_empty() { - warn!("Glyph order is empty; feature compile improbable"); - } - glyph_order - .iter() - .map(|n| Into::::into(n.as_str())) - .collect() -} - impl Work for FeatureWork { fn id(&self) -> AnyWorkId { WorkId::Features.into() @@ -705,8 +477,9 @@ impl Work for FeatureWork { Access::Set(HashSet::from([ AnyWorkId::Fe(FeWorkId::GlyphOrder), AnyWorkId::Fe(FeWorkId::StaticMetadata), - AnyWorkId::Fe(FeWorkId::Kerning), AnyWorkId::Fe(FeWorkId::Features), + AnyWorkId::Be(WorkId::Kerning), + AnyWorkId::Be(WorkId::Marks), ])) } @@ -720,17 +493,15 @@ impl Work for FeatureWork { fn exec(&self, context: &Context) -> Result<(), Error> { let static_metadata = context.ir.static_metadata.get(); - let glyph_order = context.ir.glyph_order.get(); let features = context.ir.features.get(); - let kerning = context.ir.kerning.get(); - let anchors = context.ir.anchors.all(); + let kerning = context.kerning.get(); + let marks = context.marks.get(); let result = self.compile( &static_metadata, features.as_ref(), - glyph_order.as_ref(), kerning.as_ref(), - anchors.as_ref(), + marks.as_ref(), ); if result.is_err() || context.flags.contains(Flags::EMIT_DEBUG) { if let Features::Memory { fea_content, .. } = features.as_ref() { diff --git a/fontbe/src/kern.rs b/fontbe/src/kern.rs new file mode 100644 index 000000000..0973fead2 --- /dev/null +++ b/fontbe/src/kern.rs @@ -0,0 +1,128 @@ +//! Generates a [Kerning] datastructure to be fed to fea-rs + +use std::collections::{BTreeMap, HashMap, HashSet}; + +use fea_rs::GlyphSet; +use font_types::GlyphId; +use fontdrasil::orchestration::{Access, Work}; +use write_fonts::tables::gpos::ValueRecord; + +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 {} + +pub fn create_kerning_work() -> Box { + Box::new(KerningWork {}) +} + +impl Work for KerningWork { + fn id(&self) -> AnyWorkId { + WorkId::Kerning.into() + } + + fn read_access(&self) -> Access { + Access::Set(HashSet::from([ + FeWorkId::StaticMetadata.into(), + FeWorkId::Kerning.into(), + FeWorkId::GlyphOrder.into(), + ])) + } + + /// Generate kerning data structures. + fn exec(&self, context: &Context) -> Result<(), Error> { + let static_metadata = context.ir.static_metadata.get(); + let glyph_order = context.ir.glyph_order.get(); + let ir_kerning = context.ir.kerning.get(); + let gid = |name| { + glyph_order + .glyph_id(name) + .map(|gid| GlyphId::new(gid as u16)) + .ok_or_else(|| Error::MissingGlyphId(name.clone())) + }; + + // convert the groups stored in the Kerning object into the glyph classes + // expected by fea-rs: + let glyph_classes = ir_kerning + .groups + .iter() + .map(|(class_name, glyph_set)| { + let glyph_class: GlyphSet = glyph_set + .iter() + .map(|name| GlyphId::new(glyph_order.glyph_id(name).unwrap_or(0) as u16)) + .collect(); + (class_name, glyph_class) + }) + .collect::>(); + + 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); + + match (left, right) { + (KernParticipant::Glyph(left), KernParticipant::Glyph(right)) => { + kerning.add_pair(gid(left)?, x_adv_record.clone(), gid(right)?, delta_idx); + } + (KernParticipant::Group(left), KernParticipant::Group(right)) => { + let left = glyph_classes + .get(left) + .ok_or_else(|| Error::MissingGlyphId(left.clone()))? + .clone(); + let right = glyph_classes + .get(right) + .ok_or_else(|| Error::MissingGlyphId(right.clone()))? + .clone(); + kerning.add_class(left, x_adv_record.clone(), right, delta_idx); + } + // if groups are mixed with glyphs then we enumerate the group + (KernParticipant::Glyph(left), KernParticipant::Group(right)) => { + let gid0 = GlyphId::new( + glyph_order + .glyph_id(left) + .ok_or_else(|| Error::MissingGlyphId(left.clone()))? + as u16, + ); + let right = glyph_classes + .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); + } + } + (KernParticipant::Group(left), KernParticipant::Glyph(right)) => { + let left = glyph_classes + .get(left) + .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); + } + } + } + } + + context.kerning.set(kerning); + + Ok(()) + } +} diff --git a/fontbe/src/lib.rs b/fontbe/src/lib.rs index 7a82870bd..6035bd0b8 100644 --- a/fontbe/src/lib.rs +++ b/fontbe/src/lib.rs @@ -8,6 +8,8 @@ pub mod glyphs; pub mod gvar; pub mod head; pub mod hvar; +pub mod kern; +pub mod marks; pub mod metrics_and_limits; pub mod name; pub mod orchestration; diff --git a/fontbe/src/marks.rs b/fontbe/src/marks.rs new file mode 100644 index 000000000..e0789b062 --- /dev/null +++ b/fontbe/src/marks.rs @@ -0,0 +1,208 @@ +//! Generates a [Marks] datastructure to be fed to fea-rs + +use std::{collections::BTreeMap, sync::Arc}; + +use font_types::GlyphId; +use fontdrasil::{ + orchestration::{Access, Work}, + types::GlyphName, +}; +use log::debug; + +use crate::{ + error::Error, + orchestration::{ + AnyWorkId, BeWork, Context, MarkBase, MarkEntry, MarkGroup, MarkGroupName, MarkMark, Marks, + WorkId, + }, +}; +use fontir::{ + ir::{GlyphAnchors, GlyphOrder}, + orchestration::WorkId as FeWorkId, +}; + +#[derive(Debug)] +struct MarkWork {} + +pub fn create_mark_work() -> Box { + Box::new(MarkWork {}) +} + +/// The type of an anchor, used when generating mark features +#[derive(Clone, Debug, PartialEq)] +pub(crate) enum AnchorInfo { + Base(MarkGroupName), + Mark(MarkGroupName), +} + +impl AnchorInfo { + pub(crate) fn new(name: &GlyphName) -> AnchorInfo { + // _ prefix means mark. This convention appears to come from FontLab and is now everywhere. + if name.as_str().starts_with('_') { + AnchorInfo::Mark(MarkGroupName(name.as_str()[1..].into())) + } else { + AnchorInfo::Base(MarkGroupName(name.as_str().into())) + } + } + + pub(crate) fn group_name(&self) -> MarkGroupName { + match self { + AnchorInfo::Base(group_name) | AnchorInfo::Mark(group_name) => group_name.clone(), + } + } +} + +fn create_mark_groups( + anchors: &BTreeMap>, + glyph_order: &GlyphOrder, +) -> BTreeMap { + let mut groups: BTreeMap = Default::default(); + for (glyph_name, glyph_anchors) in anchors.iter() { + // We assume the anchor list to be small + // considering only glyphs with anchors, + // - glyphs with *only* base anchors are bases + // - glyphs with *any* mark anchor are marks + // We ignore glyphs missing from the glyph order (e.g. no-export glyphs) + if !glyph_order.contains(glyph_name) { + continue; + } + let mut base = true; // TODO: only a base if user rules allow it + for anchor in glyph_anchors.anchors.iter() { + let anchor_info = AnchorInfo::new(&anchor.name); + if matches!(anchor_info, AnchorInfo::Mark(..)) { + base = false; + + // TODO: only if user rules allow us to be a mark + groups + .entry(anchor_info.group_name()) + .or_default() + .marks + .push((glyph_name.clone(), anchor.clone())); + } + } + + if base { + for anchor in glyph_anchors.anchors.iter() { + let anchor_info = AnchorInfo::new(&anchor.name); + groups + .entry(anchor_info.group_name()) + .or_default() + .bases + .push((glyph_name.clone(), anchor.clone())); + } + } + } + groups +} + +impl Work for MarkWork { + fn id(&self) -> AnyWorkId { + WorkId::Marks.into() + } + + fn read_access(&self) -> Access { + Access::Custom(Arc::new(|id| { + matches!( + id, + AnyWorkId::Fe(FeWorkId::StaticMetadata) + | AnyWorkId::Fe(FeWorkId::GlyphOrder) + | AnyWorkId::Fe(FeWorkId::Anchor(..)) + ) + })) + } + + /// Generate mark data structures. + fn exec(&self, context: &Context) -> Result<(), Error> { + let static_metadata = context.ir.static_metadata.get(); + let glyph_order = context.ir.glyph_order.get(); + let raw_anchors = context.ir.anchors.all(); + let gid = |name| { + glyph_order + .glyph_id(name) + .map(|gid| GlyphId::new(gid as u16)) + .ok_or_else(|| Error::MissingGlyphId(name.clone())) + }; + + let mut anchors = BTreeMap::new(); + for (work_id, arc_glyph_anchors) in raw_anchors { + let FeWorkId::Anchor(glyph_name) = work_id else { + return Err(Error::ExpectedAnchor(work_id.clone())); + }; + anchors.insert(glyph_name.clone(), arc_glyph_anchors.clone()); + } + + let groups = create_mark_groups(&anchors, &glyph_order); + + let mut marks = Marks { + glyphmap: glyph_order + .iter() + .map(|n| Into::::into(n.as_str())) + .collect(), + ..Default::default() + }; + + for (group_name, group) in groups.iter() { + // if we have bases *and* marks produce mark to base + if !group.bases.is_empty() && !group.marks.is_empty() { + let mut mark_base = MarkBase::new(group_name.0.clone()); + + for (mark_name, mark_anchor) in group.marks.iter() { + mark_base.insert_mark(MarkEntry::new( + &static_metadata, + gid(mark_name)?, + mark_anchor, + )?); + } + + for (base_name, base_anchor) in group.bases.iter() { + mark_base.insert_base(MarkEntry::new( + &static_metadata, + gid(base_name)?, + base_anchor, + )?); + } + + marks.mark_base.push(mark_base); + } + + // If a mark has anchors that are themselves marks what we got here is a mark to mark + + let mut mark_mark = MarkMark::new(group_name.0.clone()); + for (mark_name, _) in group.marks.iter() { + let Some(glyph_anchors) = anchors.get(mark_name) else { + continue; + }; + if !glyph_anchors + .anchors + .iter() + .any(|a| matches!(AnchorInfo::new(&a.name), AnchorInfo::Mark(..))) + { + continue; + } + let Some(anchor_my_anchor) = glyph_anchors + .anchors + .iter() + .find(|a| a.name.as_str() == group_name.0) + else { + debug!("No anchor_my_anchor for {:?}", group_name.0); + continue; + }; + + let mark_gid = gid(mark_name)?; + mark_mark.insert_mark(MarkEntry::new( + &static_metadata, + mark_gid, + anchor_my_anchor, + )?); + mark_mark.filter_set.push(mark_gid); + } + if !mark_mark.filter_set.is_empty() { + marks.mark_mark.push(mark_mark); + } + } + + context.marks.set(marks); + + Ok(()) + } +} diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index b9ff77541..fbb7730ed 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -1,18 +1,24 @@ //! Helps coordinate the graph execution for BE use std::{ + collections::HashMap, fs::File, io::{self, BufReader, BufWriter, Read, Write}, path::{Path, PathBuf}, sync::Arc, }; -use font_types::{F2Dot14, Tag}; +use fea_rs::{ + compile::{FeatureBuilder, PairPosBuilder}, + GlyphMap, GlyphSet, +}; +use font_types::{F2Dot14, GlyphId, Tag}; use fontdrasil::{ orchestration::{Access, AccessControlList, Identifier, Work}, types::GlyphName, }; use fontir::{ + ir::{Anchor, StaticMetadata}, orchestration::{ Context as FeContext, ContextItem, ContextMap, Flags, IdAware, Persistable, PersistentStorage, WorkId as FeWorkIdentifier, @@ -20,9 +26,11 @@ use fontir::{ variations::VariationRegion, }; use log::trace; +use ordered_float::OrderedFloat; use read_fonts::{FontData, FontRead}; use serde::{Deserialize, Serialize}; +use smol_str::SmolStr; use write_fonts::{ dump_table, tables::{ @@ -31,25 +39,26 @@ use write_fonts::{ fvar::Fvar, gdef::Gdef, glyf::Glyph as RawGlyph, - gpos::Gpos, + gpos::{AnchorTable, Gpos, ValueRecord}, gsub::Gsub, gvar::{GlyphDelta, GlyphDeltas}, head::Head, hhea::Hhea, hvar::Hvar, + layout::PendingVariationIndex, loca::LocaFormat, maxp::Maxp, name::Name, os2::Os2, post::Post, stat::Stat, - variations::Tuple, + variations::{Tuple, VariationRegion as BeVariationRegion}, }, validate::Validate, FontWrite, }; -use crate::{error::Error, paths::Paths}; +use crate::{error::Error, features::resolve_variable_metric, paths::Paths}; /// What exactly is being assembled from glyphs? #[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -81,8 +90,10 @@ pub enum WorkId { Hhea, Hmtx, Hvar, + Kerning, Loca, LocaFormat, + Marks, Maxp, Name, Os2, @@ -258,6 +269,273 @@ impl TupleBuilder { } } +/// Prebuilt kern to the extent we can without being able to assign deltas to a value record. +#[derive(Clone, Serialize, Deserialize, PartialEq)] +pub enum Kern { + Pair { + glyph0: GlyphId, + glyph1: GlyphId, + x_advance: ValueRecord, + delta_idx: Option, + }, + Class { + glyphs0: GlyphSet, + glyphs1: GlyphSet, + x_advance: ValueRecord, + delta_idx: Option, + }, +} + +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() + } + }; + + match self { + Kern::Pair { + glyph0, + glyph1, + x_advance, + delta_idx, + } => ppos_subtables.insert_pair( + *glyph0, + with_deltas(x_advance, *delta_idx), + *glyph1, + Default::default(), + ), + Kern::Class { + glyphs0, + glyphs1, + x_advance, + delta_idx, + } => ppos_subtables.insert_classes( + glyphs0.clone(), + with_deltas(x_advance, *delta_idx), + glyphs1.clone(), + Default::default(), + ), + } + } +} + +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(crate) struct MarkGroupName(pub(crate) SmolStr); + +#[derive(Default, Clone, Serialize, Deserialize, PartialEq)] +pub(crate) struct MarkGroup { + pub(crate) bases: Vec<(GlyphName, Anchor)>, + pub(crate) marks: Vec<(GlyphName, Anchor)>, +} + +/// A mark or base entry, prepped for submission to the fea-rs builder +#[derive(Default, Clone, Serialize, Deserialize, PartialEq)] +pub(crate) struct MarkEntry { + pub(crate) gid: GlyphId, + x_default: i16, + x_deltas: Vec<(BeVariationRegion, i16)>, + y_default: i16, + y_deltas: Vec<(BeVariationRegion, i16)>, +} + +impl MarkEntry { + pub(crate) fn new( + static_metadata: &StaticMetadata, + gid: GlyphId, + anchor: &Anchor, + ) -> Result { + // squish everything into the shape expected by `resolve_variable_metric` + let (x_values, y_values) = anchor + .positions + .iter() + .map(|(loc, pt)| { + ( + (loc.clone(), OrderedFloat::from(pt.x as f32)), + (loc.clone(), OrderedFloat::from(pt.y as f32)), + ) + }) + .unzip(); + + let (x_default, x_deltas) = resolve_variable_metric(static_metadata, &x_values)?; + let (y_default, y_deltas) = resolve_variable_metric(static_metadata, &y_values)?; + + Ok(Self { + gid, + x_default, + x_deltas, + y_default, + y_deltas, + }) + } + + 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)) + } + } +} + +#[derive(Default, Clone, Serialize, Deserialize, PartialEq)] +pub(crate) struct MarkBase { + pub(crate) class: SmolStr, + pub(crate) marks: Vec, + pub(crate) bases: Vec, +} + +impl MarkBase { + pub(crate) fn new(class: SmolStr) -> Self { + MarkBase { + class, + ..Default::default() + } + } + + pub(crate) fn insert_mark(&mut self, entry: MarkEntry) { + self.marks.push(entry); + } + + pub(crate) fn insert_base(&mut self, entry: MarkEntry) { + self.bases.push(entry); + } +} + +#[derive(Default, Clone, Serialize, Deserialize, PartialEq)] +pub(crate) struct MarkMark { + pub(crate) class: SmolStr, + pub(crate) filter_set: Vec, + pub(crate) marks: Vec, +} + +impl MarkMark { + pub(crate) fn new(class: SmolStr) -> Self { + MarkMark { + class, + ..Default::default() + } + } + + pub(crate) fn insert_mark(&mut self, entry: MarkEntry) { + self.marks.push(entry); + } +} + +/// Precomputed marks, to the extent possible given that we cannot create temporary var indices in advance. +/// +/// TODO: update once is fixed. Then we can build a +/// actual fea-rs structs in advance. +#[derive(Default, Clone, Serialize, Deserialize, PartialEq)] +pub struct Marks { + pub(crate) glyphmap: GlyphMap, + pub(crate) mark_base: Vec, + pub(crate) mark_mark: Vec, +} + +impl Persistable for Marks { + fn read(from: &mut dyn Read) -> Self { + bincode::deserialize_from(from).unwrap() + } + + fn write(&self, to: &mut dyn io::Write) { + bincode::serialize_into(to, self).unwrap() + } +} + +/// Precomputed kerning, to the extent possible given that we cannot create temporary var indices in advance. +/// +/// TODO: update once is fixed. Then we can build a +/// [`fea_rs::compile::PairPosBuilder`] in advance. +#[derive(Default, Clone, Serialize, Deserialize, PartialEq)] +pub struct Kerning { + deltas: Vec>, + kerns: Vec, +} + +impl Kerning { + pub fn is_empty(&self) -> bool { + self.kerns.is_empty() + } + + pub fn deltas(&self) -> impl Iterator> { + self.deltas.iter() + } + + pub fn kerns(&self) -> impl Iterator { + self.kerns.iter() + } + + pub fn add_deltas(&mut self, deltas: Vec<(BeVariationRegion, i16)>) -> usize { + self.deltas.push(deltas); + self.deltas.len() - 1 + } + + pub fn add_pair( + &mut self, + glyph0: GlyphId, + x_advance: ValueRecord, + glyph1: GlyphId, + delta_idx: Option, + ) { + self.kerns.push(Kern::Pair { + glyph0, + glyph1, + x_advance, + delta_idx, + }) + } + + pub fn add_class( + &mut self, + glyphs0: GlyphSet, + x_advance: ValueRecord, + glyphs1: GlyphSet, + delta_idx: Option, + ) { + self.kerns.push(Kern::Class { + glyphs0, + glyphs1, + x_advance, + delta_idx, + }) + } +} + +impl Persistable for Kerning { + fn read(from: &mut dyn Read) -> Self { + bincode::deserialize_from(from).unwrap() + } + + fn write(&self, to: &mut dyn io::Write) { + bincode::serialize_into(to, self).unwrap() + } +} + // work around orphan rules. // // FIXME: Clarify if there's a good reason not to treat glyf/loca as a single @@ -410,6 +688,8 @@ pub struct Context { pub hhea: BeContextItem>, pub hmtx: BeContextItem, pub hvar: BeContextItem>, + pub kerning: BeContextItem, + pub marks: BeContextItem, pub stat: BeContextItem>, pub font: BeContextItem, } @@ -441,6 +721,8 @@ impl Context { hhea: self.hhea.clone_with_acl(acl.clone()), hmtx: self.hmtx.clone_with_acl(acl.clone()), hvar: self.hvar.clone_with_acl(acl.clone()), + kerning: self.kerning.clone_with_acl(acl.clone()), + marks: self.marks.clone_with_acl(acl.clone()), stat: self.stat.clone_with_acl(acl.clone()), font: self.font.clone_with_acl(acl), } @@ -480,6 +762,16 @@ impl Context { hhea: ContextItem::new(WorkId::Hhea.into(), acl.clone(), persistent_storage.clone()), hmtx: ContextItem::new(WorkId::Hmtx.into(), acl.clone(), persistent_storage.clone()), hvar: ContextItem::new(WorkId::Hvar.into(), acl.clone(), persistent_storage.clone()), + kerning: ContextItem::new( + WorkId::Kerning.into(), + acl.clone(), + persistent_storage.clone(), + ), + marks: ContextItem::new( + WorkId::Marks.into(), + acl.clone(), + persistent_storage.clone(), + ), stat: ContextItem::new(WorkId::Stat.into(), acl.clone(), persistent_storage.clone()), font: ContextItem::new(WorkId::Font.into(), acl, persistent_storage), } diff --git a/fontbe/src/paths.rs b/fontbe/src/paths.rs index 3d3bca9e8..a811c409d 100644 --- a/fontbe/src/paths.rs +++ b/fontbe/src/paths.rs @@ -64,6 +64,8 @@ impl Paths { WorkId::Hhea => self.build_dir.join("hhea.table"), WorkId::Hmtx => self.build_dir.join("hmtx.table"), WorkId::Hvar => self.build_dir.join("hvar.table"), + WorkId::Kerning => self.build_dir.join("kerning.bin"), + WorkId::Marks => self.build_dir.join("marks.bin"), WorkId::Maxp => self.build_dir.join("maxp.table"), WorkId::Name => self.build_dir.join("name.table"), WorkId::Os2 => self.build_dir.join("os2.table"), diff --git a/fontc/src/change_detector.rs b/fontc/src/change_detector.rs index 0f1bba502..7f2197eb6 100644 --- a/fontc/src/change_detector.rs +++ b/fontc/src/change_detector.rs @@ -240,12 +240,31 @@ impl ChangeDetector { pub fn feature_be_change(&self) -> bool { self.feature_ir_change() + || self.kerning_be_change() + || self.mark_be_change() || !self .be_paths .target_file(&BeWorkIdentifier::Features) .is_file() } + pub fn mark_be_change(&self) -> bool { + // Glyphs produce anchors and we need anchors + !self.glyphs_changed().is_empty() + || !self + .be_paths + .target_file(&BeWorkIdentifier::Marks) + .is_file() + } + + pub fn kerning_be_change(&self) -> bool { + self.kerning_ir_change() + || !self + .be_paths + .target_file(&BeWorkIdentifier::Kerning) + .is_file() + } + pub fn kerning_ir_change(&self) -> bool { self.static_metadata_ir_change() || self.glyph_order_ir_change() diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs index 642fbbbdd..c105191e9 100644 --- a/fontc/src/lib.rs +++ b/fontc/src/lib.rs @@ -31,6 +31,8 @@ use fontbe::{ gvar::create_gvar_work, head::create_head_work, hvar::create_hvar_work, + kern::create_kerning_work, + marks::create_mark_work, metrics_and_limits::create_metric_and_limit_work, name::create_name_work, orchestration::AnyWorkId, @@ -131,6 +133,46 @@ fn add_feature_be_job(workload: &mut Workload) -> Result<(), Error> { Ok(()) } +fn add_marks_be_job(workload: &mut Workload) -> Result<(), Error> { + let work = create_mark_work(); + // Features are extremely prone to not making sense when glyphs are filtered + if workload.change_detector.mark_be_change() { + if workload.change_detector.glyph_name_filter().is_some() { + warn!("Not processing BE marks because a glyph name filter is active"); + } + if workload.change_detector.should_skip_features() { + debug!("Not processing BE marks because FEA compilation is disabled"); + } + } + workload.add( + work.into(), + workload.change_detector.mark_be_change() + && workload.change_detector.glyph_name_filter().is_none() + && !workload.change_detector.should_skip_features(), + ); + Ok(()) +} + +fn add_kerning_be_job(workload: &mut Workload) -> Result<(), Error> { + let work = create_kerning_work(); + // Features are extremely prone to not making sense when glyphs are filtered + if workload.change_detector.kerning_be_change() { + if workload.change_detector.glyph_name_filter().is_some() { + warn!("Not processing BE kerning because a glyph name filter is active"); + } + if workload.change_detector.should_skip_features() { + debug!("Not processing BE kerning because FEA compilation is disabled"); + } + } + workload.add( + work.into(), + workload.change_detector.kerning_be_change() + && workload.change_detector.glyph_name_filter().is_none() + && !workload.change_detector.should_skip_features(), + ); + Ok(()) +} + fn add_kerning_ir_job(workload: &mut Workload) -> Result<(), Error> { let work = workload .change_detector @@ -314,6 +356,8 @@ pub fn create_workload( add_fvar_be_job(&mut workload)?; add_gvar_be_job(&mut workload)?; add_head_be_job(&mut workload)?; + add_kerning_be_job(&mut workload)?; + add_marks_be_job(&mut workload)?; add_metric_and_limits_job(&mut workload)?; add_hvar_be_job(&mut workload)?; add_name_be_job(&mut workload)?; @@ -572,8 +616,10 @@ mod tests { BeWorkIdentifier::Hhea.into(), BeWorkIdentifier::Hmtx.into(), BeWorkIdentifier::Hvar.into(), + BeWorkIdentifier::Kerning.into(), BeWorkIdentifier::Loca.into(), BeWorkIdentifier::LocaFormat.into(), + BeWorkIdentifier::Marks.into(), BeWorkIdentifier::Maxp.into(), BeWorkIdentifier::Name.into(), BeWorkIdentifier::Os2.into(), @@ -670,15 +716,20 @@ mod tests { vec![ AnyWorkId::Fe(FeWorkIdentifier::Glyph("bar".into())), FeWorkIdentifier::Anchor("bar".into()).into(), + BeWorkIdentifier::Features.into(), BeWorkIdentifier::Cmap.into(), BeWorkIdentifier::Font.into(), BeWorkIdentifier::Glyf.into(), + BeWorkIdentifier::Gpos.into(), + BeWorkIdentifier::Gsub.into(), + BeWorkIdentifier::Gdef.into(), BeWorkIdentifier::Gvar.into(), BeWorkIdentifier::Hhea.into(), BeWorkIdentifier::Hmtx.into(), BeWorkIdentifier::Hvar.into(), BeWorkIdentifier::Loca.into(), BeWorkIdentifier::LocaFormat.into(), + BeWorkIdentifier::Marks.into(), BeWorkIdentifier::Maxp.into(), ], completed, @@ -708,6 +759,7 @@ mod tests { BeWorkIdentifier::Gpos.into(), BeWorkIdentifier::Gsub.into(), BeWorkIdentifier::Gdef.into(), + BeWorkIdentifier::Kerning.into(), ], completed ); @@ -821,6 +873,7 @@ mod tests { BeWorkIdentifier::Gpos.into(), BeWorkIdentifier::Gsub.into(), BeWorkIdentifier::Gdef.into(), + BeWorkIdentifier::Kerning.into(), ], completed ); diff --git a/fontc/src/timing.rs b/fontc/src/timing.rs index 281477cdf..d4315666d 100644 --- a/fontc/src/timing.rs +++ b/fontc/src/timing.rs @@ -140,8 +140,10 @@ fn short_name(id: &AnyWorkId) -> &'static str { AnyWorkId::Be(BeWorkIdentifier::Hhea) => "hhea", AnyWorkId::Be(BeWorkIdentifier::Hmtx) => "hmtx", AnyWorkId::Be(BeWorkIdentifier::Hvar) => "HVAR", + AnyWorkId::Be(BeWorkIdentifier::Kerning) => "Kern", AnyWorkId::Be(BeWorkIdentifier::Loca) => "loca", AnyWorkId::Be(BeWorkIdentifier::LocaFormat) => "loca-fmt", + AnyWorkId::Be(BeWorkIdentifier::Marks) => "Marks", AnyWorkId::Be(BeWorkIdentifier::Maxp) => "maxp", AnyWorkId::Be(BeWorkIdentifier::Name) => "name", AnyWorkId::Be(BeWorkIdentifier::Os2) => "OS/2", @@ -178,6 +180,8 @@ fn color(id: &AnyWorkId) -> &'static str { AnyWorkId::Be(BeWorkIdentifier::Hhea) => "gray", AnyWorkId::Be(BeWorkIdentifier::Hmtx) => "v", AnyWorkId::Be(BeWorkIdentifier::Hvar) => "gray", + AnyWorkId::Be(BeWorkIdentifier::Kerning) => "gray", + AnyWorkId::Be(BeWorkIdentifier::Marks) => "gray", AnyWorkId::Be(BeWorkIdentifier::Loca) => "gray", AnyWorkId::Be(BeWorkIdentifier::LocaFormat) => "gray", AnyWorkId::Be(BeWorkIdentifier::Maxp) => "gray", diff --git a/fontc/src/workload.rs b/fontc/src/workload.rs index fd7d453b9..22bdbe9ea 100644 --- a/fontc/src/workload.rs +++ b/fontc/src/workload.rs @@ -345,15 +345,17 @@ impl<'a> Workload<'a> { // Higher priority sorts last, which means run first due to pop // We basically want things that block the glyph order => kern => fea sequence to go asap match work.id() { - AnyWorkId::Be(BeWorkIdentifier::Features) => 99, AnyWorkId::Fe(FeWorkIdentifier::Features) => 99, AnyWorkId::Fe(FeWorkIdentifier::Kerning) => 99, AnyWorkId::Fe(FeWorkIdentifier::GlyphOrder) => 99, AnyWorkId::Fe(FeWorkIdentifier::PreliminaryGlyphOrder) => 99, AnyWorkId::Fe(FeWorkIdentifier::StaticMetadata) => 99, AnyWorkId::Fe(FeWorkIdentifier::GlobalMetrics) => 99, - AnyWorkId::Fe(FeWorkIdentifier::Glyph(..)) => 1, - _ => 0, + AnyWorkId::Be(BeWorkIdentifier::Kerning) => 99, + AnyWorkId::Be(BeWorkIdentifier::Features) => 99, + AnyWorkId::Be(BeWorkIdentifier::GlyfFragment(..)) => 0, + AnyWorkId::Be(BeWorkIdentifier::GvarFragment(..)) => 0, + _ => 32, } }); }