Merge pull request #387 from googlefonts/tuple-axis-order

must follow fvar axes order when building gvar GlyphDeltas
googlefonts · Aug 9, 2023 · 076d267 · 076d267
2 parents 287e6bc + aafc68c
commit 076d267
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 8 deletions.
diff --git a/fontbe/src/gvar.rs b/fontbe/src/gvar.rs
@@ -52,21 +52,29 @@ impl Work<Context, AnyWorkId, Error> for GvarWork {
         Access::Custom(Arc::new(|id| {
             matches!(
                 id,
-                AnyWorkId::Fe(FeWorkId::GlyphOrder) | AnyWorkId::Be(WorkId::GvarFragment(..))
+                AnyWorkId::Fe(FeWorkId::StaticMetadata)
+                    | AnyWorkId::Fe(FeWorkId::GlyphOrder)
+                    | AnyWorkId::Be(WorkId::GvarFragment(..))
             )
         }))
     }
 
     /// Generate [gvar](https://learn.microsoft.com/en-us/typography/opentype/spec/gvar)
     fn exec(&self, context: &Context) -> Result<(), Error> {
         // We built the gvar fragments alongside glyphs, now we need to glue them together into a gvar table
+        let static_metadata = context.ir.static_metadata.get();
+        let axis_order: Vec<_> = static_metadata
+            .variable_axes
+            .iter()
+            .map(|a| a.tag)
+            .collect();
         let glyph_order = context.ir.glyph_order.get();
 
         let variations: Vec<_> = make_variations(&glyph_order, |glyph_name| {
             context
                 .gvar_fragments
                 .get(&WorkId::GvarFragment(glyph_name.clone()).into())
-                .to_deltas()
+                .to_deltas(&axis_order)
         });
         let gvar = Gvar::new(variations).map_err(Error::GvarError)?;
 

diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs
@@ -242,7 +242,7 @@ pub struct GvarFragment {
 }
 
 impl GvarFragment {
-    pub fn to_deltas(&self) -> Vec<GlyphDeltas> {
+    pub fn to_deltas(&self, axis_order: &[Tag]) -> Vec<GlyphDeltas> {
         self.deltas
             .iter()
             .filter_map(|(region, deltas)| {
@@ -266,7 +266,7 @@ impl GvarFragment {
                     })
                     .collect();
 
-                let tuple_builder = TupleBuilder::new(region);
+                let tuple_builder = TupleBuilder::new(region, axis_order);
                 let (min, peak, max) = tuple_builder.build();
                 Some(GlyphDeltas::new(peak, deltas, Some((min, max))))
             })
@@ -300,9 +300,10 @@ struct TupleBuilder {
 }
 
 impl TupleBuilder {
-    fn new(region: &VariationRegion) -> Self {
+    fn new(region: &VariationRegion, axis_order: &[Tag]) -> Self {
         let mut builder = TupleBuilder::default();
-        for (tag, tent) in region.iter() {
+        for tag in axis_order {
+            let tent = region.get(tag).unwrap();
             builder.axes.push(*tag);
             builder.min.push(F2Dot14::from_f32(tent.min.to_f32()));
             builder.peak.push(F2Dot14::from_f32(tent.peak.to_f32()));
@@ -642,7 +643,7 @@ mod tests {
                 vec![None, Some((1.0, 0.0).into()), None],
             )],
         }
-        .to_deltas();
+        .to_deltas(&[Tag::new(b"wght")]);
         assert!(!deltas.is_empty(), "{deltas:?}");
     }
 
@@ -652,7 +653,7 @@ mod tests {
             glyph_name: "blah".into(),
             deltas: vec![(non_default_region(), vec![None, None, None])],
         }
-        .to_deltas();
+        .to_deltas(&[Tag::new(b"wght")]);
         assert!(deltas.is_empty(), "{deltas:?}");
     }
 }
diff --git a/fontir/src/variations.rs b/fontir/src/variations.rs
@@ -465,6 +465,10 @@ impl VariationRegion {
     pub fn is_default(&self) -> bool {
         self.active_axes.is_empty()
     }
+
+    pub fn get(&self, tag: &Tag) -> Option<&Tent> {
+        self.axis_tents.get(tag)
+    }
 }
 
 /// The min/peak/max of a masters influence.