Skip to content

Commit

Permalink
[iup] Fix mismatch in behaviour with fonttools
Browse files Browse the repository at this point in the history
This patch fixes an issue with iup not resolving, although I do not
fully understand the problem.

Basically: in comparing our implementation with the one in fonttools,
there is clear divergence in some of the numerical code, where we are
using Option<usize> in places where python is just using an integer.
More importantly, we are using None where Python is using -1, but our
comparison code is never checking if a value == None, whereas python
*does* check if a value == -1, which means that python is setting the
solution in cases where we are not.

This can be fixed by using `checked_sub` instead of `saturating_sub`, at
least for this input.

It feels like this is probably fine, since the structure of this loop is
such that the only negative value that is possible is -1 (at the start)
but it feels a little loose.
  • Loading branch information
cmyr committed Oct 17, 2023
1 parent ca4fea8 commit 1463559
Showing 1 changed file with 62 additions and 1 deletion.
63 changes: 62 additions & 1 deletion write-fonts/src/tables/gvar/iup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ fn iup_contour_optimize(
solution.insert(idx % n);
i = dp_result.chain[idx];
}
if i == Some(start.saturating_sub(n)) {
if i == start.checked_sub(n) {
// Python reads [-1] to get 0, usize doesn't like that
let cost = dp_result.costs[start]
- if n < start {
Expand Down Expand Up @@ -1279,4 +1279,65 @@ mod tests {
]
)
}

// https://github.com/googlefonts/fontations/issues/662
#[test]
fn googlesans_m_sc() {
let deltas = vec![
Vec2 { x: 73.0, y: 0.0 },
Vec2 { x: 73.0, y: 580.0 },
Vec2 { x: 189.0, y: 580.0 },
Vec2 { x: 381.0, y: 116.0 },
Vec2 { x: 385.0, y: 116.0 },
Vec2 { x: 578.0, y: 580.0 },
Vec2 { x: 692.0, y: 580.0 },
Vec2 { x: 692.0, y: 0.0 },
Vec2 { x: 607.0, y: 0.0 },
Vec2 { x: 607.0, y: 342.0 },
Vec2 { x: 611.0, y: 448.0 },
Vec2 { x: 607.0, y: 448.0 },
Vec2 { x: 417.0, y: 0.0 },
Vec2 { x: 349.0, y: 0.0 },
Vec2 { x: 158.0, y: 448.0 },
Vec2 { x: 154.0, y: 448.0 },
Vec2 { x: 158.0, y: 342.0 },
Vec2 { x: 158.0, y: 0.0 },
Vec2 { x: 0.0, y: 0.0 },
Vec2 { x: 765.0, y: 0.0 },
Vec2 { x: 0.0, y: 0.0 },
Vec2 { x: 0.0, y: 0.0 },
];

let coords = [
(73.0, 0.0),
(73.0, 580.0),
(189.0, 580.0),
(381.0, 116.0),
(385.0, 116.0),
(578.0, 580.0),
(692.0, 580.0),
(692.0, 0.0),
(607.0, 0.0),
(607.0, 342.0),
(611.0, 448.0),
(607.0, 448.0),
(417.0, 0.0),
(349.0, 0.0),
(158.0, 448.0),
(154.0, 448.0),
(158.0, 342.0),
(158.0, 0.0),
(0.0, 0.0),
(765.0, 0.0),
(0.0, 0.0),
(0.0, 0.0),
]
.into_iter()
.map(|(x, y)| Point::new(x, y))
.collect::<Vec<_>>();

let contour_ends = vec![17];

iup_delta_optimize(deltas, coords, 0.5, &contour_ends).unwrap();
}
}

0 comments on commit 1463559

Please sign in to comment.