STR: 1. Open this URL - https://bugzilla.mozilla.org/show_bug.cgi?id=1307833 and enjoy awful blurry fonts in text in tab title Caused by: https://hg.mozilla.org/mozilla-central/rev/6fddc51e9d15 bug 658467 - Fade out tab label on overflow instead of ellipsis. r=jaws ui-r=shorlander What's more, bug #1314133 makes fonts in text in tab titles more awful and more blurrier [Tracking Requested - why for this release]: Regression

Loss off sub-pixel AA was expected, see bug 658467 comment 59, although I thought Markus fixed this in bug 1305259. That grayscale AA looks blurry here with the GPU process enabled was definitely not expected.

Using blurry grayscale antialiasing method instead of sharp subpixel antialiasing method is not acceptable, especially for users with hyperopia, myopia, presbyopia, amblyopia and other eyes problems, so I'm readding "access" keyword, same as "ux-consistency", as Firefox started using other font scalling method in GUI based on tab titles length, which is not consistent.

(In reply to Dão Gottwald [:dao] from comment #5) > Loss off sub-pixel AA was expected, see bug 658467 comment 59, although I > thought Markus fixed this in bug 1305259. I thought so too, so I'd like to look into why that's not the case.

(In reply to Virtual_ManPL [:Virtual] - (ni? me) from comment #6) > Using blurry grayscale antialiasing method instead of sharp subpixel > antialiasing method is not acceptable, Grayscale AA is unfortunate and should be avoided where possible, but it's not always avoidable and it's not unacceptable either as Firefox and other desktop applications have been using it for years. Let's see if Markus can figure this out though. The blurriness I think is indeed unacceptable regardless of accessibility considerations. By the way, I don't actually see the blurriness myself on Windows 10 with the GPU process enabled. > same as "ux-consistency", as Firefox started using other font scalling > method in GUI based on tab titles length, which is not consistent. It's a rendering bug, not a matter that needs UX input beyond what Stephen said in bug 658467 comment 59. You're not helping moving this bug forward by adding random keywords in an attempt to make this bug look more severe than it already is. In fact this muddies the waters, making it less clear what exactly this bug is about.

(In reply to Dão Gottwald [:dao] from comment #8) > Grayscale AA is unfortunate and should be avoided where possible, but it's > not always avoidable and it's not unacceptable either as Firefox and other > desktop applications have been using it for years. Let's see if Markus can > figure this out though. It's not the point if someone else using it elsewhere, as this bug is about Firefox. What's more I didn't saw usage of blurry grayscale antialiasing method instead of sharp subpixel antialiasing method in Microsoft Windows 7 system and other software which I'm using, except Firefox unfortunately. Also I'm not seeing any problems in competitive browsers like Chrome (which subpixel font rendering method is kinda fuzzy, but whatever they use it) or Internet Explorer. You should know already that I'm very sensitive to this things. ;) (In reply to Dão Gottwald [:dao] from comment #8) > The blurriness I think is indeed unacceptable regardless of accessibility > considerations. By the way, I don't actually see the blurriness myself on > Windows 10 with the GPU process enabled. It's because Windows 10 by default had awful blurry font rendering with DPI issues in some cases, so maybe that's why you didn't see the difference, :P maybe Microsoft fixed it already, but looking on - https://www.google.pl/search?q=Windows+10+font+rendering , seems not. (In reply to Dão Gottwald [:dao] from comment #8) > > same as "ux-consistency", as Firefox started using other font scalling > > method in GUI based on tab titles length, which is not consistent. > > It's a rendering bug, not a matter that needs UX input beyond what Stephen > said in bug 658467 comment 59. True, but patch from bug #658467 uncovered long standing issues in Firefox, which are now very visible and are causing more disadvantages, than advantages. (In reply to Dão Gottwald [:dao] from comment #8) > You're not helping moving this bug forward by adding random keywords in an > attempt to make this bug look more severe than it already is. In fact this > muddies the waters, making it less clear what exactly this bug is about. They wasn't random, but to the letter appropriate to the issue description, which I already explained why I added them in Comment #6, and nitpicking won't fix this issue either. But let's stay with your way, as you have much more experience, same as power.

(In reply to Markus Stange [:mstange] from comment #7) > (In reply to Dão Gottwald [:dao] from comment #5) > > Loss off sub-pixel AA was expected, see bug 658467 comment 59, although I > > thought Markus fixed this in bug 1305259. > > I thought so too, so I'd like to look into why that's not the case. Ah, so this is on Windows 7 with the glass effect, so the destination DrawTarget is not opaque. But http://searchfox.org/mozilla-central/rev/3e157ac240611f80df409ae76221d46a31c20dfc/gfx/thebes/gfxContext.cpp#819 tries to make this work anyway, if we know that the rectangle in question contains opaque pixels. So either we have a rectangle that's larger than the tab text, or the background behind the tab text is drawn in such a way that the DrawTarget doesn't (and maybe can't easily) detect it as opaque. I'll need to look into this some more.

Workaround for this disaster... .tab-label-container[textoverflow]:not([pinned]) { mask-image: unset !important; } .tab-label-container[textoverflow]:not([pinned])::after{ content: "" !important; } .tab-label-container[textoverflow]:not([pinned]) .tab-text { white-space: unset !important; }

(In reply to Virtual_ManPL [:Virtual] - (ni? me) from comment #9) > (In reply to Dão Gottwald [:dao] from comment #8) > > The blurriness I think is indeed unacceptable regardless of accessibility > > considerations. By the way, I don't actually see the blurriness myself on > > Windows 10 with the GPU process enabled. > > It's because Windows 10 by default had awful blurry font rendering with DPI > issues in some cases, so maybe that's why you didn't see the difference, :P Nope. Here's what I see on Windows 10.

[Tracking Requested - why for this release]: Bug 1314133 landed in 52. If the blurriness that :Virtual is seeing is indeed due to the GPU process and could affect web content, then we should probably take care of that in 52 even if the tab strip doesn't expose the bug in that release.

Ignore my previous comment, layers.gpu-process.dev.enabled is only true in nightly builds, it doesn't currently ride release trains.

(In reply to Dão Gottwald [:dao] from comment #12) > Created attachment 8818039 [details] > Windows 10 with GPU process enabled > > (In reply to Virtual_ManPL [:Virtual] - (ni? me) from comment #9) > > (In reply to Dão Gottwald [:dao] from comment #8) > > > The blurriness I think is indeed unacceptable regardless of accessibility > > > considerations. By the way, I don't actually see the blurriness myself on > > > Windows 10 with the GPU process enabled. > > > > It's because Windows 10 by default had awful blurry font rendering with DPI > > issues in some cases, so maybe that's why you didn't see the difference, :P > > Nope. Here's what I see on Windows 10. Looks nice, but it's probably with non default text size and DPI or on some HiDPI screen. Can you please try it on normal screen and report back? Thank you very much for looking on web content fonts, but thankfully most of blurriness were taken care by Mason Chang [:mchang] in bug #1315568, still there are some minor leftovers which I'm yet to report.

I tried making a smaller test case based on the tab title bar, but couldn't create the same situation after plugging away for a while. Dao, do you think you could please make a smaller test case for the tab title bar? Thanks! A patch to fix the test case in comment 17 fixes only that test case, but not the title bar.

Bug 1323587 will help with the blurriness, but not completely. In non-tab cases where we have grayscale AA, Skia is equal to D2D, so it's hard to tell what's going on here w/o a better test case. Also, there are two issues here. 1) When text along a tab is longer than the tab, lose subpixel AA. In this case, Skia == D2D. 2) When we fallback to grayscale AA, Skia != D2D.

(In reply to Mason Chang [:mchang] from comment #18) > I tried making a smaller test case based on the tab title bar, but couldn't > create the same situation after plugging away for a while. Dao, do you think > you could please make a smaller test case for the tab title bar? Thanks! I can't make a testcase since I can't reproduce the blurriness.

Tracking 53+ for this regression so we can address the issues noted in Comment 13.

I narrowed it down to how skia renders grayscale AA fonts. With D2D, we just set a flag to disable subpixel AA and off we go. Skia renders grayscale AA fonts by first rendering it as a cleartype font, then converting it to a grayscale AA font. [1] This causes the blurriness as it isn't quite equal to what D2D gives us. I did some experiments outside of gecko, and the fonts don't quite look the same. Given a glyph run, there seems to be no way to get a grayscale mask like we can get a cleartype mask. Jeff took a look at how d2d is getting grayscale AA masks and it's calling some internal APIs that we don't quite have access to. An alternate route is to create a WICBitmapRenderTarget[2], render the glyph through d2d, then readback the pixels. I have a prototype that mostly resolves the blurriness at [3]. However, the problem with this is that is VERY slow. On a Windows 10 Skylake machine Hidpi, it takes 14-16 ms (milliseconds) to render 1 glyph, whereas creating an alpha texture and gamma correction takes ~50 microseconds. Interestingly enough, in this test case, directly asking d2d to render text also takes ~6-16ms. On a non-hidpi Windows 7 machine, rendering with D2D takes ~4-8 ms, still takes ~14-16ms to render a glyph with a bitmap render target, and ~40-50 microseconds to create an alpha texture. If we cache the BitmapRenderTarget and the underlying bitmap, just drawing the glyph and reading it back still takes ~2ms. With the way Skia is built, it asks d2d to render 1 glyph at a time, caches the glyph, then blits it as needed. This would cause lots of jank for grayscale AA as it takes a lot of time to render a glyph. It would only take 8 required characters (8 * 2 ms per glyph) before we'd skip a frame. With Cairo instead of Skia, grayscale fonts still aren't equal to d2d, but better. This is because Cairo uses GDI fonts, but from a cursory glance, it looks like Cairo uses the BitmapRenderTarget and will be slow as well [4] if we did support dwrite fonts with Cairo. I'm leaning towards accepting this regression for grayscale fonts. For the URL bar / tabs, this is actually a layout bug that should be enabling subpixel AA. Chrome is currently shipping with these blurry fonts for grayscale AA, but I haven't found a test case where they fallback to grayscale AA yet. [1] http://searchfox.org/mozilla-central/source/gfx/skia/skia/src/ports/SkScalerContext_win_dw.cpp#886 [2] https://msdn.microsoft.com/en-us/library/windows/desktop/dd371309(v=vs.85).aspx [3] https://github.com/changm/DWriteFont/blob/master/DWriteFont/D2DSetup.cpp#L522 [4] http://searchfox.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-dwrite-font.cpp#1168

(In reply to Mason Chang [:mchang] from comment #22) > ... For the > URL bar / tabs, this is actually a layout bug that should be enabling > subpixel AA. Chrome is currently shipping with these blurry fonts for > grayscale AA, but I haven't found a test case where they fallback to > grayscale AA yet. Mason, is this bug 1307833?

(In reply to Milan Sreckovic [:milan] from comment #23) > (In reply to Mason Chang [:mchang] from comment #22) > > ... For the > > URL bar / tabs, this is actually a layout bug that should be enabling > > subpixel AA. Chrome is currently shipping with these blurry fonts for > > grayscale AA, but I haven't found a test case where they fallback to > > grayscale AA yet. > > Mason, is this bug 1307833? Yes. Essentially, DWrite is giving us fonts that look quite different from regular d2d grayscale AA Fonts. We fixed the issues for subpixel AA. But for the url / tabs, layout shouldn't be switching from subpixel to grayscale depending on the length of the title bar, which looks like that bug.

(In reply to Mason Chang [:mchang] from comment #22) > For the URL bar / tabs, this is actually a layout bug that should be > enabling subpixel AA. Is there a list of the places where the layout bug is confirmed (search bar, tooltips, ...) ? IHMO, there is a second bug (in layout too ?) : when subpixel AA is disabled, the hinting should be strong (which should help with the issue, the result would be different but not blurry). On Linux, Chrome follows the system settings, so it can do grayscale AA with strong hinting, and it disables subpixel positionning in this case (https://bugs.chromium.org/p/chromium/issues/detail?id=390317).

In a couple of hours, these should be ready - https://archive.mozilla.org/pub/firefox/try-builds/mchang@mozilla.com-5f31b2a5d32c2be3ad1c1e7917c4be59a335d9c2/ Can you try these builds please? It maybe fixes the grayscale AA bluriness by telling skia to use only the green channel from the cleartype texture we get from dwrite fonts. Credit to Lee for his protip.

(In reply to Mason Chang [:mchang] from comment #27) > In a couple of hours, these should be ready - > https://archive.mozilla.org/pub/firefox/try-builds/mchang@mozilla.com- > 5f31b2a5d32c2be3ad1c1e7917c4be59a335d9c2/ > > Can you try these builds please? > > It maybe fixes the grayscale AA bluriness by telling skia to use only the > green channel from the cleartype texture we get from dwrite fonts. Credit to > Lee for his protip. Looks better, but unfortunately it's still not how it looked before, as still some focus, sharpness and clearness are missing, compared to Firefox builds without patches from bug #658467 and with latest Firefox builds with Classic Theme Restorer extension + 'No overflow effect' enabled, but we're going in the right direction tho.

(In reply to Virtual_ManPL [:Virtual] - (ni? me) from comment #28) > Created attachment 8832649 [details] > Firefox 54 (Classic Theme Restorer extension + 'No overflow effect' > enabled).png > > (In reply to Mason Chang [:mchang] from comment #27) > > In a couple of hours, these should be ready - > > https://archive.mozilla.org/pub/firefox/try-builds/mchang@mozilla.com- > > 5f31b2a5d32c2be3ad1c1e7917c4be59a335d9c2/ > > > > Can you try these builds please? > > > > It maybe fixes the grayscale AA bluriness by telling skia to use only the > > green channel from the cleartype texture we get from dwrite fonts. Credit to > > Lee for his protip. > > Looks better, but unfortunately it's still not how it looked before, as > still some focus, sharpness and clearness are missing, compared to Firefox > builds without patches from bug #658467 and with latest Firefox builds with > Classic Theme Restorer extension + 'No overflow effect' enabled, but we're > going in the right direction tho. Thanks for testing! It won't look the same because we should be using subpixel AA here, which is a separate layout bug. This patch just makes grayscale AA look slightly more like D2D's grayscale AA.

(In reply to Mason Chang [:mchang] from comment #32) > Thanks for testing! It won't look the same because we should be using > subpixel AA here, which is a separate layout bug. This patch just makes > grayscale AA look slightly more like D2D's grayscale AA. So I assume that layout has been done with subpixel AA, and it's too late to ask DirectWrite to render with grayscale AA instead of emulating it. Could the UI ask for grayscale AA in this case (like https://developer.mozilla.org/en/docs/Web/CSS/font-smooth, even if no one seems to be supported on content) ?

Comment on attachment 8833029 [details] [diff] [review] Use only G channel for grayscale AA fonts in skia Review of attachment 8833029 [details] [diff] [review]: ----------------------------------------------------------------- Apologies if this was already discussed and rejected :) ::: gfx/skia/skia/src/ports/SkScalerContext_win_dw.cpp @@ +687,5 @@ > + > + // Ignore the R, B channels. It looks the closest to what > + // D2D with grayscale AA has. But there's no way > + // to just get a grayscale AA alpha texture from a glyph run. > + dst[i] = sk_apply_lut_if<APPLY_PREBLEND>(g, table8); I'd still be interested to see what this would look like if we used luminance, instead of the original RGB average, or this change's "green only"; e.g., dst[i] = sk_apply_lut_if<APPLY_PREBLEND>(0.2126*r + 0.7152*g + 0.0722*b, table8); For comparison, the original did this: dst[i] = sk_apply_lut_if<APPLY_PREBLEND>(0.3333*r + 0.3333*g + 0.3334*b, table8); and this change does this: dst[i] = sk_apply_lut_if<APPLY_PREBLEND>(0.0*r + 1.0*g + 0.0*b, table8);

And one way to compare the solutions would be to look at, say, red text.

(In reply to Milan Sreckovic [:milan] from comment #35) > Comment on attachment 8833029 [details] [diff] [review] > Use only G channel for grayscale AA fonts in skia > > Review of attachment 8833029 [details] [diff] [review]: > ----------------------------------------------------------------- > > Apologies if this was already discussed and rejected :) > > ::: gfx/skia/skia/src/ports/SkScalerContext_win_dw.cpp > @@ +687,5 @@ > > + > > + // Ignore the R, B channels. It looks the closest to what > > + // D2D with grayscale AA has. But there's no way > > + // to just get a grayscale AA alpha texture from a glyph run. > > + dst[i] = sk_apply_lut_if<APPLY_PREBLEND>(g, table8); > > I'd still be interested to see what this would look like if we used > luminance, instead of the original RGB average, or this change's "green > only"; e.g., > > dst[i] = sk_apply_lut_if<APPLY_PREBLEND>(0.2126*r + 0.7152*g + 0.0722*b, > table8); > > For comparison, the original did this: > > dst[i] = sk_apply_lut_if<APPLY_PREBLEND>(0.3333*r + 0.3333*g + 0.3334*b, > table8); > > and this change does this: > > dst[i] = sk_apply_lut_if<APPLY_PREBLEND>(0.0*r + 1.0*g + 0.0*b, table8); I think the intuition of using luminance here would be "wrong", in the sense that these are to be thought of more like discrete monochromatic subpixels, as opposed to normal color components, i.e. imagine a monochromatic monitor with 3x the horizontal resolution. Thus by averaging them in any way we are actually blurring the result, rather than making luminance. So to the contrary, to make the result sharper, we are sort of applying a form of nearest samplest, where we sample the middle subpixel without any form of averaging, getting an aesthetically more pleasing result.

Tracking 54+.

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

Pushed by mchang@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/af248616e757 Use only the G channel for grayscale AA Dwrite fonts and Skia. r=lsalzman https://hg.mozilla.org/integration/mozilla-inbound/rev/83a56e0f45ff Part 2: Reftest fuzzing for skia dwrite grayscale fonts. r=lsalzman

https://hg.mozilla.org/mozilla-central/rev/af248616e757 https://hg.mozilla.org/mozilla-central/rev/83a56e0f45ff

Is there still something left here for followup bugs given comment #25, comment #28 and/or comment #33 ? Do we need an additional blocking bug filed for bug 1307833? It's not clear to me why that bug has 3 separate blocking bugs for the selected/unselected with/without favicon cases, and the comments here suggest that the problem is related to whether or not the page title fits on the tab (which seems orthogonal to those other qualifiers)...

(In reply to :Gijs from comment #43) > Is there still something left here for followup bugs given comment #25, > comment #28 and/or comment #33 ? Do we need an additional blocking bug filed > for bug 1307833? It's not clear to me why that bug has 3 separate blocking > bugs for the selected/unselected with/without favicon cases, and the > comments here suggest that the problem is related to whether or not the page > title fits on the tab (which seems orthogonal to those other qualifiers)... Yes. There are still problems where layout is falling back to grayscale AA instead of subpixel AA.

Bug 1307833 should be used as a bug for layout to fix to stop falling back to grayscale AA.

Comment on attachment 8833029 [details] [diff] [review] Use only G channel for grayscale AA fonts in skia Approval Request Comment [Feature/Bug causing the regression]: Skia on windows 1007702, and bug 658467 and GPU process [User impact if declined]: Grayscale AA fonts look bad [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]: See comment 0. Check in Nightly vs Aurora. [List of other uplifts needed for the feature/fix]: Whatever end result of bug 1307833 is. [Is the change risky?]: No [Why is the change risky/not risky?]: This just changes the rendering of grayscale AA fonts in Skia to only use 1 channel of the 3 that we used to use, causing sharper fonts. The real fix is to not fallback to grayscale AA. This just makes grayscale AA look more like D2D. (We can't make it equal, see comment 22). [String changes made/needed]: None

Note, we don't need the reftest fuzzing patch as it wasn't added until 54.

Thank you very much! \o/ Fonts are now much clearer with this patch starting on Firefox Nightly 54.0a1 (2017-02-07). Looks like, the leftovers like no subpixel antialiasing in text in long tab titles after landing patch from bug #658467, will be probably fixed with others in bug #1307833, so I won't be creating another one. ;P

Comment on attachment 8833029 [details] [diff] [review] Use only G channel for grayscale AA fonts in skia Fix for font fallback on linux, let's bring this up to beta.

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #49) > Comment on attachment 8833029 [details] [diff] [review] > Use only G channel for grayscale AA fonts in skia > > Fix for font fallback on linux, let's bring this up to beta. This should only be on Windows.