Removes the rotation and scaling in SnapshotAura.

It seems that the compositor already takes care of the screen
rotation and scaling. SnapshotAura doesn't need to do that.

Also, snapshot_aura_unittest.cc is not good for rotation and
scaling because the existing test just paint plain red. This
CL changes the paint content to fail if the rotation is not
handled correctly.

BUG=337559
R=piman@chromium.org, enne@chromium.org, sky@chromium.org
TEST=manually && snapshot_unittests

Review URL: https://codereview.chromium.org/137143005

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

2 files changed, 43 insertions, 117 deletions

diff --git a/ui/snapshot/snapshot_aura.cc b/ui/snapshot/snapshot_aura.cc
index b926818..b40ea47 100644
--- a/ui/snapshot/snapshot_aura.cc
+++ b/ui/snapshot/snapshot_aura.cc
@@ -40,45 +40,17 @@ void OnFrameScalingFinished(
   callback.Run(gfx::Image(gfx::ImageSkia::CreateFrom1xBitmap(scaled_bitmap)));
 }
 
-void RotateBitmap(SkBitmap* bitmap, gfx::Display::Rotation rotation) {
-  switch (rotation) {
-    case gfx::Display::ROTATE_0:
-      break;
-    case gfx::Display::ROTATE_90:
-      *bitmap = SkBitmapOperations::Rotate(*bitmap,
-                                           SkBitmapOperations::ROTATION_270_CW);
-      break;
-    case gfx::Display::ROTATE_180:
-      *bitmap = SkBitmapOperations::Rotate(*bitmap,
-                                           SkBitmapOperations::ROTATION_180_CW);
-      break;
-    case gfx::Display::ROTATE_270:
-      *bitmap = SkBitmapOperations::Rotate(*bitmap,
-                                           SkBitmapOperations::ROTATION_90_CW);
-      break;
-  }
-}
-
-SkBitmap ScaleAndRotateBitmap(const SkBitmap& input_bitmap,
-                              gfx::Size target_size_pre_rotation,
-                              gfx::Display::Rotation rotation) {
-  SkBitmap bitmap;
-  bitmap =
-      skia::ImageOperations::Resize(input_bitmap,
-                                    skia::ImageOperations::RESIZE_GOOD,
-                                    target_size_pre_rotation.width(),
-                                    target_size_pre_rotation.height(),
-                                    static_cast<SkBitmap::Allocator*>(NULL));
-  RotateBitmap(&bitmap, rotation);
-  return bitmap;
+SkBitmap ScaleBitmap(const SkBitmap& input_bitmap,
+                     const gfx::Size& target_size) {
+  return skia::ImageOperations::Resize(
+      input_bitmap,
+      skia::ImageOperations::RESIZE_GOOD,
+      target_size.width(),
+      target_size.height(),
+      static_cast<SkBitmap::Allocator*>(NULL));
 }
 
-scoped_refptr<base::RefCountedBytes> ScaleRotateAndEncodeBitmap(
-    const SkBitmap& input_bitmap,
-    gfx::Size target_size_pre_rotation,
-    gfx::Display::Rotation rotation) {
-  SkBitmap bitmap =
-      ScaleAndRotateBitmap(input_bitmap, target_size_pre_rotation, rotation);
+scoped_refptr<base::RefCountedBytes> EncodeBitmap(const SkBitmap& bitmap) {
   scoped_refptr<base::RefCountedBytes> png_data(new base::RefCountedBytes);
   unsigned char* pixels =
       reinterpret_cast<unsigned char*>(bitmap.pixelRef()->pixels());
@@ -94,10 +66,9 @@ scoped_refptr<base::RefCountedBytes> ScaleRotateAndEncodeBitmap(
   return png_data;
 }
 
-void ScaleAndRotateCopyOutputResult(
+void ScaleCopyOutputResult(
     const GrabWindowSnapshotAsyncCallback& callback,
     const gfx::Size& target_size,
-    gfx::Display::Rotation rotation,
     scoped_refptr<base::TaskRunner> background_task_runner,
     scoped_ptr<cc::CopyOutputResult> result) {
   if (result->IsEmpty()) {
@@ -112,15 +83,12 @@ void ScaleAndRotateCopyOutputResult(
   base::PostTaskAndReplyWithResult(
       background_task_runner,
       FROM_HERE,
-      base::Bind(
-          ScaleAndRotateBitmap, *result->TakeBitmap(), target_size, rotation),
+      base::Bind(ScaleBitmap, *result->TakeBitmap(), target_size),
       base::Bind(&OnFrameScalingFinished, callback));
 }
 
-void ScaleRotateAndEncodeCopyOutputResult(
+void EncodeCopyOutputResult(
     const GrabWindowSnapshotAsyncPNGCallback& callback,
-    const gfx::Size& target_size,
-    gfx::Display::Rotation rotation,
     scoped_refptr<base::TaskRunner> background_task_runner,
     scoped_ptr<cc::CopyOutputResult> result) {
   if (result->IsEmpty()) {
@@ -134,38 +102,11 @@ void ScaleRotateAndEncodeCopyOutputResult(
   // somewhere so that it can be reused here.
   base::PostTaskAndReplyWithResult(background_task_runner,
                                    FROM_HERE,
-                                   base::Bind(ScaleRotateAndEncodeBitmap,
-                                              *result->TakeBitmap(),
-                                              target_size,
-                                              rotation),
+                                   base::Bind(EncodeBitmap,
+                                              *result->TakeBitmap()),
                                    callback);
 }
 
-gfx::Rect GetTargetBoundsFromWindow(gfx::NativeWindow window,
-                                    gfx::Rect snapshot_bounds) {
-  gfx::RectF read_pixels_bounds = snapshot_bounds;
-
-  // We must take into account the window's position on the desktop.
-  read_pixels_bounds.Offset(
-      window->GetBoundsInRootWindow().origin().OffsetFromOrigin());
-  aura::WindowEventDispatcher* dispatcher = window->GetDispatcher();
-  if (dispatcher)
-    dispatcher->host()->GetRootTransform().TransformRect(&read_pixels_bounds);
-
-  gfx::Rect read_pixels_bounds_in_pixel =
-      gfx::ToEnclosingRect(read_pixels_bounds);
-
-  // Sometimes (i.e. when using Aero on Windows) the compositor's size is
-  // smaller than the window bounds. So trim appropriately.
-  ui::Compositor* compositor = window->layer()->GetCompositor();
-  read_pixels_bounds_in_pixel.Intersect(gfx::Rect(compositor->size()));
-
-  DCHECK_LE(0, read_pixels_bounds.x());
-  DCHECK_LE(0, read_pixels_bounds.y());
-
-  return read_pixels_bounds_in_pixel;
-}
-
 }  // namespace
 
 bool GrabViewSnapshot(gfx::NativeView view,
@@ -197,31 +138,11 @@ void GrabWindowSnapshotAndScaleAsync(
     const gfx::Size& target_size,
     scoped_refptr<base::TaskRunner> background_task_runner,
     const GrabWindowSnapshotAsyncCallback& callback) {
-  // target_size is post-rotation, and so logically this is a rotate and then
-  // scale operation.  However, it will usually be more efficient to scale first
-  // (given that this is mostly used for thumbnails) and then rotate.
-  gfx::Display::Rotation rotation = gfx::Screen::GetScreenFor(window)
-                                        ->GetDisplayNearestWindow(window)
-                                        .rotation();
-  gfx::Size rotated_target_size;
-  switch (rotation) {
-    case gfx::Display::ROTATE_0:
-    case gfx::Display::ROTATE_180:
-      rotated_target_size = target_size;
-      break;
-    case gfx::Display::ROTATE_90:
-    case gfx::Display::ROTATE_270:
-      rotated_target_size =
-          gfx::Size(target_size.height(), target_size.width());
-      break;
-  };
-
   MakeAsyncCopyRequest(window,
                        source_rect,
-                       base::Bind(&ScaleAndRotateCopyOutputResult,
+                       base::Bind(&ScaleCopyOutputResult,
                                   callback,
-                                  rotated_target_size,
-                                  rotation,
+                                  target_size,
                                   background_task_runner));
 }
 
@@ -230,16 +151,10 @@ void GrabWindowSnapshotAsync(
     const gfx::Rect& source_rect,
     scoped_refptr<base::TaskRunner> background_task_runner,
     const GrabWindowSnapshotAsyncPNGCallback& callback) {
-  gfx::Size target_size = GetTargetBoundsFromWindow(window, source_rect).size();
-  gfx::Display::Rotation rotation = gfx::Screen::GetScreenFor(window)
-                                        ->GetDisplayNearestWindow(window)
-                                        .rotation();
   MakeAsyncCopyRequest(window,
                        source_rect,
-                       base::Bind(&ScaleRotateAndEncodeCopyOutputResult,
+                       base::Bind(&EncodeCopyOutputResult,
                                   callback,
-                                  target_size,
-                                  rotation,
                                   background_task_runner));
 }
 
diff --git a/ui/snapshot/snapshot_aura_unittest.cc b/ui/snapshot/snapshot_aura_unittest.cc
index eab87a4..6914e7a 100644
--- a/ui/snapshot/snapshot_aura_unittest.cc
+++ b/ui/snapshot/snapshot_aura_unittest.cc
@@ -24,7 +24,10 @@
 
 namespace ui {
 namespace {
-const SkColor kPaintColor = SK_ColorRED;
+
+SkColor GetExpectedColorForPoint(int x, int y) {
+  return SkColorSetRGB(std::min(x, 255), std::min(y, 255), 0);
+}
 
 // Paint simple rectangle on the specified aura window.
 class TestPaintingWindowDelegate : public aura::test::TestWindowDelegate {
@@ -37,7 +40,10 @@ class TestPaintingWindowDelegate : public aura::test::TestWindowDelegate {
   }
 
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE {
-    canvas->FillRect(gfx::Rect(window_size_), kPaintColor);
+    for (int y = 0; y < window_size_.height(); ++y) {
+      for (int x = 0; x < window_size_.width(); ++x)
+        canvas->FillRect(gfx::Rect(x, y, 1, 1), GetExpectedColorForPoint(x, y));
+    }
   }
 
  private:
@@ -46,18 +52,27 @@ class TestPaintingWindowDelegate : public aura::test::TestWindowDelegate {
   DISALLOW_COPY_AND_ASSIGN(TestPaintingWindowDelegate);
 };
 
-size_t GetFailedPixelsCount(const gfx::Image& image) {
+size_t GetFailedPixelsCountWithScaleFactor(const gfx::Image& image,
+                                           int scale_factor) {
   const SkBitmap* bitmap = image.ToSkBitmap();
   uint32* bitmap_data = reinterpret_cast<uint32*>(
       bitmap->pixelRef()->pixels());
   size_t result = 0;
-  for (int i = 0; i < bitmap->width() * bitmap->height(); ++i) {
-    if (static_cast<SkColor>(bitmap_data[i]) != kPaintColor)
-      ++result;
+  for (int y = 0; y < bitmap->height(); y += scale_factor) {
+    for (int x = 0; x < bitmap->width(); x += scale_factor) {
+      if (static_cast<SkColor>(bitmap_data[x + y * bitmap->width()]) !=
+          GetExpectedColorForPoint(x / scale_factor, y / scale_factor)) {
+        ++result;
+      }
+    }
   }
   return result;
 }
 
+size_t GetFailedPixelsCount(const gfx::Image& image) {
+  return GetFailedPixelsCountWithScaleFactor(image, 1);
+}
+
 }  // namespace
 
 class SnapshotAuraTest : public testing::Test {
@@ -175,8 +190,7 @@ TEST_F(SnapshotAuraTest, PartialBounds) {
   WaitForDraw();
 
   gfx::Image snapshot = GrabSnapshotForTestWindow();
-  EXPECT_EQ(test_bounds.size().ToString(),
-            snapshot.Size().ToString());
+  EXPECT_EQ(test_bounds.size().ToString(), snapshot.Size().ToString());
   EXPECT_EQ(0u, GetFailedPixelsCount(snapshot));
 }
 
@@ -188,8 +202,7 @@ TEST_F(SnapshotAuraTest, Rotated) {
   WaitForDraw();
 
   gfx::Image snapshot = GrabSnapshotForTestWindow();
-  EXPECT_EQ(test_bounds.size().ToString(),
-            snapshot.Size().ToString());
+  EXPECT_EQ(test_bounds.size().ToString(), snapshot.Size().ToString());
   EXPECT_EQ(0u, GetFailedPixelsCount(snapshot));
 }
 
@@ -203,7 +216,6 @@ TEST_F(SnapshotAuraTest, UIScale) {
 
   // Snapshot always captures the physical pixels.
   gfx::SizeF snapshot_size(test_bounds.size());
-  snapshot_size.Scale(1.0f / kUIScale);
 
   gfx::Image snapshot = GrabSnapshotForTestWindow();
   EXPECT_EQ(gfx::ToRoundedSize(snapshot_size).ToString(),
@@ -225,7 +237,7 @@ TEST_F(SnapshotAuraTest, DeviceScaleFactor) {
   gfx::Image snapshot = GrabSnapshotForTestWindow();
   EXPECT_EQ(gfx::ToRoundedSize(snapshot_size).ToString(),
             snapshot.Size().ToString());
-  EXPECT_EQ(0u, GetFailedPixelsCount(snapshot));
+  EXPECT_EQ(0u, GetFailedPixelsCountWithScaleFactor(snapshot, 2));
 }
 
 TEST_F(SnapshotAuraTest, RotateAndUIScale) {
@@ -239,7 +251,6 @@ TEST_F(SnapshotAuraTest, RotateAndUIScale) {
 
   // Snapshot always captures the physical pixels.
   gfx::SizeF snapshot_size(test_bounds.size());
-  snapshot_size.Scale(1.0f / kUIScale);
 
   gfx::Image snapshot = GrabSnapshotForTestWindow();
   EXPECT_EQ(gfx::ToRoundedSize(snapshot_size).ToString(),
@@ -259,12 +270,12 @@ TEST_F(SnapshotAuraTest, RotateAndUIScaleAndScaleFactor) {
 
   // Snapshot always captures the physical pixels.
   gfx::SizeF snapshot_size(test_bounds.size());
-  snapshot_size.Scale(2.0f / kUIScale);
+  snapshot_size.Scale(2.0f);
 
   gfx::Image snapshot = GrabSnapshotForTestWindow();
   EXPECT_EQ(gfx::ToRoundedSize(snapshot_size).ToString(),
             snapshot.Size().ToString());
-  EXPECT_EQ(0u, GetFailedPixelsCount(snapshot));
+  EXPECT_EQ(0u, GetFailedPixelsCountWithScaleFactor(snapshot, 2));
 }
 
 }  // namespace ui