Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Hoolean opened this issue Feb 14, 2023 · 2 comments · Fixed by #713
Closed

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

Hoolean opened this issue Feb 14, 2023 · 2 comments · Fixed by #713

Comments

@Hoolean
Copy link
Collaborator

Hoolean commented Feb 14, 2023

Problem

  • If a kerning pair references a group that contains some 'mark' glyphs, it is placed in the 'marks' kern lookup.
  • If a glyph-to-glyph exception is made for the group(s) that references only 'base' glyphs, it is placed in the 'bases' kern lookup.
  • The glyph-to-glyph exception should prevent the group-based kerning pair from applying, but as it is placed in a different lookup, it does not, and instead the pairs are applied in sequence.

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
Copy link
Collaborator Author

Hoolean commented Feb 14, 2023

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

@madig
Copy link
Collaborator

madig commented Feb 21, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants