SixLabors / Fonts

:black_nib: Font loading and layout library.
https://sixlabors.com/products/fonts
Other
306 stars 71 forks source link

Fix Font Hinting #295

Closed JimBobSquarePants closed 2 years ago

JimBobSquarePants commented 2 years ago

Prerequisites

Description

Fixes #293 and #294

We were passing the multiplied pixel size when we shouldn't have. Some pre-cleartype fonts don't seem to work all that well so I've internally disabled hinting for them for now.

codecov[bot] commented 2 years ago

Codecov Report

Merging #295 (f704bbd) into main (6d831c6) will decrease coverage by 0%. The diff coverage is 89%.

@@          Coverage Diff          @@
##            main    #295   +/-   ##
=====================================
- Coverage     83%     83%   -1%     
=====================================
  Files        222     224    +2     
  Lines      12281   12299   +18     
  Branches    1784    1788    +4     
=====================================
+ Hits       10272   10279    +7     
- Misses      1585    1600   +15     
+ Partials     424     420    -4     
Flag Coverage Δ
unittests 83% <89%> (-1%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/SixLabors.Fonts/ArrayBuilder{T}.cs 84% <ø> (ø)
src/SixLabors.Fonts/GlyphMetrics.cs 50% <0%> (ø)
src/SixLabors.Fonts/GlyphShapingData.cs 90% <0%> (-4%) :arrow_down:
src/SixLabors.Fonts/GlyphSubstitutionCollection.cs 89% <0%> (-1%) :arrow_down:
src/SixLabors.Fonts/MappedArraySlice{T}.cs 100% <ø> (ø)
src/SixLabors.Fonts/Tables/Cff/RefStack{T}.cs 41% <ø> (ø)
...nts/Tables/TrueType/Hinting/TrueTypeInterpreter.cs 50% <ø> (-2%) :arrow_down:
src/SixLabors.Fonts/ByteMemoryManager{T}.cs 40% <40%> (ø)
src/SixLabors.Fonts/ReadOnlyMappedArraySlice{T}.cs 83% <83%> (ø)
src/SixLabors.Fonts/StreamFontMetrics.TrueType.cs 97% <83%> (ø)
... and 12 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

JimBobSquarePants commented 2 years ago

@tocsoft I've cracked it! 🎉

I had a good dig through the FreeType interpreter code and accompanying docs and figured out what they did to get their interpreter working well with legacy fonts. Basically, they never move anything horizontally when hinting at all which fixes most issues then there's a couple of additional tweaks for legacy fonts. No need for lists or targeted hacks - it just works!

Here's Tahoma (This was really bad before with odd offsetting of individual glyphs.)

HintingMode.HintXY _six_tahoma_hintxy_

HintingMode.HintY _six_tahoma_hinty_

HintingMode.None _six_tahoma_none_

HintingMode.HintXY vs HintingMode.None (You can clearly see the hinting benefit in the 'o' here)

image

And here's Open Sans

HintingMode.HintXY (Both hint modes Y and XY produce identical results with this font) _six_open_sans_hintxy_

HintingMode.None _six_open_sans_none_

HintingMode.HintXY vs HintingMode.None (You can clearly see the hinting benefit in the 'o' here)

image

For most fonts there really miniscule difference between HintingMode.HintXY and HintingMode.HintY. There are some notable improvements in Times New Roman when using HintingMode.HintY in that the over-thinning of stems does not happen. I'm thinking of reworking the enum to rename HintingMode.HintY as HintingMode.Compatibility moving it to the third position as there's additional overheads (scaling of control point x-values before and after hinting) which, while SIMD optimized, you don't really want people using as a first-choice approach.

What do you think?

image
JimBobSquarePants commented 2 years ago

@tocsoft It gets better. Turns out I had my enum wired back-to-front in GlyphVector and I was oversampling in XY mode not Y. The default output was the best one!

I've deleted the oversampling code and changed the enum to None and Standard. I left the option as an enum in case we ever do additional hinting options.