[skrifa] remove ugly OutlinesCommon abstraction (#1281)

Just embeds the `FontRef` and a new `GlyphHMetrics` type for common access to horizontal glyph metrics. Also adds a method to retrieving the cvt to `TableProvider`.

No functional changes.

read-fonts/src/table_provider.rs

@@ -1,6 +1,6 @@
 //! a trait for things that can serve font tables
 
-use types::Tag;
+use types::{BigEndian, Tag};
 
 use crate::{tables, FontData, FontRead, ReadError};
 
@@ -120,6 +120,13 @@ pub trait TableProvider<'a> {
         self.expect_table()
     }
 
+    /// Returns the array of entries for the control value table which is used
+    /// for TrueType hinting.
+    fn cvt(&self) -> Result<&'a [BigEndian<i16>], ReadError> {
+        let table_data = self.expect_data_for_tag(Tag::new(b"cvt "))?;
+        table_data.read_array(0..table_data.len())
+    }
+
     fn cvar(&self) -> Result<tables::cvar::Cvar<'a>, ReadError> {
         self.expect_table()
     }

skrifa/src/outline/autohint/instance.rs

@@ -36,13 +36,12 @@ pub struct GlyphStyles(Arc<GlyphStyleMap>);
 impl GlyphStyles {
     /// Precomputes the full set of glyph styles for the given outlines.
     pub fn new(outlines: &OutlineGlyphCollection) -> Self {
-        if let Some(outlines) = outlines.common() {
-            let glyph_count = outlines
-                .font
+        if let Some(font) = outlines.font() {
+            let glyph_count = font
                 .maxp()
                 .map(|maxp| maxp.num_glyphs() as u32)
                 .unwrap_or_default();
-            let shaper = Shaper::new(&outlines.font, SHAPER_MODE);
+            let shaper = Shaper::new(font, SHAPER_MODE);
             Self(Arc::new(GlyphStyleMap::new(glyph_count, &shaper)))
         } else {
             Self(Default::default())
@@ -99,7 +98,7 @@ impl Instance {
         path_style: PathStyle,
         pen: &mut impl OutlinePen,
     ) -> Result<AdjustedMetrics, DrawError> {
-        let common = glyph.outlines_common();
+        let font = glyph.font();
         let glyph_id = glyph.glyph_id();
         let style = self
             .styles
@@ -108,7 +107,7 @@ impl Instance {
             .ok_or(DrawError::GlyphNotFound(glyph_id))?;
         let metrics = self
             .metrics
-            .get(&common.font, coords, SHAPER_MODE, &self.styles.0, glyph_id)
+            .get(font, coords, SHAPER_MODE, &self.styles.0, glyph_id)
             .ok_or(DrawError::GlyphNotFound(glyph_id))?;
         let units_per_em = glyph.units_per_em() as i32;
         let scale = Scale::new(

skrifa/src/outline/cff/hint.rs

@@ -1108,9 +1108,8 @@ mod tests {
     use read_fonts::{tables::postscript::charstring::CommandSink, types::F2Dot14, FontRef};
 
     use super::{
-        super::OutlinesCommon, BlueZone, Blues, Fixed, Hint, HintMap, HintMask, HintParams,
-        HintState, HintingSink, StemHint, GHOST_BOTTOM, GHOST_TOP, HINT_MASK_SIZE, LOCKED,
-        PAIR_BOTTOM, PAIR_TOP,
+        BlueZone, Blues, Fixed, Hint, HintMap, HintMask, HintParams, HintState, HintingSink,
+        StemHint, GHOST_BOTTOM, GHOST_TOP, HINT_MASK_SIZE, LOCKED, PAIR_BOTTOM, PAIR_TOP,
     };
 
     fn make_hint_state() -> HintState {
@@ -1306,8 +1305,7 @@ mod tests {
     #[test]
     fn hint_mapping() {
         let font = FontRef::new(font_test_data::CANTARELL_VF_TRIMMED).unwrap();
-        let base = OutlinesCommon::new(&font).unwrap();
-        let cff_font = super::super::Outlines::new(&base).unwrap();
+        let cff_font = super::super::Outlines::new(&font).unwrap();
         let state = cff_font
             .subfont(0, Some(8.0), &[F2Dot14::from_f32(-1.0); 2])
             .unwrap()

skrifa/src/outline/cff/mod.rs

@@ -2,8 +2,9 @@
 
 mod hint;
 
-use super::{common::OutlinesCommon, OutlinePen};
+use super::{GlyphHMetrics, OutlinePen};
 use hint::{HintParams, HintState, HintingSink};
+use raw::FontRef;
 use read_fonts::{
     tables::{
         postscript::{
@@ -36,7 +37,8 @@ use std::ops::Range;
 /// subfont for that glyph.
 #[derive(Clone)]
 pub(crate) struct Outlines<'a> {
-    pub(crate) common: OutlinesCommon<'a>,
+    pub(crate) font: FontRef<'a>,
+    pub(crate) glyph_metrics: GlyphHMetrics<'a>,
     offset_data: FontData<'a>,
     global_subrs: Index<'a>,
     top_dict: TopDict<'a>,
@@ -49,13 +51,14 @@ impl<'a> Outlines<'a> {
     ///
     /// This will choose an underlying CFF2 or CFF table from the font, in that
     /// order.
-    pub fn new(common: &OutlinesCommon<'a>) -> Option<Self> {
-        let units_per_em = common.font.head().ok()?.units_per_em();
-        Self::from_cff2(common, units_per_em).or_else(|| Self::from_cff(common, units_per_em))
+    pub fn new(font: &FontRef<'a>) -> Option<Self> {
+        let units_per_em = font.head().ok()?.units_per_em();
+        Self::from_cff2(font, units_per_em).or_else(|| Self::from_cff(font, units_per_em))
     }
 
-    pub fn from_cff(common: &OutlinesCommon<'a>, units_per_em: u16) -> Option<Self> {
-        let cff1 = common.font.cff().ok()?;
+    pub fn from_cff(font: &FontRef<'a>, units_per_em: u16) -> Option<Self> {
+        let cff1 = font.cff().ok()?;
+        let glyph_metrics = GlyphHMetrics::new(font)?;
         // "The Name INDEX in the CFF data must contain only one entry;
         // that is, there must be only one font in the CFF FontSet"
         // So we always pass 0 for Top DICT index when reading from an
@@ -64,7 +67,8 @@ impl<'a> Outlines<'a> {
         let top_dict_data = cff1.top_dicts().get(0).ok()?;
         let top_dict = TopDict::new(cff1.offset_data().as_bytes(), top_dict_data, false).ok()?;
         Some(Self {
-            common: common.clone(),
+            font: font.clone(),
+            glyph_metrics,
             offset_data: cff1.offset_data(),
             global_subrs: cff1.global_subrs().into(),
             top_dict,
@@ -73,12 +77,14 @@ impl<'a> Outlines<'a> {
         })
     }
 
-    pub fn from_cff2(common: &OutlinesCommon<'a>, units_per_em: u16) -> Option<Self> {
-        let cff2 = common.font.cff2().ok()?;
+    pub fn from_cff2(font: &FontRef<'a>, units_per_em: u16) -> Option<Self> {
+        let cff2 = font.cff2().ok()?;
+        let glyph_metrics = GlyphHMetrics::new(font)?;
         let table_data = cff2.offset_data().as_bytes();
         let top_dict = TopDict::new(table_data, cff2.top_dict_data(), true).ok()?;
         Some(Self {
-            common: common.clone(),
+            font: font.clone(),
+            glyph_metrics,
             offset_data: cff2.offset_data(),
             global_subrs: cff2.global_subrs().into(),
             top_dict,
@@ -656,8 +662,7 @@ mod tests {
     #[test]
     fn read_cff_static() {
         let font = FontRef::new(font_test_data::NOTO_SERIF_DISPLAY_TRIMMED).unwrap();
-        let base = OutlinesCommon::new(&font).unwrap();
-        let cff = Outlines::new(&base).unwrap();
+        let cff = Outlines::new(&font).unwrap();
         assert!(!cff.is_cff2());
         assert!(cff.top_dict.var_store.is_none());
         assert!(cff.top_dict.font_dicts.count() == 0);
@@ -671,8 +676,7 @@ mod tests {
     #[test]
     fn read_cff2_static() {
         let font = FontRef::new(font_test_data::CANTARELL_VF_TRIMMED).unwrap();
-        let base = OutlinesCommon::new(&font).unwrap();
-        let cff = Outlines::new(&base).unwrap();
+        let cff = Outlines::new(&font).unwrap();
         assert!(cff.is_cff2());
         assert!(cff.top_dict.var_store.is_some());
         assert!(cff.top_dict.font_dicts.count() != 0);
@@ -741,8 +745,7 @@ mod tests {
     #[test]
     fn empty_private_dict() {
         let font = FontRef::new(font_test_data::MATERIAL_ICONS_SUBSET).unwrap();
-        let common = OutlinesCommon::new(&font).unwrap();
-        let outlines = super::Outlines::new(&common).unwrap();
+        let outlines = super::Outlines::new(&font).unwrap();
         assert!(outlines.top_dict.private_dict_range.is_empty());
         assert!(outlines.private_dict_range(0).unwrap().is_empty());
     }
@@ -810,8 +813,7 @@ mod tests {
         use super::super::testing;
         let font = FontRef::new(font_data).unwrap();
         let expected_outlines = testing::parse_glyph_outlines(expected_outlines);
-        let base = OutlinesCommon::new(&font).unwrap();
-        let outlines = super::Outlines::new(&base).unwrap();
+        let outlines = super::Outlines::new(&font).unwrap();
         let mut path = testing::Path::default();
         for expected_outline in &expected_outlines {
             if expected_outline.size == 0.0 && !expected_outline.coords.is_empty() {

skrifa/src/outline/glyf/hint/instance.rs

@@ -191,8 +191,8 @@ impl HintInstance {
             Definition::default(),
         );
         self.cvt.clear();
-        let cvt = outlines.common.cvt();
-        if let Ok(cvar) = outlines.common.font.cvar() {
+        let cvt = outlines.font.cvt().unwrap_or_default();
+        if let Ok(cvar) = outlines.font.cvar() {
             // First accumulate all the deltas in 16.16
             self.cvt.resize(cvt.len(), 0);
             let _ = cvar.deltas(axis_count, coords, &mut self.cvt);
@@ -237,17 +237,13 @@ impl HintInstance {
 
 #[cfg(test)]
 mod tests {
-    use super::{
-        super::super::{Outlines, OutlinesCommon},
-        HintInstance,
-    };
+    use super::{super::super::Outlines, HintInstance};
     use read_fonts::{types::F2Dot14, FontRef};
 
     #[test]
     fn scaled_cvar_cvt() {
         let font = FontRef::new(font_test_data::CVAR).unwrap();
-        let base = OutlinesCommon::new(&font).unwrap();
-        let outlines = Outlines::new(&base).unwrap();
+        let outlines = Outlines::new(&font).unwrap();
         let mut instance = HintInstance::default();
         let coords = [0.5, -0.5].map(F2Dot14::from_f32);
         let ppem = 16;

skrifa/src/outline/glyf/mod.rs

@@ -11,8 +11,9 @@ use core_maths::CoreFloat;
 
 pub use hint::{HintError, HintInstance, HintOutline};
 pub use outline::{Outline, ScaledOutline};
+use raw::FontRef;
 
-use super::{common::OutlinesCommon, DrawError, Hinting};
+use super::{DrawError, GlyphHMetrics, Hinting};
 use crate::GLYF_COMPOSITE_RECURSION_LIMIT;
 use memory::{FreeTypeOutlineMemory, HarfBuzzOutlineMemory};
 
@@ -35,7 +36,8 @@ pub const PHANTOM_POINT_COUNT: usize = 4;
 /// Scaler state for TrueType outlines.
 #[derive(Clone)]
 pub struct Outlines<'a> {
-    pub(crate) common: OutlinesCommon<'a>,
+    pub(crate) font: FontRef<'a>,
+    pub(crate) glyph_metrics: GlyphHMetrics<'a>,
     loca: Loca<'a>,
     glyf: Glyf<'a>,
     gvar: Option<Gvar<'a>>,
@@ -56,9 +58,11 @@ pub struct Outlines<'a> {
 }
 
 impl<'a> Outlines<'a> {
-    pub fn new(common: &OutlinesCommon<'a>) -> Option<Self> {
-        let font = &common.font;
-        let has_var_lsb = common
+    pub fn new(font: &FontRef<'a>) -> Option<Self> {
+        let loca = font.loca(None).ok()?;
+        let glyf = font.glyf().ok()?;
+        let glyph_metrics = GlyphHMetrics::new(font)?;
+        let has_var_lsb = glyph_metrics
             .hvar
             .as_ref()
             .map(|hvar| hvar.lsb_mapping().is_some())
@@ -108,11 +112,12 @@ impl<'a> Outlines<'a> {
         // Copy FreeType's logic on whether to use the interpreter:
         // <https://gitlab.freedesktop.org/freetype/freetype/-/blob/57617782464411201ce7bbc93b086c1b4d7d84a5/src/base/ftobjs.c#L1001>
         let prefer_interpreter = !(max_instructions == 0 && fpgm.is_empty() && prep.is_empty());
-        let cvt_len = common.cvt().len() as u32;
+        let cvt_len = font.cvt().map(|cvt| cvt.len() as u32).unwrap_or_default();
         Some(Self {
-            common: common.clone(),
-            loca: font.loca(None).ok()?,
-            glyf: font.glyf().ok()?,
+            font: font.clone(),
+            glyph_metrics,
+            loca,
+            glyf,
             gvar: font.gvar().ok(),
             hdmx: font.hdmx().ok(),
             fpgm,
@@ -283,8 +288,8 @@ trait Scaler {
         };
         let outlines = self.outlines();
         let coords: &[F2Dot14] = self.coords();
-        let lsb = outlines.common.lsb(glyph_id, coords);
-        let advance = outlines.common.advance_width(glyph_id, coords);
+        let lsb = outlines.glyph_metrics.lsb(glyph_id, coords);
+        let advance = outlines.glyph_metrics.advance_width(glyph_id, coords);
         let [ascent, descent] = outlines.os2_vmetrics.map(|x| x as i32);
         let tsb = ascent - bounds[3] as i32;
         let vadvance = ascent - descent;
@@ -491,7 +496,7 @@ impl Scaler for FreeTypeScaler<'_> {
         // <https://gitlab.freedesktop.org/freetype/freetype/-/blob/57617782464411201ce7bbc93b086c1b4d7d84a5/src/truetype/ttgload.c#L1572>
         let scale = self.scale;
         let mut unscaled = self.phantom.map(|point| point.map(|x| x.to_bits()));
-        if self.outlines.common.hvar.is_none()
+        if self.outlines.glyph_metrics.hvar.is_none()
             && self.outlines.gvar.is_some()
             && !self.coords.is_empty()
         {
@@ -643,7 +648,7 @@ impl Scaler for FreeTypeScaler<'_> {
             }
         }
         // Commit our potentially modified phantom points.
-        if self.outlines.common.hvar.is_some() && self.is_hinted {
+        if self.outlines.glyph_metrics.hvar.is_some() && self.is_hinted {
             self.phantom[0] *= self.scale;
             self.phantom[1] *= self.scale;
         } else {
@@ -1012,7 +1017,7 @@ impl Scaler for HarfBuzzScaler<'_> {
         // FreeType version above but changed to use floating point
         let scale = self.scale.to_f32();
         let mut unscaled = self.phantom;
-        if self.outlines.common.hvar.is_none()
+        if self.outlines.glyph_metrics.hvar.is_none()
             && self.outlines.gvar.is_some()
             && !self.coords.is_empty()
         {
@@ -1281,7 +1286,7 @@ mod tests {
     #[test]
     fn overlap_flags() {
         let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap();
-        let scaler = Outlines::new(&OutlinesCommon::new(&font).unwrap()).unwrap();
+        let scaler = Outlines::new(&font).unwrap();
         let glyph_count = font.maxp().unwrap().num_glyphs();
         // GID 2 is a composite glyph with the overlap bit on a component
         // GID 3 is a simple glyph with the overlap bit on the first flag
@@ -1298,20 +1303,20 @@ mod tests {
     fn interpreter_preference() {
         // no instructions in this font...
         let font = FontRef::new(font_test_data::COLRV0V1).unwrap();
-        let outlines = Outlines::new(&OutlinesCommon::new(&font).unwrap()).unwrap();
+        let outlines = Outlines::new(&font).unwrap();
         // thus no preference for the interpreter
         assert!(!outlines.prefer_interpreter());
         // but this one has instructions...
         let font = FontRef::new(font_test_data::TTHINT_SUBSET).unwrap();
-        let outlines = Outlines::new(&OutlinesCommon::new(&font).unwrap()).unwrap();
+        let outlines = Outlines::new(&font).unwrap();
         // so let's use it
         assert!(outlines.prefer_interpreter());
     }
 
     #[test]
     fn empty_glyph_advance() {
         let font = FontRef::new(font_test_data::HVAR_WITH_TRUNCATED_ADVANCE_INDEX_MAP).unwrap();
-        let mut outlines = Outlines::new(&OutlinesCommon::new(&font).unwrap()).unwrap();
+        let mut outlines = Outlines::new(&font).unwrap();
         let coords = [F2Dot14::from_f32(0.5)];
         let ppem = Some(24.0);
         let gid = font.charmap().map(' ').unwrap();
@@ -1324,7 +1329,7 @@ mod tests {
         let scaled = scaler.scale(&outline.glyph, gid).unwrap();
         let advance_hvar = scaled.adjusted_advance_width();
         // Set HVAR table to None to force loading metrics from gvar
-        outlines.common.hvar = None;
+        outlines.glyph_metrics.hvar = None;
         let scaler =
             FreeTypeScaler::unhinted(&outlines, &outline, &mut buf, ppem, &coords).unwrap();
         let scaled = scaler.scale(&outline.glyph, gid).unwrap();
@@ -1337,7 +1342,7 @@ mod tests {
     #[test]
     fn empty_glyphs_have_phantom_points_too() {
         let font = FontRef::new(font_test_data::HVAR_WITH_TRUNCATED_ADVANCE_INDEX_MAP).unwrap();
-        let outlines = Outlines::new(&OutlinesCommon::new(&font).unwrap()).unwrap();
+        let outlines = Outlines::new(&font).unwrap();
         let gid = font.charmap().map(' ').unwrap();
         let outline = outlines.outline(gid).unwrap();
         assert!(outline.glyph.is_none());

skrifa/src/outline/hint.rs

@@ -368,7 +368,7 @@ impl HintingInstance {
                 OutlineCollectionKind::None => {}
             },
             Engine::Auto(styles) => {
-                let Some(font) = outlines.common().map(|scaler| &scaler.font) else {
+                let Some(font) = outlines.font() else {
                     return Ok(());
                 };
                 let instance = autohint::Instance::new(