1345222 - ClearType rendering broken and inconsistent with OS settings

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Petr Vones]

After upgrading from 51.0.1 to 52.0 I found that font rendering is completely broken. It seems as it uses a default ClearType rendering instead of system specific settings adjusted by Clear Type Tuner.

Comment 1

[Petr Vones]

Attachment: Incorrect ClearType rendering in Firefox 52

Comment 2

[Petr Vones]

Note that Thunderbird 45.8.0 on same machines does NOT have the font rendering issue.

Comment 3

[Petr Vones]

More information, it occurs on machines where hardware accelearation (DIRECT2D etc) is disabled because of outdated drivers (ATI X550 hardware) or when Firefox is running inside a virtual machine. On a more up-to-date system (Intel HD3000) the issue is not present. 

It is still a regression because the issue was not present on any system configuration before version 52.

Comment 4

[Loic]

Is it possible to run the tool mozregression to narrow down a regression range?
See http://mozilla.github.io/mozregression/ for details.
Run the command "mozregression --bits=32 --good=51" then copy here the final pushlog from mozilla-inbound.

In addition, could you paste the section "graphics" from about:support.

Comment 5

[Petr Vones]

Graphics
--------

Features
Compositing: Basic
Asynchronous Pan/Zoom: wheel input enabled; touch input enabled
WebGL Renderer: Google Inc. -- ANGLE (Software Adapter Direct3D11 vs_5_0 ps_5_0)
WebGL2 Renderer: Google Inc. -- ANGLE (Software Adapter Direct3D11 vs_5_0 ps_5_0)
Hardware H264 Decoding: No; Hardware video decoding disabled or blacklisted
Audio Backend: wasapi
DirectWrite: true (6.2.9200.21976)
GPU #1
Active: Yes
Description: Radeon X300/X550/X1050 Series
Vendor ID: 0x1002
Device ID: 0x5b63
Driver Version: 8.593.100.0
Driver Date: 2-10-2010
Drivers: atiumd64 atiumdag atiumdva atiumd6a atitmm64
Subsys ID: 21261458
RAM: Unknown

Diagnostics
ClearType Parameters: Gamma: 2,2 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 50
AzureCanvasAccelerated: 0
AzureCanvasBackend: skia
AzureContentBackend: skia
AzureFallbackCanvasBackend: cairo
ClearType Parameters: Gamma: 2,2 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 50
Decision Log
D3D11_COMPOSITING:
failed by runtime: Failed to acquire a D3D11 device
D3D9_COMPOSITING:
disabled by default: Disabled by default
DIRECT2D:
failed by runtime: Failed to acquire a Direct3D 11 content device
D3D11_HW_ANGLE:
disabled by env: D3D11 compositing is disabled

Comment 6

[Petr Vones]

The issue also persists with new fresh profile on affected machines.

Comment 7

[Petr Vones]

I reverted back to 51.0.1 because the 52.0 fonts are unreadable for me, below is the Display section from this version:

Name: Firefox
Version: 51.0.1

Graphics
--------

Features
Compositing: Basic
Asynchronous Pan/Zoom: wheel input enabled
WebGL Renderer: Google Inc. -- ANGLE (Software Adapter Direct3D11 vs_5_0 ps_5_0)
WebGL2 Renderer: Google Inc. -- ANGLE (Software Adapter Direct3D11 vs_5_0 ps_5_0)
Hardware H264 Decoding: No; Hardware video decoding disabled or blacklisted
Audio Backend: wasapi
DirectWrite: false (6.2.9200.21976)
GPU #1
Active: Yes
Description: Radeon X300/X550/X1050 Series
Vendor ID: 0x1002
Device ID: 0x5b63
Driver Version: 8.593.100.0
Driver Date: 2-10-2010
Drivers: atiumd64 atiumdag atiumdva atiumd6a atitmm64
Subsys ID: 21261458
RAM: Unknown

Diagnostics
ClearType Parameters: Gamma: 2,2 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 50
AzureCanvasAccelerated: 0
AzureCanvasBackend: skia
AzureContentBackend: cairo
AzureFallbackCanvasBackend: cairo
ClearType Parameters: Gamma: 2,2 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 50
failures: [GFX1]: D3D11 device creation failed: 0x887a0004
Decision Log
D3D11_COMPOSITING:
failed by runtime: Failed to acquire a D3D11 device
D3D9_COMPOSITING:
disabled by default: Disabled by default
DIRECT2D:
failed by runtime: Failed to acquire a Direct3D 11 content device
D3D11_HW_ANGLE:
disabled by env: D3D11 compositing is disabled


Failure Log
(#0) Assert: D3D11 device creation failed: 0x887a0004

Comment 8

[Petr Vones]

I found a temporary workaround, read value EnhancedContrastLevel from registry key HKEY_CURRENT_USER\Software\Microsoft\Avalon.Graphics\DISPLAY1 and copy it to gfx.font_rendering.cleartype_params.enhanced_contrast (that has default value -1).

Comment 9

[Loic]

Could you try to run mozregression, please. It would help to find the regressing bug introduced in FF52. Of course, you need to revert the temporary workaround in the registry.

Comment 10

[Petr Vones]

C:\Python27\Scripts>mozregression.exe --bits=32 --good=51
 0:01.04 INFO: No 'bad' option specified, using 2017-03-08
 0:01.06 INFO: Using date 2016-09-19 for release 51
 0:09.00 INFO: Testing good and bad builds to ensure that they are really good and bad...
 0:09.02 INFO: Downloading build from: https://archive.mozilla.org/pub/firefox/nightly/2016/09/2016-09-19-06-52-32 mozilla-central/firefox-52.0a1.en-US.win32.zip
===== Downloaded 100% =====
 0:24.31 INFO: Running mozilla-central build for 2016-09-19
 0:27.18 INFO: Launching c:\users\admin10\appdata\local\temp\tmpdru9gz\firefox\firefox.exe
 0:27.20 INFO: Application command: c:\users\admin10\appdata\local\temp\tmpdru9gz\firefox\firefox.exe -profile c:\users\admin10\appdata\local\temp\tmp3kqoe6.mozrunner
 0:27.22 INFO: application_buildid: 20160919065232
 0:27.24 INFO: application_changeset: fd0564234eca242b7fb753a110312679020f8059
 0:27.25 INFO: application_name: Firefox
 0:27.27 INFO: application_repository: https://hg.mozilla.org/mozilla-central
 0:27.31 INFO: application_version: 52.0a1
Was this nightly build good, bad, or broken? (type 'good', 'bad', 'skip', 'retry' or 'exit' and press Enter):

Comment 11

[Loic]

I think you missed the working of Mozregression. :)
For each build downloaded by the tool, your make the test to reproduce the issue (here displaying some text) and you enter if the build is good or bad.

After the run, Mozregression will find an interval of the last good build and the first bad build which is what we need. At the end, just paste the final pushlog from mozilla-inbound.

Comment 12

[Petr Vones]

I see :-) Wouldn't be easier just to compare source code changes between 51.0.1 and 52.0 version regarding reading ClearType parameter values ?

Comment 13

[Loic]

Not simple, many bugfixes are pushed in the graphics area.

Comment 14

[Masatoshi Kimura [:emk]]

The source code difference between 51.0.1 and 52.0 is too large to spot the offending change. It is much larger than you probably imagine. It would be really helpful if you narrow down the range using mozregression.

Comment 15

[Petr Vones]

I did the test twice, got similar results:

 6:44.80 INFO: Last good revision: 717d85fc2046149b2a3443ffcc7aff04e119b0f7
 6:44.80 INFO: First bad revision: 729307f6ac594f76d1de7d7697ca5c3880135de3
 6:44.81 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=717d85fc2046149b2a3443ffcc7aff04e119b0f7&tochange=729307f6ac594f76d1de7d7697ca5c3880135de3

 6:46.56 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1007702

Comment 16

[Loic]

Ty Petr.

Comment 17

[Masatoshi Kimura [:emk]]

I found a bug in gfxWindowsPlatform.cpp.

https://dxr.mozilla.org/mozilla-central/rev/58753259bfeb3b818eac7870871b0aae1f8de64a/gfx/thebes/gfxWindowsPlatform.cpp#1139-1140
>#define ENHANCED_CONTRAST_REGISTRY_KEY \
>    HKEY_CURRENT_USER, "Software\\Microsoft\\Avalon.Graphics\\DISPLAY1\\EnhancedContrastLevel"

https://dxr.mozilla.org/mozilla-central/rev/58753259bfeb3b818eac7870871b0aae1f8de64a/gfx/thebes/gfxWindowsPlatform.cpp#1207-1208
>            if (RegOpenKeyExA(ENHANCED_CONTRAST_REGISTRY_KEY,
>                              0, KEY_READ, &hKey) == ERROR_SUCCESS)

"EnhancedContrastLevel" is a named value, not a key. So RegOpenKeyExA(HKEY_CURRENT_USER, "Software\\Microsoft\\Avalon.Graphics\\DISPLAY1\\EnhancedContrastLevel") will always fail.

Comment 18

[Masatoshi Kimura [:emk]]

Attachment: Bug 1345222 - Fix user-set ClearType params detection.

Review commit: https://reviewboard.mozilla.org/r/118634/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/118634/

Comment 19

[Loic]

We probably need that in dot release for FF52.

Comment 20

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

https://reviewboard.mozilla.org/r/118634/#review120548

Thanks!

Comment 21

[Masatoshi Kimura [:emk]]

Petr, could you confirm that this build fixes your issue?
https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-70491b4b9ba1b24a784fb9b57730a9f71485087a/try-win32/firefox-55.0a1.en-US.win32.installer.exe

Comment 22

[Milan Sreckovic [:milan] (needinfo for best results)]

If this fixes the problem, let's ask for the uplift - 53 and 54, maybe as a ride-along for 52, likely 52ESR.

Comment 23

[Mason Chang [Inactive] [:mchang]]

(In reply to Milan Sreckovic [:milan] (not likely to respond before 3/27) from comment #22)
> If this fixes the problem, let's ask for the uplift - 53 and 54, maybe as a
> ride-along for 52, likely 52ESR.

I'm not sure we want to uplift, but maybe? We can do 53 and 54, but this has been broken for 3 years. If we fix it, we want to see how many people report that their fonts look difference because we're actually respecting contrast not versus we were ignoring it before.

Comment 24

[Masatoshi Kimura [:emk]]

Although this has been broken for 3 years, it has not been visible until Skia is enableed by default. So the reporter found the "regression" since Firefox 52.

Comment 25

[Mason Chang [Inactive] [:mchang]]

(In reply to Masatoshi Kimura [:emk] from comment #24)
> Although this has been broken for 3 years, it has not been visible until
> Skia is enableed by default. So the reporter found the "regression" since
> Firefox 52.

But this will also change cleartype fonts for people who are using d2d, which should be the majority of our users.

Comment 26

[Petr Vones]

(In reply to Masatoshi Kimura [:emk] from comment #21)
> Petr, could you confirm that this build fixes your issue?
> https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-
> 70491b4b9ba1b24a784fb9b57730a9f71485087a/try-win32/firefox-55.0a1.en-US.
> win32.installer.exe

Yes, it fixes it.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf2b0421dd57
Fix user-set ClearType params detection. r=mchang

Comment 28

[aja]

(In reply to Pulsebot from comment #27)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/autoland/rev/cf2b0421dd57
> Fix user-set ClearType params detection. r=mchang

The win8/64 opt build of this change causes startup crashes on my win10 system...but no crash on dholbert's system.
...So may still have some font config or graphic card issues.  I asked sheriff to let it merge to m/c so useful dumps with symbols could be obtained.

Comment 29

[Masatoshi Kimura [:emk]]

My wild guess is that some resident programs hook RegQueryValueExA and they don't consider the last 4 parameters can be null even though MSDN says so?

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cf2b0421dd57

Comment 31

[Marcia Knous [:marcia]]

Tracking this visible regression for all branches since 52, per Comment 24.

Comment 32

[Ryan VanderMeulen [:RyanVM]]

Backed out on suspicion of being the real root cause of the topcrashes affecting today's nightlies (bug 1346215).

https://hg.mozilla.org/mozilla-central/rev/edd424223fb1c336294a0cc1ca0e31cbec79ec31

Comment 33

[aja]

(In reply to aja from comment #28)
> (In reply to Pulsebot from comment #27)
> > Pushed by [e-mail]:
> > https://hg.mozilla.org/integration/autoland/rev/cf2b0421dd57
> > Fix user-set ClearType params detection. r=mchang
> 
> The win8/64 opt build of this change causes startup crashes on my win10
> system...but no crash on dholbert's system.
> ...So may still have some font config or graphic card issues.  I asked
> sheriff to let it merge to m/c so useful dumps with symbols could be
> obtained.

Now that this has merged over to m-c, still getting startup crash with the nightly build:
https://crash-stats.mozilla.com/report/index/528e4090-1d76-4928-a615-8ec212170310

Note that the nightly already had backout of bug 1291483 for causing startup crashes like bug 1346215...which appear to have same crash signature as crash report noted above.

Comment 34

[Daniel Holbert [:dholbert]]

(In reply to aja from comment #33)
> Note that the nightly already had backout of bug 1291483 for causing startup
> crashes like bug 1346215...which appear to have same crash signature as
> crash report noted above.

(I didn't understand this ^^ at first, but I think aja's point is just that we now have extra confirmation that this bug here (not bug 1291483) is the cause of bug 1346215's startup crashes.)

(Virtual_ManPL also independently confirmed that this is the cause via local testing, as noted in bug 1346215 comment 15.)

Comment 35

[Masatoshi Kimura [:emk]]

Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118634/diff/1-2/

Comment 36

[Masatoshi Kimura [:emk]]

Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

Bug 1195661 must die.

Comment 37

[Masatoshi Kimura [:emk]]

Aja, could you confirm that this build no longer crashes?
https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-5f87957ca1441a4e2bf036a5004296929cfd2bc7/try-win64/

Comment 38

[aja]

(In reply to Masatoshi Kimura [:emk] from comment #37)
> Aja, could you confirm that this build no longer crashes?
> https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-
> 5f87957ca1441a4e2bf036a5004296929cfd2bc7/try-win64/

Confirmed...no startup crash with this try build.

Comment 39

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

https://reviewboard.mozilla.org/r/118634/#review121534

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01c54613bf73
Fix user-set ClearType params detection. r=mchang

Comment 41

[aja]

(In reply to Pulsebot from comment #40)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/autoland/rev/01c54613bf73
> Fix user-set ClearType params detection. r=mchang

No startup crash on win10/x64 system with an autoland win8/x64 build containing this push.

Comment 42

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/01c54613bf73

Comment 44

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora/Beta/ESR52 approval on this when you get a chance. Probably not worth trying to include in a dot release, though.

Comment 45

[eganda]

Sorry, will ClearType rendering be fixed in 52.0.1 and will it run on Windows XP?

Comment 46

[Masatoshi Kimura [:emk]]

Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: 52 ESR is the last browser that supports Windows XP and this patch will fix a bug on Windows XP.
User impact if declined: Some users would have degraded font rendering.
Fix Landed on Version: Nightly 52
Risk to taking this patch (and alternatives if risky): Low, alternative is not taking the patch.
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1007702 exposed the bug, but the bug was present in the Skia backend even before enabling Skia backend by default.
[User impact if declined]: Some users would have degraded font rendering.
[Is this code covered by automated tests?]: No, manually tested and verified.
[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]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This patch will only change the rendering result that will not be critical. Also this patch added more error checks to prevent startup crash.
[String changes made/needed]: None.

Comment 47

[Masatoshi Kimura [:emk]]

(In reply to eganda from comment #45)
> Sorry, will ClearType rendering be fixed in 52.0.1 and will it run on
> Windows XP?

No, it will not be fixed in 52.0.1. But if the ESR uplift request is approved, it will be fixed in future ESR 52 releases and it will run on Windows XP.

Comment 48

[eganda]

Ok, hope staff will approve it, because it is stated above that "Some users would have degraded font rendering.", actually everyone using Windows XP will front this problem. It is very unpleasant experience for every XP user to use last Firefox, I would say unbearable and could cause eye problems. I'm not joking.

Comment 49

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

Regression from 52, with a bad impact on users, let's bring this to 53 beta.

We should consider it for the next ESR (52.1.0) if it doesn't cause obvious problems in 53 beta.

Comment 50

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4d684246e152

Comment 51

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3bc5f2964d59

Comment 52

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I think the wrong versions of these patches got uplifted, users are complaining about crashes like what happened on Nightly around comment 30.

Backed out and re-landed the newer patch on aurora and beta:
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/0defb8aa6881a113f594046fdf552ff5916e6cda
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/81dc67758a0ff6cb85516ec417e242f5dea9290b

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/fe3a14531fea8e7b9a1e8cb86d72c14d698c31a7
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/a41eef042ceb6cdba74b279d5df464f14a7bd181

Comment 53

[Julien Cristau [:jcristau]]

Tracking for 52.1.0esr, wontfix for 52.

Comment 54

[Julien Cristau [:jcristau]]

AIUI esr52 uplift should be requested for bug 1348584 as well before taking this one.

Comment 55

[Julien Cristau [:jcristau]]

Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

font rendering fix, esr52+

Comment 56

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/864644fadcb0