diff --git a/Cargo.toml b/Cargo.toml index 4e3e69943..15621d5c2 100644 --- 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 index 7951eaaff..4fc1a8eb1 100644 --- 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 index e13a875d0..f8e8fcd6a 100644 --- a/fontbe/src/glyphs.rs +++ b/fontbe/src/glyphs.rs @@ -346,7 +346,7 @@ impl Work 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 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 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 + ); + } }