Switch to use ANGLE's new APIs to query shader variables.

gpu/command_buffer/service/program_manager.cc

Index: gpu/command_buffer/service/program_manager.cc
diff --git a/gpu/command_buffer/service/program_manager.cc b/gpu/command_buffer/service/program_manager.cc
index 4dd4bc4d68f4076c8371127fac2f5a45e317f6e8..122c8b7d2f1b0ba9ad374f907e8ff01f466cd195 100644
--- a/gpu/command_buffer/service/program_manager.cc
+++ b/gpu/command_buffer/service/program_manager.cc
@@ -15,6 +15,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "gpu/command_buffer/common/gles2_cmd_format.h"
 #include "gpu/command_buffer/common/gles2_cmd_utils.h"
@@ -33,28 +34,6 @@ namespace gles2 {
 
 namespace {
 
-struct UniformType {
-  explicit UniformType(const ShaderTranslator::VariableInfo uniform)
-      : type(uniform.type),
-        size(uniform.size),
-        precision(uniform.precision) { }
-
-  UniformType()
-      : type(0),
-        size(0),
-        precision(SH_PRECISION_MEDIUMP) { }
-
-  bool operator==(const UniformType& other) const {
-    return type == other.type &&
-        size == other.size &&
-        precision == other.precision;
-  }
-
-  int type;
-  int size;
-  int precision;
-};
-
 int ShaderTypeToIndex(GLenum shader_type) {
   switch (shader_type) {
     case GL_VERTEX_SHADER:
@@ -389,17 +368,15 @@ void Program::Update() {
     DCHECK(max_len == 0 || length < max_len);
     DCHECK(length == 0 || name_buffer[length] == '\0');
     if (!ProgramManager::IsInvalidPrefix(name_buffer.get(), length)) {
-      std::string name;
       std::string original_name;
-      GetCorrectedVariableInfo(
-          false, name_buffer.get(), &name, &original_name, &size, &type);
+      GetCorrectedVertexAttrib(name_buffer.get(), &original_name, &type);
       // TODO(gman): Should we check for error?
       GLint location = glGetAttribLocation(service_id_, name_buffer.get());
       if (location > max_location) {
         max_location = location;
       }
       attrib_infos_.push_back(
-          VertexAttrib(size, type, original_name, location));
+          VertexAttrib(1, type, original_name, location));
       max_attrib_name_length_ = std::max(
           max_attrib_name_length_, static_cast<GLsizei>(original_name.size()));
     }
@@ -447,9 +424,9 @@ void Program::Update() {
     DCHECK(length == 0 || name_buffer[length] == '\0');
     if (!ProgramManager::IsInvalidPrefix(name_buffer.get(), length)) {
       data.queried_name = std::string(name_buffer.get());
-      GetCorrectedVariableInfo(
-          true, name_buffer.get(), &data.corrected_name, &data.original_name,
-          &data.size, &data.type);
+      GetCorrectedUniformData(
+          data.queried_name,
+          &data.corrected_name, &data.original_name, &data.size, &data.type);
       uniform_data.push_back(data);
     }
   }
@@ -757,41 +734,53 @@ bool Program::SetUniformLocationBinding(
 
 // Note: This is only valid to call right after a program has been linked
 // successfully.
-void Program::GetCorrectedVariableInfo(
-    bool use_uniforms,
-    const std::string& name, std::string* corrected_name,
-    std::string* original_name,
+void Program::GetCorrectedUniformData(
+    const std::string& name,
+    std::string* corrected_name, std::string* original_name,
     GLsizei* size, GLenum* type) const {
-  DCHECK(corrected_name);
-  DCHECK(original_name);
-  DCHECK(size);
-  DCHECK(type);
-  const char* kArraySpec = "[0]";
-  for (int jj = 0; jj < 2; ++jj) {
-    std::string test_name(name + ((jj == 1) ? kArraySpec : ""));
-    for (int ii = 0; ii < kMaxAttachedShaders; ++ii) {
-      Shader* shader = attached_shaders_[ii].get();
-      if (shader) {
-        const Shader::VariableInfo* variable_info =
-            use_uniforms ? shader->GetUniformInfo(test_name) :
-                           shader->GetAttribInfo(test_name);
-        // Note: There is an assuption here that if an attrib is defined in more
-        // than 1 attached shader their types and sizes match. Should we check
-        // for that case?
-        if (variable_info) {
-          *corrected_name = test_name;
-          *original_name = variable_info->name;
-          *type = variable_info->type;
-          *size = variable_info->size;
-          return;
-        }
+  DCHECK(corrected_name && original_name && size && type);
+  for (int ii = 0; ii < kMaxAttachedShaders; ++ii) {
+    Shader* shader = attached_shaders_[ii].get();
+    if (!shader)
+      continue;
+    const sh::ShaderVariable* info = NULL;
+    const sh::Uniform* uniform = shader->GetUniformInfo(name);
+    bool found = false;
+    if (uniform)
+      found = uniform->findInfoByMappedName(name, &info, original_name);
+    if (found) {
+      const std::string kArraySpec("[0]");
+      if (info->arraySize > 0 && !EndsWith(name, kArraySpec, true)) {
+        *corrected_name = name + kArraySpec;
+        *original_name += kArraySpec;
+      } else {
+        *corrected_name = name;
       }
+      *type = info->type;
+      *size = std::max(1, info->arraySize);
+      return;
     }
   }
   *corrected_name = name;
   *original_name = name;

[Ken Russell (switch to Gerrit), 2014/10/07 03:35:26]

Why is it OK to silently pass through the incoming name as corrected_name and
original_name in this case, not assign "type" or "size", and not return any
error code to the caller?

[Zhenyao Mo, 2014/10/07 18:23:40]

Good question.  In theory this path should never be hit, because ANGLE
translator should always return the correct information.  I thought about this
while working on this CL, but decided not to add extra complication in it. Add a
TODO() for now.

 }
 
+void Program::GetCorrectedVertexAttrib(
+    const std::string& name, std::string* original_name, GLenum* type) const {
+  DCHECK(original_name);
+  DCHECK(type);
+  Shader* shader = attached_shaders_[ShaderTypeToIndex(GL_VERTEX_SHADER)].get();
+  if (shader) {
+    const sh::Attribute* info = shader->GetAttribInfo(name);

[Ken Russell (switch to Gerrit), 2014/10/07 03:35:26]

Somewhere it should be documented that vertex attributes cannot be arrays or
structures, and that's why further fixups to the sh::Attribute aren't necessary
like they are for uniforms, above.

[Zhenyao Mo, 2014/10/07 18:23:40]

Done.

+    if (info) {
+      *original_name = info->name;
+      *type = info->type;
+      return;
+    }
+  }
+  *original_name = name;

[Ken Russell (switch to Gerrit), 2014/10/07 03:35:26]

Why is it OK to silently pass through the incoming name as original_name in this
case, not assign "type", and not return any error code to the caller?

[Zhenyao Mo, 2014/10/07 18:23:40]

Same reply as above.

+}
+
 bool Program::AddUniformInfo(
         GLsizei size, GLenum type, GLint location, GLint fake_base_location,
         const std::string& name, const std::string& original_name,
@@ -1014,23 +1003,20 @@ bool Program::DetectAttribLocationBindingConflicts() const {
 }
 
 bool Program::DetectUniformsMismatch(std::string* conflicting_name) const {
-  typedef std::map<std::string, UniformType> UniformMap;
-  UniformMap uniform_map;
+  typedef std::map<std::string, const sh::Uniform*> UniformPointerMap;
+  UniformPointerMap uniform_pointer_map;
   for (int ii = 0; ii < kMaxAttachedShaders; ++ii) {
-    const ShaderTranslator::VariableMap& shader_uniforms =
-        attached_shaders_[ii]->uniform_map();
-    for (ShaderTranslator::VariableMap::const_iterator iter =
-             shader_uniforms.begin();
+    const UniformMap& shader_uniforms = attached_shaders_[ii]->uniform_map();
+    for (UniformMap::const_iterator iter = shader_uniforms.begin();
          iter != shader_uniforms.end(); ++iter) {
       const std::string& name = iter->first;
-      UniformType type(iter->second);
-      UniformMap::iterator map_entry = uniform_map.find(name);
-      if (map_entry == uniform_map.end()) {
-        uniform_map[name] = type;
+      UniformPointerMap::iterator hit = uniform_pointer_map.find(name);
+      if (hit == uniform_pointer_map.end()) {
+        uniform_pointer_map[name] = &(iter->second);
       } else {
-        // If a uniform is already in the map, i.e., it has already been
-        // declared by other shader, then the type and precision must match.
-        if (map_entry->second == type)
+        // If a uniform is in the map, i.e., it has already been declared by
+        // another shader, then the type, precision, etc. must match.
+        if (hit->second->isSameUniformAtLinkTime(iter->second))
           continue;
         *conflicting_name = name;
         return true;
@@ -1045,30 +1031,25 @@ bool Program::DetectVaryingsMismatch(std::string* conflicting_name) const {
          attached_shaders_[0]->shader_type() == GL_VERTEX_SHADER &&
          attached_shaders_[1].get() &&
          attached_shaders_[1]->shader_type() == GL_FRAGMENT_SHADER);
-  const ShaderTranslator::VariableMap* vertex_varyings =
-      &(attached_shaders_[0]->varying_map());
-  const ShaderTranslator::VariableMap* fragment_varyings =
-      &(attached_shaders_[1]->varying_map());
+  const VaryingMap* vertex_varyings = &(attached_shaders_[0]->varying_map());
+  const VaryingMap* fragment_varyings = &(attached_shaders_[1]->varying_map());
 
-  for (ShaderTranslator::VariableMap::const_iterator iter =
-           fragment_varyings->begin();
+  for (VaryingMap::const_iterator iter = fragment_varyings->begin();
        iter != fragment_varyings->end(); ++iter) {
     const std::string& name = iter->first;
     if (IsBuiltInVarying(name))
       continue;
 
-    ShaderTranslator::VariableMap::const_iterator hit =
-        vertex_varyings->find(name);
+    VaryingMap::const_iterator hit = vertex_varyings->find(name);
     if (hit == vertex_varyings->end()) {
-      if (iter->second.static_use) {
+      if (iter->second.staticUse) {
         *conflicting_name = name;
         return true;
       }
       continue;
     }
 
-    if (hit->second.type != iter->second.type ||
-        hit->second.size != iter->second.size) {
+    if (!hit->second.isSameVaryingAtLinkTime(iter->second)) {
       *conflicting_name = name;
       return true;
     }
@@ -1082,14 +1063,14 @@ bool Program::DetectGlobalNameConflicts(std::string* conflicting_name) const {
          attached_shaders_[0]->shader_type() == GL_VERTEX_SHADER &&
          attached_shaders_[1].get() &&
          attached_shaders_[1]->shader_type() == GL_FRAGMENT_SHADER);
-  const ShaderTranslator::VariableMap* uniforms[2];
+  const UniformMap* uniforms[2];
   uniforms[0] = &(attached_shaders_[0]->uniform_map());
   uniforms[1] = &(attached_shaders_[1]->uniform_map());
-  const ShaderTranslator::VariableMap* attribs =
+  const AttributeMap* attribs =
       &(attached_shaders_[0]->attrib_map());
 
-  for (ShaderTranslator::VariableMap::const_iterator iter =
-           attribs->begin(); iter != attribs->end(); ++iter) {
+  for (AttributeMap::const_iterator iter = attribs->begin();
+       iter != attribs->end(); ++iter) {
     for (int ii = 0; ii < 2; ++ii) {
       if (uniforms[ii]->find(iter->first) != uniforms[ii]->end()) {
         *conflicting_name = iter->first;
@@ -1106,30 +1087,27 @@ bool Program::CheckVaryingsPacking(
          attached_shaders_[0]->shader_type() == GL_VERTEX_SHADER &&
          attached_shaders_[1].get() &&
          attached_shaders_[1]->shader_type() == GL_FRAGMENT_SHADER);
-  const ShaderTranslator::VariableMap* vertex_varyings =
-      &(attached_shaders_[0]->varying_map());
-  const ShaderTranslator::VariableMap* fragment_varyings =
-      &(attached_shaders_[1]->varying_map());
+  const VaryingMap* vertex_varyings = &(attached_shaders_[0]->varying_map());
+  const VaryingMap* fragment_varyings = &(attached_shaders_[1]->varying_map());
 
   std::map<std::string, ShVariableInfo> combined_map;
 
-  for (ShaderTranslator::VariableMap::const_iterator iter =
-           fragment_varyings->begin();
+  for (VaryingMap::const_iterator iter = fragment_varyings->begin();
        iter != fragment_varyings->end(); ++iter) {
-    if (!iter->second.static_use && option == kCountOnlyStaticallyUsed)
+    if (!iter->second.staticUse && option == kCountOnlyStaticallyUsed)
       continue;
     if (!IsBuiltInVarying(iter->first)) {
-      ShaderTranslator::VariableMap::const_iterator vertex_iter =
+      VaryingMap::const_iterator vertex_iter =
           vertex_varyings->find(iter->first);
       if (vertex_iter == vertex_varyings->end() ||
-          (!vertex_iter->second.static_use &&
+          (!vertex_iter->second.staticUse &&
            option == kCountOnlyStaticallyUsed))
         continue;
     }
 
     ShVariableInfo var;
     var.type = static_cast<sh::GLenum>(iter->second.type);
-    var.size = iter->second.size;
+    var.size = std::max(1, iter->second.arraySize);
     combined_map[iter->first] = var;
   }