Fails glLinkProgram if two glBindAttribLocation conflicts.

i.e., two declared attributes are bound to the same location, then we fail the link.

Many drivers already do this, but not all of them.  For example, mesa does not, and it caused memory corruption actually.  So we do this in chrome command buffer.

BUG=112569
TEST=test case in 112569
R=gman
Review URL: https://chromiumcodereview.appspot.com/9570023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@124707 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 126 insertions, 1 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index 8539dcd..0ecc3d8 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -3802,6 +3802,7 @@ void GLES2DecoderImpl::DoBindAttribLocation(
   if (!info) {
     return;
   }
+  info->SetAttribLocationBinding(name, static_cast<GLint>(index));
   glBindAttribLocation(info->service_id(), index, name);
 }
 
diff --git a/gpu/command_buffer/service/program_manager.cc b/gpu/command_buffer/service/program_manager.cc
index e601aed..18593356 100644
--- a/gpu/command_buffer/service/program_manager.cc
+++ b/gpu/command_buffer/service/program_manager.cc
@@ -5,6 +5,7 @@
 #include "gpu/command_buffer/service/program_manager.h"
 
 #include <algorithm>
+#include <set>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ -168,6 +169,10 @@ void ProgramManager::ProgramInfo::Link() {
     set_log_info("missing shaders");
     return;
   }
+  if (DetectAttribLocationBindingConflicts()) {
+    set_log_info("glBindAttribLocation() conflicts");
+    return;
+  }
 
   glLinkProgram(service_id());
   GLint success = 0;
@@ -442,6 +447,31 @@ bool ProgramManager::ProgramInfo::CanLink() const {
   return true;
 }
 
+bool ProgramManager::ProgramInfo::DetectAttribLocationBindingConflicts() const {
+  std::set<GLint> location_binding_used;
+  for (std::map<std::string, GLint>::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;
+    for (int ii = 0; ii < kMaxAttachedShaders; ++ii) {
+      if (!attached_shaders_[ii] || !attached_shaders_[ii]->IsValid())
+        continue;
+      if (attached_shaders_[ii]->GetAttribInfo(it->first)) {
+        active = true;
+        break;
+      }
+    }
+    if (active) {
+      std::pair<std::set<GLint>::iterator, bool> result =
+          location_binding_used.insert(it->second);
+      if (!result.second)
+        return true;
+    }
+  }
+  return false;
+}
+
 static uint32 ComputeOffset(const void* start, const void* position) {
   return static_cast<const uint8*>(position) -
          static_cast<const uint8*>(start);
diff --git a/gpu/command_buffer/service/program_manager.h b/gpu/command_buffer/service/program_manager.h
index 330ef53..c627957 100644
--- a/gpu/command_buffer/service/program_manager.h
+++ b/gpu/command_buffer/service/program_manager.h
@@ -154,6 +154,17 @@ class GPU_EXPORT ProgramManager {
       return use_count_ != 0;
     }
 
+    // Sets attribute-location binding from a glBindAttribLocation() call.
+    void SetAttribLocationBinding(const std::string& attrib,
+                                  GLint location) {
+      bind_attrib_location_map_[attrib] = location;
+    }
+
+    // Detects if there are attribute location conflicts from
+    // glBindAttribLocation() calls.
+    // We only consider the declared attributes in the program.
+    bool DetectAttribLocationBindingConflicts() const;
+
     static inline GLint GetFakeLocation(
         GLint fake_base_location, GLint element_index) {
       return fake_base_location | element_index << 16;
@@ -248,6 +259,9 @@ class GPU_EXPORT ProgramManager {
 
     // Log info
     scoped_ptr<std::string> log_info_;
+
+    // attribute-location binding map from glBindAttribLocation() calls.
+    std::map<std::string, GLint> bind_attrib_location_map_;
   };
 
   ProgramManager();
diff --git a/gpu/command_buffer/service/program_manager_unittest.cc b/gpu/command_buffer/service/program_manager_unittest.cc
index 9ac98fd..ba7288ae 100644
--- a/gpu/command_buffer/service/program_manager_unittest.cc
+++ b/gpu/command_buffer/service/program_manager_unittest.cc
@@ -334,7 +334,6 @@ class ProgramManagerWithShaderTest : public testing::Test {
     }
   }
 
-
   void SetupDefaultShaderExpectations() {
     SetupShader(kAttribs, kNumAttribs, kUniforms, kNumUniforms,
                 kServiceProgramId);
@@ -344,6 +343,20 @@ class ProgramManagerWithShaderTest : public testing::Test {
     ::gfx::GLInterface::SetGLInterface(NULL);
   }
 
+  // Return true if link status matches expected_link_status
+  bool LinkAsExpected(ProgramManager::ProgramInfo* program_info,
+                      bool expected_link_status) {
+    GLuint service_id = program_info->service_id();
+    if (expected_link_status) {
+      SetupShader(kAttribs, kNumAttribs, kUniforms, kNumUniforms,
+                  service_id);
+    }
+    program_info->Link();
+    GLint link_status;
+    program_info->GetProgramiv(GL_LINK_STATUS, &link_status);
+    return (static_cast<bool>(link_status) == expected_link_status);
+  }
+
   static AttribInfo kAttribs[];
   static UniformInfo kUniforms[];
 
@@ -938,6 +951,73 @@ TEST_F(ProgramManagerWithShaderTest, ProgramInfoGetProgramInfo) {
             static_cast<uint32>(input - inputs));
 }
 
+TEST_F(ProgramManagerWithShaderTest, BindAttribLocationConflicts) {
+  // Set up shader
+  const GLuint kVShaderClientId = 1;
+  const GLuint kVShaderServiceId = 11;
+  const GLuint kFShaderClientId = 2;
+  const GLuint kFShaderServiceId = 12;
+  MockShaderTranslator shader_translator;
+  ShaderTranslator::VariableMap attrib_map;
+  for (uint32 ii = 0; ii < kNumAttribs; ++ii) {
+    attrib_map[kAttribs[ii].name] = ShaderTranslatorInterface::VariableInfo(
+        kAttribs[ii].type, kAttribs[ii].size, kAttribs[ii].name);
+  }
+  ShaderTranslator::VariableMap uniform_map;
+  EXPECT_CALL(shader_translator, attrib_map())
+      .WillRepeatedly(ReturnRef(attrib_map));
+  EXPECT_CALL(shader_translator, uniform_map())
+      .WillRepeatedly(ReturnRef(uniform_map));
+  // Check we can create shader.
+  ShaderManager::ShaderInfo* vshader = shader_manager_.CreateShaderInfo(
+      kVShaderClientId, kVShaderServiceId, GL_VERTEX_SHADER);
+  ShaderManager::ShaderInfo* fshader = shader_manager_.CreateShaderInfo(
+      kFShaderClientId, kFShaderServiceId, GL_FRAGMENT_SHADER);
+  // Check shader got created.
+  ASSERT_TRUE(vshader != NULL && fshader != NULL);
+  // Set Status
+  vshader->SetStatus(true, "", &shader_translator);
+  // Check attrib infos got copied.
+  for (ShaderTranslator::VariableMap::const_iterator it = attrib_map.begin();
+       it != attrib_map.end(); ++it) {
+    const ShaderManager::ShaderInfo::VariableInfo* variable_info =
+        vshader->GetAttribInfo(it->first);
+    ASSERT_TRUE(variable_info != NULL);
+    EXPECT_EQ(it->second.type, variable_info->type);
+    EXPECT_EQ(it->second.size, variable_info->size);
+    EXPECT_EQ(it->second.name, variable_info->name);
+  }
+  fshader->SetStatus(true, "", NULL);
+
+  // Set up program
+  const GLuint kClientProgramId = 6666;
+  const GLuint kServiceProgramId = 8888;
+  ProgramManager::ProgramInfo* program_info =
+      manager_.CreateProgramInfo(kClientProgramId, kServiceProgramId);
+  ASSERT_TRUE(program_info != NULL);
+  EXPECT_TRUE(program_info->AttachShader(&shader_manager_, vshader));
+  EXPECT_TRUE(program_info->AttachShader(&shader_manager_, fshader));
+
+  EXPECT_FALSE(program_info->DetectAttribLocationBindingConflicts());
+  EXPECT_TRUE(LinkAsExpected(program_info, true));
+
+  program_info->SetAttribLocationBinding(kAttrib1Name, 0);
+  EXPECT_FALSE(program_info->DetectAttribLocationBindingConflicts());
+  EXPECT_TRUE(LinkAsExpected(program_info, true));
+
+  program_info->SetAttribLocationBinding("xxx", 0);
+  EXPECT_FALSE(program_info->DetectAttribLocationBindingConflicts());
+  EXPECT_TRUE(LinkAsExpected(program_info, true));
+
+  program_info->SetAttribLocationBinding(kAttrib2Name, 1);
+  EXPECT_FALSE(program_info->DetectAttribLocationBindingConflicts());
+  EXPECT_TRUE(LinkAsExpected(program_info, true));
+
+  program_info->SetAttribLocationBinding(kAttrib2Name, 0);
+  EXPECT_TRUE(program_info->DetectAttribLocationBindingConflicts());
+  EXPECT_TRUE(LinkAsExpected(program_info, false));
+}
+
 }  // namespace gles2
 }  // namespace gpu