Fix tracking variables in folded ternary operators
The result of folding a ternary operator may be a TIntermSymbol node
where the qualifier doesn't match the qualifier of the variable that
the node is referring to.
Get the qualifier from the variable instead of directly from
TIntermSymbol when collecting variables in CollectVariables or when
tracking referenced variables in OutputHLSL.
BUG=angleproject:2288
TEST=angle_unittests, angle_end2end_tests
Change-Id: If294a7fe9dca50f2ebcea3feff887e72a521d395
Reviewed-on: https://chromium-review.googlesource.com/836893
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/CollectVariables.cpp b/src/compiler/translator/CollectVariables.cpp
index f97dd3d..7a8750b 100644
--- a/src/compiler/translator/CollectVariables.cpp
+++ b/src/compiler/translator/CollectVariables.cpp
@@ -349,11 +349,15 @@
ShaderVariable *var = nullptr;
const TString &symbolName = symbol->getName().getString();
- if (IsVaryingIn(symbol->getQualifier()))
+ // Check the qualifier from the variable, not from the symbol node. The node may have a
+ // different qualifier if it's the result of a folded ternary node.
+ TQualifier qualifier = symbol->variable().getType().getQualifier();
+
+ if (IsVaryingIn(qualifier))
{
var = FindVariable(symbolName, mInputVaryings);
}
- else if (IsVaryingOut(symbol->getQualifier()))
+ else if (IsVaryingOut(qualifier))
{
var = FindVariable(symbolName, mOutputVaryings);
}
@@ -363,7 +367,7 @@
}
else if (symbolName == "gl_DepthRange")
{
- ASSERT(symbol->getQualifier() == EvqUniform);
+ ASSERT(qualifier == EvqUniform);
if (!mDepthRangeAdded)
{
@@ -406,7 +410,7 @@
}
else
{
- switch (symbol->getQualifier())
+ switch (qualifier)
{
case EvqAttribute:
case EvqVertexIn:
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 31e71b9..290476e 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -251,9 +251,12 @@
TIntermTyped *mExpression; // non-zero except for "return exp;" statements
};
-//
-// Nodes that correspond to symbols or constants in the source code.
-//
+// Nodes that correspond to variable symbols in the source code. These may be regular variables or
+// interface block instances. In declarations that only declare a struct type but no variables, a
+// TIntermSymbol node with an empty variable is used to store the type. In case the node is the
+// result of folding a more complex expression such as a ternary operator, the node takes on the
+// type of the expression. In this case the qualifier of the node may be different from the variable
+// it refers to.
class TIntermSymbol : public TIntermTyped
{
public:
diff --git a/src/compiler/translator/OutputHLSL.cpp b/src/compiler/translator/OutputHLSL.cpp
index 65270ff..9df5186 100644
--- a/src/compiler/translator/OutputHLSL.cpp
+++ b/src/compiler/translator/OutputHLSL.cpp
@@ -403,11 +403,10 @@
TString attributes;
TString mappedStructs = generateStructMapping(std140Structs);
- for (ReferencedSymbols::const_iterator varying = mReferencedVaryings.begin();
- varying != mReferencedVaryings.end(); varying++)
+ for (const auto &varying : mReferencedVaryings)
{
- const TType &type = varying->second->getType();
- const TString &name = varying->second->getSymbol();
+ const TType &type = varying.second->variable().getType();
+ const TString &name = varying.second->getSymbol();
// Program linking depends on this exact format
varyings += "static " + InterpolationString(type.getQualifier()) + " " + TypeString(type) +
@@ -880,7 +879,7 @@
else
{
const TType &nodeType = node->getType();
- TQualifier qualifier = node->getQualifier();
+ TQualifier qualifier = node->variable().getType().getQualifier();
ensureStructDefined(nodeType);
diff --git a/src/tests/compiler_tests/CollectVariables_test.cpp b/src/tests/compiler_tests/CollectVariables_test.cpp
index 25f2d18..d1d252f 100644
--- a/src/tests/compiler_tests/CollectVariables_test.cpp
+++ b/src/tests/compiler_tests/CollectVariables_test.cpp
@@ -1523,3 +1523,33 @@
EXPECT_EQ("texcoord", varying->name);
EXPECT_TRUE(varying->isInvariant);
}
+
+// Test collecting a varying variable that is used inside a folded ternary operator. The result of
+// the folded ternary operator has a different qualifier from the original variable, which makes
+// this case tricky.
+TEST_F(CollectFragmentVariablesTest, VaryingUsedInsideFoldedTernary)
+{
+ const std::string &shaderString =
+ R"(#version 300 es
+ precision highp float;
+ centroid in float vary;
+ out vec4 color;
+ void main() {
+ color = vec4(0.0, true ? vary : 0.0, 0.0, 1.0);
+ })";
+
+ compile(shaderString);
+
+ const std::vector<Varying> &varyings = mTranslator->getInputVaryings();
+ ASSERT_EQ(1u, varyings.size());
+
+ const Varying *varying = &varyings[0];
+
+ EXPECT_FALSE(varying->isArray());
+ EXPECT_GLENUM_EQ(GL_HIGH_FLOAT, varying->precision);
+ EXPECT_TRUE(varying->staticUse);
+ EXPECT_GLENUM_EQ(GL_FLOAT, varying->type);
+ EXPECT_EQ("vary", varying->name);
+ EXPECT_EQ(DecorateName("vary"), varying->mappedName);
+ EXPECT_EQ(INTERPOLATION_CENTROID, varying->interpolation);
+}
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index d0c1b92..2cacd3e 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -3881,6 +3881,41 @@
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
}
+// Test that a varying with a flat qualifier that is used as an operand of a folded ternary operator
+// is handled correctly.
+TEST_P(GLSLTest_ES3, FlatVaryingUsedInFoldedTernary)
+{
+ const std::string &vertexShader =
+ R"(#version 300 es
+
+ in vec4 inputAttribute;
+
+ flat out int v;
+
+ void main()
+ {
+ v = 1;
+ gl_Position = inputAttribute;
+ })";
+
+ const std::string &fragmentShader =
+ R"(#version 300 es
+
+ precision highp float;
+ out vec4 my_FragColor;
+
+ flat in int v;
+
+ void main()
+ {
+ my_FragColor = vec4(0, (true ? v : 0), 0, 1);
+ })";
+
+ ANGLE_GL_PROGRAM(program, vertexShader, fragmentShader);
+ drawQuad(program.get(), "inputAttribute", 0.5f);
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
// Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
ANGLE_INSTANTIATE_TEST(GLSLTest,
ES2_D3D9(),