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

gpu/command_buffer/common/gles2_cmd_utils.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file is here so other GLES2 related files can have a common set of
 // includes where appropriate.
 
 #include <sstream>
 #include <GLES2/gl2.h>
 #include <GLES2/gl2ext.h>
@@ skipping 797 matching lines @@
     case GL_INT:
       return sizeof(GLint);  // NOLINT
     case GL_UNSIGNED_INT:
       return sizeof(GLuint);  // NOLINT
     case GL_FLOAT:
       return sizeof(GLfloat);  // NOLINT
     case GL_FIXED:
       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;
   }
 }
 
 size_t GLES2Util::GetComponentCountForGLTransformType(uint32_t type) {
   switch (type) {
     case GL_TRANSLATE_X_CHROMIUM:
     case GL_TRANSLATE_Y_CHROMIUM:
       return 1;
@@ skipping 685 matching lines @@
   }
 
   return true;
 }
 
 #include "gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu