1575258 - Text blurry after expanding tiles on microsoftedgeinsider.com site

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[Lyubomir [:mystiquewolf]]

Attachment: Before expanding

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

1. Open https://www.microsoftedgeinsider.com/bg-bg/whats-new
2. Expand some tile

Actual results:

The text inside the tile after expanding it is blurry. In Chrome it is not so.

Expected results:

Text should not be blurry.

Comment 1

[Lyubomir [:mystiquewolf]]

Attachment: After expanding Firefox

Comment 2

[Lyubomir [:mystiquewolf]]

Attachment: After expanding Chrome

Comment 3

[Lyubomir [:mystiquewolf]]

Attachment: Troubleshooting info

Comment 4

[Hani Yacoub, Desktop QA]

Couldn't reproduce this issue on Ubuntu 18.04 x64 on Firefox Nightly 70.0a1, Firefox 69.0b16 and on Firefox 68.0.2.

Could you please try to reproduce this issue using a fresh profile? You have the steps here: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager

Thanks.

Comment 5

[Lyubomir [:mystiquewolf]]

The issue seems to appear only when webrender is enabled. Toggling gfx.webrender.force-disable solves the issue for me.

Comment 6

[Hani Yacoub, Desktop QA]

Setting "Core: Graphics: WebRender" component for this.

Comment 7

[Aria Beingessner [:Gankra]]

This is pretty serious. It looks like we're messing up the snapping or bounds of the text. It gets cut off, it gets blurry, it jiggles.

The core issue here is an animation that animated in a second copy of the content with opacity, scale(1.0681818182), and possibly some other key tweaks. It also sort of looks like WR is failing to "exit" animation mode?

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

My snapping changes do not fix this, not during the animation and not after. The most relevant change is that we no longer snap during the animation but it still looks odd. As such, I suspect the text shader snapping might be relevant here.

Comment 9

[Aria Beingessner [:Gankra]]

Attachment: reduced-test.html

Ok here's a heavily reduced test-case. I've identified that this requires:

• an outer div with an animation that references "transform" and has an "animation-fill-mode: forwards".
• an inner div with a subtle scale transform.

This combination makes webrender sad, and it acts like it's "stuck" in an animation (while also seemingly doing a bad job drawing the text in that animation). The animation and its transform don't need to actually do anything. For instance, in my test all it contains is 100% { transform: none }.

I think the animation-fill-mode is tricking everything to stay in "animation mode". If I replace animation-fill-mode with "animation-duration: 2s", we still render badly for 2 seconds, but then snap into a good quality text at the end.

So there's kinda two bugs here:

• we're producing really ugly results in this animation (perhaps acceptable for real animations, but this is particularly egregious. it looks like each glyph is getting randomly stretched or cut off, like we're messing up snapping)
• animation-fill-mode: forward keeps us trapped in this animation

You could also argue it's a bug that we enter animation mode even for animations that don't actually do anything, but that's more debatable, as it's a reasonable heuristic to expect an animation to be animated (and it may be difficult to detect otherwise).

Comment 10

[Aria Beingessner [:Gankra]]

Andrew seems to have fixed this while working on Bug 1555376 (which makes sense, my investigating had led me to the same basic places in the code), so transferring the bug to him.

Comment 11

[Aria Beingessner [:Gankra]]

Just for the sake of history: the issue for this one seemed to be that we were messing up some of our snapping logic in the RasterSpace::Local case, which resulted in the vertex shader producing bad coordinates for the vertices, which inevitably messed up the pixel shader (since it couldn't even reach all of the pixels it needed to).

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1575258 - Make text rasterize, render and snap glyphs consistently.

The glyph pixel space in which we rasterized glyphs differed from how we
rendered the rasterized glyphs in the shader. They need to be in
agreement because the glyph subpixel offset selected during
rasterization depends on it. This patch should make the paths consistent
with each other.

Additionally, during animations, we now snap the reference frame
relative offset ignoring the impact of any animated transforms. This
helps with minimizing glyph wiggling during the transition.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c4b65f3e7fb
Make text rasterize, render and snap glyphs consistently. r=lsalzman

Comment 14

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/09a93ed6c6f7
Backed out changeset 4c4b65f3e7fb for causing wrench bustages. CLOSED TREE

Comment 15

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=274606504&resultStatus=testfailed%2Cbusted%2Cexception&revision=4c4b65f3e7fb6cd86d222f7f5bb0bfb8290b6b0d&searchStr=wrench

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=274606504&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/09a93ed6c6f7ec451a138ecc13577c0f0cdbdc39

Also these unxpected passes: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274606559&repo=autoland&lineNumber=3783

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e12f9b18127
Make text rasterize, render and snap glyphs consistently. r=lsalzman

Comment 17

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/1e12f9b18127

Comment 18

[Lyubomir [:mystiquewolf]]

This is still a little bit not as sharp as before expanding the tile, but it's better now.

Comment 19

[Lyubomir [:mystiquewolf]]

Attachment: 2019-11-18_20-31.png

Comment 20

[Lyubomir [:mystiquewolf]]

Attachment: 2019-11-28_19-48.png

Comment 21

[Lyubomir [:mystiquewolf]]

First one is a different bug.

Comment 22

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Lyubomir Parvanov from comment #21)

First one is a different bug.

If you continue to see this on the latest nightlies, please open a new bug, as I believe this should have been fixed as part of bug 1555376. Thanks!

Comment 23

[Aria Beingessner [:Gankra]]

RE: https://bugzilla.mozilla.org/show_bug.cgi?id=1575258#c20

We've effectively done everything we can for this page. The ultimate problem that remains is that:

1. they're fading out the old text, and fading in similar-but-scaled text, which looks bad
2. they're scaling and animating the text with animation-fill-mode: forwards, which the spec currently specifies as implying will-change, which looks bad

1 is just fundamentally a bad effect, but 2 is more subtle and merits a bit of explanation. Basically, they're animating in some slightly scaled text which requires us to slightly stretch the text and use (less crisp) greyscale-aa. Normally this would be fine as it would only be like that for the extent of the animation, and then snap back to a higher quality rendering... BUT they're explicitly requesting that the content stay in "animation mode" forever by using fill-mode, so we have to keep the text in this ugly form forever to preserve how it looked during the animation.

So IMO the page is definitely ugly but that's just because they made an ugly page and we're accurately rendering it. So, WONTFIX from our side.

Comment 24

[Alexandru Trif, Desktop QA [:atrif]]

Reproduced the issue with firefox 70.0a1 (20190820215247) on Windows 10x64 using STR from comment 0 and test case from comment 9.
The issue is verified fixed with Firefox 72.0b5 (20191210230245) on Windows 10x64, macOS 10.15 and Ubuntu 18.04 while having WebRender enabled.