Closed Bug 1345222 Opened 3 years ago Closed 3 years ago

ClearType rendering broken and inconsistent with OS settings

Categories

(Core :: Graphics, defect, major)

52 Branch
x86_64
Windows 7
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox52 + wontfix
firefox-esr52 53+ fixed
firefox53 + fixed
firefox54 + fixed
firefox55 + fixed

People

(Reporter: petr.v, Assigned: emk)

References

Details

(Keywords: regression)

Attachments

(2 files)

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.
Note that Thunderbird 45.8.0 on same machines does NOT have the font rendering issue.
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.
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.
Component: Untriaged → Graphics
Flags: needinfo?(petr.v)
Product: Firefox → Core
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
The issue also persists with new fresh profile on affected machines.
Flags: needinfo?(petr.v)
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
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).
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.
Flags: needinfo?(petr.v)
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):
Flags: needinfo?(petr.v)
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.
Flags: needinfo?(petr.v)
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 ?
Flags: needinfo?(petr.v)
Not simple, many bugfixes are pushed in the graphics area.
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.
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

Ty Petr.
Blocks: skia-windows
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
Ever confirmed: true
Flags: needinfo?(mchang)
See Also: → 1345379
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.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
We probably need that in dot release for FF52.
Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

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

Thanks!
Attachment #8845470 - Flags: review?(mchang) → review+
Flags: needinfo?(mchang)
If this fixes the problem, let's ask for the uplift - 53 and 54, maybe as a ride-along for 52, likely 52ESR.
Flags: needinfo?(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.
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.
(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.
(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/VYV03354@nifty.ne.jp-
> 70491b4b9ba1b24a784fb9b57730a9f71485087a/try-win32/firefox-55.0a1.en-US.
> win32.installer.exe

Yes, it fixes it.
Flags: needinfo?(petr.v)
(In reply to Pulsebot from comment #27)
> Pushed by VYV03354@nifty.ne.jp:
> 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.
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?
Tracking this visible regression for all branches since 52, per Comment 24.
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
Status: RESOLVED → REOPENED
Flags: needinfo?(VYV03354)
Resolution: FIXED → ---
Target Milestone: mozilla55 → ---
(In reply to aja from comment #28)
> (In reply to Pulsebot from comment #27)
> > Pushed by VYV03354@nifty.ne.jp:
> > 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.
Depends on: 1346215
(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 on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

Bug 1195661 must die.
Flags: needinfo?(VYV03354)
Attachment #8845470 - Flags: review+ → review?(mchang)
Aja, could you confirm that this build no longer crashes?
https://archive.mozilla.org/pub/firefox/try-builds/VYV03354@nifty.ne.jp-5f87957ca1441a4e2bf036a5004296929cfd2bc7/try-win64/
Flags: needinfo?(ajarope)
(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/VYV03354@nifty.ne.jp-
> 5f87957ca1441a4e2bf036a5004296929cfd2bc7/try-win64/

Confirmed...no startup crash with this try build.
Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

https://reviewboard.mozilla.org/r/118634/#review121534
Attachment #8845470 - Flags: review?(mchang) → review+
See Also: → 1346558
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/01c54613bf73
Fix user-set ClearType params detection. r=mchang
(In reply to Pulsebot from comment #40)
> Pushed by VYV03354@nifty.ne.jp:
> 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.
Flags: needinfo?(ajarope)
https://hg.mozilla.org/mozilla-central/rev/01c54613bf73
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Duplicate of this bug: 1347199
Please request Aurora/Beta/ESR52 approval on this when you get a chance. Probably not worth trying to include in a dot release, though.
Flags: needinfo?(mchang) → needinfo?(VYV03354)
Sorry, will ClearType rendering be fixed in 52.0.1 and will it run on Windows XP?
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.
Flags: needinfo?(VYV03354)
Attachment #8845470 - Flags: approval-mozilla-esr52?
Attachment #8845470 - Flags: approval-mozilla-beta?
Attachment #8845470 - Flags: approval-mozilla-aurora?
(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.
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 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.
Attachment #8845470 - Flags: approval-mozilla-beta?
Attachment #8845470 - Flags: approval-mozilla-beta+
Attachment #8845470 - Flags: approval-mozilla-aurora?
Attachment #8845470 - Flags: approval-mozilla-aurora+
Tracking for 52.1.0esr, wontfix for 52.
Depends on: 1349220
AIUI esr52 uplift should be requested for bug 1348584 as well before taking this one.
Comment on attachment 8845470 [details]
Bug 1345222 - Fix user-set ClearType params detection.

font rendering fix, esr52+
Attachment #8845470 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Depends on: 1358078
You need to log in before you can comment on or make changes to this bug.