fonttools / fontbakery

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

com.google.fonts/check/062: Is Gasp correctly set? #1708

Open m4rc1e opened 6 years ago

m4rc1e commented 6 years ago

I'm reviewing IBM Plex. For test 062 we get:

>> com.google.fonts/check/062 with ((u'font[12]', '/Users/marc/Downloads/type-master/fonts/Serif/desktop/pc/IBMPlexSerif-Thin.ttf'),)
   Is GASP table correctly set?
 * FAIL: GASP should only have 0xFFFF gaspRange, but 0x8 gaspRange was also found.
 * FAIL: GASP should only have 0xFFFF gaspRange, but 0x10 gaspRange was also found.

   Result: FAIL

The family has custom hinting so they've set the GASP to the following:

screen shot 2018-02-14 at 14 14 21

FB expects every font to have the following settings: screen shot 2018-02-14 at 14 15 45

I don't see why the GASP should be set the same for every font. Perhaps this test and its values should only apply to fonts which have autohinting instead of manual hinting?

davelab6 commented 6 years ago

Maybe we should adopt those settings globally? What's the difference between the two in actual rendering for Plex vs unhinted fonts?

On Feb 14, 2018 6:20 AM, "Marc Foley" notifications@github.com wrote:

I'm reviewing IBM Plex. For test 062 we get:

com.google.fonts/check/062 with ((u'font[12]', '/Users/marc/Downloads/type-master/fonts/Serif/desktop/pc/IBMPlexSerif-Thin.ttf'),) Is GASP table correctly set?

  • FAIL: GASP should only have 0xFFFF gaspRange, but 0x8 gaspRange was also found.
  • FAIL: GASP should only have 0xFFFF gaspRange, but 0x10 gaspRange was also found.

Result: FAIL

The family has custom hinting so they've set the GASP to the following:

[image: screen shot 2018-02-14 at 14 14 21] https://user-images.githubusercontent.com/7525512/36208631-704a8b96-1191-11e8-915e-e6b0d5b2c9da.png

FB expects every font to have the following settings: [image: screen shot 2018-02-14 at 14 15 45] https://user-images.githubusercontent.com/7525512/36208728-aa20c11e-1191-11e8-8d61-a2f137a194df.png

I don't see why the GASP should be set the same for every font. Perhaps this test and its values should only apply to fonts which have autohinting instead of manual hinting?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/googlefonts/fontbakery/issues/1708, or mute the thread https://github.com/notifications/unsubscribe-auth/AAP9y0QREQVaj_aQtpjIdEHUdc_BMXqdks5tUuvCgaJpZM4SFY0A .

m4rc1e commented 6 years ago

Just ran a quick diff test on Montserrat which uses ttfautohint.

font set before: single gasp entry fonts set after: same gasp as plex Win 7 IE9 11px

(Click to view fullsize) desktop_windows_7_ie_9 0_

If the fonts are hinted with ttf autohint, my preference is for the single gasp entry. Plex's settings only work well if the fonts have been hinted by hand. We can see the marks get distorted and more collisions occur etc. Because of this I don't think we should roll it out globally.

I still think we need a GASP test. It should be "If the font has been released, do the GASP entries match the previously released font?"

Something else which is rather interesting. Changing the GASP only affected Edge and IE9 on Windows.

Regression Report:

Fonts: ["Montserrat-Regular.ttf"]

View: glyphs-all_11pt
PASSED: Desktop_Windows_7_chrome_50.0_.jpg is the same
WARNING: Desktop_Windows_10_edge_15.0_.jpg is different by 55463 pixels
WARNING: Desktop_Windows_7_ie_9.0_.jpg is different by 58048 pixels
PASSED: Desktop_Windows_7_firefox_45.0_.jpg is the same
PASSED: Desktop_OS_X_El_Capitan_safari_9.1_.jpg is the same
PASSED: Google_Nexus_5_android_5.0_Android_Browser_.jpg is the same
m4rc1e commented 6 years ago

What if a family has never had a gasp table?

I'm still sticking with my original statement

I still think we need a GASP test. It should be "If the font has been released, do the GASP entries match the previously released font?"

cc @khaledhosny