Implement GLHelperReadbackSupport for GLHelper usage.

content/common/gpu/client/gl_helper.h

Index: content/common/gpu/client/gl_helper.h
diff --git a/content/common/gpu/client/gl_helper.h b/content/common/gpu/client/gl_helper.h
index 46db753207217ae13659920803008f68b739df8c..38618c0494b03bdf7eaf93992a313b2b4783d84c 100644
--- a/content/common/gpu/client/gl_helper.h
+++ b/content/common/gpu/client/gl_helper.h
@@ -5,6 +5,8 @@
 #ifndef CONTENT_COMMON_GPU_CLIENT_GL_HELPER_H_
 #define CONTENT_COMMON_GPU_CLIENT_GL_HELPER_H_
 
+#include <map>
+
 #include "base/atomicops.h"
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ -277,8 +279,12 @@ class CONTENT_EXPORT GLHelper {
   // size of the framebuffer.
   void CopyTextureFullImage(GLuint texture, const gfx::Size& size);
 
-  // Check whether rgb565 readback is supported or not.
-  bool CanUseRgb565Readback();
+  // Helper functions for checking the supported texture formats.
+  bool SupportsFormat(GLint format, GLint type);

[no sievers, 2014/02/18 19:35:44]

nit: can be private

[sivag, 2014/02/19 15:49:04]

Done.

+
+  // Checks whether the readback using passed texture format
+  // is supported or not.
+  bool IsReadBackConfigSupported(const SkBitmap::Config& texture_format);

[no sievers, 2014/02/18 19:35:44]

Please describe that this does not specify whether conversion during readback is
supported. Based on the test we do, we only know that the readback *should* work
if the texture format/type exactly match the readback format/type.
Maybe it even makes more sense to pass in the GL format/type here?

[sivag, 2014/02/19 15:49:04]

The reason why i kept a SkBitmap::Config is  in CopyfromCOmpositingSurface i
wanted to do early return if the requested format is not supported.

Any way we have the SupportsFormat API which takes format/type as the input.

Do you think we have to alter the function names?

 
   // A scaler will cache all intermediate textures and programs
   // needed to scale from a specified size to a destination size.
@@ -340,8 +346,7 @@ class CONTENT_EXPORT GLHelper {
   gpu::ContextSupport* context_support_;
   scoped_ptr<CopyTextureToImpl> copy_texture_to_impl_;
   scoped_ptr<GLHelperScaling> scaler_impl_;
-  bool initialized_565_format_check_;
-  bool support_565_format_;
+  std::map<SkBitmap::Config, bool> config_map_;

[piman, 2014/02/18 22:23:15]

map is way overkill.
SkBitmap::Config is a finite set - just use an array?

[sivag, 2014/02/19 15:49:04]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(GLHelper);
 };