search

fonttools / fontbakery

🧁 A font quality assurance tool for everyone
https://fontbakery.readthedocs.io
Apache License 2.0
534 stars 99 forks source link

[com.google.fonts/check/vertical_metrics]: Downgrade FAIL to WARN for 120% of upm #4669

Closed m4rc1e closed 2 months ago

m4rc1e commented 2 months ago

Observed behaviour

Check is failing for Tiny5 since it isn't 120% of upm. However, it looks just fine.

Expected behaviour

Imo, some fonts such as pixel fonts or black weights need tighter spacing. I don't think we should fail these. A warning is just fine.

felipesanches commented 2 months ago

Current rationale text:

This check generally enforces Google Fonts’ vertical metrics specifications. In particular:

  • lineGap must be 0
  • Sum of hhea ascender + abs(descender) + linegap must be between 120% and 200% of UPM
  • Warning if sum is over 150% of UPM

The threshold levels 150% (WARN) and 200% (FAIL) are somewhat arbitrarily chosen and may hint at a glaring mistake in the metrics calculations or UPM settings.

Our documentation includes further information: https://github.com/googlefonts/gf-docs/tree/main/VerticalMetrics

Are you proposing an update to gf-docs as well?

At https://github.com/googlefonts/gf-docs/tree/main/VerticalMetrics#1-calculating-the-vertical-metrics-for-a-new-family:

Expected result: vertical metrics should be around 120-130% of UPM. Anything greater, and the metrics may look too loose.

felipesanches commented 2 months ago

What I meant above is that we can relax the "must be" wording in the rationale, so that we only WARN for lower values. But still FAIL for very large values and WARN for a bit large values.

And a corresponding edit would then be also needed on gf-docs.

vv-monsalve commented 2 months ago

For what is worth, the up-to-date GF font specifications are in the GF-Guide, and these are the latest for Vertical Metrics.

Imo, some fonts such as pixel fonts or black weights need tighter spacing. I don't think we should fail these. A warning is just fine.

This approach may work for some cases, but how could FB differentiate between them?. If possible, a warning would be good. If not, it's better to continue treating it as a Fail and to evaluate each case. Vertical Metrics are sensitive, given the strictness around them in the face of updates

m4rc1e commented 2 months ago

Let's leave it as is and I'll just ignore it for this font. Thanks for the input.