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

Kerning is applied two times #196

Closed
TimmyTommy opened this issue Mar 10, 2022 · 6 comments
Closed

Kerning is applied two times #196

TimmyTommy opened this issue Mar 10, 2022 · 6 comments

Comments

@TimmyTommy
Copy link

Hello,

Thank you for this amazing library!

Unfortunately I found a bug with the kerning of texts.
It's probably a bug in the Typr.js/Typr.ts version you are using.
In FontParser.js the line with Typr.U.getPairAdjustment returns the double amount of kerning offset.

I changed
glyphX += Typr.U.getPairAdjustment(typrFont, prevGlyphIndex, glyphIndex) * fontScale
to
glyphX += Typr.U.getPairAdjustment(typrFont, prevGlyphIndex, glyphIndex) / 2 * fontScale
which fixed the Problem.

glyphX += Typr.U.getPairAdjustment(typrFont, prevGlyphIndex, glyphIndex) * fontScale

Here is a screenshot of before and after. The top one is wrong. The 'T' and the 'e' should not be that close to each other.
The font I used is Calibri.

image

@lojjic
Copy link
Collaborator

lojjic commented Mar 10, 2022

Interesting. Is it always exactly doubled from what it should be?

Here's what Typr does: https://github.com/fredli74/Typr.ts/blob/master/src/Typr.U.js#L153

If we can identify it as a bug there then I'd much prefer to have it fixed upstream.

@TimmyTommy
Copy link
Author

It seems like this change broke it.

fredli74/Typr.ts@a9f3fe3

@fredli74 sums the 'GPOS' offset and the 'kern' offset.

I don't have that much know how about font files,
but it seems like getPairAdjustment should only use either the 'GPOS' data or the 'kern' data and not both at the same time.

https://docs.microsoft.com/en-us/typography/opentype/spec/gpos
https://docs.microsoft.com/en-us/typography/opentype/spec/kern

@lojjic
Copy link
Collaborator

lojjic commented Mar 10, 2022

It seems like this change broke it.

Hmm, I'm not sure that explains it -- the copy of Calibri.ttf I found doesn't appear to contain a GPOS table, so I don't think that upper loop would be contributing anything there ...? Unless your copy of the font has both.

@lojjic
Copy link
Collaborator

lojjic commented Mar 11, 2022

OK, nevermind, I see now that it does have a GPOS table with kern features in it, I'm not sure why that wasn't showing up in the tool I was using before. I wonder why this font includes both, that seems very inefficient?

Anyway, it seems like an easy fix to only use one and ignore the other. Would you be up for submitting a PR to fredli74's repo?

@lojjic
Copy link
Collaborator

lojjic commented Mar 11, 2022

@TimmyTommy I've released 0.46.3 with the updated Typr, can you please verify and close? Thanks!

@TimmyTommy
Copy link
Author

Looks good! Thank you very much.

image

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

2 participants