Disallow active attrib aliasing at shader program link time.

BUG=415688
TEST=gpu_unittests, webgl_conformance_tests
R=kbr@chromium.org,bajones@chromium.org

Review URL: https://codereview.chromium.org/651543003

Cr-Commit-Position: refs/heads/master@{#299387}

gpu/command_buffer/service/gles2_cmd_decoder.cc

@@ -4864,7 +4864,13 @@ void GLES2DecoderImpl::DoBindAttribLocation(
   if (!program) {
     return;
   }
+  // At this point, the program's shaders may not be translated yet,
+  // therefore, we may not find the hashed attribute name.
+  // glBindAttribLocation call with original name is useless.
+  // So instead, we should simply cache the binding, and then call
+  // Program::ExecuteBindAttribLocationCalls() right before link.
   program->SetAttribLocationBinding(name, static_cast<GLint>(index));
+  // TODO(zmo): Get rid of the following glBindAttribLocation call.
   glBindAttribLocation(program->service_id(), index, name);
 }
 

gpu/command_buffer/service/program_manager.cc

@@ -493,7 +493,7 @@ void Program::ExecuteBindAttribLocationCalls() {
   for (LocationMap::const_iterator it = bind_attrib_location_map_.begin();
        it != bind_attrib_location_map_.end(); ++it) {
     const std::string* mapped_name = GetAttribMappedName(it->first);
-    if (mapped_name && *mapped_name != it->first)
+    if (mapped_name)
       glBindAttribLocation(service_id_, it->second, mapped_name->c_str());
   }
 }
@@ -655,10 +655,10 @@ GLint Program::GetUniformFakeLocation(
 }
 
 GLint Program::GetAttribLocation(
-    const std::string& name) const {
+    const std::string& original_name) const {
   for (GLuint ii = 0; ii < attrib_infos_.size(); ++ii) {
     const VertexAttrib& info = attrib_infos_[ii];
-    if (info.name == name) {
+    if (info.name == original_name) {
       return info.location;
     }
   }
@@ -989,21 +989,44 @@ bool Program::DetectAttribLocationBindingConflicts() const {
   std::set<GLint> location_binding_used;
   for (LocationMap::const_iterator it = bind_attrib_location_map_.begin();
        it != bind_attrib_location_map_.end(); ++it) {
-    // Find out if an attribute is declared in this program's shaders.
-    bool active = false;
+    // Find out if an attribute is statically used in this program's shaders.
+    const sh::Attribute* attrib = NULL;
+    const std::string* mapped_name = GetAttribMappedName(it->first);
+    if (!mapped_name)
+      continue;
     for (int ii = 0; ii < kMaxAttachedShaders; ++ii) {
       if (!attached_shaders_[ii].get() || !attached_shaders_[ii]->valid())
         continue;
-      if (attached_shaders_[ii]->GetAttribInfo(it->first)) {
-        active = true;
-        break;
+      attrib = attached_shaders_[ii]->GetAttribInfo(*mapped_name);
+      if (attrib) {
+        if (attrib->staticUse)
+          break;
+        else
+          attrib = NULL;
       }
     }
-    if (active) {
-      std::pair<std::set<GLint>::iterator, bool> result =
-          location_binding_used.insert(it->second);
-      if (!result.second)
-        return true;
+    if (attrib) {
+      size_t num_of_locations = 1;
+      switch (attrib->type) {
+        case GL_FLOAT_MAT2:
+          num_of_locations = 2;
+          break;
+        case GL_FLOAT_MAT3:
+          num_of_locations = 3;
+          break;
+        case GL_FLOAT_MAT4:
+          num_of_locations = 4;
+          break;
+        default:
+          break;
+      }
+      for (size_t ii = 0; ii < num_of_locations; ++ii) {
+        GLint loc = it->second + ii;
+        std::pair<std::set<GLint>::iterator, bool> result =
+            location_binding_used.insert(loc);
+        if (!result.second)
+          return true;
+      }
     }
   }
   return false;

gpu/command_buffer/service/program_manager.h

@@ -115,7 +115,7 @@ class GPU_EXPORT Program : public base::RefCounted<Program> {
        &attrib_infos_[index] : NULL;
   }
 
-  GLint GetAttribLocation(const std::string& name) const;
+  GLint GetAttribLocation(const std::string& original_name) const;
 
   const VertexAttrib* GetAttribInfoByLocation(GLuint location) const {
     if (location < attrib_location_to_index_map_.size()) {

gpu/command_buffer/service/program_manager_unittest.cc

@@ -154,7 +154,7 @@ class ProgramManagerWithShaderTest : public GpuServiceTest {
   static const GLenum kAttrib1Precision = GL_MEDIUM_FLOAT;
   static const GLenum kAttrib2Precision = GL_HIGH_FLOAT;
   static const GLenum kAttrib3Precision = GL_LOW_FLOAT;
-  static const bool kAttribStaticUse = false;
+  static const bool kAttribStaticUse = true;
   static const GLint kAttrib1Location = 0;
   static const GLint kAttrib2Location = 1;
   static const GLint kAttrib3Location = 2;
@@ -1134,6 +1134,13 @@ TEST_F(ProgramManagerWithShaderTest, BindAttribLocationConflicts) {
         kAttribStaticUse,
         kAttribs[ii].name);
   }
+  const char kAttribMatName[] = "matAttrib";
+  attrib_map[kAttribMatName] = TestHelper::ConstructAttribute(
+      GL_FLOAT_MAT2,
+      1,
+      SH_PRECISION_MEDIUMP,
+      kAttribStaticUse,
+      kAttribMatName);
   // Check we can create shader.
   Shader* vshader = shader_manager_.CreateShader(
       kVShaderClientId, kVShaderServiceId, GL_VERTEX_SHADER);
@@ -1173,19 +1180,40 @@ TEST_F(ProgramManagerWithShaderTest, BindAttribLocationConflicts) {
 
   program->SetAttribLocationBinding(kAttrib1Name, 0);
   EXPECT_FALSE(program->DetectAttribLocationBindingConflicts());
+  EXPECT_CALL(*(gl_.get()), BindAttribLocation(_, 0, _))
+      .Times(1)
+      .RetiresOnSaturation();
   EXPECT_TRUE(LinkAsExpected(program, true));
 
   program->SetAttribLocationBinding("xxx", 0);
   EXPECT_FALSE(program->DetectAttribLocationBindingConflicts());
+  EXPECT_CALL(*(gl_.get()), BindAttribLocation(_, 0, _))
+      .Times(1)
+      .RetiresOnSaturation();
   EXPECT_TRUE(LinkAsExpected(program, true));
 
   program->SetAttribLocationBinding(kAttrib2Name, 1);
   EXPECT_FALSE(program->DetectAttribLocationBindingConflicts());
+  EXPECT_CALL(*(gl_.get()), BindAttribLocation(_, _, _))
+      .Times(2)
+      .RetiresOnSaturation();
   EXPECT_TRUE(LinkAsExpected(program, true));
 
   program->SetAttribLocationBinding(kAttrib2Name, 0);
   EXPECT_TRUE(program->DetectAttribLocationBindingConflicts());
   EXPECT_TRUE(LinkAsExpected(program, false));
+
+  program->SetAttribLocationBinding(kAttribMatName, 1);
+  program->SetAttribLocationBinding(kAttrib2Name, 3);
+  EXPECT_CALL(*(gl_.get()), BindAttribLocation(_, _, _))
+      .Times(3)
+      .RetiresOnSaturation();
+  EXPECT_FALSE(program->DetectAttribLocationBindingConflicts());
+  EXPECT_TRUE(LinkAsExpected(program, true));
+
+  program->SetAttribLocationBinding(kAttrib2Name, 2);
+  EXPECT_TRUE(program->DetectAttribLocationBindingConflicts());
+  EXPECT_TRUE(LinkAsExpected(program, false));
 }
 
 TEST_F(ProgramManagerWithShaderTest, UniformsPrecisionMismatch) {