Repository: googlefonts/fontations
Pull request #984 (2024-06-30 17:10:26)

[skrifa] internal compact unscaled outline repr
Autohinting requires loading a large number of outlines and the algorithms generally expect these structures to match that of `FT_Outline`. This PR includes a new internal API for doing so with a simple compact representation that should meet all of our autohinting needs.

Notably, we do not want to run TrueType outlines through an `OutlinePen` because that materializes implied oncurves which should not affect correctness but will likely prevent us from generating results that exactly match FreeType. It's also just generally wasteful.

Based on #962 since the next PR will need both.


Patch:
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 2fb1c1a3..88eb5470 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -15,7 +15,7 @@ jobs:
     name: Rustfmt
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
       - run: rustup component add rustfmt
 
@@ -34,7 +34,7 @@ jobs:
     name: Clippy lints
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: install stable toolchain
         uses: dtolnay/rust-toolchain@stable
 
@@ -48,7 +48,7 @@ jobs:
     name: cargo test stable
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: install stable toolchain
         uses: dtolnay/rust-toolchain@stable
@@ -80,7 +80,7 @@ jobs:
     name: check codegen is up-to-date
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: install stable toolchain
         uses: dtolnay/rust-toolchain@stable
@@ -97,7 +97,7 @@ jobs:
     name: cargo check no std
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: install stable toolchain
         uses: dtolnay/rust-toolchain@stable
@@ -115,7 +115,7 @@ jobs:
     name: cargo check wasm
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 
       - name: install stable toolchain
         uses: dtolnay/rust-toolchain@stable
diff --git a/skrifa/src/instance.rs b/skrifa/src/instance.rs
index c5a79e40..8a17f37b 100644
--- a/skrifa/src/instance.rs
+++ b/skrifa/src/instance.rs
@@ -14,7 +14,7 @@ pub type NormalizedCoord = read_fonts::types::F2Dot14;
 /// in CSS (assuming a DPI scale factor of 1.0).
 ///
 /// To retrieve metrics and outlines in font units, use the [unscaled](Self::unscaled)
-/// construtor on this type.
+/// constructor on this type.
 #[derive(Copy, Clone, PartialEq, PartialOrd, Debug)]
 pub struct Size(Option<f32>);
 
diff --git a/skrifa/src/metrics.rs b/skrifa/src/metrics.rs
index f65df908..80e3f4c2 100644
--- a/skrifa/src/metrics.rs
+++ b/skrifa/src/metrics.rs
@@ -16,7 +16,7 @@
 //! to select the desired instance of a font:
 //! * Size: represented by the [Size] type, this determines the scaling factor that is
 //! applied to all metrics.
-//! * Normalized variation coordinates: respresented by the [LocationRef] type,
+//! * Normalized variation coordinates: represented by the [LocationRef] type,
 //! these define the position in design space for a variable font. For a non-variable
 //! font, these coordinates are ignored and you can pass [LocationRef::default()]
 //! as an argument for this parameter.
diff --git a/skrifa/src/outline/cff/hint.rs b/skrifa/src/outline/cff/hint.rs
index 002910b4..a84ab09c 100644
--- a/skrifa/src/outline/cff/hint.rs
+++ b/skrifa/src/outline/cff/hint.rs
@@ -81,7 +81,7 @@ pub(crate) struct HintState {
     blue_shift: Fixed,
     blue_fuzz: Fixed,
     language_group: i32,
-    supress_overshoot: bool,
+    suppress_overshoot: bool,
     do_em_box_hints: bool,
     boost: Fixed,
     darken_y: Fixed,
@@ -97,7 +97,7 @@ impl HintState {
             blue_shift: params.blue_shift,
             blue_fuzz: params.blue_fuzz,
             language_group: params.language_group,
-            supress_overshoot: false,
+            suppress_overshoot: false,
             do_em_box_hints: false,
             boost: Fixed::ZERO,
             darken_y: Fixed::ZERO,
@@ -242,7 +242,7 @@ impl HintState {
         }
         // Suppress overshoot and boost blue zones at small sizes
         if self.scale < self.blue_scale {
-            self.supress_overshoot = true;
+            self.suppress_overshoot = true;
             self.boost =
                 Fixed::from_f64(0.6) - Fixed::from_f64(0.6).mul_div(self.scale, self.blue_scale);
             // boost must remain less than 0.5, or baseline could go negative
@@ -277,7 +277,7 @@ impl HintState {
                 && bottom_edge.cs_coord <= (zone.cs_top_edge + fuzz)
             {
                 // Bottom edge captured by bottom zone.
-                adjustment = if self.supress_overshoot {
+                adjustment = if self.suppress_overshoot {
                     zone.ds_flat_edge
                 } else if zone.cs_top_edge - bottom_edge.cs_coord >= self.blue_shift {
                     // Guarantee minimum of 1 pixel overshoot
@@ -298,7 +298,7 @@ impl HintState {
                 && top_edge.cs_coord <= (zone.cs_top_edge + fuzz)
             {
                 // Top edge captured by top zone.
-                adjustment = if self.supress_overshoot {
+                adjustment = if self.suppress_overshoot {
                     zone.ds_flat_edge
                 } else if top_edge.cs_coord - zone.cs_bottom_edge >= self.blue_shift {
                     // Guarantee minimum of 1 pixel overshoot
@@ -1137,7 +1137,7 @@ mod tests {
         assert!(!state.do_em_box_hints);
         assert_eq!(state.zone_count, 6);
         assert_eq!(state.boost, Fixed::from_bits(27035));
-        assert!(state.supress_overshoot);
+        assert!(state.suppress_overshoot);
         // FreeType generates the following zones:
         let expected_zones = &[
             // csBottomEdge	-983040	int
diff --git a/skrifa/src/outline/cff/mod.rs b/skrifa/src/outline/cff/mod.rs
index d5e90e00..7264ce09 100644
--- a/skrifa/src/outline/cff/mod.rs
+++ b/skrifa/src/outline/cff/mod.rs
@@ -47,7 +47,7 @@ pub(crate) struct Outlines<'a> {
 impl<'a> Outlines<'a> {
     /// Creates a new scaler for the given font.
     ///
-    /// This will choose an underyling CFF2 or CFF table from the font, in that
+    /// This will choose an underlying CFF2 or CFF table from the font, in that
     /// order.
     pub fn new(font: &impl TableProvider<'a>) -> Result<Self, Error> {
         let units_per_em = font.head()?.units_per_em();
@@ -467,7 +467,7 @@ impl<'a, S: CommandSink> CommandSink for ScalingSink26Dot6<'a, S> {
     }
 }
 
-/// Command sink adapter that supresses degenerate move and line commands.
+/// Command sink adapter that suppresses degenerate move and line commands.
 ///
 /// FreeType avoids emitting empty contours and zero length lines to prevent
 /// artifacts when stem darkening is enabled. We don't support stem darkening
diff --git a/skrifa/src/outline/error.rs b/skrifa/src/outline/error.rs
index 3778fc63..42846516 100644
--- a/skrifa/src/outline/error.rs
+++ b/skrifa/src/outline/error.rs
@@ -21,7 +21,7 @@ pub enum DrawError {
     InsufficientMemory,
     /// Exceeded a recursion limit when loading a glyph.
     RecursionLimitExceeded(GlyphId),
-    /// Error occured during hinting.
+    /// Error occurred during hinting.
     HintingFailed(HintError),
     /// An anchor point had invalid indices.
     InvalidAnchorPoint(GlyphId, u16),
@@ -29,7 +29,7 @@ pub enum DrawError {
     PostScript(CffError),
     /// Conversion from outline to path failed.
     ToPath(ToPathError),
-    /// Error occured when reading font data.
+    /// Error occurred when reading font data.
     Read(ReadError),
     /// HarfBuzz style drawing with hints is not supported
     // Error rather than silently returning unhinted per f2f discussion.
diff --git a/skrifa/src/outline/glyf/hint/engine/logical.rs b/skrifa/src/outline/glyf/hint/engine/logical.rs
index 1015a1f3..a51cbf0b 100644
--- a/skrifa/src/outline/glyf/hint/engine/logical.rs
+++ b/skrifa/src/outline/glyf/hint/engine/logical.rs
@@ -42,7 +42,7 @@ impl<'a> Engine<'a> {
         self.value_stack.apply_binary(|a, b| Ok((a <= b) as i32))
     }
 
-    /// Greather than.
+    /// Greater than.
     ///
     /// GT[] (0x52)
     ///
diff --git a/skrifa/src/outline/glyf/hint/error.rs b/skrifa/src/outline/glyf/hint/error.rs
index 925c8c96..9610a551 100644
--- a/skrifa/src/outline/glyf/hint/error.rs
+++ b/skrifa/src/outline/glyf/hint/error.rs
@@ -46,7 +46,7 @@ impl core::fmt::Display for HintErrorKind {
             Self::NestedDefinition => write!(f, "nested function or instruction definition"),
             Self::DefinitionTooLarge => write!(
                 f,
-                "function or instruction definition exceeded the maxmium size of 64k"
+                "function or instruction definition exceeded the maximum size of 64k"
             ),
             Self::TooManyDefinitions => write!(f, "too many function or instruction definitions"),
             Self::InvalidDefinition(key) => {
diff --git a/skrifa/src/outline/glyf/hint/graphics.rs b/skrifa/src/outline/glyf/hint/graphics.rs
index 46b5ba42..9046f54c 100644
--- a/skrifa/src/outline/glyf/hint/graphics.rs
+++ b/skrifa/src/outline/glyf/hint/graphics.rs
@@ -22,7 +22,7 @@ pub enum CoordAxis {
 /// See <https://developer.apple.com/fonts/TrueType-Reference-Manual/RM04/Chap4.html>
 #[derive(Debug)]
 pub struct GraphicsState<'a> {
-    /// Fields of the graphics state that persist between calls to the intepreter.
+    /// Fields of the graphics state that persist between calls to the interpreter.
     pub retained: RetainedGraphicsState,
     /// A unit vector whose direction establishes an axis along which
     /// distances are measured.
@@ -88,7 +88,7 @@ pub struct GraphicsState<'a> {
     pub zones: [Zone<'a>; 2],
     /// True if the current glyph is a composite.
     pub is_composite: bool,
-    /// If true, enables a set of backward compabitility heuristics that
+    /// If true, enables a set of backward compatibility heuristics that
     /// prevent certain modifications to the outline. The purpose is to
     /// support "modern" vertical only hinting that attempts to preserve
     /// outline shape and metrics in the horizontal direction. This is
diff --git a/skrifa/src/outline/glyf/hint/zone.rs b/skrifa/src/outline/glyf/hint/zone.rs
index 7631450e..bc11c515 100644
--- a/skrifa/src/outline/glyf/hint/zone.rs
+++ b/skrifa/src/outline/glyf/hint/zone.rs
@@ -421,7 +421,7 @@ impl GraphicsState<'_> {
         distance: F26Dot6,
     ) -> Result<(), HintErrorKind> {
         // Note: we never adjust x in backward compatibility mode and we never
-        // adjust y in backward compability mode after IUP has been done in
+        // adjust y in backward compatibility mode after IUP has been done in
         // both directions.
         //
         // The primary motivation is to avoid horizontal adjustments in cases
diff --git a/skrifa/src/outline/glyf/outline.rs b/skrifa/src/outline/glyf/outline.rs
index 579cc56e..ea52ee46 100644
--- a/skrifa/src/outline/glyf/outline.rs
+++ b/skrifa/src/outline/glyf/outline.rs
@@ -38,7 +38,7 @@ pub struct Outline<'a> {
     /// for each component. This tracks the maximum stack depth necessary
     /// to store those values during processing.
     pub max_component_delta_stack: usize,
-    /// Number of entires in the hinting value stack.
+    /// Number of entries in the hinting value stack.
     pub max_stack: usize,
     /// Number of CVT entries for copy-on-write support.
     pub cvt_count: usize,
diff --git a/skrifa/src/outline/mod.rs b/skrifa/src/outline/mod.rs
index fc97d401..83e2e3ef 100644
--- a/skrifa/src/outline/mod.rs
+++ b/skrifa/src/outline/mod.rs
@@ -82,6 +82,7 @@ mod autohint;
 mod cff;
 mod glyf;
 mod hint;
+mod unscaled;
 
 pub mod error;
 
@@ -409,6 +410,55 @@ impl<'a> OutlineGlyph<'a> {
             }
         }
     }
+
+    /// Internal drawing API for autohinting that offers unified compact
+    /// storage for unscaled outlines.
+    #[allow(dead_code)]
+    fn draw_unscaled(
+        &self,
+        location: impl Into<LocationRef<'a>>,
+        user_memory: Option<&mut [u8]>,
+        sink: &mut impl unscaled::UnscaledOutlineSink,
+    ) -> Result<(), DrawError> {
+        let coords = location.into().coords();
+        let ppem = None;
+        match &self.kind {
+            OutlineKind::Glyf(glyf, outline) => {
+                with_glyf_memory(outline, Hinting::None, user_memory, |buf| {
+                    let outline =
+                        FreeTypeScaler::unhinted(glyf.clone(), outline, buf, ppem, coords)?
+                            .scale(&outline.glyph, outline.glyph_id)?;
+                    sink.try_reserve(outline.points.len())?;
+                    let mut contour_start = 0;
+                    for contour_end in outline.contours.iter().map(|contour| *contour as usize) {
+                        if contour_end > contour_start {
+                            if let Some(points) = outline.points.get(contour_start..=contour_end) {
+                                let flags = &outline.flags[contour_start..=contour_end];
+                                sink.extend(points.iter().zip(flags).enumerate().map(
+                                    |(ix, (point, flags))| {
+                                        unscaled::UnscaledPoint::from_glyf_point(
+                                            *point,
+                                            *flags,
+                                            ix == 0,
+                                        )
+                                    },
+                                ))?;
+                            }
+                        }
+                        contour_start = contour_end + 1;
+                    }
+                    Ok(())
+                })
+            }
+            OutlineKind::Cff(cff, glyph_id, subfont_ix) => {
+                let subfont = cff.subfont(*subfont_ix, ppem, coords)?;
+                let mut adapter = unscaled::UnscaledPenAdapter::new(sink);
+                cff.draw(&subfont, *glyph_id, coords, false, &mut adapter)?;
+                adapter.finish()?;
+                Ok(())
+            }
+        }
+    }
 }
 
 #[derive(Clone)]
@@ -1067,7 +1117,7 @@ mod tests {
         let bez = Affine::FLIP_Y * pen.path; // like an icon svg
         let actual_path_start = &bez.elements()[..expected_path_start.len()];
         // round2 can still leave very small differences from the typed 2-decimal value
-        // and the diff isn't pleasent so just compare as svg string fragments
+        // and the diff isn't pleasant so just compare as svg string fragments
         assert_eq!(
             svg_commands(expected_path_start),
             svg_commands(actual_path_start)
diff --git a/skrifa/src/outline/unscaled.rs b/skrifa/src/outline/unscaled.rs
new file mode 100644
index 00000000..41cd7cd4
--- /dev/null
+++ b/skrifa/src/outline/unscaled.rs
@@ -0,0 +1,437 @@
+//! Compact representation of an unscaled, unhinted outline.
+
+#![allow(dead_code)]
+
+use super::DrawError;
+use alloc::vec::Vec;
+use core::ops::Range;
+use raw::{
+    tables::glyf::PointFlags,
+    types::{F26Dot6, Point},
+};
+
+#[derive(Copy, Clone, Default, Debug)]
+pub(super) struct UnscaledPoint {
+    pub x: i16,
+    pub y: i16,
+    pub flags: u16,
+}
+
+impl UnscaledPoint {
+    pub const ON_CURVE: u16 = 1;
+    pub const CUBIC: u16 = 2;
+    pub const CONTOUR_START: u16 = 4;
+
+    pub fn from_glyf_point(
+        point: Point<F26Dot6>,
+        point_flags: PointFlags,
+        is_contour_start: bool,
+    ) -> Self {
+        let point = point.map(|x| (x.to_bits() >> 6) as i16);
+        let mut flags = is_contour_start as u16 * Self::CONTOUR_START;
+        if point_flags.is_on_curve() {
+            flags |= Self::ON_CURVE
+        } else if point_flags.is_off_curve_cubic() {
+            flags |= Self::CUBIC
+        };
+        Self {
+            x: point.x,
+            y: point.y,
+            flags,
+        }
+    }
+
+    pub fn is_on_curve(self) -> bool {
+        self.flags & Self::ON_CURVE != 0
+    }
+
+    pub fn is_off_curve_quad(self) -> bool {
+        self.flags & (Self::ON_CURVE | Self::CUBIC) == 0
+    }
+
+    pub fn is_off_curve_cubic(self) -> bool {
+        self.flags & (Self::CUBIC | Self::ON_CURVE) == Self::CUBIC
+    }
+
+    pub fn is_contour_start(self) -> bool {
+        self.flags & Self::CONTOUR_START != 0
+    }
+}
+
+pub(super) trait UnscaledOutlineSink {
+    fn try_reserve(&mut self, additional: usize) -> Result<(), DrawError>;
+    fn push(&mut self, point: UnscaledPoint) -> Result<(), DrawError>;
+    fn extend(&mut self, points: impl IntoIterator<Item = UnscaledPoint>) -> Result<(), DrawError> {
+        for point in points.into_iter() {
+            self.push(point)?;
+        }
+        Ok(())
+    }
+}
+
+// please can I have smallvec?
+pub(super) struct UnscaledOutlineBuf<const INLINE_CAP: usize> {
+    inline: [UnscaledPoint; INLINE_CAP],
+    inline_len: usize,
+    use_heap: bool,
+    heap: Vec<UnscaledPoint>,
+}
+
+impl<const INLINE_CAP: usize> UnscaledOutlineBuf<INLINE_CAP> {
+    pub fn new() -> Self {
+        Self {
+            inline: [Default::default(); INLINE_CAP],
+            inline_len: 0,
+            use_heap: false,
+            heap: vec![],
+        }
+    }
+
+    pub fn push(&mut self, point: UnscaledPoint) {
+        if !self.use_heap {
+            if self.inline_len < INLINE_CAP {
+                self.inline[self.inline_len] = point;
+                self.inline_len += 1;
+                return;
+            } else {
+                self.use_heap = true;
+                self.heap.extend(&self.inline);
+                self.inline_len = 0;
+            }
+        }
+        self.heap.push(point);
+    }
+
+    pub fn clear(&mut self) {
+        self.inline_len = 0;
+        self.heap.clear();
+    }
+
+    pub fn as_ref(&self) -> UnscaledOutlineRef {
+        let points = if self.use_heap {
+            &self.heap
+        } else {
+            &self.inline[..self.inline_len]
+        };
+        UnscaledOutlineRef { points }
+    }
+}
+
+impl<const INLINE_CAP: usize> UnscaledOutlineSink for UnscaledOutlineBuf<INLINE_CAP> {
+    fn try_reserve(&mut self, additional: usize) -> Result<(), DrawError> {
+        if !self.use_heap {
+            let new_len = self
+                .inline_len
+                .checked_add(additional)
+                .ok_or(DrawError::InsufficientMemory)?;
+            if new_len <= INLINE_CAP {
+                return Ok(());
+            }
+            self.heap.reserve(new_len);
+            self.heap.extend(&self.inline[..self.inline_len]);
+            self.use_heap = true;
+            self.inline_len = 0;
+        } else {
+            self.heap.reserve(additional);
+        }
+        Ok(())
+    }
+
+    fn push(&mut self, point: UnscaledPoint) -> Result<(), DrawError> {
+        self.push(point);
+        Ok(())
+    }
+}
+
+#[derive(Copy, Clone, Debug)]
+pub(super) struct UnscaledOutlineRef<'a> {
+    pub points: &'a [UnscaledPoint],
+}
+
+impl<'a> UnscaledOutlineRef<'a> {
+    /// Returns the range of contour points and the index of the point within
+    /// that contour for the last point where `f` returns true.
+    ///
+    /// This is common code used for finding extrema when materializing blue
+    /// zones.
+    ///
+    /// For example: <https://gitlab.freedesktop.org/freetype/freetype/-/blob/57617782464411201ce7bbc93b086c1b4d7d84a5/src/autofit/aflatin.c#L509>
+    pub fn find_last_contour(
+        &self,
+        mut f: impl FnMut(&UnscaledPoint) -> bool,
+    ) -> Option<(Range<usize>, usize)> {
+        if self.points.is_empty() {
+            return None;
+        }
+        let mut best_contour = 0..0;
+        // Index of the best point relative to the start of the best contour
+        let mut best_point = 0;
+        let mut cur_contour = 0..0;
+        let mut found_best_in_cur_contour = false;
+        for (point_ix, point) in self.points.iter().enumerate() {
+            if point.is_contour_start() {
+                if found_best_in_cur_contour {
+                    best_contour = cur_contour;
+                }
+                cur_contour = point_ix..point_ix;
+                found_best_in_cur_contour = false;
+            }
+            cur_contour.end += 1;
+            if f(point) {
+                best_point = point_ix - cur_contour.start;
+                found_best_in_cur_contour = true;
+            }
+        }
+        if found_best_in_cur_contour {
+            best_contour = cur_contour;
+        }
+        if !best_contour.is_empty() {
+            Some((best_contour, best_point))
+        } else {
+            None
+        }
+    }
+}
+
+/// Adapts an UnscaledOutlineSink to be fed from a pen while tracking
+/// memory allocation errors.
+pub(super) struct UnscaledPenAdapter<'a, T> {
+    sink: &'a mut T,
+    failed: bool,
+}
+
+impl<'a, T> UnscaledPenAdapter<'a, T> {
+    pub fn new(sink: &'a mut T) -> Self {
+        Self {
+            sink,
+            failed: false,
+        }
+    }
+
+    pub fn finish(self) -> Result<(), DrawError> {
+        if self.failed {
+            Err(DrawError::InsufficientMemory)
+        } else {
+            Ok(())
+        }
+    }
+}
+
+impl<'a, T> UnscaledPenAdapter<'a, T>
+where
+    T: UnscaledOutlineSink,
+{
+    fn push(&mut self, x: f32, y: f32, flags: u16) {
+        if self
+            .sink
+            .push(UnscaledPoint {
+                x: x as i16,
+                y: y as i16,
+                flags,
+            })
+            .is_err()
+        {
+            self.failed = true;
+        }
+    }
+}
+
+impl<'a, T: UnscaledOutlineSink> super::OutlinePen for UnscaledPenAdapter<'a, T> {
+    fn move_to(&mut self, x: f32, y: f32) {
+        self.push(x, y, UnscaledPoint::ON_CURVE | UnscaledPoint::CONTOUR_START);
+    }
+
+    fn line_to(&mut self, x: f32, y: f32) {
+        self.push(x, y, UnscaledPoint::ON_CURVE);
+    }
+
+    fn quad_to(&mut self, cx0: f32, cy0: f32, x: f32, y: f32) {
+        self.push(cx0, cy0, 0);
+        self.push(x, y, UnscaledPoint::ON_CURVE);
+    }
+
+    fn curve_to(&mut self, cx0: f32, cy0: f32, cx1: f32, cy1: f32, x: f32, y: f32) {
+        self.push(cx0, cy0, UnscaledPoint::CUBIC);
+        self.push(cx1, cy1, UnscaledPoint::CUBIC);
+        self.push(x, y, UnscaledPoint::ON_CURVE);
+    }
+
+    fn close(&mut self) {}
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{prelude::LocationRef, MetadataProvider};
+    use raw::{types::GlyphId, FontRef};
+
+    #[test]
+    fn read_glyf_outline() {
+        let font = FontRef::new(font_test_data::MATERIAL_SYMBOLS_SUBSET).unwrap();
+        let glyph = font.outline_glyphs().get(GlyphId::new(5)).unwrap();
+        let mut outline = UnscaledOutlineBuf::<32>::new();
+        glyph
+            .draw_unscaled(LocationRef::default(), None, &mut outline)
+            .unwrap();
+        let outline = outline.as_ref();
+        let expected = [
+            // contour 0
+            (400, 80, 5),
+            (400, 360, 1),
+            (320, 360, 1),
+            (320, 600, 1),
+            (320, 633, 0),
+            (367, 680, 0),
+            (400, 680, 1),
+            (560, 680, 1),
+            (593, 680, 0),
+            (640, 633, 0),
+            (640, 600, 1),
+            (640, 360, 1),
+            (560, 360, 1),
+            (560, 80, 1),
+            // contour 1
+            (480, 720, 5),
+            (447, 720, 0),
+            (400, 767, 0),
+            (400, 800, 1),
+            (400, 833, 0),
+            (447, 880, 0),
+            (480, 880, 1),
+            (513, 880, 0),
+            (560, 833, 0),
+            (560, 800, 1),
+            (560, 767, 0),
+            (513, 720, 0),
+        ];
+        let points = outline
+            .points
+            .iter()
+            .map(|point| (point.x, point.y, point.flags))
+            .collect::<Vec<_>>();
+        assert_eq!(points, expected);
+    }
+
+    #[test]
+    fn read_cubic_glyf_outline() {
+        let font = FontRef::new(font_test_data::CUBIC_GLYF).unwrap();
+        let glyph = font.outline_glyphs().get(GlyphId::new(2)).unwrap();
+        let mut outline = UnscaledOutlineBuf::<32>::new();
+        glyph
+            .draw_unscaled(LocationRef::default(), None, &mut outline)
+            .unwrap();
+        let outline = outline.as_ref();
+        let expected = [
+            // contour 0
+            (278, 710, 5),
+            (278, 470, 1),
+            (300, 500, 2),
+            (800, 500, 2),
+            (998, 470, 1),
+            (998, 710, 1),
+        ];
+        let points = outline
+            .points
+            .iter()
+            .map(|point| (point.x, point.y, point.flags))
+            .collect::<Vec<_>>();
+        assert_eq!(points, expected);
+    }
+
+    #[test]
+    fn read_cff_outline() {
+        let font = FontRef::new(font_test_data::CANTARELL_VF_TRIMMED).unwrap();
+        let glyph = font.outline_glyphs().get(GlyphId::new(2)).unwrap();
+        let mut outline = UnscaledOutlineBuf::<32>::new();
+        glyph
+            .draw_unscaled(LocationRef::default(), None, &mut outline)
+            .unwrap();
+        let outline = outline.as_ref();
+        let expected = [
+            // contour 0
+            (83, 0, 5),
+            (163, 0, 1),
+            (163, 482, 1),
+            (83, 482, 1),
+            // contour 1
+            (124, 595, 5),
+            (160, 595, 2),
+            (181, 616, 2),
+            (181, 652, 1),
+            (181, 688, 2),
+            (160, 709, 2),
+            (124, 709, 1),
+            (88, 709, 2),
+            (67, 688, 2),
+            (67, 652, 1),
+            (67, 616, 2),
+            (88, 595, 2),
+            (124, 595, 1),
+        ];
+        let points = outline
+            .points
+            .iter()
+            .map(|point| (point.x, point.y, point.flags))
+            .collect::<Vec<_>>();
+        assert_eq!(points, expected);
+    }
+
+    #[test]
+    fn buf_inline_and_heap() {
+        let font = FontRef::new(font_test_data::MATERIAL_SYMBOLS_SUBSET).unwrap();
+        let glyph = font.outline_glyphs().get(GlyphId::new(5)).unwrap();
+        // This glyph has 26 points so should fit in the inline buffer
+        let mut inline_outline = UnscaledOutlineBuf::<26>::new();
+        glyph
+            .draw_unscaled(LocationRef::default(), None, &mut inline_outline)
+            .unwrap();
+        assert!(!inline_outline.use_heap);
+        // Force overflow to heap
+        let mut heap_outline = UnscaledOutlineBuf::<8>::new();
+        glyph
+            .draw_unscaled(LocationRef::default(), None, &mut heap_outline)
+            .unwrap();
+        assert!(heap_outline.use_heap);
+    }
+
+    #[test]
+    fn find_vertical_extrema() {
+        let font = FontRef::new(font_test_data::MATERIAL_SYMBOLS_SUBSET).unwrap();
+        let glyph = font.outline_glyphs().get(GlyphId::new(5)).unwrap();
+        let mut outline = UnscaledOutlineBuf::<32>::new();
+        glyph
+            .draw_unscaled(LocationRef::default(), None, &mut outline)
+            .unwrap();
+        let outline = outline.as_ref();
+        // Find the maximum Y value and its containing contour
+        let mut top_y = None;
+        let (top_contour, top_point_ix) = outline
+            .find_last_contour(|point| {
+                if top_y.is_none() || Some(point.y) > top_y {
+                    top_y = Some(point.y);
+                    true
+                } else {
+                    false
+                }
+            })
+            .unwrap();
+        assert_eq!(top_contour, 14..26);
+        assert_eq!(top_point_ix, 5);
+        assert_eq!(top_y, Some(880));
+        // Find the minimum Y value and its containing contour
+        let mut bottom_y = None;
+        let (bottom_contour, bottom_point_ix) = outline
+            .find_last_contour(|point| {
+                if bottom_y.is_none() || Some(point.y) < bottom_y {
+                    bottom_y = Some(point.y);
+                    true
+                } else {
+                    false
+                }
+            })
+            .unwrap();
+        assert_eq!(bottom_contour, 0..14);
+        assert_eq!(bottom_point_ix, 0);
+        assert_eq!(bottom_y, Some(80));
+    }
+}
diff --git a/skrifa/src/setting.rs b/skrifa/src/setting.rs
index aead4d9c..338c8466 100644
--- a/skrifa/src/setting.rs
+++ b/skrifa/src/setting.rs
@@ -83,7 +83,7 @@ impl<T: Copy> From<&(&str, T)> for Setting<T> {
 /// The `selector` field should contain a tag that corresponds to a
 /// variation axis while the `value` field specifies the desired position
 /// on the axis in user coordinates (i.e. within the range defined by
-/// the mininum and maximum values of the axis).
+/// the minimum and maximum values of the axis).
 ///
 /// # Example
 /// ```
diff --git a/skrifa/src/string.rs b/skrifa/src/string.rs
index 627d05da..0b3d221c 100644
--- a/skrifa/src/string.rs
+++ b/skrifa/src/string.rs
@@ -372,7 +372,7 @@ const LANGUAGE_ID_TO_BCP47: &[(u16, &str)] = &[
     /* A mapping from Windows LCID to BCP 47 codes.
      *  This list is the sorted, curated output of tools/win_lcid.cpp.
      *  Note that these are sorted by value for quick binary lookup, and not logically by lsb.
-     *  The 'bare' language ids (e.g. 0x0001 for Arabic) are ommitted
+     *  The 'bare' language ids (e.g. 0x0001 for Arabic) are omitted
      *  as they do not appear as valid language ids in the OpenType specification.
      */
     (0x0401, "ar-SA"),        //Arabic
diff --git a/skrifa/src/variation.rs b/skrifa/src/variation.rs
index 7872dd8a..4d7c3a0d 100644
--- a/skrifa/src/variation.rs
+++ b/skrifa/src/variation.rs
@@ -394,7 +394,7 @@ impl<'a> NamedInstanceCollection<'a> {
         })
     }
 
-    /// Returns an iterator over the instances in the colletion.
+    /// Returns an iterator over the instances in the collection.
     pub fn iter(&self) -> impl Iterator<Item = NamedInstance<'a>> + 'a + Clone {
         let copy = self.clone();
         (0..self.len()).filter_map(move |i| copy.get(i))


Test patch:
diff --git a/skrifa/src/color/traversal_tests/mod.rs b/skrifa/src/color/traversal_tests/mod.rs
index 6dc5a266..be3b675d 100644
--- a/skrifa/src/color/traversal_tests/mod.rs
+++ b/skrifa/src/color/traversal_tests/mod.rs
@@ -253,7 +253,7 @@ fn should_rebaseline() -> bool {
     env::var("REBASELINE_COLRV1_TESTS").is_ok()
 }
 
-// To regenerate the baselines, set the enviroment variable `REBASELINE_COLRV1_TESTS`
+// To regenerate the baselines, set the environment variable `REBASELINE_COLRV1_TESTS`
 // when running tests, for example like this:
 // $ REBASELINE_COLRV1_TESTS=1 cargo test color::traversal
 fn colrv1_traversal_test(