[read-fonts] Add exact variation deltas to HVAR, VVAR and MVAR

fauntlet/src/font/skrifa.rs

@@ -143,7 +143,7 @@ impl<'a> SkrifaSfntInstance<'a> {
     pub fn hvar_and_gvar_advance_deltas(&self, glyph_id: GlyphId) -> Option<(i32, i32)> {
         let hvar = self.font.hvar().ok()?;
         let gvar = self.font.gvar().ok()?;
-        let hvar_delta = hvar.advance_width_delta(glyph_id, &self.coords).ok()?;
+        let hvar_delta = hvar.advance_delta(glyph_id, &self.coords)?;
         let gvar_delta = gvar
             .phantom_point_deltas(
                 &self.font.glyf().ok()?,

read-fonts/src/tables/hvar.rs

@@ -5,13 +5,14 @@ use super::variations::{self, DeltaSetIndexMap, ItemVariationStore};
 include!("../../generated/generated_hvar.rs");
 
 impl Hvar<'_> {
-    /// Returns the advance width delta for the specified glyph identifier and
-    /// normalized variation coordinates.
-    pub fn advance_width_delta(
-        &self,
-        glyph_id: GlyphId,
-        coords: &[F2Dot14],
-    ) -> Result<Fixed, ReadError> {
+    /// Returns the change a location makes to the advance width of a glyph.
+    ///
+    /// The value carries every bit the item variation store computed. It is
+    /// a caller that decides how to round it into a whole design unit, and
+    /// implementations differ on that.
+    ///
+    /// Returns `None` where the table says nothing readable about the glyph.
+    pub fn advance_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Option<F48Dot16> {
         variations::advance_delta(
             self.advance_width_mapping(),
             self.item_variation_store(),
@@ -20,9 +21,15 @@ impl Hvar<'_> {
         )
     }
 
-    /// Returns the left side bearing delta for the specified glyph identifier and
-    /// normalized variation coordinates.
-    pub fn lsb_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Result<Fixed, ReadError> {
+    /// Returns the change a location makes to the left side bearing of a
+    /// glyph.
+    ///
+    /// The value carries every bit the item variation store computed. It is
+    /// a caller that decides how to round it into a whole design unit, and
+    /// implementations differ on that.
+    ///
+    /// Returns `None` where the table says nothing readable about the glyph.
+    pub fn lsb_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Option<F48Dot16> {
         variations::item_delta(
             self.lsb_mapping(),
             self.item_variation_store(),
@@ -31,9 +38,15 @@ impl Hvar<'_> {
         )
     }
 
-    /// Returns the left side bearing delta for the specified glyph identifier and
-    /// normalized variation coordinates.
-    pub fn rsb_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Result<Fixed, ReadError> {
+    /// Returns the change a location makes to the right side bearing of a
+    /// glyph.
+    ///
+    /// The value carries every bit the item variation store computed. It is
+    /// a caller that decides how to round it into a whole design unit, and
+    /// implementations differ on that.
+    ///
+    /// Returns `None` where the table says nothing readable about the glyph.
+    pub fn rsb_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Option<F48Dot16> {
         variations::item_delta(
             self.rsb_mapping(),
             self.item_variation_store(),
@@ -46,43 +59,69 @@ impl Hvar<'_> {
 #[cfg(test)]
 mod tests {
     use crate::{tables::variations::DeltaSetIndexMap, FontRef, TableProvider};
-    use types::{F2Dot14, Fixed, GlyphId};
+    use types::{F2Dot14, F48Dot16, GlyphId};
+
+    #[test]
+    fn a_delta_keeps_the_fraction_the_variation_store_computed() {
+        // These locations land between whole design units. Rounding is left
+        // to a caller, and implementations disagree on how, so the value
+        // reported here keeps what the store worked out.
+        let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap();
+        let hvar = font.hvar().unwrap();
+        let gid = GlyphId::new(1);
+        for (coord, expected) in [(-1.0, -113.0), (-0.75, -84.75), (-0.5, -56.5)] {
+            let coords = [F2Dot14::from_f32(coord)];
+            assert_eq!(
+                hvar.advance_delta(gid, &coords),
+                Some(F48Dot16::from_f64(expected)),
+                "at {coord}"
+            );
+        }
+    }
+
+    #[test]
+    fn a_default_location_moves_nothing() {
+        let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap();
+        let hvar = font.hvar().unwrap();
+        let gid = GlyphId::new(1);
+        assert_eq!(hvar.advance_delta(gid, &[]), Some(F48Dot16::ZERO));
+        assert_eq!(hvar.lsb_delta(gid, &[]), Some(F48Dot16::ZERO));
+        assert_eq!(hvar.rsb_delta(gid, &[]), Some(F48Dot16::ZERO));
+    }
+
+    #[test]
+    fn a_mapping_the_font_does_not_state_is_absent() {
+        // This font maps advances but neither side bearing.
+        let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap();
+        let hvar = font.hvar().unwrap();
+        let gid = GlyphId::new(1);
+        let coords = [F2Dot14::from_f32(-0.75)];
+        assert!(hvar.advance_delta(gid, &coords).is_some());
+        assert_eq!(hvar.lsb_delta(gid, &coords), None);
+        assert_eq!(hvar.rsb_delta(gid, &coords), None);
+    }
 
     #[test]
     fn advance_deltas() {
         let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap();
         let hvar = font.hvar().unwrap();
         let gid_a = GlyphId::new(1);
-        assert_eq!(
-            hvar.advance_width_delta(gid_a, &[F2Dot14::from_f32(-1.0)])
-                .unwrap(),
-            Fixed::from_f64(-113.0)
-        );
-        assert_eq!(
-            hvar.advance_width_delta(gid_a, &[F2Dot14::from_f32(-0.75)])
-                .unwrap(),
-            Fixed::from_f64(-85.0)
-        );
-        assert_eq!(
-            hvar.advance_width_delta(gid_a, &[F2Dot14::from_f32(-0.5)])
-                .unwrap(),
-            Fixed::from_f64(-56.0)
-        );
-        assert_eq!(
-            hvar.advance_width_delta(gid_a, &[F2Dot14::from_f32(0.0)])
-                .unwrap(),
-            Fixed::from_f64(0.0)
-        );
-        assert_eq!(
-            hvar.advance_width_delta(gid_a, &[F2Dot14::from_f32(0.5)])
-                .unwrap(),
-            Fixed::from_f64(30.0)
-        );
-        assert_eq!(
-            hvar.advance_width_delta(gid_a, &[F2Dot14::from_f32(1.0)])
-                .unwrap(),
-            Fixed::from_f64(59.0)
-        );
+        // The odd quarters are what the store computed; the rounded form of
+        // this accessor reported -85 and -56 for them.
+        for (coord, expected) in [
+            (-1.0, -113.0),
+            (-0.75, -84.75),
+            (-0.5, -56.5),
+            (0.0, 0.0),
+            (0.5, 29.5),
+            (1.0, 59.0),
+        ] {
+            assert_eq!(
+                hvar.advance_delta(gid_a, &[F2Dot14::from_f32(coord)]),
+                Some(F48Dot16::from_f64(expected)),
+                "at {coord}"
+            );
+        }
     }
 
     #[test]
@@ -99,14 +138,14 @@ mod tests {
         assert_eq!(num_glyphs, 24);
         assert_eq!(adv_index_map.map_count(), 15);
         let last_mapped_gid = adv_index_map.map_count() - 1;
-        // We expect the last 10 glyphs to have the same advance width delta as the last mapped glyph.
-        // Crucially, hvar.advance_width_delta() should not return OutOfBounds for these glyphs.
+        // We expect the last 10 glyphs to have the same advance width delta as
+        // the last mapped glyph. Crucially, the accessor should answer for
+        // them rather than reporting the glyph as out of bounds.
         for idx in last_mapped_gid..num_glyphs {
             let gid = GlyphId::new(idx as _);
             assert_eq!(
-                hvar.advance_width_delta(gid, &[F2Dot14::from_f32(1.0)])
-                    .unwrap(),
-                Fixed::from_f64(100.0)
+                hvar.advance_delta(gid, &[F2Dot14::from_f32(1.0)]),
+                Some(F48Dot16::from_f64(100.0))
             );
         }
     }

read-fonts/src/tables/mvar.rs

@@ -1,6 +1,7 @@
 //! The [MVAR (Metrics Variation)](https://docs.microsoft.com/en-us/typography/opentype/spec/mvar) table
 
 use super::variations::{DeltaSetIndex, ItemVariationStore};
+use types::F48Dot16;
 
 /// Four-byte tags used to represent particular metric or other values.
 pub mod tags {
@@ -96,6 +97,58 @@ pub mod tags {
 
 include!("../../generated/generated_mvar.rs");
 
+/// `MVAR` at one location, with the variation store resolved once.
+///
+/// Prefer this over [`Mvar::metric_delta`] when reading more than one metric;
+/// that resolves the store on every lookup.
+pub struct MvarInstance<'a> {
+    records: &'a [ValueRecord],
+    ivs: ItemVariationStore<'a>,
+    coords: &'a [F2Dot14],
+}
+
+impl<'a> Mvar<'a> {
+    /// Returns the table at a location.
+    ///
+    /// `None` at the default location, where every delta is zero, and for a
+    /// font whose variation store cannot be read.
+    pub fn at(&self, coords: &'a [F2Dot14]) -> Option<MvarInstance<'a>> {
+        if coords.is_empty() {
+            return None;
+        }
+        Some(MvarInstance {
+            records: self.value_records(),
+            ivs: self.item_variation_store()?.ok()?,
+            coords,
+        })
+    }
+}
+
+impl MvarInstance<'_> {
+    /// Returns the delta for a metric, or `None` for one the font does not
+    /// vary.
+    ///
+    /// Tags are in the [`tags`] module. The delta is in design units, added
+    /// to the value the metric's own table holds, and left unrounded so a
+    /// caller can round it its own way.
+    pub fn get(&self, tag: Tag) -> Option<F48Dot16> {
+        let index = self
+            .records
+            .binary_search_by(|record| record.value_tag().cmp(&tag))
+            .ok()?;
+        let record = &self.records[index];
+        self.ivs
+            .compute_delta(
+                DeltaSetIndex {
+                    outer: record.delta_set_outer_index(),
+                    inner: record.delta_set_inner_index(),
+                },
+                self.coords,
+            )
+            .ok()
+    }
+}
+
 impl Mvar<'_> {
     /// Returns the metric delta for the specified tag and normalized
     /// variation coordinates. Possible tags are found in the [tags]
@@ -133,3 +186,66 @@ impl Mvar<'_> {
         Err(ReadError::MetricIsMissing(tag))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{FontRef, TableProvider};
+    use types::F2Dot14;
+
+    /// Twelve axes, and an `MVAR` varying several metrics.
+    const VAR: &[u8] = font_test_data::AMSTELVAR_AVAR2_A;
+
+    fn far() -> [F2Dot14; 12] {
+        [F2Dot14::from_f32(1.0); 12]
+    }
+
+    #[test]
+    fn the_default_location_has_no_instance() {
+        // Every delta is zero there, so there is nothing to resolve.
+        let font = FontRef::new(VAR).unwrap();
+        assert!(font.mvar().unwrap().at(&[]).is_none());
+    }
+
+    #[test]
+    fn an_instance_agrees_with_the_single_metric_accessor() {
+        let font = FontRef::new(VAR).unwrap();
+        let mvar = font.mvar().unwrap();
+        let coords = far();
+        let instance = mvar.at(&coords).expect("a location away from the default");
+        let mut varied = 0;
+        for record in mvar.value_records() {
+            let tag = record.value_tag();
+            let exact = instance.get(tag).expect("a tag the table states");
+            let rounded = mvar.metric_delta(tag, &coords).unwrap();
+            assert_eq!(exact.to_i32(), rounded.to_i32(), "{tag}");
+            varied += (exact != F48Dot16::ZERO) as u32;
+        }
+        assert!(varied > 0, "no metric moved at the far end of every axis");
+    }
+
+    #[test]
+    fn a_tag_the_font_does_not_state_is_absent() {
+        let font = FontRef::new(VAR).unwrap();
+        let mvar = font.mvar().unwrap();
+        let coords = far();
+        let instance = mvar.at(&coords).unwrap();
+        assert!(instance.get(Tag::new(b"zzzz")).is_none());
+    }
+
+    #[test]
+    fn an_instance_keeps_a_fraction_the_single_accessor_drops() {
+        // `metric_delta` reports whole design units; the instance reports
+        // what the variation store computed.
+        let font = FontRef::new(VAR).unwrap();
+        let mvar = font.mvar().unwrap();
+        let coords = [F2Dot14::from_f32(0.37); 12];
+        let instance = mvar.at(&coords).unwrap();
+        let fractional = mvar
+            .value_records()
+            .iter()
+            .filter_map(|record| instance.get(record.value_tag()))
+            .any(|delta| delta.to_bits() & 0xFFFF != 0);
+        assert!(fractional, "no metric delta carried a fraction");
+    }
+}

read-fonts/src/tables/variations.rs

@@ -1644,41 +1644,49 @@ impl Iterator for ItemDeltas<'_> {
     }
 }
 
+/// The delta for a glyph's advance.
+///
+/// Keeps every bit the variation store computed. Rounding it to a whole
+/// design unit is left to a caller, and implementations differ on how.
 pub(crate) fn advance_delta(
     dsim: Option<Result<DeltaSetIndexMap, ReadError>>,
     ivs: Result<ItemVariationStore, ReadError>,
     glyph_id: GlyphId,
     coords: &[F2Dot14],
-) -> Result<Fixed, ReadError> {
+) -> Option<F48Dot16> {
     if coords.is_empty() {
-        return Ok(Fixed::ZERO);
+        return Some(F48Dot16::ZERO);
     }
     let gid = glyph_id.to_u32();
     let ix = match dsim {
-        Some(Ok(dsim)) => dsim.get(gid)?,
+        Some(Ok(dsim)) => dsim.get(gid).ok()?,
         _ => DeltaSetIndex {
             outer: 0,
             inner: gid as _,
         },
     };
-    Ok(Fixed::from_i32(ivs?.compute_delta(ix, coords)?.to_i32()))
+    ivs.ok()?.compute_delta(ix, coords).ok()
 }
 
+/// The delta for an item.
+///
+/// See [`advance_delta`]; this is the same for the mappings that require an
+/// index map rather than falling back to the glyph id.
 pub(crate) fn item_delta(
     dsim: Option<Result<DeltaSetIndexMap, ReadError>>,
     ivs: Result<ItemVariationStore, ReadError>,
     glyph_id: GlyphId,
     coords: &[F2Dot14],
-) -> Result<Fixed, ReadError> {
+) -> Option<F48Dot16> {
     if coords.is_empty() {
-        return Ok(Fixed::ZERO);
+        return Some(F48Dot16::ZERO);
     }
     let gid = glyph_id.to_u32();
     let ix = match dsim {
-        Some(Ok(dsim)) => dsim.get(gid)?,
-        _ => return Err(ReadError::NullOffset),
+        Some(Ok(dsim)) => dsim.get(gid).ok()?,
+        _ => return None,
     };
-    Ok(Fixed::from_i32(ivs?.compute_delta(ix, coords)?.to_i32()))
+    ivs.ok()?.compute_delta(ix, coords).ok()
 }
 
 #[cfg(test)]