RTL kerning missing from font

[yanone]

This is a strange one, but I don’t know how to help myself any further:

A font of mine doesn't generate RTL kerning (or at least it doesn't show in both FontGoggles and Indesign), while LTR kerning generates fine.

ufo2ft code base is up-to-date with the repo.

I've had fontmake export the debug feature file and pasted all the code relevant to the kerning pair راbelow (ر is supposed to be first from right, ا is second):

languagesystem DFLT dflt; # Default, Default
languagesystem arab dflt; # Arabic, Default

@kern1.reh = [reh-ar ...];
@kern2.alef = [alef-ar ...];

lookup kern_Arab {
    lookupflag IgnoreMarks;
    pos @kern1.reh @kern2.alef <-100 0 -100 0>;
} kern_Arab;

feature kern {
    script arab;
    language dflt;
    lookup kern_Arab;
} kern;

Looks correct, doesn't it?
Any idea where to look further? Thank you.

[yanone]

I may have found something in the GPOS TTX dump, or rather the lack thereof.

Below is all the kerning code I could find for the ra (uni0631) as first glyph. The second glyphs shown are all actual glyphs, not classes. I was expecting to find classes here as well. I confirmed that the pairs shown below also show in FontGoggles.

What confuses me a bit is that the ra is also part of a kerning class but shows with its glyph name below. But I'm not versed in the details of the GSUB table to know how this is supposed to be defined.

In short: I couldn't find any class kerning in the TTX file.
But it’s also possible that I'm searching wrong, as class kerning may be defined in a different way.

        <ExtensionPos index="1" Format="1">
          <ExtensionLookupType value="2"/>
          <PairPos Format="1">
            <Coverage>
              <Glyph value="uni0631"/>
            </Coverage>
            <ValueFormat1 value="85"/>
            <ValueFormat2 value="0"/>
            <!-- PairSetCount=1 -->
            <PairSet index="0">
              <!-- PairValueCount=6 -->
              <PairValueRecord index="0">
                <SecondGlyph value="uni0625"/>
                <Value1 XPlacement="10" XAdvance="10">
                  <XPlaDevice>
                    <StartSize value="2"/>
                    <EndSize value="866"/>
                    <DeltaFormat value="32768"/>
                  </XPlaDevice>
                  <XAdvDevice>
                    <StartSize value="2"/>
                    <EndSize value="866"/>
                    <DeltaFormat value="32768"/>
                  </XAdvDevice>
                </Value1>
              </PairValueRecord>
              <PairValueRecord index="1">
                <SecondGlyph value="uni0673"/>
                <Value1 XPlacement="-10" XAdvance="-10">
                  <XPlaDevice>
                    <StartSize value="1"/>
                    <EndSize value="19"/>
                    <DeltaFormat value="32768"/>
                  </XPlaDevice>
                  <XAdvDevice>
                    <StartSize value="1"/>
                    <EndSize value="19"/>
                    <DeltaFormat value="32768"/>
                  </XAdvDevice>
                </Value1>
              </PairValueRecord>
              <PairValueRecord index="2">
                <SecondGlyph value="uni066E.init.dotColl"/>
                <Value1 XPlacement="10" XAdvance="10">
                  <XPlaDevice>
                    <StartSize value="2"/>
                    <EndSize value="1193"/>
                    <DeltaFormat value="32768"/>
                  </XPlaDevice>
                  <XAdvDevice>
                    <StartSize value="2"/>
                    <EndSize value="1193"/>
                    <DeltaFormat value="32768"/>
                  </XAdvDevice>
                </Value1>
              </PairValueRecord>
              <PairValueRecord index="3">
                <SecondGlyph value="uni060C"/>
                <Value1 XPlacement="-100" XAdvance="-100">
                  <XPlaDevice>
                    <StartSize value="2"/>
                    <EndSize value="901"/>
                    <DeltaFormat value="32768"/>
                  </XPlaDevice>
                  <XAdvDevice>
                    <StartSize value="2"/>
                    <EndSize value="901"/>
                    <DeltaFormat value="32768"/>
                  </XAdvDevice>
                </Value1>
              </PairValueRecord>
              <PairValueRecord index="4">
                <SecondGlyph value="period"/>
                <Value1 XPlacement="-60" XAdvance="-60">
                  <XPlaDevice>
                    <StartSize value="2"/>
                    <EndSize value="2206"/>
                    <DeltaFormat value="32768"/>
                  </XPlaDevice>
                  <XAdvDevice>
                    <StartSize value="2"/>
                    <EndSize value="2206"/>
                    <DeltaFormat value="32768"/>
                  </XAdvDevice>
                </Value1>
              </PairValueRecord>
              <PairValueRecord index="5">
                <SecondGlyph value="period.arab"/>
                <Value1 XPlacement="-100" XAdvance="-100">
                  <XPlaDevice>
                    <StartSize value="2"/>
                    <EndSize value="901"/>
                    <DeltaFormat value="32768"/>
                  </XPlaDevice>
                  <XAdvDevice>
                    <StartSize value="2"/>
                    <EndSize value="901"/>
                    <DeltaFormat value="32768"/>
                  </XAdvDevice>
                </Value1>
              </PairValueRecord>
            </PairSet>
          </PairPos>
        </ExtensionPos>

[yanone]

Further findings: I followed a tip of Simon’s to use otf2fea for debugging the feature code.

otf2fea expresses class kerning as single lines. I found two lines with a ra as part of the first glyph class, but the combination with an alef as part of the second glyph class is definitely missing from the font binary.

I found this, but it’s the wrong combination:
pos [uni0631 ...] [uni0639.init ...] <-80 0 -80 0>;

FYI, I used a regular expression for searching the large feature file, so I’m confident that I didn’t miss it: pos (\[)?.*?uni0631.*?(\])? (\[)?.*?uni0627.*?(\])?

In conclusion: Some kerning gets dropped from the font binary after ufo2ft has assembled the AFDKO feature code as debugged through --debug-feature-file

[anthrotype]

did you use ufo2ft from the repo's main branch or the latest stable release from PyPI (v2.28.0)?
Since last release, @simoncozens made a big change to the kern feature writer with #636 - instead of splitting into two big LTR and RTL lookups, it now splits lookups by individual scripts. Maybe there's something going on there with that particular font of yours.

[anthrotype]

A font of mine

if you could share the source with us (me and/or Simon, even privately) would be great, thanks

[yanone]

@simoncozens @anthrotype I’m inviting you both to the private repo. The build is triggered with sh build.sh. As stated, I’ve used the repo's main branch for ufo2ft. Note that I’m using the support_glyphs3_rtl_kerning-2 branch for glyphsLib or otherwise the RTL kerning wouldn't even make it past the glyphsLib stage (ongoing PR discussion with Georg). This branch is built on top of Georg’s GlyphInfo3 branch which supplies RTL glyph data.

Please note that I’ve pasted compressed feature code as output by fontmake --debug-feature-file in the original comment above, so it’s split into scripts. In my understanding, the feature code gets written correctly and the mistake happens a bit later when the feature code is assembled into byte code (or something, this is where my understanding ends).

In my opinion this is not related to the feature code and could possibly affect all scripts, not just RTL. Some class-based RTL kerning makes it into the font, and some not, and I happened to originally check for a pair that didn’t make it.

A more thorough QA tool could be written that reliably compares the original feature file with the binary font or otf2fea output, but for now I need to expect that any kerning, class-based or not, could be affected.

[anthrotype]

could you check if the same unexpected behaviour happens if you build with the stable ufo2ft v2.28.0 without the split-by-script kerning? Or do you need something else which is on ufo2ft/main that you can't use stable?

[yanone]

I’ll check now

[yanone]

Indeed, the missing kerning shows up now with ufo2ft==2.28.0 as part of the kern_rtl lookup.

[anthrotype]

the missing kerning shows up

do you mean, it "shows up" in the compiled table (ttx dump) and it works when the text is shaped? It was also already "showing up" in the debug feature file if I understood correctly, only that once compiled you could no longer find it and wasn't shaping as expected.

[anthrotype]

I hope @simoncozens can take a look at this at some point. I am actually convincing myself that we should revert #638 #636 as it also creates other issues elsewhere (#642)

[yanone]

It shows up in the shaping in FontGoggles now, so I’m skipping debugging the binary.

[simoncozens]

Do you mean just reverting #638 or the whole splitting-by-script thing?

[anthrotype]

yep sorry I meant the whole #636

[simoncozens]

Huh, maybe. I don't think this is that, though. If the kern is there in the feature file but not in the binary, I would be pointing fingers at feaLib.

[yanone]

FWIW, my fonttools installation is 4.36.0 (I’m guessing you referenced fonttools.feaLib) and it hasn’t changed between using ufo2ft 2.28.0 and the repo’s main branch.

[anthrotype]

I’m inviting you both to the private repo

@yanone can you please also invite Nikolaus Waxweiler @madig to that private repo so he can also help with debugging this? Thanks

[yanone]

Sure, done.
To summarize: The font generates fine now using the packages noted in requirements.txt, so only using ufo2ft==2.28.0 and a certain branch of glyphsLib that sits on top of Georg’s GlyphInfo3 branch (I keep reminding him to finish this one), or otherwise the kerning wouldn't even make it from the Glyphs source into the UFO.
As soon as ufo2ft main repository is used instead, RTL kerning gets partially dropped.

[khaledhosny]

We maintain all the relevant libraries, so I don’t know why we are adding workarounds. Lets make things simple and robust: add two private keys for LTR and RTL kerning, and when ufo2ft sees them it uses them directly and don’t try to do any direction guessing. We have glyphsLib so we can build Glyphs sources with ufo2ft, not to convert to general purpose UFOs that can be consumed by arbitrary tools.

[khaledhosny]

And lets face, UFO is an ll-defined and poor source format, we can’t represent many fonts with vanilla UFO, so lets not shy away from adding private keys or whatever we need to represent the fonts in source format, third-party tools shouldn’t be our concern.

[yanone]

If I understand you correctly, you call into question this PR (googlefonts/glyphsLib#778) as well as Georg’s branch (https://github.com/googlefonts/glyphsLib/tree/GlyphInfo3) that implements writing direction per glyph.

I’m just asking because this issue here isn't related to those.

I would love if we could find a way to move forward with the general RTL kerning issue from Glyphs 3 sources. It's a major obstacle and headache in my daily work, as most of Google Font’s externally submitted Arabic fonts land on my table, and most come in Glyphs 3 sources nowadays.

So far only my PR exists. Shall I create a separate issue on glyphsLib to discuss this orderly and from the beginning?

[anthrotype]

add two private keys for LTR and RTL kerning, and when ufo2ft sees them it uses them directly and don’t try to do any direction guessing.

Actually, if glyphsLib can encode the writing direction per glyph and store that in the ufo, then ufo2ft could use that to determine whether a kerning pair is LTR or RTL by checking whether kerned glyphs belong to either one or the other group, and avoid doing the inference. Then kerning.plist would continue to be used to store the kerning pairs, instead of having the kerning stored in those additional private keys as Khaled was suggesting, if I understood correctly.

[anthrotype]

Shall I create a separate issue on glyphsLib

yes please. Or we continue the discussion in googlefonts/glyphsLib#778 which is the PR for that issue

[yanone]

@madig has investigated the font in my private repo and couldn’t find any issue.

I used that opportunity to investigate my Python setup. I try to avoid venvs whenever I can, but installing everything in a venv did indeed produce a font with correct kerning, even with the dev version of ufo2ft. Of course I had kept all my packages up-to-date in my main environment, but some package somewhere must have been outdated.

Therefore I must conclude that the problem doesn't really exist. I'm really sorry for work that has been spent rewriting the latest kerning generator changes and thank everyone who helped resolve this.

[yanone]

Sorry, I'm reopening this. There is still kerning missing. I'll investigate further.

[yanone]

Yeah, it's confirmed, again: The current dev version of ufo2ft leads to dropped kerning, even LTR kerning. I repeated the whole process in a clean venv with ufo2ft==2.28.0 as well as latest dev and found more missing kerning using the dev version. Handing back to @madig in the private repo.

[madig]

Solution: add languagesystem grek dflt; to the feature file, current ufo2ft will wrongly drop kerning for undeclared scripts.

There is another problem inherent in the current kern feature writer that will be solved by #667, which results in these kerning failures:

script='Zyyy' direction='LTR': asterisk increment should be -90 but is 0
script='Zyyy' direction='LTR': degree increment should be -90 but is 0
script='Zyyy' direction='LTR': guillemetleft.case increment should be -30 but is 0
script='Zyyy' direction='LTR': guillemetright.case increment should be -30 but is 0
script='Zyyy' direction='LTR': increment asterisk should be -90 but is 0
script='Zyyy' direction='LTR': increment backslash should be -40 but is 0
script='Zyyy' direction='LTR': increment backslash.case should be -50 but is 0
script='Zyyy' direction='LTR': increment degree should be -90 but is 0
script='Zyyy' direction='LTR': increment guillemetleft.case should be -30 but is 0
script='Zyyy' direction='LTR': increment guillemetright.case should be -30 but is 0
script='Zyyy' direction='LTR': increment increment should be -10 but is 0
script='Zyyy' direction='LTR': increment minute should be -90 but is 0
script='Zyyy' direction='LTR': increment question should be -70 but is 0
script='Zyyy' direction='LTR': increment quotedbl should be -60 but is 0
script='Zyyy' direction='LTR': increment quotedblleft should be -60 but is 0
script='Zyyy' direction='LTR': increment quotedblright should be -60 but is 0
script='Zyyy' direction='LTR': increment quoteleft should be -60 but is 0
script='Zyyy' direction='LTR': increment quoteright should be -60 but is 0
script='Zyyy' direction='LTR': increment quotesingle should be -60 but is 0
script='Zyyy' direction='LTR': increment ringhalfleft should be -60 but is 0
script='Zyyy' direction='LTR': increment ringhalfright should be -60 but is 0
script='Zyyy' direction='LTR': increment second should be -90 but is 0
script='Zyyy' direction='LTR': increment trademark should be -90 but is 0
script='Zyyy' direction='LTR': minute increment should be -90 but is 0
script='Zyyy' direction='LTR': question increment should be -40 but is 0
script='Zyyy' direction='LTR': quotedbl increment should be -60 but is 0
script='Zyyy' direction='LTR': quotedblleft increment should be -60 but is 0
script='Zyyy' direction='LTR': quotedblright increment should be -60 but is 0
script='Zyyy' direction='LTR': quoteleft increment should be -60 but is 0
script='Zyyy' direction='LTR': quoteright increment should be -60 but is 0
script='Zyyy' direction='LTR': quotesingle increment should be -60 but is 0
script='Zyyy' direction='LTR': ringhalfleft increment should be -60 but is 0
script='Zyyy' direction='LTR': ringhalfright increment should be -60 but is 0
script='Zyyy' direction='LTR': second increment should be -90 but is 0
script='Zyyy' direction='LTR': slash increment should be -40 but is 0
script='Zyyy' direction='LTR': slash.case increment should be -70 but is 0
script='Zyyy' direction='LTR': trademark increment should be -90 but is 0