Vulkan: Redo RewriteStructSamplers

This transformation is split into two.  The first transformation solely
takes out the samplers out of structs, and potentially generates array
of array of samplers.  A second transformation is added that takes any
array of array of opaque uniforms and flattens it.

A follow up change will simplify RewriteAtomicCounters which also
handles array of arrays (which is no longer possible), and removes
dependency on shaderStorageBufferArrayDynamicIndexing.

Bug: angleproject:2703
Bug: angleproject:3881
Bug: angleproject:4071
Bug: angleproject:4211
Change-Id: I352bb2bbe65ac49f4d7d753c0ba3160fa3cc925a
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2628138
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

include/GLSLANG/ShaderLang.h

@@ -305,9 +305,7 @@ const ShCompileOptions SH_TAKE_VIDEO_TEXTURE_AS_EXTERNAL_OES = UINT64_C(1) << 45
 // If requested, validates the AST after every transformation.  Useful for debugging.
 const ShCompileOptions SH_VALIDATE_AST = UINT64_C(1) << 46;
 
-// Use old version of RewriteStructSamplers, which doesn't produce as many
-// sampler arrays in parameters. This causes a few tests to pass on Android.
-const ShCompileOptions SH_USE_OLD_REWRITE_STRUCT_SAMPLERS = UINT64_C(1) << 47;
+// Bit 47 is available
 
 // This flag works around a inconsistent behavior in Mac AMD driver where gl_VertexID doesn't
 // include base vertex value. It replaces gl_VertexID with (gl_VertexID + angle_BaseVertex)

include/platform/FeaturesVk.h

@@ -255,14 +255,6 @@ struct FeaturesVk : FeatureSetBase
         "Seamful cube map emulation misbehaves on some drivers, so it's disallowed", &members,
         "http://anglebug.com/3243"};
 
-    // Qualcomm and SwiftShader shader compiler doesn't support sampler arrays as parameters, so
-    // revert to old RewriteStructSamplers behavior, which produces fewer.
-    Feature forceOldRewriteStructSamplers = {
-        "forceOldRewriteStructSamplers", FeatureCategory::VulkanWorkarounds,
-        "Some shader compilers don't support sampler arrays as parameters, so revert to old "
-        "RewriteStructSamplers behavior, which produces fewer.",
-        &members, "http://anglebug.com/2703"};
-
     // Vulkan considers vertex attribute accesses to count up to the last multiple of the stride.
     // This additional access supports AMD's robust buffer access implementation.
     // AMDVLK in particular will return incorrect values when the vertex access extends into the

src/compiler.gni

@@ -321,6 +321,8 @@ angle_translator_lib_vulkan_sources = [
   "src/compiler/translator/tree_ops/vulkan/RemoveAtomicCounterBuiltins.h",
   "src/compiler/translator/tree_ops/vulkan/RemoveInactiveInterfaceVariables.cpp",
   "src/compiler/translator/tree_ops/vulkan/RemoveInactiveInterfaceVariables.h",
+  "src/compiler/translator/tree_ops/vulkan/RewriteArrayOfArrayOfOpaqueUniforms.cpp",
+  "src/compiler/translator/tree_ops/vulkan/RewriteArrayOfArrayOfOpaqueUniforms.h",
   "src/compiler/translator/tree_ops/vulkan/RewriteAtomicCounters.cpp",
   "src/compiler/translator/tree_ops/vulkan/RewriteAtomicCounters.h",
   "src/compiler/translator/tree_ops/vulkan/RewriteCubeMapSamplersAs2DArray.cpp",
@@ -331,7 +333,6 @@ angle_translator_lib_vulkan_sources = [
   "src/compiler/translator/tree_ops/vulkan/RewriteInterpolateAtOffset.h",
   "src/compiler/translator/tree_ops/vulkan/RewriteStructSamplers.cpp",
   "src/compiler/translator/tree_ops/vulkan/RewriteStructSamplers.h",
-  "src/compiler/translator/tree_ops/vulkan/RewriteStructSamplersOld.cpp",
 ]
 if (is_android) {
   angle_translator_sources += [

src/compiler/fuzz/translator_fuzzer.cpp

@@ -56,10 +56,9 @@ constexpr ShCompileOptions kGLSLMacOnlyOptions =
 
 // Options supported by Vulkan GLSL only
 constexpr ShCompileOptions kVulkanGLSLOnlyOptions =
-    SH_ADD_PRE_ROTATION | SH_USE_OLD_REWRITE_STRUCT_SAMPLERS |
-    SH_EMULATE_SEAMFUL_CUBE_MAP_SAMPLING | SH_ADD_BRESENHAM_LINE_RASTER_EMULATION |
-    SH_EARLY_FRAGMENT_TESTS_OPTIMIZATION | SH_USE_SPECIALIZATION_CONSTANT |
-    SH_ADD_VULKAN_XFB_EMULATION_SUPPORT_CODE;
+    SH_ADD_PRE_ROTATION | SH_EMULATE_SEAMFUL_CUBE_MAP_SAMPLING |
+    SH_ADD_BRESENHAM_LINE_RASTER_EMULATION | SH_EARLY_FRAGMENT_TESTS_OPTIMIZATION |
+    SH_USE_SPECIALIZATION_CONSTANT | SH_ADD_VULKAN_XFB_EMULATION_SUPPORT_CODE;
 
 // Options supported by HLSL output only
 constexpr ShCompileOptions kHLSLOnlyOptions = SH_EXPAND_SELECT_HLSL_INTEGER_POW_EXPRESSIONS |

src/compiler/translator/TranslatorVulkan.cpp

@@ -24,6 +24,7 @@
 #include "compiler/translator/tree_ops/vulkan/NameEmbeddedUniformStructs.h"
 #include "compiler/translator/tree_ops/vulkan/RemoveAtomicCounterBuiltins.h"
 #include "compiler/translator/tree_ops/vulkan/RemoveInactiveInterfaceVariables.h"
+#include "compiler/translator/tree_ops/vulkan/RewriteArrayOfArrayOfOpaqueUniforms.h"
 #include "compiler/translator/tree_ops/vulkan/RewriteAtomicCounters.h"
 #include "compiler/translator/tree_ops/vulkan/RewriteCubeMapSamplersAs2DArray.h"
 #include "compiler/translator/tree_ops/vulkan/RewriteDfdy.h"
@@ -812,21 +813,9 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
             return false;
         }
 
-        bool rewriteStructSamplersResult;
         int removedUniformsCount;
 
-        if (compileOptions & SH_USE_OLD_REWRITE_STRUCT_SAMPLERS)
-        {
-            rewriteStructSamplersResult =
-                RewriteStructSamplersOld(this, root, &getSymbolTable(), &removedUniformsCount);
-        }
-        else
-        {
-            rewriteStructSamplersResult =
-                RewriteStructSamplers(this, root, &getSymbolTable(), &removedUniformsCount);
-        }
-
-        if (!rewriteStructSamplersResult)
+        if (!RewriteStructSamplers(this, root, &getSymbolTable(), &removedUniformsCount))
         {
             return false;
         }
@@ -841,6 +830,15 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
         }
     }
 
+    // Replace array of array of opaque uniforms with a flattened array.  This is run after
+    // MonomorphizeUnsupportedFunctionsInVulkanGLSL and RewriteStructSamplers so that it's not
+    // possible for an array of array of opaque type to be partially subscripted and passed to a
+    // function.
+    if (!RewriteArrayOfArrayOfOpaqueUniforms(this, root, &getSymbolTable()))
+    {
+        return false;
+    }
+
     // Rewrite samplerCubes as sampler2DArrays.  This must be done after rewriting struct samplers
     // as it doesn't expect that.
     if (compileOptions & SH_EMULATE_SEAMFUL_CUBE_MAP_SAMPLING)

src/compiler/translator/tree_ops/RemoveArrayLengthMethod.cpp

@@ -42,6 +42,8 @@ class RemoveArrayLengthTraverser : public TIntermTraverser
     bool foundArrayLength() const { return mFoundArrayLength; }
 
   private:
+    void insertSideEffectsInParentBlock(TIntermTyped *node);
+
     bool mFoundArrayLength;
 };
 
@@ -51,12 +53,7 @@ bool RemoveArrayLengthTraverser::visitUnary(Visit visit, TIntermUnary *node)
     if (node->getOp() == EOpArrayLength && !node->getOperand()->getType().isUnsizedArray())
     {
         mFoundArrayLength = true;
-        if (!node->getOperand()->hasSideEffects())
-        {
-            queueReplacement(node->fold(nullptr), OriginalNode::IS_DROPPED);
-            return false;
-        }
-        insertStatementInParentBlock(node->getOperand()->deepCopy());
+        insertSideEffectsInParentBlock(node->getOperand());
         TConstantUnion *constArray = new TConstantUnion[1];
         constArray->setIConst(node->getOperand()->getOutermostArraySize());
         queueReplacement(new TIntermConstantUnion(constArray, node->getType()),
@@ -66,6 +63,28 @@ bool RemoveArrayLengthTraverser::visitUnary(Visit visit, TIntermUnary *node)
     return true;
 }
 
+void RemoveArrayLengthTraverser::insertSideEffectsInParentBlock(TIntermTyped *node)
+{
+    // If the node is an index type, traverse it and add the indices as side effects.  If at the end
+    // an expression without side effect is encountered, such as an opaque uniform or a lone symbol,
+    // don't generate a statement for it.
+    if (!node->hasSideEffects())
+    {
+        return;
+    }
+
+    TIntermBinary *asBinary = node->getAsBinaryNode();
+    if (asBinary && !asBinary->isAssignment())
+    {
+        insertSideEffectsInParentBlock(asBinary->getLeft());
+        insertSideEffectsInParentBlock(asBinary->getRight());
+    }
+    else
+    {
+        insertStatementInParentBlock(node);
+    }
+}
+
 }  // anonymous namespace
 
 bool RemoveArrayLengthMethod(TCompiler *compiler, TIntermBlock *root)

src/compiler/translator/tree_ops/vulkan/RewriteArrayOfArrayOfOpaqueUniforms.cpp

@@ -0,0 +1,357 @@
+//
+// Copyright 2019 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.
+//
+// RewriteAtomicCounters: Emulate atomic counter buffers with storage buffers.
+//
+
+#include "compiler/translator/tree_ops/vulkan/RewriteAtomicCounters.h"
+
+#include "compiler/translator/Compiler.h"
+#include "compiler/translator/ImmutableStringBuilder.h"
+#include "compiler/translator/StaticType.h"
+#include "compiler/translator/SymbolTable.h"
+#include "compiler/translator/tree_util/IntermNode_util.h"
+#include "compiler/translator/tree_util/IntermTraverse.h"
+#include "compiler/translator/tree_util/ReplaceVariable.h"
+
+namespace sh
+{
+namespace
+{
+struct UniformData
+{
+    // Corresponding to an array of array of opaque uniform variable, this is the flattened variable
+    // that is replacing it.
+    const TVariable *flattened;
+    // Assume a general case of array declaration with N dimensions:
+    //
+    //     uniform type u[Dn]..[D2][D1];
+    //
+    // Let's define
+    //
+    //     Pn = D(n-1)*...*D2*D1
+    //
+    // In that case, we have:
+    //
+    //     u[In]         = ac + In*Pn
+    //     u[In][I(n-1)] = ac + In*Pn + I(n-1)*P(n-1)
+    //     u[In]...[Ii]  = ac + In*Pn + ... + Ii*Pi
+    //
+    // This array contains Pi.  Note that the like TType::mArraySizes, the last element is the
+    // outermost dimension.  Element 0 is necessarily 1.
+    TVector<unsigned int> mSubArraySizes;
+};
+
+using UniformMap = angle::HashMap<const TVariable *, UniformData>;
+
+TIntermTyped *RewriteArrayOfArraySubscriptExpression(TCompiler *compiler,
+                                                     TIntermBinary *node,
+                                                     const UniformMap &uniformMap);
+
+// Given an expression, this traverser calculates a new expression where array of array of opaque
+// uniforms are replaced with their flattened ones.  In particular, this is run on the right node of
+// EOpIndexIndirect binary nodes, so that the expression in the index gets a chance to go through
+// this transformation.
+class RewriteExpressionTraverser final : public TIntermTraverser
+{
+  public:
+    explicit RewriteExpressionTraverser(TCompiler *compiler, const UniformMap &uniformMap)
+        : TIntermTraverser(true, false, false), mCompiler(compiler), mUniformMap(uniformMap)
+    {}
+
+    bool visitBinary(Visit visit, TIntermBinary *node) override
+    {
+        // Only interested in opaque uniforms.
+        if (!IsOpaqueType(node->getType().getBasicType()))
+        {
+            return true;
+        }
+
+        TIntermTyped *rewritten =
+            RewriteArrayOfArraySubscriptExpression(mCompiler, node, mUniformMap);
+        if (rewritten == nullptr)
+        {
+            return true;
+        }
+
+        queueReplacement(rewritten, OriginalNode::IS_DROPPED);
+
+        // Don't iterate as the expression is rewritten.
+        return false;
+    }
+
+    void visitSymbol(TIntermSymbol *node) override
+    {
+        // We cannot reach here for an opaque uniform that is being replaced.  visitBinary should
+        // have taken care of it.
+        ASSERT(!IsOpaqueType(node->getType().getBasicType()) ||
+               mUniformMap.find(&node->variable()) == mUniformMap.end());
+    }
+
+  private:
+    TCompiler *mCompiler;
+
+    const UniformMap &mUniformMap;
+};
+
+// Rewrite the index of an EOpIndexIndirect expression.  The root can never need replacing, because
+// it cannot be an opaque uniform itself.
+void RewriteIndexExpression(TCompiler *compiler,
+                            TIntermTyped *expression,
+                            const UniformMap &uniformMap)
+{
+    RewriteExpressionTraverser traverser(compiler, uniformMap);
+    expression->traverse(&traverser);
+    bool valid = traverser.updateTree(compiler, expression);
+    ASSERT(valid);
+}
+
+// Given an expression such as the following:
+//
+//                                              EOpIndex(In)Direct (opaque uniform)
+//                                                    /           \
+//                                            EOpIndex(In)Direct   I1
+//                                                  /           \
+//                                                ...            I2
+//                                            /
+//                                    EOpIndex(In)Direct
+//                                          /           \
+//                                      uniform          In
+//
+// produces:
+//
+//          EOpIndex(In)Direct
+//            /        \
+//        uniform    In*Pn + ... + I2*P2 + I1*P1
+//
+TIntermTyped *RewriteArrayOfArraySubscriptExpression(TCompiler *compiler,
+                                                     TIntermBinary *node,
+                                                     const UniformMap &uniformMap)
+{
+    ASSERT(IsOpaqueType(node->getType().getBasicType()));
+
+    TIntermSymbol *opaqueUniform = nullptr;
+
+    // Iterate once and find the opaque uniform that's being indexed.
+    TIntermBinary *iter = node;
+    while (opaqueUniform == nullptr)
+    {
+        ASSERT(iter->getOp() == EOpIndexDirect || iter->getOp() == EOpIndexIndirect);
+
+        opaqueUniform = iter->getLeft()->getAsSymbolNode();
+        iter          = iter->getLeft()->getAsBinaryNode();
+    }
+
+    // If not being replaced, there's nothing to do.
+    auto flattenedIter = uniformMap.find(&opaqueUniform->variable());
+    if (flattenedIter == uniformMap.end())
+    {
+        return nullptr;
+    }
+
+    const UniformData &data = flattenedIter->second;
+
+    // Iterate again and build the index expression.  The index expression constitutes the sum of
+    // the variable indices plus a constant offset calculated from the constant indices.  For
+    // example, smplr[1][x][2][y] will have an index of x*P3 + y*P1 + c, where c = (1*P4 + 2*P2).
+    unsigned int constantOffset = 0;
+    TIntermTyped *variableIndex = nullptr;
+
+    // Since the opaque uniforms are fully subscripted, we know exactly how many EOpIndex* nodes
+    // there should be.
+    for (size_t dimIndex = 0; dimIndex < data.mSubArraySizes.size(); ++dimIndex)
+    {
+        ASSERT(node);
+
+        unsigned int subArraySize = data.mSubArraySizes[dimIndex];
+
+        switch (node->getOp())
+        {
+            case EOpIndexDirect:
+                // Accumulate the constant index.
+                constantOffset +=
+                    node->getRight()->getAsConstantUnion()->getIConst(0) * subArraySize;
+                break;
+            case EOpIndexIndirect:
+            {
+                // Run RewriteExpressionTraverser on the right node.  It may itself be an expression
+                // with an array of array of opaque uniform inside that needs to be rewritten.
+                TIntermTyped *indexExpression = node->getRight();
+                RewriteIndexExpression(compiler, indexExpression, uniformMap);
+
+                // Scale and accumulate.
+                if (subArraySize != 1)
+                {
+                    indexExpression =
+                        new TIntermBinary(EOpMul, indexExpression, CreateIndexNode(subArraySize));
+                }
+
+                if (variableIndex == nullptr)
+                {
+                    variableIndex = indexExpression;
+                }
+                else
+                {
+                    variableIndex = new TIntermBinary(EOpAdd, variableIndex, indexExpression);
+                }
+                break;
+            }
+            default:
+                UNREACHABLE();
+                break;
+        }
+
+        node = node->getLeft()->getAsBinaryNode();
+    }
+
+    // Add the two accumulated indices together.
+    TIntermTyped *index = nullptr;
+    if (constantOffset == 0 && variableIndex != nullptr)
+    {
+        // No constant offset, but there's variable offset.  Take that as offset.
+        index = variableIndex;
+    }
+    else
+    {
+        // Either the constant offset is non zero, or there's no variable offset (so constant 0
+        // should be used).
+        index = CreateIndexNode(constantOffset);
+
+        if (variableIndex)
+        {
+            index = new TIntermBinary(EOpAdd, index, variableIndex);
+        }
+    }
+
+    // Create an index into the flattened uniform.
+    TOperator op = variableIndex ? EOpIndexIndirect : EOpIndexDirect;
+    return new TIntermBinary(op, new TIntermSymbol(data.flattened), index);
+}
+
+// Traverser that takes:
+//
+//     uniform sampler/image/atomic_uint u[N][M]..
+//
+// and transforms it to:
+//
+//     uniform sampler/image/atomic_uint u[N * M * ..]
+//
+// MonomorphizeUnsupportedFunctionsInVulkanGLSL makes it impossible for this array to be partially
+// subscripted, or passed as argument to a function unsubscripted.  This means that every encounter
+// of this uniform can be expected to be fully subscripted.
+//
+class RewriteArrayOfArrayOfOpaqueUniformsTraverser : public TIntermTraverser
+{
+  public:
+    RewriteArrayOfArrayOfOpaqueUniformsTraverser(TCompiler *compiler, TSymbolTable *symbolTable)
+        : TIntermTraverser(true, false, false, symbolTable), mCompiler(compiler)
+    {}
+
+    bool visitDeclaration(Visit visit, TIntermDeclaration *node) override
+    {
+        if (!mInGlobalScope)
+        {
+            return true;
+        }
+
+        const TIntermSequence &sequence = *(node->getSequence());
+
+        TIntermTyped *variable = sequence.front()->getAsTyped();
+        const TType &type      = variable->getType();
+        bool isOpaqueUniform =
+            type.getQualifier() == EvqUniform && IsOpaqueType(type.getBasicType());
+
+        // Only interested in array of array of opaque uniforms.
+        if (!isOpaqueUniform || !type.isArrayOfArrays())
+        {
+            return false;
+        }
+
+        // Opaque uniforms cannot have initializers, so the declaration must necessarily be a
+        // symbol.
+        TIntermSymbol *symbol = variable->getAsSymbolNode();
+        ASSERT(symbol != nullptr);
+
+        const TVariable *uniformVariable = &symbol->variable();
+
+        // Create an entry in the map.
+        ASSERT(mUniformMap.find(uniformVariable) == mUniformMap.end());
+        UniformData &data = mUniformMap[uniformVariable];
+
+        // Calculate the accumulated dimension products.  See UniformData::mSubArraySizes.
+        const TSpan<const unsigned int> &arraySizes = type.getArraySizes();
+        mUniformMap[uniformVariable].mSubArraySizes.resize(arraySizes.size());
+        unsigned int runningProduct = 1;
+        for (size_t dimension = 0; dimension < arraySizes.size(); ++dimension)
+        {
+            data.mSubArraySizes[dimension] = runningProduct;
+            runningProduct *= arraySizes[dimension];
+        }
+
+        // Create a replacement variable with the array flattened.
+        TType *newType = new TType(type);
+        newType->toArrayBaseType();
+        newType->makeArray(runningProduct);
+
+        data.flattened = new TVariable(mSymbolTable, uniformVariable->name(), newType,
+                                       uniformVariable->symbolType());
+
+        TIntermDeclaration *decl = new TIntermDeclaration;
+        decl->appendDeclarator(new TIntermSymbol(data.flattened));
+
+        queueReplacement(decl, OriginalNode::IS_DROPPED);
+        return false;
+    }
+
+    bool visitFunctionDefinition(Visit visit, TIntermFunctionDefinition *node) override
+    {
+        // As an optimization, don't bother inspecting functions if there aren't any opaque uniforms
+        // to replace.
+        return !mUniformMap.empty();
+    }
+
+    // Same implementation as in RewriteExpressionTraverser.  That traverser cannot replace root.
+    bool visitBinary(Visit visit, TIntermBinary *node) override
+    {
+        // Only interested in opaque uniforms.
+        if (!IsOpaqueType(node->getType().getBasicType()))
+        {
+            return true;
+        }
+
+        TIntermTyped *rewritten =
+            RewriteArrayOfArraySubscriptExpression(mCompiler, node, mUniformMap);
+        if (rewritten == nullptr)
+        {
+            return true;
+        }
+
+        queueReplacement(rewritten, OriginalNode::IS_DROPPED);
+
+        // Don't iterate as the expression is rewritten.
+        return false;
+    }
+
+    void visitSymbol(TIntermSymbol *node) override
+    {
+        ASSERT(!IsOpaqueType(node->getType().getBasicType()) ||
+               mUniformMap.find(&node->variable()) == mUniformMap.end());
+    }
+
+  private:
+    TCompiler *mCompiler;
+    UniformMap mUniformMap;
+};
+}  // anonymous namespace
+
+bool RewriteArrayOfArrayOfOpaqueUniforms(TCompiler *compiler,
+                                         TIntermBlock *root,
+                                         TSymbolTable *symbolTable)
+{
+    RewriteArrayOfArrayOfOpaqueUniformsTraverser traverser(compiler, symbolTable);
+    root->traverse(&traverser);
+    return traverser.updateTree(compiler, root);
+}
+}  // namespace sh

src/compiler/translator/tree_ops/vulkan/RewriteArrayOfArrayOfOpaqueUniforms.h

@@ -0,0 +1,25 @@
+//
+// Copyright 2021 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.
+//
+// RewriteArrayOfArrayOfOpaqueUniforms: Flatten array of array of opaque uniforms.  Requires
+// MonomorphizeUnsupportedFunctionsInVulkanGLSL and RewriteStructSamplers to have been run.
+
+#ifndef COMPILER_TRANSLATOR_TREEOPS_VULKAN_REWRITEARRAYOFARRAYOFOPAQUEUNIFORMS_H_
+#define COMPILER_TRANSLATOR_TREEOPS_VULKAN_REWRITEARRAYOFARRAYOFOPAQUEUNIFORMS_H_
+
+#include "common/angleutils.h"
+
+namespace sh
+{
+class TCompiler;
+class TIntermBlock;
+class TSymbolTable;
+
+ANGLE_NO_DISCARD bool RewriteArrayOfArrayOfOpaqueUniforms(TCompiler *compiler,
+                                                          TIntermBlock *root,
+                                                          TSymbolTable *symbolTable);
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_TREEOPS_VULKAN_REWRITEARRAYOFARRAYOFOPAQUEUNIFORMS_H_

src/compiler/translator/tree_ops/vulkan/RewriteStructSamplers.h

@@ -5,10 +5,10 @@
 //
 // RewriteStructSamplers: Extract structs from samplers.
 //
-// This traverser is designed to strip out samplers from structs. It moves them into
-// separate uniform sampler declarations. This allows the struct to be stored in the
-// default uniform block. It also requires that we rewrite any functions that take the
-// struct as an argument. The struct is split into two arguments.
+// This traverser is designed to strip out samplers from structs. It moves them into separate
+// uniform sampler declarations. This allows the struct to be stored in the default uniform block.
+// This transformation requires MonomorphizeUnsupportedFunctionsInVulkanGLSL to have been run so it
+// wouldn't need to deal with functions that are passed such structs.
 //
 // For example:
 //   struct S { sampler2D samp; int i; };
@@ -33,10 +33,6 @@ ANGLE_NO_DISCARD bool RewriteStructSamplers(TCompiler *compiler,
                                             TIntermBlock *root,
                                             TSymbolTable *symbolTable,
                                             int *removedUniformsCountOut);
-ANGLE_NO_DISCARD bool RewriteStructSamplersOld(TCompiler *compier,
-                                               TIntermBlock *root,
-                                               TSymbolTable *symbolTable,
-                                               int *removedUniformsCountOut);
 }  // namespace sh
 
 #endif  // COMPILER_TRANSLATOR_TREEOPS_VULKAN_REWRITESTRUCTSAMPLERS_H_

src/compiler/translator/tree_ops/vulkan/RewriteStructSamplersOld.cpp

@@ -1,705 +0,0 @@
-//
-// Copyright 2018 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.
-//
-// RewriteStructSamplers: Extract structs from samplers.
-//
-
-#include "compiler/translator/tree_ops/vulkan/RewriteStructSamplers.h"
-
-#include "compiler/translator/ImmutableStringBuilder.h"
-#include "compiler/translator/SymbolTable.h"
-#include "compiler/translator/tree_util/IntermTraverse.h"
-
-namespace sh
-{
-namespace
-{
-// Helper method to get the sampler extracted struct type of a parameter.
-TType *GetStructSamplerParameterType(TSymbolTable *symbolTable, const TVariable &param)
-{
-    const TStructure *structure = param.getType().getStruct();
-    const TSymbol *structSymbol = symbolTable->findUserDefined(structure->name());
-    ASSERT(structSymbol && structSymbol->isStruct());
-    const TStructure *structVar = static_cast<const TStructure *>(structSymbol);
-    TType *structType           = new TType(structVar, false);
-
-    if (param.getType().isArray())
-    {
-        structType->makeArrays(param.getType().getArraySizes());
-    }
-
-    ASSERT(!structType->isStructureContainingSamplers());
-
-    return structType;
-}
-
-TIntermSymbol *ReplaceTypeOfSymbolNode(TIntermSymbol *symbolNode, TSymbolTable *symbolTable)
-{
-    const TVariable &oldVariable = symbolNode->variable();
-
-    TType *newType = GetStructSamplerParameterType(symbolTable, oldVariable);
-
-    TVariable *newVariable =
-        new TVariable(oldVariable.uniqueId(), oldVariable.name(), oldVariable.symbolType(),
-                      oldVariable.extension(), newType);
-    return new TIntermSymbol(newVariable);
-}
-
-TIntermTyped *ReplaceTypeOfTypedStructNode(TIntermTyped *argument, TSymbolTable *symbolTable)
-{
-    TIntermSymbol *asSymbol = argument->getAsSymbolNode();
-    if (asSymbol)
-    {
-        ASSERT(asSymbol->getType().getStruct());
-        return ReplaceTypeOfSymbolNode(asSymbol, symbolTable);
-    }
-
-    TIntermTyped *replacement = argument->deepCopy();
-    TIntermBinary *binary     = replacement->getAsBinaryNode();
-    ASSERT(binary);
-
-    while (binary)
-    {
-        ASSERT(binary->getOp() == EOpIndexDirectStruct || binary->getOp() == EOpIndexDirect);
-
-        asSymbol = binary->getLeft()->getAsSymbolNode();
-
-        if (asSymbol)
-        {
-            ASSERT(asSymbol->getType().getStruct());
-            TIntermSymbol *newSymbol = ReplaceTypeOfSymbolNode(asSymbol, symbolTable);
-            binary->replaceChildNode(binary->getLeft(), newSymbol);
-            return replacement;
-        }
-
-        binary = binary->getLeft()->getAsBinaryNode();
-    }
-
-    UNREACHABLE();
-    return nullptr;
-}
-
-// Maximum string size of a hex unsigned int.
-constexpr size_t kHexSize = ImmutableStringBuilder::GetHexCharCount<unsigned int>();
-
-class Traverser final : public TIntermTraverser
-{
-  public:
-    explicit Traverser(TSymbolTable *symbolTable)
-        : TIntermTraverser(true, false, true, symbolTable), mRemovedUniformsCount(0)
-    {
-        mSymbolTable->push();
-    }
-
-    ~Traverser() override { mSymbolTable->pop(); }
-
-    int removedUniformsCount() const { return mRemovedUniformsCount; }
-
-    // Each struct sampler declaration is stripped of its samplers. New uniforms are added for each
-    // stripped struct sampler.
-    bool visitDeclaration(Visit visit, TIntermDeclaration *decl) override
-    {
-        if (visit != PreVisit)
-            return true;
-
-        if (!mInGlobalScope)
-        {
-            return true;
-        }
-
-        const TIntermSequence &sequence = *(decl->getSequence());
-        TIntermTyped *declarator        = sequence.front()->getAsTyped();
-        const TType &type               = declarator->getType();
-
-        if (type.isStructureContainingSamplers())
-        {
-            TIntermSequence *newSequence = new TIntermSequence;
-
-            if (type.isStructSpecifier())
-            {
-                stripStructSpecifierSamplers(type.getStruct(), newSequence);
-            }
-            else
-            {
-                TIntermSymbol *asSymbol = declarator->getAsSymbolNode();
-                ASSERT(asSymbol);
-                const TVariable &variable = asSymbol->variable();
-                ASSERT(variable.symbolType() != SymbolType::Empty);
-                extractStructSamplerUniforms(decl, variable, type.getStruct(), newSequence);
-            }
-
-            mMultiReplacements.emplace_back(getParentNode()->getAsBlock(), decl, *newSequence);
-        }
-
-        return true;
-    }
-
-    // Each struct sampler reference is replaced with a reference to the new extracted sampler.
-    bool visitBinary(Visit visit, TIntermBinary *node) override
-    {
-        if (visit != PreVisit)
-            return true;
-
-        if (node->getOp() == EOpIndexDirectStruct && node->getType().isSampler())
-        {
-            ImmutableString newName = GetStructSamplerNameFromTypedNode(node);
-            const TVariable *samplerReplacement =
-                static_cast<const TVariable *>(mSymbolTable->findUserDefined(newName));
-            ASSERT(samplerReplacement);
-
-            TIntermSymbol *replacement = new TIntermSymbol(samplerReplacement);
-
-            queueReplacement(replacement, OriginalNode::IS_DROPPED);
-            return true;
-        }
-
-        return true;
-    }
-
-    // In we are passing references to structs containing samplers we must new additional
-    // arguments. For each extracted struct sampler a new argument is added. This chains to nested
-    // structs.
-    void visitFunctionPrototype(TIntermFunctionPrototype *node) override
-    {
-        const TFunction *function = node->getFunction();
-
-        if (!function->hasSamplerInStructOrArrayOfArrayParams())
-        {
-            return;
-        }
-
-        const TSymbol *foundFunction = mSymbolTable->findUserDefined(function->name());
-        if (foundFunction)
-        {
-            ASSERT(foundFunction->isFunction());
-            function = static_cast<const TFunction *>(foundFunction);
-        }
-        else
-        {
-            TFunction *newFunction = createStructSamplerFunction(function);
-            mSymbolTable->declareUserDefinedFunction(newFunction, true);
-            function = newFunction;
-        }
-
-        ASSERT(!function->hasSamplerInStructOrArrayOfArrayParams());
-        TIntermFunctionPrototype *newProto = new TIntermFunctionPrototype(function);
-        queueReplacement(newProto, OriginalNode::IS_DROPPED);
-    }
-
-    // We insert a new scope for each function definition so we can track the new parameters.
-    bool visitFunctionDefinition(Visit visit, TIntermFunctionDefinition *node) override
-    {
-        if (visit == PreVisit)
-        {
-            mSymbolTable->push();
-        }
-        else
-        {
-            ASSERT(visit == PostVisit);
-            mSymbolTable->pop();
-        }
-        return true;
-    }
-
-    // For function call nodes we pass references to the extracted struct samplers in that scope.
-    bool visitAggregate(Visit visit, TIntermAggregate *node) override
-    {
-        if (visit != PreVisit)
-            return true;
-
-        if (!node->isFunctionCall())
-            return true;
-
-        const TFunction *function = node->getFunction();
-        if (!function->hasSamplerInStructOrArrayOfArrayParams())
-            return true;
-
-        ASSERT(node->getOp() == EOpCallFunctionInAST);
-        TFunction *newFunction        = mSymbolTable->findUserDefinedFunction(function->name());
-        TIntermSequence *newArguments = getStructSamplerArguments(function, node->getSequence());
-
-        TIntermAggregate *newCall =
-            TIntermAggregate::CreateFunctionCall(*newFunction, newArguments);
-        queueReplacement(newCall, OriginalNode::IS_DROPPED);
-        return true;
-    }
-
-  private:
-    // This returns the name of a struct sampler reference. References are always TIntermBinary.
-    static ImmutableString GetStructSamplerNameFromTypedNode(TIntermTyped *node)
-    {
-        std::string stringBuilder;
-
-        TIntermTyped *currentNode = node;
-        while (currentNode->getAsBinaryNode())
-        {
-            TIntermBinary *asBinary = currentNode->getAsBinaryNode();
-
-            switch (asBinary->getOp())
-            {
-                case EOpIndexDirect:
-                {
-                    const int index = asBinary->getRight()->getAsConstantUnion()->getIConst(0);
-                    const std::string strInt = Str(index);
-                    stringBuilder.insert(0, strInt);
-                    stringBuilder.insert(0, "_");
-                    break;
-                }
-                case EOpIndexDirectStruct:
-                {
-                    stringBuilder.insert(0, asBinary->getIndexStructFieldName().data());
-                    stringBuilder.insert(0, "_");
-                    break;
-                }
-
-                default:
-                    UNREACHABLE();
-                    break;
-            }
-
-            currentNode = asBinary->getLeft();
-        }
-
-        const ImmutableString &variableName = currentNode->getAsSymbolNode()->variable().name();
-        stringBuilder.insert(0, variableName.data());
-
-        return stringBuilder;
-    }
-
-    // Removes all the struct samplers from a struct specifier.
-    void stripStructSpecifierSamplers(const TStructure *structure, TIntermSequence *newSequence)
-    {
-        TFieldList *newFieldList = new TFieldList;
-        ASSERT(structure->containsSamplers());
-
-        for (const TField *field : structure->fields())
-        {
-            const TType &fieldType = *field->type();
-            if (!fieldType.isSampler() && !isRemovedStructType(fieldType))
-            {
-                TType *newType = nullptr;
-
-                if (fieldType.isStructureContainingSamplers())
-                {
-                    const TSymbol *structSymbol =
-                        mSymbolTable->findUserDefined(fieldType.getStruct()->name());
-                    ASSERT(structSymbol && structSymbol->isStruct());
-                    const TStructure *fieldStruct = static_cast<const TStructure *>(structSymbol);
-                    newType                       = new TType(fieldStruct, true);
-                    if (fieldType.isArray())
-                    {
-                        newType->makeArrays(fieldType.getArraySizes());
-                    }
-                }
-                else
-                {
-                    newType = new TType(fieldType);
-                }
-
-                TField *newField =
-                    new TField(newType, field->name(), field->line(), field->symbolType());
-                newFieldList->push_back(newField);
-            }
-        }
-
-        // Prune empty structs.
-        if (newFieldList->empty())
-        {
-            mRemovedStructs.insert(structure->name());
-            return;
-        }
-
-        TStructure *newStruct =
-            new TStructure(mSymbolTable, structure->name(), newFieldList, structure->symbolType());
-        TType *newStructType = new TType(newStruct, true);
-        TVariable *newStructVar =
-            new TVariable(mSymbolTable, kEmptyImmutableString, newStructType, SymbolType::Empty);
-        TIntermSymbol *newStructRef = new TIntermSymbol(newStructVar);
-
-        TIntermDeclaration *structDecl = new TIntermDeclaration;
-        structDecl->appendDeclarator(newStructRef);
-
-        newSequence->push_back(structDecl);
-
-        mSymbolTable->declare(newStruct);
-    }
-
-    // Returns true if the type is a struct that was removed because we extracted all the members.
-    bool isRemovedStructType(const TType &type) const
-    {
-        const TStructure *structure = type.getStruct();
-        return (structure && (mRemovedStructs.count(structure->name()) > 0));
-    }
-
-    // Removes samplers from struct uniforms. For each sampler removed also adds a new globally
-    // defined sampler uniform.
-    void extractStructSamplerUniforms(TIntermDeclaration *oldDeclaration,
-                                      const TVariable &variable,
-                                      const TStructure *structure,
-                                      TIntermSequence *newSequence)
-    {
-        ASSERT(structure->containsSamplers());
-
-        size_t nonSamplerCount = 0;
-
-        for (const TField *field : structure->fields())
-        {
-            nonSamplerCount +=
-                extractFieldSamplers(variable.name(), field, variable.getType(), newSequence);
-        }
-
-        if (nonSamplerCount > 0)
-        {
-            // Keep the old declaration around if it has other members.
-            newSequence->push_back(oldDeclaration);
-        }
-        else
-        {
-            mRemovedUniformsCount++;
-        }
-    }
-
-    // Extracts samplers from a field of a struct. Works with nested structs and arrays.
-    size_t extractFieldSamplers(const ImmutableString &prefix,
-                                const TField *field,
-                                const TType &containingType,
-                                TIntermSequence *newSequence)
-    {
-        if (containingType.isArray())
-        {
-            size_t nonSamplerCount = 0;
-
-            // Name the samplers internally as varName_<index>_fieldName
-            const TSpan<const unsigned int> &arraySizes = containingType.getArraySizes();
-            for (unsigned int arrayElement = 0; arrayElement < arraySizes[0]; ++arrayElement)
-            {
-                ImmutableStringBuilder stringBuilder(prefix.length() + kHexSize + 1);
-                stringBuilder << prefix << "_";
-                stringBuilder.appendHex(arrayElement);
-                nonSamplerCount = extractFieldSamplersImpl(stringBuilder, field, newSequence);
-            }
-
-            return nonSamplerCount;
-        }
-
-        return extractFieldSamplersImpl(prefix, field, newSequence);
-    }
-
-    // Extracts samplers from a field of a struct. Works with nested structs and arrays.
-    size_t extractFieldSamplersImpl(const ImmutableString &prefix,
-                                    const TField *field,
-                                    TIntermSequence *newSequence)
-    {
-        size_t nonSamplerCount = 0;
-
-        const TType &fieldType = *field->type();
-        if (fieldType.isSampler() || fieldType.isStructureContainingSamplers())
-        {
-            ImmutableStringBuilder stringBuilder(prefix.length() + field->name().length() + 1);
-            stringBuilder << prefix << "_" << field->name();
-            ImmutableString newPrefix(stringBuilder);
-
-            if (fieldType.isSampler())
-            {
-                extractSampler(newPrefix, fieldType, newSequence);
-            }
-            else
-            {
-                const TStructure *structure = fieldType.getStruct();
-                for (const TField *nestedField : structure->fields())
-                {
-                    nonSamplerCount +=
-                        extractFieldSamplers(newPrefix, nestedField, fieldType, newSequence);
-                }
-            }
-        }
-        else
-        {
-            nonSamplerCount++;
-        }
-
-        return nonSamplerCount;
-    }
-
-    // Extracts a sampler from a struct. Declares the new extracted sampler.
-    void extractSampler(const ImmutableString &newName,
-                        const TType &fieldType,
-                        TIntermSequence *newSequence) const
-    {
-        TType *newType = new TType(fieldType);
-        newType->setQualifier(EvqUniform);
-        TVariable *newVariable =
-            new TVariable(mSymbolTable, newName, newType, SymbolType::AngleInternal);
-        TIntermSymbol *newRef = new TIntermSymbol(newVariable);
-
-        TIntermDeclaration *samplerDecl = new TIntermDeclaration;
-        samplerDecl->appendDeclarator(newRef);
-
-        newSequence->push_back(samplerDecl);
-
-        mSymbolTable->declareInternal(newVariable);
-    }
-
-    // Returns the chained name of a sampler uniform field.
-    static ImmutableString GetFieldName(const ImmutableString &paramName,
-                                        const TField *field,
-                                        unsigned arrayIndex)
-    {
-        ImmutableStringBuilder nameBuilder(paramName.length() + kHexSize + 2 +
-                                           field->name().length());
-        nameBuilder << paramName << "_";
-
-        if (arrayIndex < std::numeric_limits<unsigned>::max())
-        {
-            nameBuilder.appendHex(arrayIndex);
-            nameBuilder << "_";
-        }
-        nameBuilder << field->name();
-
-        return nameBuilder;
-    }
-
-    // A pattern that visits every parameter of a function call. Uses different handlers for struct
-    // parameters, struct sampler parameters, and non-struct parameters.
-    class StructSamplerFunctionVisitor : angle::NonCopyable
-    {
-      public:
-        StructSamplerFunctionVisitor()          = default;
-        virtual ~StructSamplerFunctionVisitor() = default;
-
-        virtual void traverse(const TFunction *function)
-        {
-            size_t paramCount = function->getParamCount();
-
-            for (size_t paramIndex = 0; paramIndex < paramCount; ++paramIndex)
-            {
-                const TVariable *param = function->getParam(paramIndex);
-                const TType &paramType = param->getType();
-
-                if (paramType.isStructureContainingSamplers())
-                {
-                    const ImmutableString &baseName = getNameFromIndex(function, paramIndex);
-                    if (traverseStructContainingSamplers(baseName, paramType))
-                    {
-                        visitStructParam(function, paramIndex);
-                    }
-                }
-                else
-                {
-                    visitNonStructParam(function, paramIndex);
-                }
-            }
-        }
-
-        virtual ImmutableString getNameFromIndex(const TFunction *function, size_t paramIndex) = 0;
-        virtual void visitSamplerInStructParam(const ImmutableString &name,
-                                               const TField *field)                            = 0;
-        virtual void visitStructParam(const TFunction *function, size_t paramIndex)            = 0;
-        virtual void visitNonStructParam(const TFunction *function, size_t paramIndex)         = 0;
-
-      private:
-        bool traverseStructContainingSamplers(const ImmutableString &baseName,
-                                              const TType &structType)
-        {
-            bool hasNonSamplerFields    = false;
-            const TStructure *structure = structType.getStruct();
-            for (const TField *field : structure->fields())
-            {
-                if (field->type()->isStructureContainingSamplers() || field->type()->isSampler())
-                {
-                    if (traverseSamplerInStruct(baseName, structType, field))
-                    {
-                        hasNonSamplerFields = true;
-                    }
-                }
-                else
-                {
-                    hasNonSamplerFields = true;
-                }
-            }
-            return hasNonSamplerFields;
-        }
-
-        bool traverseSamplerInStruct(const ImmutableString &baseName,
-                                     const TType &baseType,
-                                     const TField *field)
-        {
-            bool hasNonSamplerParams = false;
-
-            if (baseType.isArray())
-            {
-                const TSpan<const unsigned int> &arraySizes = baseType.getArraySizes();
-                ASSERT(arraySizes.size() == 1);
-
-                for (unsigned int arrayIndex = 0; arrayIndex < arraySizes[0]; ++arrayIndex)
-                {
-                    ImmutableString name = GetFieldName(baseName, field, arrayIndex);
-
-                    if (field->type()->isStructureContainingSamplers())
-                    {
-                        if (traverseStructContainingSamplers(name, *field->type()))
-                        {
-                            hasNonSamplerParams = true;
-                        }
-                    }
-                    else
-                    {
-                        ASSERT(field->type()->isSampler());
-                        visitSamplerInStructParam(name, field);
-                    }
-                }
-            }
-            else if (field->type()->isStructureContainingSamplers())
-            {
-                ImmutableString name =
-                    GetFieldName(baseName, field, std::numeric_limits<unsigned>::max());
-                hasNonSamplerParams = traverseStructContainingSamplers(name, *field->type());
-            }
-            else
-            {
-                ASSERT(field->type()->isSampler());
-                ImmutableString name =
-                    GetFieldName(baseName, field, std::numeric_limits<unsigned>::max());
-                visitSamplerInStructParam(name, field);
-            }
-
-            return hasNonSamplerParams;
-        }
-    };
-
-    // A visitor that replaces functions with struct sampler references. The struct sampler
-    // references are expanded to include new fields for the structs.
-    class CreateStructSamplerFunctionVisitor final : public StructSamplerFunctionVisitor
-    {
-      public:
-        CreateStructSamplerFunctionVisitor(TSymbolTable *symbolTable)
-            : mSymbolTable(symbolTable), mNewFunction(nullptr)
-        {}
-
-        ImmutableString getNameFromIndex(const TFunction *function, size_t paramIndex) override
-        {
-            const TVariable *param = function->getParam(paramIndex);
-            return param->name();
-        }
-
-        void traverse(const TFunction *function) override
-        {
-            mNewFunction =
-                new TFunction(mSymbolTable, function->name(), function->symbolType(),
-                              &function->getReturnType(), function->isKnownToNotHaveSideEffects());
-
-            StructSamplerFunctionVisitor::traverse(function);
-        }
-
-        void visitSamplerInStructParam(const ImmutableString &name, const TField *field) override
-        {
-            TVariable *fieldSampler =
-                new TVariable(mSymbolTable, name, field->type(), SymbolType::AngleInternal);
-            mNewFunction->addParameter(fieldSampler);
-            mSymbolTable->declareInternal(fieldSampler);
-        }
-
-        void visitStructParam(const TFunction *function, size_t paramIndex) override
-        {
-            const TVariable *param = function->getParam(paramIndex);
-            TType *structType      = GetStructSamplerParameterType(mSymbolTable, *param);
-            TVariable *newParam =
-                new TVariable(mSymbolTable, param->name(), structType, param->symbolType());
-            mNewFunction->addParameter(newParam);
-        }
-
-        void visitNonStructParam(const TFunction *function, size_t paramIndex) override
-        {
-            const TVariable *param = function->getParam(paramIndex);
-            mNewFunction->addParameter(param);
-        }
-
-        TFunction *getNewFunction() const { return mNewFunction; }
-
-      private:
-        TSymbolTable *mSymbolTable;
-        TFunction *mNewFunction;
-    };
-
-    TFunction *createStructSamplerFunction(const TFunction *function) const
-    {
-        CreateStructSamplerFunctionVisitor visitor(mSymbolTable);
-        visitor.traverse(function);
-        return visitor.getNewFunction();
-    }
-
-    // A visitor that replaces function calls with expanded struct sampler parameters.
-    class GetSamplerArgumentsVisitor final : public StructSamplerFunctionVisitor
-    {
-      public:
-        GetSamplerArgumentsVisitor(TSymbolTable *symbolTable, const TIntermSequence *arguments)
-            : mSymbolTable(symbolTable), mArguments(arguments), mNewArguments(new TIntermSequence)
-        {}
-
-        ImmutableString getNameFromIndex(const TFunction *function, size_t paramIndex) override
-        {
-            TIntermTyped *argument = (*mArguments)[paramIndex]->getAsTyped();
-            return GetStructSamplerNameFromTypedNode(argument);
-        }
-
-        void visitSamplerInStructParam(const ImmutableString &name, const TField *field) override
-        {
-            TVariable *argSampler =
-                new TVariable(mSymbolTable, name, field->type(), SymbolType::AngleInternal);
-            TIntermSymbol *argSymbol = new TIntermSymbol(argSampler);
-            mNewArguments->push_back(argSymbol);
-        }
-
-        void visitStructParam(const TFunction *function, size_t paramIndex) override
-        {
-            // The tree structure of the parameter is modified to point to the new type. This leaves
-            // the tree in a consistent state.
-            TIntermTyped *argument    = (*mArguments)[paramIndex]->getAsTyped();
-            TIntermTyped *replacement = ReplaceTypeOfTypedStructNode(argument, mSymbolTable);
-            mNewArguments->push_back(replacement);
-        }
-
-        void visitNonStructParam(const TFunction *function, size_t paramIndex) override
-        {
-            TIntermTyped *argument = (*mArguments)[paramIndex]->getAsTyped();
-            mNewArguments->push_back(argument);
-        }
-
-        TIntermSequence *getNewArguments() const { return mNewArguments; }
-
-      private:
-        TSymbolTable *mSymbolTable;
-        const TIntermSequence *mArguments;
-        TIntermSequence *mNewArguments;
-    };
-
-    TIntermSequence *getStructSamplerArguments(const TFunction *function,
-                                               const TIntermSequence *arguments) const
-    {
-        GetSamplerArgumentsVisitor visitor(mSymbolTable, arguments);
-        visitor.traverse(function);
-        return visitor.getNewArguments();
-    }
-
-    int mRemovedUniformsCount;
-    std::set<ImmutableString> mRemovedStructs;
-};
-}  // anonymous namespace
-
-bool RewriteStructSamplersOld(TCompiler *compiler,
-                              TIntermBlock *root,
-                              TSymbolTable *symbolTable,
-                              int *removedUniformsCountOut)
-{
-    Traverser rewriteStructSamplers(symbolTable);
-    root->traverse(&rewriteStructSamplers);
-    if (!rewriteStructSamplers.updateTree(compiler, root))
-    {
-        return false;
-    }
-    *removedUniformsCountOut = rewriteStructSamplers.removedUniformsCount();
-    return true;
-}
-}  // namespace sh

src/libANGLE/renderer/glslang_wrapper_utils.cpp

@@ -138,22 +138,10 @@ void GlslangWarmup()
 // array (except for the innermost dimension).  When assigning decorations (set/binding/etc), only
 // the indices corresponding to the first element of the array should be specified.  This function
 // is used to skip the other indices.
-//
-// If useOldRewriteStructSamplers, there are multiple samplers extracted out of struct arrays
-// though, so the above only applies to the sampler array defined in the struct.
-bool UniformNameIsIndexZero(const std::string &name, bool excludeCheckForOwningStructArrays)
+bool UniformNameIsIndexZero(const std::string &name)
 {
     size_t lastBracketClose = 0;
 
-    if (excludeCheckForOwningStructArrays)
-    {
-        size_t lastDot = name.find_last_of('.');
-        if (lastDot != std::string::npos)
-        {
-            lastBracketClose = lastDot;
-        }
-    }
-
     while (true)
     {
         size_t openBracket = name.find('[', lastBracketClose);
@@ -998,18 +986,15 @@ void AssignTextureBindings(const GlslangSourceOptions &options,
     {
         const gl::LinkedUniform &samplerUniform = uniforms[uniformIndex];
 
-        if (!options.useOldRewriteStructSamplers &&
-            gl::SamplerNameContainsNonZeroArrayElement(samplerUniform.name))
+        if (gl::SamplerNameContainsNonZeroArrayElement(samplerUniform.name))
         {
             continue;
         }
 
-        if (UniformNameIsIndexZero(samplerUniform.name, options.useOldRewriteStructSamplers))
+        if (UniformNameIsIndexZero(samplerUniform.name))
         {
             // Samplers in structs are extracted and renamed.
-            const std::string samplerName = options.useOldRewriteStructSamplers
-                                                ? GetMappedSamplerNameOld(samplerUniform.name)
-                                                : GlslangGetMappedSamplerName(samplerUniform.name);
+            const std::string samplerName = GlslangGetMappedSamplerName(samplerUniform.name);
 
             // TODO: http://anglebug.com/4523: All uniforms should be active
             if (programExecutable.hasLinkedShaderStage(shaderType) &&
@@ -3774,7 +3759,7 @@ bool GetImageNameWithoutIndices(std::string *name)
         return true;
     }
 
-    if (!UniformNameIsIndexZero(*name, false))
+    if (!UniformNameIsIndexZero(*name))
     {
         return false;
     }
@@ -3784,25 +3769,6 @@ bool GetImageNameWithoutIndices(std::string *name)
     return true;
 }
 
-std::string GetMappedSamplerNameOld(const std::string &originalName)
-{
-    std::string samplerName = gl::ParseResourceName(originalName, nullptr);
-
-    // Samplers in structs are extracted.
-    std::replace(samplerName.begin(), samplerName.end(), '.', '_');
-
-    // Samplers in arrays of structs are also extracted.
-    std::replace(samplerName.begin(), samplerName.end(), '[', '_');
-    samplerName.erase(std::remove(samplerName.begin(), samplerName.end(), ']'), samplerName.end());
-
-    if (MappedSamplerNameNeedsUserDefinedPrefix(originalName))
-    {
-        samplerName = sh::kUserDefinedNamePrefix + samplerName;
-    }
-
-    return samplerName;
-}
-
 std::string GlslangGetMappedSamplerName(const std::string &originalName)
 {
     std::string samplerName = originalName;

src/libANGLE/renderer/glslang_wrapper_utils.h

@@ -47,7 +47,6 @@ struct GlslangProgramInterfaceInfo
 
 struct GlslangSourceOptions
 {
-    bool useOldRewriteStructSamplers        = false;
     bool supportsTransformFeedbackExtension = false;
     bool emulateTransformFeedback           = false;
     bool emulateBresenhamLines              = false;
@@ -159,7 +158,6 @@ void GlslangRelease();
 bool GetImageNameWithoutIndices(std::string *name);
 
 // Get the mapped sampler name after the source is transformed by GlslangGetShaderSource()
-std::string GetMappedSamplerNameOld(const std::string &originalName);
 std::string GlslangGetMappedSamplerName(const std::string &originalName);
 std::string GetXfbBufferName(const uint32_t bufferIndex);
 

src/libANGLE/renderer/vulkan/ContextVk.cpp

@@ -392,7 +392,6 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk
       mFlipYForCurrentSurface(false),
       mIsAnyHostVisibleBufferWritten(false),
       mEmulateSeamfulCubeMapSampling(false),
-      mUseOldRewriteStructSamplers(false),
       mOutsideRenderPassCommands(nullptr),
       mRenderPassCommands(nullptr),
       mGpuEventsEnabled(false),
@@ -655,8 +654,6 @@ angle::Result ContextVk::initialize()
 
     mEmulateSeamfulCubeMapSampling = shouldEmulateSeamfulCubeMapSampling();
 
-    mUseOldRewriteStructSamplers = shouldUseOldRewriteStructSamplers();
-
     // Assign initial command buffers from queue
     mOutsideRenderPassCommands = mRenderer->getCommandBufferHelper(false);
     mRenderPassCommands        = mRenderer->getCommandBufferHelper(true);
@@ -4580,11 +4577,6 @@ bool ContextVk::shouldEmulateSeamfulCubeMapSampling() const
     return true;
 }
 
-bool ContextVk::shouldUseOldRewriteStructSamplers() const
-{
-    return mRenderer->getFeatures().forceOldRewriteStructSamplers.enabled;
-}
-
 angle::Result ContextVk::onBufferReleaseToExternal(const vk::BufferHelper &buffer)
 {
     if (mRenderPassCommands->usesBuffer(buffer))

src/libANGLE/renderer/vulkan/ContextVk.h

@@ -421,8 +421,6 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
 
     bool emulateSeamfulCubeMapSampling() const { return mEmulateSeamfulCubeMapSampling; }
 
-    bool useOldRewriteStructSamplers() const { return mUseOldRewriteStructSamplers; }
-
     const gl::Debug &getDebug() const { return mState.getDebug(); }
     const gl::OverlayType *getOverlay() const { return mState.getOverlay(); }
 
@@ -832,7 +830,6 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
     void flushGpuEvents(double nextSyncGpuTimestampS, double nextSyncCpuTimestampS);
     void handleDeviceLost();
     bool shouldEmulateSeamfulCubeMapSampling() const;
-    bool shouldUseOldRewriteStructSamplers() const;
     void clearAllGarbage();
     bool hasRecordedCommands();
     void dumpCommandStreamDiagnostics();
@@ -952,10 +949,6 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
     // Whether this context should do seamful cube map sampling emulation.
     bool mEmulateSeamfulCubeMapSampling;
 
-    // Whether this context should use the old version of the
-    // RewriteStructSamplers pass.
-    bool mUseOldRewriteStructSamplers;
-
     angle::PackedEnumMap<PipelineType, DriverUniformsDescriptorSet> mDriverUniforms;
 
     // This cache should also probably include the texture index (shader location) and array

src/libANGLE/renderer/vulkan/GlslangWrapperVk.cpp

@@ -28,7 +28,6 @@ GlslangSourceOptions GlslangWrapperVk::CreateSourceOptions(const angle::Features
 {
     GlslangSourceOptions options;
 
-    options.useOldRewriteStructSamplers = features.forceOldRewriteStructSamplers.enabled;
     options.supportsTransformFeedbackExtension =
         features.supportsTransformFeedbackExtension.enabled;
     options.emulateTransformFeedback = features.emulateTransformFeedback.enabled;

src/libANGLE/renderer/vulkan/ProgramExecutableVk.cpp

@@ -486,7 +486,6 @@ void ProgramExecutableVk::addAtomicCounterBufferDescriptorSetDesc(
 }
 
 void ProgramExecutableVk::addImageDescriptorSetDesc(const gl::ProgramExecutable &executable,
-                                                    bool useOldRewriteStructSamplers,
                                                     vk::DescriptorSetLayoutDesc *descOut)
 {
     const std::vector<gl::ImageBinding> &imageBindings = executable.getImageBindings();
@@ -498,27 +497,22 @@ void ProgramExecutableVk::addImageDescriptorSetDesc(const gl::ProgramExecutable
         uint32_t uniformIndex                = executable.getUniformIndexFromImageIndex(imageIndex);
         const gl::LinkedUniform &imageUniform = uniforms[uniformIndex];
 
-        std::string imageName = useOldRewriteStructSamplers
-                                    ? GetMappedSamplerNameOld(imageUniform.name)
-                                    : GlslangGetMappedSamplerName(imageUniform.name);
+        std::string imageName = GlslangGetMappedSamplerName(imageUniform.name);
 
         // The front-end always binds array image units sequentially.
         uint32_t arraySize = static_cast<uint32_t>(imageBinding.boundImageUnits.size());
 
-        if (!useOldRewriteStructSamplers)
+        // 2D arrays are split into multiple 1D arrays when generating LinkedUniforms. Since they
+        // are flattened into one array, ignore the nonzero elements and expand the array to the
+        // total array size.
+        if (gl::SamplerNameContainsNonZeroArrayElement(imageUniform.name))
         {
-            // 2D arrays are split into multiple 1D arrays when generating
-            // LinkedUniforms. Since they are flattened into one array, ignore the
-            // nonzero elements and expand the array to the total array size.
-            if (gl::SamplerNameContainsNonZeroArrayElement(imageUniform.name))
-            {
-                continue;
-            }
+            continue;
+        }
 
-            for (unsigned int outerArraySize : imageUniform.outerArraySizes)
-            {
-                arraySize *= outerArraySize;
-            }
+        for (unsigned int outerArraySize : imageUniform.outerArraySizes)
+        {
+            arraySize *= outerArraySize;
         }
 
         for (const gl::ShaderType shaderType : executable.getLinkedShaderStages())
@@ -542,7 +536,6 @@ void ProgramExecutableVk::addImageDescriptorSetDesc(const gl::ProgramExecutable
 
 void ProgramExecutableVk::addTextureDescriptorSetDesc(
     const gl::ProgramState &programState,
-    bool useOldRewriteStructSamplers,
     const gl::ActiveTextureArray<vk::TextureUnit> *activeTextures,
     vk::DescriptorSetLayoutDesc *descOut)
 {
@@ -556,27 +549,22 @@ void ProgramExecutableVk::addTextureDescriptorSetDesc(
         uint32_t uniformIndex = programState.getUniformIndexFromSamplerIndex(textureIndex);
         const gl::LinkedUniform &samplerUniform = uniforms[uniformIndex];
 
-        const std::string samplerName = useOldRewriteStructSamplers
-                                            ? GetMappedSamplerNameOld(samplerUniform.name)
-                                            : GlslangGetMappedSamplerName(samplerUniform.name);
+        const std::string samplerName = GlslangGetMappedSamplerName(samplerUniform.name);
 
         // The front-end always binds array sampler units sequentially.
         uint32_t arraySize = static_cast<uint32_t>(samplerBinding.boundTextureUnits.size());
 
-        if (!useOldRewriteStructSamplers)
+        // 2D arrays are split into multiple 1D arrays when generating LinkedUniforms. Since they
+        // are flattened into one array, ignore the nonzero elements and expand the array to the
+        // total array size.
+        if (gl::SamplerNameContainsNonZeroArrayElement(samplerUniform.name))
         {
-            // 2D arrays are split into multiple 1D arrays when generating
-            // LinkedUniforms. Since they are flattened into one array, ignore the
-            // nonzero elements and expand the array to the total array size.
-            if (gl::SamplerNameContainsNonZeroArrayElement(samplerUniform.name))
-            {
-                continue;
-            }
+            continue;
+        }
 
-            for (unsigned int outerArraySize : samplerUniform.outerArraySizes)
-            {
-                arraySize *= outerArraySize;
-            }
+        for (unsigned int outerArraySize : samplerUniform.outerArraySizes)
+        {
+            arraySize *= outerArraySize;
         }
 
         for (const gl::ShaderType shaderType : programState.getExecutable().getLinkedShaderStages())
@@ -857,8 +845,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout(
     {
         const gl::ProgramState *programState = programStates[shaderType];
         ASSERT(programState);
-        addImageDescriptorSetDesc(programState->getExecutable(),
-                                  contextVk->useOldRewriteStructSamplers(), &resourcesSetDesc);
+        addImageDescriptorSetDesc(programState->getExecutable(), &resourcesSetDesc);
     }
 
     ANGLE_TRY(contextVk->getDescriptorSetLayoutCache().getDescriptorSetLayout(
@@ -872,8 +859,7 @@ angle::Result ProgramExecutableVk::createPipelineLayout(
     {
         const gl::ProgramState *programState = programStates[shaderType];
         ASSERT(programState);
-        addTextureDescriptorSetDesc(*programState, contextVk->useOldRewriteStructSamplers(),
-                                    activeTextures, &texturesSetDesc);
+        addTextureDescriptorSetDesc(*programState, activeTextures, &texturesSetDesc);
     }
 
     ANGLE_TRY(contextVk->getDescriptorSetLayoutCache().getDescriptorSetLayout(
@@ -1221,7 +1207,6 @@ angle::Result ProgramExecutableVk::updateImagesDescriptorSet(
 
     const gl::ActiveTextureArray<TextureVk *> &activeImages = contextVk->getActiveImages();
 
-    bool useOldRewriteStructSamplers = contextVk->useOldRewriteStructSamplers();
     angle::HashMap<std::string, uint32_t> mappedImageNameToArrayOffset;
 
     // Write images.
@@ -1245,28 +1230,17 @@ angle::Result ProgramExecutableVk::updateImagesDescriptorSet(
         }
         ASSERT(descriptorSet != VK_NULL_HANDLE);
 
-        std::string mappedImageName;
-        if (!useOldRewriteStructSamplers)
-        {
-            mappedImageName = GlslangGetMappedSamplerName(imageUniform.name);
-        }
-        else
-        {
-            mappedImageName = imageUniform.mappedName;
-        }
+        std::string mappedImageName = GlslangGetMappedSamplerName(imageUniform.name);
 
         GetImageNameWithoutIndices(&mappedImageName);
 
         uint32_t arrayOffset = 0;
         uint32_t arraySize   = static_cast<uint32_t>(imageBinding.boundImageUnits.size());
 
-        if (!useOldRewriteStructSamplers)
-        {
-            arrayOffset = mappedImageNameToArrayOffset[mappedImageName];
-            // Front-end generates array elements in order, so we can just increment
-            // the offset each time we process a nested array.
-            mappedImageNameToArrayOffset[mappedImageName] += arraySize;
-        }
+        arrayOffset = mappedImageNameToArrayOffset[mappedImageName];
+        // Front-end generates array elements in order, so we can just increment the offset each
+        // time we process a nested array.
+        mappedImageNameToArrayOffset[mappedImageName] += arraySize;
 
         VkWriteDescriptorSet *writeInfos = contextVk->allocWriteDescriptorSets(arraySize);
 
@@ -1324,9 +1298,7 @@ angle::Result ProgramExecutableVk::updateImagesDescriptorSet(
             imageInfos[arrayElement].imageView   = imageView->getHandle();
             imageInfos[arrayElement].imageLayout = image->getCurrentLayout();
 
-            const std::string imageName = useOldRewriteStructSamplers
-                                              ? GetMappedSamplerNameOld(imageUniform.name)
-                                              : GlslangGetMappedSamplerName(imageUniform.name);
+            const std::string imageName = GlslangGetMappedSamplerName(imageUniform.name);
             const ShaderInterfaceVariableInfo &info = mVariableInfoMap.get(shaderType, imageName);
 
             writeInfos[arrayElement].sType            = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET;
@@ -1463,7 +1435,6 @@ angle::Result ProgramExecutableVk::updateTexturesDescriptorSet(ContextVk *contex
 
     const gl::ActiveTextureArray<vk::TextureUnit> &activeTextures = contextVk->getActiveTextures();
     bool emulateSeamfulCubeMapSampling = contextVk->emulateSeamfulCubeMapSampling();
-    bool useOldRewriteStructSamplers   = contextVk->useOldRewriteStructSamplers();
 
     gl::ShaderMap<const gl::ProgramState *> programStates;
     fillProgramStateMap(contextVk, &programStates);
@@ -1509,13 +1480,10 @@ angle::Result ProgramExecutableVk::updateTexturesDescriptorSet(ContextVk *contex
             uint32_t arrayOffset = 0;
             uint32_t arraySize   = static_cast<uint32_t>(samplerBinding.boundTextureUnits.size());
 
-            if (!useOldRewriteStructSamplers)
-            {
-                arrayOffset = mappedSamplerNameToArrayOffset[mappedSamplerName];
-                // Front-end generates array elements in order, so we can just increment
-                // the offset each time we process a nested array.
-                mappedSamplerNameToArrayOffset[mappedSamplerName] += arraySize;
-            }
+            arrayOffset = mappedSamplerNameToArrayOffset[mappedSamplerName];
+            // Front-end generates array elements in order, so we can just increment the offset each
+            // time we process a nested array.
+            mappedSamplerNameToArrayOffset[mappedSamplerName] += arraySize;
 
             VkWriteDescriptorSet *writeInfos = contextVk->allocWriteDescriptorSets(arraySize);
 
@@ -1582,10 +1550,7 @@ angle::Result ProgramExecutableVk::updateTexturesDescriptorSet(ContextVk *contex
                 {
                     imageInfos[arrayElement].sampler = textureVk->getSampler().get().getHandle();
                 }
-                const std::string samplerName =
-                    contextVk->getRenderer()->getFeatures().forceOldRewriteStructSamplers.enabled
-                        ? GetMappedSamplerNameOld(samplerUniform.name)
-                        : GlslangGetMappedSamplerName(samplerUniform.name);
+                const std::string samplerName = GlslangGetMappedSamplerName(samplerUniform.name);
                 const ShaderInterfaceVariableInfo &info =
                     mVariableInfoMap.get(shaderType, samplerName);
 

src/libANGLE/renderer/vulkan/ProgramExecutableVk.h

@@ -199,10 +199,8 @@ class ProgramExecutableVk
         const gl::ShaderType shaderType,
         vk::DescriptorSetLayoutDesc *descOut);
     void addImageDescriptorSetDesc(const gl::ProgramExecutable &executable,
-                                   bool useOldRewriteStructSamplers,
                                    vk::DescriptorSetLayoutDesc *descOut);
     void addTextureDescriptorSetDesc(const gl::ProgramState &programState,
-                                     bool useOldRewriteStructSamplers,
                                      const gl::ActiveTextureArray<vk::TextureUnit> *activeTextures,
                                      vk::DescriptorSetLayoutDesc *descOut);
 

src/libANGLE/renderer/vulkan/RendererVk.cpp

@@ -1377,7 +1377,6 @@ angle::Result RendererVk::initializeDevice(DisplayVk *displayVk, uint32_t queueF
     // Used to support EXT_gpu_shader5:
     enabledFeatures.features.shaderUniformBufferArrayDynamicIndexing =
         mPhysicalDeviceFeatures.shaderUniformBufferArrayDynamicIndexing;
-    // Used to support EXT_gpu_shader5 and sampler array of array emulation:
     enabledFeatures.features.shaderSampledImageArrayDynamicIndexing =
         mPhysicalDeviceFeatures.shaderSampledImageArrayDynamicIndexing;
     // Used to support atomic counter emulation:
@@ -1946,10 +1945,6 @@ void RendererVk::initFeatures(DisplayVk *displayVk,
     ANGLE_FEATURE_CONDITION(&mFeatures, bindEmptyForUnusedDescriptorSets,
                             IsAndroid() && isQualcomm);
 
-    ANGLE_FEATURE_CONDITION(
-        &mFeatures, forceOldRewriteStructSamplers,
-        !mPhysicalDeviceFeatures.shaderSampledImageArrayDynamicIndexing || isQualcomm);
-
     ANGLE_FEATURE_CONDITION(&mFeatures, perFrameWindowSizeQuery,
                             isIntel || (IsWindows() && isAMD) || IsFuchsia());
 

src/libANGLE/renderer/vulkan/ShaderVk.cpp

@@ -58,11 +58,6 @@ std::shared_ptr<WaitableCompileEvent> ShaderVk::compile(const gl::Context *conte
         compileOptions |= SH_EMULATE_SEAMFUL_CUBE_MAP_SAMPLING;
     }
 
-    if (contextVk->useOldRewriteStructSamplers())
-    {
-        compileOptions |= SH_USE_OLD_REWRITE_STRUCT_SAMPLERS;
-    }
-
     if (!contextVk->getFeatures().enablePrecisionQualifiers.enabled)
     {
         compileOptions |= SH_IGNORE_PRECISION_QUALIFIERS;

src/tests/deqp_support/deqp_gles31_test_expectations.txt

@@ -169,10 +169,6 @@
 // Not failing in last official run, but failed recently:
 4110 SWIFTSHADER : dEQP-GLES31.functional.shaders.helper_invocation.* = FAIL
 
-// Vulkan Android doesn't support array of array of opaque uniforms. Emulation is not yet done for images
-3881 VULKAN ANDROID : dEQP-GLES31.functional.program_interface_query.uniform.location.default_block.array.array.image_2d = SKIP
-3881 VULKAN ANDROID : dEQP-GLES31.functional.program_interface_query.uniform.location.default_block.array.array.iimage_2d_array = SKIP
-
 ////
 //// General Vulkan expectations
 ////
@@ -228,16 +224,17 @@
 // Seem to trigger LowMemoryKiller when run in a certain sequence
 5185 VULKAN ANDROID : dEQP-GLES31.functional.atomic_counter.* = SKIP
 
-// Arrays of opaque types as function parameters (possibly related to issue 3882)
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_literal_vertex = FAIL
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_literal_fragment = FAIL
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_literal_compute = FAIL
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_expression_vertex = FAIL
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_expression_fragment = FAIL
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_expression_compute = FAIL
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.*uniform_vertex = FAIL
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.*uniform_fragment = FAIL
-2703 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.*uniform_compute = FAIL
+// Arrays of atomic counters not supported on Android (lacking Vulkan feature)
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_literal_vertex = FAIL
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_literal_fragment = FAIL
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_literal_compute = FAIL
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_expression_vertex = FAIL
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_expression_fragment = FAIL
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.const_expression_compute = FAIL
+
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.*uniform_vertex = FAIL
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.*uniform_fragment = FAIL
+3881 VULKAN ANDROID : dEQP-GLES31.functional.shaders.opaque_type_indexing.atomic_counter.*uniform_compute = FAIL
 
 // SSBO synchronization:
 4097 VULKAN PIXEL2ORXL : dEQP-GLES31.functional.synchronization.in_invocation.ssbo_alias_overwrite = FAIL
@@ -255,10 +252,6 @@
 // Need to support non-color when staging image updates
 4080 ANDROID VULKAN : dEQP-GLES31.functional.stencil_texturing.misc.base_level = SKIP
 
-// Android uses "old" sampler rewrite that doesn't support array of arrays
-2703 VULKAN ANDROID : dEQP-GLES31.functional.program_interface_query.uniform.location.default_block.array.array.*sampler* = SKIP
-2703 VULKAN ANDROID : dEQP-GLES31.functional.uniform_location.nested_array.*sampler2D* = SKIP
-
 // Fail on tests that use an unsupported extension!
 4371 VULKAN ANDROID : dEQP-GLES31.functional.shaders.implicit_conversions.es31.arithmetic.input_before_literal.add.int_to_vec3_vertex = FAIL
 4371 VULKAN ANDROID : dEQP-GLES31.functional.shaders.implicit_conversions.es31.arithmetic.input_before_literal.add.int_to_uvec3_vertex = FAIL

src/tests/deqp_support/deqp_khr_gles31_test_expectations.txt

@@ -103,9 +103,6 @@
 // Fails to link the shader program on Pixel2 and Pixel2 XL
 3588 VULKAN PIXEL2ORXL : KHR-GLES31.core.sample_variables.mask.rgba8.samples_*.mask_* = FAIL
 
-// Android uses "old" sampler rewrite that doesn't support array of arrays
-2703 VULKAN ANDROID : KHR-GLES31.core.arrays_of_arrays.* = SKIP
-
 // Failing on Android with SwiftShader
 4300 SWIFTSHADER ANDROID : KHR-GLES31.core.shader_integer_mix.mix-bvec4 = FAIL
 4300 SWIFTSHADER ANDROID : KHR-GLES31.core.shader_integer_mix.mix-ivec4 = FAIL

src/tests/deqp_support/deqp_khr_gles32_test_expectations.txt

@@ -55,8 +55,8 @@
 // Android failures
 
 // Dynamic indexing features not supported on Qualcomm
-2703 VULKAN ANDROID : KHR-GLES32.core.gpu_shader5.atomic_counters_array_indexing = FAIL
-2703 VULKAN ANDROID : KHR-GLES32.core.gpu_shader5.images_array_indexing = FAIL
+5435 VULKAN ANDROID : KHR-GLES32.core.gpu_shader5.atomic_counters_array_indexing = FAIL
+5435 VULKAN ANDROID : KHR-GLES32.core.gpu_shader5.images_array_indexing = FAIL
 
 // Texture buffer failures
 5392 VULKAN ANDROID : KHR-GLES32.core.texture_buffer.texture_buffer_active_uniform_validation_compute_shader = FAIL

src/tests/gl_tests/GLSLTest.cpp

@@ -3273,7 +3273,7 @@ TEST_P(GLSLTest_ES31, AtomicCounterArrayLength)
 precision mediump float;
 layout(local_size_x=1) in;
 
-layout(binding = 0) uniform atomic_uint ac1[2][3];
+layout(binding = 0) uniform atomic_uint ac1[2][3][4];
 uniform uint testSideEffectValue;
 
 layout(binding = 1, std140) buffer Result
@@ -3288,7 +3288,7 @@ void main() {
         passed = false;
     }
     uint value = 0u;
-    if (ac1[(value = testSideEffectValue)].length() != 3)
+    if (ac1[value = testSideEffectValue].length() != 3)
     {
         passed = false;
     }
@@ -3296,6 +3296,14 @@ void main() {
     {
         passed = false;
     }
+    if (ac1[1][value = testSideEffectValue + 1u].length() != 4)
+    {
+        passed = false;
+    }
+    if (value != testSideEffectValue + 1u)
+    {
+        passed = false;
+    }
     result.value = passed ? 255u : 127u;
 })";
 
@@ -3329,9 +3337,10 @@ void main() {
                  GL_STATIC_DRAW);
     glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, shaderStorageBuffer);
 
-    GLint uniformLocation = glGetUniformLocation(program.get(), "testSideEffectValue");
+    GLint uniformLocation = glGetUniformLocation(program, "testSideEffectValue");
     EXPECT_NE(uniformLocation, -1);
-    glUniform1i(uniformLocation, kUniformTestValue);
+    glUniform1ui(uniformLocation, kUniformTestValue);
+    EXPECT_GL_NO_ERROR();
 
     glDispatchCompute(1, 1, 1);
 
@@ -3402,10 +3411,6 @@ void main()
 // Test that inactive samplers in structs don't cause any errors.
 TEST_P(GLSLTest_ES31, InactiveSamplersInStructCS)
 {
-    // While the sampler is being extracted and declared outside of the struct, it's not removed
-    // from the struct definition.  http://anglebug.com/4211
-    ANGLE_SKIP_TEST_IF(IsVulkan() || IsMetal());
-
     constexpr char kCS[] = R"(#version 310 es
 layout(local_size_x=1, local_size_y=1, local_size_z=1) in;
 struct S
@@ -3532,10 +3537,6 @@ TEST_P(GLSLTest_ES31, ArraysOfArraysBlockBasicType)
 // Test that arrays of arrays of samplers work as expected.
 TEST_P(GLSLTest_ES31, ArraysOfArraysSampler)
 {
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     constexpr char kFS[] =
         "#version 310 es\n"
         "precision mediump float;\n"
@@ -3588,10 +3589,6 @@ TEST_P(GLSLTest_ES31, ArraysOfArraysImage)
     // http://anglebug.com/5072
     ANGLE_SKIP_TEST_IF(IsIntel() && IsLinux() && IsOpenGL());
 
-    // anglebug.com/2703 - QC doesn't support arrays of image as parameters,
-    // so image array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     // Fails on D3D due to mistranslation.
     ANGLE_SKIP_TEST_IF(IsD3D());
 
@@ -3706,10 +3703,6 @@ TEST_P(GLSLTest_ES31, StructArraySampler)
 // Test that arrays of arrays of samplers inside structs work as expected.
 TEST_P(GLSLTest_ES31, StructArrayArraySampler)
 {
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     constexpr char kFS[] =
         "#version 310 es\n"
         "precision mediump float;\n"
@@ -3760,10 +3753,6 @@ TEST_P(GLSLTest_ES31, StructArrayArraySampler)
 // Test that an array of structs with arrays of arrays of samplers works.
 TEST_P(GLSLTest_ES31, ArrayStructArrayArraySampler)
 {
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     GLint numTextures;
     glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &numTextures);
     ANGLE_SKIP_TEST_IF(numTextures < 2 * (2 * 2 + 2 * 2));
@@ -3832,10 +3821,6 @@ TEST_P(GLSLTest_ES31, ArrayStructArrayArraySampler)
 // Test that a complex chain of structs and arrays of samplers works as expected.
 TEST_P(GLSLTest_ES31, ComplexStructArraySampler)
 {
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     GLint numTextures;
     glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &numTextures);
     ANGLE_SKIP_TEST_IF(numTextures < 2 * 3 * (2 + 3));
@@ -3921,10 +3906,6 @@ TEST_P(GLSLTest_ES31, ComplexStructArraySampler)
 
 TEST_P(GLSLTest_ES31, ArraysOfArraysStructDifferentTypesSampler)
 {
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     GLint numTextures;
     glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &numTextures);
     ANGLE_SKIP_TEST_IF(numTextures < 3 * (2 + 2));
@@ -4001,9 +3982,7 @@ TEST_P(GLSLTest_ES31, ParameterArraysOfArraysSampler)
 {
     // anglebug.com/3832 - no sampler array params on Android
     ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
+
     constexpr char kFS[] =
         "#version 310 es\n"
         "precision mediump float;\n"
@@ -4063,9 +4042,7 @@ TEST_P(GLSLTest_ES31, ParameterStructArrayArraySampler)
 {
     // anglebug.com/3832 - no sampler array params on Android
     ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
+
     constexpr char kFS[] =
         "#version 310 es\n"
         "precision mediump float;\n"
@@ -4126,9 +4103,7 @@ TEST_P(GLSLTest_ES31, ParameterArrayArrayStructArrayArraySampler)
 {
     // anglebug.com/3832 - no sampler array params on Android
     ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
+
     GLint numTextures;
     glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &numTextures);
     ANGLE_SKIP_TEST_IF(numTextures < 3 * 2 * 2 * 2);
@@ -4202,10 +4177,6 @@ TEST_P(GLSLTest_ES31, ParameterArrayArrayStructArrayArraySampler)
 // Test that 3D arrays with sub-arrays passed as parameters works as expected.
 TEST_P(GLSLTest_ES31, ParameterArrayArrayArraySampler)
 {
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     GLint numTextures;
     glGetIntegerv(GL_MAX_TEXTURE_IMAGE_UNITS, &numTextures);
     ANGLE_SKIP_TEST_IF(numTextures < 2 * 3 * 4 + 4);
@@ -4343,10 +4314,6 @@ TEST_P(GLSLTest_ES31, ArraysOfArraysNameCollisionSampler)
 // Test that regular arrays are unmodified.
 TEST_P(GLSLTest_ES31, BasicTypeArrayAndArrayOfSampler)
 {
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     constexpr char kFS[] =
         "#version 310 es\n"
         "precision mediump sampler2D;\n"
@@ -5476,9 +5443,6 @@ TEST_P(GLSLTest, StructWithSamplerArrayAsFunctionArg)
     // Shader failed to compile on Nexus devices. http://anglebug.com/2114
     ANGLE_SKIP_TEST_IF(IsNexus5X() && IsAdreno() && IsOpenGLES());
 
-    // TODO(jmadill): Fix on Android/vulkan if possible. http://anglebug.com/2703
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     constexpr char kFS[] =
         "precision mediump float;\n"
         "struct S\n"
@@ -7656,10 +7620,6 @@ TEST_P(GLSLTest, MemoryExhaustedTest)
 // Test that inactive samplers in structs don't cause any errors.
 TEST_P(GLSLTest, InactiveSamplersInStruct)
 {
-    // While the sampler is being extracted and declared outside of the struct, it's not removed
-    // from the struct definition.  http://anglebug.com/4211
-    ANGLE_SKIP_TEST_IF(IsVulkan() || IsMetal());
-
     constexpr char kVS[] = R"(attribute vec4 a_position;
 void main() {
   gl_Position = a_position;
@@ -8533,10 +8493,6 @@ TEST_P(GLSLTest_ES31, MixOfResourcesAsFunctionArgs)
     // anglebug.com/3832 - no sampler array params on Android
     ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
 
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     constexpr char kComputeShader[] = R"(#version 310 es
 layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
 
@@ -8644,10 +8600,6 @@ TEST_P(GLSLTest_ES31, ArrayOfArrayOfSamplerAsFunctionParameterIndexedWithSideEff
     // anglebug.com/3832 - no sampler array params on Android
     ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
 
-    // anglebug.com/2703 - QC doesn't support arrays of samplers as parameters,
-    // so sampler array of array handling is disabled
-    ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
-
     // Skip if EXT_gpu_shader5 is not enabled.
     ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_gpu_shader5"));
 
@@ -8747,6 +8699,446 @@ void main(void)
     glUnmapBuffer(GL_SHADER_STORAGE_BUFFER);
 }
 
+// Test that array of array of samplers can be indexed correctly with dynamic indices.
+TEST_P(GLSLTest_ES31, ArrayOfArrayOfSamplerDynamicIndex)
+{
+    // Skip if EXT_gpu_shader5 is not enabled.
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_gpu_shader5"));
+
+    int maxTextureImageUnits = 0;
+    glGetIntegerv(GL_MAX_COMPUTE_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits);
+    ANGLE_SKIP_TEST_IF(maxTextureImageUnits < 24);
+
+    // anglebug.com/3832 - no sampler array params on Android
+    ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
+
+    // http://anglebug.com/5546
+    ANGLE_SKIP_TEST_IF(IsWindows() && IsIntel() && IsOpenGL());
+
+    constexpr char kComputeShader[] = R"(#version 310 es
+#extension GL_EXT_gpu_shader5 : require
+
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+
+layout(binding = 1, std430) buffer Output {
+  uint success;
+} outbuf;
+
+uniform sampler2D smplr[2][3][4];
+layout(binding=0) uniform atomic_uint ac;
+
+bool sampler1DAndAtomicCounter(uvec4 sExpect, in sampler2D s[4], in atomic_uint a, uint aExpect)
+{
+    uvec4 sResult = uvec4(uint(texture(s[0], vec2(0.5, 0.5)).x * 255.0),
+                          uint(texture(s[1], vec2(0.5, 0.5)).x * 255.0),
+                          uint(texture(s[2], vec2(0.5, 0.5)).x * 255.0),
+                          uint(texture(s[3], vec2(0.5, 0.5)).x * 255.0));
+    uint aResult = atomicCounter(a);
+
+    return sExpect == sResult && aExpect == aResult;
+}
+
+bool sampler3DAndAtomicCounter(in sampler2D s[2][3][4], uint aInitial, in atomic_uint a)
+{
+    bool success = true;
+    // [0][0]
+    success = sampler1DAndAtomicCounter(uvec4(0, 8, 16, 24),
+                    s[atomicCounterIncrement(ac)][0], a, aInitial + 1u) && success;
+    // [1][0]
+    success = sampler1DAndAtomicCounter(uvec4(96, 104, 112, 120),
+                    s[atomicCounterIncrement(ac)][0], a, aInitial + 2u) && success;
+    // [0][1]
+    success = sampler1DAndAtomicCounter(uvec4(32, 40, 48, 56),
+                    s[0][atomicCounterIncrement(ac) - 1u], a, aInitial + 3u) && success;
+    // [0][2]
+    success = sampler1DAndAtomicCounter(uvec4(64, 72, 80, 88),
+                    s[0][atomicCounterIncrement(ac) - 1u], a, aInitial + 4u) && success;
+    // [1][1]
+    success = sampler1DAndAtomicCounter(uvec4(128, 136, 144, 152),
+                    s[1][atomicCounterIncrement(ac) - 3u], a, aInitial + 5u) && success;
+    // [1][2]
+    uint acValue = atomicCounterIncrement(ac);  // Returns 5
+    success = sampler1DAndAtomicCounter(uvec4(160, 168, 176, 184),
+                    s[acValue - 4u][atomicCounterIncrement(ac) - 4u], a, aInitial + 7u) && success;
+
+    return success;
+}
+
+void main(void)
+{
+    outbuf.success = uint(sampler3DAndAtomicCounter(smplr, 0u, ac));
+}
+)";
+    ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShader);
+    EXPECT_GL_NO_ERROR();
+
+    glUseProgram(program);
+
+    unsigned int outputInitData = 0x12345678u;
+    GLBuffer outputBuffer;
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, outputBuffer);
+    glBufferData(GL_SHADER_STORAGE_BUFFER, sizeof(outputInitData), &outputInitData, GL_STATIC_DRAW);
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, outputBuffer);
+    EXPECT_GL_NO_ERROR();
+
+    unsigned int acData = 0u;
+    GLBuffer atomicCounterBuffer;
+    glBindBuffer(GL_ATOMIC_COUNTER_BUFFER, atomicCounterBuffer);
+    glBufferData(GL_ATOMIC_COUNTER_BUFFER, sizeof(acData), &acData, GL_STATIC_DRAW);
+    glBindBufferBase(GL_ATOMIC_COUNTER_BUFFER, 0, atomicCounterBuffer);
+    EXPECT_GL_NO_ERROR();
+
+    const std::array<GLColor, 24> kTextureData = {
+        GLColor(0, 0, 0, 0),   GLColor(8, 0, 0, 0),   GLColor(16, 0, 0, 0),  GLColor(24, 0, 0, 0),
+        GLColor(32, 0, 0, 0),  GLColor(40, 0, 0, 0),  GLColor(48, 0, 0, 0),  GLColor(56, 0, 0, 0),
+        GLColor(64, 0, 0, 0),  GLColor(72, 0, 0, 0),  GLColor(80, 0, 0, 0),  GLColor(88, 0, 0, 0),
+        GLColor(96, 0, 0, 0),  GLColor(104, 0, 0, 0), GLColor(112, 0, 0, 0), GLColor(120, 0, 0, 0),
+        GLColor(128, 0, 0, 0), GLColor(136, 0, 0, 0), GLColor(144, 0, 0, 0), GLColor(152, 0, 0, 0),
+        GLColor(160, 0, 0, 0), GLColor(168, 0, 0, 0), GLColor(176, 0, 0, 0), GLColor(184, 0, 0, 0),
+    };
+    GLTexture textures[2][3][4];
+
+    for (int dim1 = 0; dim1 < 2; ++dim1)
+    {
+        for (int dim2 = 0; dim2 < 3; ++dim2)
+        {
+            for (int dim3 = 0; dim3 < 4; ++dim3)
+            {
+                int textureUnit = (dim1 * 3 + dim2) * 4 + dim3;
+                glActiveTexture(GL_TEXTURE0 + textureUnit);
+                glBindTexture(GL_TEXTURE_2D, textures[dim1][dim2][dim3]);
+                glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                             &kTextureData[textureUnit]);
+                glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+                glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+                std::stringstream uniformName;
+                uniformName << "smplr[" << dim1 << "][" << dim2 << "][" << dim3 << "]";
+                GLint samplerLocation = glGetUniformLocation(program, uniformName.str().c_str());
+                EXPECT_NE(samplerLocation, -1);
+                glUniform1i(samplerLocation, textureUnit);
+            }
+        }
+    }
+    ASSERT_GL_NO_ERROR();
+
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+
+    glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT);
+
+    // read back
+    const GLuint *ptr = reinterpret_cast<const GLuint *>(
+        glMapBufferRange(GL_SHADER_STORAGE_BUFFER, 0, sizeof(outputInitData), GL_MAP_READ_BIT));
+    EXPECT_EQ(ptr[0], 1u);
+
+    glUnmapBuffer(GL_SHADER_STORAGE_BUFFER);
+}
+
+// Test that array of array of samplers can be indexed correctly with dynamic indices.  Uses
+// samplers in structs.
+TEST_P(GLSLTest_ES31, ArrayOfArrayOfSamplerInStructDynamicIndex)
+{
+    // Skip if EXT_gpu_shader5 is not enabled.
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_gpu_shader5"));
+
+    int maxTextureImageUnits = 0;
+    glGetIntegerv(GL_MAX_COMPUTE_TEXTURE_IMAGE_UNITS, &maxTextureImageUnits);
+    ANGLE_SKIP_TEST_IF(maxTextureImageUnits < 24);
+
+    // http://anglebug.com/5072
+    ANGLE_SKIP_TEST_IF(IsIntel() && IsLinux() && IsOpenGL());
+
+    // anglebug.com/3832 - no sampler array params on Android
+    ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
+
+    // http://anglebug.com/5546
+    ANGLE_SKIP_TEST_IF(IsWindows() && IsIntel() && IsOpenGL());
+
+    constexpr char kComputeShader[] = R"(#version 310 es
+#extension GL_EXT_gpu_shader5 : require
+
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+
+layout(binding = 1, std430) buffer Output {
+  uint success;
+} outbuf;
+
+struct I
+{
+    uint index;
+};
+
+struct S
+{
+    sampler2D smplr[4];
+    I nested;
+};
+
+struct T
+{
+    S nested[3];
+    uint tIndex;
+};
+
+uniform T u[2];
+
+uint getValue(in sampler2D s)
+{
+    return uint(texture(s, vec2(0.5, 0.5)).x * 255.0);
+}
+
+bool sampler1DTest(uvec4 sExpect, in sampler2D s[4])
+{
+    uvec4 sResult = uvec4(getValue(s[0]), getValue(s[1]),
+                          getValue(s[2]), getValue(s[3]));
+
+    return sExpect == sResult;
+}
+
+bool samplerTest(T t, uint N)
+{
+    // u[N].tIndex == 0 + N*4
+    // u[N].nested[0].nested.index == 1 + N*4
+    // u[N].nested[1].nested.index == 2 + N*4
+    // u[N].nested[2].nested.index == 3 + N*4
+
+    uvec4 colorOffset = N * 3u * 4u * uvec4(8);
+
+    bool success = true;
+    // [N][0]
+    success = sampler1DTest(uvec4(0, 8, 16, 24) + colorOffset,
+                    t.nested[t.nested[0].nested.index - t.tIndex - 1u].smplr) && success;
+    // [N][1]
+    success = sampler1DTest(uvec4(32, 40, 48, 56) + colorOffset,
+                    t.nested[t.nested[1].nested.index - t.tIndex - 1u].smplr) && success;
+    // [N][2]
+    success = sampler1DTest(uvec4(64, 72, 80, 88) + colorOffset,
+                    t.nested[t.nested[2].nested.index - t.tIndex - 1u].smplr) && success;
+
+    return success;
+}
+
+bool uniformTest(T t, uint N)
+{
+    // Also verify that expressions that involve structs-with-samplers are correct when not
+    // referecing the sampler.
+
+    bool success = true;
+    success = (t.nested[0].nested.index - t.tIndex == 1u) && success;
+    success = (t.nested[1].nested.index - t.tIndex == 2u) && success;
+    success = (t.nested[2].nested.index - t.tIndex == 3u) && success;
+
+    success = (t.nested[t.nested[0].nested.index - t.tIndex - 1u].nested.index - t.tIndex == 1u)
+                && success;
+    success = (t.nested[t.nested[0].nested.index - t.tIndex     ].nested.index - t.tIndex == 2u)
+                && success;
+    success = (t.nested[t.nested[0].nested.index - t.tIndex + 1u].nested.index - t.tIndex == 3u)
+                && success;
+
+    success = (t.nested[
+                          t.nested[
+                                     t.nested[2].nested.index - t.tIndex - 1u  // 2
+                                  ].nested.index - t.tIndex - 2u               // 1
+                       ].nested.index - t.tIndex                               // 2
+                == 2u) && success;
+
+    return success;
+}
+
+void main(void)
+{
+    bool success = samplerTest(u[0], 0u) && samplerTest(u[1], 1u)
+                    && uniformTest(u[0], 0u) && uniformTest(u[1], 1u);
+    outbuf.success = uint(success);
+}
+)";
+    ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShader);
+    EXPECT_GL_NO_ERROR();
+
+    glUseProgram(program);
+
+    unsigned int outputInitData = 0x12345678u;
+    GLBuffer outputBuffer;
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, outputBuffer);
+    glBufferData(GL_SHADER_STORAGE_BUFFER, sizeof(outputInitData), &outputInitData, GL_STATIC_DRAW);
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, outputBuffer);
+    EXPECT_GL_NO_ERROR();
+
+    const std::array<GLColor, 24> kTextureData = {
+        GLColor(0, 0, 0, 0),   GLColor(8, 0, 0, 0),   GLColor(16, 0, 0, 0),  GLColor(24, 0, 0, 0),
+        GLColor(32, 0, 0, 0),  GLColor(40, 0, 0, 0),  GLColor(48, 0, 0, 0),  GLColor(56, 0, 0, 0),
+        GLColor(64, 0, 0, 0),  GLColor(72, 0, 0, 0),  GLColor(80, 0, 0, 0),  GLColor(88, 0, 0, 0),
+        GLColor(96, 0, 0, 0),  GLColor(104, 0, 0, 0), GLColor(112, 0, 0, 0), GLColor(120, 0, 0, 0),
+        GLColor(128, 0, 0, 0), GLColor(136, 0, 0, 0), GLColor(144, 0, 0, 0), GLColor(152, 0, 0, 0),
+        GLColor(160, 0, 0, 0), GLColor(168, 0, 0, 0), GLColor(176, 0, 0, 0), GLColor(184, 0, 0, 0),
+    };
+    GLTexture textures[2][3][4];
+
+    for (int dim1 = 0; dim1 < 2; ++dim1)
+    {
+        for (int dim2 = 0; dim2 < 3; ++dim2)
+        {
+            for (int dim3 = 0; dim3 < 4; ++dim3)
+            {
+                int textureUnit = (dim1 * 3 + dim2) * 4 + dim3;
+                glActiveTexture(GL_TEXTURE0 + textureUnit);
+                glBindTexture(GL_TEXTURE_2D, textures[dim1][dim2][dim3]);
+                glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                             &kTextureData[textureUnit]);
+                glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+                glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+                std::stringstream uniformName;
+                uniformName << "u[" << dim1 << "].nested[" << dim2 << "].smplr[" << dim3 << "]";
+                GLint samplerLocation = glGetUniformLocation(program, uniformName.str().c_str());
+                EXPECT_NE(samplerLocation, -1);
+                glUniform1i(samplerLocation, textureUnit);
+            }
+
+            std::stringstream uniformName;
+            uniformName << "u[" << dim1 << "].nested[" << dim2 << "].nested.index";
+            GLint nestedIndexLocation = glGetUniformLocation(program, uniformName.str().c_str());
+            EXPECT_NE(nestedIndexLocation, -1);
+            glUniform1ui(nestedIndexLocation, dim1 * 4 + dim2 + 1);
+        }
+
+        std::stringstream uniformName;
+        uniformName << "u[" << dim1 << "].tIndex";
+        GLint indexLocation = glGetUniformLocation(program, uniformName.str().c_str());
+        EXPECT_NE(indexLocation, -1);
+        glUniform1ui(indexLocation, dim1 * 4);
+    }
+    ASSERT_GL_NO_ERROR();
+
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+
+    glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT);
+
+    // read back
+    const GLuint *ptr = reinterpret_cast<const GLuint *>(
+        glMapBufferRange(GL_SHADER_STORAGE_BUFFER, 0, sizeof(outputInitData), GL_MAP_READ_BIT));
+    EXPECT_EQ(ptr[0], 1u);
+
+    glUnmapBuffer(GL_SHADER_STORAGE_BUFFER);
+}
+
+// Test that array of array of samplers work when indexed with an expression that's derived from an
+// array of array of samplers.
+TEST_P(GLSLTest_ES31, ArrayOfArrayOfSamplerIndexedWithArrayOfArrayOfSamplers)
+{
+    // Skip if EXT_gpu_shader5 is not enabled.
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_gpu_shader5"));
+
+    // anglebug.com/3832 - no sampler array params on Android
+    ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGLES());
+
+    constexpr char kComputeShader[] = R"(#version 310 es
+#extension GL_EXT_gpu_shader5 : require
+
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+
+layout(binding = 1, std430) buffer Output {
+  uint success;
+} outbuf;
+
+uniform sampler2D smplr[2][3];
+
+uint getValue(in sampler2D s)
+{
+    return uint(texture(s, vec2(0.5, 0.5)).x * 255.0);
+}
+
+bool runTest(in sampler2D s[2][3])
+{
+    // s[0][0] should contain 2
+    // s[0][1] should contain 0
+    // s[0][2] should contain 1
+    // s[1][0] should contain 1
+    // s[1][1] should contain 2
+    // s[1][2] should contain 0
+
+    uint result = getValue(
+                       s[
+                           getValue(
+                                s[
+                                    getValue(s[0][1])   // 0
+                                ][
+                                    getValue(s[0][0])   // 2
+                                ]
+                           )                      // s[0][2] -> 1
+                       ][
+                           getValue(
+                                s[
+                                    getValue(s[1][0])   // 1
+                                ][
+                                    getValue(s[1][1])   // 2
+                                ]
+                           )                      // s[1][2] -> 0
+                       ]
+                  );                      // s[1][0] -> 1
+
+    return result == 1u;
+}
+
+void main(void)
+{
+    outbuf.success = uint(runTest(smplr));
+}
+)";
+    ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShader);
+    EXPECT_GL_NO_ERROR();
+
+    glUseProgram(program);
+
+    unsigned int outputInitData = 0x12345678u;
+    GLBuffer outputBuffer;
+    glBindBuffer(GL_SHADER_STORAGE_BUFFER, outputBuffer);
+    glBufferData(GL_SHADER_STORAGE_BUFFER, sizeof(outputInitData), &outputInitData, GL_STATIC_DRAW);
+    glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, outputBuffer);
+    EXPECT_GL_NO_ERROR();
+
+    const std::array<GLColor, 6> kTextureData = {
+        GLColor(2, 0, 0, 0), GLColor(0, 0, 0, 0), GLColor(1, 0, 0, 0),
+        GLColor(1, 0, 0, 0), GLColor(2, 0, 0, 0), GLColor(0, 0, 0, 0),
+    };
+    GLTexture textures[2][3];
+
+    for (int dim1 = 0; dim1 < 2; ++dim1)
+    {
+        for (int dim2 = 0; dim2 < 3; ++dim2)
+        {
+            int textureUnit = dim1 * 3 + dim2;
+            glActiveTexture(GL_TEXTURE0 + textureUnit);
+            glBindTexture(GL_TEXTURE_2D, textures[dim1][dim2]);
+            glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                         &kTextureData[textureUnit]);
+            glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+            glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+            std::stringstream uniformName;
+            uniformName << "smplr[" << dim1 << "][" << dim2 << "]";
+            GLint samplerLocation = glGetUniformLocation(program, uniformName.str().c_str());
+            EXPECT_NE(samplerLocation, -1);
+            glUniform1i(samplerLocation, textureUnit);
+        }
+    }
+    ASSERT_GL_NO_ERROR();
+
+    glDispatchCompute(1, 1, 1);
+    EXPECT_GL_NO_ERROR();
+
+    glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT);
+
+    // read back
+    const GLuint *ptr = reinterpret_cast<const GLuint *>(
+        glMapBufferRange(GL_SHADER_STORAGE_BUFFER, 0, sizeof(outputInitData), GL_MAP_READ_BIT));
+    EXPECT_EQ(ptr[0], 1u);
+
+    glUnmapBuffer(GL_SHADER_STORAGE_BUFFER);
+}
+
 // Test that multiple nested assignments are handled correctly.
 TEST_P(GLSLTest_ES31, MixedRowAndColumnMajorMatrices_WriteSideEffect)
 {