Store shader interface variables as per query spec

src/common/string_utils.cpp

@@ -158,6 +158,11 @@ Optional<std::vector<wchar_t>> WidenString(size_t length, const char *cString)
     return Optional<std::vector<wchar_t>>(wcstring);
 }
 
+bool BeginsWith(const std::string &str, const std::string &prefix)
+{
+    return strncmp(str.c_str(), prefix.c_str(), prefix.length()) == 0;
+}
+
 bool BeginsWith(const std::string &str, const char *prefix)
 {
     return strncmp(str.c_str(), prefix, strlen(prefix)) == 0;
@@ -168,6 +173,11 @@ bool BeginsWith(const char *str, const char *prefix)
     return strncmp(str, prefix, strlen(prefix)) == 0;
 }
 
+bool BeginsWith(const std::string &str, const std::string &prefix, const size_t prefixLength)
+{
+    return strncmp(str.c_str(), prefix.c_str(), prefixLength) == 0;
+}
+
 bool EndsWith(const std::string &str, const char *suffix)
 {
     const auto len = strlen(suffix);

src/common/string_utils.h

@@ -49,6 +49,10 @@ bool ReadFileToString(const std::string &path, std::string *stringOut);
 Optional<std::vector<wchar_t>> WidenString(size_t length, const char *cString);
 
 // Check if the string str begins with the given prefix.
+// The comparison is case sensitive.
+bool BeginsWith(const std::string &str, const std::string &prefix);
+
+// Check if the string str begins with the given prefix.
 // Prefix may not be NULL and needs to be NULL terminated.
 // The comparison is case sensitive.
 bool BeginsWith(const std::string &str, const char *prefix);
@@ -58,6 +62,11 @@ bool BeginsWith(const std::string &str, const char *prefix);
 // The comparison is case sensitive.
 bool BeginsWith(const char *str, const char *prefix);
 
+// Check if the string str begins with the first prefixLength characters of the given prefix.
+// The length of the prefix string should be greater than or equal to prefixLength.
+// The comparison is case sensitive.
+bool BeginsWith(const std::string &str, const std::string &prefix, const size_t prefixLength);
+
 // Check if the string str ends with the given suffix.
 // Suffix may not be NUL and needs to be NULL terminated.
 // The comparison is case sensitive.

src/common/string_utils_unittest.cpp

@@ -138,18 +138,73 @@ TEST(StringUtilsTest, HexStringToUIntBasic)
 
 // Note: ReadFileToString is harder to test
 
+class BeginsWithTest : public testing::Test
+{
+  public:
+    BeginsWithTest() : mMode(TestMode::CHAR_ARRAY) {}
+
+    enum class TestMode
+    {
+        CHAR_ARRAY,
+        STRING_AND_CHAR_ARRAY,
+        STRING
+    };
+
+    void setMode(TestMode mode) { mMode = mode; }
+
+    bool runBeginsWith(const char *str, const char *prefix)
+    {
+        if (mMode == TestMode::CHAR_ARRAY)
+        {
+            return BeginsWith(str, prefix);
+        }
+        if (mMode == TestMode::STRING_AND_CHAR_ARRAY)
+        {
+            return BeginsWith(std::string(str), prefix);
+        }
+        return BeginsWith(std::string(str), std::string(prefix));
+    }
+
+    void runTest()
+    {
+        ASSERT_FALSE(runBeginsWith("foo", "bar"));
+        ASSERT_FALSE(runBeginsWith("", "foo"));
+        ASSERT_FALSE(runBeginsWith("foo", "foobar"));
+
+        ASSERT_TRUE(runBeginsWith("foobar", "foo"));
+        ASSERT_TRUE(runBeginsWith("foobar", ""));
+        ASSERT_TRUE(runBeginsWith("foo", "foo"));
+        ASSERT_TRUE(runBeginsWith("", ""));
+    }
+
+  private:
+    TestMode mMode;
+};
+
+// Test that BeginsWith works correctly for const char * arguments.
+TEST_F(BeginsWithTest, CharArrays)
+{
+    setMode(TestMode::CHAR_ARRAY);
+    runTest();
+}
 
-TEST(StringUtilsTest, BeginsEndsWith)
+// Test that BeginsWith works correctly for std::string and const char * arguments.
+TEST_F(BeginsWithTest, StringAndCharArray)
 {
-    ASSERT_FALSE(BeginsWith("foo", "bar"));
-    ASSERT_FALSE(BeginsWith("", "foo"));
-    ASSERT_FALSE(BeginsWith("foo", "foobar"));
+    setMode(TestMode::STRING_AND_CHAR_ARRAY);
+    runTest();
+}
 
-    ASSERT_TRUE(BeginsWith("foobar", "foo"));
-    ASSERT_TRUE(BeginsWith("foobar", ""));
-    ASSERT_TRUE(BeginsWith("foo", "foo"));
-    ASSERT_TRUE(BeginsWith("", ""));
+// Test that BeginsWith works correctly for std::string arguments.
+TEST_F(BeginsWithTest, Strings)
+{
+    setMode(TestMode::STRING);
+    runTest();
+}
 
+// Test that EndsWith works correctly.
+TEST(EndsWithTest, EndsWith)
+{
     ASSERT_FALSE(EndsWith("foo", "bar"));
     ASSERT_FALSE(EndsWith("", "bar"));
     ASSERT_FALSE(EndsWith("foo", "foobar"));
@@ -160,4 +215,4 @@ TEST(StringUtilsTest, BeginsEndsWith)
     ASSERT_TRUE(EndsWith("", ""));
 }
 
-}
+}  // anonymous namespace
\ No newline at end of file

src/common/utilities.cpp

@@ -770,19 +770,33 @@ std::string ParseResourceName(const std::string &name, std::vector<unsigned int>
     return name.substr(0, baseNameLength);
 }
 
-unsigned int ParseAndStripArrayIndex(std::string *name)
+unsigned int ParseArrayIndex(const std::string &name, size_t *nameLengthWithoutArrayIndexOut)
 {
+    ASSERT(nameLengthWithoutArrayIndexOut != nullptr);
     unsigned int subscript = GL_INVALID_INDEX;
 
     // Strip any trailing array operator and retrieve the subscript
-    size_t open  = name->find_last_of('[');
-    size_t close = name->find_last_of(']');
-    if (open != std::string::npos && close == name->length() - 1)
+    size_t open = name.find_last_of('[');
+    if (open != std::string::npos && name.back() == ']')
     {
-        subscript = atoi(name->c_str() + open + 1);
-        name->erase(open);
+        bool indexIsValidDecimalNumber = true;
+        for (size_t i = open + 1; i < name.length() - 1u; ++i)
+        {
+            if (!isdigit(name[i]))
+            {
+                indexIsValidDecimalNumber = false;
+                break;
+            }
+        }
+        if (indexIsValidDecimalNumber)
+        {
+            subscript                       = atoi(name.c_str() + open + 1);
+            *nameLengthWithoutArrayIndexOut = open;
+            return subscript;
+        }
     }
 
+    *nameLengthWithoutArrayIndexOut = name.length();
     return subscript;
 }
 

src/common/utilities.h

@@ -70,7 +70,10 @@ GLuint GetPrimitiveRestartIndex(GLenum indexType);
 bool IsTriangleMode(GLenum drawMode);
 bool IsIntegerFormat(GLenum unsizedFormat);
 
-unsigned int ParseAndStripArrayIndex(std::string *name);
+// Return the array index at the end of name, and write the length of name before the final array
+// index into nameLengthWithoutArrayIndexOut. In case name doesn't include an array index, return
+// GL_INVALID_INDEX and write the length of the original string.
+unsigned int ParseArrayIndex(const std::string &name, size_t *nameLengthWithoutArrayIndexOut);
 
 struct UniformTypeInfo final : angle::NonCopyable
 {

src/common/utilities_unittest.cpp

@@ -75,4 +75,97 @@ TEST(ParseResourceName, TrailingWhitespace)
     EXPECT_TRUE(indices.empty());
 }
 
+// Parse a string without any index.
+TEST(ParseArrayIndex, NoArrayIndex)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(3u, nameLengthWithoutArrayIndex);
+}
+
+// Parse an empty string for an array index.
+TEST(ParseArrayIndex, EmptyString)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(0u, nameLengthWithoutArrayIndex);
+}
+
+// A valid array index is parsed correctly from the end of the string.
+TEST(ParseArrayIndex, ArrayIndex)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(123u, gl::ParseArrayIndex("foo[123]", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(3u, nameLengthWithoutArrayIndex);
+}
+
+// An array index from the middle of the string is not parsed.
+TEST(ParseArrayIndex, ArrayIndexInMiddle)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[123].bar", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(12u, nameLengthWithoutArrayIndex);
+}
+
+// Trailing whitespace in the parsed string is taken into account.
+TEST(ParseArrayIndex, TrailingWhitespace)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[123] ", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(9u, nameLengthWithoutArrayIndex);
+}
+
+// Only the last index is parsed.
+TEST(ParseArrayIndex, MultipleArrayIndices)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(34u, gl::ParseArrayIndex("foo[12][34]", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(7u, nameLengthWithoutArrayIndex);
 }
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 mentions "decimal" integer.
+// So an integer in hexadecimal format should not parse as an array index.
+TEST(ParseArrayIndex, HexArrayIndex)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[0xff]", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(9u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 mentions that the array
+// index should not contain a leading plus sign.
+TEST(ParseArrayIndex, ArrayIndexLeadingPlus)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[+1]", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(7u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 says that index should not
+// contain whitespace. Test leading whitespace.
+TEST(ParseArrayIndex, ArrayIndexLeadingWhiteSpace)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[ 0]", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(7u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 says that index should not
+// contain whitespace. Test trailing whitespace.
+TEST(ParseArrayIndex, ArrayIndexTrailingWhiteSpace)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[0 ]", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(7u, nameLengthWithoutArrayIndex);
+}
+
+// GetProgramResourceLocation spec in GLES 3.1 November 2016 page 87 says that index should only
+// contain an integer.
+TEST(ParseArrayIndex, ArrayIndexBogus)
+{
+    size_t nameLengthWithoutArrayIndex;
+    EXPECT_EQ(GL_INVALID_INDEX, gl::ParseArrayIndex("foo[0bogus]", &nameLengthWithoutArrayIndex));
+    EXPECT_EQ(11u, nameLengthWithoutArrayIndex);
+}
+
+}  // anonymous namespace

src/libANGLE/MemoryProgramCache.cpp

@@ -316,15 +316,15 @@ LinkResult MemoryProgramCache::Deserialize(const Context *context,
     }
 
     unsigned int outputVarCount = stream.readInt<unsigned int>();
+    ASSERT(state->mOutputLocations.empty());
     for (unsigned int outputIndex = 0; outputIndex < outputVarCount; ++outputIndex)
     {
-        int locationIndex = stream.readInt<int>();
         VariableLocation locationData;
         stream.readIntVector<unsigned int>(&locationData.arrayIndices);
         stream.readInt(&locationData.index);
         stream.readInt(&locationData.flattenedArrayOffset);
         stream.readBool(&locationData.ignored);
-        state->mOutputLocations[locationIndex] = locationData;
+        state->mOutputLocations.push_back(locationData);
     }
 
     unsigned int outputTypeCount = stream.readInt<unsigned int>();
@@ -481,13 +481,12 @@ void MemoryProgramCache::Serialize(const Context *context,
     }
 
     stream.writeInt(state.getOutputLocations().size());
-    for (const auto &outputPair : state.getOutputLocations())
+    for (const auto &outputVar : state.getOutputLocations())
     {
-        stream.writeInt(outputPair.first);
-        stream.writeIntVector(outputPair.second.arrayIndices);
-        stream.writeIntOrNegOne(outputPair.second.index);
-        stream.writeInt(outputPair.second.flattenedArrayOffset);
-        stream.writeInt(outputPair.second.ignored);
+        stream.writeIntVector(outputVar.arrayIndices);
+        stream.writeIntOrNegOne(outputVar.index);
+        stream.writeInt(outputVar.flattenedArrayOffset);
+        stream.writeInt(outputVar.ignored);
     }
 
     stream.writeInt(state.mOutputVariableTypes.size());

src/libANGLE/Program.cpp

@@ -14,6 +14,7 @@
 #include "common/bitset_utils.h"
 #include "common/debug.h"
 #include "common/platform.h"
+#include "common/string_utils.h"
 #include "common/utilities.h"
 #include "compiler/translator/blocklayout.h"
 #include "libANGLE/Context.h"
@@ -140,33 +141,71 @@ bool ComparePackedVarying(const PackedVarying &x, const PackedVarying &y)
 template <typename VarT>
 GLuint GetResourceIndexFromName(const std::vector<VarT> &list, const std::string &name)
 {
-    std::vector<unsigned int> subscripts;
-    std::string baseName = ParseResourceName(name, &subscripts);
-
-    // The app is not allowed to specify array indices other than 0 for arrays of basic types
-    for (unsigned int subscript : subscripts)
+    std::string nameAsArrayName = name + "[0]";
+    for (size_t index = 0; index < list.size(); index++)
     {
-        if (subscript != 0u)
+        const VarT &resource = list[index];
+        if (resource.name == name || (resource.isArray() && resource.name == nameAsArrayName))
         {
-            return GL_INVALID_INDEX;
+            return static_cast<GLuint>(index);
         }
     }
 
-    for (size_t index = 0; index < list.size(); index++)
+    return GL_INVALID_INDEX;
+}
+
+template <typename VarT>
+GLint GetVariableLocation(const std::vector<VarT> &list,
+                          const std::vector<VariableLocation> &locationList,
+                          const std::string &name)
+{
+    size_t nameLengthWithoutArrayIndex;
+    unsigned int arrayIndex = ParseArrayIndex(name, &nameLengthWithoutArrayIndex);
+
+    for (size_t location = 0u; location < locationList.size(); ++location)
     {
-        const VarT &resource = list[index];
-        if (resource.name == baseName)
+        const VariableLocation &variableLocation = locationList[location];
+        if (!variableLocation.used())
+        {
+            continue;
+        }
+
+        const VarT &variable = list[variableLocation.index];
+
+        if (angle::BeginsWith(variable.name, name))
         {
-            // TODO(oetuaho@nvidia.com): Check array nesting >= number of specified
-            // subscripts once arrays of arrays are supported in ShaderVariable.
-            if ((resource.isArray() || subscripts.empty()) && subscripts.size() <= 1u)
+            if (name.length() == variable.name.length())
+            {
+                ASSERT(name == variable.name);
+                // GLES 3.1 November 2016 page 87.
+                // The string exactly matches the name of the active variable.
+                return static_cast<GLint>(location);
+            }
+            if (name.length() + 3u == variable.name.length() && variable.isArray())
             {
-                return static_cast<GLuint>(index);
+                ASSERT(name + "[0]" == variable.name);
+                // The string identifies the base name of an active array, where the string would
+                // exactly match the name of the variable if the suffix "[0]" were appended to the
+                // string.
+                return static_cast<GLint>(location);
             }
         }
+        if (variable.isArray() && variableLocation.arrayIndices[0] == arrayIndex &&
+            nameLengthWithoutArrayIndex + 3u == variable.name.length() &&
+            angle::BeginsWith(variable.name, name, nameLengthWithoutArrayIndex))
+        {
+            ASSERT(name.substr(0u, nameLengthWithoutArrayIndex) + "[0]" == variable.name);
+            // The string identifies an active element of the array, where the string ends with the
+            // concatenation of the "[" character, an integer (with no "+" sign, extra leading
+            // zeroes, or whitespace) identifying an array element, and the "]" character, the
+            // integer is less than the number of active elements of the array variable, and where
+            // the string would exactly match the enumerated name of the array if the decimal
+            // integer were replaced with zero.
+            return static_cast<GLint>(location);
+        }
     }
 
-    return GL_INVALID_INDEX;
+    return -1;
 }
 
 void CopyStringToBuffer(GLchar *buffer, const std::string &string, GLsizei bufSize, GLsizei *length)
@@ -361,44 +400,6 @@ const std::string &ProgramState::getLabel()
     return mLabel;
 }
 
-GLint ProgramState::getUniformLocation(const std::string &name) const
-{
-    std::vector<unsigned int> subscripts;
-    std::string baseName = ParseResourceName(name, &subscripts);
-
-    for (size_t location = 0; location < mUniformLocations.size(); ++location)
-    {
-        const VariableLocation &uniformLocation = mUniformLocations[location];
-        if (!uniformLocation.used())
-        {
-            continue;
-        }
-
-        const LinkedUniform &uniform = mUniforms[uniformLocation.index];
-
-        if (uniform.name == baseName)
-        {
-            if (uniform.isArray())
-            {
-                if (uniformLocation.arrayIndices == subscripts ||
-                    (uniformLocation.areAllArrayIndicesZero() && subscripts.empty()))
-                {
-                    return static_cast<GLint>(location);
-                }
-            }
-            else
-            {
-                if (subscripts.empty())
-                {
-                    return static_cast<GLint>(location);
-                }
-            }
-        }
-    }
-
-    return -1;
-}
-
 GLuint ProgramState::getUniformIndexFromName(const std::string &name) const
 {
     return GetResourceIndexFromName(mUniforms, name);
@@ -576,8 +577,7 @@ void Program::bindAttributeLocation(GLuint index, const char *name)
 
 void Program::bindUniformLocation(GLuint index, const char *name)
 {
-    // Bind the base uniform name only since array indices other than 0 cannot be bound
-    mUniformLocationBindings.bindLocation(index, ParseResourceName(name, nullptr));
+    mUniformLocationBindings.bindLocation(index, name);
 }
 
 void Program::bindFragmentInputLocation(GLint index, const char *name)
@@ -604,12 +604,13 @@ BindingInfo Program::getFragmentInputBindingInfo(const Context *context, GLint i
 
         ret.valid = true;
 
-        std::string originalName = binding.first;
-        unsigned int arrayIndex  = ParseAndStripArrayIndex(&originalName);
+        size_t nameLengthWithoutArrayIndex;
+        unsigned int arrayIndex = ParseArrayIndex(binding.first, &nameLengthWithoutArrayIndex);
 
         for (const auto &in : inputs)
         {
-            if (in.name == originalName)
+            if (in.name.length() == nameLengthWithoutArrayIndex &&
+                angle::BeginsWith(in.name, binding.first, nameLengthWithoutArrayIndex))
             {
                 if (in.isArray())
                 {
@@ -1133,15 +1134,7 @@ GLint Program::getActiveAttributeMaxLength() const
 
 GLuint Program::getInputResourceIndex(const GLchar *name) const
 {
-    for (GLuint attributeIndex = 0; attributeIndex < mState.mAttributes.size(); ++attributeIndex)
-    {
-        const sh::Attribute &attribute = mState.mAttributes[attributeIndex];
-        if (attribute.name == name)
-        {
-            return attributeIndex;
-        }
-    }
-    return GL_INVALID_INDEX;
+    return GetResourceIndexFromName(mState.mAttributes, std::string(name));
 }
 
 GLuint Program::getOutputResourceIndex(const GLchar *name) const
@@ -1179,9 +1172,7 @@ void Program::getResourceName(GLuint index,
 
     if (bufSize > 0)
     {
-        std::string nameWithArray = (resource.isArray() ? resource.name + "[0]" : resource.name);
-
-        CopyStringToBuffer(name, nameWithArray, bufSize, length);
+        CopyStringToBuffer(name, resource.name, bufSize, length);
     }
 }
 
@@ -1223,21 +1214,7 @@ const sh::OutputVariable &Program::getOutputResource(GLuint index) const
 
 GLint Program::getFragDataLocation(const std::string &name) const
 {
-    std::string baseName(name);
-    unsigned int arrayIndex = ParseAndStripArrayIndex(&baseName);
-    for (auto outputPair : mState.mOutputLocations)
-    {
-        const VariableLocation &locationInfo     = outputPair.second;
-        const sh::OutputVariable &outputVariable = mState.mOutputVariables[locationInfo.index];
-        ASSERT(locationInfo.arrayIndices.size() <= 1);
-        if (outputVariable.name == baseName &&
-            (arrayIndex == GL_INVALID_INDEX || (!locationInfo.arrayIndices.empty() &&
-                                                arrayIndex == locationInfo.arrayIndices.back())))
-        {
-            return static_cast<GLint>(outputPair.first);
-        }
-    }
-    return -1;
+    return GetVariableLocation(mState.mOutputVariables, mState.mOutputLocations, name);
 }
 
 void Program::getActiveUniform(GLuint index,
@@ -1256,10 +1233,6 @@ void Program::getActiveUniform(GLuint index,
         if (bufsize > 0)
         {
             std::string string = uniform.name;
-            if (uniform.isArray())
-            {
-                string += "[0]";
-            }
             CopyStringToBuffer(name, string, bufsize, length);
         }
 
@@ -1350,7 +1323,7 @@ const LinkedUniform &Program::getUniformByIndex(GLuint index) const
 
 GLint Program::getUniformLocation(const std::string &name) const
 {
-    return mState.getUniformLocation(name);
+    return GetVariableLocation(mState.mUniforms, mState.mUniformLocations, name);
 }
 
 GLuint Program::getUniformIndex(const std::string &name) const
@@ -2075,6 +2048,11 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
     // Link attributes that have a binding location
     for (sh::Attribute &attribute : mState.mAttributes)
     {
+        // GLSL ES 3.10 January 2016 section 4.3.4: Vertex shader inputs can't be arrays or
+        // structures, so we don't need to worry about adjusting their names or generating entries
+        // for each member/element (unlike uniforms for example).
+        ASSERT(!attribute.isArray() && !attribute.isStruct());
+
         int bindingLocation = mAttributeBindings.getBinding(attribute.name);
         if (attribute.location == -1 && bindingLocation != -1)
         {
@@ -2772,18 +2750,32 @@ void Program::linkOutputVariables(const Context *context)
     {
         const sh::OutputVariable &outputVariable = mState.mOutputVariables[outputVariableIndex];
 
+        if (outputVariable.isArray())
+        {
+            // We're following the GLES 3.1 November 2016 spec section 7.3.1.1 Naming Active
+            // Resources and including [0] at the end of array variable names.
+            mState.mOutputVariables[outputVariableIndex].name += "[0]";
+            mState.mOutputVariables[outputVariableIndex].mappedName += "[0]";
+        }
+
         // Don't store outputs for gl_FragDepth, gl_FragColor, etc.
         if (outputVariable.isBuiltIn())
             continue;
 
         // Since multiple output locations must be specified, use 0 for non-specified locations.
-        int baseLocation = (outputVariable.location == -1 ? 0 : outputVariable.location);
+        unsigned int baseLocation =
+            (outputVariable.location == -1 ? 0u
+                                           : static_cast<unsigned int>(outputVariable.location));
 
         for (unsigned int elementIndex = 0; elementIndex < outputVariable.elementCount();
              elementIndex++)
         {
-            const int location = baseLocation + elementIndex;
-            ASSERT(mState.mOutputLocations.count(location) == 0);
+            const unsigned int location = baseLocation + elementIndex;
+            if (location >= mState.mOutputLocations.size())
+            {
+                mState.mOutputLocations.resize(location + 1);
+            }
+            ASSERT(!mState.mOutputLocations.at(location).used());
             if (outputVariable.isArray())
             {
                 mState.mOutputLocations[location] =
@@ -2806,7 +2798,7 @@ void Program::setUniformValuesFromBindingQualifiers()
         const auto &samplerUniform = mState.mUniforms[samplerIndex];
         if (samplerUniform.binding != -1)
         {
-            GLint location = mState.getUniformLocation(samplerUniform.name);
+            GLint location = getUniformLocation(samplerUniform.name);
             ASSERT(location != -1);
             std::vector<GLint> boundTextureUnits;
             for (unsigned int elementIndex = 0; elementIndex < samplerUniform.elementCount();
@@ -2942,9 +2934,9 @@ void Program::defineUniformBlockMembers(const std::vector<VarT> &fields,
 {
     for (const VarT &field : fields)
     {
-        const std::string &fullName = (prefix.empty() ? field.name : prefix + "." + field.name);
+        std::string fullName = (prefix.empty() ? field.name : prefix + "." + field.name);
 
-        const std::string &fullMappedName =
+        std::string fullMappedName =
             (mappedPrefix.empty() ? field.mappedName : mappedPrefix + "." + field.mappedName);
 
         if (field.isStruct())
@@ -2968,6 +2960,12 @@ void Program::defineUniformBlockMembers(const std::vector<VarT> &fields,
                 continue;
             }
 
+            if (field.isArray())
+            {
+                fullName += "[0]";
+                fullMappedName += "[0]";
+            }
+
             LinkedUniform newUniform(field.type, field.precision, fullName, field.arraySize, -1, -1,
                                      -1, blockIndex, memberInfo);
             newUniform.mappedName = fullMappedName;

src/libANGLE/Program.h

@@ -275,7 +275,7 @@ class ProgramState final : angle::NonCopyable
     unsigned int getMaxActiveAttribLocation() const { return mMaxActiveAttribLocation; }
     DrawBufferMask getActiveOutputVariables() const { return mActiveOutputVariables; }
     const std::vector<sh::OutputVariable> &getOutputVariables() const { return mOutputVariables; }
-    const std::map<int, VariableLocation> &getOutputLocations() const { return mOutputLocations; }
+    const std::vector<VariableLocation> &getOutputLocations() const { return mOutputLocations; }
     const std::vector<LinkedUniform> &getUniforms() const { return mUniforms; }
     const std::vector<VariableLocation> &getUniformLocations() const { return mUniformLocations; }
     const std::vector<InterfaceBlock> &getUniformBlocks() const { return mUniformBlocks; }
@@ -299,7 +299,6 @@ class ProgramState final : angle::NonCopyable
         return mAtomicCounterBuffers;
     }
 
-    GLint getUniformLocation(const std::string &name) const;
     GLuint getUniformIndexFromName(const std::string &name) const;
     GLuint getUniformIndexFromLocation(GLint location) const;
     Optional<GLuint> getSamplerIndex(GLint location) const;
@@ -340,7 +339,12 @@ class ProgramState final : angle::NonCopyable
     //  4. Atomic counter uniforms
     //  5. Uniform block uniforms
     // This makes opaque uniform validation easier, since we don't need a separate list.
+    // For generating the entries and naming them we follow the spec: GLES 3.1 November 2016 section
+    // 7.3.1.1 Naming Active Resources. There's a separate entry for each struct member and each
+    // inner array of an array of arrays. Names and mapped names of uniforms that are arrays include
+    // [0] in the end. This makes implementation of queries simpler.
     std::vector<LinkedUniform> mUniforms;
+
     std::vector<VariableLocation> mUniformLocations;
     std::vector<InterfaceBlock> mUniformBlocks;
     std::vector<InterfaceBlock> mShaderStorageBlocks;
@@ -355,8 +359,10 @@ class ProgramState final : angle::NonCopyable
     // An array of the images that are used by the program
     std::vector<gl::ImageBinding> mImageBindings;
 
+    // Names and mapped names of output variables that are arrays include [0] in the end, similarly
+    // to uniforms.
     std::vector<sh::OutputVariable> mOutputVariables;
-    std::map<int, VariableLocation> mOutputLocations;
+    std::vector<VariableLocation> mOutputLocations;
     DrawBufferMask mActiveOutputVariables;
 
     // Fragment output variable base types: FLOAT, INT, or UINT.  Ordered by location.

src/libANGLE/UniformLinker.cpp

@@ -10,6 +10,7 @@
 
 #include "libANGLE/UniformLinker.h"
 
+#include "common/string_utils.h"
 #include "common/utilities.h"
 #include "libANGLE/Caps.h"
 #include "libANGLE/Context.h"
@@ -33,7 +34,21 @@ LinkedUniform *FindUniform(std::vector<LinkedUniform> &list, const std::string &
     return nullptr;
 }
 
-}  // anonymouse namespace
+int GetUniformLocationBinding(const Program::Bindings &uniformLocationBindings,
+                              const sh::Uniform &uniform)
+{
+    int binding = uniformLocationBindings.getBinding(uniform.name);
+    if (uniform.isArray() && binding == -1)
+    {
+        // Bindings for array uniforms can be set either with or without [0] in the end.
+        ASSERT(angle::EndsWith(uniform.name, "[0]"));
+        std::string nameWithoutIndex = uniform.name.substr(0u, uniform.name.length() - 3u);
+        return uniformLocationBindings.getBinding(nameWithoutIndex);
+    }
+    return binding;
+}
+
+}  // anonymous namespace
 
 UniformLinker::UniformLinker(const ProgramState &state) : mState(state)
 {
@@ -191,7 +206,7 @@ bool UniformLinker::indexUniforms(InfoLog &infoLog,
             continue;
         }
 
-        int preSetLocation = uniformLocationBindings.getBinding(uniform.name);
+        int preSetLocation = GetUniformLocationBinding(uniformLocationBindings, uniform);
         int shaderLocation = uniform.location;
 
         if (shaderLocation != -1)
@@ -264,7 +279,7 @@ bool UniformLinker::gatherUniformLocationsAndCheckConflicts(
             continue;
         }
 
-        int apiBoundLocation = uniformLocationBindings.getBinding(uniform.name);
+        int apiBoundLocation = GetUniformLocationBinding(uniformLocationBindings, uniform);
         int shaderLocation   = uniform.location;
 
         if (shaderLocation != -1)
@@ -518,7 +533,19 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenUniformImpl(
     {
         uniformList = atomicCounterUniforms;
     }
-    LinkedUniform *existingUniform = FindUniform(*uniformList, fullName);
+
+    std::string fullNameWithArrayIndex(fullName);
+    std::string fullMappedNameWithArrayIndex(fullMappedName);
+
+    if (uniform.isArray())
+    {
+        // We're following the GLES 3.1 November 2016 spec section 7.3.1.1 Naming Active Resources
+        // and including [0] at the end of array variable names.
+        fullNameWithArrayIndex += "[0]";
+        fullMappedNameWithArrayIndex += "[0]";
+    }
+
+    LinkedUniform *existingUniform = FindUniform(*uniformList, fullNameWithArrayIndex);
     if (existingUniform)
     {
         if (binding != -1)
@@ -541,10 +568,10 @@ UniformLinker::ShaderUniformCount UniformLinker::flattenUniformImpl(
     }
     else
     {
-        LinkedUniform linkedUniform(uniform.type, uniform.precision, fullName, uniform.arraySize,
-                                    binding, offset, *location, -1,
+        LinkedUniform linkedUniform(uniform.type, uniform.precision, fullNameWithArrayIndex,
+                                    uniform.arraySize, binding, offset, *location, -1,
                                     sh::BlockMemberInfo::getDefaultBlockInfo());
-        linkedUniform.mappedName = fullMappedName;
+        linkedUniform.mappedName = fullMappedNameWithArrayIndex;
         linkedUniform.staticUse = markStaticUse;
         if (markStaticUse)
         {

src/libANGLE/queryutils.cpp

@@ -453,15 +453,8 @@ GLint GetLocationVariableProperty(const sh::VariableWithLocation &var, GLenum pr
 
         case GL_NAME_LENGTH:
         {
-            size_t length = var.name.size();
-            if (var.isArray())
-            {
-                // Counts "[0]".
-                length += 3;
-            }
             // ES31 spec p84: This counts the terminating null char.
-            ++length;
-            return clampCast<GLint>(length);
+            return clampCast<GLint>(var.name.size() + 1u);
         }
 
         case GL_LOCATION:
@@ -1471,7 +1464,7 @@ GLint QueryProgramResourceLocation(const Program *program,
             return program->getFragDataLocation(name);
 
         case GL_UNIFORM:
-            return program->getState().getUniformLocation(name);
+            return program->getUniformLocation(name);
 
         default:
             UNREACHABLE();

src/libANGLE/renderer/d3d/DynamicHLSL.cpp

@@ -1265,9 +1265,15 @@ void DynamicHLSL::getPixelShaderOutputKey(const gl::ContextState &data,
         const auto &shaderOutputVars =
             metadata.getFragmentShader()->getData().getActiveOutputVariables();
 
-        for (auto outputPair : programData.getOutputLocations())
+        for (size_t outputLocationIndex = 0u;
+             outputLocationIndex < programData.getOutputLocations().size(); ++outputLocationIndex)
         {
-            const VariableLocation &outputLocation   = outputPair.second;
+            const VariableLocation &outputLocation =
+                programData.getOutputLocations().at(outputLocationIndex);
+            if (!outputLocation.used())
+            {
+                continue;
+            }
             const sh::ShaderVariable &outputVariable = shaderOutputVars[outputLocation.index];
             const std::string &variableName          = "out_" + outputVariable.name;
 
@@ -1283,7 +1289,7 @@ void DynamicHLSL::getPixelShaderOutputKey(const gl::ContextState &data,
             outputKeyVariable.type        = outputVariable.type;
             outputKeyVariable.name        = variableName + elementString;
             outputKeyVariable.source = variableName + ArrayIndexString(outputLocation.arrayIndices);
-            outputKeyVariable.outputIndex = outputPair.first;
+            outputKeyVariable.outputIndex = outputLocationIndex;
 
             outPixelShaderKey->push_back(outputKeyVariable);
         }

src/libANGLE/renderer/d3d/ProgramD3D.cpp

@@ -9,6 +9,7 @@
 #include "libANGLE/renderer/d3d/ProgramD3D.h"
 
 #include "common/bitset_utils.h"
+#include "common/string_utils.h"
 #include "common/utilities.h"
 #include "libANGLE/Context.h"
 #include "libANGLE/Framebuffer.h"
@@ -1979,7 +1980,17 @@ void ProgramD3D::defineUniformsAndAssignRegisters(const gl::Context *context)
         if (!glUniform.isInDefaultBlock())
             continue;
 
-        auto mapEntry = uniformMap.find(glUniform.name);
+        std::string name = glUniform.name;
+        if (glUniform.isArray())
+        {
+            // In the program state, array uniform names include [0] as in the program resource
+            // spec. Here we don't include it.
+            // TODO(oetuaho@nvidia.com): consider using the same uniform naming here as in the GL
+            // layer.
+            ASSERT(angle::EndsWith(name, "[0]"));
+            name.resize(name.length() - 3);
+        }
+        auto mapEntry = uniformMap.find(name);
         ASSERT(mapEntry != uniformMap.end());
         mD3DUniforms.push_back(mapEntry->second);
     }
@@ -2588,8 +2599,9 @@ bool ProgramD3D::getUniformBlockSize(const std::string &blockName,
                                      const std::string & /* blockMappedName */,
                                      size_t *sizeOut) const
 {
-    std::string baseName = blockName;
-    gl::ParseAndStripArrayIndex(&baseName);
+    size_t nameLengthWithoutArrayIndex;
+    gl::ParseArrayIndex(blockName, &nameLengthWithoutArrayIndex);
+    std::string baseName = blockName.substr(0u, nameLengthWithoutArrayIndex);
 
     auto sizeIter = mBlockDataSizes.find(baseName);
     if (sizeIter == mBlockDataSizes.end())

src/libANGLE/renderer/d3d/ProgramD3D.h

@@ -50,7 +50,7 @@ struct D3DUniform : private angle::NonCopyable
 
     // Duplicated from the GL layer
     const gl::UniformTypeInfo &typeInfo;
-    std::string name;
+    std::string name;  // Names of arrays don't include [0], unlike at the GL layer.
     unsigned int arraySize;
 
     // Pointer to a system copies of the data. Separate pointers for each uniform storage type.

src/libANGLE/renderer/gl/ProgramGL.cpp

@@ -663,14 +663,15 @@ void ProgramGL::postLink()
         // "Locations for sequential array indices are not required to be sequential."
         const gl::LinkedUniform &uniform = uniforms[entry.index];
         std::stringstream fullNameStr;
-        fullNameStr << uniform.mappedName;
         if (uniform.isArray())
         {
-            for (auto arrayElementIndexIt = entry.arrayIndices.rbegin();
-                 arrayElementIndexIt != entry.arrayIndices.rend(); ++arrayElementIndexIt)
-            {
-                fullNameStr << "[" << (*arrayElementIndexIt) << "]";
-            }
+            ASSERT(angle::EndsWith(uniform.mappedName, "[0]"));
+            fullNameStr << uniform.mappedName.substr(0, uniform.mappedName.length() - 3);
+            fullNameStr << "[" << entry.arrayIndices[0] << "]";
+        }
+        else
+        {
+            fullNameStr << uniform.mappedName;
         }
         const std::string &fullName = fullNameStr.str();