1695807 - Text corruption when scrolling down in colorjs.io

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

Attachment: screenshot

Seems to happen reliably (yay)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=033aea12b973e40d7215914aeebe0e2606d088e9&tochange=23bd3cec784397ac9ce2d1d69fda2aa334ed9515

Comment 2

[Glenn Watson [:gw]]

I can reproduce this locally.

However, although the referenced patch (bug 1684781) results in it occurring, the problem appears to be caused by bug 1691859.

The mix-blend on the page that is corrupted is a multiply which runs through the fast path enabled in bug 1691859 (it doesn't hit the new mix-blend code path in bug 1684781). However, bug 1684781 does result in the advanced dual-source mix-blend occurring directly on a picture cache tile instead of an off-screen surface, perhaps exposing some kind of driver bug or similar (or a bug in the WR advanced blend impl).

Bug 1693998 was a previously reported regression with the same symptoms, that was also caused by bug 1691859. We thought that bug 1684781 fixed that issue, but it just seems to make it occur in different circumstances.

I think that until we understand better what's going on, we should reluctantly disable the advanced blend path in bug 1691859. I've verified that without that, neither of the reported regressions occur with the other mix-blend patch that I landed. Thoughts Lee?

Comment 3

[Glenn Watson [:gw]]

Confirmed that this also occurs with SWGL enabled, so it must be a bug in the advanced blend equation code path, rather than a driver issue. I still think the safest bet for now is to disable that advanced blend path, since it's in beta.

Comment 4

[Lee Salzman [:lsalzman]]

Attachment: Bug 1695807 - Mix blend image brush data should be RasterizationSpace::Screen. r=gw

Comment 5

[Lee Salzman [:lsalzman]]

Emilio, can you try my patch and verify if it fixes the issue for you?

It appears there was just a small innocuous bug in the original advanced blend equation code, where it chose raster-space local, instead of raster-space screen. Not sure if that was the right thing to do at the time, but right now it definitely needs to be raster-space screen to properly read the right UVs for the primitive in the shader. It stood out as a "one of these things is not like the others" the minute Glenn and I spotted it in the code.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/140002ffa867
Mix blend image brush data should be RasterizationSpace::Screen. r=gw

Comment 7

[Alexandru Michis [:malexandru]]

Backed out changeset 140002ffa867 (Bug 1695807) for causing reftest failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/8f9de8c6d2207a513d9685fb0825c913e3ae6242
Push with failures
Failure logs:

• mix-blend-mode-both-parent-and-blended-with-3D-transform.html
• mix-blend-mode-and-filter.svg

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Yes, the patch fixes the issue. All those failures look like actually passes, so I relanded with the fuzzy annotations removed.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aa752a99fe85
Mix blend image brush data should be RasterizationSpace::Screen. r=gw

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85db133d8b52
Extra test expectation adjustment.

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/aa752a99fe85
https://hg.mozilla.org/mozilla-central/rev/85db133d8b52

Comment 12

[Lee Salzman [:lsalzman]]

Comment on attachment 9206268 [details]
Bug 1695807 - Mix blend image brush data should be RasterizationSpace::Screen. r=gw

Beta/Release Uplift Approval Request

• User impact if declined: Visual artifacts in many sites that use mix-blend-mode in their CSS.
• 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): Fixes a long-standing bug in in mix-blend-mode support in WebRender, but doesn't add new features or semantics.
• String changes made/needed:

Comment 13

[Julien Cristau [:jcristau]]

This bug is marked as regressed by https://hg.mozilla.org/integration/autoland/rev/23bd3cec784397ac9ce2d1d69fda2aa334ed9515 which landed in 88. Is beta actually affected?

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 15

[Lee Salzman [:lsalzman]]

(In reply to Julien Cristau [:jcristau] from comment #13)

This bug is marked as regressed by https://hg.mozilla.org/integration/autoland/rev/23bd3cec784397ac9ce2d1d69fda2aa334ed9515 which landed in 88. Is beta actually affected?

Yes

Comment 16

[Mathew Hodson]

(In reply to Julien Cristau [:jcristau] from comment #13)

This bug is marked as regressed by https://hg.mozilla.org/integration/autoland/rev/23bd3cec784397ac9ce2d1d69fda2aa334ed9515 which landed in 88. Is beta actually affected?

From comment #2, it sounds like this was actually regressed by bug 1691859, which landed in Firefox 87.

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 9206268 [details]
Bug 1695807 - Mix blend image brush data should be RasterizationSpace::Screen. r=gw

approved for 87.0b8

Comment 18

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/1c1f65188757