Vulkan: Support struct initializers in shaders
Also adds a new test in GLSLTest to validate the initialization of
a struct on the same line as its declaration.
Bug: angleproject:2459
Change-Id: Ib37e20378f8ec76541db26392663bcba03390756
Reviewed-on: https://chromium-review.googlesource.com/1017340
Commit-Queue: Luc Ferron <lucferron@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/OutputGLSLBase.cpp b/src/compiler/translator/OutputGLSLBase.cpp
index fd1958c..14140eb 100644
--- a/src/compiler/translator/OutputGLSLBase.cpp
+++ b/src/compiler/translator/OutputGLSLBase.cpp
@@ -314,11 +314,6 @@
const TStructure *structure = type.getStruct();
declareStruct(structure);
-
- if (structure->symbolType() != SymbolType::Empty)
- {
- mDeclaredStructs.insert(structure->uniqueId().get());
- }
}
else if (type.getBasicType() == EbtInterfaceBlock)
{
@@ -1180,6 +1175,11 @@
out << ";\n";
}
out << "}";
+
+ if (structure->symbolType() != SymbolType::Empty)
+ {
+ mDeclaredStructs.insert(structure->uniqueId().get());
+ }
}
void TOutputGLSLBase::declareInterfaceBlockLayout(const TInterfaceBlock *interfaceBlock)
diff --git a/src/compiler/translator/OutputGLSLBase.h b/src/compiler/translator/OutputGLSLBase.h
index 272d276..20c2146 100644
--- a/src/compiler/translator/OutputGLSLBase.h
+++ b/src/compiler/translator/OutputGLSLBase.h
@@ -78,9 +78,9 @@
void declareStruct(const TStructure *structure);
virtual void writeQualifier(TQualifier qualifier, const TSymbol *symbol);
+ bool structDeclared(const TStructure *structure) const;
private:
- bool structDeclared(const TStructure *structure) const;
void declareInterfaceBlockLayout(const TInterfaceBlock *interfaceBlock);
void declareInterfaceBlock(const TInterfaceBlock *interfaceBlock);
diff --git a/src/compiler/translator/OutputVulkanGLSL.cpp b/src/compiler/translator/OutputVulkanGLSL.cpp
index c4b44b1..99b0e40 100644
--- a/src/compiler/translator/OutputVulkanGLSL.cpp
+++ b/src/compiler/translator/OutputVulkanGLSL.cpp
@@ -106,8 +106,11 @@
void TOutputVulkanGLSL::writeStructType(const TStructure *structure)
{
- declareStruct(structure);
- objSink() << ";\n";
+ if (!structDeclared(structure))
+ {
+ declareStruct(structure);
+ objSink() << ";\n";
+ }
}
} // namespace sh
diff --git a/src/compiler/translator/TranslatorVulkan.cpp b/src/compiler/translator/TranslatorVulkan.cpp
index f427d56..2dd790e 100644
--- a/src/compiler/translator/TranslatorVulkan.cpp
+++ b/src/compiler/translator/TranslatorVulkan.cpp
@@ -41,9 +41,6 @@
if (!mInGlobalScope)
{
- // We only want to declare the global structs in this traverser.
- // TODO(lucferron): Add a test in GLSLTest for this specific case.
- // http://anglebug.com/2459
return false;
}
@@ -53,22 +50,16 @@
if (type.isStructSpecifier())
{
- TIntermSymbol *symbolNode = declarator->getAsSymbolNode();
- if (symbolNode != nullptr && symbolNode->variable().symbolType() == SymbolType::Empty)
- {
- mOutputVulkanGLSL->writeStructType(type.getStruct());
+ mOutputVulkanGLSL->writeStructType(type.getStruct());
+ TIntermSymbol *symbolNode = declarator->getAsSymbolNode();
+ if (symbolNode && symbolNode->variable().symbolType() == SymbolType::Empty)
+ {
// Remove the struct specifier declaration from the tree so it isn't parsed again.
TIntermSequence emptyReplacement;
mMultiReplacements.emplace_back(getParentNode()->getAsBlock(), node,
emptyReplacement);
}
- else
- {
- // TODO(lucferron): Support structs with initializers correctly.
- // http://anglebug.com/2459
- UNIMPLEMENTED();
- }
}
return false;
@@ -256,7 +247,7 @@
bool hasGLFragColor = false;
bool hasGLFragData = false;
- for (const auto &outputVar : outputVariables)
+ for (const OutputVariable &outputVar : outputVariables)
{
if (outputVar.name == "gl_FragColor")
{
diff --git a/src/tests/gl_tests/GLSLTest.cpp b/src/tests/gl_tests/GLSLTest.cpp
index 349604f..af050ec 100644
--- a/src/tests/gl_tests/GLSLTest.cpp
+++ b/src/tests/gl_tests/GLSLTest.cpp
@@ -519,8 +519,9 @@
// (http://anglebug.com/1291)
ANGLE_SKIP_TEST_IF(IsDesktopOpenGL() && (IsOSX() || !IsNVIDIA()));
- // TODO(lucferron): Support for inner scoped structs
- // http://anglebug.com/2459
+ // TODO(lucferron): Support for inner scoped structs being redeclared in inner scopes
+ // This bug in glslang is preventing us from supporting this use case for now.
+ // https://github.com/KhronosGroup/glslang/issues/1358
ANGLE_SKIP_TEST_IF(IsVulkan());
const std::string fragmentShaderSource =
@@ -1468,14 +1469,10 @@
// Test that structs defined in uniforms are translated correctly.
TEST_P(GLSLTest, StructSpecifiersUniforms)
{
- // TODO(lucferron): Support struct initializers.
- // http://anglebug.com/2459
- ANGLE_SKIP_TEST_IF(IsVulkan());
-
const std::string fragmentShaderSource =
R"(precision mediump float;
- uniform struct S { float field;} s;
+ uniform struct S { float field; } s;
void main()
{
@@ -1487,6 +1484,80 @@
EXPECT_NE(0u, program);
}
+// Test that structs declaration followed directly by an initialization is translated correctly.
+TEST_P(GLSLTest, StructWithInitializer)
+{
+ const std::string fragmentShaderSource =
+ R"(precision mediump float;
+
+ struct S { float a; } s = S(1.0);
+
+ void main()
+ {
+ gl_FragColor = vec4(0, 0, 0, 1);
+ gl_FragColor.r += s.a;
+ })";
+
+ ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), fragmentShaderSource);
+ glUseProgram(program);
+
+ // Test drawing, should be red.
+ drawQuad(program.get(), essl1_shaders::PositionAttrib(), 0.5f);
+
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+ EXPECT_GL_NO_ERROR();
+}
+
+// Test that structs without initializer, followed by a uniform usage works as expected.
+TEST_P(GLSLTest, UniformStructWithoutInitializer)
+{
+ const std::string fragmentShaderSource =
+ R"(precision mediump float;
+
+ struct S { float a; };
+ uniform S u_s;
+
+ void main()
+ {
+ gl_FragColor = vec4(u_s.a);
+ })";
+
+ ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), fragmentShaderSource);
+ glUseProgram(program);
+
+ // Test drawing, should be red.
+ drawQuad(program.get(), essl1_shaders::PositionAttrib(), 0.5f);
+
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::transparentBlack);
+ EXPECT_GL_NO_ERROR();
+}
+
+// Test that structs declaration followed directly by an initialization in a uniform.
+TEST_P(GLSLTest, StructWithUniformInitializer)
+{
+ const std::string fragmentShaderSource =
+ R"(precision mediump float;
+
+ struct S { float a; } s = S(1.0);
+ uniform S us;
+
+ void main()
+ {
+ gl_FragColor = vec4(0, 0, 0, 1);
+ gl_FragColor.r += s.a;
+ gl_FragColor.g += us.a;
+ })";
+
+ ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), fragmentShaderSource);
+ glUseProgram(program);
+
+ // Test drawing, should be red.
+ drawQuad(program.get(), essl1_shaders::PositionAttrib(), 0.5f);
+
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red);
+ EXPECT_GL_NO_ERROR();
+}
+
// Test that gl_DepthRange is not stored as a uniform location. Since uniforms
// beginning with "gl_" are filtered out by our validation logic, we must
// bypass the validation to test the behaviour of the implementation.