diff --git a/fauntlet/src/font/skrifa.rs b/fauntlet/src/font/skrifa.rs index 863212500..caa9d63ac 100644 --- a/fauntlet/src/font/skrifa.rs +++ b/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()?, diff --git a/read-fonts/src/tables/hvar.rs b/read-fonts/src/tables/hvar.rs index a74f08baf..0419ca9e7 100644 --- a/read-fonts/src/tables/hvar.rs +++ b/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 { + /// 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 { 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 { + /// 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 { 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 { + /// 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 { 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)) ); } } diff --git a/read-fonts/src/tables/mvar.rs b/read-fonts/src/tables/mvar.rs index 51da5c0f3..e48e6fe17 100644 --- a/read-fonts/src/tables/mvar.rs +++ b/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> { + 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 { + 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"); + } +} diff --git a/read-fonts/src/tables/variations.rs b/read-fonts/src/tables/variations.rs index f731e12a8..0c2bbb3f9 100644 --- a/read-fonts/src/tables/variations.rs +++ b/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>, ivs: Result, glyph_id: GlyphId, coords: &[F2Dot14], -) -> Result { +) -> Option { 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>, ivs: Result, glyph_id: GlyphId, coords: &[F2Dot14], -) -> Result { +) -> Option { 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)] diff --git a/read-fonts/src/tables/vvar.rs b/read-fonts/src/tables/vvar.rs index 9be643845..d02f782a1 100644 --- a/read-fonts/src/tables/vvar.rs +++ b/read-fonts/src/tables/vvar.rs @@ -1,17 +1,19 @@ //! The [VVAR (Vertical Metrics Variation)](https://docs.microsoft.com/en-us/typography/opentype/spec/vvar) table use super::variations::{self, DeltaSetIndexMap, ItemVariationStore}; +use types::F48Dot16; include!("../../generated/generated_vvar.rs"); impl Vvar<'_> { - /// Returns the advance height delta for the specified glyph identifier and - /// normalized variation coordinates. - pub fn advance_height_delta( - &self, - glyph_id: GlyphId, - coords: &[F2Dot14], - ) -> Result { + /// Returns the change a location makes to the advance height 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 { variations::advance_delta( self.advance_height_mapping(), self.item_variation_store(), @@ -20,9 +22,15 @@ impl Vvar<'_> { ) } - /// Returns the top side bearing delta for the specified glyph identifier and - /// normalized variation coordinates. - pub fn tsb_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Result { + /// Returns the change a location makes to the top 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 tsb_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Option { variations::item_delta( self.tsb_mapping(), self.item_variation_store(), @@ -31,9 +39,15 @@ impl Vvar<'_> { ) } - /// Returns the bottom side bearing delta for the specified glyph identifier and - /// normalized variation coordinates. - pub fn bsb_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Result { + /// Returns the change a location makes to the bottom 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 bsb_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Option { variations::item_delta( self.bsb_mapping(), self.item_variation_store(), @@ -42,9 +56,18 @@ impl Vvar<'_> { ) } - /// Returns the vertical origin delta for the specified glyph identifier and - /// normalized variation coordinates. - pub fn v_org_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Result { + /// Returns the change a location makes to the y coordinate of a glyph's + /// vertical origin. + /// + /// The x coordinate is not stated by any table: it is half the advance + /// width, and does not vary here. + /// + /// 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 v_origin_y_delta(&self, glyph_id: GlyphId, coords: &[F2Dot14]) -> Option { variations::item_delta( self.v_org_mapping(), self.item_variation_store(), @@ -53,3 +76,53 @@ impl Vvar<'_> { ) } } + +#[cfg(test)] +mod tests { + use crate::{FontRef, TableProvider}; + use types::{F2Dot14, F48Dot16, GlyphId}; + + /// The only fixture stating `VVAR`. Every one of its mappings resolves to + /// zero at every location, so this covers the shape of the accessors + /// rather than their arithmetic; `HVAR` carries that, over the same code + /// in `variations`. + const VAR: &[u8] = font_test_data::ift::CFF2_FONT; + + #[test] + fn a_default_location_moves_nothing() { + let font = FontRef::new(VAR).unwrap(); + let vvar = font.vvar().unwrap(); + let gid = GlyphId::new(1); + assert_eq!(vvar.advance_delta(gid, &[]), Some(F48Dot16::ZERO)); + } + + #[test] + fn every_glyph_answers_at_a_location() { + let font = FontRef::new(VAR).unwrap(); + let vvar = font.vvar().unwrap(); + let num_glyphs = font.maxp().unwrap().num_glyphs() as u32; + for coord in [-1.0, -0.4, 0.25, 1.0] { + let coords = [F2Dot14::from_f32(coord)]; + for gid in (0..num_glyphs).map(GlyphId::new) { + assert!( + vvar.advance_delta(gid, &coords).is_some(), + "glyph {gid} at {coord}" + ); + } + } + } + + #[test] + fn a_mapping_the_font_does_not_state_is_absent() { + // This font maps advance heights and vertical origins, but neither + // side bearing. + let font = FontRef::new(VAR).unwrap(); + let vvar = font.vvar().unwrap(); + let gid = GlyphId::new(1); + let coords = [F2Dot14::from_f32(-1.0)]; + assert!(vvar.advance_delta(gid, &coords).is_some()); + assert!(vvar.v_origin_y_delta(gid, &coords).is_some()); + assert_eq!(vvar.tsb_delta(gid, &coords), None); + assert_eq!(vvar.bsb_delta(gid, &coords), None); + } +} diff --git a/skrifa/src/metrics.rs b/skrifa/src/metrics.rs index 695171e1a..944f9261d 100644 --- a/skrifa/src/metrics.rs +++ b/skrifa/src/metrics.rs @@ -299,10 +299,11 @@ impl<'a> GlyphMetrics<'a> { .unwrap_or(self.default_advance_width) as i32; if let Some(hvar) = &self.hvar { advance += hvar - .advance_width_delta(glyph_id, self.coords) - // FreeType truncates metric deltas... + .advance_delta(glyph_id, self.coords) + // The delta is exact, and this rounds it to a whole design + // unit before it is added. // https://github.com/freetype/freetype/blob/7838c78f53f206ac5b8e9cefde548aa81cb00cf4/src/truetype/ttgxvar.c#L1027 - .map(|delta| delta.to_f64() as i32) + .map(|delta| delta.to_i32()) .unwrap_or(0); } else if self.gvar.is_some() { advance += self.metric_deltas_from_gvar(glyph_id).unwrap_or_default()[1]; @@ -335,9 +336,10 @@ impl<'a> GlyphMetrics<'a> { if let Some(hvar) = &self.hvar { lsb += hvar .lsb_delta(glyph_id, self.coords) - // FreeType truncates metric deltas... + // The delta is exact, and this rounds it to a whole design + // unit before it is added. // https://github.com/freetype/freetype/blob/7838c78f53f206ac5b8e9cefde548aa81cb00cf4/src/truetype/ttgxvar.c#L1027 - .map(|delta| delta.to_f64() as i32) + .map(|delta| delta.to_i32()) .unwrap_or(0); } else if self.gvar.is_some() { lsb += self.metric_deltas_from_gvar(glyph_id).unwrap_or_default()[0]; diff --git a/skrifa/src/outline/metrics.rs b/skrifa/src/outline/metrics.rs index 58749ae76..c136f35b3 100644 --- a/skrifa/src/outline/metrics.rs +++ b/skrifa/src/outline/metrics.rs @@ -28,7 +28,7 @@ impl<'a> GlyphHMetrics<'a> { let mut advance = self.hmtx.advance(gid).unwrap_or_default() as i32; if let (false, Some(hvar)) = (coords.is_empty(), &self.hvar) { advance += hvar - .advance_width_delta(gid, coords) + .advance_delta(gid, coords) .map(|delta| delta.to_i32()) .unwrap_or(0); }