Some fonts were light or too dark depending on the font. We originally went solved this by forcing the default contrast / gamma of 0.5 / 1.8 respectively. However, skia fonts were still not quite right. Outside of gecko, after tracing through the LUT values, I could tell no difference. The problem was that for dwrite fonts with the recommended rendering mode, we should be using a contrast of 1.0, which is what d2d was doing. However, originally, this caused other fonts to look odd or too bold. After lots of head banging, the fonts that force GDI backwards compatibility were the ones looking odd, and a contrast of 1.0 for these fonts was too dark compared to what d2d was rendering. I also verified this outside of gecko. These need a LUT of 2.3f gamma [1] with a contrast of 1.0. So even though we tell d2d to use a gamma of 1.8 and a contrast of 1.0f for GDI fonts, it looks roughly equal to that of a LUT of 2.3f and contrast 1.0. This patch reaches out to gfxWindowsPlatform to get the contrast / gamma from the cleartype parameters we setup, and forces a 2.3f gamma LUT for forced GDI typefaces. [1] http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/ports/SkFontHost_win.cpp#1047

Comment on attachment 8818712 [details] [diff] [review] Set contrast to 1.0f for dwrite fonts and 2.3f gamma for GDI fonts Review of attachment 8818712 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/skia/skia/src/ports/SkTypeface_win_dw.cpp @@ +269,3 @@ > h = SkPaint::kSlight_Hinting; > rec->setHinting(h); > I need to fix my non windows 10 indentation. I'll fix before push.

(In reply to Mason Chang [:mchang] from comment #1) > Created attachment 8818712 [details] [diff] [review] > Set contrast to 1.0f for dwrite fonts and 2.3f gamma for GDI fonts > > Some fonts were light or too dark depending on the font. We originally went > solved this by forcing the default contrast / gamma of 0.5 / 1.8 > respectively. However, skia fonts were still not quite right. Outside of > gecko, after tracing through the LUT values, I could tell no difference. The > problem was that for dwrite fonts with the recommended rendering mode, we > should be using a contrast of 1.0, which is what d2d was doing. > > However, originally, this caused other fonts to look odd or too bold. After > lots of head banging, the fonts that force GDI backwards compatibility were > the ones looking odd, and a contrast of 1.0 for these fonts was too dark > compared to what d2d was rendering. I also verified this outside of gecko. > These need a LUT of 2.3f gamma [1] with a contrast of 1.0. So even though we > tell d2d to use a gamma of 1.8 and a contrast of 1.0f for GDI fonts, it > looks roughly equal to that of a LUT of 2.3f and contrast 1.0. > > This patch reaches out to gfxWindowsPlatform to get the contrast / gamma > from the cleartype parameters we setup, and forces a 2.3f gamma LUT for > forced GDI typefaces. > > [1] > http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/ports/ > SkFontHost_win.cpp#1047 You need to add the line to set MOZILLA_INTERNAL_API to generate_mozbuild.py as well, not just moz.build Also, you should add it under the section where it is already checking if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows'. Chucking 'windows' on to the gtk/android section like you did is not safe and will break.

Try looks good - https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2ad93578dac1c4c6b14a18b10cc82aa9bafb923

Pushed by mchang@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/def622794e83 Part 1: Use correct gamma and contrast for dwrite fonts. r=lsalzman https://hg.mozilla.org/integration/mozilla-inbound/rev/6b11cbf213c4 Part 2: Reftest fuzzing for updated gamma and contrast. r=lsalzman

If these stick, can you please uplift to aurora Lee? Not sure you're on PTO next week though. Thanks!

https://hg.mozilla.org/mozilla-central/rev/def622794e83 https://hg.mozilla.org/mozilla-central/rev/6b11cbf213c4

Comment on attachment 8818943 [details] [diff] [review] gamma.patch Approval Request Comment [Feature/Bug causing the regression]: bug 1314090, 52+, since we moved to Skia for content rendering [User impact if declined]: Fonts on Windows can have incorrect gamma, thus looking too thin or too fat, depending on whether DirectWrite fonts are used. [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: aurora [Is the change risky?]: No [Why is the change risky/not risky?]: Ensures fonts with Skia content on Windows look closer to what users had with Direct2D or Cairo content rendering. [String changes made/needed]: None

Comment on attachment 8819382 [details] [diff] [review] reftest fuzzing for gamma change Note that this patch is just fuzz necessitated by the real patch above. So this patch must be uplifted along with the other one. Approval Request Comment [Feature/Bug causing the regression]: bug 1314090, 52+, since we moved to Skia for content rendering [User impact if declined]: Fonts on Windows can have incorrect gamma, thus looking too thin or too fat, depending on whether DirectWrite fonts are used. [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: aurora [Is the change risky?]: No [Why is the change risky/not risky?]: Ensures fonts with Skia content on Windows look closer to what users had with Direct2D or Cairo content rendering. [String changes made/needed]: None

Comment on attachment 8819382 [details] [diff] [review] reftest fuzzing for gamma change Tests for skia fix, let's try these on aurora.

Comment on attachment 8818943 [details] [diff] [review] gamma.patch Fix for font regression from 52, let's uplift this to aurora.

https://hg.mozilla.org/releases/mozilla-aurora/rev/e4b8e4c0c669 https://hg.mozilla.org/releases/mozilla-aurora/rev/7abe1b00cb19