Make command buffer handles the situation that unused uniform array elements can be optimized out.

BUG=310883
TEST=conformance/uniform/gl-uniform-arrays.html on mac
R=bajones@chromium.org, kbr@chromium.org

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

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

2 files changed, 66 insertions, 10 deletions

diff --git a/gpu/command_buffer/service/program_manager.cc b/gpu/command_buffer/service/program_manager.cc
index 0fb6ab32..0437209 100644
--- a/gpu/command_buffer/service/program_manager.cc
+++ b/gpu/command_buffer/service/program_manager.cc
@@ -391,7 +391,7 @@ void Program::Update() {
   name_buffer.reset(new char[max_len]);
 
   // Reads all the names.
-  std::vector<UniformData> uniform_data_;
+  std::vector<UniformData> uniform_data;
   for (GLint ii = 0; ii < num_uniforms; ++ii) {
     GLsizei length = 0;
     UniformData data;
@@ -405,7 +405,7 @@ void Program::Update() {
       GetCorrectedVariableInfo(
           true, name_buffer.get(), &data.corrected_name, &data.original_name,
           &data.size, &data.type);
-      uniform_data_.push_back(data);
+      uniform_data.push_back(data);
     }
   }
 
@@ -419,8 +419,8 @@ void Program::Update() {
 
   // Assigns the uniforms with bindings.
   size_t next_available_index = 0;
-  for (size_t ii = 0; ii < uniform_data_.size(); ++ii) {
-    UniformData& data = uniform_data_[ii];
+  for (size_t ii = 0; ii < uniform_data.size(); ++ii) {
+    UniformData& data = uniform_data[ii];
     data.location = glGetUniformLocation(
         service_id_, data.queried_name.c_str());
     // remove "[0]"
@@ -439,8 +439,8 @@ void Program::Update() {
   }
 
   // Assigns the uniforms that were not bound.
-  for (size_t ii = 0; ii < uniform_data_.size(); ++ii) {
-    const UniformData& data = uniform_data_[ii];
+  for (size_t ii = 0; ii < uniform_data.size(); ++ii) {
+    const UniformData& data = uniform_data[ii];
     if (!data.added) {
       AddUniformInfo(
           data.size, data.type, data.location, -1, data.corrected_name,
@@ -672,6 +672,9 @@ GLint Program::GetUniformFakeLocation(
       if (open_pos_2 == open_pos &&
           name.compare(0, open_pos, info.name, 0, open_pos) == 0) {
         if (index >= 0 && index < info.size) {
+          DCHECK_GT(static_cast<int>(info.element_locations.size()), index);
+          if (info.element_locations[index] == -1)
+            return -1;
           return ProgramManager::MakeFakeLocation(
               info.fake_location_base, index);
         }
@@ -1221,7 +1224,10 @@ void Program::GetProgramInfo(
       inputs->name_length = info.name.size();
       DCHECK(static_cast<size_t>(info.size) == info.element_locations.size());
       for (size_t jj = 0; jj < info.element_locations.size(); ++jj) {
-        *locations++ = ProgramManager::MakeFakeLocation(ii, jj);
+        if (info.element_locations[jj] == -1)
+          *locations++ = -1;
+        else
+          *locations++ = ProgramManager::MakeFakeLocation(ii, jj);
       }
       memcpy(strings, info.name.c_str(), info.name.size());
       strings += info.name.size();
diff --git a/gpu/command_buffer/service/program_manager_unittest.cc b/gpu/command_buffer/service/program_manager_unittest.cc
index 43217f4..2c51c88 100644
--- a/gpu/command_buffer/service/program_manager_unittest.cc
+++ b/gpu/command_buffer/service/program_manager_unittest.cc
@@ -608,12 +608,19 @@ TEST_F(ProgramManagerWithShaderTest, AttachDetachShader) {
 TEST_F(ProgramManagerWithShaderTest, GetUniformFakeLocation) {
   const Program* program = manager_.GetProgram(kClientProgramId);
   ASSERT_TRUE(program != NULL);
+  // Emulate the situation that uniform3[1] isn't used and optimized out by
+  // a driver, so it's location is -1.
+  Program::UniformInfo* uniform = const_cast<Program::UniformInfo*>(
+      program->GetUniformInfo(2));
+  ASSERT_TRUE(uniform != NULL && kUniform3Size == 2);
+  EXPECT_EQ(kUniform3Size, uniform->size);
+  uniform->element_locations[1] = -1;
   EXPECT_EQ(kUniform1FakeLocation,
             program->GetUniformFakeLocation(kUniform1Name));
   EXPECT_EQ(kUniform2FakeLocation,
             program->GetUniformFakeLocation(kUniform2Name));
   EXPECT_EQ(kUniform3FakeLocation,
-             program->GetUniformFakeLocation(kUniform3BadName));
+            program->GetUniformFakeLocation(kUniform3BadName));
   // Check we can get uniform2 as "uniform2" even though the name is
   // "uniform2[0]"
   EXPECT_EQ(kUniform2FakeLocation,
@@ -628,8 +635,7 @@ TEST_F(ProgramManagerWithShaderTest, GetUniformFakeLocation) {
   EXPECT_EQ(ProgramManager::MakeFakeLocation(kUniform2FakeLocation, 2),
             program->GetUniformFakeLocation("uniform2[2]"));
   EXPECT_EQ(-1, program->GetUniformFakeLocation("uniform2[3]"));
-  EXPECT_EQ(ProgramManager::MakeFakeLocation(kUniform3FakeLocation, 1),
-            program->GetUniformFakeLocation("uniform3[1]"));
+  EXPECT_EQ(-1, program->GetUniformFakeLocation("uniform3[1]"));
   EXPECT_EQ(-1, program->GetUniformFakeLocation("uniform3[2]"));
 }
 
@@ -1067,6 +1073,50 @@ TEST_F(ProgramManagerWithShaderTest, ProgramInfoGetProgramInfo) {
             static_cast<uint32>(input - inputs));
 }
 
+// Some drivers optimize out unused uniform array elements, so their
+// location would be -1.
+TEST_F(ProgramManagerWithShaderTest, UnusedUniformArrayElements) {
+  CommonDecoder::Bucket bucket;
+  const Program* program = manager_.GetProgram(kClientProgramId);
+  ASSERT_TRUE(program != NULL);
+  // Emulate the situation that only the first element has a valid location.
+  // TODO(zmo): Don't assume these are in order.
+  for (size_t ii = 0; ii < arraysize(kUniforms); ++ii) {
+    Program::UniformInfo* uniform = const_cast<Program::UniformInfo*>(
+        program->GetUniformInfo(ii));
+    ASSERT_TRUE(uniform != NULL);
+    EXPECT_EQ(static_cast<size_t>(kUniforms[ii].size),
+              uniform->element_locations.size());
+    for (GLsizei jj = 1; jj < uniform->size; ++jj)
+      uniform->element_locations[jj] = -1;
+  }
+  program->GetProgramInfo(&manager_, &bucket);
+  ProgramInfoHeader* header =
+      bucket.GetDataAs<ProgramInfoHeader*>(0, sizeof(ProgramInfoHeader));
+  ASSERT_TRUE(header != NULL);
+  EXPECT_EQ(1u, header->link_status);
+  EXPECT_EQ(arraysize(kAttribs), header->num_attribs);
+  EXPECT_EQ(arraysize(kUniforms), header->num_uniforms);
+  const ProgramInput* inputs = bucket.GetDataAs<const ProgramInput*>(
+      sizeof(*header),
+      sizeof(ProgramInput) * (header->num_attribs + header->num_uniforms));
+  ASSERT_TRUE(inputs != NULL);
+  const ProgramInput* input = inputs + header->num_attribs;
+  for (uint32 ii = 0; ii < header->num_uniforms; ++ii) {
+    const UniformInfo& expected = kUniforms[ii];
+    EXPECT_EQ(expected.size, input->size);
+    const int32* locations = bucket.GetDataAs<const int32*>(
+        input->location_offset, sizeof(int32) * input->size);
+    ASSERT_TRUE(locations != NULL);
+    EXPECT_EQ(
+        ProgramManager::MakeFakeLocation(expected.fake_location, 0),
+        locations[0]);
+    for (int32 jj = 1; jj < input->size; ++jj)
+      EXPECT_EQ(-1, locations[jj]);
+    ++input;
+  }
+}
+
 TEST_F(ProgramManagerWithShaderTest, BindAttribLocationConflicts) {
   // Set up shader
   const GLuint kVShaderClientId = 1;