Bug 1728064 - Add test to ensure shaders don't contain flat scalar varyings on android. r=gfx-reviewers,kvark

There is a driver bug on Adreno 3xx devices causing incorrect
rendering when flat scalar varyings are used in fragment shaders. This
has occured several times in different shaders, so this patch finally
adds a test to ensure it does not occur again.

We have used the glsl crate to parse and validate the shaders rather
than angle, as exposing the required bindings to mozangle is messy. We
must therefore use the pre-optimized shaders as the glsl crate does
not handle preprocessor directives correctly.

This has been implemented as a wrench test rather than a unit test as
running unit tests on android is difficult. Additionally we want to
use the shaders specific to the platform the tests are ran on, the bug
only affects (some) android devices, and shaders on other platforms
may differ.

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

gfx/wr/Cargo.lock

@@ -1848,6 +1848,7 @@ dependencies = [
  "env_logger",
  "font-loader",
  "gleam 0.13.1",
+ "glsl",
  "glutin",
  "image",
  "log",

gfx/wr/ci-scripts/linux-release-tests.sh

@@ -16,9 +16,10 @@ set -o xtrace
 CARGOFLAGS=${CARGOFLAGS:-""}  # default to empty if not set
 
 pushd wrench
-# Test that all shaders compile successfully.
+# Test that all shaders compile successfully and pass tests.
 python script/headless.py --precache test_init
 python script/headless.py --precache --use-unoptimized-shaders test_init
+python script/headless.py test_shaders
 
 python script/headless.py reftest
 python script/headless.py rawtest

gfx/wr/ci-scripts/macos-release-tests.sh

@@ -20,9 +20,10 @@ WRENCH_BINARY=${WRENCH_BINARY:-""}
 
 pushd wrench
 
-# Test that all shaders compile successfully.
+# Test that all shaders compile successfully and pass tests.
 python script/headless.py --precache test_init
 python script/headless.py --precache --use-unoptimized-shaders test_init
+python script/headless.py test_shaders
 
 python script/headless.py reftest
 python script/headless.py test_invalidation

gfx/wr/ci-scripts/windows-tests.cmd

@@ -21,12 +21,15 @@ popd
 pushd wrench
 cargo test --verbose
 if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%
-:: Test that all shaders compile successfully. --precache compiles all shaders
-:: during initialization, therefore if init is successful then the shaders compile.
+:: Test that all shaders compile successfully and pass tests.
+:: --precache compiles all shaders during initialization, therefore if init
+:: is successful then the shaders compile.
 cargo run --release -- --angle --precache test_init
 if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%
 cargo run --release -- --angle --precache --use-unoptimized-shaders test_init
 if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%
+cargo run --release -- --angle test_shaders
+if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%
 
 cargo run --release -- --angle reftest
 if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%

gfx/wr/webrender/src/lib.rs

@@ -137,7 +137,7 @@ pub mod intern;
 ///
 pub mod render_api;
 
-mod shader_source {
+pub mod shader_source {
     include!(concat!(env!("OUT_DIR"), "/shaders.rs"));
 }
 

gfx/wr/wrench/Cargo.toml

@@ -12,6 +12,7 @@ env_logger = { version = "0.5", optional = true }
 gleam = "0.13"
 glutin = "0.21"
 clap = { version = "2", features = ["yaml"] }
+glsl = "4.0"
 log = "0.4"
 yaml-rust = "0.4"
 serde_json = "1.0"

gfx/wr/wrench/src/args.yaml

@@ -184,3 +184,5 @@ subcommands:
               index: 2
     - test_init:
         about: Test for successful initialization then exit immediately
+    - test_shaders:
+        about: run shader tests

gfx/wr/wrench/src/main.rs

@@ -21,6 +21,7 @@ mod premultiply;
 mod rawtest;
 mod reftest;
 mod test_invalidation;
+mod test_shaders;
 mod wrench;
 mod yaml_frame_reader;
 mod yaml_helper;
@@ -757,6 +758,8 @@ fn main() {
         // Wrench::new() unwraps the Renderer initialization, so if
         // we reach this point then we have initialized successfully.
         println!("Initialization successful");
+    } else if let Some(_) = args.subcommand_matches("test_shaders") {
+        test_shaders::test_shaders();
     } else {
         panic!("Should never have gotten here! {:?}", args);
     };

gfx/wr/wrench/src/test_shaders.rs

@@ -0,0 +1,67 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use webrender::shader_source::OPTIMIZED_SHADERS;
+use glsl::parser::Parse as _;
+use glsl::syntax::{InterpolationQualifier, ShaderStage, SingleDeclaration};
+use glsl::syntax::{TypeSpecifierNonArray, TypeQualifierSpec};
+use glsl::visitor::{Host, Visit, Visitor};
+
+/// Tests that a shader contains no flat scalar varyings.
+/// These must be avoided on Adreno 3xx devices due to bug 1630356.
+fn test_no_flat_scalar_varyings(name: &str, shader: &mut ShaderStage) {
+    struct FlatScalarVaryingsVisitor {
+        shader_name: String,
+    }
+
+    impl Visitor for FlatScalarVaryingsVisitor {
+        fn visit_single_declaration(&mut self, declaration: &mut SingleDeclaration) -> Visit {
+            let is_scalar = match declaration.ty.ty.ty {
+                TypeSpecifierNonArray::Bool
+                | TypeSpecifierNonArray::Int
+                | TypeSpecifierNonArray::UInt
+                | TypeSpecifierNonArray::Float
+                | TypeSpecifierNonArray::Double => true,
+                _ => false,
+            };
+
+            let qualifiers = declaration
+                .ty
+                .qualifier
+                .as_ref()
+                .map(|q| q.qualifiers.0.as_slice())
+                .unwrap_or(&[]);
+
+            let is_flat = qualifiers.contains(&TypeQualifierSpec::Interpolation(
+                InterpolationQualifier::Flat,
+            ));
+
+            assert!(
+                !(is_scalar && is_flat),
+                "{}: {} is a flat scalar varying",
+                self.shader_name,
+                &declaration.name.as_ref().unwrap()
+            );
+
+            Visit::Parent
+        }
+    }
+
+    let mut visitor = FlatScalarVaryingsVisitor {
+        shader_name: name.to_string(),
+    };
+    shader.visit(&mut visitor);
+}
+
+pub fn test_shaders() {
+    for ((_version, name), shader) in OPTIMIZED_SHADERS.iter() {
+        let mut vert = ShaderStage::parse(shader.vert_source).unwrap();
+        let mut frag = ShaderStage::parse(shader.frag_source).unwrap();
+
+        if cfg!(target_os = "android") {
+            test_no_flat_scalar_varyings(&format!("{}.vert", name), &mut vert);
+            test_no_flat_scalar_varyings(&format!("{}.frag", name), &mut frag);
+        }
+    }
+}

testing/mozharness/scripts/android_wrench.py

@@ -29,7 +29,8 @@ from mozharness.mozilla.testing.testbase import TestingMixin
 class TestMode(enum.Enum):
     OPTIMIZED_SHADER_COMPILATION = 0
     UNOPTIMIZED_SHADER_COMPILATION = 1
-    REFTEST = 2
+    SHADER_TEST = 2
+    REFTEST = 3
 
 
 class AndroidWrench(TestingMixin, BaseScript, MozbaseMixin, AndroidMixin):
@@ -171,6 +172,8 @@ class AndroidWrench(TestingMixin, BaseScript, MozbaseMixin, AndroidMixin):
                 argfile.write("--precache test_init")
             elif test_mode == TestMode.UNOPTIMIZED_SHADER_COMPILATION:
                 argfile.write("--precache --use-unoptimized-shaders test_init")
+            elif test_mode == TestMode.SHADER_TEST:
+                argfile.write("--precache test_shaders")
             elif test_mode == TestMode.REFTEST:
                 argfile.write("reftest")
         self.device.push(args_file, "/sdcard/wrench/args")
@@ -272,6 +275,12 @@ class AndroidWrench(TestingMixin, BaseScript, MozbaseMixin, AndroidMixin):
             self.info("Running unoptimized shader compilation tests...")
             self.run_tests(60)
 
+        if not self._errored:
+            self.info("Setting up SD card...")
+            self.setup_sdcard(TestMode.SHADER_TEST)
+            self.info("Running shader tests...")
+            self.run_tests(60)
+
         if not self._errored:
             self.info("Setting up SD card...")
             self.setup_sdcard(TestMode.REFTEST)