1730458 - Border/outline styles (dotted, dashed etc) render with triangle artifacts [Adreno 3xx]

Status: RESOLVED FIXED

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

Description

[Kestrel]

Attachment: Different border styles showing small triangle artifacts.

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

Border/outline styles render with many small triangle artifacts.

https://www.w3schools.com/css/css_border.asp

Comment 1

[Brad Werth [:bradwerth]]

Jaime, can you investigate?

Comment 2

[Jamie Nicol [:jnicol]]

I think this should have been fixed by bug 1728064.

Could you please try Nightly and see if the issue is still present there?

Comment 3

[Kestrel]

I can still reproduce it on latest Nightly 94.0a1 (Build #2015835241).

Comment 4

[Jamie Nicol [:jnicol]]

Oh, my apologies, I can reproduce on Nightly too. I had previously had software webrender enabled on my nightly installation when I tested.

In that case this seems unrelated to bug 1728064 and 1630356. I will investigate further.

Comment 5

[Jamie Nicol [:jnicol]]

Looking in to this, the problem appears to come from the flat varying ivec4 vConfig. I cannot figure out exactly why, but there appears to sometimes be issues with ivecs - we have seen this in bug 1731758, but it's not as simple as "ivecs are always broken". Perhaps it also does not like how we use bitshifting and masking to pack the 2 edge axis and style values in to a single component each, but again it's not as simple as bitwise operations are always broken.

We can work around this by use float vecs instead. Style and edge axis need will therefore need 2 components each, so we can put them in a vec4. And the segment and clip mode can go in a vec2.

Comment 6

[Jamie Nicol [:jnicol]]

Attachment: Bug 1730458 - Work around driver bugs in cs_border_segment on Adreno 3xx. r?#gfx-reviewers

Due to a driver bug, cs_border_segment renders incorrectly on some
Adreno 3xx devices. The problem lies with the "ivec4 vConfig" varying,
whose values appear to be incorrect in the fragment shader. I have
attempted splitting these all in to separate varyings, removing the
bit packing for style and edge_axis, and using unsigned uvecs, each of
which solved some issues but not all.

The only workaround which appeared to fix all of the problems was
using floating point vecs rather than ivecs, and casting the values to
and from floats. This means that we need two components each for style
and edge_axis (as we cannot use bitwise operations with floats), so
they are packed in a vec4. The segment and clip_mode must therefore be
packed in a separate vec2. This patch implements this workaround.

It certainly seems as if the Adreno driver has serious issues using
integer (or ivec) varyings, however it is not as simple as them never
working correctly. For that reason I have, for now, resisted
converting all integer varyings in all shaders to floats. If we
encounter more similar issues in the future it may be worthwhile doing
so (and adding a test to ensure we do not reintroduce them).

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10c1bbd01bbc
Work around driver bugs in cs_border_segment on Adreno 3xx. r=gfx-reviewers,nical

Comment 8

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/10c1bbd01bbc

Comment 9

[Ryan VanderMeulen [:RyanVM]]

I think we can live with this on 93, but should we consider uplifting to Beta for 94? Please nominate if you agree.

Comment 10

[Jamie Nicol [:jnicol]]

Comment on attachment 9245211 [details]
Bug 1730458 - Work around driver bugs in cs_border_segment on Adreno 3xx. r?#gfx-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Users on Adreno 3xx devices will have complex borders (dotted, dashed, inset, groove, etc) rendered incorrectly
• 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): There are always some risks with shader changes due to driver bugs, but this seems fairly innocuous and no issues have been reported on nightly
• String changes made/needed:

Comment 11

[Dianna Smith [:diannaS]]

Comment on attachment 9245211 [details]
Bug 1730458 - Work around driver bugs in cs_border_segment on Adreno 3xx. r?#gfx-reviewers

Approved for Fenix 94.0b4 & Desktop 94.0b6

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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