Closed Bug 1726755 Opened 2 months ago Closed 2 months ago

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

Categories

(Core :: Graphics: WebRender, defect)

Firefox 93
defect

Tracking

()

RESOLVED FIXED
93 Branch
Tracking Status
firefox91 --- wontfix
firefox92 --- fixed
firefox93 --- fixed

People

(Reporter: ke5trel, Assigned: jnicol)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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

Jamie, any idea what could be happening here?

Flags: needinfo?(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.

Flags: needinfo?(jnicol)

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.

Assignee: nobody → jnicol
Status: NEW → ASSIGNED
Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a2714880c85e
Fix brush_mix_blend shader on Adreno 3xx. r=gfx-reviewers,kvark

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

Flags: needinfo?(jnicol)
Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bad21e5a3707
Fix brush_mix_blend shader on Adreno 3xx. r=gfx-reviewers,kvark
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch

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.

Flags: needinfo?(jnicol)

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:
Attachment #9238171 - Flags: approval-mozilla-beta?

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

Approved for 92.0rc1.

Flags: needinfo?(jnicol)
Attachment #9238171 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1630356
You need to log in before you can comment on or make changes to this bug.