googlefonts / ufo2ft

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

Kerning not active for non-default languages #699

Closed yanone closed 1 year ago

yanone commented 1 year ago

I'm guessing in the wake of the recent work on splitting kerning by scripts, kerning support got lost for non-default languages. (I'm using the lastest main codebase)

Notice how ldot get replaced for Catalan but the kerning pair Pa disappears in the lower screenshot.

Bildschirm­foto 2023-02-05 um 15 17 00 Bildschirm­foto 2023-02-05 um 15 17 08

Peeking into the feature file as produced by fontmake confirms:

feature kern {
    script DFLT;
    language dflt;
    lookup kern_Default;

    script arab;
    language dflt;
    lookup kern_Default;
    lookup kern_Arab;

    script cyrl;
    language dflt;
    lookup kern_Default;
    lookup kern_Cyrl;

    script grek;
    language dflt;
    lookup kern_Default;
    lookup kern_Grek;

    script latn;
    language dflt;
    lookup kern_Default;
    lookup kern_Latn;
} kern;
yanone commented 1 year ago

cc @madig

anthrotype commented 1 year ago

did you define latn CAT in the global languagesystems?

anthrotype commented 1 year ago

or is there another GPOS feature apart from kern that uses script latn; language CAT?

yanone commented 1 year ago

Yes, these language systems are defined:

languagesystem DFLT dflt; # Default, Default
languagesystem grek dflt; # Greek, Default
languagesystem arab dflt; # Arabic, Default
languagesystem latn dflt; # Latin, Default
languagesystem cyrl dflt; # Cyrillic, Default
languagesystem latn CAT; # Latin, Catalan
languagesystem latn NLD; # Latin, Dutch
languagesystem latn KAZ; # Latin, Kazakh
languagesystem latn TAT; # Latin, Tatar
languagesystem latn TRK; # Latin, Turkish
languagesystem latn CRT; # Latin, Crimean Tatar
languagesystem latn AZE; # Latin, Azeri
languagesystem latn MOL; # Latin, Moldavian
languagesystem latn ROM; # Latin, Romanian
languagesystem latn VIT; # Latin, Vietnamese
languagesystem cyrl BGR; # Cyrillic, Bulgarian
languagesystem cyrl SRB; # Cyrillic, Serbian
languagesystem arab URD; # Arabic, Urdu

All non-default languages are missing kerning.

I didn't check then, but I checked now: No other kerning features are defined other than the one already posted at the top.

yanone commented 1 year ago

or is there another GPOS feature apart from kern that uses script latn; language CAT?

Sorry, I didn't answer your question correctly. I don't have any other GPOS features for these languages. They are all used in the locl feature, so they're GSUB.

anthrotype commented 1 year ago

seems to be intentional:

https://github.com/googlefonts/ufo2ft/blob/5a5f6dfdebeaaaa50de0b927f122776f3783e1d9/Lib/ufo2ft/featureWriters/kernFeatureWriter.py#L507-L511

but it is nevertheless a bug in my opinion. The old kernFeatureWriter would register to all declared languages, not just dflt. Can you use ufo2ft v2.30.0 (stable release) instead of the pre-release or github main branch? at least while we investigate/fix thanks for reporting

yanone commented 1 year ago

Can you use ufo2ft v2.30.0 (stable release)

No, because the kerning of v2.30.0 was faulty for my font to begin with, which triggered Nikolaus rewriting it. It's not super urgent for me. As long as it gets recognized as an important bug that must affect all fonts, I'll wait. Thank you

madig commented 1 year ago

Todo: add all languages the designer defined for existing scripts, keep using "dflt" for scripts we add dynamically.

madig commented 1 year ago

@yanone can you please test with https://github.com/googlefonts/ufo2ft/pull/711?

yanone commented 1 year ago

@madig Looks good!