GLHelper: Address inconsistent mapping of SkColorType to GL format

content/common/gpu/client/gl_helper_readback_support.h

 // Copyright 2014 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.
 
 #ifndef CONTENT_COMMON_GPU_CLIENT_GL_HELPER_READBACK_SUPPORT_H_
 #define CONTENT_COMMON_GPU_CLIENT_GL_HELPER_READBACK_SUPPORT_H_
 
 #include <vector>
 
 #include "content/common/gpu/client/gl_helper.h"
 
 namespace content {
 
+enum FormatSupport { kReadbackSupported,

[piman, 2014/07/22 19:57:55]

nit: leave the enum in GLHelperReadbackSupport for scoping
nit2: enum values should be MACRO_STYLE per style guide. e.g. READBACK_SUPPORTED
nit3: formatting looks odd. Run 'git cl format' ?

[robert.bradford, 2014/07/23 16:58:45]

Enum moved in, and moved to MACRO_STYLE.

+                     kReadbackSwizzle,
+                     kReadbackNotSupported};
+
 class CONTENT_EXPORT GLHelperReadbackSupport {
  public:
   GLHelperReadbackSupport(gpu::gles2::GLES2Interface* gl);
 
   ~GLHelperReadbackSupport();
 
-  // Checks whether the readback (Color read format and type) is supported
-  // for the requested texture format  by the hardware or not.
-  // For ex: some hardwares have rgb565 readback
-  // support when binded with the frame buffer for others it may fail.
-  // Here we pass the internal textureformat as skia config.
-  bool IsReadbackConfigSupported(SkColorType texture_format);
 
+  // For a given color type retrieve whether readback is supported and if so
+  // how it should be performed. The |format|, |type| and |bytes_per_pixel| are
+  // the values that should be used with glReadPixels to facilitate the
+  // readback. If |can_swizzle| is true then this method will return
+  // kReadbackSwizzle if the data needs to be swizzled before using the
+  // returned |format| otherwise the method will return kReadbackSupported to
+  // indicate that readback is permitted of this color othewise
+  // kReadbackNotSupported will be returned.  This method always overwrites the
+  // out values irrespective of the return value

[piman, 2014/07/22 19:57:55]

nit: end comment with a period.

[robert.bradford, 2014/07/23 16:58:45]

Done.

+  FormatSupport ReadbackConfig(SkColorType color_type,
+                               const bool& can_swizzle,

[piman, 2014/07/22 19:57:55]

nit: bool instead of const bool &.

[robert.bradford, 2014/07/23 16:58:45]

Done.

+                               GLint& format,

[piman, 2014/07/22 19:57:56]

non-const references are forbidden by style guide. Use pointers.
GLenum

[robert.bradford, 2014/07/23 16:58:45]

Done.

+                               GLint& type,

[piman, 2014/07/22 19:57:56]

GLenum

[robert.bradford, 2014/07/23 16:58:45]

Done.

+                               int32& bytes_per_pixel);

[piman, 2014/07/22 19:57:55]

size_t

[robert.bradford, 2014/07/23 16:58:45]

Done.

   // Provides the additional readback format/type pairing for a render target
   // of a given format/type pairing
   void GetAdditionalFormat(GLint format, GLint type, GLint *format_out,
                            GLint *type_out);
  private:
-  enum FormatSupport { FORMAT_NOT_SUPPORTED = 0, FORMAT_SUPPORTED, };
-
   struct FormatCacheEntry {
     GLint format;
     GLint type;
     GLint read_format;
     GLint read_type;
   };
 
   // This populates the format_support_table with the list of supported
   // formats.
   void InitializeReadbackSupport();
@@ skipping 11 matching lines @@
 
   FormatSupport format_support_table_[kLastEnum_SkColorType + 1];
 
   gpu::gles2::GLES2Interface* gl_;
   std::vector<struct FormatCacheEntry> format_cache_;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_COMMON_GPU_CLIENT_GL_HELPER_READBACK_SUPPORT_H_