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

[OSX] Replace FreeType by native glyph rasterization #8518

Merged
merged 3 commits into from Feb 14, 2021

Conversation

@michicc
Copy link
Member

@michicc michicc commented Jan 7, 2021

Motivation / Problem

On OSX, text layout for TrueType fonts and more complex languages is done by CoreText. Font rendering on the other hand is done by the FreeType library, which assumes these two components never disagree.

Font selection in CoreText has lots of fallbacks built-in; it is extremly hard to not get some font glyphs from CoreText. FreeType on the other hand needs a reference to an on-disk font file. We try hard to match the proper font, but might not always succeed if CoreText is applying some magic for exotic scripts. It is unlikely to ever be a problem for any kind of western script, though.

Description

Letting CoreText render the font glyphs as well avoids any potential conflict or disagreement. It also removes an external dependency.

On average, CoreText appears to rasterize glyphs at least as fast as FreeType, often even faster. The implementation is similar to the Win32 implementation, as many concepts are analogue.

Limitations

CoreText and FreeType make slightly different stylistic choices rasterizing glyphs. This is most apparent when anti-aliasing is activated, as CoreText has a tendency to render "thicker". CoreText appears to render best when using native (HiDPI) resolution (separate PR incoming).

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')
    This is just a code move/rename, not a functionality change.
src/fontcache.cpp Outdated Show resolved Hide resolved
@LordAro LordAro requested a review from andythenorth Jan 13, 2021
@michicc michicc force-pushed the michicc:pr/osx_font branch 2 times, most recently from b1a3e45 to e14b38e Feb 6, 2021
Copy link
Member

@TrueBrain TrueBrain left a comment

@orudge : did you have the chance to test this already? Would be good to have feedback from users on real mac hardware :)

src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
src/fontcache.cpp Outdated Show resolved Hide resolved
@orudge
Copy link
Contributor

@orudge orudge commented Feb 13, 2021

Finally got this tested again - the rendering looks higher quality than FreeType for sure. Particularly noticeable with bold fonts I've found (e.g., Tahoma Bold) - FreeType is very 'squashed together', with the native rendering looking considerably better. Certainly from a visual point of view (plus getting rid of a dependency in the Mac builds!) I'd be keen for this to go in.

@michicc michicc force-pushed the michicc:pr/osx_font branch from e14b38e to 871bbb1 Feb 13, 2021
@michicc
Copy link
Member Author

@michicc michicc commented Feb 13, 2021

@orudge Got a personal taste question for you:

If you enable font anti-aliasing (config option only I'm afraid), which rendering is better:

  1. As in this PR.
  2. If you change the 0.0f to a 0.5f as well at https://github.com/michicc/OpenTTD/blob/871bbb1fa7c1275157c56866cc629b2a0d12766e/src/os/macosx/font_osx.cpp#L306-L310
Copy link
Member

@TrueBrain TrueBrain left a comment

Looks really good; when you have an answer from @orudge, I see no blocking issues :)

src/os/macosx/font_osx.cpp Outdated Show resolved Hide resolved
src/os/macosx/font_osx.cpp Show resolved Hide resolved
@orudge
Copy link
Contributor

@orudge orudge commented Feb 13, 2021

2. If you change the 0.0f to a 0.5f as well at https://github.com/michicc/OpenTTD/blob/871bbb1fa7c1275157c56866cc629b2a0d12766e/src/os/macosx/font_osx.cpp#L306-L310

This looks just a touch clearer to me I would say; as you say, it is very subtle.

michicc added 3 commits Feb 13, 2021
On a display, especially with small fonts or low pixel sizes, sans-serif
fonts are usually easier to read than serif fonts.
@michicc michicc force-pushed the michicc:pr/osx_font branch from 871bbb1 to 60df815 Feb 14, 2021
@michicc
Copy link
Member Author

@michicc michicc commented Feb 14, 2021

Artistic 2 choice applied.

@michicc michicc merged commit b66e977 into OpenTTD:master Feb 14, 2021
10 checks passed
10 checks passed
@github-actions
Emscripten
Details
@github-actions
Commit checker
Details
@github-actions
Check preview needs update Check preview needs update
Details
@github-actions
Linux (clang, clang++)
Details
@github-actions
Linux (gcc, g++)
Details
@github-actions
Mac OS (x64, x86_64)
Details
@github-actions
Windows (windows-latest, x86)
Details
@github-actions
Windows (windows-latest, x64)
Details
@github-actions
Windows (windows-2016, x86) Windows (windows-2016, x86)
Details
@github-actions
Windows (windows-2016, x64) Windows (windows-2016, x64)
Details
@michicc michicc deleted the michicc:pr/osx_font branch Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants