[skrifa] autohint bug fixes to match FT (#1145)

This is a largish collection of small changes that bring the autohinter into a state where we match with FreeType for at least the Google Fonts and Noto collections.

Also updates fauntlet to use the new hinting API.

fauntlet/src/compare_glyphs.rs

@@ -1,3 +1,5 @@
+use crate::Hinting;
+
 use super::{FreeTypeInstance, InstanceOptions, RegularizingPen, SkrifaInstance};
 use skrifa::{outline::pen::PathElement, GlyphId};
 use std::{io::Write, path::Path};
@@ -13,6 +15,21 @@ pub fn compare_glyphs(
         // Don't run on bitmap fonts (yet)
         return true;
     }
+    if let Some(Hinting::Auto(_)) = options.hinting {
+        // This font is a pathological case for autohinting due to the
+        // extreme number of generated segments and edges. To be
+        // precise, it takes longer to test this single font than the
+        // entire remainder of the Google Fonts corpus so we skip it
+        // here.
+        // Discussion at <https://github.com/googlefonts/fontations/issues/1147>
+        if ft_instance
+            .family_name()
+            .unwrap_or_default()
+            .contains("Handjet")
+        {
+            return true;
+        }
+    }
     let glyph_count = skrifa_instance.glyph_count();
     let is_scaled = options.ppem != 0;
 
@@ -86,6 +103,7 @@ pub fn compare_glyphs(
             if exit_on_fail {
                 std::process::exit(1);
             }
+            break;
         }
     }
     ok

fauntlet/src/font/freetype.rs

@@ -3,26 +3,12 @@ use freetype::{
     ffi::{FT_Long, FT_Vector},
     Face, Library,
 };
-use skrifa::{
-    outline::{HintingMode, LcdLayout, OutlinePen},
-    GlyphId,
-};
+use skrifa::{outline::OutlinePen, GlyphId};
 
 use std::ffi::{c_int, c_void};
 
 use super::{InstanceOptions, SharedFontData};
 
-fn load_flags_from_hinting(mode: HintingMode) -> LoadFlag {
-    match mode {
-        HintingMode::Strong => LoadFlag::TARGET_MONO,
-        HintingMode::Smooth { lcd_subpixel, .. } => match lcd_subpixel {
-            Some(LcdLayout::Horizontal) => LoadFlag::TARGET_LCD,
-            Some(LcdLayout::Vertical) => LoadFlag::TARGET_LCD_V,
-            None => LoadFlag::TARGET_NORMAL,
-        },
-    }
-}
-
 pub struct FreeTypeInstance {
     face: Face<SharedFontData>,
     load_flags: LoadFlag,
@@ -37,10 +23,10 @@ impl FreeTypeInstance {
         let mut face = library
             .new_memory_face2(data.clone(), options.index as isize)
             .ok()?;
-        let mut load_flags = LoadFlag::NO_AUTOHINT | LoadFlag::NO_BITMAP;
+        let mut load_flags = LoadFlag::NO_BITMAP;
         match options.hinting {
             None => load_flags |= LoadFlag::NO_HINTING,
-            Some(hinting) => load_flags |= load_flags_from_hinting(hinting),
+            Some(hinting) => load_flags |= hinting.freetype_load_flags(),
         };
         if options.ppem != 0 {
             face.set_pixel_sizes(options.ppem, options.ppem).ok()?;
@@ -76,6 +62,10 @@ impl FreeTypeInstance {
         Some(Self { face, load_flags })
     }
 
+    pub fn family_name(&self) -> Option<String> {
+        self.face.family_name()
+    }
+
     pub fn is_scalable(&self) -> bool {
         self.face.is_scalable()
     }

fauntlet/src/font/mod.rs

@@ -4,9 +4,9 @@ use std::{
     sync::Arc,
 };
 
-use ::freetype::Library;
+use ::freetype::{face::LoadFlag, Library};
 use ::skrifa::{
-    outline::HintingMode,
+    outline::{HintingOptions, SmoothMode, Target},
     raw::{types::F2Dot14, FileRef, FontRef, TableProvider},
 };
 
@@ -16,21 +16,76 @@ mod skrifa;
 pub use freetype::FreeTypeInstance;
 pub use skrifa::SkrifaInstance;
 
+#[derive(Copy, Clone, PartialEq, Eq, Default, Debug)]
+pub enum HintingTarget {
+    #[default]
+    Normal,
+    Light,
+    Lcd,
+    VerticalLcd,
+    Mono,
+}
+
+impl HintingTarget {
+    pub fn to_skrifa_target(self) -> Target {
+        match self {
+            Self::Normal => SmoothMode::Normal.into(),
+            Self::Light => SmoothMode::Light.into(),
+            Self::Lcd => SmoothMode::Lcd.into(),
+            Self::VerticalLcd => SmoothMode::VerticalLcd.into(),
+            Self::Mono => Target::Mono,
+        }
+    }
+
+    pub fn to_freetype_load_flags(self) -> LoadFlag {
+        match self {
+            Self::Normal => LoadFlag::TARGET_NORMAL,
+            Self::Light => LoadFlag::TARGET_LIGHT,
+            Self::Lcd => LoadFlag::TARGET_LCD,
+            Self::VerticalLcd => LoadFlag::TARGET_LCD_V,
+            Self::Mono => LoadFlag::TARGET_MONO,
+        }
+    }
+}
+
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub enum Hinting {
+    Interpreter(HintingTarget),
+    Auto(HintingTarget),
+}
+
+impl Hinting {
+    pub fn skrifa_options(self) -> HintingOptions {
+        match self {
+            Self::Interpreter(target) => HintingOptions {
+                engine: ::skrifa::outline::Engine::Interpreter,
+                target: target.to_skrifa_target(),
+            },
+            Self::Auto(target) => HintingOptions {
+                engine: ::skrifa::outline::Engine::Auto(None),
+                target: target.to_skrifa_target(),
+            },
+        }
+    }
+
+    pub fn freetype_load_flags(self) -> LoadFlag {
+        match self {
+            Self::Interpreter(target) => LoadFlag::NO_AUTOHINT | target.to_freetype_load_flags(),
+            Self::Auto(target) => LoadFlag::FORCE_AUTOHINT | target.to_freetype_load_flags(),
+        }
+    }
+}
+
 #[derive(Copy, Clone, Debug)]
 pub struct InstanceOptions<'a> {
     pub index: usize,
     pub ppem: u32,
     pub coords: &'a [F2Dot14],
-    pub hinting: Option<HintingMode>,
+    pub hinting: Option<Hinting>,
 }
 
 impl<'a> InstanceOptions<'a> {
-    pub fn new(
-        index: usize,
-        ppem: u32,
-        coords: &'a [F2Dot14],
-        hinting: Option<HintingMode>,
-    ) -> Self {
+    pub fn new(index: usize, ppem: u32, coords: &'a [F2Dot14], hinting: Option<Hinting>) -> Self {
         Self {
             index,
             ppem,

fauntlet/src/font/skrifa.rs

@@ -27,8 +27,13 @@ impl<'a> SkrifaInstance<'a> {
         let outlines = font.outline_glyphs();
         let hinter = if options.ppem != 0 && options.hinting.is_some() {
             Some(
-                HintingInstance::new(&outlines, size, options.coords, options.hinting.unwrap())
-                    .ok()?,
+                HintingInstance::new(
+                    &outlines,
+                    size,
+                    options.coords,
+                    options.hinting.unwrap().skrifa_options(),
+                )
+                .ok()?,
             )
         } else {
             None

fauntlet/src/lib.rs

@@ -3,5 +3,7 @@ mod font;
 mod pen;
 
 pub use compare_glyphs::compare_glyphs;
-pub use font::{Font, FreeTypeInstance, InstanceOptions, SharedFontData, SkrifaInstance};
+pub use font::{
+    Font, FreeTypeInstance, Hinting, HintingTarget, InstanceOptions, SharedFontData, SkrifaInstance,
+};
 pub use pen::RegularizingPen;