Fix uniform array support.

The issues fixed are 2 fold.

1) OpenGL ES 2.0 requires that array names be returned as "name[0]" 
where as OpenGL implementations sometimes return just "name"

2) When the name is an array name as in "array[0]" we need to match
both "array" and "array[0]"

TEST=various unit tests
BUG=40172

Review URL: http://codereview.chromium.org/1607007

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

5 files changed, 89 insertions, 24 deletions

diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
index ffe6cf0..2e78278 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
@@ -521,8 +521,8 @@ const char* GLES2DecoderWithShaderTestBase::kAttrib1Name = "attrib1";
 const char* GLES2DecoderWithShaderTestBase::kAttrib2Name = "attrib2";
 const char* GLES2DecoderWithShaderTestBase::kAttrib3Name = "attrib3";
 const char* GLES2DecoderWithShaderTestBase::kUniform1Name = "uniform1";
-const char* GLES2DecoderWithShaderTestBase::kUniform2Name = "uniform2";
-const char* GLES2DecoderWithShaderTestBase::kUniform3Name = "uniform3";
+const char* GLES2DecoderWithShaderTestBase::kUniform2Name = "uniform2[0]";
+const char* GLES2DecoderWithShaderTestBase::kUniform3Name = "uniform3[0]";
 
 }  // namespace gles2
 }  // namespace gpu
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
index 4290978..4eb317a 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
+++ b/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
@@ -236,7 +236,7 @@ class GLES2DecoderWithShaderTestBase : public GLES2DecoderTestBase {
   static const GLint kInvalidAttribLocation = 30;
   static const GLint kBadAttribIndex = kNumVertexAttribs;
 
-  static const GLint kMaxUniformLength = 10;
+  static const GLint kMaxUniformLength = 12;
   static const char* kUniform1Name;
   static const char* kUniform2Name;
   static const char* kUniform3Name;
diff --git a/gpu/command_buffer/service/program_manager.cc b/gpu/command_buffer/service/program_manager.cc
index b53938f..88dd714 100644
--- a/gpu/command_buffer/service/program_manager.cc
+++ b/gpu/command_buffer/service/program_manager.cc
@@ -77,16 +77,23 @@ void ProgramManager::ProgramInfo::Update() {
   }
 }
 
-GLint ProgramManager::ProgramInfo::GetUniformLocation(const std::string& name) {
+GLint ProgramManager::ProgramInfo::GetUniformLocation(
+    const std::string& name) const {
   for (GLuint ii = 0; ii < uniform_infos_.size(); ++ii) {
     const UniformInfo& info = uniform_infos_[ii];
-    if (info.name == name) {
+    if (info.name == name ||
+        (info.is_array &&
+         info.name.compare(0, info.name.size() - 3, name) == 0)) {
       return info.element_locations[0];
-    } else if (name.size() >= 3 && name[name.size() - 1] == ']') {
-      // Look for an array specfication.
+    } else if (info.is_array &&
+               name.size() >= 3 && name[name.size() - 1] == ']') {
+      // Look for an array specification.
       size_t open_pos = name.find_last_of('[');
-      if (open_pos != std::string::npos && open_pos < name.size() - 2) {
-        GLuint index = 0;
+      if (open_pos != std::string::npos &&
+          open_pos < name.size() - 2 &&
+          info.name.size() > open_pos &&
+          name.compare(0, open_pos, info.name, 0, open_pos) == 0) {
+        GLint index = 0;
         size_t last = name.size() - 1;
         bool bad = false;
         for (size_t pos = open_pos + 1; pos < last; ++pos) {
@@ -97,8 +104,8 @@ GLint ProgramManager::ProgramInfo::GetUniformLocation(const std::string& name) {
           }
           index = index * 10 + digit;
         }
-        if (!bad) {
-          return index;
+        if (!bad && index >= 0 && index < info.size) {
+          return info.element_locations[index];
         }
       }
     }
@@ -134,6 +141,7 @@ void ProgramManager::ProgramInfo::SetUniformInfo(
     GLint index, GLsizei size, GLenum type, GLint location,
     const std::string& name) {
   DCHECK(static_cast<unsigned>(index) < uniform_infos_.size());
+  const char* kArraySpec = "[0]";
   UniformInfo& info = uniform_infos_[index];
   info.size = size;
   info.type = type;
@@ -144,16 +152,32 @@ void ProgramManager::ProgramInfo::SetUniformInfo(
   info.texture_units.clear();
   info.texture_units.resize(num_texture_units, 0);
 
-  // Go through the array element locations looking for a match.
-  // We can skip the first element because it's the same as the
-  // the location without the array operators.
   if (size > 1) {
     for (GLsizei ii = 1; ii < info.size; ++ii) {
       std::string element_name(name + "[" + IntToString(ii) + "]");
       info.element_locations[ii] =
           glGetUniformLocation(program_id_, element_name.c_str());
     }
+    // Sadly there is no way to tell if this is an array except if the name
+    // has an array string or the size > 1. That means an array of size 1 can
+    // be ambiguous.
+    //
+    // For now we just make sure that if the size is > 1 then the name must have
+    // an array spec.
+
+    // Go through the array element locations looking for a match.
+    // We can skip the first element because it's the same as the
+    // the location without the array operators.
+    size_t array_pos = name.rfind(kArraySpec);
+    if (name.size() > 3 && array_pos != name.size() - 3) {
+      info.name = name + kArraySpec;
+    }
   }
+
+  info.is_array =
+     (size > 1 ||
+      (info.name.size() > 3 &&
+       info.name.rfind(kArraySpec) == info.name.size() - 3));
 }
 
 bool ProgramManager::ProgramInfo::SetSamplers(
diff --git a/gpu/command_buffer/service/program_manager.h b/gpu/command_buffer/service/program_manager.h
index 47a1927..c890d21 100644
--- a/gpu/command_buffer/service/program_manager.h
+++ b/gpu/command_buffer/service/program_manager.h
@@ -36,6 +36,7 @@ class ProgramManager {
 
       GLsizei size;
       GLenum type;
+      bool is_array;
       std::string name;
       std::vector<GLint> element_locations;
       std::vector<GLuint> texture_units;
@@ -78,7 +79,7 @@ class ProgramManager {
     }
 
     // Gets the location of a uniform by name.
-    GLint GetUniformLocation(const std::string& name);
+    GLint GetUniformLocation(const std::string& name) const;
 
     // Gets the type of a uniform by location.
     bool GetUniformTypeByLocation(GLint location, GLenum* type) const;
@@ -88,7 +89,7 @@ class ProgramManager {
     // a sampler uniform nothing will happen.
     bool SetSamplers(GLint location, GLsizei count, const GLint* value);
 
-    bool IsDeleted() {
+    bool IsDeleted() const {
       return program_id_ == 0;
     }
 
diff --git a/gpu/command_buffer/service/program_manager_unittest.cc b/gpu/command_buffer/service/program_manager_unittest.cc
index 2ec65f8..32d5a5a 100644
--- a/gpu/command_buffer/service/program_manager_unittest.cc
+++ b/gpu/command_buffer/service/program_manager_unittest.cc
@@ -80,7 +80,7 @@ class ProgramManagerWithShaderTest : public testing::Test {
   static const GLint kInvalidAttribLocation = 30;
   static const GLint kBadAttribIndex = kNumVertexAttribs;
 
-  static const GLint kMaxUniformLength = 10;
+  static const GLint kMaxUniformLength = 12;
   static const char* kUniform1Name;
   static const char* kUniform2Name;
   static const char* kUniform3Name;
@@ -89,7 +89,6 @@ class ProgramManagerWithShaderTest : public testing::Test {
   static const GLint kUniform3Size = 2;
   static const GLint kUniform1Location = 3;
   static const GLint kUniform2Location = 10;
-  static const GLint kUniform2ElementLocation = 12;
   static const GLint kUniform3Location = 20;
   static const GLenum kUniform1Type = GL_FLOAT_VEC4;
   static const GLenum kUniform2Type = GL_INT_VEC2;
@@ -188,7 +187,7 @@ class ProgramManagerWithShaderTest : public testing::Test {
                 std::string(info.name) + "[" + IntToString(jj) + "]");
             EXPECT_CALL(*gl_, GetUniformLocation(service_id,
                                                  StrEq(element_name)))
-                .WillOnce(Return(info.location + jj))
+                .WillOnce(Return(info.location + jj * 2))
                 .RetiresOnSaturation();
           }
         }
@@ -244,7 +243,6 @@ const GLint ProgramManagerWithShaderTest::kUniform2Size;
 const GLint ProgramManagerWithShaderTest::kUniform3Size;
 const GLint ProgramManagerWithShaderTest::kUniform1Location;
 const GLint ProgramManagerWithShaderTest::kUniform2Location;
-const GLint ProgramManagerWithShaderTest::kUniform2ElementLocation;
 const GLint ProgramManagerWithShaderTest::kUniform3Location;
 const GLenum ProgramManagerWithShaderTest::kUniform1Type;
 const GLenum ProgramManagerWithShaderTest::kUniform2Type;
@@ -270,7 +268,9 @@ const char* ProgramManagerWithShaderTest::kAttrib1Name = "attrib1";
 const char* ProgramManagerWithShaderTest::kAttrib2Name = "attrib2";
 const char* ProgramManagerWithShaderTest::kAttrib3Name = "attrib3";
 const char* ProgramManagerWithShaderTest::kUniform1Name = "uniform1";
-const char* ProgramManagerWithShaderTest::kUniform2Name = "uniform2";
+// Correctly has array spec.
+const char* ProgramManagerWithShaderTest::kUniform2Name = "uniform2[0]";
+// Incorrectly missing array spec.
 const char* ProgramManagerWithShaderTest::kUniform3Name = "uniform3";
 
 TEST_F(ProgramManagerWithShaderTest, GetAttribInfos) {
@@ -314,22 +314,62 @@ TEST_F(ProgramManagerWithShaderTest, GetAttribLocation) {
   EXPECT_EQ(-1, program_info->GetAttribLocation(kInvalidName));
 }
 
-TEST_F(ProgramManagerWithShaderTest, GetUniformLocation) {
-  const GLint kValidIndex = 1;
+TEST_F(ProgramManagerWithShaderTest, GetUniformInfo) {
   const GLint kInvalidIndex = 1000;
   const ProgramManager::ProgramInfo* program_info =
       manager_.GetProgramInfo(kProgramId);
   ASSERT_TRUE(program_info != NULL);
   const ProgramManager::ProgramInfo::UniformInfo* info =
-      program_info->GetUniformInfo(kValidIndex);
+      program_info->GetUniformInfo(0);
+  ASSERT_TRUE(info != NULL);
+  EXPECT_EQ(kUniform1Size, info->size);
+  EXPECT_EQ(kUniform1Type, info->type);
+  EXPECT_EQ(kUniform1Location, info->element_locations[0]);
+  EXPECT_STREQ(kUniform1Name, info->name.c_str());
+  info = program_info->GetUniformInfo(1);
   ASSERT_TRUE(info != NULL);
   EXPECT_EQ(kUniform2Size, info->size);
   EXPECT_EQ(kUniform2Type, info->type);
   EXPECT_EQ(kUniform2Location, info->element_locations[0]);
   EXPECT_STREQ(kUniform2Name, info->name.c_str());
+  info = program_info->GetUniformInfo(2);
+  // We emulate certain OpenGL drivers by supplying the name without
+  // the array spec. Our implementation should correctly add the required spec.
+  const std::string expected_name(std::string(kUniform3Name) + "[0]");
+  ASSERT_TRUE(info != NULL);
+  EXPECT_EQ(kUniform3Size, info->size);
+  EXPECT_EQ(kUniform3Type, info->type);
+  EXPECT_EQ(kUniform3Location, info->element_locations[0]);
+  EXPECT_STREQ(expected_name.c_str(), info->name.c_str());
   EXPECT_TRUE(program_info->GetUniformInfo(kInvalidIndex) == NULL);
 }
 
+TEST_F(ProgramManagerWithShaderTest, GetUniformLocation) {
+  const ProgramManager::ProgramInfo* program_info =
+      manager_.GetProgramInfo(kProgramId);
+  ASSERT_TRUE(program_info != NULL);
+  EXPECT_EQ(kUniform1Location, program_info->GetUniformLocation(kUniform1Name));
+  EXPECT_EQ(kUniform2Location, program_info->GetUniformLocation(kUniform2Name));
+  EXPECT_EQ(kUniform3Location, program_info->GetUniformLocation(kUniform3Name));
+  // Check we can get uniform2 as "uniform2" even though the name is
+  // "uniform2[0]"
+  EXPECT_EQ(kUniform2Location, program_info->GetUniformLocation("uniform2"));
+  // Check we can get uniform3 as "uniform3[0]" even though we simulated GL
+  // returning "uniform3"
+  EXPECT_EQ(kUniform3Location, program_info->GetUniformLocation("uniform3[0]"));
+  // Check that we can get the locations of the array elements > 1
+  EXPECT_EQ(kUniform2Location + 2,
+            program_info->GetUniformLocation("uniform2[1]"));
+  EXPECT_EQ(kUniform2Location + 4,
+            program_info->GetUniformLocation("uniform2[2]"));
+  EXPECT_EQ(-1,
+            program_info->GetUniformLocation("uniform2[3]"));
+  EXPECT_EQ(kUniform3Location + 2,
+            program_info->GetUniformLocation("uniform3[1]"));
+  EXPECT_EQ(-1,
+            program_info->GetUniformLocation("uniform3[2]"));
+}
+
 TEST_F(ProgramManagerWithShaderTest, GetUniformTypeByLocation) {
   const GLint kInvalidLocation = 1234;
   GLenum type = 0u;