protectwise / troika

A JavaScript framework for interactive 3D and 2D visualizations
MIT License
1.61k stars 117 forks source link

Kerning is applied two times #196

Closed TimmyTommy closed 2 years ago

TimmyTommy commented 2 years ago

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.

https://github.com/protectwise/troika/blob/bf68d74c6a55449567fba8a610844c102a7e930a/packages/troika-three-text/src/FontParser.js#L223

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 commented 2 years ago

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 commented 2 years ago

It seems like this change broke it.

https://github.com/fredli74/Typr.ts/commit/a9f3fe3a2e55c01aeae28ac0ef1ac3e848478b7f

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

TimmyTommy commented 2 years ago

Looks good! Thank you very much.

image