Command buffer: Fix bugs for drawing when the type of vertex attrib is a packed type

gpu/command_buffer/common/gles2_cmd_utils.cc

Index: gpu/command_buffer/common/gles2_cmd_utils.cc
diff --git a/gpu/command_buffer/common/gles2_cmd_utils.cc b/gpu/command_buffer/common/gles2_cmd_utils.cc
index 0011053ad2d63e0892e809baf6c74e1feb57d83b..67a390faeb085b16c23682e46fb8e8bbffd8c9df 100644
--- a/gpu/command_buffer/common/gles2_cmd_utils.cc
+++ b/gpu/command_buffer/common/gles2_cmd_utils.cc
@@ -815,10 +815,16 @@ size_t GLES2Util::GetGLTypeSizeForTexturesAndBuffers(uint32_t type) {
       return sizeof(GLfixed);  // NOLINT
     case GL_HALF_FLOAT:
       return sizeof(GLushort);  // NOLINT
+    // Packed types need to be handled specially for data_size computation.
+    // See the code in GLES2DecoderImpl::HandleVertexAttribPointer and
+    // VertexAttrib::canAccess: group_size = component_count * component_size.
+    // component_count is assigned by 'size' parameter in vertexAttribPointer,
+    // which is constant 4. component_size is get by this function, so the
+    // size of every component should be 1 for packed types.
     case GL_INT_2_10_10_10_REV:
-      return sizeof(GLint);  // NOLINT
+      return sizeof(GLint) / 4;  // NOLINT

[Zhenyao Mo, 2016/02/25 21:40:43]

I still disagree with this.

This function calls GetTypeSize, and we should return type size, as simple as
that.  Any hacking to fix a bug somewhere, you make the implementation different
than what the function name indicates, then sooner or later a bug might happen
because someone didn't realize your special case.

I suggest the following:

1) Change this function name to GetGLTypeSizeForBuffers().  For textures we have
BytesPerElement(). The function stays as is.

2) Add another helper function GetGroupSizeForBufferType(GLenum type, GLint
size), inside you can deal with the packed type situation. Most callers of
GetGLTypeSizeForBuffers() can be switched to GetGroupSizeForBufferType().

3) In GLES2DecoderImpl::HandleVertexAttribPointer, you write this special case
handling for packed type there.

[yunchao, 2016/02/26 06:45:26]

Thanks for your suggestion, Zhenyao. It is a pretty good solution.I should think
of this carefully and get a good solution like this. 

     case GL_UNSIGNED_INT_2_10_10_10_REV:
-      return sizeof(GLuint);  // NOLINT
+      return sizeof(GLuint) / 4;  // NOLINT
     default:
       return 0;
   }