Fontbakery: Make sure fsSelection bit 7 is enabled

[RosaWagner]

I thought we already had a check for that, but 3 fonts currently being reviewed are not having it enabled and FB didn't fail.

[vv-monsalve]

The only check I found in the google profile for fsSelection is about RIBBI stylename relations, but not the Use of Typo Metrics.
https://font-bakery.readthedocs.io/en/stable/fontbakery/profiles/googlefonts.html#com.google.fonts/check/fsselection

[m4rc1e]

We shouldn't require all fonts to have fsSelection bit 7 enabled since this will fail a tonne of older families and for static fonts it isn't a requirement according to the MS Spec. You want the bit enabled if you're updating a family and are changing to win metrics to avoid clipping issues.

The problem isn't with the fsSelection check, it's with the vertical regressions metric check, https://github.com/googlefonts/fontbakery/blob/main/Lib/fontbakery/profiles/googlefonts.py#L4404-L4405. We need add a conditional so that if both fonts don't have bit 7 enabled (which is the case in Be Vietnam) and the win metrics are different, it should fail.

[vv-monsalve]

We need add a conditional so that if both fonts don't have bit 7 enabled (which is the case in Be Vietnam) and the win metrics are different, it should fail.

Being the Use Typo Metrics mandatory, wouldn't it be also needed for new fonts? Particularly if for any reason (e.g. the charset supported, like math glyphs) it has very high values for win metrics.

Maybe (a) it could have a note for older families in the rational or (b) it could have a WARN result instead of a Fail. I think having this check would be of great help.

[m4rc1e]

Being the Use Typo Metrics mandatory, wouldn't it be also needed for new fonts?

I think requiring bit 7 to be enabled for new families is actually a good idea since it will stop future metric incompatibilities if the family is upgraded. For existing families, we should just ignore it.

[chrissimpkins]

Is it possible to distinguish new vs old families so that you can add it in an upstream profile? I have the source for this check in a downstream profile and can open a PR on an upstream profile or add a custom profile template that includes the check and googlefonts profile checks for use with new families.

[m4rc1e]

Sorry Chris but I haven't worked on any upstream profiles so I don't know.

I guess you want this for your project? Perhaps you could override the remote_styles conditional so that it fetches the tagged releases it could work.

[chrissimpkins]

I guess you want this for your project?

We have the check in a custom profile. I would be happy to push the source upstream in some fashion so that you have access to the check.

[felipesanches]

@chrissimpkins, please either create a PR adding the check to the googlefonts profile, or simply copy & paste the implementation of the check and I can take care of adding it to Font Bakery

[chrissimpkins]

@chrissimpkins, please either create a PR adding the check to the googlefonts profile, or simply copy & paste the implementation of the check and I can take care of adding it to Font Bakery

https://github.com/googlefonts/fontbakery/pull/3314

[chrissimpkins]

3314 is nearly ready. One minor hiccup in the unit tests with CJK fonts and then should be ready to merge.

[bobh0303]

Could someone help me understand this? This test is only used for non-CJK fonts, and for those com.google.fonts/check/os2_metrics_match_hhea FAILs if the the OS/2.sTypoAscender/Descender values do not equal the hhea.ascent/descent values, and com.google.fonts/check/linegaps WARNs if the two linegap metrics are not zero. If both these tests pass, then the USE_TYPO_METRICS bit would be a don't-care, wouldn't it? That is, line metrics would be the same no matter which metrics are being used.

[RosaWagner]

@bobh0303 use_typo_metric is to prioritize Typo metrics over Win metrics

[bobh0303]

I am aware of what USE_TYPO_METRICS does. I'll write my question in another way:

Using a font for which both com.google.fonts/check/os2_metrics_match_hhea and com.google.fonts/check/linegaps PASS, can you/anyone name an environment where the value of USE_TYPO_METRICS makes a difference in rendering?

If not, then there is no reason to require any particular value for bit 7 unless one of those tests WARNs.

[felipesanches]

The use of USE_TYPO_METRICS is part of the grand-scheme of vertical metrics settings established as a Google Fonts policy aiming at a common ground supported by all major font rendering environments. To be honest, I am not able at this moment to pinpoint the specific reasons for each decision, even though I know that this scheme was deliberately designed by @m4rc1e, @RosaWagner and others involved in maintaining the Google Fonts collection.

Since FontBakery aims at providing this kind of info so that we do not have to depend on dogmatic ideas on how things should be done, I agree that we probably need better documentation of this scheme on the rationale statements of these checks.

I believe that the GF Spec explains the vertical metrics strategy in details: https://github.com/googlefonts/gf-docs/blob/main/VerticalMetrics/README.md

[felipesanches]

I quote a relevant part of that strategy here:

5. Use_Typo_Metrics must be enabled

This will force MS Applications to use the OS/2 Typo values instead of the Win values. By doing this, we can freely set the Win values to avoid clipping and control the line height with the typo values. It has the added benefit of future line height compatibility. When a new script is added, we simply change the Win values to the new yMin and yMax, without needing to worry if the line height have changed.

[RosaWagner]

If win typo and hhea metrics are matching, there is no need for use_typo_metrics. But for the fonts we onboard at google, it is a very rare case - win metrics often needing to be greater than the others. Thus, it doesn’t kill to enable it in order to avoid errors with future updates including language expansion resulting in adapting the win metrics.

Also the check is new, all edge cases are still not covered by it.

[RosaWagner]

@bobh0303 sorry for the misunderstanding. Did we bring an answer to your question?

[felipesanches]

@RosaWagner, at which severity level would you classify this check?

note: from 1 (lowest) to 10 (highest)

[felipesanches]

The main task of this issue was now done. We have the new check and now it has much more precise wording on its rationale so that it is clear why the policy is in place and there's even a reference to the online documentation for users to learn more about it. So I'll close this issue now.

@RosaWagner, at which severity level would you classify this check?

Feel free to answer this later by opening a new issue and I'll add the severity metadata in an upcoming PR

[vv-monsalve]

@RosaWagner, at which severity level would you classify this check?

note: from 1 (lowest) to 10 (highest)

I would like to propose a 10. Enabling this allows:

• Setting the Win Metrics to the max/min font's bounding box avoiding clipping issues. ​
• Ease update processes, since the prioritization of Typo Metrics will allow to update the Win Metrics with no regressions conflict.
• Prevent having loose line heights as fsSelection bit 7 mechanism is supposed to indicate to software that the OS/2 typo metrics should be used for linespacing instead of the win metrics, avoiding :

[bobh0303]

@bobh0303 sorry for the misunderstanding. Did we bring an answer to your question?

First, I realize now my question was incorrect as stated. Let me re-phrase more accurately:

I believe that IF all of these conditions are true:

• hhea.ascent = OS/2.sTypoAscender = OS/2.usWinAscent
• hhea.descent = OS/2.sTypoDescender = - OS/2.usWinDescent
• hhea.linegap = OS/2.sTypoLineGap

THEN the line metrics will be the same whether or not USE_TYPO_METRICS is set and whether or not it is obeyed by software.

Does the above match your understanding?

If the above is accurate, then for fonts that meet above conditions, what reasons are there to FAIL them for having the USE_TYPO_METRICS bit set to zero?

Maybe I'm missing something, but the arguments presented don't seem to hold for this case. For example, if a subsequent font version has a clipping issue, enlarging usWinAscent/Descent and _now_ setting USE_TYPO_METRICS solves the problem and doesn't cause a line metrics regression.

[vv-monsalve]

THEN the line metrics will be the same whether or not USE_TYPO_METRICS is set and whether or not it is obeyed by software.

As Rosa said above "But for the fonts we onboard at google, it is a very rare case - win metrics often needing to be greater than the others." So it would be more like an exception.

For those cases, it would be possible to (a) If you assert a FAIL is fine, leave a note for the project to open a discussion to agree to make an exception to accept the FAIL. (b) as Rosa also mentioned, it doesn’t kill to enable it in order to avoid errors with future updates including language expansion resulting in adapting the win metrics.

Maybe I'm missing something, but the arguments presented don't seem to hold for this case. For example, if a subsequent font version has a clipping issue, enlarging usWinAscent/Descent and now setting USE_TYPO_METRICS solves the problem and doesn't cause a line metrics regression.

The checks are thought considering the most common situations. In this case, not only the difference in the Win Metrics for most font projects but also that very often the projects are updated by a person different to the original designer, which would lead us to the above (b) point.