Closed Bug 1277404 Opened 4 years ago Closed 4 years ago

GDI fonts with Skia are both thicker and thinner than Cairo

Categories

(Core :: Graphics: Text, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: mchang, Assigned: mchang)

Details

(Whiteboard: gfx-noted)

Attachments

(1 file, 2 obsolete files)

e.g. Normal text is slightly less bold and text for the chrome process such as tab titles are too thick.
Whiteboard: gfx-noted
Attached patch gdi.patch (obsolete) — Splinter Review
This mostly fixes the GDI font issue on Windows 7. One issue is the gamma should be set to 1.8 for Skia's gamma look up table. We do this already with Dwrite fonts via use system settings, and now this applies it to GDI fonts as well. This mostly fixes the "fonts are too thin" problem.

The fonts are too thick problem comes from hinting. I'm still not sure why, but hinted GDI Fonts are much too thick, but unhinted GDI fonts are just right. However, unhinted GDI Fonts causes Skia to ask GDI for an outline of the font via a path, then skia converts it to a SkPath and blits the mask. I'm not sure why regular GDI mask creation looks so different yet.
Err also, the fonts are too thick problem only happens when we can't use LCD rendered text, mostly because we have an RGBA surface instead of an RGBX surface. This causes us to fallback to grayscale AA. On a side note, things like the text in tabs and such now have subpixel AA whereas with cairo they don't!
Attached patch winGamma.patch (obsolete) — Splinter Review
Most GDI text on opaque surfaces look odd because of our good friend gamma. Instead of using the system host settings which only works for dwrite fonts, let's just set the gamma to 1.8 as that's what is the default for most text.

As for GDI fonts on non-opaque surfaces, cairo was using cleartype GDI fonts explicitly [1] and converting them to grayscale whereas Skia was using AA quality [2] along with a constant 2.3 gamma correction for the LUT. Instead, we want to force Skia to use CLEARTYPE fonts and convert them to Grayscale so they look like what cairo does.

[1] https://dxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-win32-font.c#282
[2] https://dxr.mozilla.org/mozilla-central/source/gfx/skia/skia/src/ports/SkFontHost_win.cpp?from=SkFontHost_win.cpp#614
Attachment #8759448 - Attachment is obsolete: true
Attachment #8759875 - Flags: review?(lsalzman)
(In reply to Mason Chang [:mchang] from comment #3)
> Created attachment 8759875 [details] [diff] [review]
> winGamma.patch
> 
> Most GDI text on opaque surfaces look odd because of our good friend gamma.
> Instead of using the system host settings which only works for dwrite fonts,
> let's just set the gamma to 1.8 as that's what is the default for most text.
> 
> As for GDI fonts on non-opaque surfaces, cairo was using cleartype GDI fonts
> explicitly [1] and converting them to grayscale whereas Skia was using AA
> quality [2] along with a constant 2.3 gamma correction for the LUT. Instead,
> we want to force Skia to use CLEARTYPE fonts and convert them to Grayscale
> so they look like what cairo does.
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-
> win32-font.c#282
> [2]
> https://dxr.mozilla.org/mozilla-central/source/gfx/skia/skia/src/ports/
> SkFontHost_win.cpp?from=SkFontHost_win.cpp#614

This patch causes it to skip the call to paint.mPaint.setHinting(SkPaint::kNormal_Hinting) right after you set the flags. Was that on purpose or accident?
Attached patch winGamma.patchSplinter Review
Good catch, sorry about that.
Attachment #8759875 - Attachment is obsolete: true
Attachment #8759875 - Flags: review?(lsalzman)
Attachment #8759916 - Flags: review?(lsalzman)
Attachment #8759916 - Flags: review?(lsalzman) → review+
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ae7b65e1c12
Set font gamma for windows fonts to 1.8 and force skia to use cleartype fonts on non-opaque surfaces. r=lsalzman
https://hg.mozilla.org/mozilla-central/rev/0ae7b65e1c12
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.