WIP: Reproducing tests for F26Dot6 multiply and divide bug

[Hoolean]

Hello!

We spotted some funky behaviour when using HarfBuzzScaler, and think we have narrowed it down to F26Dot6 multiplication and division. On the other hand, as the FreeTypeScaler and hinting virtual machine are having no such issues, there may be something else at play...

This PR adds failing tests for the F26Dot6 results that seem suspect, as well as the original HarfBuzzScaler issue that led us to begin investigating : )

(As there's no fix yet, it's not ready to be merged as-is)

[Hoolean]

At a glance, it looks like fixed_mul_div may need constants made generic to work for both Fixed and F26Dot6 simultaneously, but I'm afraid I'm unable to investigate further right now

[anthrotype]

yes, looks fixed_mul_div! macro hardcodes 16-bit shifts which is incorrect for F26Dot6 (26.6). Maybe it should be parametrized?

[anthrotype]

If I apply this patch to parametrize fixed_mul_div! and use 6 fractional bits for F26Dot6, then I get a bunch of failures in the freetype integration tests, not sure why...

commit 2c0d9bcfa380bb8bc3445af78f4db2d1bf959411
Author: Cosimo Lupo <clupo@google.com>
Date:   Fri Nov 14 15:31:33 2025 -0800

    fix fixed_mul_div for F26Dot6

diff --git a/font-types/src/fixed.rs b/font-types/src/fixed.rs
index 65a26bb7..558e4e13 100644
--- a/font-types/src/fixed.rs
+++ b/font-types/src/fixed.rs
@@ -139,7 +139,7 @@ macro_rules! fixed_impl {
 
 /// Implements multiplication and division operators for fixed types.
 macro_rules! fixed_mul_div {
-    ($ty:ty) => {
+    ($ty:ty, $fract_bits:literal) => {
         impl $ty {
             /// Multiplies `self` by `a` and divides the product by `b`.
             // This one is specifically not always inlined due to size and
@@ -180,7 +180,8 @@ macro_rules! fixed_mul_div {
             #[inline(always)]
             fn mul(self, other: Self) -> Self::Output {
                 let ab = self.0 as i64 * other.0 as i64;
-                Self(((ab + 0x8000 - i64::from(ab < 0)) >> 16) as i32)
+                let round = 1i64 << ($fract_bits - 1);
+                Self(((ab + round - i64::from(ab < 0)) >> $fract_bits) as i32)
             }
         }
 
@@ -209,7 +210,7 @@ macro_rules! fixed_mul_div {
                 let q = if b == 0 {
                     0x7FFFFFFF
                 } else {
-                    ((((a as u64) << 16) + ((b as u64) >> 1)) / (b as u64)) as u32
+                    ((((a as u64) << $fract_bits) + ((b as u64) >> 1)) / (b as u64)) as u32
                 };
                 Self(if sign < 0 {
                     (q as i32).wrapping_neg()
@@ -287,8 +288,8 @@ fixed_impl!(F4Dot12, 16, 12, i16);
 fixed_impl!(F6Dot10, 16, 10, i16);
 fixed_impl!(Fixed, 32, 16, i32);
 fixed_impl!(F26Dot6, 32, 6, i32);
-fixed_mul_div!(Fixed);
-fixed_mul_div!(F26Dot6);
+fixed_mul_div!(Fixed, 16);
+fixed_mul_div!(F26Dot6, 6);
 float_conv!(F2Dot14, to_f32, from_f32, f32);
 float_conv!(F4Dot12, to_f32, from_f32, f32);
 float_conv!(F6Dot10, to_f32, from_f32, f32);

[anthrotype]

also... why in skrifa's glyf::Outlines::compute_scale (where we're supposed to divide ppem / upem) do we interpret the self.units_per_em in the denominator as a F26Dot6 from raw bits? Shouldn't it be from_i32 instead? With F26Dot6::from_bits it'd be like e.g. interpreting 1000 units per em as 1000/64 = 15.625 in decimals which doesn't make sense to me.

Also why is the unscaled default value returned from the same compute_scale method F26Dot6::from_bits(0x10000) and not simply F26Dot6::ONE? 0x10000 only means 1.0 for a Fixed16.16... Highly suspicious

diff --git a/skrifa/src/outline/glyf/mod.rs b/skrifa/src/outline/glyf/mod.rs
index 0526e25f..394eea8b 100644
--- a/skrifa/src/outline/glyf/mod.rs
+++ b/skrifa/src/outline/glyf/mod.rs
@@ -174,11 +174,11 @@ impl<'a> Outlines<'a> {
                 return (
                     true,
                     F26Dot6::from_bits((ppem * 64.) as i32)
-                        / F26Dot6::from_bits(self.units_per_em as i32),
+                        / F26Dot6::from_i32(self.units_per_em as i32),
                 );
             }
         }
-        (false, F26Dot6::from_bits(0x10000))
+        (false, F26Dot6::ONE)
     }
 
     pub fn compute_hinted_scale(&self, ppem: Option<f32>) -> (bool, F26Dot6) {

[anthrotype]

there's an asymmetry in glyf::Outlines::compute_scale between the scaled case (when ppem is Some) vs the unscaled one (when it is None):

we have:

• Scaled scale: 64x larger than a naive ppem / upem (which one would get by treating them as floats or decimals)
• Unscaled scale: 1024x larger than 1.0

how's that possible?

[dfrg]

Good catch. This is a known bug that fell off the radar long ago. The short story is that FreeType only has 16.16 mul/div functions that are used in various ways to handle 26.6 arithmetic. We copied this to match the output but at some point, also added a type for F26Dot6 which seriously complicated things. Trying to generalize mul/div caused some subtle bugs so I decided to deal with it later and never got around to it.

[anthrotype]

ah! I see, so it's deliberate...

Ok then in the interim, until this is fixed properly, we could have the HarfBuzzScaler "fix" the return value from compute_scale method to obtain the "true" scale, i.e. by dividing it by 65536. That's the total "excess" factor resulting from the combination of two bugs:

1. units_per_em denominator is 64x too small (using F26Dot6::from_bits instead of from_i32)
2. F26Dot6 division shifts by >> 16 bits instead of >> 6 bits, i.e. 10 bits difference which make results 2^10 or 1024 larger

so in total 64 * 1024 == 65536

The following patch would keep both the freetype and harfbuzz scaler happy without actually addressing the elephant in the room (f26dot6_div_identity itself can't pass until the fix_mul_div! macro respect the nominal 6 bits):

diff --git a/skrifa/src/outline/glyf/mod.rs b/skrifa/src/outline/glyf/mod.rs
index 0526e25f..ac5f469e 100644
--- a/skrifa/src/outline/glyf/mod.rs
+++ b/skrifa/src/outline/glyf/mod.rs
@@ -375,6 +375,18 @@ impl<'a> HarfBuzzScaler<'a> {
             self.outlines.hdmx_width(self.ppem, glyph_id),
         ))
     }
+
+    /// Returns the corrected scale factor for HarfBuzz.
+    ///
+    /// The scale from `compute_scale` is 65536x too large due to two bugs:
+    /// 1. Using 16-bit shifts instead of 6-bit shifts for F26Dot6 division (1024x error)
+    /// 2. Using `from_bits` instead of `from_i32` for the denominator (64x error)
+    ///
+    /// FreeType's coordinate handling has compensating bugs, but HarfBuzz does correct
+    /// arithmetic, so we need to divide by 65536 to get the true scale.
+    fn corrected_scale(&self) -> f32 {
+        self.scale.to_f32() / 65536.0
+    }
 }
 
 /// F26Dot6 coords, Fixed deltas, and a penchant for rounding
@@ -1040,7 +1052,7 @@ impl Scaler for HarfBuzzScaler<'_> {
     fn load_empty(&mut self, glyph_id: GlyphId) -> Result<(), DrawError> {
         // HB doesn't have an equivalent so this version just copies the
         // FreeType version above but changed to use floating point
-        let scale = self.scale.to_f32();
+        let scale = self.corrected_scale();
         let mut unscaled = self.phantom;
         if self.outlines.glyph_metrics.hvar.is_none()
             && self.outlines.gvar.is_some()
@@ -1076,6 +1088,7 @@ impl Scaler for HarfBuzzScaler<'_> {
         let phantom_start = point_count;
         let points_end = points_start + point_count + PHANTOM_POINT_COUNT;
         let point_range = points_start..points_end;
+        let scale = self.corrected_scale();
         // Points and flags are accumulated as we load the outline.
         let points = self
             .memory
@@ -1137,7 +1150,6 @@ impl Scaler for HarfBuzzScaler<'_> {
         }
         // Apply scaling
         if self.is_scaled {
-            let scale = self.scale.to_f32();
             for point in points.iter_mut() {
                 *point = point.map(|c| c * scale);
             }
@@ -1159,7 +1171,7 @@ impl Scaler for HarfBuzzScaler<'_> {
         recurse_depth: usize,
     ) -> Result<(), DrawError> {
         use DrawError::InsufficientMemory;
-        let scale = self.scale.to_f32();
+        let scale = self.corrected_scale();
         // The base indices of the points for the current glyph.
         let point_base = self.point_count;
         // Compute the per component deltas. Since composites can be nested, we