Add check to ensure a base glyph isn't classified as a mark glyph in GDEF table

[m4rc1e]

We pushed Hebo VF to production yesterday and a user reported that the comma and underscore are not working as intended, google/fonts#2453.

When I inspect the GDEF table, we can see that the "comma" is a class 3 aka a "Mark" glyph, https://docs.microsoft.com/en-us/typography/opentype/spec/gdef#glyph-class-definition-table

(venv) Marcs-Air:heebo marcfoley$ grep "comma" Heebo\[wght\].ttx
      <ClassDef glyph="comma" class="3"/>

When I inspect the source files, we can see a mark anchor exists in the comma.

An _ denotes that this glyph is a mark glyph

To ensure that base glyphs are not classified as a mark glyph, we could raise a fail If a glyph has an advance width greater than 0 and it exists in the gdef.GlyphClassDef as class 3.

More than happy to submit a pr.

[anthrotype]

IMO it would be better to recommend that the sources contain an explicit GDEF table definition in the UFO/features.fea (this is created automatically by fontmake for .glyphs sources using GlyphData.xml database in glyphsLib).
When there is an explicit GDEF definition in the features.fea, ufo2ft does not guesses whether a glyph is a mark by the presence of an _-prefixed anchor, but instead it uses the GDEF to classify glyphs as bases, marks and ligatures accordingly.

[anthrotype]

we could raise a fail If a glyph has an advance width greater than 0 and it exists in the gdef.GlyphClassDef as class 3

yes, that could be a signal that something is not ok, either the GDEF classifies a base glyph incorrectly as a mark, or a glyph that is supposed to be used as a mark has an incorrect advance width that should have been 0.

Note that in glyphsLib we automatically set the advance width to 0 for glyphs that are classified as nonspacing marks (via GlyphData.xml or custom glyph properties).

Technically one does not need to set width to 0 because OT layout engines would automatically do when if a glyph is classified as mark.

[m4rc1e]

IMO it would be better to recommend that the sources contain an explicit GDEF table definition in the UFO/features.fea

Thanks for the suggestion. We can add this check to the ufo profile. We can then get vendors to check their sources using this profile as well. We will still need a check for font binaries though.

we could raise a fail If a glyph has an advance width greater than 0 and it exists in the gdef.GlyphClassDef as class 3

yes, that could be a signal that something is not ok, either the GDEF classifies a base glyph incorrectly as a mark, or a glyph that is supposed to be used as a mark has an incorrect advance width that should have been 0.

So far this is the best solution I have. Another solution is to use unicodedata.category. It would raise a fail If a glyph is not in the unicode mark category but it has been specified in the GDEF as a mark. Unfortunately this approach won't work for unencoded glyphs.

Thanks for the additional info regarding OT layout engines.

[moyogo]

We can probably split this into a couple of checks:

• a check should report a fail when a non-mark character’s glyph is in the GDEF mark glyph class, and optionally warn when a mark character’s glyph is not in the GDEF mark glyph class
• a separate check could warn when a spacing glyph is in the GDEF mark glyph class.

I have a PR ready for the first check (fail when non-mark characters are in GDEF mark glyph class).

Fontmake/ufo2ft could be smarter when generating the mark features and when defining the GDEF from scratch.
Non-mark Unicode characters and glyphs that result from their substitutions should not get base-to-mark mark anchors or mark-to-mark anchors in GPOS lookups, unless they are substituted through features like abvf Above-base Forms or blwf "Below-base Forms.

[arrowtype]

This came up in a client project. I’m lucky Marc mentioned it on a call awhile back! This would be a great check to add.

IMO it would be better to recommend that the sources contain an explicit GDEF table definition in the UFO/features.fea

How would you recommend doing this? I can’t help but feeling that it could easily get pretty verbose, and therefore hard to properly maintain... but I may not be thinking about it correctly. Not trying to argue the point, just curious what this would look like in practice.

[felipesanches]

Checks for this were added on FB 0.7.26 but had bugs that were fixed today and will be included in FB 0.7.27