Make ThumbnailGenerator support high DPI.

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 a338c5e089ea42cb00599286412440a390c7f43e..fd76f9a1753299e7f949013334cb6b56cc7359df 100644
--- a/chrome/browser/tab_contents/thumbnail_generator.cc
+++ b/chrome/browser/tab_contents/thumbnail_generator.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/tab_contents/thumbnail_generator.h"
 
 #include <algorithm>
+#include <cmath>
 #include <map>
 
 #include "base/bind.h"
@@ -31,11 +32,13 @@
 #include "ui/base/layout.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/rect.h"
+#include "ui/gfx/screen.h"
 #include "ui/gfx/scrollbar_size.h"
 #include "ui/gfx/skbitmap_operations.h"
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
+#include "ui/base/win/dpi.h"
 #endif
 
 // Overview
@@ -65,32 +68,89 @@ using content::WebContents;
 
 namespace {
 
+// The thumbnail size in DIP.
 static const int kThumbnailWidth = 212;
 static const int kThumbnailHeight = 132;
 
-// This factor determines the number of pixels to be copied by
-// RenderWidgetHost::CopyFromBackingStore for generating thumbnail.
-// Smaller scale is good for performance, but too small scale causes aliasing
-// because the resampling method is not good enough to retain the image quality.
-// TODO(mazda): the Improve resampling method and use a smaller scale
-// (http://crbug.com/118571).
-static const double kThumbnailCopyScale = 2.0;
-
 static const char kThumbnailHistogramName[] = "Thumbnail.ComputeMS";
 
-// Calculates the size used by RenderWidgetHost::CopyFromBackingStore.
-// The result is computed as the minimum size that satisfies the following
-// conditions.
-//   result.width : result.height == view_size.width : view_size.height
-//   result.width >= kThumbnailCopyScale * desired_size.width
-//   result.height >= kThumbnailCopyScale * desired_size.height
-gfx::Size GetCopySizeForThumbnail(const gfx::Size& view_size,
-                                  const gfx::Size& desired_size) {
-  const double scale = kThumbnailCopyScale *
-      std::max(static_cast<double>(desired_size.width()) / view_size.width(),
-               static_cast<double>(desired_size.height()) / view_size.height());
-  return gfx::Size(static_cast<int>(scale * view_size.width()),
-                   static_cast<int>(scale * view_size.height()));
+// Returns the size used by RenderWidgetHost::CopyFromBackingStore.
+//
+// The size is calculated in such a way that the copied size in pixel becomes
+// equal to (f * kThumbnailWidth, f * kThumbnailHeight), where f is the scale
+// of ui::SCALE_FACTOR_200P. Since RenderWidgetHost::CopyFromBackingStore takes
+// the size in DIP, we need to adjust the size based on |view|'s device scale
+// factor in order to copy the pixels with the size above.

[oshima, 2012/08/10 02:01:15]

I'm a bit confused by this comment. It thought the size in pixel is always 2 x
kThumbnailWidth/Size in pixels, and that's what the following code is doing?

[mazda, 2012/08/17 06:10:51]

Yes, that's correct.
RenderWidgetHost::CopyFromBackingStore takes the size argument in DIP, but here
I'd like to ensure the data is copied in constant size in pixels (2 x
kThumbnailWidth/Size) for the reasons written below in the comment.

+//
+// The copied size was chosen for the following reasons.
+//
+// 1. When the scale factor of the primary monitor is ui::SCALE_FACTOR_200P, the
+// generated thumbnail size is (f * kThumbnailWidth, f * kThumbnailHeight).
+// In order to avoid degrading the image quality by magnification, the size
+// of the copied pixels should be equal to or larger than this thumbnail size.
+//
+// 2. RenderWidgetHost::CopyFromBackingStore can be costly especially when
+// it is necessary to read back the web contents image data from GPU. As the
+// cost is roughly propotional to the number of the copied pixels, the size of
+// the copied pixels should be as small as possible.
+//
+// When the scale factor of the primary monitor is ui::SCALE_FACTOR_100P,
+// we still copy the pixels with the same size as ui::SCALE_FACTOR_200P because
+// the resampling method used in RenderWidgetHost::CopyFromBackingStore is not
+// good enough for the resampled image to be used directly for the thumbnail
+// (http://crbug.com/141235). We assume this is not an issue in case of
+// ui::SCALE_FACTOR_200P because the high resolution thumbnail on high density
+// display alleviates the aliasing.
+gfx::Size GetCopySizeForThumbnail(content::RenderWidgetHostView* view) {
+  gfx::Size copy_size(kThumbnailWidth, kThumbnailHeight);
+  ui::ScaleFactor scale_factor =
+      ui::GetScaleFactorForNativeView(view->GetNativeView());
+  switch (scale_factor) {
+    case ui::SCALE_FACTOR_100P:
+      copy_size =
+          copy_size.Scale(ui::GetScaleFactorScale(ui::SCALE_FACTOR_200P));
+      break;
+    case ui::SCALE_FACTOR_200P:
+      // Use the size as-is.
+      break;
+    default:
+      LOG(WARNING) << "Unsupported scale factor. Use the same copy size as "
+                   << "ui::SCALE_FACTOR_100P";
+      copy_size =
+          copy_size.Scale(ui::GetScaleFactorScale(ui::SCALE_FACTOR_200P));
+      break;
+  }
+  return copy_size;
+}
+
+// Returns the size of the thumbnail stored in the database in pixel.
+gfx::Size GetThumbnailSizeInPixel() {
+  gfx::Size thumbnail_size(kThumbnailWidth, kThumbnailHeight);
+  // Determine the resolution of the thumbnail based on the primary monitor.
+  gfx::Display primary_display = gfx::Screen::GetPrimaryDisplay();

[oshima, 2012/08/09 02:11:02]

Can you add comment

TODO(oshima): Use device's default scale factor.

This may not give the right scale factor. For example,
you can mirror the display on Arrow, which will most likely give you 1x scale
factor. I'm planning to add "device's default scale factor" to address this.

[mazda, 2012/08/09 17:07:38]

Done.

+  return thumbnail_size.Scale(primary_display.device_scale_factor());
+}
+
+// Returns the scrollbar size in DIP.
+int GetScrollbarSize() {
+#if defined(OS_WIN)

[oshima, 2012/08/09 02:11:02]

Is it better to have "&& !defined(USE_AURA) "?
I know we don't officially support 2x scale factor on windows, but we don't have
to break it either.

[mazda, 2012/08/09 17:07:38]

Done.
Even with Aura, gfx::scrollbar_size() returns the size taking into account
system DPI, so the returned size may be incorrect.
Probably it's better to fix gfx::scrollbar_size() to handle system DPI and
chrome's scale factor properly, but that can break other parts that use this
function.
I added a comment to revisit Win Aura case. 

+  // On windows gfx::scrollbar_size() returns the size taking into account DPI.
+  return static_cast<int>(std::ceil(gfx::scrollbar_size() / ui::GetDPIScale()));
+#else
+  return gfx::scrollbar_size();
+#endif
+}
+
+// Returns the scrollbar size in pixel.
+int GetScrollbarSizeInPixel(content::RenderWidgetHostView* view) {
+#if defined(OS_WIN)
+  // On windows gfx::scrollbar_size() returns the size taking into account DPI.
+  return gfx::scrollbar_size();
+#else
+  float scale_factor = ui::GetScaleFactorScale(ui::GetScaleFactorForNativeView(
+      view->GetNativeView()));
+  return static_cast<int>(scale_factor * gfx::scrollbar_size());
+#endif
 }
 
 // Returns the clipping rectangle that is used for creating a thumbnail with
@@ -142,8 +202,8 @@ gfx::Rect GetClippingRect(const gfx::Size& source_size,
 // store. The returned bitmap will be isNull if there was an error creating it.
 SkBitmap CreateThumbnail(
     const SkBitmap& bitmap,
-    int desired_width,
-    int desired_height,
+    const gfx::Size& desired_size,
+    int scrollbar_size,
     ThumbnailGenerator::ClipResult* clip_result) {
   base::TimeTicks begin_compute_thumbnail = base::TimeTicks::Now();
 
@@ -151,7 +211,6 @@ SkBitmap CreateThumbnail(
   if (*clip_result == ThumbnailGenerator::kUnprocessed) {
     // Clip the pixels that will commonly hold a scrollbar, which looks bad in
     // thumbnails.
-    int scrollbar_size = gfx::scrollbar_size();
     SkIRect scrollbarless_rect =
         { 0, 0,
           std::max(1, bitmap.width() - scrollbar_size),
@@ -160,7 +219,7 @@ SkBitmap CreateThumbnail(
     bitmap.extractSubset(&bmp, scrollbarless_rect);
 
     clipped_bitmap = ThumbnailGenerator::GetClippedBitmap(
-        bmp, desired_width, desired_height, clip_result);
+        bmp, desired_size.width(), desired_size.height(), clip_result);
   } else {
     clipped_bitmap = bitmap;
   }
@@ -168,7 +227,7 @@ SkBitmap CreateThumbnail(
   // 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.
   SkBitmap result = SkBitmapOperations::DownsampleByTwoUntilSize(
-      clipped_bitmap, desired_width, desired_height);
+      clipped_bitmap, desired_size.width(), desired_size.height());
 #if !defined(USE_AURA)
   // This is a bit subtle. SkBitmaps are refcounted, but the magic
   // ones in PlatformCanvas can't be assigned to SkBitmap with proper
@@ -517,14 +576,13 @@ void ThumbnailGenerator::AsyncUpdateThumbnail(
   gfx::Rect copy_rect = gfx::Rect(view->GetViewBounds().size());
   // Clip the pixels that will commonly hold a scrollbar, which looks bad in
   // thumbnails.
-  int scrollbar_size = gfx::scrollbar_size();
+  int scrollbar_size = GetScrollbarSize();
   copy_rect.Inset(0, 0, scrollbar_size, scrollbar_size);
   ClipResult clip_result = ThumbnailGenerator::kUnprocessed;
   copy_rect = GetClippingRect(copy_rect.size(),
                               gfx::Size(kThumbnailWidth, kThumbnailHeight),
                               &clip_result);
-  gfx::Size copy_size =
-      gfx::Size(kThumbnailWidth, kThumbnailHeight).Scale(kThumbnailCopyScale);
+  gfx::Size copy_size = GetCopySizeForThumbnail(view);
   skia::PlatformCanvas* temp_canvas = new skia::PlatformCanvas;
   render_widget_host->CopyFromBackingStore(
       copy_rect,
@@ -545,10 +603,11 @@ void ThumbnailGenerator::UpdateThumbnailWithBitmap(
   if (bitmap.isNull() || bitmap.empty())
     return;
 
-  SkBitmap thumbnail = CreateThumbnail(bitmap,
-                                       kThumbnailWidth,
-                                       kThumbnailHeight,
-                                       &clip_result);
+  SkBitmap thumbnail = CreateThumbnail(
+      bitmap,
+      GetThumbnailSizeInPixel(),
+      GetScrollbarSizeInPixel(web_contents->GetRenderViewHost()->GetView()),
+      &clip_result);
   UpdateThumbnail(web_contents, thumbnail, clip_result);
 }