[fea-rs] Remove ClassId, add gdef classes to output

- ClassId was a pre-write-fonts enum standing in for the GlyphClassDef
  enum, which we can now juse use directly
- passing any explicit GDEF classes through to the output feels like the
  simplest way to expose these to manual feature generation code, since
  that code needs to compile anyway to get the GSUB table.

fea-rs/src/compile/compile_ctx.rs

@@ -13,7 +13,7 @@ use smol_str::SmolStr;
 use write_fonts::{
     tables::{
         self,
-        gdef::CaretValue,
+        gdef::{CaretValue, GlyphClassDef},
         gpos::ValueFormat,
         layout::{ConditionFormat1, ConditionSet, FeatureVariations, LookupFlag},
         variations::ivs_builder::{RemapVariationIndices, VariationStoreBuilder},
@@ -45,7 +45,7 @@ use super::{
     },
     metrics::{Anchor, DeviceOrDeltas, Metric, ValueRecord},
     output::Compilation,
-    tables::{ClassId, ScriptRecord, Tables},
+    tables::{GlyphClassDefExt, ScriptRecord, Tables},
     tags, VariationInfo,
 };
 
@@ -240,6 +240,10 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> {
             }
         }
 
+        let gdef_classes = self.tables.gdef.as_ref().and_then(|gdef| {
+            (!gdef.glyph_classes_were_inferred).then(|| gdef.glyph_classes.clone())
+        });
+
         Ok((
             Compilation {
                 head: self.tables.head.as_ref().map(|raw| raw.build(None)),
@@ -253,6 +257,7 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> {
                 gsub,
                 gpos,
                 opts: self.opts.clone(),
+                gdef_classes,
             },
             self.errors.clone(),
         ))
@@ -294,6 +299,7 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> {
         let mut gdef = self.tables.gdef.take().unwrap_or_default();
         // infer glyph classes, if they were not declared explicitly
         if gdef.glyph_classes.is_empty() {
+            gdef.glyph_classes_were_inferred = true;
             self.lookups.infer_glyph_classes(|glyph, class_id| {
                 gdef.glyph_classes.insert(glyph, class_id);
             });
@@ -303,7 +309,7 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> {
                 .flat_map(|class| class.members.iter().map(|(cls, _)| cls.iter()))
                 .flatten()
             {
-                gdef.glyph_classes.insert(glyph, ClassId::Mark);
+                gdef.glyph_classes.insert(glyph, GlyphClassDef::Mark);
             }
         }
 
@@ -1691,19 +1697,20 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> {
 
                 typed::GdefTableItem::ClassDef(rule) => {
                     for (class, id) in [
-                        (rule.base_glyphs(), ClassId::Base),
-                        (rule.ligature_glyphs(), ClassId::Ligature),
-                        (rule.mark_glyphs(), ClassId::Mark),
-                        (rule.component_glyphs(), ClassId::Component),
+                        (rule.base_glyphs(), GlyphClassDef::Base),
+                        (rule.ligature_glyphs(), GlyphClassDef::Ligature),
+                        (rule.mark_glyphs(), GlyphClassDef::Mark),
+                        (rule.component_glyphs(), GlyphClassDef::Component),
                     ] {
                         let Some(class) = class else {
                             continue;
                         };
                         if let Err((bad_glyph, old_class)) =
                             gdef.add_glyph_class(self.resolve_glyph_class(&class), id)
                         {
-                            let bad_glyph_name = self.reverse_glyph_map.get(&bad_glyph).unwrap();
-                            self.error(class.range(), format!("class includes glyph '{bad_glyph_name}', already in class {old_class}"));
+                            let bad_name = self.reverse_glyph_map.get(&bad_glyph).unwrap();
+                            let class_name = old_class.display();
+                            self.error(class.range(), format!("class includes glyph '{bad_name}', already in class {class_name}"));
                         }
                     }
                 }

fea-rs/src/compile/lookups.rs

@@ -14,6 +14,7 @@ use smol_str::SmolStr;
 
 use write_fonts::{
     tables::{
+        gdef::GlyphClassDef,
         gpos::{self as write_gpos},
         gsub as write_gsub,
         layout::{
@@ -33,7 +34,7 @@ use crate::{
     Kind, Opts,
 };
 
-use super::{features::AllFeatures, metrics::Anchor, tables::ClassId, tags};
+use super::{features::AllFeatures, metrics::Anchor, tags};
 
 use contextual::{
     ContextualLookupBuilder, PosChainContextBuilder, PosContextBuilder, ReverseChainBuilder,
@@ -506,27 +507,35 @@ impl AllLookups {
         )));
     }
 
-    pub(crate) fn infer_glyph_classes(&self, mut f: impl FnMut(GlyphId, ClassId)) {
+    pub(crate) fn infer_glyph_classes(&self, mut f: impl FnMut(GlyphId, GlyphClassDef)) {
         for lookup in &self.gpos {
             match lookup {
                 PositionLookup::MarkToBase(lookup) => {
                     for subtable in &lookup.subtables {
-                        subtable.base_glyphs().for_each(|k| f(k, ClassId::Base));
-                        subtable.mark_glyphs().for_each(|k| f(k, ClassId::Mark));
+                        subtable
+                            .base_glyphs()
+                            .for_each(|k| f(k, GlyphClassDef::Base));
+                        subtable
+                            .mark_glyphs()
+                            .for_each(|k| f(k, GlyphClassDef::Mark));
                     }
                 }
                 PositionLookup::MarkToLig(lookup) => {
                     for subtable in &lookup.subtables {
-                        subtable.lig_glyphs().for_each(|k| f(k, ClassId::Ligature));
-                        subtable.mark_glyphs().for_each(|k| f(k, ClassId::Mark));
+                        subtable
+                            .lig_glyphs()
+                            .for_each(|k| f(k, GlyphClassDef::Ligature));
+                        subtable
+                            .mark_glyphs()
+                            .for_each(|k| f(k, GlyphClassDef::Mark));
                     }
                 }
                 PositionLookup::MarkToMark(lookup) => {
                     for subtable in &lookup.subtables {
                         subtable
                             .mark1_glyphs()
                             .chain(subtable.mark2_glyphs())
-                            .for_each(|k| f(k, ClassId::Mark));
+                            .for_each(|k| f(k, GlyphClassDef::Mark));
                     }
                 }
                 _ => (),

fea-rs/src/compile/output.rs

@@ -1,7 +1,10 @@
 //! The result of a compilation
 
+use std::collections::HashMap;
+
 use write_fonts::{
-    tables::{self as wtables, maxp::Maxp},
+    tables::{self as wtables, gdef::GlyphClassDef, maxp::Maxp},
+    types::GlyphId,
     BuilderError, FontBuilder,
 };
 
@@ -42,6 +45,11 @@ pub struct Compilation {
     pub gsub: Option<wtables::gsub::Gsub>,
     /// The `GPOS` table, if one was generated
     pub gpos: Option<wtables::gpos::Gpos>,
+    /// Any *explicit* gdef classes declared in the FEA.
+    ///
+    /// This is provided so that the user can reference them if they are going
+    /// to manually generate kerning or markpos lookups.
+    pub gdef_classes: Option<HashMap<GlyphId, GlyphClassDef>>,
 }
 
 impl Compilation {

fea-rs/src/compile/tables.rs

@@ -27,7 +27,7 @@ mod os2;
 mod stat;
 
 pub(crate) use base::{BaseBuilder, ScriptRecord};
-pub(crate) use gdef::{ClassId, GdefBuilder};
+pub(crate) use gdef::{GdefBuilder, GlyphClassDefExt};
 pub(crate) use name::{NameBuilder, NameSpec};
 pub(crate) use os2::{CodePageRange, Os2Builder};
 pub(crate) use stat::{AxisLocation, AxisRecord, AxisValue, StatBuilder, StatFallbackName};

fea-rs/src/compile/tables/gdef.rs

@@ -7,6 +7,7 @@
 //! [gdef-spec]: http://adobe-type-tools.github.io/afdko/OpenTypeFeatureFileSpecification.html#9b-gdef-table
 
 use std::collections::{BTreeMap, BTreeSet, HashMap};
+use std::fmt::Display;
 
 use write_fonts::types::GlyphId;
 
@@ -24,34 +25,19 @@ use crate::common::{GlyphClass, GlyphSet};
 /// Data collected from a GDEF block.
 #[derive(Clone, Debug, Default)]
 pub struct GdefBuilder {
-    pub glyph_classes: HashMap<GlyphId, ClassId>,
+    pub glyph_classes: HashMap<GlyphId, GlyphClassDef>,
+    /// if `true`, then glyph classes were not declared explicitly.
+    ///
+    /// we track this because it is an important distinction when using the
+    /// glyph classes for manually generated kern/markpos lookups
+    pub glyph_classes_were_inferred: bool,
     pub attach: BTreeMap<GlyphId, BTreeSet<u16>>,
     pub ligature_pos: BTreeMap<GlyphId, Vec<CaretValue>>,
     pub mark_attach_class: BTreeMap<GlyphId, u16>,
     pub mark_glyph_sets: Vec<GlyphSet>,
     pub var_store: Option<VariationStoreBuilder>,
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-#[repr(u16)]
-pub enum ClassId {
-    Base = 1,
-    Ligature = 2,
-    Mark = 3,
-    Component = 4,
-}
-
-impl From<ClassId> for GlyphClassDef {
-    fn from(src: ClassId) -> GlyphClassDef {
-        match src {
-            ClassId::Base => GlyphClassDef::Base,
-            ClassId::Ligature => GlyphClassDef::Ligature,
-            ClassId::Mark => GlyphClassDef::Mark,
-            ClassId::Component => GlyphClassDef::Component,
-        }
-    }
-}
-
 impl GdefBuilder {
     pub fn build(&self) -> (tables::gdef::Gdef, Option<VariationIndexRemapping>) {
         let mut table = tables::gdef::Gdef::new(
@@ -128,8 +114,8 @@ impl GdefBuilder {
         // technically should be a GlyphSet, but we're storing it as individual
         // glyphs and this is private API, so :shrug:
         glyphs: GlyphClass,
-        class: ClassId,
-    ) -> Result<(), (GlyphId, ClassId)> {
+        class: GlyphClassDef,
+    ) -> Result<(), (GlyphId, GlyphClassDef)> {
         for glyph in glyphs.iter() {
             if let Some(prev_class) = self.glyph_classes.insert(glyph, class) {
                 if prev_class != class {
@@ -150,13 +136,18 @@ impl GdefBuilder {
     }
 }
 
-impl std::fmt::Display for ClassId {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+pub(crate) trait GlyphClassDefExt {
+    fn display(&self) -> impl Display;
+}
+
+impl GlyphClassDefExt for GlyphClassDef {
+    fn display(&self) -> impl Display {
         match self {
-            ClassId::Base => write!(f, "Base"),
-            ClassId::Ligature => write!(f, "Ligature"),
-            ClassId::Mark => write!(f, "Mark"),
-            ClassId::Component => write!(f, "Component"),
+            GlyphClassDef::Base => "Base",
+            GlyphClassDef::Ligature => "Ligature",
+            GlyphClassDef::Mark => "Mark",
+            GlyphClassDef::Component => "Component",
+            _ => "Invalid",
         }
     }
 }