Improving GestureNav screenshotting performance - part 2 - GestureNav

content/browser/compositor/delegated_frame_host.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/compositor/delegated_frame_host.h"
 
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/compositor_frame_ack.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/resources/single_release_callback.h"
 #include "cc/resources/texture_mailbox.h"
 #include "cc/surfaces/surface_factory.h"
 #include "content/browser/compositor/resize_lock.h"
 #include "content/common/gpu/client/gl_helper.h"
 #include "content/public/browser/render_widget_host_view_frame_subscriber.h"
 #include "content/public/common/content_switches.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"
 #include "skia/ext/image_operations.h"
+#include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkPaint.h"
+#include "third_party/skia/include/effects/SkLumaColorFilter.h"
 #include "ui/gfx/frame_time.h"
 
 namespace content {
 
 ////////////////////////////////////////////////////////////////////////////////
 // DelegatedFrameHostClient
 
 bool DelegatedFrameHostClient::ShouldCreateResizeLock() {
   // On Windows and Linux, holding pointer moves will not help throttling
   // resizes.
@@ skipping 97 matching lines @@
     scoped_ptr<cc::CopyOutputRequest> request) {
   client_->GetLayer()->RequestCopyOfOutput(request.Pass());
 }
 
 void DelegatedFrameHost::CopyFromCompositingSurface(
     const gfx::Rect& src_subrect,
     const gfx::Size& output_size,
     const base::Callback<void(bool, const SkBitmap&)>& callback,
     const SkColorType color_type) {
   // Only ARGB888 and RGB565 supported as of now.
-  bool format_support = ((color_type == kRGB_565_SkColorType) ||
+  bool format_support = ((color_type == kAlpha_8_SkColorType) ||
+                         (color_type == kRGB_565_SkColorType) ||
                          (color_type == kN32_SkColorType));
   DCHECK(format_support);
   if (!CanCopyToBitmap()) {
     callback.Run(false, SkBitmap());
     return;
   }
 
   scoped_ptr<cc::CopyOutputRequest> request =
       cc::CopyOutputRequest::CreateRequest(base::Bind(
           &DelegatedFrameHost::CopyFromCompositingSurfaceHasResult,
@@ skipping 429 matching lines @@
       color_type,
       base::Bind(&CopyFromCompositingSurfaceFinished,
                  callback,
                  base::Passed(&release_callback),
                  base::Passed(&bitmap),
                  base::Passed(&bitmap_pixels_lock)),
       GLHelper::SCALER_QUALITY_FAST);
 }
 
 // static
 void DelegatedFrameHost::PrepareBitmapCopyOutputResult(

[no sievers, 2014/08/25 21:45:12]

This is only for the software renderer, right? Because otherwise for
result->HasTexture() we go through PrepareTextureCopyOutputResult() which calls
CropScaleReadbackAndCleanMailbox() and I assume already does the right thing
after your changes?

[mfomitchev, 2014/08/25 22:38:35]

Yup. We used to paint the bitmap to A8 bitmap in
ScreenshotData::EncodeOnWorker(), and now we don't since
CopyFromCompositingSurfaceFinished/CropScaleReadbackAndCleanMailbox now creates
a grayscale bitmap.

     const gfx::Size& dst_size_in_pixel,
     const SkColorType color_type,
     const base::Callback<void(bool, const SkBitmap&)>& callback,
     scoped_ptr<cc::CopyOutputResult> result) {
-  if (color_type != kN32_SkColorType) {
+  if (color_type != kN32_SkColorType && color_type != kAlpha_8_SkColorType) {
     NOTIMPLEMENTED();
     callback.Run(false, SkBitmap());
     return;
   }
   DCHECK(result->HasBitmap());
   scoped_ptr<SkBitmap> source = result->TakeBitmap();
   DCHECK(source);
-  SkBitmap bitmap = skia::ImageOperations::Resize(
-      *source,
-      skia::ImageOperations::RESIZE_BEST,
-      dst_size_in_pixel.width(),
-      dst_size_in_pixel.height());
-  callback.Run(true, bitmap);
+  SkBitmap scaled_bitmap;
+  if (source->width() != dst_size_in_pixel.width() ||
+      source->height() != dst_size_in_pixel.height()) {
+    scaled_bitmap =
+        skia::ImageOperations::Resize(*source,
+                                      skia::ImageOperations::RESIZE_BEST,
+                                      dst_size_in_pixel.width(),
+                                      dst_size_in_pixel.height());
+  } else {
+    scaled_bitmap = *source;
+  }
+  if (color_type == kN32_SkColorType) {
+    DCHECK(scaled_bitmap.colorType() == kN32_SkColorType);
+    callback.Run(true, scaled_bitmap);
+    return;
+  }
+  DCHECK_EQ(color_type, kAlpha_8_SkColorType);

[no sievers, 2014/08/25 21:45:12]

It's a bit unfortunate that all over the place we treat Alpha8 as a grayscale
format now. I guess luminance or something would be better.
But seems like it's the only Skia format that works?

Or would SkUnknown_SkColorType actually be better and then we define our own
enums for GLHelper?

[mfomitchev, 2014/08/25 22:38:35]

Using kUnknown_SkColorType seems pretty hacky to me.. I guess we could get a
Skia owner to comment on this. It's worth noting, we don't actually use this A8
bitmap to paint. We use its pixel data to encode to grayscale PNG...

+  if (scaled_bitmap.colorType() == kAlpha_8_SkColorType) {
+    callback.Run(true, scaled_bitmap);

[no sievers, 2014/08/25 21:45:12]

How can we end up here? Who converted the bitmap?

[mfomitchev, 2014/08/25 22:38:35]

I don't think we can currently. However  kAlpha_8_SkColorType is the color type
we put into cc::CopyOutputRequest object. Currently it's ignored by the software
renderer, but I thought it's worth adding this check in case software renderer
decides to respect the color type in the request in the future (and perhaps
implements some sort of an optimized conversion).

[no sievers, 2014/08/25 23:05:47]

Or maybe put a DCHECK(scaled_bitmap.colorType() != kAlpha_8_SkColorType) here
for the time being.

+    return;
+  }
+  // Paint |scaledBitmap| to alpha-only |a8Bitmap|.
+  SkBitmap grayscale_bitmap;
+  bool success = grayscale_bitmap.allocPixels(
+      SkImageInfo::MakeA8(scaled_bitmap.width(), scaled_bitmap.height()));
+  if (!success) {
+    callback.Run(false, SkBitmap());
+    return;
+  }
+  SkCanvas canvas(grayscale_bitmap);
+  SkPaint paint;
+  skia::RefPtr<SkColorFilter> filter =

[no sievers, 2014/08/25 21:45:12]

So if we wanted to scale and filter in one pass, we couldn't use the BEST
resizer from ImageOperations?

[mfomitchev, 2014/08/25 22:38:35]

I am not sure. Do you know how would one resize and color-filter in one path
with Skia? FWIW, currently we only use this for grayscale conversion and never
scale.

[no sievers, 2014/08/25 23:05:47]

I *think* if you make the canvas smaller/bigger it will do scaling, but probably
not the filtering you want. But not sure how much you care about the sw
compositor here. I guess you are not really making it slower for your use case
compared to before, since you are just moving the encoding pass from caller to
callee.

+      skia::AdoptRef(SkLumaColorFilter::Create());
+  paint.setColorFilter(filter.get());
+  canvas.drawBitmap(scaled_bitmap, SkIntToScalar(0), SkIntToScalar(0), &paint);
+  callback.Run(true, grayscale_bitmap);
 }
 
 // static
 void DelegatedFrameHost::ReturnSubscriberTexture(
     base::WeakPtr<DelegatedFrameHost> dfh,
     scoped_refptr<OwnedMailbox> subscriber_texture,
     uint32 sync_point) {
   if (!subscriber_texture.get())
     return;
   if (!dfh)
     return;
 
   subscriber_texture->UpdateSyncPoint(sync_point);
 
   if (dfh->frame_subscriber_ && subscriber_texture->texture_id())
     dfh->idle_frame_subscriber_textures_.push_back(subscriber_texture);
 }
 
+// static
 void DelegatedFrameHost::CopyFromCompositingSurfaceFinishedForVideo(
     base::WeakPtr<DelegatedFrameHost> dfh,
     const base::Callback<void(bool)>& callback,
     scoped_refptr<OwnedMailbox> subscriber_texture,
     scoped_ptr<cc::SingleReleaseCallback> release_callback,
     bool result) {
   callback.Run(result);
 
   uint32 sync_point = 0;
   if (result) {
@@ skipping 282 matching lines @@
   if (frame_provider_.get()) {
     new_layer->SetShowDelegatedContent(frame_provider_.get(),
                                        current_frame_size_in_dip_);
   }
   if (!surface_id_.is_null()) {
     new_layer->SetShowSurface(surface_id_, current_frame_size_in_dip_);
   }
 }
 
 }  // namespace content