[skrifa] reduce pedantry

Propagate the `is_pedantic` flag to a few places and reduce the strictness in various parts of the hinter based on that flag.

With these changes we match FT hinting for all of Google Fonts and Noto.

fauntlet/src/font/skrifa.rs

@@ -1,12 +1,9 @@
 use ::skrifa::{
+    outline::{DrawError, DrawSettings, EmbeddedHintingInstance},
     prelude::{LocationRef, Size},
-    raw::{types::Pen, FontRef, TableProvider},
-    GlyphId, MetadataProvider,
-};
-use skrifa::{
-    outline::{DrawError, EmbeddedHintingInstance},
     raw::types::F2Dot14,
-    OutlineGlyphCollection,
+    raw::{types::Pen, FontRef, TableProvider},
+    GlyphId, MetadataProvider, OutlineGlyphCollection,
 };
 
 use super::{InstanceOptions, SharedFontData};
@@ -36,7 +33,7 @@ impl<'a> SkrifaInstance<'a> {
                     options.coords,
                     options.hinting.unwrap(),
                 )
-                .unwrap(),
+                .ok()?,
             )
         } else {
             None
@@ -69,7 +66,7 @@ impl<'a> SkrifaInstance<'a> {
             .get(glyph_id)
             .ok_or(DrawError::GlyphNotFound(glyph_id))?;
         if let Some(hinter) = self.hinter.as_ref() {
-            outline.draw(hinter, pen)?;
+            outline.draw(DrawSettings::hinted(hinter, false), pen)?;
         } else {
             outline.draw((self.size, self.coords.as_slice()), pen)?;
         }

skrifa/src/outline/embedded_hinting.rs

@@ -202,7 +202,7 @@ impl EmbeddedHintingInstance {
                         outline,
                         ppem,
                         coords,
-                        |mut hint_outline| instance.hint(glyf, &mut hint_outline),
+                        |mut hint_outline| instance.hint(glyf, &mut hint_outline, is_pedantic),
                         is_pedantic,
                     )?;
                     scaled_outline.to_path(path_style, pen)?;

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

@@ -88,6 +88,14 @@ impl<'a> CowSlice<'a> {
         *self.data_mut.get_mut(index)? = value;
         Some(())
     }
+
+    pub fn len(&self) -> usize {
+        if self.use_mut {
+            self.data_mut.len()
+        } else {
+            self.data.len()
+        }
+    }
 }
 
 /// Error returned when the sizes of the immutable and mutable buffers

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

@@ -21,6 +21,10 @@ impl<'a> Cvt<'a> {
             .set(index, value.to_bits())
             .ok_or(HintErrorKind::InvalidCvtIndex(index))
     }
+
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
 }
 
 impl<'a> From<CowSlice<'a>> for Cvt<'a> {

skrifa/src/outline/glyf/hint/engine/data.rs

@@ -28,6 +28,10 @@ impl<'a> Engine<'a> {
     pub(super) fn op_gc(&mut self, opcode: u8) -> OpResult {
         let p = self.value_stack.pop_usize()?;
         let gs = &mut self.graphics;
+        if !gs.is_pedantic && !gs.in_bounds([(gs.zp2, p)]) {
+            self.value_stack.push(0)?;
+            return Ok(());
+        }
         let value = if (opcode & 1) != 0 {
             gs.dual_project(gs.zp2().original(p)?, Default::default())
         } else {
@@ -86,6 +90,10 @@ impl<'a> Engine<'a> {
         let p1 = self.value_stack.pop_usize()?;
         let p2 = self.value_stack.pop_usize()?;
         let gs = &self.graphics;
+        if !gs.is_pedantic && !gs.in_bounds([(gs.zp0, p2), (gs.zp1, p1)]) {
+            self.value_stack.push(0)?;
+            return Ok(());
+        }
         let distance = if (opcode & 1) != 0 {
             // measure in grid fitted outline
             gs.project(gs.zp0().point(p2)?, gs.zp1().point(p1)?)

skrifa/src/outline/glyf/hint/engine/dispatch.rs

@@ -11,16 +11,17 @@ const MAX_RUN_INSTRUCTIONS: usize = 1_000_000;
 
 impl<'a> Engine<'a> {
     /// Resets state for the specified program and executes all instructions.
-    pub fn run_program(&mut self, program: Program) -> Result<(), HintError> {
-        self.reset(program);
+    pub fn run_program(&mut self, program: Program, is_pedantic: bool) -> Result<(), HintError> {
+        self.reset(program, is_pedantic);
         self.run()
     }
 
     /// Set internal state for running the specified program.
-    pub fn reset(&mut self, program: Program) {
+    pub fn reset(&mut self, program: Program, is_pedantic: bool) {
         self.program.reset(program);
         // Reset overall graphics state, keeping the retained bits.
         self.graphics.reset();
+        self.graphics.is_pedantic = is_pedantic;
         self.loop_budget.reset();
         // Program specific setup.
         match program {
@@ -62,6 +63,7 @@ impl<'a> Engine<'a> {
                     program: self.program.current,
                     glyph_id: None,
                     pc: ins.pc,
+                    opcode: Some(ins.opcode),
                     kind: HintErrorKind::ExceededExecutionBudget,
                 });
             }
@@ -76,6 +78,7 @@ impl<'a> Engine<'a> {
             program: self.program.current,
             glyph_id: None,
             pc: self.program.decoder.pc,
+            opcode: None,
             kind: HintErrorKind::UnexpectedEndOfBytecode,
         }))
     }
@@ -87,6 +90,7 @@ impl<'a> Engine<'a> {
             program: current_program,
             glyph_id: None,
             pc: ins.pc,
+            opcode: Some(ins.opcode),
             kind,
         })
     }

skrifa/src/outline/glyf/hint/engine/mod.rs

@@ -230,7 +230,7 @@ mod mock {
                 graphics: graphics_state,
                 cvt: CowSlice::new_mut(cvt).into(),
                 storage: CowSlice::new_mut(storage).into(),
-                value_stack: ValueStack::new(&mut self.value_stack),
+                value_stack: ValueStack::new(&mut self.value_stack, false),
                 program: ProgramState::new(font_code, cv_code, glyph_code, Program::Font),
                 loop_budget: LoopBudget {
                     limit: 10,

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

@@ -138,6 +138,9 @@ impl<'a> Engine<'a> {
     pub(super) fn op_shc(&mut self, opcode: u8) -> OpResult {
         let gs = &mut self.graphics;
         let contour_ix = self.value_stack.pop_usize()?;
+        if !gs.is_pedantic && contour_ix >= gs.zp2().contours.len() {
+            return Ok(());
+        }
         let point_disp = gs.point_displacement(opcode)?;
         let start = if contour_ix != 0 {
             gs.zp2().contour(contour_ix - 1)? as usize + 1
@@ -264,6 +267,9 @@ impl<'a> Engine<'a> {
         let gs = &mut self.graphics;
         let distance = self.value_stack.pop_f26dot6()?;
         let point_ix = self.value_stack.pop_usize()?;
+        if !gs.is_pedantic && !gs.in_bounds([(gs.zp1, point_ix), (gs.zp0, gs.rp0)]) {
+            return Ok(());
+        }
         if gs.zp1.is_twilight() {
             *gs.zp1_mut().point_mut(point_ix)? = gs.zp0().original(gs.rp0)?;
             gs.move_original(gs.zp1, point_ix, distance)?;
@@ -299,6 +305,11 @@ impl<'a> Engine<'a> {
     pub(super) fn op_mdap(&mut self, opcode: u8) -> OpResult {
         let gs = &mut self.graphics;
         let p = self.value_stack.pop_usize()?;
+        if !gs.is_pedantic && !gs.in_bounds([(gs.zp0, p)]) {
+            gs.rp0 = p;
+            gs.rp1 = p;
+            return Ok(());
+        }
         let distance = if (opcode & 1) != 0 {
             let cur_dist = gs.project(gs.zp0().point(p)?, Default::default());
             gs.round(cur_dist) - cur_dist
@@ -387,6 +398,14 @@ impl<'a> Engine<'a> {
     pub(super) fn op_mdrp(&mut self, opcode: u8) -> OpResult {
         let gs = &mut self.graphics;
         let p = self.value_stack.pop_usize()?;
+        if !gs.is_pedantic && !gs.in_bounds([(gs.zp1, p), (gs.zp0, gs.rp0)]) {
+            gs.rp1 = gs.rp0;
+            gs.rp2 = p;
+            if (opcode & 16) != 0 {
+                gs.rp0 = p;
+            }
+            return Ok(());
+        }
         let mut original_distance = if gs.zp0.is_twilight() || gs.zp1.is_twilight() {
             gs.dual_project(gs.zp1().original(p)?, gs.zp0().original(gs.rp0)?)
         } else {
@@ -471,6 +490,16 @@ impl<'a> Engine<'a> {
         let gs = &mut self.graphics;
         let n = (self.value_stack.pop()? + 1) as usize;
         let p = self.value_stack.pop_usize()?;
+        if !gs.is_pedantic
+            && (!gs.in_bounds([(gs.zp1, p), (gs.zp0, gs.rp0)]) || (n > self.cvt.len()))
+        {
+            gs.rp1 = gs.rp0;
+            if (opcode & 16) != 0 {
+                gs.rp0 = p;
+            }
+            gs.rp2 = p;
+            return Ok(());
+        }
         let mut cvt_distance = if n == 0 {
             F26Dot6::ZERO
         } else {

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

@@ -97,6 +97,7 @@ pub struct HintError {
     pub program: Program,
     pub glyph_id: Option<GlyphId>,
     pub pc: usize,
+    pub opcode: Option<Opcode>,
     pub kind: HintErrorKind,
 }
 
@@ -105,16 +106,15 @@ impl core::fmt::Display for HintError {
         match self.program {
             Program::ControlValue => write!(f, "prep")?,
             Program::Font => write!(f, "fpgm")?,
-            Program::Glyph => {
-                write!(f, "glyf[")?;
-                if let Some(glyph_id) = self.glyph_id {
-                    write!(f, "{}", glyph_id.to_u16())?;
-                } else {
-                    write!(f, "?")?;
-                }
-                write!(f, "]")?
-            }
+            Program::Glyph => write!(f, "glyf")?,
+        }
+        if let Some(glyph_id) = self.glyph_id {
+            write!(f, "[{}]", glyph_id.to_u16())?;
         }
-        write!(f, "+{}: {}", self.pc, self.kind)
+        let (opcode, colon) = match self.opcode {
+            Some(opcode) => (opcode.name(), ":"),
+            _ => ("", ""),
+        };
+        write!(f, "@{}:{opcode}{colon} {}", self.pc, self.kind)
     }
 }

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

@@ -49,7 +49,7 @@ impl HintInstance {
         );
         let glyph = Zone::default();
         let mut stack_buf = vec![0; self.max_stack];
-        let value_stack = ValueStack::new(&mut stack_buf);
+        let value_stack = ValueStack::new(&mut stack_buf, false);
         let graphics = RetainedGraphicsState::new(scale, ppem, mode);
         let mut engine = Engine::new(
             outlines,
@@ -69,9 +69,9 @@ impl HintInstance {
             false,
         );
         // Run the font program (fpgm)
-        engine.run_program(Program::Font)?;
+        engine.run_program(Program::Font, false)?;
         // Run the control value program (prep)
-        engine.run_program(Program::ControlValue)?;
+        engine.run_program(Program::ControlValue, false)?;
         // Save the retained state from the CV program
         self.graphics = *engine.retained_graphics_state();
         Ok(())
@@ -85,7 +85,12 @@ impl HintInstance {
         self.graphics.instruct_control & 1 == 0
     }
 
-    pub fn hint(&self, outlines: &Outlines, outline: &mut HintOutline) -> Result<(), HintError> {
+    pub fn hint(
+        &self,
+        outlines: &Outlines,
+        outline: &mut HintOutline,
+        is_pedantic: bool,
+    ) -> Result<(), HintError> {
         // Twilight zone
         let twilight_count = outline.twilight_scaled.len();
         let twilight_contours = [twilight_count as u16];
@@ -111,7 +116,7 @@ impl HintInstance {
             outline.flags,
             outline.contours,
         );
-        let value_stack = ValueStack::new(outline.stack);
+        let value_stack = ValueStack::new(outline.stack, is_pedantic);
         let cvt = CowSlice::new(&self.cvt, outline.cvt).unwrap();
         let storage = CowSlice::new(&self.storage, outline.storage).unwrap();
         let mut engine = Engine::new(
@@ -136,10 +141,12 @@ impl HintInstance {
             outline.coords,
             outline.is_composite,
         );
-        engine.run_program(Program::Glyph).map_err(|mut e| {
-            e.glyph_id = Some(outline.glyph_id);
-            e
-        })?;
+        engine
+            .run_program(Program::Glyph, is_pedantic)
+            .map_err(|mut e| {
+                e.glyph_id = Some(outline.glyph_id);
+                e
+            })?;
         // If we're not running in backward compatibility mode, capture
         // modified phantom points.
         if !engine.backward_compatibility() {