googlefonts / ufo2ft

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

Go halfway back to the old kern writer #858

Closed madig closed 2 months ago

madig commented 4 months ago

Take the new kern writer and rework it to split by direction again, like the old writer. Replace the old writer.

Internal testing showed that the new writer likes to stick together a lot of scripts in fonts that employ cross-script kerning for ease of design, negating the advantages of the script splitter. We had the idea of reworking the splitter to put things into the same lookup instead and insert subtable breaks by script, but then we found that repacking the kerning generated by the old writer with Harfbuzz and then using the GPOS compression in fonttools actually gets us back to the small sizes by the original splitter at reasonable compile times. Maybe there is no need to try and be smarter.

We decided to propose going back to the old writer that splits by direction (plus the detail enhancements the new one got) and use GPOS compression instead for releasing fonts. The resulting kerning will work in InDesign with the default composer and in other apps that allow cross-script kerning. The difference is that it will always work, not just when the font has cross-script kerning between the desired scripts.

I took the opportunity to refactor the code a bit, by copy-pasting and adapting the new writer piece by piece and getting the important business logic in a straight line. The writer has been validated with this branch of kerning-validator, which was modified to generate all glyph pairs that are allowed by (my understanding of the) BiDi logic to be in the same run.

I rewrote the tests to use syrupy for snapshot testing, because it made it easier to update tests after changing the writer. I also ported all the tests to use plain pytest conventions instead of unittest.

TODO:

madig commented 3 months ago

Cosimo suggests replacing the old writer with the new old writer but leave the default writer alone.

anthrotype commented 3 months ago

Take the new kern writer and rework it to split by direction again

could you summarize what actually changed in the resulting generated kern feature compared to the old kernFeatureWriter2.py?

madig commented 2 months ago

could you summarize what actually changed in the resulting generated kern feature compared to the old kernFeatureWriter2.py?

I think the primary difference is that the kern feature is always structured like

script languages lookups

whereas the old writer would try to leave out script statements if it could. The lookups should be about the same, with some detail improvements that landed in the newer one like https://github.com/googlefonts/ufo2ft/pull/720.

anthrotype commented 2 months ago

the fact that the tests were overhauled doesn't make that super clear, I wish you had split the two changes

madig commented 2 months ago

You mean keep the old test structure but insert new results, and then overhaul it? I could spend some time on that, but not sure if this week.

anthrotype commented 2 months ago

You mean keep the old test structure but insert new results, and then overhaul it?

maybe the reverse (change the tests infra first, then change the code, let the tests fail so we can actually see what changed, then update the test expectations), or whichever is easier. As long as the two sets of changes (to the tests and the lib itself) are kept separate

madig commented 2 months ago

Continued in https://github.com/googlefonts/ufo2ft/pull/870.