Make GL_CHROMIUM_consistent_uniform_locations slighty more robust

Added a program argument so that at least in debug we can
verify locations are correct. It also means we could fallback
to actually calling GetUniformLocation if need be.

TEST=unit tests
BUG=132844


Review URL: https://chromiumcodereview.appspot.com/10581029

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

13 files changed, 132 insertions, 25 deletions

diff --git a/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_consistent_uniform_locations.txt b/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_consistent_uniform_locations.txt
index 8a7d163..e7e8907 100644
--- a/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_consistent_uniform_locations.txt
+++ b/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_consistent_uniform_locations.txt
@@ -19,14 +19,13 @@ Overview
     This extension guarantees uniforms always have the same locations.
 
     This allows the client program to know the locations of uniforms
-    without having to compile the shaders, link the program and query
-    the locations and therefore have no blocking calls when initializing
-    programs.
+    without having to wait for shaders to compile or GLSL programs to
+    link to query the locations and therefore have no blocking calls
+    when initializing programs.
 
     To be forward compatible the locations are provided through the
-    function GetUniformLocationsCHROMIUM but they can be provided
-    even before linking a program and therefore do not have to wait
-    for compile or link completion to return results.
+    function GetUniformLocationsCHROMIUM. LinkProgram must be called
+    before calling GetUniformLocationsCHROMIUM.
 
 Issues
 
@@ -40,7 +39,8 @@ New Tokens
 
 New Procedures and Functions
 
-    void GetUniformLocationsCHROMIUM (const GLUniformDefinitionCHROMIUM* uniforms,
+    void GetUniformLocationsCHROMIUM (GLuint program,
+                                      const GLUniformDefinitionCHROMIUM* uniforms,
                                       GLsizei count, GLsizei max_locations,
                                       GLint* locations);
 
@@ -84,3 +84,5 @@ New State
 Revision History
 
     7/17/2012    Documented the extension
+    7/19/2012    Added <program> argument.
+
diff --git a/gpu/command_buffer/client/gles2_c_lib_autogen.h b/gpu/command_buffer/client/gles2_c_lib_autogen.h
index 22016bd..5a36c8b 100644
--- a/gpu/command_buffer/client/gles2_c_lib_autogen.h
+++ b/gpu/command_buffer/client/gles2_c_lib_autogen.h
@@ -652,10 +652,10 @@ void GLES2ConsumeTextureCHROMIUM(GLenum target, const GLbyte* mailbox) {
   gles2::GetGLContext()->ConsumeTextureCHROMIUM(target, mailbox);
 }
 void GLES2GetUniformLocationsCHROMIUM(
-    const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count,
+    GLuint program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count,
     GLsizei max_locations, GLint* locations) {
   gles2::GetGLContext()->GetUniformLocationsCHROMIUM(
-      uniforms, count, max_locations, locations);
+      program, uniforms, count, max_locations, locations);
 }
 
 #endif  // GPU_COMMAND_BUFFER_CLIENT_GLES2_C_LIB_AUTOGEN_H_
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index 575d180..892d565 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -3295,13 +3295,18 @@ class GLUniformDefinitionComparer {
   const GLUniformDefinitionCHROMIUM* uniforms_;
 };
 
+
+
 }  // anonymous namespace.
 
 void GLES2Implementation::GetUniformLocationsCHROMIUM(
+    GLuint program,
     const GLUniformDefinitionCHROMIUM* uniforms,
     GLsizei count,
     GLsizei max_locations,
     GLint* locations) {
+  (void)program;  // To keep the compiler happy as it's unused in release.
+
   GPU_CLIENT_SINGLE_THREAD_CHECK();
   GPU_CLIENT_LOG("[" << this << "] glGenUniformLocationsCHROMIUM("
       << static_cast<const void*>(uniforms) << ", "  << count << ", "
@@ -3342,9 +3347,23 @@ void GLES2Implementation::GetUniformLocationsCHROMIUM(
       if (max_locations <= 0) {
         return;
       }
-      *locations++ = GLES2Util::SwizzleLocation(
+      GLint location = GLES2Util::SwizzleLocation(
           GLES2Util::MakeFakeLocation(base_location, jj));
+      *locations++ = location;
+      #if defined(GPU_CLIENT_DEBUG)
+        std::string name(def.name);
+        if (jj > 0) {
+          char buf[20];
+          sprintf(buf, "%d", jj);
+          name = name + "[" + buf + "]";
+        }
+        GPU_DCHECK_EQ(
+            location,
+            share_group_->program_info_manager()->GetUniformLocation(
+                this, program, name.c_str()));
+      #endif
       --max_locations;
+
     }
   }
 }
diff --git a/gpu/command_buffer/client/gles2_implementation_autogen.h b/gpu/command_buffer/client/gles2_implementation_autogen.h
index d756ea3..f4b18ae 100644
--- a/gpu/command_buffer/client/gles2_implementation_autogen.h
+++ b/gpu/command_buffer/client/gles2_implementation_autogen.h
@@ -1560,7 +1560,7 @@ void ConsumeTextureCHROMIUM(GLenum target, const GLbyte* mailbox) {
 }
 
 void GetUniformLocationsCHROMIUM(
-    const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count,
+    GLuint program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count,
     GLsizei max_locations, GLint* locations);
 
 #endif  // GPU_COMMAND_BUFFER_CLIENT_GLES2_IMPLEMENTATION_AUTOGEN_H_
diff --git a/gpu/command_buffer/client/gles2_implementation_unittest.cc b/gpu/command_buffer/client/gles2_implementation_unittest.cc
index 38d651b..9d40493 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest.cc
+++ b/gpu/command_buffer/client/gles2_implementation_unittest.cc
@@ -8,6 +8,7 @@
 
 #include <GLES2/gl2ext.h>
 #include "gpu/command_buffer/client/client_test_helper.h"
+#include "gpu/command_buffer/client/program_info_manager.h"
 #include "gpu/command_buffer/client/transfer_buffer.h"
 #include "gpu/command_buffer/common/command_buffer.h"
 #include "gpu/command_buffer/common/compiler_specific.h"
@@ -445,6 +446,12 @@ class GLES2ImplementationTest : public testing::Test {
     return transfer_buffer_->GetExpectedResultMemory(size);
   }
 
+  // Sets the ProgramInfoManager. The manager will be owned
+  // by the ShareGroup.
+  void SetProgramInfoManager(ProgramInfoManager* manager) {
+    gl_->share_group()->set_program_info_manager(manager);
+  }
+
   int CheckError() {
     ExpectedMemoryInfo result =
         GetExpectedResultMemory(sizeof(GetError::Result));
@@ -2558,7 +2565,43 @@ TEST_F(GLES2ImplementationTest, BeginEndQueryEXT) {
   EXPECT_EQ(0u, available);
 }
 
+namespace {
+
+class MockProgramInfoManager : public ProgramInfoManager {
+ public:
+  virtual ~MockProgramInfoManager() {};
+
+  MOCK_METHOD1(CreateInfo, void(GLuint program));
+
+  MOCK_METHOD1(DeleteInfo, void(GLuint program));
+
+  MOCK_METHOD4(GetProgramiv, bool(
+      GLES2Implementation* gl, GLuint program, GLenum pname, GLint* params));
+
+  MOCK_METHOD3(GetAttribLocation, GLint(
+      GLES2Implementation* gl, GLuint program, const char* name));
+
+  MOCK_METHOD3(GetUniformLocation, GLint(
+      GLES2Implementation* gl, GLuint program, const char* name));
+
+  MOCK_METHOD8(GetActiveAttrib, bool(
+      GLES2Implementation* gl,
+      GLuint program, GLuint index, GLsizei bufsize, GLsizei* length,
+      GLint* size, GLenum* type, char* name));
+
+  MOCK_METHOD8(GetActiveUniform, bool(
+      GLES2Implementation* gl,
+      GLuint program, GLuint index, GLsizei bufsize, GLsizei* length,
+      GLint* size, GLenum* type, char* name));
+};
+
+}  // anonymous namespace
+
 TEST_F(GLES2ImplementationTest, GetUniformLocationsCHROMIUM) {
+  MockProgramInfoManager* manager = new MockProgramInfoManager();
+  SetProgramInfoManager(manager);
+
+  const GLuint kProgramId = 123;
   static const GLUniformDefinitionCHROMIUM good_defs[] = {
     { GL_FLOAT_VEC4, 1, "moo", },
     { GL_FLOAT_VEC4, 4, "bar", },
@@ -2573,19 +2616,30 @@ TEST_F(GLES2ImplementationTest, GetUniformLocationsCHROMIUM) {
 
   // Test bad count
   GLint locations[50] = { -1, };
-  gl_->GetUniformLocationsCHROMIUM(bad_defs, 0, 1, locations);
+  gl_->GetUniformLocationsCHROMIUM(kProgramId, bad_defs, 0, 1, locations);
   EXPECT_EQ(GL_INVALID_VALUE, CheckError());
   EXPECT_EQ(-1, locations[0]);
 
   // Test bad size.
   gl_->GetUniformLocationsCHROMIUM(
-      bad_defs, arraysize(bad_defs), 1, locations);
+      kProgramId, bad_defs, arraysize(bad_defs), 1, locations);
   EXPECT_EQ(GL_INVALID_VALUE, CheckError());
   EXPECT_EQ(-1, locations[0]);
 
+  #if defined(GPU_CLIENT_DEBUG)
+  EXPECT_CALL(*manager, GetUniformLocation(_, kProgramId, _))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(2, 0))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 0))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 1))))
+      .RetiresOnSaturation();
+  #endif
+
   // Test max_locations
   gl_->GetUniformLocationsCHROMIUM(
-      good_defs, arraysize(good_defs), 3, locations);
+      kProgramId, good_defs, arraysize(good_defs), 3, locations);
   EXPECT_EQ(GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(2, 0)),
             locations[0]);
   EXPECT_EQ(GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 0)),
@@ -2594,9 +2648,31 @@ TEST_F(GLES2ImplementationTest, GetUniformLocationsCHROMIUM) {
             locations[2]);
   EXPECT_EQ(0, locations[3]);
 
+  #if defined(GPU_CLIENT_DEBUG)
+  EXPECT_CALL(*manager, GetUniformLocation(_, kProgramId, _))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(2, 0))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 0))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 1))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 2))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 3))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(1, 0))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(1, 1))))
+      .WillOnce(Return(
+          GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(1, 2))))
+      .RetiresOnSaturation();
+  #endif
+
   // Test all.
   gl_->GetUniformLocationsCHROMIUM(
-      good_defs, arraysize(good_defs), arraysize(locations), locations);
+      kProgramId, good_defs, arraysize(good_defs), arraysize(locations),
+      locations);
   EXPECT_EQ(GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(2, 0)),
             locations[0]);
   EXPECT_EQ(GLES2Util::SwizzleLocation(GLES2Util::MakeFakeLocation(0, 0)),
@@ -2621,4 +2697,3 @@ TEST_F(GLES2ImplementationTest, GetUniformLocationsCHROMIUM) {
 }  // namespace gles2
 }  // namespace gpu
 
-
diff --git a/gpu/command_buffer/client/program_info_manager.h b/gpu/command_buffer/client/program_info_manager.h
index 25108e2..099f182 100644
--- a/gpu/command_buffer/client/program_info_manager.h
+++ b/gpu/command_buffer/client/program_info_manager.h
@@ -6,6 +6,7 @@
 #define GPU_COMMAND_BUFFER_CLIENT_PROGRAM_INFO_MANAGER_H_
 
 #include <GLES2/gl2.h>
+#include "gles2_impl_export.h"
 
 namespace gpu {
 namespace gles2 {
@@ -13,7 +14,7 @@ namespace gles2 {
 class GLES2Implementation;
 
 // Manages info about OpenGL ES Programs.
-class ProgramInfoManager {
+class GLES2_IMPL_EXPORT ProgramInfoManager {
  public:
   virtual ~ProgramInfoManager();
 
diff --git a/gpu/command_buffer/client/share_group.cc b/gpu/command_buffer/client/share_group.cc
index 39d9b85..fbddbb8 100644
--- a/gpu/command_buffer/client/share_group.cc
+++ b/gpu/command_buffer/client/share_group.cc
@@ -228,6 +228,10 @@ void ShareGroup::SetGLES2ImplementationForDestruction(
   gles2_ = gl_impl;
 }
 
+void ShareGroup::set_program_info_manager(ProgramInfoManager* manager) {
+  program_info_manager_.reset(manager);
+}
+
 ShareGroup::~ShareGroup() {
   for (int i = 0; i < id_namespaces::kNumIdNamespaces; ++i) {
     id_handlers_[i]->Destroy(gles2_);
diff --git a/gpu/command_buffer/client/share_group.h b/gpu/command_buffer/client/share_group.h
index 6762ff6..40cdfbe 100644
--- a/gpu/command_buffer/client/share_group.h
+++ b/gpu/command_buffer/client/share_group.h
@@ -15,6 +15,7 @@ namespace gpu {
 namespace gles2 {
 
 class GLES2Implementation;
+class GLES2ImplementationTest;
 class ProgramInfoManager;
 
 typedef void (GLES2Implementation::*DeleteFn)(GLsizei n, const GLuint* ids);
@@ -72,8 +73,12 @@ class GLES2_IMPL_EXPORT ShareGroup
 
  private:
   friend class gpu::RefCountedThreadSafe<ShareGroup>;
+  friend class gpu::gles2::GLES2ImplementationTest;
   ~ShareGroup();
 
+  // Install a new program info manager. Used for testing only;
+  void set_program_info_manager(ProgramInfoManager* manager);
+
   scoped_ptr<IdHandlerInterface> id_handlers_[id_namespaces::kNumIdNamespaces];
   scoped_ptr<ProgramInfoManager> program_info_manager_;
 
diff --git a/gpu/command_buffer/cmd_buffer_functions.txt b/gpu/command_buffer/cmd_buffer_functions.txt
index 5e36578..5fdb18a 100644
--- a/gpu/command_buffer/cmd_buffer_functions.txt
+++ b/gpu/command_buffer/cmd_buffer_functions.txt
@@ -185,5 +185,5 @@ GL_APICALL void         GL_APIENTRY glVertexAttribDivisorANGLE (GLuint index, GL
 GL_APICALL void         GL_APIENTRY glGenMailboxCHROMIUM (GLbyte* mailbox);
 GL_APICALL void         GL_APIENTRY glProduceTextureCHROMIUM (GLenumTextureTarget target, const GLbyte* mailbox);
 GL_APICALL void         GL_APIENTRY glConsumeTextureCHROMIUM (GLenumTextureTarget target, const GLbyte* mailbox);
-GL_APICALL void         GL_APIENTRY glGetUniformLocationsCHROMIUM (const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
+GL_APICALL void         GL_APIENTRY glGetUniformLocationsCHROMIUM (GLidProgram program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
 
diff --git a/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h b/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h
index 35982fe..8f2818b 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h
+++ b/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h
@@ -224,7 +224,7 @@ static GLES2Util::EnumToString enum_to_string_table[] = {
   { 0x00000400, "GL_STENCIL_BUFFER_BIT", },
   { 0x800A, "GL_FUNC_SUBTRACT", },
   { 0x8E2C, "GL_DEPTH_COMPONENT16_NONLINEAR_NV", },
-  { 0x8508, "GL_DECR_WRAP", },
+  { 0x889F, "GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING", },
   { 0x8006, "GL_FUNC_ADD", },
   { 0x8007, "GL_MIN_EXT", },
   { 0x8004, "GL_ONE_MINUS_CONSTANT_ALPHA", },
@@ -501,7 +501,7 @@ static GLES2Util::EnumToString enum_to_string_table[] = {
   { 0x8CD6, "GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT", },
   { 0x8253, "GL_GUILTY_CONTEXT_RESET_EXT", },
   { 0x8872, "GL_MAX_TEXTURE_IMAGE_UNITS", },
-  { 0x889F, "GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING", },
+  { 0x8508, "GL_DECR_WRAP", },
   { 0x8507, "GL_INCR_WRAP", },
   { 0x8895, "GL_ELEMENT_ARRAY_BUFFER_BINDING", },
   { 0x8894, "GL_ARRAY_BUFFER_BINDING", },
diff --git a/gpu/command_buffer/tests/gl_consistent_uniform_locations_unittest.cc b/gpu/command_buffer/tests/gl_consistent_uniform_locations_unittest.cc
index b831368..e5f7dc3 100644
--- a/gpu/command_buffer/tests/gl_consistent_uniform_locations_unittest.cc
+++ b/gpu/command_buffer/tests/gl_consistent_uniform_locations_unittest.cc
@@ -65,18 +65,18 @@ TEST_F(ConsistenUniformLocationsTest, Basic) {
     { GL_FLOAT_VEC4, 1, "u_colorA", },
   };
 
+  GLuint program = GLTestHelper::LoadProgram(v_shader_str, f_shader_str);
+
   GLint locations[4];
 
   glGetUniformLocationsCHROMIUM(
-      defs, arraysize(defs), arraysize(locations), locations);
+      program, defs, arraysize(defs), arraysize(locations), locations);
 
   GLint u_colorCLocation = locations[0];
   GLint u_colorB0Location = locations[1];
   GLint u_colorB1Location = locations[2];
   GLint u_colorALocation = locations[3];
 
-  GLuint program = GLTestHelper::LoadProgram(v_shader_str, f_shader_str);
-
   GLint position_loc = glGetAttribLocation(program, "a_position");
 
   GLTestHelper::SetupUnitQuad(position_loc);
diff --git a/third_party/khronos/GLES2/gl2ext.h b/third_party/khronos/GLES2/gl2ext.h
index e6a728c..0c22849 100644
--- a/third_party/khronos/GLES2/gl2ext.h
+++ b/third_party/khronos/GLES2/gl2ext.h
@@ -2018,10 +2018,10 @@ struct GLUniformDefinitionCHROMIUM {
 #ifdef GL_GLEXT_PROTOTYPES
 #define glGetUniformLocationsCHROMIUM GLES2_GET_FUN(GetUniformLocationsCHROMIUM)
 #if !defined(GLES2_USE_CPP_BINDINGS)
-GL_APICALL void GL_APIENTRY glGetUniformLocationsCHROMIUM (const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
+GL_APICALL void GL_APIENTRY glGetUniformLocationsCHROMIUM (GLuint program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
 #endif
 #else
-typedef void (GL_APIENTRYP PFNGLGETUNIFORMLOCATIONSCHROMIUM) (const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
+typedef void (GL_APIENTRYP PFNGLGETUNIFORMLOCATIONSCHROMIUM) (GLuint program, const GLUniformDefinitionCHROMIUM* uniforms, GLsizei count, GLsizei max_locations, GLint* locations);
 #endif
 #endif
 
diff --git a/third_party/khronos/README.chromium b/third_party/khronos/README.chromium
index 1910b57..25489ef 100644
--- a/third_party/khronos/README.chromium
+++ b/third_party/khronos/README.chromium
@@ -23,5 +23,6 @@ GLES2/gl2ext.h
  - Added GL_CHROMIUM_command_buffer_query
  - Added GL_CHROMIUM_copy_texture
  - Added GL_CHROMIUM_consistent_uniform_locations
+ - Update GL_CHROMIUM_consistent_uniform_locations to have a program argument.
 EGL/eglplatform.h
  - Added EGLNative*Type for Mac.