[skrifa] glyph metrics fixes

[dfrg]

A few changes to fix correctness and/or match FreeType:

1. Make unscaled case pass-through. Avoids overflow on large source values.
2. Cast advance width to u16 before scaling to match FT
3. Don't reject empty glyf glyphs when reading metric deltas from gvar

[drott]

With this applied, when comparing advances of FT for the COLRv1 test font, in the colrv1.cpp gm test in Skia, at font size 120, I get

Fontations:

Glyph advance for codepoint 984320 0xf0500: 119.999893
[...]
Glyph advance for codepoint 984328 0xf0508: 119.999893

FreeType:

Glyph advance for codepoint 984320 0xf0500: 120.000000
[...]
Glyph advance for codepoint 984328 0xf0508: 120.000000

In hmtx these glyphs have width 1000, units per em is 1000.

[dfrg]

Hrm, yes, this patch matches linearHoriAdvance. I’m guessing Skia is pulling from the glyph slot advance field which is calculated differently and rounded somewhere along the way. My intention was to provide this value in ScalerMetrics::adjusted_advance but I haven’t hooked that up yet.

[drott]

Hrm, yes, this patch matches linearHoriAdvance. I’m guessing Skia is pulling from the glyph slot advance field which is calculated differently and rounded somewhere along the way.

Yes, looks like it

mx.advance.fX =  SkFDot6ToFloat(fFace->glyph->advance.x);

in SkFontHost_FreeType.cpp:315

My intention was to provide this value in ScalerMetrics::adjusted_advance but I haven’t hooked that up yet.

SGTM, I will update then to retrieve it from that field.

[dfrg]

By “haven’t hooked it up” I mean that field is an option and is not populated yet :(

[drott]

By “haven’t hooked it up” I mean that field is an option and is not populated yet :(

Not to rush, but any updates on that? I was feeling like I needed a break from bitmap glyphs for a day and came back to this.

[dfrg]

Yes, this is computed now for TrueType outlines. It will still be None for CFF because that scaler doesn’t modify the advance width.

[drott]

Yes, this is computed now for TrueType outlines.

Thanks for the update. Do you mean in shipping/merged code? Is this in GlyphMetrics::advance_width or in AdjustedMetrics?

[dfrg]

It’s in AdjustedMetrics and published to crates.io.

[dfrg]

Closing this for now to clean things up but keeping the branch around in case we find a need to match linearHoriAdvance.