[fontbe] make cu2qu tolerance relative to UPEM

We use the same default tolerance as fontTools.cu2qu, i.e. 1/1000th of the font's UPEM. Fixes #474
googlefonts · Nov 24, 2023 · a37e8ba · a37e8ba
1 parent 0a2d4b8
commit a37e8ba
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 5 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -33,6 +33,7 @@ tempfile = "3.3.0"
 more-asserts = "0.3.1"
 pretty_assertions = "1.3.0"
 temp-env = "0.3.3"
+rstest = "0.18.2"
 
 [workspace]
 

diff --git a/fontbe/Cargo.toml b/fontbe/Cargo.toml
@@ -43,3 +43,4 @@ ansi_term.workspace = true
 tempfile.workspace = true
 more-asserts.workspace = true
 temp-env.workspace = true
+rstest.workspace = true
diff --git a/fontbe/src/glyphs.rs b/fontbe/src/glyphs.rs
@@ -346,7 +346,7 @@ impl Work<Context, AnyWorkId, Error> for GlyphWork {
         let glyph = CheckedGlyph::new(ir_glyph)?;
 
         // Hopefully in time https://github.com/harfbuzz/boring-expansion-spec means we can drop this
-        let mut glyph = cubics_to_quadratics(glyph);
+        let mut glyph = cubics_to_quadratics(glyph, static_metadata.units_per_em);
 
         if !context.flags.contains(Flags::KEEP_DIRECTION) {
             glyph.reverse_contour_direction();
@@ -451,7 +451,7 @@ impl Work<Context, AnyWorkId, Error> for GlyphWork {
     }
 }
 
-fn cubics_to_quadratics(glyph: CheckedGlyph) -> CheckedGlyph {
+fn cubics_to_quadratics(glyph: CheckedGlyph, units_per_em: u16) -> CheckedGlyph {
     let CheckedGlyph::Contour {
         name,
         paths: contours,
@@ -462,6 +462,10 @@ fn cubics_to_quadratics(glyph: CheckedGlyph) -> CheckedGlyph {
 
     trace!("Convert '{name}' to quadratic");
 
+    // match fontTools.cu2qu default tolerance (i.e 1/1000th of UPEM):
+    // https://github.com/fonttools/fonttools/blob/f99774a/Lib/fontTools/cu2qu/ufo.py#L43-L46
+    let tolerance = units_per_em as f64 / 1000.0;
+
     // put all the loc + path iters into a vec
     let mut loc_iters: Vec<_> = contours
         .iter()
@@ -514,8 +518,7 @@ fn cubics_to_quadratics(glyph: CheckedGlyph) -> CheckedGlyph {
                 .collect();
 
             // At long last, actually convert something to quadratic
-            // TODO what should we pass for accuracy
-            let Some(quad_splines) = cubics_to_quadratic_splines(&cubics, 1.0) else {
+            let Some(quad_splines) = cubics_to_quadratic_splines(&cubics, tolerance) else {
                 panic!("'{name}': unable to convert to quadratic {cubics:?}");
             };
             if quad_splines.len() != loc_iters.len() {
@@ -908,8 +911,14 @@ impl Work<Context, AnyWorkId, Error> for GlyfLocaWork {
 mod tests {
     use super::*;
 
+    use font_types::Tag;
+    use fontdrasil::{
+        coords::{NormalizedCoord, NormalizedLocation},
+        types::GlyphName,
+    };
     use fontir::ir;
-    use kurbo::Affine;
+    use kurbo::{Affine, BezPath, PathEl};
+    use rstest::rstest;
 
     /// Returns a glyph instance and another one that can be its component
     fn create_reusable_component() -> (ir::GlyphInstance, ir::GlyphInstance) {
@@ -983,4 +992,46 @@ mod tests {
             assert_eq!(should_be_required, post.required)
         }
     }
+
+    fn simple_static_contour_glyph() -> CheckedGlyph {
+        // Contains one default instance with one contour comprising two segments, i.e.
+        // a cubic curve and a closing line
+        let mut paths = HashMap::new();
+        paths.insert(
+            NormalizedLocation::from(vec![(Tag::new(b"wght"), NormalizedCoord::new(0.0))]),
+            BezPath::from_vec(vec![
+                PathEl::MoveTo((0.0, 500.0).into()),
+                PathEl::CurveTo(
+                    (200.0, 500.0).into(),
+                    (500.0, 200.0).into(),
+                    (500.0, 0.0).into(),
+                ),
+                PathEl::ClosePath,
+            ]),
+        );
+        CheckedGlyph::Contour {
+            name: GlyphName::from("test"),
+            paths,
+        }
+    }
+
+    #[rstest]
+    #[case::small_upem(500, 8)]
+    #[case::default_upem(1000, 7)]
+    #[case::large_upem(2000, 6)]
+    fn cubics_to_quadratics_at_various_upems(#[case] upem: u16, #[case] expected_segments: usize) {
+        // The default conversion accuracy/tolerance is set to 1/1000th of the UPEM.
+        // Therefore, the number of converted quadratic segments increases as the UPEM
+        // decreases, or decreases as the UPEM increases.
+        let CheckedGlyph::Contour { paths, .. } =
+            cubics_to_quadratics(simple_static_contour_glyph(), upem)
+        else {
+            panic!("Expected a contour glyph");
+        };
+
+        assert_eq!(
+            paths.values().next().unwrap().segments().count(),
+            expected_segments
+        );
+    }
 }