From 35d6d13fb0a3c398ebd80008f9cfa9ddf25d20ee Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 9 Aug 2023 16:47:35 +0100 Subject: [PATCH 1/4] add pretty_assertions to write-fonts' dev-dependencies --- Cargo.toml | 4 ++++ write-fonts/Cargo.toml | 1 + 2 files changed, 5 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 687fb1382..eb2cb3659 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,5 +10,9 @@ members = [ "skrifa", ] +[workspace.dependencies] +# dev dependencies +pretty_assertions = "1.3.0" + [workspace.metadata.release] allow-branch = ["main"] diff --git a/write-fonts/Cargo.toml b/write-fonts/Cargo.toml index ac2e9f050..2ef66411f 100644 --- a/write-fonts/Cargo.toml +++ b/write-fonts/Cargo.toml @@ -26,3 +26,4 @@ font-test-data = { path = "../font-test-data" } read-fonts = { version = "0.10.0", path = "../read-fonts", features = [ "codegen_test"] } env_logger = "0.10.0" rstest = "0.18.0" +pretty_assertions.workspace = true From 8f3cc9beb6b983f7dcfd898fcd766adbc01a3b96 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 9 Aug 2023 16:50:17 +0100 Subject: [PATCH 2/4] iup: add failing test case for Oswald glyph 'two' --- write-fonts/src/tables/variations/iup.rs | 135 +++++++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/write-fonts/src/tables/variations/iup.rs b/write-fonts/src/tables/variations/iup.rs index b859af97f..8aedba250 100644 --- a/write-fonts/src/tables/variations/iup.rs +++ b/write-fonts/src/tables/variations/iup.rs @@ -551,6 +551,7 @@ fn iup_contour_optimize( #[cfg(test)] mod tests { use super::*; + use pretty_assertions::assert_eq; struct IupScenario { deltas: Vec, @@ -958,4 +959,138 @@ mod tests { fn iup_test_scenario08_optimize_contour() { iup_scenario8().assert_optimize_contour(); } + + #[test] + fn iup_delta_optimize_oswald_glyph_two() { + // https://github.com/googlefonts/fontations/issues/564 + let deltas: Vec<_> = vec![ + (0.0, 0.0), + (41.0, 0.0), + (41.0, 41.0), + (60.0, 41.0), + (22.0, -22.0), + (27.0, -15.0), + (38.0, -4.0), + (44.0, 2.0), + (44.0, -1.0), + (44.0, 2.0), + (29.0, 4.0), + (18.0, 4.0), + (9.0, 4.0), + (-4.0, -4.0), + (-11.0, -12.0), + (-11.0, -10.0), + (-11.0, -25.0), + (44.0, -25.0), + (44.0, -12.0), + (44.0, -20.0), + (39.0, -38.0), + (26.0, -50.0), + (16.0, -50.0), + (-5.0, -50.0), + (-13.0, -21.0), + (-13.0, 1.0), + (-13.0, 11.0), + (-13.0, 16.0), + (-13.0, 16.0), + (-12.0, 19.0), + (0.0, 42.0), + (0.0, 0.0), + (36.0, 0.0), + (0.0, 0.0), + (0.0, 0.0), + ] + .into_iter() + .map(|c| c.into()) + .collect(); + let coords: Vec<_> = vec![ + (41.0, 0.0), + (423.0, 0.0), + (423.0, 90.0), + (167.0, 90.0), + (353.0, 374.0), + (377.0, 410.0), + (417.0, 478.0), + (442.0, 556.0), + (442.0, 608.0), + (442.0, 706.0), + (346.0, 817.0), + (248.0, 817.0), + (176.0, 817.0), + (89.0, 759.0), + (50.0, 654.0), + (50.0, 581.0), + (50.0, 553.0), + (157.0, 553.0), + (157.0, 580.0), + (157.0, 619.0), + (173.0, 687.0), + (215.0, 729.0), + (253.0, 729.0), + (298.0, 729.0), + (334.0, 665.0), + (334.0, 609.0), + (334.0, 564.0), + (309.0, 495.0), + (270.0, 433.0), + (247.0, 397.0), + (41.0, 76.0), + (0.0, 0.0), + (478.0, 0.0), + (0.0, 0.0), + (0.0, 0.0), + ] + .into_iter() + .map(|c| c.into()) + .collect(); + + // using fonttools varLib's default tolerance + let tolerance = 0.5; + // a single contour, minus the phantom points + let contour_ends = vec![coords.len() - 4]; + + let result = iup_delta_optimize(deltas, coords, tolerance, &contour_ends).unwrap(); + + assert_eq!( + result.into_iter().enumerate().collect::>(), + // this is what fonttools iup_delta_optimize returns and what we want to match + vec![ + (0, None), + (1, Some(Vec2 { x: 41.0, y: 0.0 })), + (2, None), + (3, Some(Vec2 { x: 60.0, y: 41.0 })), + (4, Some(Vec2 { x: 22.0, y: -22.0 })), + (5, Some(Vec2 { x: 27.0, y: -15.0 })), + (6, Some(Vec2 { x: 38.0, y: -4.0 })), + (7, Some(Vec2 { x: 44.0, y: 2.0 })), + (8, Some(Vec2 { x: 44.0, y: -1.0 })), + (9, Some(Vec2 { x: 44.0, y: 2.0 })), + (10, Some(Vec2 { x: 29.0, y: 4.0 })), + (11, Some(Vec2 { x: 18.0, y: 4.0 })), + (12, Some(Vec2 { x: 9.0, y: 4.0 })), + (13, Some(Vec2 { x: -4.0, y: -4.0 })), + (14, Some(Vec2 { x: -11.0, y: -12.0 })), + (15, Some(Vec2 { x: -11.0, y: -10.0 })), + (16, None), + (17, Some(Vec2 { x: 44.0, y: -25.0 })), + (18, Some(Vec2 { x: 44.0, y: -12.0 })), + (19, Some(Vec2 { x: 44.0, y: -20.0 })), + (20, Some(Vec2 { x: 39.0, y: -38.0 })), + (21, Some(Vec2 { x: 26.0, y: -50.0 })), + (22, Some(Vec2 { x: 16.0, y: -50.0 })), + (23, Some(Vec2 { x: -5.0, y: -50.0 })), + (24, Some(Vec2 { x: -13.0, y: -21.0 })), + (25, Some(Vec2 { x: -13.0, y: 1.0 })), + (26, Some(Vec2 { x: -13.0, y: 11.0 })), + (27, Some(Vec2 { x: -13.0, y: 16.0 })), + (28, Some(Vec2 { x: -13.0, y: 16.0 })), + (29, None), + (30, Some(Vec2 { x: 0.0, y: 42.0 })), + (31, None), + (32, Some(Vec2 { x: 36.0, y: 0.0 })), + (33, None), + (34, None), + ] + ) + } } From 1fce739d61827a4708c4a119111b62dcd8af46df Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 10 Aug 2023 11:14:22 +0100 Subject: [PATCH 3/4] iup_delta_optimize: contour_ends contains endpoint indices, not contour lengths Towards fixing https://github.com/googlefonts/fontations/issues/564 Also needs corresponding changes in the fontc calling site --- write-fonts/src/tables/variations/iup.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/write-fonts/src/tables/variations/iup.rs b/write-fonts/src/tables/variations/iup.rs index 8aedba250..0a5f83371 100644 --- a/write-fonts/src/tables/variations/iup.rs +++ b/write-fonts/src/tables/variations/iup.rs @@ -47,7 +47,7 @@ pub fn iup_delta_optimize( let expected_num_coords = contour_ends .last() .copied() - //.map(|v| v + 1) + .map(|v| v + 1) .unwrap_or_default() + NUM_PHANTOM_POINTS; if num_coords != expected_num_coords { @@ -1047,7 +1047,7 @@ mod tests { // using fonttools varLib's default tolerance let tolerance = 0.5; // a single contour, minus the phantom points - let contour_ends = vec![coords.len() - 4]; + let contour_ends = vec![coords.len() - 1 - 4]; let result = iup_delta_optimize(deltas, coords, tolerance, &contour_ends).unwrap(); From 36a8929947c1147cb5f56c2d6117b1bb9b28ee3b Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 10 Aug 2023 11:15:41 +0100 Subject: [PATCH 4/4] iup: when all deltas equal (0,0) return all Nones Fixes https://github.com/googlefonts/fontations/issues/567 --- write-fonts/src/tables/variations/iup.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/write-fonts/src/tables/variations/iup.rs b/write-fonts/src/tables/variations/iup.rs index 0a5f83371..aeabd48b1 100644 --- a/write-fonts/src/tables/variations/iup.rs +++ b/write-fonts/src/tables/variations/iup.rs @@ -427,7 +427,9 @@ fn iup_contour_optimize( }; if deltas.iter().all(|d| d == first_delta) { let mut result = vec![None; n]; - result[0] = Some(*first_delta); + if first_delta.x != 0.0 || first_delta.y != 0.0 { + result[0] = Some(*first_delta); + } return Ok(result); }