Clip the bitmap for creating the thumbnail on GPU when accelerated compositing is active.

chrome/browser/tab_contents/thumbnail_generator.cc

Index: chrome/browser/tab_contents/thumbnail_generator.cc
diff --git a/chrome/browser/tab_contents/thumbnail_generator.cc b/chrome/browser/tab_contents/thumbnail_generator.cc
index f5cfb5790b4a28ef32796fc61f404515a21408da..a18443a6b5aed140df4fa4cef6d27ead42d7a867 100644
--- a/chrome/browser/tab_contents/thumbnail_generator.cc
+++ b/chrome/browser/tab_contents/thumbnail_generator.cc
@@ -65,6 +65,7 @@ namespace {
 
 static const int kThumbnailWidth = 212;
 static const int kThumbnailHeight = 132;
+static const int kScrollbarThickness = 15;

[brettw, 2012/08/05 05:38:39]

I wonder, can we get the actual value from the theming code?

[mazda, 2012/08/06 17:22:16]

I found a function to get scrollbar size in ui/gfx/scrollbar_size.h and replaced
this constant with that function.
Thanks.

 
 // This factor determines the number of pixels to be copied by
 // RenderWidgetHost::CopyFromBackingStore for generating thumbnail.
@@ -91,26 +92,76 @@ gfx::Size GetCopySizeForThumbnail(const gfx::Size& view_size,
                    static_cast<int>(scale * view_size.height()));
 }
 
+// Returns the clipping rectangle that is used for creating a thumbnail with
+// the size of |desired_size| from the bitmap with the size of |source_size|.
+// The type of clipping that needs to be done is assigned to |clip_result|.
+gfx::Rect GetClippingRect(const gfx::Size& source_size,
+                          const gfx::Size& desired_size,
+                          ThumbnailGenerator::ClipResult* clip_result) {
+  DCHECK(clip_result);
+
+  const float desired_aspect =

[brettw, 2012/08/05 05:38:39]

Optional: IMO const in this case just adds noise (same with src_rect below).

Since you just moved this, I didn't re-review this logic.

[mazda, 2012/08/06 17:22:16]

Removed const.

+      static_cast<float>(desired_size.width()) / desired_size.height();
+
+  // Get the clipping rect so that we can preserve the aspect ratio while
+  // filling the destination.
+  gfx::Rect clipping_rect;
+  if (source_size.width() < desired_size.width() ||
+      source_size.height() < desired_size.height()) {
+    // Source image is smaller: we clip the part of source image within the
+    // dest rect, and then stretch it to fill the dest rect. We don't respect
+    // the aspect ratio in this case.
+    clipping_rect = gfx::Rect(desired_size);
+    *clip_result = ThumbnailGenerator::kSourceIsSmaller;
+  } else {
+    const float src_aspect =
+        static_cast<float>(source_size.width()) / source_size.height();
+    if (src_aspect > desired_aspect) {
+      // Wider than tall, clip horizontally: we center the smaller
+      // thumbnail in the wider screen.
+      int new_width = static_cast<int>(source_size.height() * desired_aspect);
+      int x_offset = (source_size.width() - new_width) / 2;
+      clipping_rect.SetRect(x_offset, 0, new_width, source_size.height());
+      *clip_result = (src_aspect >= ThumbnailScore::kTooWideAspectRatio) ?
+          ThumbnailGenerator::kTooWiderThanTall :
+          ThumbnailGenerator::kWiderThanTall;
+    } else if (src_aspect < desired_aspect) {
+      clipping_rect =
+          gfx::Rect(source_size.width(), source_size.width() / desired_aspect);
+      *clip_result = ThumbnailGenerator::kTallerThanWide;
+    } else {
+      clipping_rect = gfx::Rect(source_size);
+      *clip_result = ThumbnailGenerator::kNotClipped;
+    }
+  }
+  return clipping_rect;
+}
+
 // Creates a downsampled thumbnail from the given bitmap.
 // store. The returned bitmap will be isNull if there was an error creating it.
 SkBitmap CreateThumbnail(
-    const SkBitmap& bmp_with_scrollbars,
+    const SkBitmap& bitmap,
     int desired_width,
     int desired_height,
     ThumbnailGenerator::ClipResult* clip_result) {
   base::TimeTicks begin_compute_thumbnail = base::TimeTicks::Now();
 
-  // Clip the edgemost 15 pixels as that will commonly hold a scrollbar, which
-  // looks bad in thumbnails.
-  SkIRect scrollbarless_rect =
-      { 0, 0,
-        std::max(1, bmp_with_scrollbars.width() - 15),
-        std::max(1, bmp_with_scrollbars.height() - 15) };
-  SkBitmap bmp;
-  bmp_with_scrollbars.extractSubset(&bmp, scrollbarless_rect);
-
-  SkBitmap clipped_bitmap = ThumbnailGenerator::GetClippedBitmap(
-      bmp, desired_width, desired_height, clip_result);
+  SkBitmap clipped_bitmap;
+  if (*clip_result == ThumbnailGenerator::kUnprocessed) {
+    // Clip the edgemost 15 pixels as that will commonly hold a scrollbar, which

[brettw, 2012/08/05 05:38:39]

I'd remove the 15 pixel comment here since the actual value is defined
elsewhere.

[mazda, 2012/08/06 17:22:16]

Removed the magic number (15) from the comment.

+    // looks bad in thumbnails.
+    SkIRect scrollbarless_rect =
+        { 0, 0,
+          std::max(1, bitmap.width() - kScrollbarThickness),
+          std::max(1, bitmap.height() - kScrollbarThickness) };
+    SkBitmap bmp;
+    bitmap.extractSubset(&bmp, scrollbarless_rect);
+
+    clipped_bitmap = ThumbnailGenerator::GetClippedBitmap(
+        bmp, desired_width, desired_height, clip_result);
+  } else {
+    clipped_bitmap = bitmap;
+  }
 
   // Need to resize it to the size we want, so downsample until it's
   // close, and let the caller make it the exact size if desired.
@@ -376,49 +427,12 @@ SkBitmap ThumbnailGenerator::GetClippedBitmap(const SkBitmap& bitmap,
                                               int desired_width,
                                               int desired_height,
                                               ClipResult* clip_result) {
-  const SkRect dest_rect = { 0, 0,
-                             SkIntToScalar(desired_width),
-                             SkIntToScalar(desired_height) };
-  const float dest_aspect = dest_rect.width() / dest_rect.height();
-
-  // Get the src rect so that we can preserve the aspect ratio while filling
-  // the destination.
-  SkIRect src_rect;
-  if (bitmap.width() < dest_rect.width() ||
-      bitmap.height() < dest_rect.height()) {
-    // Source image is smaller: we clip the part of source image within the
-    // dest rect, and then stretch it to fill the dest rect. We don't respect
-    // the aspect ratio in this case.
-    src_rect.set(0, 0, static_cast<S16CPU>(dest_rect.width()),
-                 static_cast<S16CPU>(dest_rect.height()));
-    if (clip_result)
-      *clip_result = ThumbnailGenerator::kSourceIsSmaller;
-  } else {
-    const float src_aspect =
-        static_cast<float>(bitmap.width()) / bitmap.height();
-    if (src_aspect > dest_aspect) {
-      // Wider than tall, clip horizontally: we center the smaller
-      // thumbnail in the wider screen.
-      S16CPU new_width = static_cast<S16CPU>(bitmap.height() * dest_aspect);
-      S16CPU x_offset = (bitmap.width() - new_width) / 2;
-      src_rect.set(x_offset, 0, new_width + x_offset, bitmap.height());
-      if (clip_result) {
-        *clip_result = (src_aspect >= ThumbnailScore::kTooWideAspectRatio) ?
-            ThumbnailGenerator::kTooWiderThanTall :
-            ThumbnailGenerator::kWiderThanTall;
-      }
-    } else if (src_aspect < dest_aspect) {
-      src_rect.set(0, 0, bitmap.width(),
-                   static_cast<S16CPU>(bitmap.width() / dest_aspect));
-      if (clip_result)
-        *clip_result = ThumbnailGenerator::kTallerThanWide;
-    } else {
-      src_rect.set(0, 0, bitmap.width(), bitmap.height());
-      if (clip_result)
-        *clip_result = ThumbnailGenerator::kNotClipped;
-    }
-  }
-
+  gfx::Rect clipping_rect =
+      GetClippingRect(gfx::Size(bitmap.width(), bitmap.height()),
+                      gfx::Size(desired_width, desired_height),
+                      clip_result);
+  SkIRect src_rect = { clipping_rect.x(), clipping_rect.y(),
+                       clipping_rect.right(), clipping_rect.bottom() };
   SkBitmap clipped_bitmap;
   bitmap.extractSubset(&clipped_bitmap, src_rect);
   return clipped_bitmap;
@@ -443,7 +457,7 @@ void ThumbnailGenerator::UpdateThumbnailIfNecessary(
 
 void ThumbnailGenerator::UpdateThumbnail(
     WebContents* web_contents, const SkBitmap& thumbnail,
-    const ThumbnailGenerator::ClipResult& clip_result) {
+    const ClipResult& clip_result) {
 
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
@@ -486,7 +500,8 @@ void ThumbnailGenerator::AsyncUpdateThumbnail(
       AskForSnapshot(render_widget_host,
                      base::Bind(&ThumbnailGenerator::UpdateThumbnailWithBitmap,
                                 weak_factory_.GetWeakPtr(),
-                                web_contents),
+                                web_contents,
+                                ThumbnailGenerator::kUnprocessed),
                      view_size,
                      view_size);
     }
@@ -494,28 +509,36 @@ void ThumbnailGenerator::AsyncUpdateThumbnail(
     return;
   }
 
+  gfx::Rect copy_rect = gfx::Rect(view->GetViewBounds().size());
+  // Clip the edgemost 15 pixels as that will commonly hold a scrollbar, which

[brettw, 2012/08/05 05:38:39]

Ditto w/ "15 pixels."

[mazda, 2012/08/06 17:22:16]

Done.

+  // looks bad in thumbnails.
+  copy_rect.Inset(0, 0, kScrollbarThickness, kScrollbarThickness);
+  ClipResult clip_result = ThumbnailGenerator::kUnprocessed;
+  copy_rect = GetClippingRect(copy_rect.size(),
+                              gfx::Size(kThumbnailWidth, kThumbnailHeight),
+                              &clip_result);
   gfx::Size copy_size =
-      GetCopySizeForThumbnail(view->GetViewBounds().size(),
-                              gfx::Size(kThumbnailWidth, kThumbnailHeight));
+      gfx::Size(kThumbnailWidth, kThumbnailHeight).Scale(kThumbnailCopyScale);
   skia::PlatformCanvas* temp_canvas = new skia::PlatformCanvas;
   render_widget_host->CopyFromBackingStore(
-      gfx::Rect(),
+      copy_rect,
       copy_size,
       base::Bind(&ThumbnailGenerator::UpdateThumbnailWithCanvas,
                  weak_factory_.GetWeakPtr(),
                  web_contents,
+                 clip_result,
                  base::Owned(temp_canvas)),
       temp_canvas);
 }
 
 void ThumbnailGenerator::UpdateThumbnailWithBitmap(
     WebContents* web_contents,
+    ClipResult clip_result,
     const SkBitmap& bitmap) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   if (bitmap.isNull() || bitmap.empty())
     return;
 
-  ClipResult clip_result;
   SkBitmap thumbnail = CreateThumbnail(bitmap,
                                        kThumbnailWidth,
                                        kThumbnailHeight,
@@ -525,15 +548,15 @@ void ThumbnailGenerator::UpdateThumbnailWithBitmap(
 
 void ThumbnailGenerator::UpdateThumbnailWithCanvas(
     WebContents* web_contents,
+    ClipResult clip_result,
     skia::PlatformCanvas* temp_canvas,
     bool succeeded) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   if (!succeeded)
     return;
 
-  SkBitmap bmp_with_scrollbars =
-      skia::GetTopDevice(*temp_canvas)->accessBitmap(false);
-  UpdateThumbnailWithBitmap(web_contents, bmp_with_scrollbars);
+  SkBitmap bitmap = skia::GetTopDevice(*temp_canvas)->accessBitmap(false);
+  UpdateThumbnailWithBitmap(web_contents, clip_result, bitmap);
 }
 
 bool ThumbnailGenerator::ShouldUpdateThumbnail(Profile* profile,