Refactor vertical metrics checks (attempt 2)

[m4rc1e]

I'm going to refactor the vertical metric checks into the following four separate checks:

1. Check vertical metrics
2. Check vertical metrics regressions
3. Check CJK vertical metrics
4. Check CJK vertical metric regressions

This will be much cleaner than our current setup.

[eliheuer]

One possible test case for this is Titillium Web and Cairo, there was a post about this on Google Fonts discuss I have been trying to get to: https://groups.google.com/g/googlefonts-discuss/c/zB1h6ZtvW30

[davelab6]

The general/typical vertical metrics challenges I'm aware of are....

• latin core fonts later upgraded to latin plus, specifically tall vietnamese
• CJK fonts which follow different heuristics, as you already coded for
• Arabic, Indic, and S.E.A fonts which have very tall/deep shaping results, especially Urdu Nastaliq (Simon) and Myanmar (Simon) and Telugu (Kalapi, Ek)
• Arabic fonts that were scaled very small on the em square to prevent clipping and have 'safe' vertical metrics for older systems, but result in tiny glyphs at typical font-size values. (examples)
• merging multi-script fonts
• variable fonts MVAR table data

[arrowtype]

A couple of thoughts:

• Fonts that prioritize centering basic Latin caps should hopefully pass the check, even if it means that some accents are cut off in some contexts. Centering caps tends to make fonts look decently centered in UI buttons and next to icons (because the first letter is usually uppercase). An example font that specifically does this is Inter.
• Recursive in releases 1.073 and before had inconsistent default line heights between instances, on Mac. It turned out it was caused by the Agrave that exceeded typoAscender in the problem instances only. (Thanks to Rosalie Wagner for helping point out the specific cause.)

[m4rc1e]

• latin core fonts later upgraded to latin plus, specifically tall vietnamese

I've got this covered. If the user adds Vietnamese, the winAsccent and winDescent will need to change. If the user changes the win values, they will also need to enable fsSelection bit 7 and ensure that the typo metrics now use the old version's win metrics.

• CJK fonts which follow different heuristics, as you already coded for

Yep, the existing check is fine.

• Arabic, Indic, and S.E.A fonts which have very tall/deep shaping results, especially Urdu Nastaliq (Simon) and Myanmar (Simon) and Telugu (Kalapi, Ek)

I won't cover this situation in this PR. At DaMa, we actually set the win metrics so that tall glyphs with vowel marks wouldn't get clipped. If you just set the win metrics to the tallest glyphs, it will cut of cantillation/vowel marks etc.

• Arabic fonts that were scaled very small on the em square to prevent clipping and have 'safe' vertical metrics for older systems, but result in tiny glyphs at typical font-size values. (examples)

• merging multi-script fonts

Perhaps this should be implemented in fontTools merge (it may already have it implemented). I think you want to lowest/greatest value for each of the three sets.

• variable fonts MVAR table data

This can happen in a future PR. Atm, I'm waiting for Win users to upgrade so they're not using version of windows that have the MVAR issue.

Thanks Dave for listing these. They are most helpful.

[m4rc1e]

@felipesanches once you're happy, I will update the release notes.

This PR doesn't include the check, 1. Check vertical metrics. This can be added as a separate PR.

This PR fixes #3242 and #3241.

[felipesanches]

@felipesanches once you're happy, I will update the release notes.

Please, always add relevant entries to CHANGELOG even before I review PRs, as it is a regular mandatory procedure for any code submission.

[m4rc1e]

@felipesanches Changelog updated. PTAL.

[felipesanches]

@m4rc1e, I've seen you using deepcopy in the past just like you do here. I am not sure this is actually needed, though. My guess is that by reusing a ttFont variable and assigning to it a new instance of a fontTools.ttLib.TTFont object, the previous instance would be destroyed as python's garbage collection would delete it. Are you sure we need deepcopy? Or maybe there's more in the way it behaves that I am not fully aware of. If so please let me know.

[m4rc1e]

Plain ol copy doesn't work on TTFont objects. I guess could just reinstantiate the TTFonts but I think this is more cumbersome than just calling deepcopy

https://docs.python.org/3/library/copy.html#:~:text=A%20shallow%20copy%20constructs%20a,objects%20found%20in%20the%20original.

[m4rc1e]

Just to double check, I replaced deepcopy with copy and it didn't work.

[m4rc1e]

@felipesanches thank you for the feedback and apologies on the forgetting the constants. I have no clue why pylint is raising an error though but I will look into it.

[m4rc1e]

@felipesanches there's definitely something wrong with pylint. I just reverted my last commit back to 9ddddc0 which previously passed. Perhaps it has been upgraded and introduced a breaking change?

[m4rc1e]

I thought I was going insane but you've mentioned it here, #3255 (comment). I'll pick Simon's commit since he's already fixed this.

[google-cla]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[m4rc1e]

@simoncozens I hope you don't mind but I stole your commit to fix pylint :-). More than happy to hold off merging this pr until your ISO pr is merged.

[m4rc1e]

@felipesanches thanks for merging Simon's pr which fixed pylint. This pr can be reviewed again.

[felipesanches]

thanks @m4rc1e ;-)