-
Notifications
You must be signed in to change notification settings - Fork 43
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
RTL kerning missing from font #658
Comments
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 What confuses me a bit is that the In short: I couldn't find any class kerning in the TTX file. <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> |
Further findings: I followed a tip of Simon’s to use
I found this, but it’s the wrong combination: FYI, I used a regular expression for searching the large feature file, so I’m confident that I didn’t miss it: In conclusion: Some kerning gets dropped from the font binary after |
did you use ufo2ft from the repo's main branch or the latest stable release from PyPI (v2.28.0)? |
if you could share the source with us (me and/or Simon, even privately) would be great, thanks |
@simoncozens @anthrotype I’m inviting you both to the private repo. The build is triggered with Please note that I’ve pasted compressed feature code as output by 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 |
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? |
I’ll check now |
Indeed, the missing kerning shows up now with |
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. |
I hope @simoncozens can take a look at this at some point. I am actually convincing myself that we should revert |
It shows up in the shaping in FontGoggles now, so I’m skipping debugging the binary. |
Do you mean just reverting #638 or the whole splitting-by-script thing? |
yep sorry I meant the whole #636 |
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. |
FWIW, my |
Sure, done. |
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. |
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. |
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? |
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. |
yes please. Or we continue the discussion in googlefonts/glyphsLib#778 which is the PR for that issue |
@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. |
Sorry, I'm reopening this. There is still kerning missing. I'll investigate further. |
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. |
Solution: add There is another problem inherent in the current kern feature writer that will be solved by #667, which results in these kerning failures:
|
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):Looks correct, doesn't it?
Any idea where to look further? Thank you.
The text was updated successfully, but these errors were encountered: