Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit bb52c52

Browse files
Olli EtuahoCommit Bot
authored andcommitted
Invariant declaration doesn't make a variable active
Invariant declarations didn't affect static use before, but now they are also skipped in CollectVariables so an invariant declaration is not enough in itself to mark a variable as active. This fixes an assert in CollectVariables. BUG=chromium:829553 TEST=angle_unittests Change-Id: I3e51d2916f091bcc283af136a4abc846ff71447d Reviewed-on: https://chromium-review.googlesource.com/999532 Reviewed-by: Corentin Wallez <[email protected]> Reviewed-by: Jamie Madill <[email protected]> Commit-Queue: Olli Etuaho <[email protected]>
1 parent 6c59e4a commit bb52c52

File tree

2 files changed

+35
-6
lines changed

2 files changed

+35
-6
lines changed

src/compiler/translator/CollectVariables.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ class CollectVariablesTraverser : public TIntermTraverser
119119
GLenum shaderType,
120120
const TExtensionBehavior &extensionBehavior);
121121

122+
bool visitInvariantDeclaration(Visit visit, TIntermInvariantDeclaration *node) override;
122123
void visitSymbol(TIntermSymbol *symbol) override;
123124
bool visitDeclaration(Visit, TIntermDeclaration *node) override;
124125
bool visitBinary(Visit visit, TIntermBinary *binaryNode) override;
@@ -336,11 +337,17 @@ InterfaceBlock *CollectVariablesTraverser::recordGLInUsed(const TType &glInType)
336337
}
337338
}
338339

339-
// We want to check whether a uniform/varying is statically used
340-
// because we only count the used ones in packing computing.
341-
// Also, gl_FragCoord, gl_PointCoord, and gl_FrontFacing count
342-
// toward varying counting if they are statically used in a fragment
343-
// shader.
340+
bool CollectVariablesTraverser::visitInvariantDeclaration(Visit visit,
341+
TIntermInvariantDeclaration *node)
342+
{
343+
// We should not mark variables as active just based on an invariant declaration, so we don't
344+
// traverse the symbols declared invariant.
345+
return false;
346+
}
347+
348+
// We want to check whether a uniform/varying is active because we need to skip updating inactive
349+
// ones. We also only count the active ones in packing computing. Also, gl_FragCoord, gl_PointCoord,
350+
// and gl_FrontFacing count toward varying counting if they are active in a fragment shader.
344351
void CollectVariablesTraverser::visitSymbol(TIntermSymbol *symbol)
345352
{
346353
ASSERT(symbol != nullptr);

src/tests/compiler_tests/CollectVariables_test.cpp

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2035,4 +2035,26 @@ TEST_F(CollectVertexVariablesTest, StaticallyUsedButNotActiveInstancedInterfaceB
20352035
// See TODO in CollectVariables.cpp about tracking instanced interface block field static use.
20362036
// EXPECT_TRUE(field.staticUse);
20372037
EXPECT_FALSE(field.active);
2038-
}
2038+
}
2039+
2040+
// Test a varying that is declared invariant but not otherwise used.
2041+
TEST_F(CollectVertexVariablesTest, VaryingOnlyDeclaredInvariant)
2042+
{
2043+
const std::string &shaderString =
2044+
R"(precision mediump float;
2045+
varying float vf;
2046+
invariant vf;
2047+
void main()
2048+
{
2049+
})";
2050+
2051+
compile(shaderString);
2052+
2053+
const auto &varyings = mTranslator->getOutputVaryings();
2054+
ASSERT_EQ(1u, varyings.size());
2055+
2056+
const Varying &varying = varyings[0];
2057+
EXPECT_EQ("vf", varying.name);
2058+
EXPECT_FALSE(varying.staticUse);
2059+
EXPECT_FALSE(varying.active);
2060+
}

0 commit comments

Comments
 (0)