[skrifa] support Size::new(0) (#770)

The `Size::new(ppem: f32)` constructor interpreted a ppem of 0.0 as a request for no scaling which might be surprising behavior for users. This changes the internal representation of size to hold an `Option` so that a ppem of 0.0 works as expected.

Fixes #627

skrifa/src/instance.rs

@@ -16,28 +16,25 @@ pub type NormalizedCoord = read_fonts::types::F2Dot14;
 /// To retrieve metrics and outlines in font units, use the [unscaled](Self::unscaled)
 /// construtor on this type.
 #[derive(Copy, Clone, PartialEq, PartialOrd, Debug)]
-pub struct Size(f32);
+pub struct Size(Option<f32>);
 
 impl Size {
     /// Creates a new font size from the given value in pixels per em units.
-    ///
-    /// Providing a value `<= 0.0` is equivalent to creating an unscaled size
-    /// and will result in metrics and outlines generated in font units.
     pub fn new(ppem: f32) -> Self {
-        Self(ppem)
+        Self(Some(ppem))
     }
 
     /// Creates a new font size for generating unscaled metrics or outlines in
     /// font units.
     pub fn unscaled() -> Self {
-        Self(0.0)
+        Self(None)
     }
 
     /// Returns the raw size in pixels per em units.
     ///
     /// Results in `None` if the size is unscaled.
     pub fn ppem(self) -> Option<f32> {
-        (self.0 > 0.0).then_some(self.0)
+        self.0
     }
 
     /// Computes a linear scale factor for this font size and the given units
@@ -46,10 +43,9 @@ impl Size {
     ///
     /// Returns 1.0 for an unscaled size or when `units_per_em` is 0.
     pub fn linear_scale(self, units_per_em: u16) -> f32 {
-        if self.0 > 0.0 && units_per_em != 0 {
-            self.0 / units_per_em as f32
-        } else {
-            1.0
+        match self.0 {
+            Some(ppem) if units_per_em != 0 => ppem / units_per_em as f32,
+            _ => 1.0,
         }
     }
 
@@ -62,12 +58,15 @@ impl Size {
         // 2) this value is divided by UPEM:
         //    (here, scaled_h=height and h=upem)
         //    <https://gitlab.freedesktop.org/freetype/freetype/-/blob/49781ab72b2dfd0f78172023921d08d08f323ade/src/base/ftobjs.c#L3312>
-        if self.0 > 0.0 && units_per_em > 0 {
-            Fixed::from_bits((self.0 * 64.) as i32) / Fixed::from_bits(units_per_em as i32)
-        } else {
-            // This is an identity scale for the pattern
-            // `mul_div(value, scale, 64)`
-            Fixed::from_bits(0x10000 * 64)
+        match self.0 {
+            Some(ppem) if units_per_em > 0 => {
+                Fixed::from_bits((ppem * 64.) as i32) / Fixed::from_bits(units_per_em as i32)
+            }
+            _ => {
+                // This is an identity scale for the pattern
+                // `mul_div(value, scale, 64)`
+                Fixed::from_bits(0x10000 * 64)
+            }
         }
     }
 }

skrifa/src/outline/cff/hint.rs

@@ -1304,7 +1304,7 @@ mod tests {
         let font = FontRef::new(font_test_data::CANTARELL_VF_TRIMMED).unwrap();
         let cff_font = super::super::Outlines::new(&font).unwrap();
         let state = cff_font
-            .subfont(0, 8.0, &[F2Dot14::from_f32(-1.0); 2])
+            .subfont(0, Some(8.0), &[F2Dot14::from_f32(-1.0); 2])
             .unwrap()
             .hint_state;
         let mut initial_map = HintMap::new(state.scale);

skrifa/src/outline/cff/mod.rs

@@ -135,7 +135,12 @@ impl<'a> Outlines<'a> {
     ///
     /// The index of a subfont for a particular glyph can be retrieved with
     /// the [`subfont_index`](Self::subfont_index) method.
-    pub fn subfont(&self, index: u32, size: f32, coords: &[F2Dot14]) -> Result<Subfont, Error> {
+    pub fn subfont(
+        &self,
+        index: u32,
+        size: Option<f32>,
+        coords: &[F2Dot14],
+    ) -> Result<Subfont, Error> {
         let private_dict_range = self.private_dict_range(index)?;
         let private_dict_data = self.offset_data().read_array(private_dict_range.clone())?;
         let mut hint_params = HintParams::default();
@@ -164,12 +169,13 @@ impl<'a> Outlines<'a> {
                 _ => {}
             }
         }
-        let scale = if size <= 0.0 {
-            Fixed::ONE
-        } else {
-            // Note: we do an intermediate scale to 26.6 to ensure we
-            // match FreeType
-            Fixed::from_bits((size * 64.) as i32) / Fixed::from_bits(self.units_per_em as i32)
+        let scale = match size {
+            Some(ppem) if self.units_per_em > 0 => {
+                // Note: we do an intermediate scale to 26.6 to ensure we
+                // match FreeType
+                Fixed::from_bits((ppem * 64.) as i32) / Fixed::from_bits(self.units_per_em as i32)
+            }
+            _ => Fixed::ONE,
         };
         // When hinting, use a modified scale factor
         // <https://gitlab.freedesktop.org/freetype/freetype/-/blob/80a507a6b8e3d2906ad2c8ba69329bd2fb2a85ef/src/psaux/psft.c#L279>
@@ -672,22 +678,23 @@ mod tests {
     /// fonts), locations in variation space.
     fn compare_glyphs(font_data: &[u8], expected_outlines: &str) {
         let font = FontRef::new(font_data).unwrap();
-        let outlines = read_fonts::scaler_test::parse_glyph_outlines(expected_outlines);
-        let scaler = super::Outlines::new(&font).unwrap();
+        let expected_outlines = read_fonts::scaler_test::parse_glyph_outlines(expected_outlines);
+        let outlines = super::Outlines::new(&font).unwrap();
         let mut path = read_fonts::scaler_test::Path::default();
-        for expected_outline in &outlines {
+        for expected_outline in &expected_outlines {
             if expected_outline.size == 0.0 && !expected_outline.coords.is_empty() {
                 continue;
             }
+            let size = (expected_outline.size != 0.0).then_some(expected_outline.size);
             path.elements.clear();
-            let subfont = scaler
+            let subfont = outlines
                 .subfont(
-                    scaler.subfont_index(expected_outline.glyph_id),
-                    expected_outline.size,
+                    outlines.subfont_index(expected_outline.glyph_id),
+                    size,
                     &expected_outline.coords,
                 )
                 .unwrap();
-            scaler
+            outlines
                 .draw(
                     &subfont,
                     expected_outline.glyph_id,

skrifa/src/outline/embedded_hinting.rs

@@ -93,9 +93,9 @@ impl EmbeddedHintingInstance {
                     _ => vec![],
                 };
                 subfonts.clear();
-                let size = size.ppem().unwrap_or_default();
+                let ppem = size.ppem();
                 for i in 0..cff.subfont_count() {
-                    subfonts.push(cff.subfont(i, size, &self.coords)?);
+                    subfonts.push(cff.subfont(i, ppem, &self.coords)?);
                 }
                 self.kind = HinterKind::Cff(subfonts);
             }
@@ -110,7 +110,7 @@ impl EmbeddedHintingInstance {
         memory: Option<&mut [u8]>,
         pen: &mut impl OutlinePen,
     ) -> Result<AdjustedMetrics, DrawError> {
-        let ppem = self.size.ppem().unwrap_or_default();
+        let ppem = self.size.ppem();
         let coords = self.coords.as_slice();
         match (&self.kind, &glyph.kind) {
             (HinterKind::Glyf, OutlineKind::Glyf(glyf, outline)) => {

skrifa/src/outline/glyf/mod.rs

@@ -135,7 +135,7 @@ impl<'a> Outlines<'a> {
         &self,
         memory: OutlineMemory<'a>,
         outline: &Outline,
-        size: f32,
+        size: Option<f32>,
         coords: &'a [F2Dot14],
     ) -> Result<ScaledOutline<'a>, DrawError> {
         Scaler::new(self.clone(), memory, size, coords, |_| true, false)
@@ -146,7 +146,7 @@ impl<'a> Outlines<'a> {
         &self,
         memory: OutlineMemory<'a>,
         outline: &Outline,
-        size: f32,
+        size: Option<f32>,
         coords: &'a [F2Dot14],
         hint_fn: impl FnMut(HintOutline) -> bool,
     ) -> Result<ScaledOutline<'a>, DrawError> {
@@ -285,19 +285,18 @@ where
     fn new(
         outlines: Outlines<'a>,
         memory: OutlineMemory<'a>,
-        size: f32,
+        size: Option<f32>,
         coords: &'a [F2Dot14],
         hint_fn: H,
         is_hinted: bool,
     ) -> Self {
-        let (is_scaled, scale) = if size > 0.0 && outlines.units_per_em > 0 {
-            (
+        let (is_scaled, scale) = match size {
+            Some(ppem) if outlines.units_per_em > 0 => (
                 true,
-                F26Dot6::from_bits((size * 64.) as i32)
+                F26Dot6::from_bits((ppem * 64.) as i32)
                     / F26Dot6::from_bits(outlines.units_per_em as i32),
-            )
-        } else {
-            (false, F26Dot6::from_bits(0x10000))
+            ),
+            _ => (false, F26Dot6::from_bits(0x10000)),
         };
         Self {
             outlines,

skrifa/src/outline/mod.rs

@@ -230,7 +230,7 @@ impl<'a> OutlineGlyph<'a> {
         memory: Option<&mut [u8]>,
         pen: &mut impl OutlinePen,
     ) -> Result<AdjustedMetrics, DrawError> {
-        let ppem = size.ppem().unwrap_or_default();
+        let ppem = size.ppem();
         let coords = location.into().coords();
         match &self.kind {
             OutlineKind::Glyf(glyf, outline) => {
@@ -462,15 +462,17 @@ mod tests {
             if expected_outline.size == 0.0 && !expected_outline.coords.is_empty() {
                 continue;
             }
+            let size = if expected_outline.size != 0.0 {
+                Size::new(expected_outline.size)
+            } else {
+                Size::unscaled()
+            };
             path.elements.clear();
             font.outline_glyphs()
                 .get(expected_outline.glyph_id)
                 .unwrap()
                 .draw(
-                    DrawSettings::unhinted(
-                        Size::new(expected_outline.size),
-                        expected_outline.coords.as_slice(),
-                    ),
+                    DrawSettings::unhinted(size, expected_outline.coords.as_slice()),
                     &mut path,
                 )
                 .unwrap();

skrifa/src/scale.rs

@@ -463,9 +463,14 @@ mod tests {
                 continue;
             }
             path.elements.clear();
+            let size = if expected_outline.size != 0.0 {
+                Size::new(expected_outline.size)
+            } else {
+                Size::unscaled()
+            };
             let mut scaler = cx
                 .new_scaler()
-                .size(Size::new(expected_outline.size))
+                .size(size)
                 .normalized_coords(&expected_outline.coords)
                 .build(&font);
             scaler