GL: Add explicitFragmentLocations workaround

Some drivers produce incorrect results when
a fragment output has an implicit location
and gl_SampleMask[] is written to.

Fixed: angleproject:8308
Change-Id: I615952ef61b1cb611984ec7defb189d89ab3281c
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4777702
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Alexey Knyazev <lexa.knyazev@gmail.com>

include/GLSLANG/ShaderLang.h

@@ -26,7 +26,7 @@
 
 // Version number for shader translation API.
 // It is incremented every time the API changes.
-#define ANGLE_SH_VERSION 337
+#define ANGLE_SH_VERSION 338
 
 enum ShShaderSpec
 {
@@ -396,9 +396,8 @@ struct ShCompileOptions
     // if gl_FragColor is not written.
     uint64_t initFragmentOutputVariables : 1;
 
-    // Unused.  Kept to avoid unnecessarily changing the layout of this structure and tripping up
-    // the fuzzer's hash->bug map.
-    uint64_t unused : 1;
+    // Always write explicit location layout qualifiers for fragment outputs.
+    uint64_t explicitFragmentLocations : 1;
 
     // Insert explicit casts for float/double/unsigned/signed int on macOS 10.15 with Intel driver
     uint64_t addExplicitBoolCasts : 1;

include/platform/autogen/FeaturesGL_autogen.h

@@ -682,6 +682,13 @@ struct FeaturesGL : FeatureSetBase
         &members, "http://crbug.com/1456243"
     };
 
+    FeatureInfo explicitFragmentLocations = {
+        "explicitFragmentLocations",
+        FeatureCategory::OpenGLWorkarounds,
+        "Always write explicit location layout qualifiers for fragment outputs.",
+        &members, "https://anglebug.com/8308"
+    };
+
 };
 
 inline FeaturesGL::FeaturesGL()  = default;

include/platform/gl_features.json

@@ -739,6 +739,14 @@
                 "Apple OpenGL drivers crash when drawing with a zero-sized buffer bound using a non-zero divisor."
             ],
             "issue": "http://crbug.com/1456243"
+        },
+        {
+            "name": "explicit_fragment_locations",
+            "category": "Workarounds",
+            "description": [
+                "Always write explicit location layout qualifiers for fragment outputs."
+            ],
+            "issue": "https://anglebug.com/8308"
         }
     ]
 }

src/compiler/translator/glsl/OutputGLSLBase.cpp

@@ -96,9 +96,10 @@ TOutputGLSLBase::TOutputGLSLBase(TCompiler *compiler,
       // If pixel local storage introduces new fragment outputs, we are now required to specify a
       // location for _all_ fragment outputs, including previously valid outputs that had an
       // implicit location of zero.
-      mAlwaysSpecifyFragOutLocation(compiler->hasPixelLocalStorageUniforms() &&
-                                    compileOptions.pls.type ==
-                                        ShPixelLocalStorageType::FramebufferFetch),
+      mAlwaysSpecifyFragOutLocation(
+          compileOptions.explicitFragmentLocations ||
+          (compiler->hasPixelLocalStorageUniforms() &&
+           compileOptions.pls.type == ShPixelLocalStorageType::FramebufferFetch)),
       mCompileOptions(compileOptions)
 {}
 
@@ -277,7 +278,8 @@ void TOutputGLSLBase::writeLayoutQualifier(TIntermSymbol *variable)
         IsVarying(type.getQualifier()))
     {
         if (layoutQualifier.location >= 0 ||
-            (mAlwaysSpecifyFragOutLocation && IsFragmentOutput(type.getQualifier())))
+            (mAlwaysSpecifyFragOutLocation && IsFragmentOutput(type.getQualifier()) &&
+             !layoutQualifier.yuv))
         {
             out << listItemPrefix << "location = " << std::max(layoutQualifier.location, 0);
         }
@@ -1466,7 +1468,8 @@ bool TOutputGLSLBase::needsToWriteLayoutQualifier(const TType &type)
         IsVarying(type.getQualifier()))
     {
         if (layoutQualifier.location >= 0 ||
-            (mAlwaysSpecifyFragOutLocation && IsFragmentOutput(type.getQualifier())))
+            (mAlwaysSpecifyFragOutLocation && IsFragmentOutput(type.getQualifier()) &&
+             !layoutQualifier.yuv))
         {
             return true;
         }

src/libANGLE/renderer/gl/ShaderGL.cpp

@@ -386,6 +386,11 @@ std::shared_ptr<WaitableCompileEvent> ShaderGL::compile(const gl::Context *conte
         options->scalarizeVecAndMatConstructorArgs = true;
     }
 
+    if (features.explicitFragmentLocations.enabled)
+    {
+        options->explicitFragmentLocations = true;
+    }
+
     if (mRenderer->getNativeExtensions().shaderPixelLocalStorageANGLE)
     {
         options->pls = mRenderer->getNativePixelLocalStorageOptions();

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -2560,6 +2560,9 @@ void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *feature
     // https://anglebug.com/7880
     ANGLE_FEATURE_CONDITION(features, emulateClipDistanceState, isQualcomm);
 
+    // https://anglebug.com/8308
+    ANGLE_FEATURE_CONDITION(features, explicitFragmentLocations, isQualcomm);
+
     // Desktop GLSL-only fragment synchronization extensions. These are injected internally by the
     // compiler to make pixel local storage coherent.
     // https://anglebug.com/7279

src/tests/gl_tests/SampleVariablesTest.cpp

@@ -464,8 +464,7 @@ TEST_P(SampleVariablesTest, SampleMask)
 precision highp float;
 uniform highp int sampleMask;
 
-// Fails on Adreno without explicit location
-layout(location = 0) out vec4 color;
+out vec4 color;
 
 void main()
 {

util/autogen/angle_features_autogen.cpp

@@ -139,6 +139,7 @@ constexpr PackedEnumMap<Feature, const char *> kFeatureNames = {{
     {Feature::EnableTranslatedShaderSubstitution, "enableTranslatedShaderSubstitution"},
     {Feature::EnsureNonEmptyBufferIsBoundForDraw, "ensureNonEmptyBufferIsBoundForDraw"},
     {Feature::ExpandIntegerPowExpressions, "expandIntegerPowExpressions"},
+    {Feature::ExplicitFragmentLocations, "explicitFragmentLocations"},
     {Feature::ExplicitlyCastMediumpFloatTo16Bit, "explicitlyCastMediumpFloatTo16Bit"},
     {Feature::ExplicitlyEnablePerSampleShading, "explicitlyEnablePerSampleShading"},
     {Feature::ExposeNonConformantExtensionsAndVersions, "exposeNonConformantExtensionsAndVersions"},

util/autogen/angle_features_autogen.h

@@ -139,6 +139,7 @@ enum class Feature
     EnableTranslatedShaderSubstitution,
     EnsureNonEmptyBufferIsBoundForDraw,
     ExpandIntegerPowExpressions,
+    ExplicitFragmentLocations,
     ExplicitlyCastMediumpFloatTo16Bit,
     ExplicitlyEnablePerSampleShading,
     ExposeNonConformantExtensionsAndVersions,