Revert 110783 - Adds a bounds parameter to ui::Compositor::ReadPixels

Review URL: http://codereview.chromium.org/8561016

TBR=pkotwicz@chromium.org
Review URL: http://codereview.chromium.org/8558033

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@110785 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 26 insertions, 52 deletions

diff --git a/ui/gfx/compositor/compositor.h b/ui/gfx/compositor/compositor.h
index 2b41553..23fff5b 100644
--- a/ui/gfx/compositor/compositor.h
+++ b/ui/gfx/compositor/compositor.h
@@ -136,10 +136,9 @@ class COMPOSITOR_EXPORT Compositor : public base::RefCounted<Compositor> {
   // compositing.
   void Draw(bool force_clear);
 
-  // Reads the region |bounds| of the contents of the last rendered frame
-  // into the given bitmap.
+  // Reads the contents of the last rendered frame into the given bitmap.
   // Returns false if the pixels could not be read.
-  virtual bool ReadPixels(SkBitmap* bitmap, const gfx::Rect& bounds) = 0;
+  virtual bool ReadPixels(SkBitmap* bitmap) = 0;
 
   // Notifies the compositor that the size of the widget that it is
   // drawing to has changed.
diff --git a/ui/gfx/compositor/compositor_cc.cc b/ui/gfx/compositor/compositor_cc.cc
index ec70eb3..ce75713 100644
--- a/ui/gfx/compositor/compositor_cc.cc
+++ b/ui/gfx/compositor/compositor_cc.cc
@@ -182,21 +182,14 @@ void CompositorCC::DrawTree() {
   host_.composite();
 }
 
-bool CompositorCC::ReadPixels(SkBitmap* bitmap, const gfx::Rect& bounds) {
-  if (bounds.right() > size().width() || bounds.bottom() > size().height())
-    return false;
-  // Convert to OpenGL coordinates.
-  gfx::Point new_origin(bounds.x(),
-                        size().height() - bounds.height() - bounds.y());
-
+bool CompositorCC::ReadPixels(SkBitmap* bitmap) {
   bitmap->setConfig(SkBitmap::kARGB_8888_Config,
-                    bounds.width(), bounds.height());
+                    size().width(), size().height());
   bitmap->allocPixels();
   SkAutoLockPixels lock_image(*bitmap);
   unsigned char* pixels = static_cast<unsigned char*>(bitmap->getPixels());
-  if (host_.compositeAndReadback(pixels,
-                                 gfx::Rect(new_origin, bounds.size()))) {
-    SwizzleRGBAToBGRAAndFlip(pixels, bounds.size());
+  if (host_.compositeAndReadback(pixels, gfx::Rect(size()))) {
+    SwizzleRGBAToBGRAAndFlip(pixels, size());
     return true;
   }
   return false;
diff --git a/ui/gfx/compositor/compositor_cc.h b/ui/gfx/compositor/compositor_cc.h
index 5e2842f..8ffcd6c 100644
--- a/ui/gfx/compositor/compositor_cc.h
+++ b/ui/gfx/compositor/compositor_cc.h
@@ -100,7 +100,7 @@ class COMPOSITOR_EXPORT CompositorCC
   virtual void OnWidgetSizeChanged() OVERRIDE;
   virtual void OnRootLayerChanged() OVERRIDE;
   virtual void DrawTree() OVERRIDE;
-  virtual bool ReadPixels(SkBitmap* bitmap, const gfx::Rect& bounds) OVERRIDE;
+  virtual bool ReadPixels(SkBitmap* bitmap) OVERRIDE;
 
   // WebLayerTreeViewClient implementation.
   virtual void animateAndLayout(double frameBeginTime);
diff --git a/ui/gfx/compositor/compositor_gl.cc b/ui/gfx/compositor/compositor_gl.cc
index bbe5365..e71a0e0 100644
--- a/ui/gfx/compositor/compositor_gl.cc
+++ b/ui/gfx/compositor/compositor_gl.cc
@@ -558,15 +558,12 @@ CompositorGL::~CompositorGL() {
   gl_context_ = NULL;
 }
 
-bool CompositorGL::ReadPixels(SkBitmap* bitmap, const gfx::Rect& bounds) {
+bool CompositorGL::ReadPixels(SkBitmap* bitmap) {
   MakeCurrent();
 
-  if (bounds.right() > size().width() || bounds.bottom() > size().height())
-    return false;
-
   bitmap->setConfig(SkBitmap::kARGB_8888_Config,
-                    bounds.width(),
-                    bounds.height());
+                    size().width(),
+                    size().height());
   bitmap->allocPixels();
   SkAutoLockPixels lock(*bitmap);
   unsigned char* pixels = static_cast<unsigned char*>(bitmap->getPixels());
@@ -578,18 +575,16 @@ bool CompositorGL::ReadPixels(SkBitmap* bitmap, const gfx::Rect& bounds) {
   GLint current_alignment = 0;
   glGetIntegerv(GL_PACK_ALIGNMENT, &current_alignment);
   glPixelStorei(GL_PACK_ALIGNMENT, 4);
-
-  // Flip vertically to convert to OpenGL coordinates.
-  glReadPixels(bounds.x(),
-               size().height() - bounds.y() - bounds.height(),
-               bounds.width(),
-               bounds.height(),
+  glReadPixels(0,
+               0,
+               size().width(),
+               size().height(),
                GL_RGBA,
                GL_UNSIGNED_BYTE,
                pixels);
   glPixelStorei(GL_PACK_ALIGNMENT, current_alignment);
 
-  SwizzleRGBAToBGRAAndFlip(pixels, bounds.size());
+  SwizzleRGBAToBGRAAndFlip(pixels, size());
   return true;
 }
 
diff --git a/ui/gfx/compositor/compositor_gl.h b/ui/gfx/compositor/compositor_gl.h
index 209ec70..2b23f67 100644
--- a/ui/gfx/compositor/compositor_gl.h
+++ b/ui/gfx/compositor/compositor_gl.h
@@ -102,7 +102,7 @@ class COMPOSITOR_EXPORT CompositorGL : public Compositor {
   virtual ~CompositorGL();
 
   // Overridden from Compositor.
-  virtual bool ReadPixels(SkBitmap* bitmap, const gfx::Rect& bounds) OVERRIDE;
+  virtual bool ReadPixels(SkBitmap* bitmap) OVERRIDE;
 
   void MakeCurrent();
   gfx::Size GetSize();
diff --git a/ui/gfx/compositor/layer_unittest.cc b/ui/gfx/compositor/layer_unittest.cc
index c8a609f..703f9ca 100644
--- a/ui/gfx/compositor/layer_unittest.cc
+++ b/ui/gfx/compositor/layer_unittest.cc
@@ -170,11 +170,6 @@ class LayerWithRealCompositorTest : public testing::Test {
     GetCompositor()->Draw(false);
   }
 
-  bool ReadPixels(SkBitmap* bitmap) {
-    return GetCompositor()->ReadPixels(bitmap,
-                                       gfx::Rect(GetCompositor()->size()));
-  }
-
   void RunPendingMessages() {
     MessageLoopForUI::current()->RunAllPending();
   }
@@ -1093,24 +1088,16 @@ TEST_F(LayerWithNullDelegateTest, SetBoundsSchedulesPaint) {
 TEST_F(LayerWithRealCompositorTest, MAYBE_DrawPixels) {
   scoped_ptr<Layer> layer(CreateColorLayer(SK_ColorRED,
                                            gfx::Rect(0, 0, 500, 500)));
-  scoped_ptr<Layer> layer2(CreateColorLayer(SK_ColorBLUE,
-                                            gfx::Rect(0, 0, 500, 10)));
-
-  layer->Add(layer2.get());
-
   DrawTree(layer.get());
 
   SkBitmap bitmap;
-  gfx::Size size = GetCompositor()->size();
-  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap,
-                                          gfx::Rect(0, 10,
-                                          size.width(), size.height() - 10)));
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
   ASSERT_FALSE(bitmap.empty());
 
   SkAutoLockPixels lock(bitmap);
   bool is_all_red = true;
   for (int x = 0; is_all_red && x < 500; x++)
-    for (int y = 0; is_all_red && y < 490; y++)
+    for (int y = 0; is_all_red && y < 500; y++)
       is_all_red = is_all_red && (bitmap.getColor(x, y) == SK_ColorRED);
 
   EXPECT_TRUE(is_all_red);
@@ -1236,14 +1223,14 @@ TEST_F(LayerWithRealCompositorTest, MAYBE_ModifyHierarchy) {
   l11->Add(l21.get());
   l0->Add(l12.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
   ASSERT_FALSE(bitmap.empty());
   // WritePNGFile(bitmap, ref_img1);
   EXPECT_TRUE(IsSameAsPNGFile(bitmap, ref_img1));
 
   l0->MoveToFront(l11.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
   ASSERT_FALSE(bitmap.empty());
   // WritePNGFile(bitmap, ref_img2);
   EXPECT_TRUE(IsSameAsPNGFile(bitmap, ref_img2));
@@ -1251,21 +1238,21 @@ TEST_F(LayerWithRealCompositorTest, MAYBE_ModifyHierarchy) {
   // l11 is already at the front, should have no effect.
   l0->MoveToFront(l11.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
   ASSERT_FALSE(bitmap.empty());
   EXPECT_TRUE(IsSameAsPNGFile(bitmap, ref_img2));
 
   // l11 is already at the front, should have no effect.
   l0->MoveAbove(l11.get(), l12.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
   ASSERT_FALSE(bitmap.empty());
   EXPECT_TRUE(IsSameAsPNGFile(bitmap, ref_img2));
 
   // should restore to original configuration
   l0->MoveAbove(l12.get(), l11.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
   ASSERT_FALSE(bitmap.empty());
   EXPECT_TRUE(IsSameAsPNGFile(bitmap, ref_img1));
 }
@@ -1288,7 +1275,7 @@ TEST_F(LayerWithRealCompositorTest, MAYBE_Opacity) {
   l0->Add(l11.get());
   DrawTree(l0.get());
   SkBitmap bitmap;
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ASSERT_TRUE(GetCompositor()->ReadPixels(&bitmap));
   ASSERT_FALSE(bitmap.empty());
   // WritePNGFile(bitmap, ref_img);
   EXPECT_TRUE(IsSameAsPNGFile(bitmap, ref_img));
diff --git a/ui/gfx/compositor/test/test_compositor.cc b/ui/gfx/compositor/test/test_compositor.cc
index f3eb7ec..b0b7d33 100644
--- a/ui/gfx/compositor/test/test_compositor.cc
+++ b/ui/gfx/compositor/test/test_compositor.cc
@@ -48,7 +48,7 @@ void TestCompositor::DrawTree() {
 #endif
 }
 
-bool TestCompositor::ReadPixels(SkBitmap* bitmap, const gfx::Rect& bounds) {
+bool TestCompositor::ReadPixels(SkBitmap* bitmap) {
   return false;
 }
 
diff --git a/ui/gfx/compositor/test/test_compositor.h b/ui/gfx/compositor/test/test_compositor.h
index 8cfc251..568c159 100644
--- a/ui/gfx/compositor/test/test_compositor.h
+++ b/ui/gfx/compositor/test/test_compositor.h
@@ -27,7 +27,7 @@ class TestCompositor : public ui::Compositor {
   virtual void OnNotifyEnd() OVERRIDE;
   virtual void Blur(const gfx::Rect& bounds) OVERRIDE;
   virtual void DrawTree() OVERRIDE;
-  virtual bool ReadPixels(SkBitmap* bitmap, const gfx::Rect& bounds) OVERRIDE;
+  virtual bool ReadPixels(SkBitmap* bitmap) OVERRIDE;
 
   // A simple factory that creates a test compositor with a given delegate
   static ui::Compositor* Create(ui::CompositorDelegate* owner);