Fix preview thumbnail aspect ratio issues.

This CL fixes an error where the preview captured from a loading
webpage (used in hover cards and tablet tabstrip) could appear blurry
or stretched when the aspect ratio of the window did not match the
thumbnail.

It solves the problem by providing a max capture size based on the
ratio between the desired and current window size, then clipping the
image down later, just as we do with single-frame capture of active
windows on tab switch.

Still to do:
 - Trim scroll bars out of the final captured image.

Change-Id: I8cbeee5c9c5959163d2e7f0b6251802f40455f98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1818641
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702127}

chrome/browser/ui/thumbnails/thumbnail_tab_helper.cc

@@ -65,18 +65,22 @@ void ThumbnailTabHelper::CaptureThumbnailOnTabSwitch() {
 
   // Note: this is the size in pixels on-screen, not the size in DIPs.
   gfx::Size source_size = source_view->GetViewBounds().size();
-  // Clip the pixels that will commonly hold a scrollbar, which looks bad in
-  // thumbnails.
+  if (source_size.IsEmpty())
+    return;
+
+  const gfx::Size desired_size = GetThumbnailSize();
   const float scale_factor = source_view->GetDeviceScaleFactor();
+  // Clip the pixels that will commonly hold a scrollbar, which looks bad in
+  // thumbnails - but only if that wouldn't make the thumbnail too small.
   const int scrollbar_size = gfx::scrollbar_size() * scale_factor;
+  if (source_size.width() - scrollbar_size > desired_size.width() &&
+      source_size.height() - scrollbar_size > desired_size.height()) {
     source_size.Enlarge(-scrollbar_size, -scrollbar_size);
+  }
 
-  if (source_size.IsEmpty())
-    return;
-
-  const gfx::Size desired_size = TabStyle::GetPreviewImageSize();
   thumbnails::CanvasCopyInfo copy_info =
       thumbnails::GetCanvasCopyInfo(source_size, scale_factor, desired_size);
+
   source_view->CopyFromSurface(
       copy_info.copy_rect, copy_info.target_size,
       base::BindOnce(&ThumbnailTabHelper::StoreThumbnail,
@@ -104,10 +108,10 @@ void ThumbnailTabHelper::StartVideoCapture() {
   if (video_capturer_) {
     // Already capturing: We're already forcing rendering. Clear the capturer.
     video_capturer_->Stop();
-    video_capturer_ = nullptr;
+    video_capturer_.reset();
   } else {
     // *Not* already capturing: Force rendering.
-    web_contents()->IncrementCapturerCount(TabStyle::GetPreviewImageSize());
+    web_contents()->IncrementCapturerCount(GetThumbnailSize());
   }
 
   // Get the WebContents' main view.
@@ -117,11 +121,43 @@ void ThumbnailTabHelper::StartVideoCapture() {
     return;
   }
 
-  // Start capturing.
-  const gfx::Size desired_size = TabStyle::GetPreviewImageSize();
+  const gfx::Size preview_size = GetThumbnailSize();
+  const float scale_factor = source_view->GetDeviceScaleFactor();
+  const gfx::Size source_size_px = source_view->GetViewBounds().size();
+  const gfx::Size target_size_px =
+      gfx::ScaleToFlooredSize(preview_size, scale_factor);
+  DCHECK(!target_size_px.IsEmpty());
+  if (source_size_px.IsEmpty()) {
+    web_contents()->DecrementCapturerCount();
+    return;
+  }
+
+  // TODO(dfried): this doesn't take scroll bars into account, so they show up
+  // in the thumbnail. Fix it.
+  gfx::Size max_size_px;
+  if (source_size_px.width() < target_size_px.width() ||
+      source_size_px.height() < target_size_px.height()) {
+    // The source is smaller than the target - typically shorter, since normal
+    // browser windows have a minimum width. Allowing the capture to use up to
+    // double the size of the target bitmap provides decent results in most
+    // cases (and with a window that is a sliver you can't get a good result
+    // anyway).
+    max_size_px =
+        gfx::Size(target_size_px.width() * 2, target_size_px.height() * 2);
+  } else {
+    // This scaling logic makes the maximum size equal to the size of the source
+    // scaled down so it is no smaller than the target bitmap in either
+    // dimension. It means we always have an appropriate sized frame to clip
+    // from (the final clip region will be determined after capture).
+    const float min_ratio =
+        std::min(float{source_size_px.width()} / target_size_px.width(),
+                 float{source_size_px.height()} / target_size_px.height());
+    max_size_px = gfx::ScaleToCeiledSize(source_size_px, 1.0f / min_ratio);
+  }
+
   constexpr int kMaxFrameRate = 5;
   video_capturer_ = source_view->CreateVideoCapturer();
-  video_capturer_->SetResolutionConstraints(desired_size, desired_size, true);
+  video_capturer_->SetResolutionConstraints(target_size_px, max_size_px, false);
   video_capturer_->SetAutoThrottlingEnabled(false);
   video_capturer_->SetMinSizeChangePeriod(base::TimeDelta());
   video_capturer_->SetFormat(media::PIXEL_FORMAT_ARGB,
@@ -143,6 +179,10 @@ content::RenderWidgetHostView* ThumbnailTabHelper::GetView() {
   return web_contents()->GetRenderViewHost()->GetWidget()->GetView();
 }
 
+gfx::Size ThumbnailTabHelper::GetThumbnailSize() const {
+  return TabStyle::GetPreviewImageSize();
+}
+
 void ThumbnailTabHelper::OnVisibilityChanged(content::Visibility visibility) {
   if (last_visibility_ == content::Visibility::VISIBLE &&
       visibility != content::Visibility::VISIBLE) {
@@ -203,12 +243,22 @@ void ThumbnailTabHelper::OnFrameCaptured(
     viz::mojom::FrameSinkVideoConsumerFrameCallbacksPtr releaser;
   };
 
-  const gfx::Size desired_size = TabStyle::GetPreviewImageSize();
-  LOG(ERROR) << desired_size.ToString();
-
+  // We want to grab a subset of the captured content rect. Since we've ensured
+  // that in the vast majority of cases the captured frame will be an
+  // appropriate size to clip a thumbnail from, our standard clipping logic
+  // should be adequate here.
+  content::RenderWidgetHostView* const source_view = GetView();
+  const float scale_factor =
+      source_view ? source_view->GetDeviceScaleFactor() : 1.0f;
+  auto copy_info = thumbnails::GetCanvasCopyInfo(
+      content_rect.size(), scale_factor, GetThumbnailSize());
+  gfx::Rect copy_rect = copy_info.copy_rect;
+  copy_rect.Offset(content_rect.x(), content_rect.y());
+
+  const gfx::Size bitmap_size(content_rect.right(), content_rect.bottom());
   SkBitmap frame;
   frame.installPixels(
-      SkImageInfo::MakeN32(content_rect.width(), content_rect.height(),
+      SkImageInfo::MakeN32(bitmap_size.width(), bitmap_size.height(),
                            kPremul_SkAlphaType,
                            info->color_space->ToSkColorSpace()),
       pixels,
@@ -221,8 +271,7 @@ void ThumbnailTabHelper::OnFrameCaptured(
   frame.setImmutable();
 
   SkBitmap cropped_frame;
-  frame.extractSubset(&cropped_frame, gfx::RectToSkIRect(content_rect));
-
+  if (frame.extractSubset(&cropped_frame, gfx::RectToSkIRect(copy_rect)))
     StoreThumbnail(cropped_frame);
 }
 

chrome/browser/ui/thumbnails/thumbnail_tab_helper.h

@@ -48,6 +48,8 @@ class ThumbnailTabHelper
 
   content::RenderWidgetHostView* GetView();
 
+  gfx::Size GetThumbnailSize() const;
+
   // content::WebContentsObserver:
   void OnVisibilityChanged(content::Visibility visibility) override;
   void DidFinishNavigation(

chrome/browser/ui/thumbnails/thumbnail_utils.cc

@@ -20,24 +20,29 @@ bool IsGoodClipping(ClipResult clip_result) {
 
 CanvasCopyInfo GetCanvasCopyInfo(const gfx::Size& source_size,
                                  float scale_factor,
-                                 const gfx::Size& target_size) {
+                                 const gfx::Size& unscaled_target_size) {
   DCHECK(!source_size.IsEmpty());
-  DCHECK(!target_size.IsEmpty());
+  DCHECK(!unscaled_target_size.IsEmpty());
   DCHECK_GT(scale_factor, 0.0f);
 
-  CanvasCopyInfo copy_info;
-
   const float desired_aspect =
-      float{target_size.width()} / float{target_size.height()};
+      float{unscaled_target_size.width()} / unscaled_target_size.height();
+
+  CanvasCopyInfo copy_info;
+  copy_info.target_size =
+      gfx::ScaleToFlooredSize(unscaled_target_size, scale_factor);
 
   // Get the clipping rect so that we can preserve the aspect ratio while
   // filling the destination.
-  if (source_size.width() < target_size.width() ||
-      source_size.height() < target_size.height()) {
+  if (source_size.width() < unscaled_target_size.width() ||
+      source_size.height() < unscaled_target_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.
-    copy_info.copy_rect = gfx::Rect(target_size);
+    gfx::Size copy_size = source_size;
+    copy_size.SetToMin(copy_info.target_size);
+
+    copy_info.copy_rect = gfx::Rect(copy_size);
     copy_info.clip_result = ClipResult::kSourceSmallerThanTarget;
 
   } else {
@@ -66,8 +71,6 @@ CanvasCopyInfo GetCanvasCopyInfo(const gfx::Size& source_size,
     }
   }
 
-  copy_info.target_size = gfx::ScaleToFlooredSize(target_size, scale_factor);
-
   return copy_info;
 }
 

chrome/browser/ui/thumbnails/thumbnail_utils.h

@@ -47,7 +47,7 @@ bool IsGoodClipping(ClipResult clip_result);
 // value contains the type of clip and the clip parameters.
 CanvasCopyInfo GetCanvasCopyInfo(const gfx::Size& source_size,
                                  float scale_factor,
-                                 const gfx::Size& target_size);
+                                 const gfx::Size& unscaled_target_size);
 
 }  // namespace thumbnails