googlefonts / ufo2ft

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

Fully 'base' kerning exceptions do not apply correctly to groups containing 'marks' #706

Closed Hoolean closed 1 year ago

Hoolean commented 1 year ago

Problem

Scope

Although the issue was found in testing for the new kern splitter, both the old and new implementations are affected. In particular, incorrect kerning values were identified only in Noto Serif Tamil (with kerning-validator).

Reproducing

In Noto Tamil Serif

$ kerning-validator NotoSerifTamil-Regular.ufo

Minimal

(derived from Noto Serif Tami, which is under the OFL)

Kerning:

...
    <key>aa-tamil</key>
    <dict>
      <key>lu-tamil</key>
      <real>-20</real>
      <key>public.kern2.e-tamil</key>
      <real>-35</real>
    </dict>
...

Groups:

...
    <key>public.kern2.e-tamil</key>
    <array>
      <string>aulengthmark-tamil</string>
      <string>lu-tamil</string>
    </array>
...

Where /aa-tamil and /lu-tamil are bases, and /aulengthmark-tamil is a mark, /aa-tamil/lu-tamil is kerned to -55 instead of -20:

@kern2.etamil = [aulengthmark-tamil lu-tamil];

lookup kern_ltr {
    lookupflag IgnoreMarks;
    pos aa-tamil lu-tamil -20;
} kern_ltr;

lookup kern_ltr_marks {
    enum pos aa-tamil @kern2.etamil -35;
} kern_ltr_marks;

# ...

feature dist {
    # ...
    script tml2;
    language dflt;
    lookup kern_ltr;
    lookup kern_ltr_marks;
} dist;

Workaround

Manually adjust your kerning to avoid mixing bases and marks in the same group.

Fix

Split bases and marks into separate groups in KernFeatureWriter (needs further thought).

Hoolean commented 1 year ago

(issue now filed for Noto Serif Tamil, where this causes some clashes)

madig commented 1 year ago

Harry outlines a potential fix:

@Hoolean so it would be changing the logic in https://github.com/googlefonts/ufo2ft/blob/da917d/Lib/ufo2ft/featureWriters/kernFeatureWriter.py#L385-L395 to a split:

   for pair in pairs:
      leftBases, leftMarks = split left hand side into bases/marks
      rightBases, rightMarks = split right hand side into bases/marks

      # Only matches pairs with no marks
      basePairs.append(Pair(leftBases, rightBases)) # no marks on left/right

      # Only matches pairs with at least one mark (no overlap with above)
      markPairs.append(Pair(leftMarks, rightMarks)) # marks on left/right
      markPairs.append(Pair(leftMarks, rightBases)) # marks on left
      markPairs.append(Pair(leftBases, rightMarks)) # marks on right