googlefonts / sleipnir

Memory safe implementations of Google Fonts specific font manipulations.
Apache License 2.0
1 stars 1 forks source link

Verify we match hb rounding for an off-default example that used to fail #15

Closed rsheeter closed 6 months ago

rsheeter commented 6 months ago

The expected value is taken from a google3 test that draws with hb-draw, just like all the opsz 24 examples. The paths are very similar but decline to match. I'm not entirely sure if this is expected, we aim to match freetype and hb-draw is slightly different, or unexpected.

image

I don't think humans should be able to tell but even so, not sure if this is expected.

Update: per @dfrg :+1:

I think that means working as currently intended. Potentially path style harfbuzz could skip the rounding which might make the results identical.

Update2: https://github.com/googlefonts/fontations/pull/880 adds harfbuzz style f32 computation to skrifa.

Update3: now works using published copy of skrifa

rsheeter commented 6 months ago

Per @dfrg to remove the rounding to confirm/deny this produces exact matches, https://github.com/googlefonts/fontations/blob/112861b9a02a992b5f02eb10d51a933b0791f35b/skrifa/src/outline/glyf/mod.rs#L510 Fixed::to_i32 rounds, likely fold into the following loop and do 26.6 conversion first

E.g. *point = unscaled.map(F26Dot6::from_i32) + delta.map(Fixed::to_26dot6)

rsheeter commented 6 months ago

^ didn't quite get me there. Loop amended to:

        } else {
            // Unlike FreeType, we also store unscaled outlines in 26.6.
            for (point, unscaled) in scaled.iter_mut().zip(unscaled.iter()) {
                *point = unscaled.map(F26Dot6::from_i32);
            }
            if have_deltas {
                // TODO: conditionally Round off deltas for unscaled outlines.
                eprintln!("TODO: conditionally round");
                for (point, delta) in scaled.iter_mut().zip(self.memory.deltas.iter()) {
                    *point += delta.map(Fixed::to_f26dot6);
                }
            }
        }
M150,-138 Q113.225,  -138 86.1125,  -165.612
M150,-138 Q113.21875,-138 86.109375,-165.609375
rsheeter commented 6 months ago

For my previously failing svg with 2 decimal place coordinates we now match. Some work to exactly match (full float) is deferred to https://github.com/googlefonts/fontations/issues/887.