Don’t set IgnoreMarks flag if there is no GDEF table

[khaledhosny]

See notofonts/noto-fonts#984

Alternatively, always generate a GDEF table.

[anthrotype]

@khaledhosny thanks

so is it just the ots complaining, or is it actually invalid to have that flag without a gdef? how does harfbuzz behave in the presence of that?

[khaledhosny]

The spec says:

IgnoreBaseGlyphs, IgnoreLigatures, or IgnoreMarks refer to base glyphs, ligatures and marks as defined in the Glyph Class Definition Table in the GDEF table. If any of these flags are set, a Glyph Class Definition Table must be present.

(note that not only GDEF is needed, but also it needs to have a Glyph Class Definition Table).

But whether this has any effect in practice is debatable, HarfBuzz certainly has no issue with such fonts (it also synthesizes a GDEF table from character properties if one is missing IIRC).

[anthrotype]

I see. Well, the KernFeatureWriter has an option to turn that off. The FeatureCompiler should maybe pass that argument if it cannot find any GDEF with GlyphGlassDef statements in the features.fea.
Wdyt?

[anthrotype]

or maybe better, the kern writer should not write ignoreMarks, even when the argument defaults to True, if there's no GDEF in features.fea

[khaledhosny]

Sounds good. But one complication is that feaLib can generate a GDEF table in its own even if the feature file does not specify any. I don’t know, this is getting complicated.

[anthrotype]

what's the condition that triggers feaLib to generate a GDEF? we can use the same

[khaledhosny]

It always generates one, but only saves it if it has any useful data, see https://github.com/fonttools/fonttools/blob/master/Lib/fontTools/feaLib/builder.py#L396

[anthrotype]

hm.. so the kern feature writer alone won't have all that information. I don't know.
If a shaper breaks because of the presence of that flag (despite the "must" in the spec wording), then I would worry. Otherwise perhaps... ¯\(ツ)/¯

[anthrotype]

ok, I think we should do this: set ignoreMarks by default in kern, but only when a GDEF table with GlyphClassDef (and actual marks with class 3) is present. Otherwise, well.. feaLib is going to make GDEF if it finds any markClass definitions (if the markFeatureWriter has done its job, for example), and the kern lookups won't have ignoreMarks (which is not the end of the world).

Or is the latter situation (missing ignoreMarks flag) worse than the other one we are trying to avoid (flag set but no GDEF)?

[behdad]

I'm against this. Let me read the full report and reply in detail.

[anthrotype]

thanks. I have no rush to fix this

[behdad]

The spec says:

IgnoreBaseGlyphs, IgnoreLigatures, or IgnoreMarks refer to base glyphs, ligatures and marks as defined in the Glyph Class Definition Table in the GDEF table. If any of these flags are set, a Glyph Class Definition Table must be present.

(note that not only GDEF is needed, but also it needs to have a Glyph Class Definition Table).

This is yet another bogus and inconsistent wording in the OpenType spec. Please do NOT follow it. Maybe @PeterCon can fix it. Last time I reported a one-line change to the spec to allow null DefaultLangSys in DFLT script, so far couldn't get that in, so I'm not holding my breathe.

Here's the bogus part: the GDEF table says:

In addition, a client uses class definitions to apply GSUB and GPOS LookupFlag data correctly. For example, a LookupFlag may specify ignoring ligatures and marks during a glyph operation. If the font does not include a GlyphClassDef table, the client must define and maintain this information when using the GSUB and GPOS tables.

So, IMO, the spec makes it clear that it's perfectly legal to have a null GlyphClassDef while using IgnoreMarks. I'll extend that to say a missing GDEF should also be accepted, because, if there's no data to encode in GDEF, it should be discarded. This whole business of "if this then that must be present" is non-sense. It feels to me like someone wrote down their understanding of how the spec is supposed to work into the spec itself. In reality, one should encode GDEF if they need to encode it to get their font work the way they want. Period.

[behdad]

Same thing about, eg, MarkFilteringSet numbers. If you use a set number that refers to a nonexisting set, client should just assume an empty set. There's nothing structurally wrong with this font, and OTS definitely should not be in the business of rejecting those. Again, this is an instance of the issue that OTS doesn't seem to know what its goal is. It was originally written to protect against crashing code we don't control, but somehow during implementation it became a spec addict. Not useful.

[khaledhosny]

OK, the spec is contradicting itself so I can chose which part to follow 😄. Since we don’t have any indication that not having GlyphClassDef causes any issues, I can just fix OTS.

[behdad]

Not only not having GlyphCassDef does not cause any issues, it's a valid and efficient way to assign Unicode marks a mark class so they are properly skipped for kerning, ligatures, etc, and attached, ...