Add RGB565 Texture readback support in gl_helper

content/common/gpu/client/gl_helper_unittests.cc

Index: content/common/gpu/client/gl_helper_unittests.cc
diff --git a/content/common/gpu/client/gl_helper_unittests.cc b/content/common/gpu/client/gl_helper_unittests.cc
index 5abeabd260096ffc88cf4bdd0bf1b1ce1ec672d1..2adc00dba56cf041e0808ddb58d6b29553e39701 100644
--- a/content/common/gpu/client/gl_helper_unittests.cc
+++ b/content/common/gpu/client/gl_helper_unittests.cc
@@ -662,7 +662,8 @@ class GLHelperTest : public testing::Test {
     helper_->ReadbackTextureSync(
         dst_texture,
         gfx::Rect(0, 0, scaled_xsize, scaled_ysize),
-        static_cast<unsigned char*>(output_pixels.getPixels()));
+        static_cast<unsigned char*>(output_pixels.getPixels()),
+        SkBitmap::kARGB_8888_Config);
     if (flip) {
       // Flip the pixels back.
       FlipSKBitmap(&output_pixels);
@@ -841,6 +842,113 @@ class GLHelperTest : public testing::Test {
     }
   }
 
+  bool ColorComponentsClose(SkColor component1, SkColor component2) {
+    int c1 = static_cast<int>(component1);
+    int c2 = static_cast<int>(component2);
+    return std::abs(c1 - c2) <= 40;

[piman, 2014/01/15 21:32:53]

nit: 40 is arbitrary. It also seems high...
The right value probably depends on the config (565 has less resolution, so
needs more wiggle room than 8888). Mind passing it through from ColorClose, and
adjust as needed?

Given the test, for 8888 we should really have an exact result. For 565 we
should be within the precision difference, i.e. 3 bits, i.e. 7.

[sivag, 2014/01/16 11:50:59]

Done.

+  }
+
+  bool ColorsClose(SkColor color1, SkColor color2, SkBitmap::Config config) {
+    bool red = ColorComponentsClose(SkColorGetR(color1),
+                                    SkColorGetR(color2));
+    bool green = ColorComponentsClose(SkColorGetG(color1),
+                                        SkColorGetG(color2));
+    bool blue = ColorComponentsClose(SkColorGetB(color1),
+                                     SkColorGetB(color2));
+    bool alpha = ColorComponentsClose(SkColorGetB(color1),
+                                      SkColorGetB(color2));

[piman, 2014/01/15 21:32:53]

SkColorGetA ?

[sivag, 2014/01/16 11:50:59]

Done.

+    if (config == SkBitmap::kRGB_565_Config) {
+      return  red && blue && green;

[piman, 2014/01/15 21:32:53]

nit: You have 2 spaces before 'red'. Remove one.

[sivag, 2014/01/16 11:50:59]

Done.

+    }
+    return red && blue && green && alpha;
+  }
+
+  bool IsEqual(const SkBitmap& bmp1, const SkBitmap& bmp2) {
+    if (bmp1.isNull() && bmp2.isNull())
+      return true;
+    if (bmp1.width() != bmp2.width() ||
+        bmp1.height() != bmp2.height()) {
+        LOG(ERROR) << "Bitmap geometry check failure";
+        return false;
+    }
+    if (bmp1.getConfig() != bmp2.getConfig())
+      return false;
+
+    SkAutoLockPixels lock1(bmp1);
+    SkAutoLockPixels lock2(bmp2);
+    if (!bmp1.getPixels() || !bmp2.getPixels()) {
+      LOG(ERROR) << "Empty Bitmap!";
+      return false;
+    }
+    for (int y = 0; y < bmp1.height(); ++y) {
+      for (int x = 0; x < bmp1.width(); ++x) {
+        if (!ColorsClose(bmp1.getColor(x,y),
+                         bmp2.getColor(x,y),
+                         bmp1.getConfig())) {
+          LOG(ERROR) << "Bitmap color comparision failure";
+          return false;
+        }
+      }
+    }
+    return true;
+  }
+
+  // Test basic format readback.
+  bool TestTextureFormatReadback(const gfx::Size& src_size,
+                         SkBitmap::Config bitmap_config) {
+    DCHECK((bitmap_config == SkBitmap::kRGB_565_Config) ||
+           (bitmap_config == SkBitmap::kARGB_8888_Config));
+    bool rgb565_format = (bitmap_config == SkBitmap::kRGB_565_Config) ?
+                         true :

[piman, 2014/01/15 21:32:53]

nit: no need for ? true : false. (bitmap_config == SkBitmap::kRGB_565_Config) is
already a boolean.

[sivag, 2014/01/16 11:50:59]

Done.

+                         false;
+    if (rgb565_format && !helper_->CanUseRgb565Readback()) {
+      LOG(ERROR) << "RGB565 Format Not supported on this platform";
+      return false;
+    }
+    WebGLId src_texture = context_->createTexture();
+    SkBitmap input_pixels;
+    input_pixels.setConfig(bitmap_config, src_size.width(),
+                           src_size.height());
+    input_pixels.allocPixels();
+    SkAutoLockPixels lock1(input_pixels);
+    input_pixels.eraseARGB(0, 255, 0, 0);
+    context_->bindTexture(GL_TEXTURE_2D, src_texture);
+    GLenum format = (bitmap_config == SkBitmap::kRGB_565_Config) ?
+                    GL_RGB : GL_RGBA;
+    GLenum type = (bitmap_config == SkBitmap::kRGB_565_Config) ?
+                  GL_UNSIGNED_SHORT_5_6_5 : GL_UNSIGNED_BYTE;
+    context_->texImage2D(GL_TEXTURE_2D,
+                         0,
+                         format,
+                         src_size.width(),
+                         src_size.height(),
+                         0,
+                         format,
+                         type,
+                         input_pixels.getPixels());
+    SkBitmap output_pixels;
+    output_pixels.setConfig(bitmap_config, src_size.width(),
+                           src_size.height());
+    output_pixels.allocPixels();
+    SkAutoLockPixels lock2(output_pixels);
+    output_pixels.eraseARGB(0, 0, 255, 0);

[piman, 2014/01/15 21:32:53]

Can you add an explanation for why this doesn't match the color set in
input_pixels?

[sivag, 2014/01/16 11:50:59]

Done.

+    uint8* pixels = static_cast<uint8*>(output_pixels.getPixels());
+    helper_->ReadbackTextureSync(src_texture,
+                                 gfx::Rect(src_size),
+                                 pixels,
+                                 bitmap_config);
+    bool result = IsEqual(input_pixels, output_pixels);
+    if (!result) {
+      LOG(ERROR) << "Bitmap comparision failure";
+      return false;
+    }
+    context_->deleteTexture(src_texture);
+    if (HasFailure()) {
+      return false;
+    }
+    return true;
+  }
+
   // YUV readback test. Create a test pattern, convert to YUV
   // with reference implementation and compare to what gl_helper
   // returns.
@@ -1216,6 +1324,20 @@ class GLHelperTest : public testing::Test {
   std::deque<GLHelperScaling::ScaleOp> x_ops_, y_ops_;
 };
 
+TEST_F(GLHelperTest, RGBAReadBackTest) {
+  const int kTestSize = 64;
+  bool result = TestTextureFormatReadback(gfx::Size(kTestSize,kTestSize),
+                                          SkBitmap::kARGB_8888_Config);
+  EXPECT_EQ(result, true);
+}
+
+TEST_F(GLHelperTest, RGB565ReadBackTest) {
+  const int kTestSize = 64;
+  bool result = TestTextureFormatReadback(gfx::Size(kTestSize,kTestSize),
+                                          SkBitmap::kRGB_565_Config);
+  EXPECT_EQ(result, true);
+}
+
 TEST_F(GLHelperTest, YUVReadbackOptTest) {
   // This test uses the cb_command tracing events to detect how many
   // scaling passes are actually performed by the YUV readback pipeline.