From a79f00b0a39f04c72f99dbc4f9365ce780a1116e Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Fri, 20 Dec 2024 10:18:50 -0500 Subject: [PATCH] [read-fonts] revert midpoint change to match FT (#1294) 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. The results differ when `a > b` and the sum is not even. For example, when a = 7 and b = 4, the first expression is `(7 + 4) / 2` which yields 5 (and matches FreeType) while the second is `7 + (4 - 7) / 2` which yields 6. This just changes the expression to `a.wrapping_add(b) / 2` to restore compatibility while still avoiding the panic on overflow. --- read-fonts/src/tables/glyf.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/read-fonts/src/tables/glyf.rs b/read-fonts/src/tables/glyf.rs index c8ef8ddff..d37af56d2 100644 --- a/read-fonts/src/tables/glyf.rs +++ b/read-fonts/src/tables/glyf.rs @@ -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 { @@ -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);