1734815 - Component transfer filters not rendered correctly on Adreno 3xx

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Jamie Nicol [:jnicol]]

This was spotted whilst investigating bug 1731758, in which no filters rendered with the brush_blend shader were rendered correctly on some Adreno 3xx devices. (ie my Nexus 5: Adreno 330 V@95.0 Android 5.0)

The problem there was the presence of the ivec4 varying vFuncs, which appeared to break the entire shader. Changing it to an ivec2 fixed most of the blend types, but component transfer remained broken. In this bug we'll fix component transfer.

Comment 1

[Jamie Nicol [:jnicol]]

From playing around with the shader, it seems like the vFuncs values are incorrect in the fragment shader. Perhaps an issue with the bit wise operations, like we've seen in bug 1730458. This was previously an ivec4 rather than using bit masks, but that broke the entire shader for some reason. A workaround which appears to work is using a float vec4. Though we do need to convert this to an array prior to looping through the components, like we previously did - presumably a bug to do with indexing vectors.

Additionally, we need to mark the table_address varying and parameters as highp to ensure we can read from any gpu cache address successfully. Strangely the shader doesn't currently work even though the addresses being used are lows value which are representable with mediump. The GPU cache lookup functions (correctly) require a highp argument, so perhaps there is a bug to do with conversion between precisions. We must also mark the k and offset values as highp in order for the shader to work on this platform, which supports that theory.

Comment 2

[Jamie Nicol [:jnicol]]

Attachment: Bug 1734815 - Fix component transfer filters in brush_blend on Adreno 3xx. r?#gfx-reviewers

Due to driver bugs, component transfer filters using brush_blend do
not currently work on some Adreno 3xx devices.

The first issue is that the values for which function to use for each
component (the "v_funcs" varying) appear to be incorrect in the
fragment shader. Previously we passed these values as an int[4], but
due to this requiring to many varying slots we changed this to an
ivec4. This broke the entire shader for all blend operations on this
device (bug 1731758), so we recently changed to bit packing the 4
values in to a single int. This fixed the rest of the blend ops, but
component transfer still didn't work. This patch switches to using a
vec4, casting the values to and from floats, which works correctly.

The second issue appears to be due to converting between integer
precisions. The component transfer functions require a texelFetch from
the GPU cache. The fetch_from_gpu_cache* functions accept a highp int
argument, as GPU cache addresses can exceed those represenatable with
a mediump int. brush_blend is currently using a mediump int (the
default fragment shader precision) for the table_address, which is
therefore incorrect. However, the shader is buggy even when the actual
value is representable with mediump, indicating a driver bug to do
with precision conversion, rather than the value overflowing. To avoid
this we must make the table_address varying and variables highp (as
they should be anyway), but additionally must make the "k" and
"offset" variables (which get added to the table address) highp too.

Depends on D128049

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5aa16ef77492
Fix component transfer filters in brush_blend on Adreno 3xx. r=gfx-reviewers,nical

Comment 4

[Marian-Vasile Laza]

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