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

Fix kerning issues in ofTrueTypeFont #6136

Closed

Conversation

chriship
Copy link
Contributor

@chriship chriship commented Sep 27, 2018

Freetype uses fixed point number for font calculation (26.6).

It means 26 bit integer and 6 bit decimal.

So we can not simply use the value of FT_Get_Kerning because we need to account for the .6

How to calculate?
We do bit shift to the right.

pen_x += delta.x >> 6;

This is same calculation as dividing by 64 (2^6=64).

See Simple Text Rendering: Kerning and Centering sesction in this page. https://www.freetype.org/freetype2/docs/tutorial/step2.html#section-4

Thanks to @hiroMTB for figuring this out.

@arturoc
Copy link
Member

arturoc commented Sep 27, 2018

I think fontUnitSize does this for a more complex case since we don't load the font at 72dpi but by default at 96 and the user can set any dpi so this line should account for that:

fontUnitScale = (float(settings.fontSize * settings.dpi)) / (72 * face->units_per_EM);

Kerning has never really worked completely correctly though, just looking at the fontsExample you can see how some characters kerning doesn't look completely correct but this doesn't seem to fix it either

Do you have any example where this actually makes it better?

@hiroMTB
Copy link
Contributor

hiroMTB commented Sep 27, 2018

Oh didn't think about fontUnitScale thing.
It looks like calculated by font, fontSize and dpi.
But kerning depends on font, fontSize and character combination (c, prevc).
So fontUnitScale can not contain 6 bit shift conversion of kerning.
I guess we can calculate it by following.

(kerning.x >> 6 ) * fontUnitScale;

In any case, we can not use returning value of FT_Get_Kerning() without conversion. Because it is 26.6 fixed point format.

This kerning problem is obvious when we set bigger font size like 100. Especially text including "Y". But sometimes difficult to notice. Some font has big trouble but the other doesn't, probably because of bottom 6 bits.

p.s.
This issue is not tracked well because fontExample only use small fontSize.
Compiling and linking does not produce any errors but developer can only notice this visual issue. Would be nice have bigger fontSize in fontExample so that we can make sure everything works ok in different cases. (maybe bigger letterSpacing as well)

@arturoc
Copy link
Member

arturoc commented Sep 27, 2018

what i mean is that fontUnitScale already is >> 6 but also takes into accout the dpi. I'm not really sure but the value of fontUnitScale is much smaller than 1/64. I'm not sure where i took the formula from but i'd say that it's the way to convert from dots to pixels.

It looks like calculated by font, fontSize and dpi.
But kerning depends on font, fontSize and character combination (c, prevc).

fontUnitScale takes into account fontSize and dpi and kerning.x takes into account the character combination

To explain it in another way what i mean is that >> 6 is the way to go from dots to pixels when the dpi is 72 and the formula for fontUnitScale should take into account any dpi.

I'm not 100% sure that fontUnitScale is correctly implemented though. Here's the full explanation: https://www.freetype.org/freetype2/docs/glyphs/glyphs-2.html i'll try to take a look later

@chriship
Copy link
Contributor Author

chriship commented Sep 27, 2018

I'm not 100% sure that fontUnitScale is correctly implemented though. Here's the full explanation: https://www.freetype.org/freetype2/docs/glyphs/glyphs-2.html i'll try to take a look later

I thing you're correct in that fontUnitScale is calculated incorrectly. It seems if I change it to:

fontUnitScale = 1.0f / 64;

Then the original getKerning method works.

EDIT: I've noticed another thing - maybe related:

In the method getKerning we have:

`FT_Get_Kerning(face.get(), FT_Get_Char_Index(face.get(), c), FT_Get_Char_Index(face.get(), prevC), FT_KERNING_UNFITTED, &kerning);

`
But FT_Get_Kerning expects the left character then the right character (see here) so it should be:

FT_Get_Kerning(face.get(), FT_Get_Char_Index(face.get(), prevC), FT_Get_Char_Index(face.get(), c), FT_KERNING_UNFITTED, &kerning);

For me this fixes some apostrophe uses (like YOU'LL).

arturoc added a commit that referenced this pull request Nov 14, 2018
We where using a calculated factor for the font size to scale the kerning
but that doesn't seem to work. We are now instead right shifting by 6
which is the truetype default
@arturoc
Copy link
Member

arturoc commented Nov 14, 2018

i've applied this fix using >> 6 instead of changing the factor which was used to calculate the font size and seems to work properly

@arturoc arturoc closed this Nov 14, 2018
@arturoc
Copy link
Member

arturoc commented Nov 14, 2018

thanks!

olekristensen added a commit to olekristensen/openFrameworks that referenced this pull request Jan 2, 2019
* commit 'faeb05d54647915464dcf2bf23ee9da03d8abcd5': (54 commits)
  changelog and thanks scripts
  fixed thanks format
  THANKS.md
  create_package: pull OF from github instead of local
  create_package: pull OF from github instead of local
  Revert "ofConstants: revert version to 0.10.1"
  create_package: pull PG from master always
  PG latest commit
  changelof for 0.10.1
  ofTruetypeFont: Fix kerning factor. openframeworks#6136
  ofConstants: revert version to 0.10.1
  ofSoundBuffer getChannel fix (openframeworks#6117)
  ofSoundBuffer getChannel fix (openframeworks#6117)
  ofxGui: slider scrolling (openframeworks#6144)
  ofxGui: slider scrolling (openframeworks#6144)
  appveyor: fix ssl dependencies (openframeworks#6170)
  appveyor: fix ssl dependencies (openframeworks#6170)
  fix for xcode template file to allow PG to add frameworks and fix broken projects. closes openframeworks#6172
  workspace files for xcode template to enable legacy build in Xcode 10
  added camera and microphone permissions for mojave
  ...

# Conflicts:
#	apps/projectGenerator
olekristensen added a commit to olekristensen/openFrameworks that referenced this pull request Jan 2, 2019
…orks into stable

* 'stable' of https://github.com/openframeworks/openFrameworks: (105 commits)
  create_package: pull OF from github instead of local
  create_package: pull PG from master always
  PG latest commit
  changelof for 0.10.1
  ofTruetypeFont: Fix kerning factor. openframeworks#6136
  ofConstants: revert version to 0.10.1
  ofSoundBuffer getChannel fix (openframeworks#6117)
  ofxGui: slider scrolling (openframeworks#6144)
  appveyor: fix ssl dependencies (openframeworks#6170)
  fix for xcode template file to allow PG to add frameworks and fix broken projects. closes openframeworks#6172
  workspace files for xcode template to enable legacy build in Xcode 10
  added camera and microphone permissions for mojave
  fix drawArrow internal matrix calculation (openframeworks#6164)
  remove i386 architecture (openframeworks#6159)
  Revert "Update main.cpp (openframeworks#6150)" (openframeworks#6160)
  Update main.cpp (openframeworks#6150)
  fixes xcode template to not have OF as a dependcy but as a pre-build script. related to openframeworks#6139
  ofCamera with ortho enabled will respect a lensOffset by translating the projection matrix (openframeworks#6138)
  ofXml: add clear method which clears the xml doc
  Bugfix vs project template (pt.2) (openframeworks#6130)
  ...

# Conflicts:
#	libs/openFrameworks/app/ofAppGLFWWindow.cpp
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

Successfully merging this pull request may close these issues.

3 participants