Vulkan:Only apply invariant pragma to output vars

The "#pragma STDGL invariant(all)" directive should only be applied to
shader output vars. This change also removes the workaround
SH_DONT_REMOVE_INVARIANT_FOR_FRAGMENT_INPUT which is no longer needed.

This change fixes two tests that were incorrectly assuming that the
pragma would be applied to inputs: GLSLTest.InvariantAll[Both|In].

Bug: angleproject:1293
Bug: angleproject:3285
Change-Id: I4eb03fa89fbc7c560150ee0cc32382024b0cb3e3
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1558678
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tobin Ehlis <tobine@google.com>
Commit-Queue: Tobin Ehlis <tobine@google.com>

src/compiler/translator/Compiler.cpp

@@ -121,8 +121,7 @@ bool RemoveInvariant(sh::GLenum shaderType,
                      ShShaderOutput outputType,
                      ShCompileOptions compileOptions)
 {
-    if ((compileOptions & SH_DONT_REMOVE_INVARIANT_FOR_FRAGMENT_INPUT) == 0 &&
-        shaderType == GL_FRAGMENT_SHADER && IsGLSL420OrNewer(outputType))
+    if (shaderType == GL_FRAGMENT_SHADER && IsGLSL420OrNewer(outputType))
         return true;
 
     if ((compileOptions & SH_REMOVE_INVARIANT_AND_CENTROID_FOR_ESSL3) != 0 &&
@@ -365,8 +364,8 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
         ++firstSource;
     }
 
-    TParseContext parseContext(mSymbolTable, mExtensionBehavior, mShaderType, mShaderSpec,
-                               compileOptions, true, &mDiagnostics, getResources());
+    TParseContext parseContext(mSymbolTable, mExtensionBehavior, mShaderType, mShaderSpec, true,
+                               &mDiagnostics, getResources());
 
     parseContext.setFragmentPrecisionHighOnESSL1(mResources.FragmentPrecisionHigh == 1);
 

src/compiler/translator/ParseContext.cpp

@@ -166,7 +166,6 @@ TParseContext::TParseContext(TSymbolTable &symt,
                              TExtensionBehavior &ext,
                              sh::GLenum type,
                              ShShaderSpec spec,
-                             ShCompileOptions options,
                              bool checksPrecErrors,
                              TDiagnostics *diagnostics,
                              const ShBuiltInResources &resources)
@@ -174,7 +173,6 @@ TParseContext::TParseContext(TSymbolTable &symt,
       mDeferredNonEmptyDeclarationErrorCheck(false),
       mShaderType(type),
       mShaderSpec(spec),
-      mCompileOptions(options),
       mShaderVersion(100),
       mTreeRoot(nullptr),
       mLoopNestingLevel(0),
@@ -2443,32 +2441,6 @@ TIntermDeclaration *TParseContext::parseSingleDeclaration(
     const ImmutableString &identifier)
 {
     TType *type = new TType(publicType);
-    if ((mCompileOptions & SH_FLATTEN_PRAGMA_STDGL_INVARIANT_ALL) &&
-        mDirectiveHandler.pragma().stdgl.invariantAll)
-    {
-        TQualifier qualifier = type->getQualifier();
-
-        // The directive handler has already taken care of rejecting invalid uses of this pragma
-        // (for example, in ESSL 3.00 fragment shaders), so at this point, flatten it into all
-        // affected variable declarations:
-        //
-        // 1. Built-in special variables which are inputs to the fragment shader. (These are handled
-        // elsewhere, in TranslatorGLSL.)
-        //
-        // 2. Outputs from vertex shaders in ESSL 1.00 and 3.00 (EvqVaryingOut and EvqVertexOut). It
-        // is actually less likely that there will be bugs in the handling of ESSL 3.00 shaders, but
-        // the way this is currently implemented we have to enable this compiler option before
-        // parsing the shader and determining the shading language version it uses. If this were
-        // implemented as a post-pass, the workaround could be more targeted.
-        //
-        // 3. Inputs in ESSL 1.00 fragment shaders (EvqVaryingIn). This is somewhat in violation of
-        // the specification, but there are desktop OpenGL drivers that expect that this is the
-        // behavior of the #pragma when specified in ESSL 1.00 fragment shaders.
-        if (qualifier == EvqVaryingOut || qualifier == EvqVertexOut || qualifier == EvqVaryingIn)
-        {
-            type->setInvariant(true);
-        }
-    }
 
     checkGeometryShaderInputAndSetArraySize(identifierOrTypeLocation, identifier, type);
 

src/compiler/translator/ParseContext.h

@@ -37,7 +37,6 @@ class TParseContext : angle::NonCopyable
                   TExtensionBehavior &ext,
                   sh::GLenum type,
                   ShShaderSpec spec,
-                  ShCompileOptions options,
                   bool checksPrecErrors,
                   TDiagnostics *diagnostics,
                   const ShBuiltInResources &resources);
@@ -597,7 +596,6 @@ class TParseContext : angle::NonCopyable
 
     sh::GLenum mShaderType;    // vertex or fragment language (future: pack or unpack)
     ShShaderSpec mShaderSpec;  // The language specification compiler conforms to - GLES2 or WebGL.
-    ShCompileOptions mCompileOptions;  // Options passed to TCompiler
     int mShaderVersion;
     TIntermBlock *mTreeRoot;  // root of parse tree being created
     int mLoopNestingLevel;    // 0 if outside all loops

src/compiler/translator/SymbolTable.cpp

@@ -17,6 +17,7 @@
 #include "compiler/translator/ImmutableString.h"
 #include "compiler/translator/IntermNode.h"
 #include "compiler/translator/StaticType.h"
+#include "compiler/translator/util.h"
 
 namespace sh
 {
@@ -196,7 +197,7 @@ void TSymbolTable::addInvariantVarying(const TVariable &variable)
 bool TSymbolTable::isVaryingInvariant(const TVariable &variable) const
 {
     ASSERT(atGlobalLevel());
-    if (mGlobalInvariant)
+    if (mGlobalInvariant && (IsShaderOutput(variable.getType().getQualifier())))
     {
         return true;
     }

src/compiler/translator/util.cpp

@@ -707,6 +707,11 @@ bool IsBuiltinFragmentInputVariable(TQualifier qualifier)
     return false;
 }
 
+bool IsShaderOutput(TQualifier qualifier)
+{
+    return IsVaryingOut(qualifier) || IsBuiltinOutputVariable(qualifier);
+}
+
 bool IsOutputESSL(ShShaderOutput output)
 {
     return output == SH_ESSL_OUTPUT;

src/compiler/translator/util.h

@@ -59,6 +59,7 @@ bool IsBuiltinOutputVariable(TQualifier qualifier);
 bool IsBuiltinFragmentInputVariable(TQualifier qualifier);
 bool CanBeInvariantESSL1(TQualifier qualifier);
 bool CanBeInvariantESSL3OrGreater(TQualifier qualifier);
+bool IsShaderOutput(TQualifier qualifier);
 bool IsOutputESSL(ShShaderOutput output);
 bool IsOutputGLSL(ShShaderOutput output);
 bool IsOutputHLSL(ShShaderOutput output);

src/libANGLE/renderer/gl/ShaderGL.cpp

@@ -282,11 +282,6 @@ std::shared_ptr<WaitableCompileEvent> ShaderGL::compile(const gl::Context *conte
         additionalOptions |= SH_USE_UNUSED_STANDARD_SHARED_BLOCKS;
     }
 
-    if (workarounds.dontRemoveInvariantForFragmentInput)
-    {
-        additionalOptions |= SH_DONT_REMOVE_INVARIANT_FOR_FRAGMENT_INPUT;
-    }
-
     if (workarounds.removeInvariantAndCentroidForESSL3)
     {
         additionalOptions |= SH_REMOVE_INVARIANT_AND_CENTROID_FOR_ESSL3;

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -1390,9 +1390,6 @@ void GenerateWorkarounds(const FunctionsGL *functions, WorkaroundsGL *workaround
     VendorID vendor = GetVendorID(functions);
     uint32_t device = GetDeviceID(functions);
 
-    workarounds->dontRemoveInvariantForFragmentInput =
-        functions->standard == STANDARD_GL_DESKTOP && IsAMD(vendor);
-
     // Don't use 1-bit alpha formats on desktop GL with AMD or Intel drivers.
     workarounds->avoid1BitAlphaTextureFormats =
         functions->standard == STANDARD_GL_DESKTOP && (IsAMD(vendor));

src/tests/deqp_support/deqp_gles2_test_expectations.txt

@@ -82,7 +82,8 @@
 3283 : dEQP-GLES2.functional.fbo.completeness.renderable.texture.color0.rgba_half_float_oes = FAIL
 
 // Shader failures.
-3285 : dEQP-GLES2.functional.shaders.preprocessor.pragmas.pragma_fragment = FAIL
+3285 NEXUS5X GLES : dEQP-GLES2.functional.shaders.preprocessor.pragmas.pragma_fragment = FAIL
+3285 NEXUS5X GLES : dEQP-GLES2.functional.shaders.preprocessor.pragmas.pragma_vertex = FAIL
 3287 : dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_vertex = FAIL
 3287 : dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment = FAIL
 

src/tests/gl_tests/GLSLTest.cpp

@@ -1079,14 +1079,9 @@ TEST_P(GLSLTest_ES3, InvariantGLPosition)
     EXPECT_NE(0u, program);
 }
 
-// Verify that using invariant(all) in both shaders succeeds in ESSL 1.00.
+// Verify that using invariant(all) in both shaders fails in ESSL 1.00.
 TEST_P(GLSLTest, InvariantAllBoth)
 {
-    // TODO: ESSL 1.00 -> GLSL 1.20 translation should add "invariant" in fragment shader
-    // for varyings which are invariant in vertex shader individually,
-    // and remove invariant(all) from fragment shader (http://anglebug.com/1293)
-    ANGLE_SKIP_TEST_IF(IsDesktopOpenGL());
-
     constexpr char kFS[] =
         "#pragma STDGL invariant(all)\n"
         "precision mediump float;\n"
@@ -1100,7 +1095,7 @@ TEST_P(GLSLTest, InvariantAllBoth)
         "void main() { v_varying = a_position.x; gl_Position = a_position; }\n";
 
     GLuint program = CompileProgram(kVS, kFS);
-    EXPECT_NE(0u, program);
+    EXPECT_EQ(0u, program);
 }
 
 // Verify that functions without return statements still compile
@@ -1259,7 +1254,7 @@ TEST_P(GLSLTest_ES3, InvariantAllBoth)
     EXPECT_EQ(0u, program);
 }
 
-// Verify that using invariant(all) only in fragment shader fails in ESSL 1.00.
+// Verify that using invariant(all) only in fragment shader succeeds in ESSL 1.00.
 TEST_P(GLSLTest, InvariantAllIn)
 {
     constexpr char kFS[] =
@@ -1274,7 +1269,7 @@ TEST_P(GLSLTest, InvariantAllIn)
         "void main() { v_varying = a_position.x; gl_Position = a_position; }\n";
 
     GLuint program = CompileProgram(kVS, kFS);
-    EXPECT_EQ(0u, program);
+    EXPECT_NE(0u, program);
 }
 
 // Verify that using invariant(all) only in fragment shader fails in ESSL 3.00.