googlefonts / fontc

Where in we pursue oxidizing (context: https://github.com/googlefonts/oxidize) fontmake.
Apache License 2.0
62 stars 10 forks source link

Generate mark2lig lookups #794

Closed cmyr closed 1 month ago

cmyr commented 1 month ago

This is based on #793, which should go in first.

This is the last major remaining piece that we were missing in order to be able to match oswald.

This code is generating the same lookups as fontmake for oswald, but it is pretty naive; in particular it's just generating one big lookup and assuming that anchors don't collide (that is, that the same glyph is not part of multiple mark classes) whereas I believe the markFeatureWriter does the same sort of splitting for mark2lig that it does for mark2base, where it generates one lookup for each mark class?

This raises the bigger question of testing: there's currently no good way to test our feature generating code in isolation. I started working on this a bit for kerning but ran into some issues, but I think now it's worth spending some time to figure that problem out, since I think that the "last 10%" of layout is going to be much more test driven (isolate some funny quirk, write a test case, write a fix)

rsheeter commented 1 month ago

"last 10%" of layout is going to be much more test driven (isolate some funny quirk, write a test case, write a fix)

+1

rsheeter commented 1 month ago

it's just generating one big lookup and assuming that anchors don't collide (that is, that the same glyph is not part of multiple mark classes) whereas I believe the markFeatureWriter does the same sort of splitting for mark2lig

Sounds worth filing an issue for. Perhaps we can create a test case (in a follow-on) taht deliberately puts a glyph into multiple mark classes and see hwo our results compare?

anthrotype commented 1 month ago

I believe the markFeatureWriter does the same sort of splitting for mark2lig that it does for mark2base, where it generates one lookup for each mark class?

yes, groupMarkClasses=False is the default behavior now, so similarly to markToBase lookup, the mark2Liga ones will only reference one mark class at a time, and the lookups themselves are sorted alphabetically by the mark class name so the more specific ('top.alt' instead 'top') would be applied last and win

see https://github.com/googlefonts/ufo2ft/blob/779bbad84aa2bba9f8e31774e2f27fe84c8c231e/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L280-L290

test case: https://github.com/googlefonts/ufo2ft/blob/779bbad84aa2bba9f8e31774e2f27fe84c8c231e/tests/featureWriters/markFeatureWriter_test.py#L1506-L1538

cmyr commented 1 month ago

I have basically completely redone this implementation, so I'm going to close this and reopen as a separate PR.