fonttools / fontbakery

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

Ignore unreachable alt caron in OTF fonts #4767

Closed arrowtype closed 1 week ago

arrowtype commented 2 weeks ago

Description

In making Latin script fonts, it is common to create the glyph /caroncomb.alt, which is then used as a component in glyphs /lcaron, /dcaron, /tcaron, and /Lcaron. (More info here.)

This /caroncomb.alt is only used as a component, and doesn't have a Unicode value, as far as I am aware. So, it is unreachable by itself. Thankfully, it is ignored by the check com.google.fonts/check/unreachable_glyphs in TTF fonts, as there, it is possible to ignore glyphs used as components, thanks to https://github.com/fonttools/fontbakery/issues/4378.

However, it is not ignore in OTF fonts, where there isn’t the same system of components. So, if users have correctly set up /caroncomb.alt, they get a WARN for this glyphs as being unreachable, when checking OTF fonts.

This PR simply removes glyph names /caroncomb.alt and /uni030C.alt from the check, when the font being checked is an OTF. I’ve tried to write the ignore list to be extendable, if there are other common cases that arise.

It is very possible that there is a better way to check whether a font is an OTF, or that there is some better way to detect this kind of case in an OTF. Or, perhaps we want to enforce some kind of removal of this glyph in OTFs, though it’s hard to imagine that such a post-build process would be worth the added complexity.

Anyway, I’m very happy to have a discussion or receive feedback, here! Thanks for reviewing this PR.

Checklist

dscorbett commented 2 weeks ago

If it is possible for this check to detect the unreachable glyph, shouldn’t it be equally straightforward for the build process to delete the unreachable glyph?

simoncozens commented 2 weeks ago

That's an unbelievably specific case. Is there not a more general principle we could check instead?

arrowtype commented 1 week ago

That's an unbelievably specific case.

This is the default name GlyphsApp gives the /caroncomb.alt, and FontMake doesn’t delete that glyph when building OTFs.

@simoncozens Is designing a font in Glyphs and building with FontMake really that specific of a use case? Or have I missed something that somehow makes my use case weirdly specific?

image
simoncozens commented 1 week ago

Now I understand the problem better - fontbakery is correct here. You have added an exported glyph to your font which is only used in a component. It's not accessible any other way. Because of OTF subroutinization it's not used directly as a component. So the glyph itself is completely dead weight in the font. Fontbakery absolutely should report that - that's the whole point of this check. The answer is to set the glyph to unexported; it will still be used to build the component, but not added as a discrete glyph. That's what this check is encouraging you to do.

arrowtype commented 1 week ago

Oh, I didn’t realize that would work! I’ve tested it out, though, and it works via both Glyphs exports and FontMake builds. Makes sense, in hindsight. Thanks for the review and helpful insight, @simoncozens!

arrowtype commented 1 week ago

@simoncozens This information might be helpful to others who are in a similar scenario... do you think a PR would be considered if I attempt to add a note about non-exporting accents in Glyphs? We could either link it to the presence of /uni030C.alt, or simply add it to the warning in general. Or, if you think it would be silly to include, I can just make a note of it on my end.

felipesanches commented 1 week ago

yes, I think it would be useful to show this tip somewhere. Either in the rationale text or in the WARN message.