Closed Bug 1303570 Opened 8 years ago Closed 8 years ago

Awful blurry fonts in text on Mozilla Firefox Nightly 51.0a1 (2016-09-16)

Categories

(Core :: Graphics: Text, defect)

Product:
Core
Component:
Graphics: Text
Version:
51 Branch
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla52
Tracking Flags:
Tracking Status
firefox48 --- unaffected
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 + verified
firefox52 + verified

People

(Reporter: Virtual, Assigned: bas.schouten)

References

Details

(Keywords: nightly-community, regression, ux-consistency, Whiteboard: [gfx-noted])

Attachments

(5 files, 5 obsolete files)

Graphics section from about:support.txt
1.36 KB, text/plain
Details
Bug 1303570 - Part 1: Support Subpixel AA drawing of DWrite fonts to transparent GDI surfaces.
58 bytes, text/x-review-board-request
jrmuizel
: review+
gchang
: approval-mozilla-aurora+
Details
Bug 1303570 - Part 2: When using Cairo use gfxWindowsSurfaces when creating surfaces in ContentClientBasic.
58 bytes, text/x-review-board-request
jrmuizel
: review+
gchang
: approval-mozilla-aurora+
Details
awfull blurry fonts.png
56.88 KB, image/png
Details
sharp fonts.png
47.53 KB, image/png
Details 
[Tracking Requested - why for this release]: Regression
Via local build
Last Good: 3d378a118900
First Bad: b2372a86671c

Regressed by:
b2372a86671c	Bas Schouten — Bug 1296658: Attempt to create the correct BackendType buffers for a LayerManager. r=jrmuizel
Blocks: 1296658
Has Regression Range: --- → yes
Has STR: --- → yes
Summary: Awful blurry fonts in text on Mozilla Firefox Nightly 51.0a1 (2016-09-17) → Awful blurry fonts in text on Mozilla Firefox Nightly 51.0a1 (2016-09-16)
Tracking 51+ as it important to be able to see fonts clearly.
tracking-firefox51: ? → +
Whiteboard: [gfx-noted]
Hmm.. I don't know about this. Before we were doing D2D and then readback. That is not really an option. Al alternative if we really want subpixel AA here is to create a special codepath, that for opaque surfaces on windows when the backend type is CAIRO, create a GDI surface. What do you think Jeff?
Flags: needinfo?(bas) → needinfo?(jmuizelaar)
(In reply to Bas Schouten (:bas.schouten) from comment #10)
> Hmm.. I don't know about this. Before we were doing D2D and then readback.
> That is not really an option. Al alternative if we really want subpixel AA
> here is to create a special codepath, that for opaque surfaces on windows
> when the backend type is CAIRO, create a GDI surface. What do you think Jeff?

I don't really understand how we regressed this. What was happening before and what's happening now? Which backends were we using before and which ones now and where?
Flags: needinfo?(jmuizelaar) → needinfo?(bas)
This is not general text - this is the transparent pulldowns, like the one in the hamburger menu.  It will go away in 52 with the switch to Skia.  Until then, we have to live with grayscale AA in these scenarios.  A lot less than ideal, but nothing else is practical.  I will leave open and tracking for 52, but it goes away when we close bug 795627.
status-firefox51: affected → wontfix
(In reply to Milan Sreckovic [:milan] from comment #13)
> Until then, we have to live with grayscale AA in these scenarios.  A lot
> less than ideal, but nothing else is practical.  I will leave open and
> tracking for 52

I think the bikeshed should be green, so I'll observe:

a) this is a very visible regression 
b) the regressing commit landed less than a week before 51 went from Nightly to Aurora

Did a ton of essential code land in those last few days that absolutely has to ship in 51? If not I don't get where the "nothing else is practical" comes from. If you mean that we have to ship this code eventually, backing out the problematic commit in Aurora gives you 6 extra weeks to fix bug 795627 without having to ship a regression.
(In reply to Milan Sreckovic [:milan] from comment #13)
> This is not general text - this is the transparent pulldowns, like the one
> in the hamburger menu.  It will go away in 52 with the switch to Skia. 
> Until then, we have to live with grayscale AA in these scenarios.  A lot
> less than ideal, but nothing else is practical.  I will leave open and
> tracking for 52, but it goes away when we close bug 795627.

I disagree making this regression as WONTFIX for any Firefox version.
Seeing text fonts clearly is have to feature.
Except we want too loose more Firefox users or making them rage on Bugzilla and Support.

Wouldn't it be better to backout an offending patch or uplift a fix that you mention?
Flags: needinfo?(milan)
Valid points.  We're not going to be able to enable Skia in 51, that is too large of a change.  
Bas, it is difficult to tell just from bug 1296658 and bug 1296657 what the visible or performance benefit we get from them.  What are they?
Flags: needinfo?(milan)
(In reply to Milan Sreckovic [:milan] from comment #16)
> Valid points.  We're not going to be able to enable Skia in 51, that is too
> large of a change.  
> Bas, it is difficult to tell just from bug 1296658 and bug 1296657 what the
> visible or performance benefit we get from them.  What are they?

There is no visual benefit to either of these. The performance benefit depends somewhat on the load the GPU is under. Since without these patches a GPU readback is forced every frame when using these pulldowns. I would fix this bug before backing out the aforementioned performance improvements.

I somewhat doubt a lot of users will actively notice this, but it's certainly possible. Font changes have a history of evoking a very loud response from a small group of users making it hard to get very good data on how they actually affect our user base.
Flags: needinfo?(bas)
So it will be like driving fast without glasses, I doubt anyone with sightedness will want that.

If users won't:
-use hamburger menu
-use bookmarks
-use download button,
they won't see the issue.

Yes...
but I doubt that most of user won't be:
-downloading files
-using bookmarks
-using hamburger menu to print, managing addons, customizing UX, changing some options, etc.

I suspect that users will gladly see text fonts clearly,
than rather have some "imaginary" not visible performance boost in synthetic test,
not that I want to neglect hard work for patch which caused this issue,
but this improvement could wait some time, and land after a side effects will be dealt off.


Not that I will fight to death for this,
but it pains me now how have to feature is being treated,
especially when I clearly remember how users (me included) react to awful blurry DirectWrite Natural rendering on some text fonts, which was if memory not fails me in Firefox 4, later this was fixed by tweaking these text fonts to use GDI Classic, not Natural mode.



Now, decide and choose your "poison". ;)
There are already 2 dupes (+ this current bug), only in alpha phase, so I think it's sufficient to back out the offending bug patch until a complete fix.
Disabling Direct2D fixes the hamburger menu, but not tooltips (which are different, but still with grayscale AA).
But it has other consequences : text in about:config is worse for example (both use subpixel AA, but color fringing is much more visible without D2D).
Summary: Awful blurry fonts in text on Mozilla Firefox Nightly 51.0a1 (2016-09-16) → Awful blurry fonts in text on Mozilla Firefox Nightly 51.0a1 (2016-09-16) causing eye strain
(In reply to Milan Sreckovic [:milan] from comment #13)
> Until then, we have to live with grayscale AA in these scenarios.  A lot
> less than ideal, but nothing else is practical.

Why is nothing else practical? Is it impossible to revert to whatever we were doing in FF50? This is a user-visible regression that's been identified at the very beginning of the Aurora cycle; what forces us to ship it in a release before we're also ready to ship the forthcoming change that will fix it?
Bas is going to come up with a fix for this without backing out the original patch.
Assignee: nobody → bas
Thank you very much for reconsidering the available options for fixing this issue.
Status: NEW → ASSIGNED
status-firefox51: wontfix → affected
Comment on attachment 8795222 [details]
Bug 1303570 - Part 1: Support Subpixel AA drawing of DWrite fonts to transparent GDI surfaces.

https://reviewboard.mozilla.org/r/81332/#review80306
Attachment #8795222 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8795223 [details]
Bug 1303570 - Part 2: When using Cairo use gfxWindowsSurfaces when creating surfaces in ContentClientBasic.

https://reviewboard.mozilla.org/r/81334/#review80308
Attachment #8795223 - Flags: review?(jmuizelaar) → review+
Removing a very subjective thing from the title.
Summary: Awful blurry fonts in text on Mozilla Firefox Nightly 51.0a1 (2016-09-16) causing eye strain → Awful blurry fonts in text on Mozilla Firefox Nightly 51.0a1 (2016-09-16)
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0963e4f02471
Part 1: Support Subpixel AA drawing of DWrite fonts to transparent GDI surfaces. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/f21ffbf119bd
Part 2: When using Cairo use gfxWindowsSurfaces when creating surfaces in ContentClientBasic. r=jrmuizel
Attached image sharp fonts.png — 

        Attachment #8792247 -
        Attachment is obsolete: true

        Attachment #8792248 -
        Attachment is obsolete: true

        Attachment #8794897 -
        Attachment is obsolete: true

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/0963e4f02471
https://hg.mozilla.org/mozilla-central/rev/f21ffbf119bd
Status: ASSIGNED → RESOLVED
Closed: 8 years ago

          status-firefox52:
          affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52

    
    

    
  
(In reply to Virtual_ManPL [:Virtual] - (ni? me) from comment #18)
> So it will be like driving fast without glasses, I doubt anyone with
> sightedness will want that.
> 
> If users won't:
> -use hamburger menu
> -use bookmarks
> -use download button,
> they won't see the issue.
> 
> Yes...
> but I doubt that most of user won't be:
> -downloading files
> -using bookmarks
> -using hamburger menu to print, managing addons, customizing UX, changing
> some options, etc.
> 
> I suspect that users will gladly see text fonts clearly,
> than rather have some "imaginary" not visible performance boost in synthetic
> test,
> not that I want to neglect hard work for patch which caused this issue,
> but this improvement could wait some time, and land after a side effects
> will be dealt off.
> 
> 
> Not that I will fight to death for this,
> but it pains me now how have to feature is being treated,
> especially when I clearly remember how users (me included) react to awful
> blurry DirectWrite Natural rendering on some text fonts, which was if memory
> not fails me in Firefox 4, later this was fixed by tweaking these text fonts
> to use GDI Classic, not Natural mode.
> 
> 
> 
> Now, decide and choose your "poison". ;)

The story is actually a little more complicated than that, it should also be noted that although there was indeed a very vocal minority on bugzilla. During the time when it came to support, comments and UX data from a statistically random sample of the population, there was no indication that these fonts were a major issue for firefox users.

Subpixel Antialiasing is a complicated issue, it should be noted a similar vocal minority appeared on bugzilla when we stopped respecting users their system settings (where you can disable subpixel AA). I can sort of understand this, my eyes are very sensitive to color and the color fringes on subpixel AA'ed text (or 'sharp' text as you call it ;-)) can be quite disturbing for me. (Note that the text in this case was regressed a little further than just subpixel AA, as it was still being hinted as if it -did- have subpixel AA)

When it comes to text, any change is bad to some, and 95% of the users will never notice, and yes, the same is probably true of performance improvements. I have no trouble putting some extra effort in to make those 5% of users happy, and I think this fix is the right decision, but it is important we stay focused on the quality of the product as a whole for all our users and don't respond disproportionately to things which happen to affect our experiences most personally (or Flash would've been gone years ago and we'd have a very different UI as far as I'm concerned ;-)).

    
    

    
  
FWIW, I don't know if this was the same issue, but I also noticed that text in changesets like [1] was very hard to read for a while, getting worse after scrolling. I'm pretty sure that's fixed now too. Unfortunately I didn't keep any examples on hand since I thought this bug was supposed to cover that too, but after reading more of the discussion I'm not so sure.

[1] https://hg.mozilla.org/integration/mozilla-inbound/rev/3d378a118900

    
    

    
  
Thank you very much Bas Schouten (:bas.schouten)! Awesome job!

You even fixed with this patch, these bugs:
bug #1299214
bug #1299221
and probably many other ones.

I only hope you didn't spend very long time working on it.


Thanks also go to Jeff Muizelaar [:jrmuizel] for fast patches reviews, and to Milan Sreckovic [:milan] for changing his opinion with a "little" persuasion.



As you can see in my attachments posted in Comment #31 and Comment #32,
this bug was the serious "awful blurry" issue,
which was especially visible on Unicode characters,
not a subjective thing for sure.



Going back in time about "very vocal minority" maybe it was like this, 
but I remember that many software news webpage sites posted how to change font rendering back to how it was before,
so it makes me in doubt if it was really that much minority. Not to mention forums, blogs, etc.
Also removing useful feature, like option to: enable/disable or respecting system settings, will cause the riot, so no wonder.



Thank you all very much one more time!



Requesting the uplift request.

    
    

    
  
Requesting the uplift request.
Flags: needinfo?(bas)

    
  
Depends on: 1306670

    
  
Depends on: 1306830

    
    

    
  

      
      
      
      

        Attached image
          
        52.0a1 (2016-09-30) (64-bit) (obsolete)
        — 
      

    


  
52.0a1 (2016-09-30) (64-bit)

Fixed, kind of, it was better before it was completely broken. Now History and Downloads flyout's text flickers and Menu > More is rendered properly only on hover.

    
    

    
  
(In reply to Yuliya from comment #39)
> Created attachment 8796823 [details]
> 52.0a1 (2016-09-30) (64-bit)
> 
> 52.0a1 (2016-09-30) (64-bit)
> 
> Fixed, kind of, it was better before it was completely broken. Now History
> and Downloads flyout's text flickers and Menu > More is rendered properly
> only on hover.

It's bug #1306670.

    
    

    
  
Hi :bas,
Since this bug is a regression and also affects 51, do you consider to uplift this for 51 if this patch is not too risky?

    
    

    
  
(In reply to Gerry Chang [:gchang] from comment #42)
> Hi :bas,
> Since this bug is a regression and also affects 51, do you consider to
> uplift this for 51 if this patch is not too risky?

Absolutely, but the two bugs it caused need to be fixed first. One has a patch, the other I'm working on.
Flags: needinfo?(bas)

    
    

    
  
(In reply to Emanuel Hoogeveen [:ehoogeveen] from comment #35)
> FWIW, I don't know if this was the same issue, but I also noticed that text
> in changesets like [1] was very hard to read for a while, getting worse
> after scrolling. I'm pretty sure that's fixed now too. Unfortunately I
> didn't keep any examples on hand since I thought this bug was supposed to
> cover that too, but after reading more of the discussion I'm not so sure.
> 
> [1] https://hg.mozilla.org/integration/mozilla-inbound/rev/3d378a118900

I think that was bug 1305381, which regressed Nightly shortly after this one did.

    
    

    
  

      
      
      
      

        Attached image
          
        52.0a1 (2016-10-11) (64-bit).gif (obsolete)
        — 
      

    


  
It looks almost fixed, text under "Open Help Menu" is still only rendered properly after you hover them by mouse. As seen on Nightly 52.0a1 (2016-10-11) (64-bit)
Is there a bug report for this?

    
    

    
  
Comment on attachment 8799881 [details]
52.0a1 (2016-10-11) (64-bit).gif

(In reply to Yuliya from comment #46)
> Created attachment 8799881 [details]
> 52.0a1 (2016-10-11) (64-bit).gif
> 
> It looks almost fixed, text under "Open Help Menu" is still only rendered
> properly after you hover them by mouse. As seen on Nightly 52.0a1
> (2016-10-11) (64-bit)
> Is there a bug report for this?

It's bug #1307833

        Attachment #8799881 -
        Attachment is obsolete: true

    
    

    
  
Hi :bas.schouten,
Since this also affects 51, do you think the patch is worth uplifting to 51?
Flags: needinfo?(bas)

    
    

    
  
(In reply to Gerry Chang [:gchang] from comment #48)
> Hi :bas.schouten,
> Since this also affects 51, do you think the patch is worth uplifting to 51?

I think the risk is sufficiently low it could be done, I'll leave the determination of whether it's important enough to release management.
Flags: needinfo?(bas)

    
    

    
  
Since this is a regression, can you help create the uplift request for 51 aurora?
Flags: needinfo?(bas)

    
    

    
  
Hi Milan,
Can you help find someone to nominate the uplift request for 51 aurora if Bas is working on others at this moment?
Flags: needinfo?(milan)

    
    

    
  
Hi :jrmuizel,
Can you help nominate the uplift request for 51 aurora?
Flags: needinfo?(jmuizelaar)
Flags: needinfo?(milan)
Flags: needinfo?(jmuizelaar)

    
    

    
  
Comment on attachment 8795222 [details]
Bug 1303570 - Part 1: Support Subpixel AA drawing of DWrite fonts to transparent GDI surfaces.

Approval Request Comment
[Feature/regressing bug #]: 1296658
[User impact if declined]: Blurry fonts in the UI
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Fairly low - this code path is being tested all the time because it's in the UI.
[String/UUID change made/needed]: n/a
Flags: needinfo?(bas)

        Attachment #8795222 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 8795223 [details]
Bug 1303570 - Part 2: When using Cairo use gfxWindowsSurfaces when creating surfaces in ContentClientBasic.

Approval Request Comment
As above.

        Attachment #8795223 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 8795222 [details]
Bug 1303570 - Part 1: Support Subpixel AA drawing of DWrite fonts to transparent GDI surfaces.

Fix a regression related to UI. Aurora51+.

        Attachment #8795222 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

        Attachment #8795223 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: