Update vhea and share logic with hhea #233
Conversation
moyogo
commented
Mar 14, 2018
moyogo
commented
- Use hhea.ascender instead of glyph.height as fallback verticalOrigin (fixes Change default verticalOrigin from glyph height to hhea.ascender? #231)
- Compile vertical metrics tables iff vhea metrics are defined instead of when verticalOrigin is defined in some glyphs.
- Update tests that have vhea metrics defined
- Share logic for setupTable_hhea and setupTable_vhea
Lib/ufo2ft/outlineCompiler.py
Outdated
| if isHhea: | ||
| reserved = range(4) | ||
| else: | ||
| reserved = range(5) |
There was a problem hiding this comment.
table__v_h_e_a.reserved0 is deprecated and now it is an alias for table__v_h_e_a.caretOffset (see
_v_h_e_a.py#L110-L117). So this line should be reserved = range(1, 5).
|
I had lost the pull request title after a browser crash. |
| "openTypeVheaVertTypoLineGap", | ||
| "openTypeVheaCaretSlopeRise", | ||
| "openTypeVheaCaretSlopeRun", | ||
| "openTypeVheaCaretOffset", |
There was a problem hiding this comment.
@moyogo that all vhea metrics be defined in fontinfo.plist seems a bit too much. Maybe only check if VertTypo* are present and leave the VheaCaret* stuff as optional?
There was a problem hiding this comment.
OK, I’ll change the fallback values to get a horizontal caret in
https://github.com/googlei18n/ufo2ft/blob/5f2e3514be3f625e3e40bbefdc8a3bbe59a0420b/Lib/ufo2ft/fontInfoData.py#L360-L368
Lib/ufo2ft/outlineCompiler.py
Outdated
| else: | ||
| boundsAdvance = (bounds.yMax - bounds.yMin) | ||
| # equation from the vhea spec for calculating yMaxExtent: | ||
| # Max(tsb - (yMax - yMin)). |
There was a problem hiding this comment.
oh I missed that. Was it correct before this PR?
There was a problem hiding this comment.
No, it may not have been in some cases: https://github.com/googlei18n/ufo2ft/pull/233/files#diff-2e916b2b1efc6fc438bed046c8c37f01L706
I’m looking into why the test hasn’t changed after the fixup commit.
There was a problem hiding this comment.
that "equation" in the OT spec is confusing...
the Apple TTRefMan has this definition for "extent"
The 'hhea' table uses the concept of extent. The extent is the distance from the left side bearing to the right most positions in the glyph outline.
There was a problem hiding this comment.
Before this PR, it did follow what Apple TTRefMan vhea says:
This is defined as the value of the minTopSideBearing field added to the result of the value of the yMin field subtracted from the value of the yMax field.
But the OTspec vhea says:
Defined as yMaxExtent = max(tsb - (yMax - yMin)).
There was a problem hiding this comment.
wow, even more confusing. Actually, I think @mashabow himself posted to the OpenType mailing list or the mpeg-OTSpec (or both?) about the yMaxExtent in vhea table recently (I can't find the url).
There was a problem hiding this comment.
Yes. Originally on the OpenType mailing list, I suggested correcting from:
yMaxExtent = minTopSideBearing + (yMax - yMin)
to:
yMaxExtent = max(tsb + (yMax - yMin))
in September 2017. Then the OpenType spec 1.8.2 was changed to:
yMaxExtent = max(tsb - (yMax - yMin))
...but I've never noticed the minus sign until now.😫 I don't know it is intentional or not.
There was a problem hiding this comment.
It looks like a typo to me. I just forwarded this question to the OpenType mailing list.
There was a problem hiding this comment.
In the OT spec, it has already been corrected.
In the erratum of 14 September 2017, a typo was introduced during the change, resulting in an incorrect calculation. The correct calculation should be yMaxExtent = max(tsb + (yMax-yMin)).
https://docs.microsoft.com/en-us/typography/opentype/spec/errata
|
It would be nice to have this in before the next ufo2ft release. Let me know if anybody is available to pick this up, or I could finish this myself. |
setupTable_Vhea() fails already unless all vhea metrics are defined. Some glyphs may have verticalOrigin set unwillingly (copy-paste from other font or user error).
vhea minTopSideBearing, minBottomSideBearing are only caclulated with contour glyphs, set to zero when there are no contour glyphs (same as b5e6710 but for vhea)
When vhea metrics are defined, vertical metrics tables are generated
