Closed Bug 1321901 Opened 3 years ago Closed 3 years ago
Font Face::Get Recommended Rendering Mode for font rendering mode in Skia
Some fonts still look off compared to d2d when using Skia. I was testing a couple of sites and noticed a difference on specific fonts at specific sizes. After looking around, it was because Skia was using CLEARTYPE_NATURAL_SYMMETRIC mode  by default. Gecko uses DWRITE_RENDERING_MODE_DEFAULT , which at least from what I can tell, uses the same mode as GetRecommendedRenderingMode. If I change Skia to use GetRecommendedRenderingMode depending on the typeface, we get pretty much the exact same fonts as D2D, fixing the lightness issue. I was able to reproduce this outside of Gecko as well.  http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/ports/SkScalerContext_win_dw.cpp#321  http://searchfox.org/mozilla-central/source/gfx/thebes/gfxWindowsPlatform.cpp#1265
Compare this on non-hidpi on d2d vs Skia. The underside of the U is lighter and there is fuzz above and below the D.
Will file upstream bug as well.
Found some text that is still lighter than d2d. The first patch fixes the original test case.
Comment on attachment 8816606 [details] [diff] [review] Use Recommended RenderingMode depending on typeface Still trying to fix the lightness issue on some fonts, but this patch fixes a few other weird looking text too. Will fixup the lightness issue in another patch.
Comment on attachment 8816606 [details] [diff] [review] Use Recommended RenderingMode depending on typeface Review of attachment 8816606 [details] [diff] [review]: ----------------------------------------------------------------- r=lsalzman yes from work week
Attachment #8816606 - Flags: review?(lsalzman) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/f7e0dfe88d42 Use IDWriteFontFace::GetRecommendedRenderingMode for font rendering mode in Skia. r=lsalzman
Backed out for failing e.g. reftest text-emphasis-style-property-002.html on unaccelerated Windows 8 x64 instances: https://hg.mozilla.org/integration/mozilla-inbound/rev/387b89eeed54d07238a2e0e878e44ca7cdbd26cb Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=f7e0dfe88d42e4c326fdb75b1ebd4228fd1d2f01 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=40373698&repo=mozilla-inbound
For the light test case, I figured out it was because the default contrast we use with d2d is 1.0, but it is 0.5 for Skia. However, setting contrast to 1.0 in Skia makes other fonts too thick.
Looks like fonts where we force GDI mode use a contrast of 0.5, but non GDI fonts use 1.0. Even though in gfxWindowsPlatform, both are set to use 1.0 contrast. Note taking for myself here.
From playing with GDI fonts outside of gecko, it looks like a contrast with the LUT of ~0.7-0.75 is ~= D2D with GDI Dwrite params which contain a contrast of 1.0.
Some of the test cases were failing because they would use DWRITE_RENDERING_MODE_OUTLINE, which isn't really supported. Cairo would fallback to DWRITE_RENDERING_MODE_CLEARTYPE_NATURAL_SYMMETRIC, which is what Skia would do previously as well. Updated to copy this behavior. Some of the reftests needed extra fuzzing too.  http://searchfox.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-d2d-surface.cpp#3767
Comment on attachment 8818681 [details] [diff] [review] Use Recommended rendering mode Do we need the include "nsDebug.h"? It doesn't look like it.
Attachment #8818924 - Flags: review?(lsalzman) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/29a19471a5aa Use IDWriteFontFace::GetRecommendedRenderingMode for font rendering mode in Skia. r=lsalzman
Comment on attachment 8818924 [details] [diff] [review] Use Recommended rendering mode Approval Request Comment [Feature/Bug causing the regression]: Skia on windows, bug 1007702 [User impact if declined]: Some fonts will look slightly different for users. [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]: Will probably also need bug 1323587. [Is the change risky?]: No [Why is the change risky/not risky?]: It just sets a different rendering mode flag depending on the font. [String changes made/needed]: None
Attachment #8818924 - Flags: approval-mozilla-aurora?
Comment on attachment 8818924 [details] [diff] [review] Use Recommended rendering mode change font rendering mode with skia on windows, for aurora52
Attachment #8818924 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.