Clarify rationale and log messages: check/041: WARN: Checking Vertical Metric Linegaps.

[thundernixon]

Observed behaviour

I'm getting the warning from Google Fonts check 041:

⚠️ WARN: Checking Vertical Metric Linegaps.
– hhea lineGap is not equal to 0. [code: hhea]

Expected behaviour

Based on the GF-docs specification of vertical metrics, I expect the lineGap to be ¼ of the UPM:

Typo LineGap = 0.25 * UPM
Hhea LineGap = Typo LineGap

These two specs contradict one another, correct?

Additionally, I have run fixfonts.py from gftools on the source, which I would expect to set the correct lineGap.

Should I ignore the warning, or have I missed something? Is the warning correct and simply relevant for other fonts? Or, is it something that could be updated to reflect the GFdocs spec?

Resources and exact process needed to replicate

My latest VF build and the associated FontBakery report is here: https://github.com/thundernixon/Libre-Caslon/tree/5792567921c8df8c23ddeabf56b27c9250f0baaa/dist/2018-11-06-21_09

In the parent Libre Caslon repo, the build script can be run in a py3 environment with scripts/build.sh. Two gftools lines are currently failing (gftools-fix-nonhinting.py & gftools fix-gasp), but the rest will build and be put a VF and report into a new, timestamped folder inside dist. (Yes, I'll clean this working directory up when this font is published).

[felipesanches]

There's absolutely no explanation on the code of why this checking routine expects linegaps to be always equal to zero:

https://github.com/googlefonts/fontbakery/blob/28427a87ae9a3b963997dd6562a877bed3d8e166/Lib/fontbakery/specifications/hhea.py#L11-L21

We need to figure out why we have that criteria (when did we decided we should check for that andwhy?) and then document that on rationale metadata for that check.

[felipesanches]

on commit 41f8bbf @m4rc1e broke up an older check called "Checking vertical metrics" into three new and simpler checks:

• "Checking OS/2 usWinAscent & usWinDescent" - nowadays called check/040
• "Checking Vertical Metric Linegaps" - nowadays called check/041
• "Checking OS/2 Metrics match hhea Metrics" - nowadays called check/042

[felipesanches]

That old check was inherited from the codebase I started working on years ago. This is the commit in which it was ported by myself: 5728f63

[felipesanches]

Still no clue about the reasoning behind the requirement of having linegaps equal to zero.But that's what the "fixer" did (when we used to fiddle with hot-fixing font-binaries).

[felipesanches]

this is when the VmetFixer class was created: 9ea53c3

[felipesanches]

it was simply a metricfix method living at bakery_cli/scripts/vmet.py by late 2014...

[felipesanches]

Commit f2296c5 brought lots of code from a different code repository (it is unclear whether this is still available online somewhere), so we may have reached a dead-end in this software archaeology effort looking for the original rationale behind this check :-P

There's maybe an easier way to answer that question:
@davelab6 Was this checking routine requested by you a long-long time ago (before 2014)? If so, what was the idea behind this?

[felipesanches]

ok, it seems it was requirements.txt:git+https://github.com/googlefonts/fontbakery-cli.git

that repo seems to have been deleted, duh! Someone deleted useful history perhaps... I guess that repo had issues with requests for new checks and maybe explanations of the rationale behind them.

[felipesanches]

Based on the GF-docs spec, I think I found some hints to the reasoning here...

• when "Recalculating the vertical metrics for an upgraded family"
• in case "Use_Typo_Metrics was not enabled in the previous release"
• then "Typo LineGap = 0. Win Metrics has no LineGap parameter so we set this to 0"

@davelab6 can you clarify the explanation?

[felipesanches]

It seems that the check could then be updated to emit a FAIL if linegap is set to anything different than zero or 0.25 * units-per-em.

Then for linegap equal to zero the WARN message should mention that "a value of linegap=0 is only acceptable for old font releases which did not enable fsSelection.useTypoMetrics bit. If you are working on a new font project, that bit must be enabled and linegap must be set to 25% of units-per-em".

We may also want to create a new check for the fsSelection.useTypoMetrics bit. I just noticed we lack that.

@davelab6 do you agree with all the above?

[m4rc1e]

We have multiple sources of truth here which isn't good. Since Dave wants hhea metrics and typo metrics to match, we should update this doc and fb checks.

I will happily take this on as a task next week. I'll also update the doc due to new discoveries:

Setting metrics for new families

I've recently found including linegaps to be a problem. When I had linegaps, the accents on the first line of text would often get clipped in primitive editors. It would've been more beneficial to include the linegap values in the ascenders to stop this from happening.

I recently pushed a lot of Thai families which had the following setup:

• TypoAscender and hheaAscender are set to height of tallest Cap glyph with single accent (Â, Å)
• Linegaps set to 0
• TypoDescender and hheaDescender set to lowest a-z letter (p, j, q)
• Win Ascent and Win Decent set to yMax and yMin
• fsSelection bit 7 enabled
• Vertical metrics on average were around 130% of upm. I felt this number was the sweet spot. Anything greater and the metrics just looked too loose.

Yes Vietnamese text will produce collisions but the user can solve this by increasing css line-height or increasing linespacing in editors. I think this approach is better than having metrics which are so loose you can drive a bus through each line.

Setting metrics for upgraded families

If hhea and typo don't match, we must decide which set must be used. I suggest that hhea metrics should be changed to match the typo because there are more win/android users than Mac citation needed.

[davelab6]

I agree with Marc

[thundernixon]

If hhea and typo don't match, we must decide which set must be used. I suggest that hhea metrics should be changed to match the typo because there are more win/android users than Mac citation needed.

Citation probably not needed, but here's one anyway:

http://gs.statcounter.com/os-market-share

74.66% = Windows + Android
20.16% = iOS + macOS

Stats are based on aggregate data collected by StatCounter on a sample exceeding 10 billion pageviews per month collected from across the StatCounter network of more than 2 million websites. Stats are updated and made available every day, however are subject to quality assurance testing and revision for 45 days from publication. About Statcounter

[m4rc1e]

@thundernixon thank you.

Going forward we should change the hhea to match the typo, unless both sets of metrics are terrible in which case we should discuss what to do.

[thundernixon]

@felipesanches nice git archaeology work!

@m4rc1e if the "TypoAscender and hheaAscender are set to height of tallest Cap glyph with single accent (Â, Å)", would the actual value of actual typographic ascenders be preserved anywhere in the output font file? Looking around the tables in a TTX output and in the MS Typography OpenType spec, I don't see anywhere else this would be recorded. I would worry that we might be removing a useful piece of metadata – and therefore causing potential collateral damage to future applications – just to support primitive editors.

Also, when we update the spec, we should probably also update fixfonts.py from gftools, correct?

[thundernixon]

I suppose future applications could potentially just measure the height of the lowercase d or something, if they did need the general ascender height. That feels a bit hacky, though.

[thundernixon]

That is how FontMetrics.js computes ascender height, and it is probably quite a lot simpler than digging into a font file for metadata. So, I'm fairly convinced that matching hhea & typo ascenders to the yMax is worthwhile.

[m4rc1e]

What happens if the yMax is 2580?

[thundernixon]

Sorry, I misused the term yMax.

I agree with your proposed metrics spec, including that TypoAscender and hheaAscender be set to height of tallest Cap glyph with single accent (probably Â, Å)

[m4rc1e]

Deleted my previous comment since it's no longer relevant.

While I'm clarifying our vert metric docs and tests, I'll provide evidence/research to support it. I'd like to dig deeper on this,

[thundernixon]

I misunderstood the meaning of the MS definition of sTypoAscender & sTypoDescender in the tables: "The typographic ascender/descender for this font." I confused "typographic ascender" as a description of how Latin glyphs are drawn. Their recommendations section provides a definition which is much clearer to me:

sTypoAscender should be used to determine an optimal default offset from the top of a text frame to the first baseline. Similarly, sTypoDescender should be used to determine an offset from the last baseline to the bottom of the text frame.

https://docs.microsoft.com/en-us/typography/opentype/spec/recom#tad

[thundernixon]

Sorry, was typing an additional comment that I meant to get rid of, but accidentally hit the "close and comment" button. I guess I need to take a break for now. 😄

[thundernixon]

@m4rc1e Quick question on this: you said...

TypoDescender and hheaDescender set to lowest a-z letter (p, j, q)

...but does this include or exclude diacritics below the baseline (e.g. /rcommaaccent)? It seems like you were indicating just base letters, but I'm setting the values and wanted to verify.

[thundernixon]

This would be a big win to figure out and close out on. (Maybe it's a good p0 issue?)

rsms/inter#124 shows an example where solid direction and checks for vertical metrics would prevent some major layout issues.

[m4rc1e]

We still don't know what's causing that issue so it may not even be vertical metrics.

[thundernixon]

Good point, I am jumping to conclusions. It needs further research before making a conclusive update here.

[kalapi]

@m4rc1e Quick question on this: you said...

TypoDescender and hheaDescender set to lowest a-z letter (p, j, q)

...but does this include or exclude diacritics below the baseline (e.g. /rcommaaccent)? It seems like you were indicating just base letters, but I'm setting the values and wanted to verify.

Any updates to this?