Fix support for GL_MAX_ATTRIBS attributes.

*re-land with fix for AMD/ES2/OpenGL.*

An off-by-one bug slipped in that broke support for these edge case
shaders.

Bug introduced in https://chromium-review.googlesource.com/#/c/266928/

BUG=angleproject:1045
BUG=500116

Change-Id: If44f809d432221d1e17afc407d49e87e0cb7504c
Reviewed-on: https://chromium-review.googlesource.com/277664
Tested-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>

src/libANGLE/Program.cpp

@@ -1291,7 +1291,7 @@ bool Program::linkAttributes(const Data &data,
     GLuint maxAttribs = data.caps->maxVertexAttributes;
 
     // TODO(jmadill): handle aliasing robustly
-    if (shaderAttributes.size() >= maxAttribs)
+    if (shaderAttributes.size() > maxAttribs)
     {
         infoLog << "Too many vertex attributes.";
         return false;

src/tests/gl_tests/VertexAttributeTest.cpp

@@ -129,6 +129,51 @@ class VertexAttributeTest : public ANGLETest
         ANGLETest::TearDown();
     }
 
+    GLuint compileMultiAttribProgram(GLint attribCount)
+    {
+        std::stringstream shaderStream;
+
+        shaderStream << "attribute highp vec4 position;" << std::endl;
+        for (GLint attribIndex = 0; attribIndex < attribCount; ++attribIndex)
+        {
+            shaderStream << "attribute float a" << attribIndex << ";" << std::endl;
+        }
+        shaderStream << "varying highp float color;" << std::endl
+                     << "void main() {" << std::endl
+                     << "  gl_Position = position;" << std::endl
+                     << "  color = 0.0;" << std::endl;
+        for (GLint attribIndex = 0; attribIndex < attribCount; ++attribIndex)
+        {
+            shaderStream << "  color += a" << attribIndex << ";" << std::endl;
+        }
+        shaderStream << "}" << std::endl;
+
+        const std::string testFragmentShaderSource = SHADER_SOURCE
+        (
+            varying highp float color;
+            void main(void)
+            {
+                gl_FragColor = vec4(color, 0.0, 0.0, 1.0);
+            }
+        );
+
+        return CompileProgram(shaderStream.str(), testFragmentShaderSource);
+    }
+
+    void setupMultiAttribs(GLuint program, GLint attribCount, GLfloat value)
+    {
+        glUseProgram(program);
+        for (GLint attribIndex = 0; attribIndex < attribCount; ++attribIndex)
+        {
+            std::stringstream attribStream;
+            attribStream << "a" << attribIndex;
+            GLint location = glGetAttribLocation(program, attribStream.str().c_str());
+            ASSERT_NE(-1, location);
+            glVertexAttrib1f(location, value);
+            glDisableVertexAttribArray(location);
+        }
+    }
+
     static const size_t mVertexCount = 24;
 
     GLuint mProgram;
@@ -240,6 +285,54 @@ TEST_P(VertexAttributeTest, ShortNormalized)
     runTest(data);
 }
 
+// Validate that we can support GL_MAX_ATTRIBS attribs
+TEST_P(VertexAttributeTest, MaxAttribs)
+{
+    // TODO(jmadill): Figure out why we get this error on AMD/ES2/OpenGL
+    if (isAMD() && GetParam() == ES2_OPENGL())
+    {
+        std::cout << "Test disabled on AMD/ES2/OpenGL" << std::endl;
+        return;
+    }
+
+    GLint maxAttribs;
+    glGetIntegerv(GL_MAX_VERTEX_ATTRIBS, &maxAttribs);
+    ASSERT_GL_NO_ERROR();
+
+    // Reserve one attrib for position
+    GLint drawAttribs = maxAttribs - 1;
+
+    GLuint program = compileMultiAttribProgram(drawAttribs);
+    ASSERT_NE(0u, program);
+
+    setupMultiAttribs(program, drawAttribs, 0.5f / static_cast<float>(drawAttribs));
+    drawQuad(program, "position", 0.5f);
+
+    EXPECT_GL_NO_ERROR();
+    EXPECT_PIXEL_NEAR(0, 0, 128, 0, 0, 255, 1);
+}
+
+// Validate that we cannot support GL_MAX_ATTRIBS+1 attribs
+TEST_P(VertexAttributeTest, MaxAttribsPlusOne)
+{
+    // TODO(jmadill): Figure out why we get this error on AMD/ES2/OpenGL
+    if (isAMD() && GetParam() == ES2_OPENGL())
+    {
+        std::cout << "Test disabled on AMD/ES2/OpenGL" << std::endl;
+        return;
+    }
+
+    GLint maxAttribs;
+    glGetIntegerv(GL_MAX_VERTEX_ATTRIBS, &maxAttribs);
+    ASSERT_GL_NO_ERROR();
+
+    // Exceed attrib count by one (counting position)
+    GLint drawAttribs = maxAttribs;
+
+    GLuint program = compileMultiAttribProgram(drawAttribs);
+    ASSERT_EQ(0u, program);
+}
+
 // 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 10_0+, so we should test them separately.
 ANGLE_INSTANTIATE_TEST(VertexAttributeTest, ES2_D3D9(), ES2_D3D11(), ES2_D3D11_FL9_3(), ES2_OPENGL(), ES3_OPENGL());

src/tests/test_utils/angle_test_configs.cpp

@@ -42,6 +42,13 @@ bool operator<(const PlatformParameters &a, const PlatformParameters &b)
     return a.eglParameters < b.eglParameters;
 }
 
+bool operator==(const PlatformParameters &a, const PlatformParameters &b)
+{
+    return (a.majorVersion == b.majorVersion) &&
+           (a.minorVersion == b.minorVersion) &&
+           (a.eglParameters == b.eglParameters);
+}
+
 std::ostream &operator<<(std::ostream& stream, const PlatformParameters &pp)
 {
     stream << "ES" << pp.majorVersion << "_" ;

src/tests/test_utils/angle_test_configs.h

@@ -30,6 +30,7 @@ struct PlatformParameters
 };
 
 bool operator<(const PlatformParameters &a, const PlatformParameters &b);
+bool operator==(const PlatformParameters &a, const PlatformParameters &b);
 std::ostream &operator<<(std::ostream& stream, const PlatformParameters &pp);
 
 // EGL platforms

util/EGLWindow.cpp

@@ -67,6 +67,14 @@ bool operator<(const EGLPlatformParameters &a, const EGLPlatformParameters &b)
     return a.deviceType < b.deviceType;
 }
 
+bool operator==(const EGLPlatformParameters &a, const EGLPlatformParameters &b)
+{
+    return (a.renderer == b.renderer) &&
+           (a.majorVersion == b.majorVersion) &&
+           (a.minorVersion == b.minorVersion) &&
+           (a.deviceType == b.deviceType);
+}
+
 EGLWindow::EGLWindow(size_t width, size_t height, EGLint glesMajorVersion, const EGLPlatformParameters &platform)
     : mDisplay(EGL_NO_DISPLAY),
       mSurface(EGL_NO_SURFACE),

util/EGLWindow.h

@@ -44,6 +44,7 @@ struct EGLPlatformParameters
 };
 
 bool operator<(const EGLPlatformParameters &a, const EGLPlatformParameters &b);
+bool operator==(const EGLPlatformParameters &a, const EGLPlatformParameters &b);
 
 class EGLWindow : angle::NonCopyable
 {