Take all attributes into account when checking for aliasing

src/libANGLE/Program.cpp

@@ -2662,8 +2662,20 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
     const ContextState &data = context->getContextState();
     Shader *vertexShader     = mState.getAttachedShader(ShaderType::Vertex);
 
+    int shaderVersion = vertexShader->getShaderVersion(context);
+
     unsigned int usedLocations = 0;
-    mState.mAttributes         = vertexShader->getActiveAttributes(context);
+    if (shaderVersion >= 300)
+    {
+        // In GLSL ES 3.00.6, aliasing checks should be done with all declared attributes - see GLSL
+        // ES 3.00.6 section 12.46. Inactive attributes will be pruned after aliasing checks.
+        mState.mAttributes = vertexShader->getAllAttributes(context);
+    }
+    else
+    {
+        // In GLSL ES 1.00.17 we only do aliasing checks for active attributes.
+        mState.mAttributes = vertexShader->getActiveAttributes(context);
+    }
     GLuint maxAttribs          = data.getCaps().maxVertexAttributes;
 
     // TODO(jmadill): handle aliasing robustly
@@ -2675,7 +2687,7 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
 
     std::vector<sh::Attribute *> usedAttribMap(maxAttribs, nullptr);
 
-    // Link attributes that have a binding location
+    // Assign locations to attributes that have a binding location and check for attribute aliasing.
     for (sh::Attribute &attribute : mState.mAttributes)
     {
         // GLSL ES 3.10 January 2016 section 4.3.4: Vertex shader inputs can't be arrays or
@@ -2696,8 +2708,8 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
 
             if (static_cast<GLuint>(regs + attribute.location) > maxAttribs)
             {
-                infoLog << "Active attribute (" << attribute.name << ") at location "
-                        << attribute.location << " is too big to fit";
+                infoLog << "Attribute (" << attribute.name << ") at location " << attribute.location
+                        << " is too big to fit";
 
                 return false;
             }
@@ -2707,12 +2719,13 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
                 const int regLocation               = attribute.location + reg;
                 sh::ShaderVariable *linkedAttribute = usedAttribMap[regLocation];
 
-                // In GLSL 3.00, attribute aliasing produces a link error
-                // In GLSL 1.00, attribute aliasing is allowed, but ANGLE currently has a bug
+                // In GLSL ES 3.00.6 and in WebGL, attribute aliasing produces a link error.
+                // In non-WebGL GLSL ES 1.00.17, attribute aliasing is allowed with some
+                // restrictions - see GLSL ES 1.00.17 section 2.10.4, but ANGLE currently has a bug.
                 if (linkedAttribute)
                 {
                     // TODO(jmadill): fix aliasing on ES2
-                    // if (mProgram->getShaderVersion() >= 300)
+                    // if (shaderVersion >= 300 && !webgl)
                     {
                         infoLog << "Attribute '" << attribute.name << "' aliases attribute '"
                                 << linkedAttribute->name << "' at location " << regLocation;
@@ -2729,7 +2742,7 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
         }
     }
 
-    // Link attributes that don't have a binding location
+    // Assign locations to attributes that don't have a binding location.
     for (sh::Attribute &attribute : mState.mAttributes)
     {
         // Not set by glBindAttribLocation or by location layout qualifier
@@ -2740,7 +2753,7 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
 
             if (availableIndex == -1 || static_cast<GLuint>(availableIndex + regs) > maxAttribs)
             {
-                infoLog << "Too many active attributes (" << attribute.name << ")";
+                infoLog << "Too many attributes (" << attribute.name << ")";
                 return false;
             }
 
@@ -2751,8 +2764,27 @@ bool Program::linkAttributes(const Context *context, InfoLog &infoLog)
     ASSERT(mState.mAttributesTypeMask.none());
     ASSERT(mState.mAttributesMask.none());
 
+    // Prune inactive attributes. This step is only needed on shaderVersion >= 300 since on earlier
+    // shader versions we're only processing active attributes to begin with.
+    if (shaderVersion >= 300)
+    {
+        for (auto attributeIter = mState.mAttributes.begin();
+             attributeIter != mState.mAttributes.end();)
+        {
+            if (attributeIter->active)
+            {
+                ++attributeIter;
+            }
+            else
+            {
+                attributeIter = mState.mAttributes.erase(attributeIter);
+            }
+        }
+    }
+
     for (const sh::Attribute &attribute : mState.mAttributes)
     {
+        ASSERT(attribute.active);
         ASSERT(attribute.location != -1);
         unsigned int regs = static_cast<unsigned int>(VariableRegisterCount(attribute.type));
 

src/libANGLE/Shader.cpp

@@ -403,8 +403,8 @@ void Shader::resolveCompile(const Context *context)
         {
             {
                 mState.mOutputVaryings = GetShaderVariables(sh::GetOutputVaryings(compilerHandle));
-                mState.mActiveAttributes =
-                    GetActiveShaderVariables(sh::GetAttributes(compilerHandle));
+                mState.mAllAttributes    = GetShaderVariables(sh::GetAttributes(compilerHandle));
+                mState.mActiveAttributes = GetActiveShaderVariables(&mState.mAllAttributes);
                 mState.mNumViews = sh::GetVertexShaderNumViews(compilerHandle);
             }
             break;
@@ -529,6 +529,12 @@ const std::vector<sh::Attribute> &Shader::getActiveAttributes(const Context *con
     return mState.getActiveAttributes();
 }
 
+const std::vector<sh::Attribute> &Shader::getAllAttributes(const Context *context)
+{
+    resolveCompile(context);
+    return mState.getAllAttributes();
+}
+
 const std::vector<sh::OutputVariable> &Shader::getActiveOutputVariables(const Context *context)
 {
     resolveCompile(context);

src/libANGLE/Shader.h

@@ -71,6 +71,7 @@ class ShaderState final : angle::NonCopyable
         return mShaderStorageBlocks;
     }
     const std::vector<sh::Attribute> &getActiveAttributes() const { return mActiveAttributes; }
+    const std::vector<sh::Attribute> &getAllAttributes() const { return mAllAttributes; }
     const std::vector<sh::OutputVariable> &getActiveOutputVariables() const
     {
         return mActiveOutputVariables;
@@ -95,6 +96,7 @@ class ShaderState final : angle::NonCopyable
     std::vector<sh::Uniform> mUniforms;
     std::vector<sh::InterfaceBlock> mUniformBlocks;
     std::vector<sh::InterfaceBlock> mShaderStorageBlocks;
+    std::vector<sh::Attribute> mAllAttributes;
     std::vector<sh::Attribute> mActiveAttributes;
     std::vector<sh::OutputVariable> mActiveOutputVariables;
 
@@ -165,6 +167,7 @@ class Shader final : angle::NonCopyable, public LabeledObject
     const std::vector<sh::InterfaceBlock> &getUniformBlocks(const Context *context);
     const std::vector<sh::InterfaceBlock> &getShaderStorageBlocks(const Context *context);
     const std::vector<sh::Attribute> &getActiveAttributes(const Context *context);
+    const std::vector<sh::Attribute> &getAllAttributes(const Context *context);
     const std::vector<sh::OutputVariable> &getActiveOutputVariables(const Context *context);
 
     // Returns mapped name of a transform feedback varying. The original name may contain array

src/tests/gl_tests/VertexAttributeTest.cpp

@@ -1471,6 +1471,39 @@ void main()
     }
 }
 
+// Test that even inactive attributes are taken into account when checking for aliasing in case the
+// shader version is >= 3.00. GLSL ES 3.00.6 section 12.46.
+TEST_P(VertexAttributeTestES3, InactiveAttributeAliasing)
+{
+    const std::string &vertexShader =
+        R"(#version 300 es
+        precision mediump float;
+        in vec4 input_active;
+        in vec4 input_unused;
+        void main()
+        {
+            gl_Position = input_active;
+        })";
+
+    const std::string &fragmentShader =
+        R"(#version 300 es
+        precision mediump float;
+        out vec4 color;
+        void main()
+        {
+            color = vec4(0.0);
+        })";
+
+    ANGLE_GL_PROGRAM(program, vertexShader, fragmentShader);
+    glBindAttribLocation(program, 0, "input_active");
+    glBindAttribLocation(program, 0, "input_unused");
+    glLinkProgram(program);
+    EXPECT_GL_NO_ERROR();
+    GLint linkStatus = 0;
+    glGetProgramiv(program, GL_LINK_STATUS, &linkStatus);
+    EXPECT_GL_FALSE(linkStatus);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these
 // tests should be run against.
 // D3D11 Feature Level 9_3 uses different D3D formats for vertex attribs compared to Feature Levels