Bug 1695912 - Ensure shaders use 16 or fewer varying vectors even if driver does not pack them. r=jrmuizel

On some Adreno 3xx devices we have observed that the driver does not
pack varyings in to vectors as efficiently as the spec mandates. This
results in some of our shaders using a greater number of varying
vectors than GL_MAX_VARYING_VECTORS (which 16 on this device), leading
to shader compilation errors at run time.

Work around this by manually packing our varyings in to fewer
vectors. Additionally, add a test to ensure that we never use more
than 16 vectors even if the driver were to perform no additional
packing.

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

gfx/wr/Cargo.lock

@@ -1105,9 +1105,9 @@ dependencies = [
 
 [[package]]
 name = "mozangle"
-version = "0.3.2"
+version = "0.3.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d77438925edbca894202489c32351b3cce7e0ab25ae7358af83824440ef5e8e2"
+checksum = "ef98797da14500fb5eaabc90dc91cf7c42710c11330351521d72f4aa268d689c"
 dependencies = [
  "cc",
  "gl_generator 0.13.1",

gfx/wr/glsl-to-cxx/src/hir.rs

@@ -1942,6 +1942,32 @@ fn is_vector(ty: &Type) -> bool {
     }
 }
 
+fn index_vector(ty: &Type) -> Option<TypeKind> {
+    use TypeKind::*;
+    if ty.array_sizes != None {
+        return None;
+    }
+    Some(match ty.kind {
+        Vec2 => Float,
+        Vec3 => Float,
+        Vec4 => Float,
+        DVec2 => Double,
+        DVec3 => Double,
+        DVec4 => Double,
+        BVec2 => Bool,
+        BVec3 => Bool,
+        BVec4 => Bool,
+        IVec2 => Int,
+        IVec3 => Int,
+        IVec4 => Int,
+        UVec2 => UInt,
+        UVec3 => UInt,
+        UVec4 => UInt,
+        _ => return None,
+    })
+
+}
+
 fn index_matrix(ty: &Type) -> Option<TypeKind> {
     use TypeKind::*;
     if ty.array_sizes != None {
@@ -2502,8 +2528,8 @@ fn translate_expression(state: &mut State, e: &syntax::Expr) -> Expr {
         }
         syntax::Expr::Bracket(e, specifier) => {
             let e = Box::new(translate_expression(state, e));
-            let ty = if is_vector(&e.ty) {
+            let ty = if let Some(ty) = index_vector(&e.ty) {
-                Type::new(TypeKind::Float)
+                Type::new(ty)
             } else if let Some(ty) = index_matrix(&e.ty) {
                 Type::new(ty)
             } else {

gfx/wr/swgl/src/glsl.h

@@ -1072,6 +1072,21 @@ struct ivec4_scalar {
   friend ivec4_scalar operator&(int32_t a, ivec4_scalar b) {
     return ivec4_scalar{a & b.x, a & b.y, a & b.z, a & b.w};
   }
+
+  int32_t& operator[](int index) {
+    switch (index) {
+      case 0:
+        return x;
+      case 1:
+        return y;
+      case 2:
+        return z;
+      case 3:
+        return w;
+      default:
+        UNREACHABLE;
+    }
+  }
 };
 
 struct ivec4 {

gfx/wr/webrender/Cargo.toml

@@ -62,7 +62,7 @@ etagere = "0.2.4"
 swgl = { path = "../swgl", optional = true }
 
 [dev-dependencies]
-mozangle = "0.3.2"
+mozangle = "0.3.3"
 rand = "0.4"
 
 [target.'cfg(any(target_os = "android", all(unix, not(target_os = "macos"))))'.dependencies]

gfx/wr/webrender/res/brush_blend.glsl

@@ -38,16 +38,14 @@ flat varying vec4 v_uv_sample_bounds;
 flat varying vec4 v_color_offset;
 
 // Flag to allow perspective interpolation of UV.
-flat varying float v_perspective;
+flat varying vec2 v_perspective_amount;
-
-flat varying float v_amount;
 
 flat varying int v_op;
 flat varying int v_table_address;
 
 flat varying mat4 vColorMat;
 
-flat varying int vFuncs[4];
+flat varying ivec4 vFuncs;
 
 #ifdef WR_VERTEX_SHADER
 
@@ -74,7 +72,7 @@ void brush_vs(
     float perspective_interpolate = (brush_flags & BRUSH_FLAG_PERSPECTIVE_INTERPOLATION) != 0 ? 1.0 : 0.0;
 
     v_uv = uv * inv_texture_size * mix(vi.world_pos.w, 1.0, perspective_interpolate);
-    v_perspective = perspective_interpolate;
+    v_perspective_amount.x = perspective_interpolate;
 
     v_uv_sample_bounds = vec4(uv0 + vec2(0.5), uv1 - vec2(0.5)) * inv_texture_size.xyxy;
 
@@ -89,7 +87,7 @@ void brush_vs(
     float invAmount = 1.0 - amount;
 
     v_op = prim_user_data.y & 0xffff;
-    v_amount = amount;
+    v_perspective_amount.y = amount;
 
     // This assignment is only used for component transfer filters but this
     // assignment has to be done here and not in the component transfer case
@@ -241,7 +239,7 @@ vec4 ComponentTransfer(vec4 colora) {
 }
 
 Fragment brush_fs() {
-    float perspective_divisor = mix(gl_FragCoord.w, 1.0, v_perspective);
+    float perspective_divisor = mix(gl_FragCoord.w, 1.0, v_perspective_amount.x);
     vec2 uv = v_uv * perspective_divisor;
     // Clamp the uvs to avoid sampling artifacts.
     uv = clamp(uv, v_uv_sample_bounds.xy, v_uv_sample_bounds.zw);
@@ -254,13 +252,13 @@ Fragment brush_fs() {
 
     switch (v_op) {
         case FILTER_CONTRAST:
-            color = Contrast(color, v_amount);
+            color = Contrast(color, v_perspective_amount.y);
             break;
         case FILTER_INVERT:
-            color = Invert(color, v_amount);
+            color = Invert(color, v_perspective_amount.y);
             break;
         case FILTER_BRIGHTNESS:
-            color = Brightness(color, v_amount);
+            color = Brightness(color, v_perspective_amount.y);
             break;
         case FILTER_SRGB_TO_LINEAR:
             color = SrgbToLinear(color);

gfx/wr/webrender/res/brush_yuv_image.glsl

@@ -15,9 +15,8 @@ flat varying vec4 vUvBounds_U;
 varying vec2 vUv_V;
 flat varying vec4 vUvBounds_V;
 
-flat varying float vCoefficient;
 flat varying mat3 vYuvColorMatrix;
-flat varying vec3 vYuvOffsetVector;
+flat varying vec4 vYuvOffsetVector_Coefficient;
 flat varying int vFormat;
 
 #ifdef SWGL_DRAW_SPAN
@@ -53,10 +52,10 @@ void brush_vs(
     vec2 f = (vi.local_pos - local_rect.p0) / local_rect.size;
 
     YuvPrimitive prim = fetch_yuv_primitive(prim_address);
-    vCoefficient = prim.coefficient;
+    vYuvOffsetVector_Coefficient.w = prim.coefficient;
 
     vYuvColorMatrix = get_yuv_color_matrix(prim.color_space);
-    vYuvOffsetVector = get_yuv_offset_vector(prim.color_space);
+    vYuvOffsetVector_Coefficient.xyz = get_yuv_offset_vector(prim.color_space);
     vFormat = prim.yuv_format;
 
 #ifdef SWGL_DRAW_SPAN
@@ -92,8 +91,8 @@ Fragment brush_fs() {
     vec4 color = sample_yuv(
         vFormat,
         vYuvColorMatrix,
-        vYuvOffsetVector,
+        vYuvOffsetVector_Coefficient.xyz,
-        vCoefficient,
+        vYuvOffsetVector_Coefficient.w,
         vUv_Y,
         vUv_U,
         vUv_V,

gfx/wr/webrender/res/cs_svg_filter.glsl

@@ -17,7 +17,7 @@ flat varying vec4 vFilterData0;
 flat varying vec4 vFilterData1;
 flat varying float vFloat0;
 flat varying mat4 vColorMat;
-flat varying int vFuncs[4];
+flat varying ivec4 vFuncs;
 
 #define FILTER_BLEND                0
 #define FILTER_FLOOD                1

gfx/wr/webrender/tests/angle_shader_validation.rs

@@ -35,8 +35,9 @@ fn validate_shaders() {
                 &|f| webrender::get_unoptimized_shader_source(f, None)
             );
 
-            validate(&vs_validator, shader, vs);
+            let full_shader_name = format!("{} {}", shader, config);
-            validate(&fs_validator, shader, fs);
+            validate(&vs_validator, &full_shader_name, vs);
+            validate(&fs_validator, &full_shader_name, fs);
         }
     }
 }
@@ -49,6 +50,18 @@ fn validate(validator: &ShaderValidator, name: &str, source: String) {
     // Run Angle validator
     match validator.compile_and_translate(&[&source]) {
         Ok(_) => {
+            // Ensure that the shader uses at most 16 varying vectors. This counts the number of
+            // vectors assuming that the driver does not perform additional packing. The spec states
+            // that the driver should pack varyings, however, on some Adreno 3xx devices we have
+            // observed that this is not the case. See bug 1695912.
+            let varying_vectors = validator.get_num_unpacked_varying_vectors();
+            let max_varying_vectors = 16;
+            assert!(
+                varying_vectors <= max_varying_vectors,
+                "Shader {} uses {} varying vectors. Max allowed {}",
+                name, varying_vectors, max_varying_vectors
+            );
+
             println!("Shader translated succesfully: {}", name);
         }
         Err(_) => {