Add support for arrays of arrays to VariableLocation

src/common/angleutils.cpp

@@ -17,6 +17,34 @@ namespace angle
 const uintptr_t DirtyPointer = std::numeric_limits<uintptr_t>::max();
 }
 
+std::string ArrayString(unsigned int i)
+{
+    // We assume that UINT_MAX and GL_INVALID_INDEX are equal.
+    ASSERT(i != UINT_MAX);
+
+    std::stringstream strstr;
+    strstr << "[";
+    strstr << i;
+    strstr << "]";
+    return strstr.str();
+}
+
+std::string ArrayIndexString(const std::vector<unsigned int> &indices)
+{
+    std::stringstream strstr;
+
+    for (auto indicesIt = indices.rbegin(); indicesIt != indices.rend(); ++indicesIt)
+    {
+        // We assume that UINT_MAX and GL_INVALID_INDEX are equal.
+        ASSERT(*indicesIt != UINT_MAX);
+        strstr << "[";
+        strstr << (*indicesIt);
+        strstr << "]";
+    }
+
+    return strstr.str();
+}
+
 size_t FormatStringIntoVector(const char *fmt, va_list vararg, std::vector<char>& outBuffer)
 {
     // The state of the va_list passed to vsnprintf is undefined after the call, do a copy in case

src/common/angleutils.h

@@ -160,23 +160,11 @@ inline const char* MakeStaticString(const std::string &str)
     return strings.insert(str).first->c_str();
 }
 
-inline std::string ArrayString(unsigned int i)
-{
-    // We assume UINT_MAX and GL_INVALID_INDEX are equal
-    // See DynamicHLSL.cpp
-    if (i == UINT_MAX)
-    {
-        return "";
-    }
-
-    std::stringstream strstr;
+std::string ArrayString(unsigned int i);
 
-    strstr << "[";
-    strstr << i;
-    strstr << "]";
-
-    return strstr.str();
-}
+// Indices are stored in vectors with the outermost index in the back. In the output of the function
+// the indices are reversed.
+std::string ArrayIndexString(const std::vector<unsigned int> &indices);
 
 inline std::string Str(int i)
 {

src/common/angleutils_unittest.cpp

+//
+// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+
+// angleutils_unittest.cpp: Unit tests for ANGLE's common utilities.
+
+#include "gtest/gtest.h"
+
+#include "common/angleutils.h"
+
+namespace
+{
+
+// Test that multiple array indices are written out in the right order.
+TEST(ArrayIndexString, MultipleArrayIndices)
+{
+    std::vector<unsigned int> indices;
+    indices.push_back(12);
+    indices.push_back(34);
+    indices.push_back(56);
+    EXPECT_EQ("[56][34][12]", ArrayIndexString(indices));
+}
+
+}  // anonymous namespace

src/common/utilities.cpp

@@ -735,35 +735,39 @@ int VariableSortOrder(GLenum type)
     }
 }
 
-std::string ParseResourceName(const std::string &name, size_t *outSubscript)
+std::string ParseResourceName(const std::string &name, std::vector<unsigned int> *outSubscripts)
 {
-    // Strip any trailing array operator and retrieve the subscript
-    size_t open = name.find_last_of('[');
-    size_t close = name.find_last_of(']');
-    bool hasIndex = (open != std::string::npos) && (close == name.length() - 1);
-    if (!hasIndex)
+    if (outSubscripts)
     {
-        if (outSubscript)
-        {
-            *outSubscript = GL_INVALID_INDEX;
-        }
-        return name;
+        outSubscripts->clear();
     }
-
-    if (outSubscript)
+    // Strip any trailing array indexing operators and retrieve the subscripts.
+    size_t baseNameLength = name.length();
+    bool hasIndex         = true;
+    while (hasIndex)
     {
-        int index = atoi(name.substr(open + 1).c_str());
-        if (index >= 0)
-        {
-            *outSubscript = index;
-        }
-        else
+        size_t open  = name.find_last_of('[', baseNameLength - 1);
+        size_t close = name.find_last_of(']', baseNameLength - 1);
+        hasIndex     = (open != std::string::npos) && (close == baseNameLength - 1);
+        if (hasIndex)
         {
-            *outSubscript = GL_INVALID_INDEX;
+            baseNameLength = open;
+            if (outSubscripts)
+            {
+                int index = atoi(name.substr(open + 1).c_str());
+                if (index >= 0)
+                {
+                    outSubscripts->push_back(index);
+                }
+                else
+                {
+                    outSubscripts->push_back(GL_INVALID_INDEX);
+                }
+            }
         }
     }
 
-    return name.substr(0, open);
+    return name.substr(0, baseNameLength);
 }
 
 unsigned int ParseAndStripArrayIndex(std::string *name)

src/common/utilities.h

@@ -12,9 +12,10 @@
 #include <EGL/egl.h>
 #include <EGL/eglext.h>
 
-#include "angle_gl.h"
-#include <string>
 #include <math.h>
+#include <string>
+#include <vector>
+#include "angle_gl.h"
 
 #include "common/mathutil.h"
 
@@ -49,10 +50,12 @@ bool IsCubeMapTextureTarget(GLenum target);
 size_t CubeMapTextureTargetToLayerIndex(GLenum target);
 GLenum LayerIndexToCubeMapTextureTarget(size_t index);
 
-// Parse the base resource name and array index.  Returns the base name of the resource.
-// outSubscript is set to GL_INVALID_INDEX if the provided name is not an array or the array index
-// is invalid.
-std::string ParseResourceName(const std::string &name, size_t *outSubscript);
+// Parse the base resource name and array indices. Returns the base name of the resource.
+// If the provided name doesn't index an array, the outSubscripts vector will be empty.
+// If the provided name indexes an array, the outSubscripts vector will contain indices with
+// outermost array indices in the back. If an array index is invalid, GL_INVALID_INDEX is added to
+// outSubscripts.
+std::string ParseResourceName(const std::string &name, std::vector<unsigned int> *outSubscripts);
 
 // Find the range of index values in the provided indices pointer.  Primitive restart indices are
 // only counted in the range if primitive restart is disabled.

src/common/utilities_unittest.cpp

@@ -13,43 +13,66 @@
 namespace
 {
 
+// Test parsing valid single array indices
 TEST(ParseResourceName, ArrayIndex)
 {
-    size_t index;
-    EXPECT_EQ("foo", gl::ParseResourceName("foo[123]", &index));
-    EXPECT_EQ(123u, index);
+    std::vector<unsigned int> indices;
+    EXPECT_EQ("foo", gl::ParseResourceName("foo[123]", &indices));
+    ASSERT_EQ(1u, indices.size());
+    EXPECT_EQ(123u, indices[0]);
 
-    EXPECT_EQ("bar", gl::ParseResourceName("bar[0]", &index));
-    EXPECT_EQ(0u, index);
+    EXPECT_EQ("bar", gl::ParseResourceName("bar[0]", &indices));
+    ASSERT_EQ(1u, indices.size());
+    EXPECT_EQ(0u, indices[0]);
 }
 
+// Parsing a negative array index should result in INVALID_INDEX.
 TEST(ParseResourceName, NegativeArrayIndex)
 {
-    size_t index;
-    EXPECT_EQ("foo", gl::ParseResourceName("foo[-1]", &index));
-    EXPECT_EQ(GL_INVALID_INDEX, index);
+    std::vector<unsigned int> indices;
+    EXPECT_EQ("foo", gl::ParseResourceName("foo[-1]", &indices));
+    ASSERT_EQ(1u, indices.size());
+    EXPECT_EQ(GL_INVALID_INDEX, indices.back());
 }
 
+// Parsing no array indices should result in an empty array.
 TEST(ParseResourceName, NoArrayIndex)
 {
-    size_t index;
-    EXPECT_EQ("foo", gl::ParseResourceName("foo", &index));
-    EXPECT_EQ(GL_INVALID_INDEX, index);
+    std::vector<unsigned int> indices;
+    EXPECT_EQ("foo", gl::ParseResourceName("foo", &indices));
+    EXPECT_TRUE(indices.empty());
 }
 
-TEST(ParseResourceName, NULLArrayIndex)
+// The ParseResourceName function should work when a nullptr is passed as the indices output vector.
+TEST(ParseResourceName, NULLArrayIndices)
 {
     EXPECT_EQ("foo", gl::ParseResourceName("foo[10]", nullptr));
 }
 
+// Parsing multiple array indices should result in outermost array indices being last in the vector.
+TEST(ParseResourceName, MultipleArrayIndices)
+{
+    std::vector<unsigned int> indices;
+    EXPECT_EQ("foo", gl::ParseResourceName("foo[12][34][56]", &indices));
+    ASSERT_EQ(3u, indices.size());
+    // Indices are sorted with outermost array index last.
+    EXPECT_EQ(56u, indices[0]);
+    EXPECT_EQ(34u, indices[1]);
+    EXPECT_EQ(12u, indices[2]);
+}
+
+// Trailing whitespace should not be accepted by ParseResourceName.
 TEST(ParseResourceName, TrailingWhitespace)
 {
-    size_t index;
-    EXPECT_EQ("foo ", gl::ParseResourceName("foo ", &index));
-    EXPECT_EQ(GL_INVALID_INDEX, index);
+    std::vector<unsigned int> indices;
+    EXPECT_EQ("foo ", gl::ParseResourceName("foo ", &indices));
+    EXPECT_TRUE(indices.empty());
+
+    EXPECT_EQ("foo[10] ", gl::ParseResourceName("foo[10] ", &indices));
+    EXPECT_TRUE(indices.empty());
 
-    EXPECT_EQ("foo[10] ", gl::ParseResourceName("foo[10] ", &index));
-    EXPECT_EQ(GL_INVALID_INDEX, index);
+    EXPECT_EQ("foo[10][20] ", gl::ParseResourceName("foo[10][20] ", &indices));
+    EXPECT_TRUE(indices.empty());
 }
 
 }

src/libANGLE/BinaryStream.h

@@ -46,6 +46,16 @@ class BinaryInputStream : angle::NonCopyable
         *outValue = readInt<IntT>();
     }
 
+    template <class IntT, class VectorElementT>
+    void readIntVector(std::vector<VectorElementT> *param)
+    {
+        unsigned int size = readInt<unsigned int>();
+        for (unsigned int index = 0; index < size; ++index)
+        {
+            param->push_back(readInt<IntT>());
+        }
+    }
+
     bool readBool()
     {
         int value = 0;
@@ -197,6 +207,16 @@ class BinaryOutputStream : angle::NonCopyable
         }
     }
 
+    template <class IntT>
+    void writeIntVector(std::vector<IntT> param)
+    {
+        writeInt(param.size());
+        for (IntT element : param)
+        {
+            writeIntOrNegOne(element);
+        }
+    }
+
     void writeString(const std::string &v)
     {
         writeInt(v.length());

src/libANGLE/MemoryProgramCache.cpp

@@ -238,8 +238,9 @@ LinkResult MemoryProgramCache::Deserialize(const Context *context,
          uniformIndexIndex++)
     {
         VariableLocation variable;
-        stream.readInt(&variable.element);
+        stream.readIntVector<unsigned int>(&variable.arrayIndices);
         stream.readInt(&variable.index);
+        stream.readInt(&variable.flattenedArrayOffset);
         stream.readBool(&variable.ignored);
 
         state->mUniformLocations.push_back(variable);
@@ -319,8 +320,9 @@ LinkResult MemoryProgramCache::Deserialize(const Context *context,
     {
         int locationIndex = stream.readInt<int>();
         VariableLocation locationData;
-        stream.readInt(&locationData.element);
+        stream.readIntVector<unsigned int>(&locationData.arrayIndices);
         stream.readInt(&locationData.index);
+        stream.readInt(&locationData.flattenedArrayOffset);
         stream.readBool(&locationData.ignored);
         state->mOutputLocations[locationIndex] = locationData;
     }
@@ -427,8 +429,9 @@ void MemoryProgramCache::Serialize(const Context *context,
     stream.writeInt(state.getUniformLocations().size());
     for (const auto &variable : state.getUniformLocations())
     {
-        stream.writeInt(variable.element);
+        stream.writeIntVector(variable.arrayIndices);
         stream.writeIntOrNegOne(variable.index);
+        stream.writeInt(variable.flattenedArrayOffset);
         stream.writeInt(variable.ignored);
     }
 
@@ -481,8 +484,9 @@ void MemoryProgramCache::Serialize(const Context *context,
     for (const auto &outputPair : state.getOutputLocations())
     {
         stream.writeInt(outputPair.first);
-        stream.writeIntOrNegOne(outputPair.second.element);
+        stream.writeIntVector(outputPair.second.arrayIndices);
         stream.writeIntOrNegOne(outputPair.second.index);
+        stream.writeInt(outputPair.second.flattenedArrayOffset);
         stream.writeInt(outputPair.second.ignored);
     }
 

src/libANGLE/Program.h

@@ -138,7 +138,7 @@ struct VariableLocation
     static constexpr unsigned int kUnused = GL_INVALID_INDEX;
 
     VariableLocation();
-    VariableLocation(unsigned int element, unsigned int index);
+    VariableLocation(unsigned int arrayIndex, unsigned int index);
 
     // If used is false, it means this location is only used to fill an empty space in an array,
     // and there is no corresponding uniform variable for this location. It can also mean the
@@ -147,9 +147,18 @@ struct VariableLocation
     void markUnused() { index = kUnused; }
     void markIgnored() { ignored = true; }
 
-    unsigned int element;
+    bool areAllArrayIndicesZero() const;
+
+    // The "arrayIndices" vector stores indices for the GLSL array. "index" is an index of the
+    // location.
+    std::vector<unsigned int> arrayIndices;  // Outermost array indices are in the back.
     unsigned int index;
 
+    unsigned int flattenedArrayOffset;  // For non-nested arrays this is the same as the array
+                                        // index. For arrays of arrays, the indices are converted to
+                                        // a single offset inside a one-dimensional array made up of
+                                        // the elements of the innermost arrays.
+
     // If this location was bound to an unreferenced uniform.  Setting data on this uniform is a
     // no-op.
     bool ignored;

src/libANGLE/VaryingPacking.cpp

@@ -262,8 +262,7 @@ bool VaryingPacking::packUserVaryings(gl::InfoLog &infoLog,
     // Make sure transform feedback varyings aren't optimized out.
     for (const std::string &transformFeedbackVaryingName : transformFeedbackVaryings)
     {
-        size_t subscript              = GL_INVALID_INDEX;
-        std::string tfVaryingBaseName = ParseResourceName(transformFeedbackVaryingName, &subscript);
+        std::string tfVaryingBaseName = ParseResourceName(transformFeedbackVaryingName, nullptr);
 
         bool found = (uniqueVaryingNames.count(transformFeedbackVaryingName) > 0 ||
                       uniqueVaryingNames.count(tfVaryingBaseName) > 0);

src/libANGLE/renderer/d3d/DynamicHLSL.cpp

@@ -1270,15 +1270,19 @@ void DynamicHLSL::getPixelShaderOutputKey(const gl::ContextState &data,
             const VariableLocation &outputLocation   = outputPair.second;
             const sh::ShaderVariable &outputVariable = shaderOutputVars[outputLocation.index];
             const std::string &variableName          = "out_" + outputVariable.name;
+
+            // Fragment outputs can't be arrays of arrays. ESSL 3.10 section 4.3.6.
+            ASSERT(outputLocation.arrayIndices.size() <= 1u);
             const std::string &elementString =
-                (outputLocation.element == GL_INVALID_INDEX ? "" : Str(outputLocation.element));
+                (outputLocation.arrayIndices.empty() ? ""
+                                                     : Str(outputLocation.arrayIndices.back()));
 
             ASSERT(outputVariable.staticUse);
 
             PixelShaderOutputVariable outputKeyVariable;
             outputKeyVariable.type        = outputVariable.type;
             outputKeyVariable.name        = variableName + elementString;
-            outputKeyVariable.source      = variableName + ArrayString(outputLocation.element);
+            outputKeyVariable.source = variableName + ArrayIndexString(outputLocation.arrayIndices);
             outputKeyVariable.outputIndex = outputPair.first;
 
             outPixelShaderKey->push_back(outputKeyVariable);

src/libANGLE/renderer/d3d/ProgramD3D.cpp

@@ -2169,11 +2169,11 @@ void ProgramD3D::setUniformImpl(const gl::VariableLocation &locationInfo,
 {
     D3DUniform *targetUniform = mD3DUniforms[locationInfo.index];
     const int components      = targetUniform->typeInfo.componentCount;
-    unsigned int arrayElement = locationInfo.element;
+    unsigned int arrayElementOffset = locationInfo.flattenedArrayOffset;
 
     if (targetUniform->typeInfo.type == uniformType)
     {
-        T *dest         = reinterpret_cast<T *>(targetData) + arrayElement * 4;
+        T *dest         = reinterpret_cast<T *>(targetData) + arrayElementOffset * 4;
         const T *source = v;
 
         for (GLint i = 0; i < count; i++, dest += 4, source += components)
@@ -2184,7 +2184,7 @@ void ProgramD3D::setUniformImpl(const gl::VariableLocation &locationInfo,
     else
     {
         ASSERT(targetUniform->typeInfo.type == gl::VariableBoolVectorType(uniformType));
-        GLint *boolParams = reinterpret_cast<GLint *>(targetData) + arrayElement * 4;
+        GLint *boolParams = reinterpret_cast<GLint *>(targetData) + arrayElementOffset * 4;
 
         for (GLint i = 0; i < count; i++)
         {
@@ -2209,7 +2209,7 @@ void ProgramD3D::setUniformInternal(GLint location, GLsizei count, const T *v, G
     {
         ASSERT(uniformType == GL_INT);
         size_t size = count * sizeof(T);
-        auto dest   = &targetUniform->mSamplerData[locationInfo.element];
+        auto dest   = &targetUniform->mSamplerData[locationInfo.flattenedArrayOffset];
         if (memcmp(dest, v, size) != 0)
         {
             memcpy(dest, v, size);
@@ -2248,12 +2248,13 @@ bool ProgramD3D::setUniformMatrixfvImpl(GLint location,
     D3DUniform *targetUniform = getD3DUniformFromLocation(location);
 
     unsigned int elementCount = targetUniform->elementCount();
-    unsigned int arrayElement = mState.getUniformLocations()[location].element;
-    unsigned int count        = std::min(elementCount - arrayElement, static_cast<unsigned int>(countIn));
+    unsigned int arrayElementOffset = mState.getUniformLocations()[location].flattenedArrayOffset;
+    unsigned int count =
+        std::min(elementCount - arrayElementOffset, static_cast<unsigned int>(countIn));
 
     const unsigned int targetMatrixStride = (4 * rows);
-    GLfloat *target = reinterpret_cast<GLfloat *>(targetData + arrayElement * sizeof(GLfloat) *
-                                                                   targetMatrixStride);
+    GLfloat *target                       = reinterpret_cast<GLfloat *>(
+        targetData + arrayElementOffset * sizeof(GLfloat) * targetMatrixStride);
 
     bool dirty = false;
 
@@ -2586,8 +2587,13 @@ void ProgramD3D::gatherTransformFeedbackVaryings(const gl::VaryingPacking &varyi
         }
         else
         {
+            std::vector<unsigned int> subscripts;
+            std::string baseName = gl::ParseResourceName(tfVaryingName, &subscripts);
             size_t subscript     = GL_INVALID_INDEX;
-            std::string baseName = gl::ParseResourceName(tfVaryingName, &subscript);
+            if (!subscripts.empty())
+            {
+                subscript = subscripts.back();
+            }
             for (const auto &registerInfo : varyingPacking.getRegisterList())
             {
                 const auto &varying   = *registerInfo.packedVarying->varying;
@@ -2694,7 +2700,8 @@ void ProgramD3D::getUniformInternal(GLint location, DestT *dataOut) const
     const gl::LinkedUniform &uniform         = mState.getUniforms()[locationInfo.index];
 
     const D3DUniform *targetUniform = getD3DUniformFromLocation(location);
-    const uint8_t *srcPointer       = targetUniform->getDataPtrToElement(locationInfo.element);
+    const uint8_t *srcPointer       = targetUniform->getDataPtrToElement(
+        locationInfo.arrayIndices.empty() ? 0u : locationInfo.flattenedArrayOffset);
 
     if (gl::IsMatrixType(uniform.type))
     {

src/libANGLE/renderer/gl/ProgramGL.cpp

@@ -659,14 +659,18 @@ void ProgramGL::postLink()
             continue;
         }
 
-        // From the spec:
+        // From the GLES 3.0.5 spec:
         // "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())
         {
-            fullNameStr << "[" << entry.element << "]";
+            for (auto arrayElementIndexIt = entry.arrayIndices.rbegin();
+                 arrayElementIndexIt != entry.arrayIndices.rend(); ++arrayElementIndexIt)
+            {
+                fullNameStr << "[" << (*arrayElementIndexIt) << "]";
+            }
         }
         const std::string &fullName = fullNameStr.str();
 

src/tests/angle_unittests.gypi

@@ -15,6 +15,7 @@
         'angle_unittests_sources':
         [
             '<(angle_path)/src/common/Optional_unittest.cpp',
+            '<(angle_path)/src/common/angleutils_unittest.cpp',
             '<(angle_path)/src/common/bitset_utils_unittest.cpp',
             '<(angle_path)/src/common/mathutil_unittest.cpp',
             '<(angle_path)/src/common/matrix_utils_unittest.cpp',