diff --git a/fea-rs/src/compile/feature_writer.rs b/fea-rs/src/compile/feature_writer.rs index d2e07e373..a8d1e4819 100644 --- a/fea-rs/src/compile/feature_writer.rs +++ b/fea-rs/src/compile/feature_writer.rs @@ -2,11 +2,12 @@ use std::collections::{BTreeMap, HashMap}; -use fontdrasil::coords::NormalizedLocation; -use smol_str::SmolStr; -use write_fonts::tables::{ - layout::{LookupFlag, PendingVariationIndex}, - variations::VariationRegion, +use write_fonts::{ + tables::{ + layout::{LookupFlag, PendingVariationIndex}, + variations::VariationRegion, + }, + types::Tag, }; use crate::common::GlyphClass; @@ -30,7 +31,6 @@ pub struct FeatureBuilder<'a> { pub(crate) tables: &'a mut Tables, pub(crate) lookups: Vec<(LookupId, PositionLookup)>, pub(crate) features: BTreeMap, - pub(crate) mark_classes: HashMap>, mark_filter_sets: HashMap, // because there may already be defined filter sets from the root fea filter_set_id_start: usize, @@ -61,7 +61,6 @@ impl<'a> FeatureBuilder<'a> { tables, lookups: Default::default(), features: Default::default(), - mark_classes: Default::default(), mark_filter_sets: Default::default(), filter_set_id_start, } @@ -77,55 +76,6 @@ impl<'a> FeatureBuilder<'a> { self.tables.gdef.as_ref() } - /// Define a mark class with a variable anchor. - /// - /// Roughly equivalent to `markClass glyph|class @name{};` - /// [markClass](https://adobe-type-tools.github.io/afdko/OpenTypeFeatureFileSpecification.html#4.f) - /// with a variable anchor record. - /// - /// TODO: do we want to ensure there are no redefinitions? do we want - /// return an error? do we want to uphold any other invariants? - /// - /// ALSO: mark class IDs depend on definition order in the source. - /// I have no idea how best to approximate that in this API :/ - pub fn define_mark_class( - &mut self, - members: impl Into, - anchors: HashMap, - ) { - // TODO: an error type - let members = members.into(); - if self.mark_classes.insert(members, anchors).is_some() { - panic!("Multiple insertions for the same glyph class") - } - } - - /// Setup a mark to base the recommended way: in a lookup of it's very own. - /// - /// Pseudo-fea: - /// - /// ```text - /// lookup name { - /// pos base glyph|glyphclass @markclass; - /// // plus variations of anchor pos - /// } - /// - /// - /// ``` - pub fn add_mark_base_pos( - &mut self, - base: impl Into, - class: impl Into, - anchors: HashMap, - ) { - // TODO: an error type - - // TODO populate the lookup - let lookup_id = self.add_lookup(LookupFlag::default(), None, Default::default()); - - //eprintln!(" lookup {lookup_name} {{\n pos base {base_name} @{class_name}; # TODO variable anchor;\n }} {lookup_name};", default_anchor.0, default_anchor.1); - } - /// Create a new lookup. /// /// The `LookupId` that is returned can then be included in features @@ -155,6 +105,16 @@ impl<'a> FeatureBuilder<'a> { PendingVariationIndex { delta_set_id } } + /// Add lookups to every default language system. + /// + /// Convenience method for recurring pattern. + pub fn add_to_default_language_systems(&mut self, feature_tag: Tag, lookups: &Vec) { + for langsys in self.language_systems() { + let feature_key = langsys.to_feature_key(feature_tag); + self.add_feature(feature_key, lookups.clone()); + } + } + /// Create a new feature, registered for a particular language system. /// /// The caller must call this method once for each language system under diff --git a/fea-rs/src/compile/lookups/gpos.rs b/fea-rs/src/compile/lookups/gpos.rs index 0656b9ff7..0ef167af5 100644 --- a/fea-rs/src/compile/lookups/gpos.rs +++ b/fea-rs/src/compile/lookups/gpos.rs @@ -447,6 +447,7 @@ impl VariationIndexContainingLookup for MarkToBaseBuilder { } /// An error indicating a given glyph has been assigned to multiple mark classes +#[derive(Clone, Debug, Default)] pub struct PreviouslyAssignedClass { /// The ID of the glyph in conflict pub glyph_id: GlyphId, diff --git a/fontbe/src/error.rs b/fontbe/src/error.rs index 31e759feb..bcaeefb9d 100644 --- a/fontbe/src/error.rs +++ b/fontbe/src/error.rs @@ -1,6 +1,6 @@ use std::{fmt::Display, io, path::PathBuf}; -use fea_rs::compile::error::CompilerError; +use fea_rs::compile::{error::CompilerError, PreviouslyAssignedClass}; use font_types::Tag; use fontdrasil::types::GlyphName; use fontir::{ @@ -65,6 +65,10 @@ pub enum Error { MissingTable(Tag), #[error("Expected an anchor, got {0:?}")] ExpectedAnchor(FeWorkId), + #[error("No glyph id for '{0}'")] + MissingGlyphId(GlyphName), + #[error("Multiple assignments for class: {0:?}")] + PreviouslyAssignedClass(PreviouslyAssignedClass), } #[derive(Debug)] diff --git a/fontbe/src/features.rs b/fontbe/src/features.rs index 497d83299..2c2984c18 100644 --- a/fontbe/src/features.rs +++ b/fontbe/src/features.rs @@ -11,7 +11,10 @@ use std::{ }; use fea_rs::{ - compile::{Compilation, FeatureBuilder, FeatureProvider, PairPosBuilder, VariationInfo}, + compile::{ + Compilation, FeatureBuilder, FeatureProvider, MarkToBaseBuilder, MarkToMarkBuilder, + PairPosBuilder, VariationInfo, + }, parse::{SourceLoadError, SourceResolver}, Compiler, GlyphClass, GlyphMap, GlyphName as FeaRsGlyphName, }; @@ -29,7 +32,12 @@ use fontdrasil::{ orchestration::{Access, Work}, types::GlyphName, }; -use write_fonts::{tables::gpos::ValueRecord, tables::layout::LookupFlag, OtRound}; +use smol_str::SmolStr; +use write_fonts::{ + tables::gpos::{AnchorFormat1, AnchorTable, ValueRecord}, + tables::layout::LookupFlag, + OtRound, +}; use crate::{ error::Error, @@ -265,14 +273,8 @@ impl<'a> FeatureWriter<'a> { // now we have a builder for the pairpos subtables, so we can make // a lookup: - let lookup_id = builder.add_lookup(LookupFlag::empty(), None, vec![ppos_subtables]); - let kern = Tag::new(b"kern"); - let lookups = vec![lookup_id]; - // now register this feature for each of the default language systems - for langsys in builder.language_systems() { - let feature_key = langsys.to_feature_key(kern); - builder.add_feature(feature_key, lookups.clone()); - } + let lookups = vec![builder.add_lookup(LookupFlag::empty(), None, vec![ppos_subtables])]; + builder.add_to_default_language_systems(Tag::new(b"kern"), &lookups) } /// Generate mark to base and mark to mark features @@ -328,132 +330,92 @@ impl<'a> FeatureWriter<'a> { } } - // **** TODO: drive builder instead of generating a string **** + // Build the actual mark base and mark mark constructs using fea-rs builders - let mut classes = Vec::new(); - let mut mark_to_bases = Vec::new(); - let mut mark_to_marks = Vec::new(); + let mut mark_base_lookups = Vec::new(); + let mut mark_mark_lookups = Vec::new(); for (group_name, group) in groups.iter() { - // write a mark class for every mark - for (glyph_name, mark) in group.marks.iter() { - let default_pos = mark.default_pos(); - classes.push(format!( - "markClass {} @MC{}; # TODO variable anchor", - glyph_name, default_pos.x, default_pos.y, mark.name - )); - // TODO error handling - let mark_gid = self.glyph_id(&mark.name).unwrap_or(GlyphId::NOTDEF); - let variations = mark - .positions - .iter() - .map(|(loc, pos)| (loc.clone(), (pos.x.ot_round(), pos.y.ot_round()))) - .collect(); - builder.define_mark_class(mark_gid, variations); - } + let mark_class_name: SmolStr = format!("MC_{group_name}").into(); - // if we have bases *and* marks emit mark to base + // if we have bases *and* marks produce mark to base for (base_name, base) in group.bases.iter() { + let Some(base_gid) = self.glyph_id(base_name) else { + return Err(Error::MissingGlyphId(base_name.clone())); + }; + let mark_class = format!("MC_{}", base.name).into(); let default_pos = base.default_pos(); - let mark_name = &base.name; - let mark_class = format!("MC_{mark_name}"); - let lookup_name = format!("mark2base_{base_name}_{group_name}"); - let variations = base - .positions - .iter() - .filter_map(|(loc, pos)| { - if loc.has_any_non_zero() { - Some((loc.clone(), (pos.x.ot_round(), pos.y.ot_round()))) - } else { - None - } - }) - .collect(); - // TODO: arithmetic for pos - mark_to_bases.push(format!(" lookup {lookup_name} {{\n pos base {base_name} @MC_{mark_name}; # TODO variable anchor;\n }} {lookup_name};", default_pos.x, default_pos.y)); - builder.add_mark_base_pos( - lookup_name, - base_name.as_str(), - mark_class, - (default_pos.x.ot_round(), default_pos.y.ot_round()), - variations, - ); + let anchor = AnchorTable::Format1(AnchorFormat1::new( + default_pos.x.ot_round(), + default_pos.y.ot_round(), + )); + + let mut mark_base = MarkToBaseBuilder::default(); + mark_base.insert_base(base_gid, &mark_class, anchor); + + // each in it's own lookup, whch differs from fontmake + mark_base_lookups.push(builder.add_lookup( + LookupFlag::default(), + None, + vec![mark_base], + )); + + // TODO: variations + // TODO how do you use self.resolve_variable_metric(values) on 2d values } // If a mark has anchors that are themselves marks what we got here is a mark to mark - let mut snippets = Vec::new(); - let mut mark_names = Vec::new(); + + let mut mark_mark = MarkToMarkBuilder::default(); + let mut filter_set = Vec::new(); + for (mark_name, mark_anchor) in group.marks.iter() { let Some(glyph_anchors) = anchors.get(mark_name) else { continue; }; + if !glyph_anchors + .anchors + .iter() + .any(|a| AnchorType::Mark == a.name.anchor_type()) + { + continue; + } + let Some(mark_gid) = self.glyph_id(mark_name) else { + return Err(Error::MissingGlyphId(mark_name.clone())); + }; + let base_name = AnchorType::Mark.group_name(mark_anchor.name.clone()); // TODO Ew let Some(anchor_my_anchor) = glyph_anchors.anchors.iter().find(|a| a.name == base_name) else { - eprintln!("No anchor_my_anchor for {base_name}"); + debug!("No anchor_my_anchor for {base_name}"); continue; }; - let Some((_, default_pos)) = anchor_my_anchor - .positions - .iter() - .find(|(loc, _)| !loc.has_any_non_zero()) - else { - panic!("TODO return a useful error"); - }; - for glyph_anchor in glyph_anchors.anchors.iter() { - if AnchorType::Mark != glyph_anchor.name.anchor_type() { - continue; - } - //let group_name = AnchorType::Mark.group_name(mark_anchor.name.clone()); - if snippets.is_empty() { - snippets.push(format!(" lookup mark2mark_{group_name} {{")); - snippets.push("".to_string()); // placeholder for MarkFilteringSet - snippets.push(format!( - " lookupflag UseMarkFilteringSet @MFS_mark2mark_{group_name};" - )); - } - mark_names.push(mark_name.to_string()); + let default_pos = anchor_my_anchor.default_pos(); + let anchor = AnchorTable::Format1(AnchorFormat1::new( + default_pos.x.ot_round(), + default_pos.y.ot_round(), + )); - // TODO: arithmetic for pos - snippets.push(format!( - " pos mark {} mark @MC_{group_name}; # TODO variable anchor", - mark_name, default_pos.x, default_pos.y - )); - } - } + // TODO: variations + // TODO how do you use self.resolve_variable_metric(values) on 2d values - if !snippets.is_empty() { - mark_names.sort(); - snippets[1] = format!( - " @MFS_mark2mark_{group_name} = [{}];", - mark_names.join(" ") - ); - snippets.push(" }".to_string()); - mark_to_marks.push(snippets.join("\n")); + mark_mark + .insert_mark1(mark_gid, mark_class_name.clone(), anchor) + .map_err(Error::PreviouslyAssignedClass)?; + filter_set.push(mark_gid); } + mark_mark_lookups.push(builder.add_lookup( + LookupFlag::default(), + Some(filter_set.into()), + vec![mark_mark], + )); } - classes.sort(); - for class in classes { - eprintln!("{class}"); - } + builder.add_to_default_language_systems(Tag::new(b"mark"), &mark_base_lookups); + builder.add_to_default_language_systems(Tag::new(b"mkmk"), &mark_mark_lookups); - mark_to_bases.sort(); - eprintln!("\nfeature mark {{"); - for mark_to_base in mark_to_bases { - eprintln!("{mark_to_base}"); - } - eprintln!("}} mark;"); - - mark_to_marks.sort(); - eprintln!("\nfeature mkmk {{"); - for mark_to_mark in mark_to_marks { - eprintln!("{mark_to_mark}"); - } - eprintln!("}} mkmk;"); - - todo!("Add marks") + Ok(()) } //NOTE: this is basically identical to the same method on FeaVariationInfo, @@ -748,14 +710,19 @@ 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 features = (*context.ir.features.get()).clone(); // TODO does this need to be cloned? - - let result = self.compile(&static_metadata, &features, &glyph_order, &kerning); + let result = self.compile( + &static_metadata, + features.as_ref(), + glyph_order.as_ref(), + kerning.as_ref(), + anchors.as_ref(), + ); if result.is_err() || context.flags.contains(Flags::EMIT_DEBUG) { - if let Features::Memory { fea_content, .. } = &features { + if let Features::Memory { fea_content, .. } = features.as_ref() { write_debug_fea(context, result.is_err(), "compile failed", fea_content); } }