Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip overlapping contour check for variable fonts #65

Open
HinTak opened this issue Oct 23, 2018 · 4 comments
Open

Skip overlapping contour check for variable fonts #65

HinTak opened this issue Oct 23, 2018 · 4 comments

Comments

@HinTak
Copy link

HinTak commented Oct 23, 2018

Copied comment from fonttools/fontbakery@c02825b

This was discussed in #2056
with @madig already. The correct way to approach this issue is to switch off those checks and not do them for variable fonts. Doing the checks then throw them away, is wasting CPU cycles. Not doing the tests also run faster and a better experience for the users. The correct way to switch it off is to put that logic in Fontval to detect variable fonts. There is already examples of such code usage in the ELDC/EBDC table checks where decisions are made based on the presence or absence of another table.

@madig @davelab6 cc

HinTak referenced this issue in fonttools/fontbakery Oct 23, 2018
Variable Fonts typically have lots of intersecting contours and components.
@thundernixon
Copy link

When I run a FontBakery test on a font I'm mastering, I get several errors that I am confused by, and which I can't find documentation for:

  • ℹ️ INFO MS-FonVal: Unable to perform test due to previously detected errors DETAILS:
    • Glyph index 1 Test: ValidateSimpContMisor
    • Glyph index 10 Test: ValidateSimpContMisor
    • Glyph index 26 Test: ValidateSimpContMisor
    • Glyph index 38 Test: ValidateSimpContMisor
    • Glyph index 44 Test: ValidateSimpContMisor
    • Glyph index 69 Test: ValidateSimpContMisor
    • Glyph index 76 Test: ValidateSimpContMisor
    • Glyph index 91 Test: ValidateSimpContMisor
    • Glyph index 105 Test: ValidateSimpContMisor
    • Glyph index 107 Test: ValidateSimpContMisor
    • NOTE: 37 other similar results were hidden!

The naming of this error (and what I can understand in the fontval code) indicates that the message is about Contour Misorientation. However, the glyphs listed have similar contour directions to other glyphs.

However, each of the glyphs listed here has overlaps, while those excluded tend to be individual shapes. Is it safe to assume that this stems from the same issue, or is this something separate?

If helpful, here's the font triggering this check, the TTX xml, and the Font Bakery report:
https:/thundernixon/Libre-Caslon/tree/2622969efd19f148fa17536345b085441fb723c9/dist/2018-11-07-15_29

@HinTak
Copy link
Author

HinTak commented Nov 7, 2018

I don't know - I think you should run FontVal bare and read the report. At least messages are in the right order. I am not sure whether test cannot continue because of contour mis-orientation, or that the mis-orientation test was not performed because of previous (other) problems. I do not want to look at FB code to find out.

I'll have a look at the font itself at some point in time, but not right away.

@thundernixon
Copy link

Thanks for your comments and the recommendation, @HinTak. I ran it in the Mac GUI from Georg Seifert, and found that some of the outlines did have incorrect path directions.

Once I fixed that, it appeared that anything with overlap triggered this error/info combo:

| E1111 | Intersecting contours |
| I1111 | Unable to perform test due to previously detected errors |

image

So, I just wanted to report this as something else that probably could be upgraded when Variable Font updates are supported.

@HinTak
Copy link
Author

HinTak commented Nov 8, 2018

Thanks for the investigation. Yes, I filed this issue because overlapping contours are common/expected of variable fonts. I'll get to it one day, when I finish the other more urgent/important issues (like getting building with visual studio to work again, after Microsoft removed it at the beginning...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants