anthrotype
commented
1 month ago Ok so, I tried to see if I could instead make so that ufo2ft and fonttools did not round the global metrics master values before computing the deltas, but nothing actually changed, I was still seeing the same diff.
it turns out the issue was not that fontc doesn't round before computing deltas... but rather that fonttools does the rounding within the delta computation itself, see https://github.com/fonttools/fonttools/issues/2213
This allows for rounding error from one set of deltas to be accounted for when computing subsequent deltas, reducing the total error from accumulating unbounded as number of masters grows, to always being limited to 0.5.
So this is the same as https://github.com/googlefonts/fontc/issues/235
If I apply the following patch, then ttx_diff.py for Gelasio.glyphspackage yields identical outputs for MVAR.
diff --git a/fontir/src/variations.rs b/fontir/src/variations.rs
index ff43b3a5..49b94046 100644
--- a/fontir/src/variations.rs
+++ b/fontir/src/variations.rs
@@ -21,6 +21,24 @@ use write_fonts::{
use crate::error::VariationModelError;
+trait BankerRound<U, T = Self> {
+ fn banker_round(self) -> U;
+}
+
+impl BankerRound<f64> for f64 {
+ #[inline]
+ fn banker_round(self) -> f64 {
+ self.round_ties_even()
+ }
+}
+
+impl BankerRound<kurbo::Vec2> for kurbo::Vec2 {
+ #[inline]
+ fn banker_round(self) -> kurbo::Vec2 {
+ kurbo::Vec2::new(self.x.round_ties_even(), self.y.round_ties_even())
+ }
+}
+
const ZERO: OrderedFloat<f32> = OrderedFloat(0.0);
const ONE: OrderedFloat<f32> = OrderedFloat(1.0);
@@ -171,7 +189,7 @@ impl VariationModel {
) -> Result<Vec<(VariationRegion, Vec<V>)>, DeltaError>
where
P: Copy + Default + Sub<P, Output = V>,
- V: Copy + Mul<f64, Output = V> + Sub<V, Output = V>,
+ V: Copy + Mul<f64, Output = V> + Sub<V, Output = V> + BankerRound<V>,
{
if point_seqs.is_empty() {
return Ok(Vec::new());
@@ -234,7 +252,8 @@ impl VariationModel {
})
.fold(initial_vector, |acc, (other, other_weight)| {
acc - *other * other_weight.into()
- }),
+ })
+ .banker_round(),
);
}
model_idx_to_result_idx.insert(model_idx, result.len());
take https://github.com/SorkinType/Gelasio/tree/main/sources/Gelasio.glyphspackage and run through ttx_diff.py
You'll notice MVAR has one delta related to "stro" tag (strikeout offset or position) which differs by 1 unit when compared with fontmake's
The reason for this is that fontmake builds TTFs for each master and then uses varLib to build MVAR among other tables, and the global metrics values as stored in the master TTFs are already rounded to integers (using otRound) so the input to varLib are ot-rounded integers, not floats.
Whereas fontc keeps the global metrics value as floats (the sources allow to store these as floats even though basically all these metrics are going to end up being rounded to int16 or uint16 when serialised to OpenType) and then it generates the deltas from these float metrics, and only at the end it ot-rounds the deltas themselves.
In this particular case, Gelasio doesn't actually contain floats for strikeout offset; in fact it doesn't specify any strikeout offset, so it is the GlobalMetrics::populate_defaults() method that is introducing the floats, here:
https://github.com/googlefonts/fontc/blob/37143a0c39535aaf8eaa42be53fa028bd6746cc3/fontir/src/ir.rs#L698-L701
Now, in order to match fontmake we could either
1) change
ir::GlobalMetricsso that it stores integer values instead of f64, and do an ot_round() for all incoming values (from any frontends, not just glyphs or ufo) as soon as they get set, eg1) keep f64 in GlobalMetrics and only perform to ot_round() only for the global metrics values that come from glyphs and ufo sources (not any frontends), be them user-defined or those introduced by the populate_defaults() method; this way we would match fontmake for .glyphs and DS+ufo sources, but not also force other frontends to round global metrics sources before deltas get computed.