1726755 - Most mix-blend-mode values draw artifacts [Adreno 3xx]

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Kestrel]

Attachment: Only three working mix-blend-modes on Adreno 305

Samsung Galaxy Tab 4 (SM-T530)
Android 5.0.2
Qualcomm Adreno (TM) 305
OpenGL ES 3.0 V@84.0

The only working mix-blend-mode values on this device are normal, screen and exclusion, the rest display artifacts.

https://www.w3schools.com/cssref/tryit.asp?filename=trycss_mix-blend-mode-all

Comment 1

[Miko Mynttinen]

Jamie, any idea what could be happening here?

Comment 2

[Jamie Nicol [:jnicol]]

The triangular shapes are the same driver issue as bug 1630356 and a few others for various shaders. Caused by using a flat varying scalar in the fragment shader, in this case brush_mix_blend's flat varying int v_op. The reason 3 of the modes work correctly is that they do not use the brush_mix_blend shader.

Previously we've worked around this by packing the varying in to a vector. Unfortunately, however, this seems to run in to yet another driver bug on this device, and rather than render as coloured triangles they appear as yellow squares. The shader initializes the output value to yellow, but should always then overwrite it with the result of the operation. It appears none of the switch cases (or the if statements glslopt converts them to) are being hit when we use a component of a varying vector as the test condition. Copying it first to a local scalar variable works around this, for both unoptimized switch cases or the glslopt optimized if statements (but in the latter case the optimizer will remove the local variable, so I had to manually edit the optimized output to confirm that.)

With some hacks we should be able to add a local variable and prevent the optimizer from removing it, which solves the problem.

Comment 3

[Jamie Nicol [:jnicol]]

Attachment: Bug 1726755 - Fix brush_mix_blend shader on Adreno 3xx. r?#gfx-reviewers

On some Adreno 3xx devices, the brush_mix_blend shader renders its
output as coloured triangles. This is another instance of the driver
bug from bug 1630356, where the presence of a flat varying
scalar ("v_op" in this case) causes the entire fragment shader output
to be flat, ie a single colour per primitive.

In the past we have worked around this by packing the varying in to a
vector. Unfortunately, however, in brush_mix_blend this causes us to
encounter yet another driver bug. Using the component of the new
vector as the switch condition (or if conditions in the optimized
output) does not work: none of the cases are exectuted and the output
is rendered as the default yellow.

Unpacking the vector's component in to a local variable prior to the
switch avoids this bug, however it gets optimized away by glslopt. To
prevent that, we perform a bitwise AND on the component. This should
have no logical effect and have negligable cost, but importantly
avoids the driver bug meaning the correct branch is taken and the
shader renders correctly.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2714880c85e
Fix brush_mix_blend shader on Adreno 3xx. r=gfx-reviewers,kvark

Comment 5

[Cristian Brindusan [:cbrindusan]]

Backed out changeset a2714880c85e (bug 1726755) for causing wrench bustages.
https://hg.mozilla.org/integration/autoland/rev/2e76dae1503abbb2a863d7b5c7ddc2026ada7b83

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=a2714880c85ebfab7088b6e2107d82a7709d6209&selectedTaskRun=DBgm0-Z3RbmcA54F6m9FUQ.0

Failure log:
https://treeherder.mozilla.org/logviewer?job_id=349682584&repo=autoland&lineNumber=1118

Comment 6

[Jamie Nicol [:jnicol]]

Oh, angle shader validation. I said "switch" in a comment so also need to add a "default".

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bad21e5a3707
Fix brush_mix_blend shader on Adreno 3xx. r=gfx-reviewers,kvark

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/bad21e5a3707

Comment 9

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

The patch landed in nightly and beta is affected.
:jnicol, 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 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9238171 [details]
Bug 1726755 - Fix brush_mix_blend shader on Adreno 3xx. r?#gfx-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Artifacts when sites use mix-blend-mode on Adreno 300 gpus
• 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): A small shader change that shouldn't affect other platforms
• String changes made/needed:

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9238171 [details]
Bug 1726755 - Fix brush_mix_blend shader on Adreno 3xx. r?#gfx-reviewers

Approved for 92.0rc1.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6c3d929f9836