1706678 - Incorrect computation of viewport units in conic-gradient on html and body elements

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[PolariTOON]

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

Steps to reproduce:

I was testing one of my sites (see https://polaritoon.github.io/web-academy/lessons/04-js/ ) when i noticed one layer of the background broke in Firefox Nightly. On this site, gradients are used to display fake pages.
I made a reduced test case there: https://codepen.io/PolariTOON/full/WNRYGwK

Actual results:

For the real use case, depending on the viewport size, the first fake page background is not always fully white.
On the reduced test case, you see some parts of one or two red rectangles, depending on the viewport size.

Expected results:

For the real use case, each fake page should be fully white.
On the reduced test case, the whole viewport should be only green, since each red rectangle should be fully covered by a green rectangle.
This is what it used to be in previous versions of Firefox (i think this broke when 89 was in Nightly) and how it appears in other browsers.

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::DOM: Core & HTML' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=758854f18b75d515f5d1d2aba6549a6cebb5ca36&tochange=aefb5f58e2c0f9787a64b5a738461e4e0b858390

Comment 3

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1702638

Comment 4

[Nicolas Silva [:nical]]

My patches in bug 1702638 introduced a non-uniform scale applied when caching large (linear/conic/radial) gradients. However I got it backwards when taking the scale into account in the shader, causing this issue. Also the linear gradient shader wasn't properly taking the scale into account, but in a different way. I have a patch coming soon.

Comment 5

[Nicolas Silva [:nical]]

Attachment: Bug 1706678 - Fix cached gradient scaling. r=#gfx-reviewers

Large gradients are cached scaled down and stretched back by the image brush. Because the scaling factor is non-uniform the shader has to take it into account. The previous implementation was incorrectly accounting for the scale.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a51e4315c4f
Fix cached gradient scaling. r=gfx-reviewers,lsalzman

Comment 7

[Dorel Luca [:dluca]]

Backed out changeset 8a51e4315c4f (bug 1706678) for Webrench failures. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=338210837&repo=autoland&lineNumber=35593

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=8a51e4315c4f0e2b47dc9f2f2dc3af716767b862

Backout:
https://hg.mozilla.org/integration/autoland/rev/1053d057b9b14219739b7c0ea32f8a837e761027

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d436e803f68
Fix cached gradient scaling. r=gfx-reviewers,lsalzman

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/5d436e803f68

Comment 10

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:nical, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 11

[Nicolas Silva [:nical]]

Comment on attachment 9219137 [details]
Bug 1706678 - Fix cached gradient scaling. r=#gfx-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Some gradients won't render correctly (wrong size).
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple enough fix.
• String changes made/needed:

Comment 12

[Pascal Chevrel (relman team) -> :pascalc]

Nicolas, this patch does not apply to beta as it is done on top of Bug 1702228 which is not in 89. Is that fixing an issue we have on the beta branch?

Comment 13

[Pascal Chevrel (relman team) -> :pascalc]

Comment on attachment 9219137 [details]
Bug 1706678 - Fix cached gradient scaling. r=#gfx-reviewers

Fix for an 89 graphics regression in early betas, approved for 89 beta 11, thanks.

Comment 14

[Pascal Chevrel (relman team) -> :pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/c22c62a33bca

Comment 15

[Atila Butkovits]

Backed out from beta for causing wrench bustages. a=backout

Backout link: https://hg.mozilla.org/releases/mozilla-beta/rev/f551157e2b987091722f80ecd733187c7fccf9a5

Push with failures: https://treeherder.mozilla.org/jobs?repo=mozilla-beta&searchStr=linux%2Cx64%2Cwebrender%2Copt%2Cwebrender%2Cstandalone%2Cwebrender-linux64-release%2Cwrench

Failure log: https://treeherder.mozilla.org/logviewer?job_id=339280614&repo=mozilla-beta&lineNumber=35743
https://treeherder.mozilla.org/logviewer?job_id=339281026&repo=mozilla-beta&lineNumber=364

Comment 16

[Pascal Chevrel (relman team) -> :pascalc]

Marking as wontfix for 89 given the backout and the fact that we are ending the early beta period. Nical if you feel strongly that this bug should be fixed in 89, please contact me. Thanks

Comment 17

[Nicolas Silva [:nical]]

Conic gradients much larger than 2048 pixels would render quite wrong during the 89 cycle but conic gradients are fairly rare so perhaps it's not worth the risk. The bustage comes from a reference image that was regenerated in Bug 1707181. It's not clear to me why but the png of the reference isn't loaded anymore (eve though it worked fine on central). This patch doesn't apply cleanly without the one in bug 1707181.

The simplest thing is to just accept that very large conic gradients will render very wrong (and very large radial gradients slightly wrong) and avoid the risk of a late uplift.

If we do want to take the risk of the late uplift, the safest/simplest thing would be to disable the reftest that fails to load the reference image (removing the line platform(linux,mac) == blend-clipped.yaml blend-clipped.png in gfx/wr/wrench/reftests/filters/reftest.list.

Comment 18

[Nicolas Silva [:nical]]

Attachment: Upliftable patch

This contains the patch from this bug and bug 1707181 folded into one patch rebase on top of beta with a couple of disabled reftests because the reference images from bug 1707181 got corrupted somehow.

Comment 19

[Emilio Cobos Álvarez [:emilio]]

(In reply to Nicolas Silva [:nical] from comment #17)

I think the image bit is bug 1709608, fwiw

Comment 20

[Pascal Chevrel (relman team) -> :pascalc]

After talking with Nicolas, let's uplift the updated patch to beta.

Comment 21

[Pascal Chevrel (relman team) -> :pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/a4ea701ea990