Vulkan: Support struct initializers in shaders

src/compiler/translator/OutputGLSLBase.cpp

@@ -314,11 +314,6 @@ void TOutputGLSLBase::writeVariableType(const TType &type, const TSymbol *symbol
         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 @@ void TOutputGLSLBase::declareStruct(const TStructure *structure)
         out << ";\n";
     }
     out << "}";
+
+    if (structure->symbolType() != SymbolType::Empty)
+    {
+        mDeclaredStructs.insert(structure->uniqueId().get());
+    }
 }
 
 void TOutputGLSLBase::declareInterfaceBlockLayout(const TInterfaceBlock *interfaceBlock)

src/compiler/translator/OutputGLSLBase.h

@@ -78,9 +78,9 @@ class TOutputGLSLBase : public TIntermTraverser
 
     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);

src/compiler/translator/OutputVulkanGLSL.cpp

@@ -106,8 +106,11 @@ void TOutputVulkanGLSL::writeQualifier(TQualifier qualifier, const TSymbol *symb
 
 void TOutputVulkanGLSL::writeStructType(const TStructure *structure)
 {
+    if (!structDeclared(structure))
+    {
         declareStruct(structure);
         objSink() << ";\n";
+    }
 }
 
 }  // namespace sh

src/compiler/translator/TranslatorVulkan.cpp

@@ -41,9 +41,6 @@ class DeclareStructTypesTraverser : public TIntermTraverser
 
         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 @@ class DeclareStructTypesTraverser : public TIntermTraverser
 
         if (type.isStructSpecifier())
         {
-            TIntermSymbol *symbolNode = declarator->getAsSymbolNode();
-            if (symbolNode != nullptr && symbolNode->variable().symbolType() == SymbolType::Empty)
-            {
             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 @@ void TranslatorVulkan::translate(TIntermBlock *root,
         bool hasGLFragColor = false;
         bool hasGLFragData  = false;
 
-        for (const auto &outputVar : outputVariables)
+        for (const OutputVariable &outputVar : outputVariables)
         {
             if (outputVar.name == "gl_FragColor")
             {

src/tests/gl_tests/GLSLTest.cpp

@@ -519,8 +519,9 @@ TEST_P(GLSLTest, ScopedStructsOrderBug)
     // (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_P(GLSLTest, BadIndexBug)
 // 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 @@ TEST_P(GLSLTest, StructSpecifiersUniforms)
     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.