Redesign HLSL scoped structures to a unique ID.

src/compiler/translator/OutputHLSL.cpp

@@ -136,8 +136,6 @@ OutputHLSL::OutputHLSL(TParseContext &context, const ShBuiltInResources& resourc
 
     mNumRenderTargets = resources.EXT_draw_buffers ? resources.MaxDrawBuffers : 1;
 
-    mScopeDepth = 0;
-
     mUniqueIndex = 0;
 
     mContainsLoopDiscontinuity = false;
@@ -2257,17 +2255,6 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
             {
                 outputLineDirective(node->getLine().first_line);
                 out << "{\n";
-
-                mScopeDepth++;
-
-                if (mScopeBracket.size() < mScopeDepth)
-                {
-                    mScopeBracket.push_back(0);   // New scope level
-                }
-                else
-                {
-                    mScopeBracket[mScopeDepth - 1]++;   // New scope at existing level
-                }
             }
 
             for (TIntermSequence::iterator sit = node->getSequence().begin(); sit != node->getSequence().end(); sit++)
@@ -2283,8 +2270,6 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
             {
                 outputLineDirective(node->getLine().last_line);
                 out << "}\n";
-
-                mScopeDepth--;
             }
 
             return false;
@@ -2299,7 +2284,7 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
             {
                 if (variable->getType().getStruct())
                 {
-                    addConstructor(variable->getType(), scopedStruct(variable->getType().getStruct()->name()), NULL);
+                    addConstructor(variable->getType(), structNameString(*variable->getType().getStruct()), NULL);
                 }
 
                 if (!variable->getAsSymbolNode() || variable->getAsSymbolNode()->getSymbol() != "")   // Variable declaration
@@ -2426,7 +2411,7 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
                 {
                     if (symbol->getType().getStruct())
                     {
-                        addConstructor(symbol->getType(), scopedStruct(symbol->getType().getStruct()->name()), NULL);
+                        addConstructor(symbol->getType(), structNameString(*symbol->getType().getStruct()), NULL);
                     }
 
                     out << argumentString(symbol);
@@ -2694,8 +2679,11 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
         outputTriplet(visit, "mat4(", ", ", ")");
         break;
       case EOpConstructStruct:
-        addConstructor(node->getType(), scopedStruct(node->getType().getStruct()->name()), &node->getSequence());
-        outputTriplet(visit, structLookup(node->getType().getStruct()->name()) + "_ctor(", ", ", ")");
+        {
+            const TString &structName = structNameString(*node->getType().getStruct());
+            addConstructor(node->getType(), structName, &node->getSequence());
+            outputTriplet(visit, structName + "_ctor(", ", ", ")");
+        }
         break;
       case EOpLessThan:         outputTriplet(visit, "(", " < ", ")");                 break;
       case EOpGreaterThan:      outputTriplet(visit, "(", " > ", ")");                 break;
@@ -3320,7 +3308,7 @@ TString OutputHLSL::typeString(const TType &type)
         const TString& typeName = structure->name();
         if (typeName != "")
         {
-            return structLookup(typeName);
+            return structNameString(*type.getStruct());
         }
         else   // Nameless structure, define in place
         {
@@ -3527,7 +3515,7 @@ TString OutputHLSL::structureTypeName(const TStructure &structure, bool useHLSLR
         prefix += "rm";
     }
 
-    return prefix + structLookup(structure.name());
+    return prefix + structNameString(structure);
 }
 
 void OutputHLSL::addConstructor(const TType &type, const TString &name, const TIntermSequence *parameters)
@@ -3752,7 +3740,7 @@ const ConstantUnion *OutputHLSL::writeConstantUnion(const TType &type, const Con
     const TStructure* structure = type.getStruct();
     if (structure)
     {
-        out << structLookup(structure->name()) + "_ctor(";
+        out << structNameString(*structure) + "_ctor(";
         
         const TFieldList& fields = structure->fields();
 
@@ -3806,46 +3794,14 @@ const ConstantUnion *OutputHLSL::writeConstantUnion(const TType &type, const Con
     return constUnion;
 }
 
-TString OutputHLSL::scopeString(unsigned int depthLimit)
-{
-    TString string;
-
-    for (unsigned int i = 0; i < mScopeBracket.size() && i < depthLimit; i++)
-    {
-        string += str(mScopeBracket[i]) + "_";
-    }
-
-    return "ss_" + string;
-}
-
-TString OutputHLSL::scopedStruct(const TString &typeName)
-{
-    if (typeName == "")
-    {
-        return typeName;
-    }
-
-    return scopeString(mScopeDepth) + typeName;
-}
-
-TString OutputHLSL::structLookup(const TString &typeName)
+TString OutputHLSL::structNameString(const TStructure &structure)
 {
-    for (int depth = mScopeDepth; depth >= 0; depth--)
+    if (structure.name().empty())
     {
-        TString scopedName = scopeString(depth) + typeName;
-
-        for (StructNames::iterator structName = mStructNames.begin(); structName != mStructNames.end(); structName++)
-        {
-            if (*structName == scopedName)
-            {
-                return scopedName;
-            }
-        }
+        return "";
     }
 
-    UNREACHABLE();   // Should have found a matching constructor
-
-    return typeName;
+    return "ss_" + str(structure.uniqueId()) + structure.name();
 }
 
 TString OutputHLSL::decorate(const TString &string)

src/compiler/translator/OutputHLSL.h

@@ -74,9 +74,7 @@ class OutputHLSL : public TIntermTraverser
     void addConstructor(const TType &type, const TString &name, const TIntermSequence *parameters);
     const ConstantUnion *writeConstantUnion(const TType &type, const ConstantUnion *constUnion);
 
-    TString scopeString(unsigned int depthLimit);
-    TString scopedStruct(const TString &typeName);
-    TString structLookup(const TString &typeName);
+    TString structNameString(const TStructure &structure);
 
     TParseContext &mContext;
     const ShShaderOutput mOutputType;
@@ -162,10 +160,6 @@ class OutputHLSL : public TIntermTraverser
     typedef std::list<TString> StructDeclarations;
     StructDeclarations mStructDeclarations;
 
-    typedef std::vector<int> ScopeBracket;
-    ScopeBracket mScopeBracket;
-    unsigned int mScopeDepth;
-
     int mUniqueIndex;   // For creating unique names
 
     bool mContainsLoopDiscontinuity;

src/compiler/translator/ParseContext.cpp

@@ -2567,6 +2567,8 @@ TPublicType TParseContext::addStructure(const TSourceLoc& structLine, const TSou
     TStructure* structure = new TStructure(structName, fieldList);
     TType* structureType = new TType(structure);
 
+    structure->setUniqueId(TSymbolTable::nextUniqueId());
+
     if (!structName->empty())
     {
         if (reservedErrorCheck(nameLine, *structName))

src/compiler/translator/SymbolTable.cpp

@@ -18,7 +18,7 @@
 #include <stdio.h>
 #include <algorithm>
 
-int TSymbolTableLevel::uniqueId = 0;
+int TSymbolTable::uniqueIdCounter = 0;
 
 //
 // Functions have buried pointers to delete.
@@ -38,6 +38,30 @@ TSymbolTableLevel::~TSymbolTableLevel()
         delete (*it).second;
 }
 
+bool TSymbolTableLevel::insert(const TString &name, TSymbol &symbol)
+{
+    symbol.setUniqueId(TSymbolTable::nextUniqueId());
+
+    // returning true means symbol was added to the table
+    tInsertResult result = level.insert(tLevelPair(name, &symbol));
+
+    return result.second;
+}
+
+bool TSymbolTableLevel::insert(TSymbol &symbol)
+{
+    return insert(symbol.getMangledName(), symbol);
+}
+
+TSymbol *TSymbolTableLevel::find(const TString &name) const
+{
+    tLevel::const_iterator it = level.find(name);
+    if (it == level.end())
+        return 0;
+    else
+        return (*it).second;
+}
+
 //
 // Change all function entries in the table with the non-mangled name
 // to be related to the provided built-in operation.  This is a low

src/compiler/translator/SymbolTable.h

@@ -288,29 +288,10 @@ class TSymbolTableLevel
     }
     ~TSymbolTableLevel();
 
-    bool insert(const TString &name, TSymbol &symbol)
-    {
-        symbol.setUniqueId(++uniqueId);
-
-        // returning true means symbol was added to the table
-        tInsertResult result = level.insert(tLevelPair(name, &symbol));
+    bool insert(const TString &name, TSymbol &symbol);
+    bool insert(TSymbol &symbol);
 
-        return result.second;
-    }
-
-    bool insert(TSymbol &symbol)
-    {
-        return insert(symbol.getMangledName(), symbol);
-    }
-
-    TSymbol *find(const TString &name) const
-    {
-        tLevel::const_iterator it = level.find(name);
-        if (it == level.end())
-            return 0;
-        else
-            return (*it).second;
-    }
+    TSymbol *find(const TString &name) const;
 
     void relateToOperator(const char *name, TOperator op);
     void relateToExtension(const char *name, const TString &ext);
@@ -429,6 +410,11 @@ class TSymbolTable
     // for the specified TBasicType
     TPrecision getDefaultPrecision(TBasicType type);
 
+    static int nextUniqueId()
+    {
+        return ++uniqueIdCounter;
+    }
+
   private:
     ESymbolLevel currentLevel() const
     {
@@ -438,6 +424,8 @@ class TSymbolTable
     std::vector<TSymbolTableLevel *> table;
     typedef TMap<TBasicType, TPrecision> PrecisionStackLevel;
     std::vector< PrecisionStackLevel *> precisionStack;
+
+    static int uniqueIdCounter;
 };
 
 #endif // _SYMBOL_TABLE_INCLUDED_

src/compiler/translator/Types.h

@@ -15,6 +15,7 @@
 
 struct TPublicType;
 class TType;
+class TSymbol;
 
 class TField
 {
@@ -115,7 +116,8 @@ class TStructure : public TFieldListCollection
     POOL_ALLOCATOR_NEW_DELETE();
     TStructure(const TString *name, TFieldList *fields)
         : TFieldListCollection(name, fields),
-          mDeepestNesting(0)
+          mDeepestNesting(0),
+          mUniqueId(0)
     {
     }
 
@@ -129,6 +131,17 @@ class TStructure : public TFieldListCollection
 
     bool equals(const TStructure &other) const;
 
+    void setUniqueId(int uniqueId)
+    {
+        mUniqueId = uniqueId;
+    }
+
+    int uniqueId() const
+    {
+        ASSERT(mUniqueId != 0);
+        return mUniqueId;
+    }
+
   private:
     DISALLOW_COPY_AND_ASSIGN(TStructure);
     virtual TString mangledNamePrefix() const
@@ -138,6 +151,7 @@ class TStructure : public TFieldListCollection
     int calculateDeepestNesting() const;
 
     mutable int mDeepestNesting;
+    int mUniqueId;
 };
 
 class TInterfaceBlock : public TFieldListCollection

tests/angle_tests/GLSLStructTest.cpp

@@ -12,19 +12,79 @@ protected:
         setConfigBlueBits(8);
         setConfigAlphaBits(8);
     }
+
+    virtual void SetUp()
+    {
+        ANGLETest::SetUp();
+
+        mVertexShaderSource = SHADER_SOURCE
+        (
+            attribute vec4 inputAttribute;
+            void main()
+            {
+                gl_Position = inputAttribute;
+            }
+        );
+    }
+
+    std::string mVertexShaderSource;
 };
 
-TEST_F(GLSLStructTest, scoped_structs_bug)
+TEST_F(GLSLStructTest, nameless_scoped_structs)
 {
-    const std::string vertexShaderSource = SHADER_SOURCE
+    const std::string fragmentShaderSource = SHADER_SOURCE
     (
-        attribute vec4 inputAttribute;
+        precision mediump float;
+
         void main()
         {
-            gl_Position = inputAttribute;
+            struct
+            {
+                float q;
+            } b;
+
+            gl_FragColor = vec4(1, 0, 0, 1);
+            gl_FragColor.a += b.q;
         }
     );
 
+    GLuint program = compileProgram(mVertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+TEST_F(GLSLStructTest, scoped_structs_order_bug)
+{
+    const std::string fragmentShaderSource = SHADER_SOURCE
+    (
+        precision mediump float;
+
+        struct T
+        {
+            float f;
+        };
+
+        void main()
+        {
+            T a;
+
+            struct T
+            {
+                float q;
+            };
+
+            T b;
+
+            gl_FragColor = vec4(1, 0, 0, 1);
+            gl_FragColor.a += a.f;
+            gl_FragColor.a += b.q;
+        }
+    );
+
+    GLuint program = compileProgram(mVertexShaderSource, fragmentShaderSource);
+    EXPECT_NE(0u, program);
+}
+
+TEST_F(GLSLStructTest, scoped_structs_bug)
+{
     const std::string fragmentShaderSource = SHADER_SOURCE
     (
         precision mediump float;
@@ -51,6 +111,6 @@ TEST_F(GLSLStructTest, scoped_structs_bug)
         }
     );
 
-    GLuint program = compileProgram(vertexShaderSource, fragmentShaderSource);
+    GLuint program = compileProgram(mVertexShaderSource, fragmentShaderSource);
     EXPECT_NE(0u, program);
 }