Clean up HLSL constructor output

src/compiler/translator/InitializeVariables.cpp

@@ -38,7 +38,7 @@ void AddStructZeroInitSequence(const TIntermTyped *initializedNode,
                                TIntermSequence *initSequenceOut)
 {
     ASSERT(initializedNode->getBasicType() == EbtStruct);
-    TStructure *structType = initializedNode->getType().getStruct();
+    const TStructure *structType = initializedNode->getType().getStruct();
     for (int i = 0; i < static_cast<int>(structType->fields().size()); ++i)
     {
         TIntermBinary *element = new TIntermBinary(EOpIndexDirectStruct,

src/compiler/translator/IntermNode_util.cpp

@@ -140,7 +140,7 @@ TIntermTyped *CreateZeroNode(const TType &type)
     {
         ASSERT(type.getBasicType() == EbtStruct);
 
-        TStructure *structure = type.getStruct();
+        const TStructure *structure = type.getStruct();
         for (const auto &field : structure->fields())
         {
             arguments->push_back(CreateZeroNode(*field->type()));

src/compiler/translator/OutputGLSLBase.cpp

@@ -306,7 +306,7 @@ void TOutputGLSLBase::writeVariableType(const TType &type)
     // Declare the struct if we have not done so already.
     if (type.getBasicType() == EbtStruct && !structDeclared(type.getStruct()))
     {
-        TStructure *structure = type.getStruct();
+        const TStructure *structure = type.getStruct();
 
         declareStruct(structure);
 

src/compiler/translator/OutputHLSL.cpp

@@ -878,11 +878,13 @@ void OutputHLSL::visitSymbol(TIntermSymbol *node)
     }
     else
     {
+        const TType &nodeType = node->getType();
         TQualifier qualifier = node->getQualifier();
 
+        ensureStructDefined(nodeType);
+
         if (qualifier == EvqUniform)
         {
-            const TType &nodeType                 = node->getType();
             const TInterfaceBlock *interfaceBlock = nodeType.getInterfaceBlock();
 
             if (interfaceBlock)
@@ -894,8 +896,6 @@ void OutputHLSL::visitSymbol(TIntermSymbol *node)
                 mReferencedUniforms[name] = node;
             }
 
-            ensureStructDefined(nodeType);
-
             out << DecorateVariableIfNeeded(node->getName());
         }
         else if (qualifier == EvqAttribute || qualifier == EvqVertexIn)
@@ -1654,7 +1654,7 @@ TString OutputHLSL::samplerNamePrefixFromStruct(TIntermTyped *node)
         }
         case EOpIndexDirectStruct:
         {
-            TStructure *s       = nodeBinary->getLeft()->getAsTyped()->getType().getStruct();
+            const TStructure *s = nodeBinary->getLeft()->getAsTyped()->getType().getStruct();
             int index           = nodeBinary->getRight()->getAsConstantUnion()->getIConst(0);
             const TField *field = s->fields()[index];
 
@@ -1821,7 +1821,8 @@ bool OutputHLSL::visitDeclaration(Visit visit, TIntermDeclaration *node)
             else if (variable->getAsSymbolNode() &&
                      variable->getAsSymbolNode()->getSymbol() == "")  // Type (struct) declaration
             {
-                // Already added to constructor map
+                ASSERT(variable->getBasicType() == EbtStruct);
+                // ensureStructDefined has already been called.
             }
             else
                 UNREACHABLE();
@@ -1992,45 +1993,7 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
             return false;
         }
         case EOpConstruct:
-            if (node->getBasicType() == EbtStruct)
-            {
-                if (node->getType().isArray())
-                {
-                    UNIMPLEMENTED();
-                }
-                const TString &structName = StructNameString(*node->getType().getStruct());
-                mStructureHLSL->addConstructor(node->getType(), structName, node->getSequence());
-                outputTriplet(out, visit, (structName + "_ctor(").c_str(), ", ", ")");
-            }
-            else
-            {
-                const char *name = "";
-                if (node->getType().getNominalSize() == 1)
-                {
-                    switch (node->getBasicType())
-                    {
-                        case EbtFloat:
-                            name = "vec1";
-                            break;
-                        case EbtInt:
-                            name = "ivec1";
-                            break;
-                        case EbtUInt:
-                            name = "uvec1";
-                            break;
-                        case EbtBool:
-                            name = "bvec1";
-                            break;
-                        default:
-                            UNREACHABLE();
-                    }
-                }
-                else
-                {
-                    name = node->getType().getBuiltInTypeNameString();
-                }
-                outputConstructor(out, visit, node->getType(), name, node->getSequence());
-            }
+            outputConstructor(out, visit, node);
             break;
         case EOpEqualComponentWise:
             outputTriplet(out, visit, "(", " == ", ")");
@@ -2742,21 +2705,23 @@ TString OutputHLSL::initializer(const TType &type)
     return "{" + string + "}";
 }
 
-void OutputHLSL::outputConstructor(TInfoSinkBase &out,
-                                   Visit visit,
-                                   const TType &type,
-                                   const char *name,
-                                   const TIntermSequence *parameters)
+void OutputHLSL::outputConstructor(TInfoSinkBase &out, Visit visit, TIntermAggregate *node)
 {
-    if (type.isArray())
-    {
-        UNIMPLEMENTED();
-    }
+    // Array constructors should have been already pruned from the code.
+    ASSERT(!node->getType().isArray());
 
     if (visit == PreVisit)
     {
-        TString constructorName = mStructureHLSL->addConstructor(type, name, parameters);
-
+        TString constructorName;
+        if (node->getBasicType() == EbtStruct)
+        {
+            constructorName = mStructureHLSL->addStructConstructor(*node->getType().getStruct());
+        }
+        else
+        {
+            constructorName =
+                mStructureHLSL->addBuiltInConstructor(node->getType(), node->getSequence());
+        }
         out << constructorName << "(";
     }
     else if (visit == InVisit)
@@ -2778,7 +2743,7 @@ const TConstantUnion *OutputHLSL::writeConstantUnion(TInfoSinkBase &out,
     const TStructure *structure = type.getStruct();
     if (structure)
     {
-        out << StructNameString(*structure) + "_ctor(";
+        out << mStructureHLSL->addStructConstructor(*structure) << "(";
 
         const TFieldList &fields = structure->fields();
 
@@ -3105,11 +3070,11 @@ TString OutputHLSL::addArrayConstructIntoFunction(const TType &type)
 
 void OutputHLSL::ensureStructDefined(const TType &type)
 {
-    TStructure *structure = type.getStruct();
-
+    const TStructure *structure = type.getStruct();
     if (structure)
     {
-        mStructureHLSL->addConstructor(type, StructNameString(*structure), nullptr);
+        ASSERT(type.getBasicType() == EbtStruct);
+        mStructureHLSL->ensureStructDefined(*structure);
     }
 }
 

src/compiler/translator/OutputHLSL.h

@@ -104,12 +104,7 @@ class OutputHLSL : public TIntermTraverser
     void outputLineDirective(TInfoSinkBase &out, int line);
     TString argumentString(const TIntermSymbol *symbol);
 
-    // Emit constructor. Called with literal names so using const char* instead of TString.
-    void outputConstructor(TInfoSinkBase &out,
-                           Visit visit,
-                           const TType &type,
-                           const char *name,
-                           const TIntermSequence *parameters);
+    void outputConstructor(TInfoSinkBase &out, Visit visit, TIntermAggregate *node);
     const TConstantUnion *writeConstantUnion(TInfoSinkBase &out,
                                              const TType &type,
                                              const TConstantUnion *constUnion);

src/compiler/translator/StructureHLSL.h

@@ -4,7 +4,7 @@
 // found in the LICENSE file.
 //
 // StructureHLSL.h:
-//   Interfaces of methods for HLSL translation of GLSL structures.
+//   HLSL translation of GLSL constructors and structures.
 //
 
 #ifndef COMPILER_TRANSLATOR_STRUCTUREHLSL_H_
@@ -49,17 +49,14 @@ class StructureHLSL : angle::NonCopyable
   public:
     StructureHLSL();
 
-    // Returns the name of the constructor function. "name" parameter is the name of the type being
-    // constructed.
-    TString addConstructor(const TType &type,
-                           const TString &name,
-                           const TIntermSequence *parameters);
-    std::string structsHeader() const;
+    // Returns the name of the constructor function.
+    TString addStructConstructor(const TStructure &structure);
+    TString addBuiltInConstructor(const TType &type, const TIntermSequence *parameters);
 
-    TString defineQualified(const TStructure &structure,
-                            bool useHLSLRowMajorPacking,
-                            bool useStd140Packing);
     static TString defineNameless(const TStructure &structure);
+    void ensureStructDefined(const TStructure &structure);
+
+    std::string structsHeader() const;
 
     Std140PaddingHelper getPaddingHelper();
 
@@ -68,20 +65,33 @@ class StructureHLSL : angle::NonCopyable
 
     std::map<TString, int> mStd140StructElementIndexes;
 
-    typedef std::set<TString> StructNames;
-    StructNames mStructNames;
+    struct TStructProperties : public angle::NonCopyable
+    {
+        POOL_ALLOCATOR_NEW_DELETE();
 
-    typedef std::set<TString> Constructors;
-    Constructors mConstructors;
+        TStructProperties() {}
 
+        // Constructor is an empty string in case the struct doesn't have a constructor yet.
+        TString constructor;
+    };
+
+    // Map from struct name to struct properties.
+    typedef std::map<TString, TStructProperties *> DefinedStructs;
+    DefinedStructs mDefinedStructs;
+
+    // Struct declarations need to be kept in a vector instead of having them inside mDefinedStructs
+    // since maintaining the original order is necessary for nested structs.
     typedef std::vector<TString> StructDeclarations;
     StructDeclarations mStructDeclarations;
 
+    typedef std::set<TString> BuiltInConstructors;
+    BuiltInConstructors mBuiltInConstructors;
+
     void storeStd140ElementIndex(const TStructure &structure, bool useHLSLRowMajorPacking);
-    static TString define(const TStructure &structure,
-                          bool useHLSLRowMajorPacking,
-                          bool useStd140Packing,
-                          Std140PaddingHelper *padHelper);
+    TString defineQualified(const TStructure &structure,
+                            bool useHLSLRowMajorPacking,
+                            bool useStd140Packing);
+    DefinedStructs::iterator defineVariants(const TStructure &structure, const TString &name);
 };
 }
 

src/compiler/translator/Types.h

@@ -383,7 +383,8 @@ class TType
 
     bool canBeConstructed() const;
 
-    TStructure *getStruct() const { return structure; }
+    TStructure *getStruct() { return structure; }
+    const TStructure *getStruct() const { return structure; }
     void setStruct(TStructure *s)
     {
         if (structure != s)

src/compiler/translator/UniformHLSL.cpp

@@ -38,7 +38,7 @@ static TString InterfaceBlockFieldTypeString(const TField &field, TLayoutBlockSt
     const TType &fieldType                   = *field.type();
     const TLayoutMatrixPacking matrixPacking = fieldType.getLayoutQualifier().matrixPacking;
     ASSERT(matrixPacking != EmpUnspecified);
-    TStructure *structure = fieldType.getStruct();
+    const TStructure *structure = fieldType.getStruct();
 
     if (fieldType.isMatrix())
     {

src/tests/compiler_tests/DebugShaderPrecision_test.cpp

@@ -817,8 +817,8 @@ TEST_F(DebugShaderPrecisionTest, ConstructorRounding)
     ASSERT_TRUE(foundInAllGLSLCode("v1 = angle_frm(vec4(_uu1, _uu2, angle_frl(_uu3), _uu4))"));
     ASSERT_TRUE(foundInAllGLSLCode("v2 = angle_frm(vec4(_uuiv))"));
 
-    ASSERT_TRUE(foundInHLSLCode("v1 = angle_frm(vec4(_u1, _u2, angle_frl(_u3), _u4))"));
-    ASSERT_TRUE(foundInHLSLCode("v2 = angle_frm(vec4(_uiv))"));
+    ASSERT_TRUE(foundInHLSLCode("v1 = angle_frm(vec4_ctor(_u1, _u2, angle_frl(_u3), _u4))"));
+    ASSERT_TRUE(foundInHLSLCode("v2 = angle_frm(vec4_ctor(_uiv))"));
 }
 
 TEST_F(DebugShaderPrecisionTest, StructConstructorNoRounding)

src/tests/compiler_tests/InitOutputVariables_test.cpp

@@ -162,7 +162,7 @@ class FindStructByName final : public TIntermTraverser
             return;
         }
 
-        TStructure *structure = symbol->getType().getStruct();
+        TStructure *structure = symbol->getTypePointer()->getStruct();
 
         if (structure != nullptr && structure->name() == mStructName)
         {

src/tests/compiler_tests/WEBGL_multiview_test.cpp

@@ -555,8 +555,8 @@ TEST_F(WEBGLMultiviewVertexShaderOutputCodeTest, ViewIDAndInstanceIDHaveCorrectV
     EXPECT_TRUE(foundInAllGLSLCode("ViewID_OVR = (uint(gl_InstanceID) % 3u)"));
     EXPECT_TRUE(foundInAllGLSLCode("InstanceID = int((uint(gl_InstanceID) / 3u))"));
 
-    EXPECT_TRUE(foundInHLSLCode("ViewID_OVR = (uvec1(gl_InstanceID) % 3)"));
-    EXPECT_TRUE(foundInHLSLCode("InstanceID = ivec1((uvec1(gl_InstanceID) / 3))"));
+    EXPECT_TRUE(foundInHLSLCode("ViewID_OVR = (uint_ctor(gl_InstanceID) % 3)"));
+    EXPECT_TRUE(foundInHLSLCode("InstanceID = int_ctor((uint_ctor(gl_InstanceID) / 3))"));
 }
 
 // The test checks that the directive enabling GL_OVR_multiview is not outputted if the extension is

src/tests/gl_tests/GLSLTest.cpp

@@ -3715,6 +3715,87 @@ TEST_P(GLSLTest_ES3, EmptySwitch)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Test that a constant struct inside an expression is handled correctly.
+TEST_P(GLSLTest_ES3, ConstStructInsideExpression)
+{
+    // Incorrect output color was seen on Android. http://anglebug.com/2226
+    ANGLE_SKIP_TEST_IF(IsAndroid() && !IsNVIDIA() && IsOpenGLES());
+
+    const std::string &fragmentShader =
+        R"(#version 300 es
+
+        precision highp float;
+        out vec4 my_FragColor;
+
+        uniform float u_zero;
+
+        struct S
+        {
+            float field;
+        };
+
+        void main()
+        {
+            const S constS = S(1.0);
+            S nonConstS = constS;
+            nonConstS.field = u_zero;
+            bool fail = (constS == nonConstS);
+            my_FragColor = vec4(0, 1, 0, 1);
+            if (fail)
+            {
+                my_FragColor = vec4(1, 0, 0, 1);
+            }
+        })";
+
+    ANGLE_GL_PROGRAM(program, mSimpleVSSource, fragmentShader);
+    drawQuad(program.get(), "inputAttribute", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
+// Test that a varying struct that's defined as a part of the declaration is handled correctly.
+TEST_P(GLSLTest_ES3, VaryingStructWithInlineDefinition)
+{
+    const std::string &vertexShader =
+        R"(#version 300 es
+            in vec4 inputAttribute;
+
+            flat out struct S
+            {
+                int field;
+            } v_s;
+
+            void main()
+            {
+                v_s.field = 1;
+                gl_Position = inputAttribute;
+            })";
+
+    const std::string &fragmentShader =
+        R"(#version 300 es
+
+        precision highp float;
+        out vec4 my_FragColor;
+
+        flat in struct S
+        {
+            int field;
+        } v_s;
+
+        void main()
+        {
+            bool success = (v_s.field == 1);
+            my_FragColor = vec4(1, 0, 0, 1);
+            if (success)
+            {
+                my_FragColor = vec4(0, 1, 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(),