Improving GestureNav screenshotting performance - part 2 - GestureNav

content/browser/compositor/delegated_frame_host.cc

Index: content/browser/compositor/delegated_frame_host.cc
diff --git a/content/browser/compositor/delegated_frame_host.cc b/content/browser/compositor/delegated_frame_host.cc
index 4cfa531c1b4a1ac295f02da092bf7d6216592de7..db3383aab3b233c49456d0a0578753f6f2d25635 100644
--- a/content/browser/compositor/delegated_frame_host.cc
+++ b/content/browser/compositor/delegated_frame_host.cc
@@ -19,6 +19,9 @@
 #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 {
@@ -136,7 +139,8 @@ void DelegatedFrameHost::CopyFromCompositingSurface(
     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()) {
@@ -591,7 +595,7 @@ void DelegatedFrameHost::PrepareBitmapCopyOutputResult(
     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;
@@ -599,12 +603,42 @@ void DelegatedFrameHost::PrepareBitmapCopyOutputResult(
   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
@@ -623,6 +657,7 @@ void DelegatedFrameHost::ReturnSubscriberTexture(
     dfh->idle_frame_subscriber_textures_.push_back(subscriber_texture);
 }
 
+// static
 void DelegatedFrameHost::CopyFromCompositingSurfaceFinishedForVideo(
     base::WeakPtr<DelegatedFrameHost> dfh,
     const base::Callback<void(bool)>& callback,