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

Must strictly separate RTL and LTR group kerning for glyphs: intended? #1004

Open
madig opened this issue May 2, 2024 · 2 comments
Open

Comments

@madig
Copy link
Collaborator

madig commented May 2, 2024

Alternative title: possible kerning data loss when glyph is group-kerned in LTR and RTL mode

As far as I understand, the way glyphsLib folds RTL into LTR kerning is like this:

  1. Gather all glyphs touched by RTL kerning in the Glyphs.app file.
  2. When generating UFO kerning groups, for each glyph check if it is in the set generated in step 1.
    a. If not, put the glyph's (leftKerningGroup, rightKerningGroup) into the corresponding (public.kern1, public.kern2) group
    b. If so, put the glyph's (leftKerningGroup, rightKerningGroup) into the corresponding (public.kern2, public.kern1) group
  3. For each kerning pair in kerningRTL, flip the kerning group name (kern1 -> kern2 and vice versa) and then merge them into the regular UFO kerning dictionary

The either-or logic in step two implies that a glyph must be used either in LTR or RTL group kerning, but never in both. Glyph-to-glyph kerning has no such limitation because it obviously works without groups.

This means that neutral glyphs like period should, if they are put in a group, kerned only in LTR or in RTL mode, but not both, because currently the LTR kerning will point at the wrong kerning name.

Some test cases:

  • KerningRTL2.zip: No RTL kerning, just group kerning of a against period.
    • Resulting kerning groups:
      • public.kern1.a: [a]
      • public.kern1.period_right: [period]
      • public.kern2.alef: [alef-ar]
      • public.kern2.period_left: [period]
    • Resulting kerning:
      • (public.kern1.a, public.kern2.period_left) = -10
  • KerningRTL.zip: Additional RTL group kerning of alef against period.
    • Resulting kerning groups:
      • public.kern1.a: [a]
      • public.kern1.alef: [alef-ar]
      • public.kern1.period_left: [period]
      • public.kern2.period_right: [period]
    • Resulting kerning:
      • (public.kern1.a, public.kern2.period_left) = -10
      • (public.kern1.alef, public.kern2.period_right) = -20

Note how the presence of RTL kerning flips the side of the period_right and period_left group, as well as the alef group. Also note how the first kerning between a and period_left gets voided because there is no public.kern2.period_left anymore. glyphs2ufo indeed logs a warning saying "Non-existent glyph class public.kern2.period_left found in kerning rules".

Do I understand correctly that this is a known limitation of folding RTL into LTR kerning and that group kerning must be strictly separated into LTR and RTL? I.e. if you want to group-kern period, you should make a period-ar for RTL and group-kern that instead?

@madig
Copy link
Collaborator Author

madig commented May 3, 2024

FWIW, the mekkablue conversion script is behaving in the same way with the same possible loss of kerning.

@madig
Copy link
Collaborator Author

madig commented May 6, 2024

I found an interdasting detail where I'm not sure how intentional it was. In

combined_kerning[flip_class_side(kern1)] = {
, a flipped class will overwrite any unflipped class that was there before from LTR kerning. I.e. if you have a pair like (public.kern1.one, public.kern2.b), and you also group-kerned one against some RTL glyph, you'd get an RTL pair like (public.kern1.one, public.kern2.hah), the highlighted code would drop all LTR kerning pairs starting with public.kern1.one in favor of RTL pairs starting with the same group name. You can see this in Kerning.zip where the LTR pair (public.kern1.one, public.kern2.b): -121 is dropped from the resulting UFO.

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

No branches or pull requests

1 participant