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>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index bdfb7f3..b987d18 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -121,8 +121,7 @@
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 @@
++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);
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 17f241f..588488a 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -166,7 +166,6 @@
TExtensionBehavior &ext,
sh::GLenum type,
ShShaderSpec spec,
- ShCompileOptions options,
bool checksPrecErrors,
TDiagnostics *diagnostics,
const ShBuiltInResources &resources)
@@ -174,7 +173,6 @@
mDeferredNonEmptyDeclarationErrorCheck(false),
mShaderType(type),
mShaderSpec(spec),
- mCompileOptions(options),
mShaderVersion(100),
mTreeRoot(nullptr),
mLoopNestingLevel(0),
@@ -2443,32 +2441,6 @@
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);
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index 035e8c7..b82816e 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -37,7 +37,6 @@
TExtensionBehavior &ext,
sh::GLenum type,
ShShaderSpec spec,
- ShCompileOptions options,
bool checksPrecErrors,
TDiagnostics *diagnostics,
const ShBuiltInResources &resources);
@@ -597,7 +596,6 @@
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
diff --git a/src/compiler/translator/SymbolTable.cpp b/src/compiler/translator/SymbolTable.cpp
index 73e40d8..5b8371b 100644
--- a/src/compiler/translator/SymbolTable.cpp
+++ b/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 @@
bool TSymbolTable::isVaryingInvariant(const TVariable &variable) const
{
ASSERT(atGlobalLevel());
- if (mGlobalInvariant)
+ if (mGlobalInvariant && (IsShaderOutput(variable.getType().getQualifier())))
{
return true;
}
diff --git a/src/compiler/translator/util.cpp b/src/compiler/translator/util.cpp
index c118469..da8f990 100644
--- a/src/compiler/translator/util.cpp
+++ b/src/compiler/translator/util.cpp
@@ -707,6 +707,11 @@
return false;
}
+bool IsShaderOutput(TQualifier qualifier)
+{
+ return IsVaryingOut(qualifier) || IsBuiltinOutputVariable(qualifier);
+}
+
bool IsOutputESSL(ShShaderOutput output)
{
return output == SH_ESSL_OUTPUT;
diff --git a/src/compiler/translator/util.h b/src/compiler/translator/util.h
index debac31..d1f4561 100644
--- a/src/compiler/translator/util.h
+++ b/src/compiler/translator/util.h
@@ -59,6 +59,7 @@
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);
diff --git a/src/libANGLE/renderer/gl/ShaderGL.cpp b/src/libANGLE/renderer/gl/ShaderGL.cpp
index 19d40cc..9da9a85 100644
--- a/src/libANGLE/renderer/gl/ShaderGL.cpp
+++ b/src/libANGLE/renderer/gl/ShaderGL.cpp
@@ -282,11 +282,6 @@
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;
diff --git a/src/libANGLE/renderer/gl/renderergl_utils.cpp b/src/libANGLE/renderer/gl/renderergl_utils.cpp
index 698b51e..7752832 100644
--- a/src/libANGLE/renderer/gl/renderergl_utils.cpp
+++ b/src/libANGLE/renderer/gl/renderergl_utils.cpp
@@ -1390,9 +1390,6 @@
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));
diff --git a/src/tests/deqp_support/deqp_gles2_test_expectations.txt b/src/tests/deqp_support/deqp_gles2_test_expectations.txt
index 2617846..14aa909 100644
--- a/src/tests/deqp_support/deqp_gles2_test_expectations.txt
+++ b/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
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index 9f0cf9b..d7fb96e 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -1079,14 +1079,9 @@
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 @@
"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 @@
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 @@
"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.