fonttools / fontbakery

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

ots-sanitize FAIL: GDEF Ligature carets #2268

Closed mjlagattuta closed 5 years ago

mjlagattuta commented 5 years ago

Observed behaviour

While working on Roboto Slab VF I encountered the following FAIL output from ots-sanitize:

>> com.google.fonts/check/036 with (('font[0]', 'RobotoSlab-VF.ttf'),)
   Checking with ots-sanitize.
 * FAIL: ots-sanitize returned an error code (1). Output follows:

ERROR: GDEF: bad caret value format: 3
ERROR: GDEF: Invalid ligature caret list
ERROR: GDEF: Failed to parse table
Failed to sanitize file!

   Result: FAIL

I am currently lacking an understanding of what is causing this to fail, as well as what constitutes an invalid ligature caret list. If anyone has any insight or has encountered this please let me know!

Expected behaviour

Perhaps fontbakery can have a more detailed output to clarify why the ligature caret list is invalid, or what constitutes a bad caret value? If it is only a specific ligature that is problematic then calling that out would be ideal.

Resources and exact process needed to replicate

See the upstream Roboto Slab Repo

mjlagattuta commented 5 years ago

I have looked into this a bit more and I suspect this may be an issue with ots-sanitize, as caret value format 3 is detailed in the spec and is valid in VFs. If this is the case, I will file an issue in the ots repo.

From the spec:

CaretValue Format 3 The third format (CaretValueFormat3) also specifies the value in design units, but, in non-variable fonts, it uses a Device table rather than a contour point to adjust the value. This format offers the advantage of fine-tuning the Coordinate value for any device resolution. (For more information about Device tables, see the chapter, Common Table Formats.)

In variable fonts, CaretValueFormat3 must be used to reference variation data to adjust caret positions for different variation instances, if needed. In this case, CaretValueFormat3 specifies an offset to a VariationIndex table, which is a variant of the Device table used for variations.

mjlagattuta commented 5 years ago

This issue has already been posted in the ots repo by @thundernixon: https://github.com/khaledhosny/ots/issues/178

As far as fontbakery is concerned, until ots is fixed, should there be a workaround to modify the output of ots sanitize when it comes to CaretValue3? or should user's know to ignore it until it is fixed?

thundernixon commented 5 years ago

I've learned from @m4rc1e that if OTS is failing a font, the font won't work in Firefox.

So, for now, it's important to remove carets from ligatures, until OTS is updated to properly handle them.

I'm not quite sure how FontBakery should handle this. FontBakery is suggesting a correct thing in check 064, but following this recommendation, for the time being, will break fonts.

My thought is that two approaches would be logical:

  1. The quick solution: making check 064 an INFO check, and adding a recommendation against adding ligature carets. This recommendation would then be removed/updated once OTS has been patched for this.

  2. The hard but better solution: doing a pull request to OTS to fix the root issue.

I don't know that I would have the technical skills to lead solution 2, but I could do a PR here for solution 1 if that is the preferred approach.

@davelab6 what would you suggest on this?

m4rc1e commented 5 years ago

@thundernixon I think this was only the case for FF nightly. However, I still think our fonts should clear OTS.

thundernixon commented 5 years ago

Ah, okay, thanks for adding that. Yeah, I guess this is a question of which one we prioritize. For now, I'll remove temporarily rename ligature caret anchors (so fontmake won't export them as carets, but that'll be easy to reverse in the future).

khaledhosny commented 5 years ago

Ligature carets are broken only in variable font AFAIK, but they still work for non-variable fonts (while still of limited value since most apps, including all major web browsers, don’t use them).

thundernixon commented 5 years ago

That's a useful clarification. Thanks!

mjlagattuta commented 5 years ago

I just tested Roboto Slab VF in axis praxis using the latest FF and FF Nightly. It seems to be fine in FF and I can confirm that it does not even render in FF Nightly.

Since VFs with caret value format 3 appear to work properly in the stable FF, is there a strong case for us to not use ligature carets?

felipesanches commented 5 years ago

@davelab6: "we'd better get started, then :D"

I've sent a pull request to khaledhosny/ots bumping up the acceptable format value for ligature carets.

My local build of OTS now passes EncodeSans-VF.ttf:

captura de tela 2019-01-18 as 12 57 29
davelab6 commented 5 years ago

Thanks @felipesanches ! Mike, Stephen, Marc, Felipe and I discusssed this on a call today and Marc said to drop ligature carets from the VF builds for now.

felipesanches commented 5 years ago

There's nothing here to do on Fontbakery itself.