Vulkan: Fix compound nested sampler struct params.

src/compiler/translator/tree_ops/RewriteStructSamplers.cpp

@@ -16,6 +16,71 @@ 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>();
 
@@ -214,7 +279,25 @@ class Traverser final : public TIntermTraverser
             const TType &fieldType = *field->type();
             if (!fieldType.isSampler() && !isRemovedStructType(fieldType))
             {
-                TType *newType = new TType(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);
@@ -416,21 +499,6 @@ class Traverser final : public TIntermTraverser
         virtual void visitStructParam(const TFunction *function, size_t paramIndex)            = 0;
         virtual void visitNonStructParam(const TFunction *function, size_t paramIndex)         = 0;
 
-      protected:
-        // 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);
-
-            ASSERT(!structType->isStructureContainingSamplers());
-
-            return structType;
-        }
-
       private:
         bool traverseStructContainingSamplers(const ImmutableString &baseName,
                                               const TType &structType)
@@ -537,7 +605,7 @@ class Traverser final : public TIntermTraverser
         void visitStructParam(const TFunction *function, size_t paramIndex) override
         {
             const TVariable *param = function->getParam(paramIndex);
-            TType *structType      = getStructSamplerParameterType(mSymbolTable, param);
+            TType *structType      = GetStructSamplerParameterType(mSymbolTable, *param);
             TVariable *newParam =
                 new TVariable(mSymbolTable, param->name(), structType, param->symbolType());
             mNewFunction->addParameter(newParam);
@@ -588,10 +656,11 @@ class Traverser final : public TIntermTraverser
 
         void visitStructParam(const TFunction *function, size_t paramIndex) override
         {
-            // The prior struct type is left intact. This leaves the tree in an inconsistent state.
-            // TODO(jmadill): Fix tree structure. http://anglebug.com/2494
+            // 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();
-            mNewArguments->push_back(argument);
+            TIntermTyped *replacement = ReplaceTypeOfTypedStructNode(argument, mSymbolTable);
+            mNewArguments->push_back(replacement);
         }
 
         void visitNonStructParam(const TFunction *function, size_t paramIndex) override

src/tests/gl_tests/GLSLTest.cpp

@@ -3523,6 +3523,136 @@ void main()
 
     EXPECT_PIXEL_COLOR_EQ(1, 1, GLColor::green);
 }
+
+// This test covers passing nested compound structs containing a sampler as a function argument.
+TEST_P(GLSLTest, NestedCompoundStructsWithSamplersAsFunctionArg)
+{
+    // Shader failed to compile on Android. http://anglebug.com/2114
+    ANGLE_SKIP_TEST_IF(IsAndroid() && IsAdreno() && IsOpenGLES());
+
+    const char kFragmentShader[] = R"(precision mediump float;
+struct S { sampler2D samplerMember; bool b; };
+struct T { S nest; bool b; };
+uniform T uStruct;
+uniform vec2 uTexCoord;
+vec4 foo2(S structVar)
+{
+    if (structVar.b)
+        return texture2D(structVar.samplerMember, uTexCoord);
+    else
+        return vec4(1, 0, 0, 1);
+}
+vec4 foo(T structVar)
+{
+    if (structVar.b)
+        return foo2(structVar.nest);
+    else
+        return vec4(1, 0, 0, 1);
+}
+void main()
+{
+    gl_FragColor = foo(uStruct);
+})";
+
+    ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), kFragmentShader);
+
+    // Initialize the texture with green.
+    GLTexture tex;
+    glActiveTexture(GL_TEXTURE0);
+    glBindTexture(GL_TEXTURE_2D, tex);
+    GLubyte texData[] = {0u, 255u, 0u, 255u};
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, texData);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    ASSERT_GL_NO_ERROR();
+
+    // Draw
+    glUseProgram(program);
+    GLint samplerMemberLoc = glGetUniformLocation(program, "uStruct.nest.samplerMember");
+    ASSERT_NE(-1, samplerMemberLoc);
+    glUniform1i(samplerMemberLoc, 0);
+    GLint texCoordLoc = glGetUniformLocation(program, "uTexCoord");
+    ASSERT_NE(-1, texCoordLoc);
+    glUniform2f(texCoordLoc, 0.5f, 0.5f);
+
+    GLint bLoc = glGetUniformLocation(program, "uStruct.b");
+    ASSERT_NE(-1, bLoc);
+    glUniform1i(bLoc, 1);
+
+    GLint nestbLoc = glGetUniformLocation(program, "uStruct.nest.b");
+    ASSERT_NE(-1, nestbLoc);
+    glUniform1i(nestbLoc, 1);
+
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_COLOR_EQ(1, 1, GLColor::green);
+}
+
+// Same as the prior test but with reordered struct members.
+TEST_P(GLSLTest, MoreNestedCompoundStructsWithSamplersAsFunctionArg)
+{
+    // Shader failed to compile on Android. http://anglebug.com/2114
+    ANGLE_SKIP_TEST_IF(IsAndroid() && IsAdreno() && IsOpenGLES());
+
+    const char kFragmentShader[] = R"(precision mediump float;
+struct S { bool b; sampler2D samplerMember; };
+struct T { bool b; S nest; };
+uniform T uStruct;
+uniform vec2 uTexCoord;
+vec4 foo2(S structVar)
+{
+    if (structVar.b)
+        return texture2D(structVar.samplerMember, uTexCoord);
+    else
+        return vec4(1, 0, 0, 1);
+}
+vec4 foo(T structVar)
+{
+    if (structVar.b)
+        return foo2(structVar.nest);
+    else
+        return vec4(1, 0, 0, 1);
+}
+void main()
+{
+    gl_FragColor = foo(uStruct);
+})";
+
+    ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), kFragmentShader);
+
+    // Initialize the texture with green.
+    GLTexture tex;
+    glActiveTexture(GL_TEXTURE0);
+    glBindTexture(GL_TEXTURE_2D, tex);
+    GLubyte texData[] = { 0u, 255u, 0u, 255u };
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, texData);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    ASSERT_GL_NO_ERROR();
+
+    // Draw
+    glUseProgram(program);
+    GLint samplerMemberLoc = glGetUniformLocation(program, "uStruct.nest.samplerMember");
+    ASSERT_NE(-1, samplerMemberLoc);
+    glUniform1i(samplerMemberLoc, 0);
+    GLint texCoordLoc = glGetUniformLocation(program, "uTexCoord");
+    ASSERT_NE(-1, texCoordLoc);
+    glUniform2f(texCoordLoc, 0.5f, 0.5f);
+
+    GLint bLoc = glGetUniformLocation(program, "uStruct.b");
+    ASSERT_NE(-1, bLoc);
+    glUniform1i(bLoc, 1);
+
+    GLint nestbLoc = glGetUniformLocation(program, "uStruct.nest.b");
+    ASSERT_NE(-1, nestbLoc);
+    glUniform1i(nestbLoc, 1);
+
+    drawQuad(program, essl1_shaders::PositionAttrib(), 0.5f);
+    ASSERT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_COLOR_EQ(1, 1, GLColor::green);
+}
 // Test that a global variable declared after main() works. This is a regression test for an issue
 // in global variable initialization.
 TEST_P(WebGLGLSLTest, GlobalVariableDeclaredAfterMain)