Bug 1734815 - Fix component transfer filters in brush_blend on Adreno 3xx. r=gfx-reviewers,nical

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

Differential Revision: https://phabricator.services.mozilla.com/D128050

gfx/wr/webrender/res/blend.glsl

@@ -30,7 +30,7 @@ void SetupFilterParams(
     int gpu_data_address,
     out vec4 color_offset,
     out mat4 color_mat,
-    out int table_address
+    out highp int table_address
 ) {
     float lumR = 0.2126;
     float lumG = 0.7152;
@@ -123,7 +123,7 @@ vec3 LinearToSrgb(vec3 color) {
 // This function has to be factored out due to the following issue:
 // https://github.com/servo/webrender/wiki/Driver-issues#bug-1532245---switch-statement-inside-control-flow-inside-switch-statement-fails-to-compile-on-some-android-phones
 // (and now the words "default: default:" so angle_shader_validation.rs passes)
-vec4 ComponentTransfer(vec4 colora, int vfuncs, int table_address) {
+vec4 ComponentTransfer(vec4 colora, vec4 vfuncs, highp int table_address) {
     // We push a different amount of data to the gpu cache depending on the
     // function type.
     // Identity => 0 blocks
@@ -135,16 +135,19 @@ vec4 ComponentTransfer(vec4 colora, int vfuncs, int table_address) {
     // function type put into the gpu cache.
     // Table/Discrete use a 256 entry look up table.
     // Linear/Gamma are a simple calculation.
-    int offset = 0;
+
+    // Both offset and k must be marked as highp due to a Adreno 3xx bug likely
+    // to do with converting between precisions (as they would otherwise be
+    // promoted when adding to table_address).
+    highp int offset = 0;
+    highp int k;
+
     vec4 texel;
-    int k;
 
+    // Dynamically indexing a vector is buggy on some platforms, so use a temporary array
+    int[4] funcs = int[4](int(vfuncs.r), int(vfuncs.g), int(vfuncs.b), int(vfuncs.a));
     for (int i = 0; i < 4; i++) {
-        // Each function value is packed in to 4 bits, with the "r" function at bits 15-12
-        // and the "a" function at bits 3-0.
-        int func = (vfuncs >> (12 - (i * 4))) & 0xf;
-
-        switch (func) {
+        switch (funcs[i]) {
             case COMPONENT_TRANSFER_IDENTITY:
                 break;
             case COMPONENT_TRANSFER_TABLE:
@@ -185,10 +188,10 @@ void CalculateFilter(
     vec4 Cs,
     int op,
     float amount,
-    int table_address,
+    highp int table_address,
     vec4 color_offset,
     mat4 color_mat,
-    int v_funcs,
+    vec4 v_funcs,
     out vec3 color,
     out float alpha
 ) {

gfx/wr/webrender/res/brush_blend.glsl

@@ -23,17 +23,18 @@ flat varying vec2 v_perspective_amount;
 
 // x: Blend op, y: Lookup table GPU cache address.
 // Packed in to a vector to work around bug 1630356.
-flat varying ivec2 v_op_table_address_vec;
+// Must be explicitly marked as highp, as the default integer precision in
+// fragment shaders is mediump which may only be 16 bits in ESSL 3, and GPU
+// cache address can exceed that maximum representable value.
+flat varying highp ivec2 v_op_table_address_vec;
 #define v_op v_op_table_address_vec.x
 #define v_table_address v_op_table_address_vec.y
 
 flat varying mat4 v_color_mat;
 // The function to use for each component of a component transfer filter. Using a int[4]
-// or vec4 (with each element or component containing the function for each component) has
-// ran in to bugs 1695912 and 1731758, so instead we pack each value in to 4 bits of an
-// integer. However, due to bug 1630356 we cannot simply use an int, so instead use the
-// x component of an ivec2.
-flat varying ivec2 v_funcs;
+// or ivec4 (with each element or component containing the function for each component) has
+// ran in to bugs 1695912 and 1731758, so instead use a vec4 and cast the values to/from floats.
+flat varying vec4 v_funcs;
 flat varying vec4 v_color_offset;
 
 #ifdef WR_VERTEX_SHADER
@@ -67,9 +68,13 @@ void brush_vs(
     float amount = float(prim_user_data.z) / 65536.0;
 
     v_op = prim_user_data.y & 0xffff;
-    v_funcs.x = (prim_user_data.y >> 16) & 0xffff;
     v_amount = amount;
 
+    v_funcs.r = float((prim_user_data.y >> 28) & 0xf);
+    v_funcs.g = float((prim_user_data.y >> 24) & 0xf);
+    v_funcs.b = float((prim_user_data.y >> 20) & 0xf);
+    v_funcs.a = float((prim_user_data.y >> 16) & 0xf);
+
     SetupFilterParams(
         v_op,
         amount,
@@ -99,7 +104,7 @@ Fragment brush_fs() {
         v_table_address,
         v_color_offset,
         v_color_mat,
-        v_funcs.x,
+        v_funcs,
         color,
         alpha
     );