Skip to content

Commit

Permalink
[read-fonts] revert midpoint change to match FT
Browse files Browse the repository at this point in the history
A recent PR to fix an overflow changed the midpoint calculation from `(a + b) / 2` to the "equivalent" `a + (b - a) / 2` to attempt to avoid overflow with large values but this ends up producing results that don't exactly match FreeType in some cases.

This just changes the expression to `a.wrapping_add(b) / 2` to restore compatibility while still avoiding the panic on overflow.
  • Loading branch information
dfrg committed Dec 20, 2024
1 parent a65d7fd commit ec3b5e2
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions read-fonts/src/tables/glyf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,13 +803,14 @@ impl PointCoord for i32 {
}

// Midpoint function that avoids overflow on large values.
#[inline(always)]
fn midpoint_i32(a: i32, b: i32) -> i32 {
// Original overflowing code was: (a + b) / 2
// Choose wrapping arithmetic here because we shouldn't ever
// hit this outside of fuzzing or broken fonts _and_ this is
// called from the outline to path conversion code which is
// very performance sensitive
a.wrapping_add(b.wrapping_sub(a) / 2)
a.wrapping_add(b) / 2
}

impl PointCoord for f32 {
Expand Down Expand Up @@ -1041,7 +1042,7 @@ mod tests {
fn avoid_midpoint_overflow() {
let a = F26Dot6::from_bits(1084092352);
let b = F26Dot6::from_bits(1085243712);
let expected = a.to_bits() + (b.to_bits() - a.to_bits()) / 2;
let expected = (a + b).to_bits() / 2;
// Don't panic!
let midpoint = a.midpoint(b);
assert_eq!(midpoint.to_bits(), expected);
Expand Down

0 comments on commit ec3b5e2

Please sign in to comment.