googlefonts / ufo2ft

A bridge from UFOs to FontTools objects (and therefore, OTFs and TTFs).
MIT License
151 stars 43 forks source link

No mark feature code for composite marks #585

Closed yanone closed 2 years ago

yanone commented 2 years ago

Please see attached minimal font: CompositeMarkToBase.glyphs.zip

Mark to base feature code is generated for shaddaFatha-ar (right in picture), which I've decomposed, but not for shaddaDamma-ar (left), which is composed of two stacked mark glyphs. Glyphs.app generates this correctly, as this is common conduct.

Use this test string: رَّرُّ

Bildschirmfoto 2022-02-10 um 16 17 19

As always, I've tried to dig into the code myself, but got lost, so I'm posting this here. I couldn't even find the code where base glyphs get parsed for their final anchor positions. But basically, the same approach needs to be used for marks: Dynamically calculate their _bottom and _top anchors when they are composites.

I'm always happy to help with the code, but couldn't get any further.

anthrotype commented 2 years ago

for the markFeatureWriter, it shouldn't matter if the glyphs are simple contours or composites, what matters is their GDEF classification as bases, marks, ligatures. The markFeatureWriter decides whether a glyph is a "mark" based on its explicit GDEF classification (the GlyphClassDef statement in features.fea), or in lack of that, based on the presence of _-prefixed anchors (aka "mark anchors). So it could be that the glyph shaddaDamma-ar doesn't get clasified as a mark for one of the two reason: namely, incorrect/missing GDEF classification (I would check the GlyphData.xml contains the Mark/Nonspacing category/subCategory for that glyph name); or the glyph contains no mark anchors.

Since you mentioned composite glyphs, it could also be the anchor propagation feature in glyphsLib, that copies anchors from base glyphs to their components in composite glyphs. Maybe your composite glyph didn't get its anchor correctly propagated, in which case it's an issue in glyphsLib, rather than ufo2ft. I hope that makes sense

yanone commented 2 years ago

Yes, makes sense. I'll take a look

yanone commented 2 years ago

Got it. The glyph name shaddaDamma-ar isn't getting recognized as a ligature in glyphsLib. Renaming them to shadda_damma-ar works. This is inconvenient as Glyphs.app proposes the former name. I'll see if I can write up a fix for that over on glyphsLib.