Improving GestureNav screenshotting performance - Part 1 - gl_helper

content/common/gpu/client/gl_helper_unittest.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 <stdio.h>
 #include <cmath>
 #include <string>
 #include <vector>
 
 #include <GLES2/gl2.h>
@@ skipping 121 matching lines @@
     float x3 = x2 * x;
     if (x <= 1) {
       return (a + 2) * x3 - (a + 3) * x2 + 1;
     } else if (x < 2) {
       return a * x3 - 5 * a * x2 + 8 * a * x - 4 * a;
     } else {
       return 0.0f;
     }
   }
 
-  // Look up a single R/G/B/A value.
-  // Clamp x/y.
+  // Look up a single channel value. Works for 4-channel and single channel
+  // bitmaps.  Clamp x/y.
   int Channel(SkBitmap* pixels, int x, int y, int c) {
-    uint32* data =
-        pixels->getAddr32(std::max(0, std::min(x, pixels->width() - 1)),
-                          std::max(0, std::min(y, pixels->height() - 1)));
-    return (*data) >> (c * 8) & 0xff;
+    if (pixels->bytesPerPixel() == 4) {
+      uint32* data =
+          pixels->getAddr32(std::max(0, std::min(x, pixels->width() - 1)),
+                            std::max(0, std::min(y, pixels->height() - 1)));
+      return (*data) >> (c * 8) & 0xff;
+    } else {
+      DCHECK_EQ(pixels->bytesPerPixel(), 1);
+      DCHECK_EQ(c, 0);
+      return *pixels->getAddr8(std::max(0, std::min(x, pixels->width() - 1)),
+                               std::max(0, std::min(y, pixels->height() - 1)));
+    }
   }
 
-  // Set a single R/G/B/A value.
+  // Set a single channel value. Works for 4-channel and single channel
+  // bitmaps.  Clamp x/y.
   void SetChannel(SkBitmap* pixels, int x, int y, int c, int v) {
     DCHECK_GE(x, 0);
     DCHECK_GE(y, 0);
     DCHECK_LT(x, pixels->width());
     DCHECK_LT(y, pixels->height());
-    uint32* data = pixels->getAddr32(x, y);
-    v = std::max(0, std::min(v, 255));
-    *data = (*data & ~(0xffu << (c * 8))) | (v << (c * 8));
+    if (pixels->bytesPerPixel() == 4) {
+      uint32* data = pixels->getAddr32(x, y);
+      v = std::max(0, std::min(v, 255));
+      *data = (*data & ~(0xffu << (c * 8))) | (v << (c * 8));
+    } else {
+      DCHECK_EQ(pixels->bytesPerPixel(), 1);
+      DCHECK_EQ(c, 0);
+      uint8* data = pixels->getAddr8(x, y);
+      v = std::max(0, std::min(v, 255));
+      *data = v;
+    }
   }
 
   // Print all the R, G, B or A values from an SkBitmap in a
   // human-readable format.
   void PrintChannel(SkBitmap* pixels, int c) {
     for (int y = 0; y < pixels->height(); y++) {
       std::string formatted;
       for (int x = 0; x < pixels->width(); x++) {
         formatted.append(base::StringPrintf("%3d, ", Channel(pixels, x, y, c)));
       }
@@ skipping 212 matching lines @@
   // is no more than |maxdiff| apart. If they are not similar enough,
   // prints out |truth|, |other|, |source|, |scaler_stages| and |message|.
   void Compare(SkBitmap* truth,
                SkBitmap* other,
                int maxdiff,
                SkBitmap* source,
                const std::vector<GLHelperScaling::ScalerStage>& scaler_stages,
                std::string message) {
     EXPECT_EQ(truth->width(), other->width());
     EXPECT_EQ(truth->height(), other->height());
+    EXPECT_EQ(truth->bytesPerPixel(), other->bytesPerPixel());
+    int bpp = truth->bytesPerPixel();
     for (int x = 0; x < truth->width(); x++) {
       for (int y = 0; y < truth->height(); y++) {
-        for (int c = 0; c < 4; c++) {
+        for (int c = 0; c < bpp; c++) {
           int a = Channel(truth, x, y, c);
           int b = Channel(other, x, y, c);
           EXPECT_NEAR(a, b, maxdiff) << " x=" << x << " y=" << y << " c=" << c
                                      << " " << message;
           if (std::abs(a - b) > maxdiff) {
             LOG(ERROR) << "-------expected--------";
-            PrintChannel(truth, c);
+            for (int i = 0; i < bpp; i++) {
+              LOG(ERROR) << "Channel " << i << ":";
+              PrintChannel(truth, i);
+            }
             LOG(ERROR) << "-------actual--------";
-            PrintChannel(other, c);
+            for (int i = 0; i < bpp; i++) {
+              LOG(ERROR) << "Channel " << i << ":";
+              PrintChannel(other, i);
+            }
             if (source) {
-              LOG(ERROR) << "-------before scaling--------";
-              PrintChannel(source, c);
+            LOG(ERROR) << "-------original--------";

[no sievers, 2014/08/20 20:29:52]

nit: indent

[mfomitchev, 2014/08/22 18:01:33]

Done.

+            for (int i = 0; i < source->bytesPerPixel(); i++) {
+                LOG(ERROR) << "Channel " << i << ":";
+                PrintChannel(source, i);
+              }
             }
             LOG(ERROR) << "-----Scaler stages------";
             LOG(ERROR) << PrintStages(scaler_stages);
             return;
           }
         }
       }
     }
   }
 
   // Get a single R, G, B or A value as a float.
   float ChannelAsFloat(SkBitmap* pixels, int x, int y, int c) {
     return Channel(pixels, x, y, c) / 255.0;
   }
 
   // Works like a GL_LINEAR lookup on an SkBitmap.
   float Bilinear(SkBitmap* pixels, float x, float y, int c) {
     x -= 0.5;
     y -= 0.5;
     int base_x = static_cast<int>(floorf(x));
     int base_y = static_cast<int>(floorf(y));
     x -= base_x;
     y -= base_y;
     return (ChannelAsFloat(pixels, base_x, base_y, c) * (1 - x) * (1 - y) +
             ChannelAsFloat(pixels, base_x + 1, base_y, c) * x * (1 - y) +
             ChannelAsFloat(pixels, base_x, base_y + 1, c) * (1 - x) * y +
             ChannelAsFloat(pixels, base_x + 1, base_y + 1, c) * x * y);
   }
 
+  // Encodes an RGBA bitmap to grayscale.
+  // Reference implementation for
+  // GLHelper::CopyToTextureImpl::EncodeTextureAsGrayscale.
+  void EncodeToGrayscaleSlow(SkBitmap* input,
+                             SkBitmap* output) {
+    const float kRGBtoGrayscaleColorWeights[3] = {0.213f, 0.715f, 0.072f};

[no sievers, 2014/08/20 20:29:52]

Should these constants be shared?

[mfomitchev, 2014/08/22 18:01:33]

It wouldn't be terribly straightforward. This array is of length 3, and the one
in gl_helper is of length 4. Also, the one in GL Helper is defined within
CopyTextureToImpl, so it's not currently visible outside of gl_helper.cc. If you
still think the values should be shared, what would be the best way to achieve
that? Move the const to GLHelper?

[no sievers, 2014/08/22 18:44:13]

If you expose them in gl_helper.h somehow, can they be shared here and inside
gl_helper.cc?

[mfomitchev, 2014/08/25 19:21:53]

Yes, I could construct the 3-element array in gl_helper_testing using the first
three elements of the 4-element array in gl_helper.
I've noticed that the other color weights (kRGBtoYColorWeights,
kRGBtoUColorWeights, kRGBtoVColorWeights) are not shared between gl_helper and
gl_helper_unittest. In fact they are not even declared as consts in
gl_helper_unittest - they are just used as plain  numbers in TestYUVReadback(). 

If we decide to expose kRGBtoGrayscaleColorWeights in gl helper, we should
probably do so for the other three weights arrays as well. Otherwise I can at
least declare the as const in gl_helper_unittest.

+    CHECK_EQ(kAlpha_8_SkColorType, output->colorType());
+    CHECK_EQ(input->width(), output->width());
+    CHECK_EQ(input->height(), output->height());
+    CHECK_EQ(input->colorType(), kRGBA_8888_SkColorType);
+
+    for (int dst_y = 0; dst_y < output->height(); dst_y++) {
+      for (int dst_x = 0; dst_x < output->width(); dst_x++) {
+        float c0 = ChannelAsFloat(input, dst_x, dst_y, 0);
+        float c1 = ChannelAsFloat(input, dst_x, dst_y, 1);
+        float c2 = ChannelAsFloat(input, dst_x, dst_y, 2);
+        float value = c0 * kRGBtoGrayscaleColorWeights[0] +
+            c1 * kRGBtoGrayscaleColorWeights[1] +
+            c2 * kRGBtoGrayscaleColorWeights[2];
+        SetChannel(output,
+                   dst_x,
+                   dst_y,
+                   0,
+                   static_cast<int>(value * 255.0f + 0.5f));

[no sievers, 2014/08/20 20:29:52]

hmm why +0.5?

[mfomitchev, 2014/08/22 18:01:33]

So that it rounds to the closest int rather than round down.

+      }
+    }
+  }
+
   // Very slow bicubic / bilinear scaler for reference.
   void ScaleSlow(SkBitmap* input,
                  SkBitmap* output,
                  content::GLHelper::ScalerQuality quality) {
     float xscale = static_cast<float>(input->width()) / output->width();
     float yscale = static_cast<float>(input->height()) / output->height();
     float clamped_xscale = xscale < 1.0 ? 1.0 : 1.0 / xscale;
     float clamped_yscale = yscale < 1.0 ? 1.0 : 1.0 / yscale;
     for (int dst_y = 0; dst_y < output->height(); dst_y++) {
       for (int dst_x = 0; dst_x < output->width(); dst_x++) {
@@ skipping 62 matching lines @@
                      dst_x,
                      dst_y,
                      channel,
                      static_cast<int>(value * 255.0f + 0.5f));
         }
       }
     }
   }
 
   void FlipSKBitmap(SkBitmap* bitmap) {
+    int bpp = bitmap->bytesPerPixel();
+    DCHECK(bpp == 4 || bpp == 1);
     int top_line = 0;
     int bottom_line = bitmap->height() - 1;
     while (top_line < bottom_line) {
       for (int x = 0; x < bitmap->width(); x++) {
-        std::swap(*bitmap->getAddr32(x, top_line),
-                  *bitmap->getAddr32(x, bottom_line));
+        bpp == 4 ?
+          std::swap(*bitmap->getAddr32(x, top_line),
+                    *bitmap->getAddr32(x, bottom_line))
+        : std::swap(*bitmap->getAddr8(x, top_line),
+                    *bitmap->getAddr8(x, bottom_line));
       }
       top_line++;
       bottom_line--;
     }
   }
 
+  // Swaps red and blue channels in each pixel in a 32-bit bitmap.
+  void SwizzleSKBitmap(SkBitmap* bitmap) {
+    int bpp = bitmap->bytesPerPixel();
+    DCHECK(bpp == 4);
+    for (int y = 0; y < bitmap->height(); y++) {
+      for (int x = 0; x < bitmap->width(); x++) {
+        // Swap channels 0 and 2 (red and blue)
+        int c0 = Channel(bitmap, x, y, 0);
+        int c2 = Channel(bitmap, x, y, 2);
+        SetChannel(bitmap, x, y, 2, c0);
+        SetChannel(bitmap, x, y, 0, c2);
+      }
+    }
+  }
+
   // gl_helper scales recursively, so we'll need to do that
   // in the reference implementation too.
   void ScaleSlowRecursive(SkBitmap* input,
                           SkBitmap* output,
                           content::GLHelper::ScalerQuality quality) {
     if (quality == content::GLHelper::SCALER_QUALITY_FAST ||
         quality == content::GLHelper::SCALER_QUALITY_GOOD) {
       ScaleSlow(input, output, quality);
       return;
     }
@@ skipping 28 matching lines @@
       }
     }
 
     SkBitmap tmp;
     tmp.allocN32Pixels(xtmp, ytmp);
 
     ScaleSlowRecursive(input, &tmp, quality);
     ScaleSlowRecursive(&tmp, output, quality);
   }
 
+  // Creates an RGBA SkBitmap
+  scoped_ptr<SkBitmap> CreateTestBitmap(int width,
+                                        int height,
+                                        int test_pattern) {
+    scoped_ptr<SkBitmap> bitmap(new SkBitmap);
+    bitmap->allocPixels(SkImageInfo::Make(
+        width, height, kRGBA_8888_SkColorType, kPremul_SkAlphaType));
+
+    for (int x = 0; x < width; ++x) {
+      for (int y = 0; y < height; ++y) {
+        switch (test_pattern) {
+          case 0:  // Smooth test pattern
+            SetChannel(bitmap.get(), x, y, 0, x * 10);
+            SetChannel(bitmap.get(), x, y, 0, y == 0 ? x * 50 : x * 10);
+            SetChannel(bitmap.get(), x, y, 1, y * 10);
+            SetChannel(bitmap.get(), x, y, 2, (x + y) * 10);
+            SetChannel(bitmap.get(), x, y, 3, 255);
+            break;
+          case 1:  // Small blocks
+            SetChannel(bitmap.get(), x, y, 0, x & 1 ? 255 : 0);
+            SetChannel(bitmap.get(), x, y, 1, y & 1 ? 255 : 0);
+            SetChannel(bitmap.get(), x, y, 2, (x + y) & 1 ? 255 : 0);
+            SetChannel(bitmap.get(), x, y, 3, 255);
+            break;
+          case 2:  // Medium blocks
+            SetChannel(bitmap.get(), x, y, 0, 10 + x / 2 * 50);
+            SetChannel(bitmap.get(), x, y, 1, 10 + y / 3 * 50);
+            SetChannel(bitmap.get(), x, y, 2, (x + y) / 5 * 50 + 5);
+            SetChannel(bitmap.get(), x, y, 3, 255);
+            break;
+        }
+      }
+    }
+    return bitmap.Pass();
+  }
+
+  // Binds texture and framebuffer and loads the bitmap pixels into the texture.
+  void BindTextureAndFrameBuffer(WebGLId texture,
+                                 WebGLId framebuffer,
+                                 SkBitmap* bitmap,
+                                 int width,
+                                 int height) {
+    context_->bindFramebuffer(GL_FRAMEBUFFER, framebuffer);
+    context_->bindTexture(GL_TEXTURE_2D, texture);
+    context_->texImage2D(GL_TEXTURE_2D,
+                         0,
+                         GL_RGBA,
+                         width,
+                         height,
+                         0,
+                         GL_RGBA,
+                         GL_UNSIGNED_BYTE,
+                         bitmap->getPixels());
+  }
+
+  // Create a test image, transform it using
+  // GLHelper::CropScaleReadbackAndCleanTexture and a reference implementation
+  // and compare the results.
+  void TestCropScaleReadbackAndCleanTexture(int xsize,
+                                            int ysize,
+                                            int scaled_xsize,
+                                            int scaled_ysize,
+                                            int test_pattern,
+                                            SkColorType out_color_type,
+                                            bool swizzle,
+                                            size_t quality) {

[no sievers, 2014/08/20 20:29:52]

nit: maybe less error-prone to just pass the ScalerQuality in here.

[mfomitchev, 2014/08/22 18:01:33]

Yeah, this is modeled after TestScale() below, but I thought about changing this
too. However we also use |quality| to fetch the string 'name' from the
kQualityNames array.. Perhaps just changing the var name to quality_index would
be good enough? If not, I guess I could write a helper function converting
ScalerQuality to a string.

[no sievers, 2014/08/22 18:44:13]

I see, maybe just keep it as is then if it matches the function below.

+    DCHECK(out_color_type == kAlpha_8_SkColorType ||
+           out_color_type == kRGBA_8888_SkColorType ||
+           out_color_type == kBGRA_8888_SkColorType);
+    WebGLId src_texture = context_->createTexture();
+    WebGLId framebuffer = context_->createFramebuffer();
+    scoped_ptr<SkBitmap> input_pixels =
+        CreateTestBitmap(xsize, ysize, test_pattern).Pass();
+    BindTextureAndFrameBuffer(
+        src_texture, framebuffer, input_pixels.get(), xsize, ysize);
+
+    std::string message = base::StringPrintf(
+        "input size: %dx%d "
+        "output size: %dx%d "
+        "pattern: %d , quality: %s, "
+        "out_color_type: %d",
+        xsize,
+        ysize,
+        scaled_xsize,
+        scaled_ysize,
+        test_pattern,
+        kQualityNames[quality],
+        out_color_type);
+
+    // Transform the bitmap using GLHelper::CropScaleReadbackAndCleanTexture.
+    SkBitmap output_pixels;
+    output_pixels.allocPixels(SkImageInfo::Make(
+        scaled_xsize, scaled_ysize, out_color_type, kPremul_SkAlphaType));
+    base::RunLoop run_loop;
+    gfx::Size encoded_texture_size;
+    helper_->CropScaleReadbackAndCleanTexture(
+        src_texture,
+        gfx::Size(xsize, ysize),
+        gfx::Rect(xsize, ysize),
+        gfx::Size(scaled_xsize, scaled_ysize),
+        static_cast<unsigned char*>(output_pixels.getPixels()),
+        out_color_type,
+        base::Bind(&callcallback, run_loop.QuitClosure()),
+        kQualities[quality]);
+    run_loop.Run();
+    // CropScaleReadbackAndCleanTexture flips the pixels. Flip them back.
+    FlipSKBitmap(&output_pixels);

[no sievers, 2014/08/20 20:29:52]

Why do we end up with an upside down image? That seems odd.

[mfomitchev, 2014/08/22 18:01:34]

The old version of CropScaleReadbackAndCleanTexture() also always flipped the
texture
(https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...).
I have no idea why, but if I don't do it, the image is upside down.

[no sievers, 2014/08/22 18:44:13]

This is intriguing, as I believe we use this path for readbacks on Android and
the images come out with the expected orientation, so I'll try to follow the
code and see why this is hardcoded to flip in CropScaleReadbackAndCleanTexture
:)

+
+    // Now transofrm the bitmap using the reference implementation.

[no sievers, 2014/08/20 20:29:52]

nit, typo: 'transform'

[mfomitchev, 2014/08/22 18:01:33]

Done.

+    SkBitmap scaled_pixels;
+    scaled_pixels.allocPixels(SkImageInfo::Make(
+        scaled_xsize,
+        scaled_ysize,
+        kRGBA_8888_SkColorType,
+        kPremul_SkAlphaType));
+    SkBitmap truth_pixels;
+    // Step 1: Scale
+    ScaleSlowRecursive(input_pixels.get(), &scaled_pixels, kQualities[quality]);
+    // Step 2: Convert to the specified output format, encoding to grayscale if
+    // needed.
+    switch (out_color_type) {
+      case kRGBA_8888_SkColorType:
+        truth_pixels = scaled_pixels;
+        break;
+    case kBGRA_8888_SkColorType:

[no sievers, 2014/08/20 20:29:52]

nit: indent here and below

[mfomitchev, 2014/08/22 18:01:33]

Done.

+      // Swizzle pixels of the scaled bitmap if the output needs to be BGRA

[no sievers, 2014/08/20 20:29:52]

or could Compare() just learn to handle the format of the bitmap?

[mfomitchev, 2014/08/22 18:01:33]

Problem is, scale tests here use Compare() to do a pixel-by-pixel comparison of
RGBA to BGRA bitmaps, and they do not swizzle. If I make Compare() learn to
handle the format, I'd have to change those tests as well or they will fail.. I
can still do it if you think it's worth the effort.

[no sievers, 2014/08/22 18:44:13]

hmm maybe put a TODO then?

[mfomitchev, 2014/08/25 19:21:53]

Ok, I've just updated TestScale to consistently use RGBA.

+      truth_pixels.installPixels(
+          SkImageInfo::Make(scaled_xsize,
+                            scaled_ysize,
+                            out_color_type,
+                            kPremul_SkAlphaType),
+          scaled_pixels.getPixels(),
+          scaled_pixels.width() * 4);
+      SwizzleSKBitmap(&truth_pixels);
+      break;
+    case kAlpha_8_SkColorType:
+      truth_pixels.allocPixels(SkImageInfo::Make(
+          scaled_xsize, scaled_ysize, out_color_type, kPremul_SkAlphaType));
+      EncodeToGrayscaleSlow(&scaled_pixels, &truth_pixels);
+      break;
+    default:
+      NOTREACHED();
+    }
+
+    // Now compare the results.
+    SkAutoLockPixels lock_input(truth_pixels);
+    const std::vector<GLHelperScaling::ScalerStage> dummy_stages;
+    Compare(&truth_pixels,
+            &output_pixels,
+            2,

[no sievers, 2014/08/20 20:29:52]

Just wondering, why 2? Maybe hubbe knows. I did notice btw that the scaler tests
which we don't run on mobile on the waterfall actually fail with larger errors
if you do try to run them.

[mfomitchev, 2014/08/22 18:01:33]

It does fail on my machine for 16x16 -> 3x6 best-quality scale if I set it to 1.
My guess would be it comes down to CPU vs GPU floating point precision.

+            input_pixels.get(),
+            dummy_stages,
+            message + " comparing against transformed/scaled");
+
+    context_->deleteTexture(src_texture);
+    context_->deleteFramebuffer(framebuffer);
+  }
+
   // Scaling test: Create a test image, scale it using GLHelperScaling
   // and a reference implementation and compare the results.
   void TestScale(int xsize,
                  int ysize,
                  int scaled_xsize,
                  int scaled_ysize,
                  int test_pattern,
                  size_t quality,
                  bool flip) {
     WebGLId src_texture = context_->createTexture();
     WebGLId framebuffer = context_->createFramebuffer();
-    SkBitmap input_pixels;
-    input_pixels.allocN32Pixels(xsize, ysize);
-
-    for (int x = 0; x < xsize; ++x) {
-      for (int y = 0; y < ysize; ++y) {
-        switch (test_pattern) {
-          case 0:  // Smooth test pattern
-            SetChannel(&input_pixels, x, y, 0, x * 10);
-            SetChannel(&input_pixels, x, y, 1, y * 10);
-            SetChannel(&input_pixels, x, y, 2, (x + y) * 10);
-            SetChannel(&input_pixels, x, y, 3, 255);
-            break;
-          case 1:  // Small blocks
-            SetChannel(&input_pixels, x, y, 0, x & 1 ? 255 : 0);
-            SetChannel(&input_pixels, x, y, 1, y & 1 ? 255 : 0);
-            SetChannel(&input_pixels, x, y, 2, (x + y) & 1 ? 255 : 0);
-            SetChannel(&input_pixels, x, y, 3, 255);
-            break;
-          case 2:  // Medium blocks
-            SetChannel(&input_pixels, x, y, 0, 10 + x / 2 * 50);
-            SetChannel(&input_pixels, x, y, 1, 10 + y / 3 * 50);
-            SetChannel(&input_pixels, x, y, 2, (x + y) / 5 * 50 + 5);
-            SetChannel(&input_pixels, x, y, 3, 255);
-            break;
-        }
-      }
-    }
-
-    context_->bindFramebuffer(GL_FRAMEBUFFER, framebuffer);
-    context_->bindTexture(GL_TEXTURE_2D, src_texture);
-    context_->texImage2D(GL_TEXTURE_2D,
-                         0,
-                         GL_RGBA,
-                         xsize,
-                         ysize,
-                         0,
-                         GL_RGBA,
-                         GL_UNSIGNED_BYTE,
-                         input_pixels.getPixels());
+    scoped_ptr<SkBitmap> input_pixels =
+        CreateTestBitmap(xsize, ysize, test_pattern).Pass();
+    BindTextureAndFrameBuffer(
+        src_texture, framebuffer, input_pixels.get(), xsize, ysize);
 
     std::string message = base::StringPrintf(
         "input size: %dx%d "
         "output size: %dx%d "
         "pattern: %d quality: %s",
         xsize,
         ysize,
         scaled_xsize,
         scaled_ysize,
         test_pattern,
@@ skipping 25 matching lines @@
     helper_->ReadbackTextureSync(
         dst_texture,
         gfx::Rect(0, 0, scaled_xsize, scaled_ysize),
         static_cast<unsigned char*>(output_pixels.getPixels()),
         kRGBA_8888_SkColorType);
     if (flip) {
       // Flip the pixels back.
       FlipSKBitmap(&output_pixels);
     }
     if (xsize == scaled_xsize && ysize == scaled_ysize) {
-      Compare(&input_pixels,
+      Compare(input_pixels.get(),
               &output_pixels,
               2,
               NULL,
               stages,
               message + " comparing against input");
     }
     SkBitmap truth_pixels;
     truth_pixels.allocN32Pixels(scaled_xsize, scaled_ysize);
 
-    ScaleSlowRecursive(&input_pixels, &truth_pixels, kQualities[quality]);
+    ScaleSlowRecursive(input_pixels.get(), &truth_pixels, kQualities[quality]);
     Compare(&truth_pixels,
             &output_pixels,
             2,
-            &input_pixels,
+            input_pixels.get(),
             stages,
             message + " comparing against scaled");
 
     context_->deleteTexture(src_texture);
     context_->deleteTexture(dst_texture);
     context_->deleteFramebuffer(framebuffer);
   }
 
   // Create a scaling pipeline and check that it is made up of
   // valid scaling operations.
@@ skipping 308 matching lines @@
                                     base::Bind(&callcallback,
                                                run_loop.QuitClosure()));
       run_loop.Run();
     } else {
       helper_->ReadbackTextureSync(src_texture,
                                    gfx::Rect(src_size),
                                    pixels,
                                    color_type);
     }
   }
-
   // Test basic format readback.
   bool TestTextureFormatReadback(const gfx::Size& src_size,
                          SkColorType color_type,
                          bool async) {
     SkImageInfo info =
         SkImageInfo::Make(src_size.width(),
                           src_size.height(),
                           color_type,
                           kPremul_SkAlphaType);
     if (!helper_->IsReadbackConfigSupported(color_type)) {
@@ skipping 591 matching lines @@
                 }
               }
             }
           }
         }
       }
     }
   }
 }
 
+// Per pixel tests, all sizes are small so that we can print
+// out the generated bitmaps.
+TEST_F(GLHelperPixelTest, CropScaleReadbackAndCleanTextureTest) {
+  int sizes[] = {3, 6, 16};
+  SkColorType color_types[] =

[no sievers, 2014/08/20 20:29:52]

nit: 
const int kSizes[] = ...
const SkColorType kColorTypes[] =...

[mfomitchev, 2014/08/22 18:01:33]

Done.

+      {kAlpha_8_SkColorType, kRGBA_8888_SkColorType, kBGRA_8888_SkColorType};
+  // Test BEST and FAST qualities, skip GOOD
+  for (size_t q = 0; q < arraysize(kQualities); q += 2) {

[no sievers, 2014/08/20 20:29:52]

why q+=2 and not q++?

[mfomitchev, 2014/08/22 18:01:33]

FAST uses a different code path in CropScaleReadbackAndCleanTexture() than GOOD
and BEST (it skips the scale step). So I wanted to test FAST and one of
GOOD/BEST. I figured I don't have to test both GOOD and BEST because
scale-specific testing is already covered by ScaleTest. And the debug version of
this test already takes 46s, so I didn't want to make it unnecessarily slower.

[no sievers, 2014/08/22 18:44:13]

Ok, maybe it's more straightforward to define your own array of qualities to
test in here then.

[mfomitchev, 2014/08/25 19:21:53]

The index is also used to fetch the name from kQualityNames. so I'd have to
re-declare kQualityNames as well, which seems like an overkill...

+    for (int color_type = 0; color_type < 3; color_type++) {

[no sievers, 2014/08/20 20:29:52]

color_type < arraysize(kColorTypes)

[mfomitchev, 2014/08/22 18:01:33]

Done.

+      for (int x = 0; x < 3; x++) {
+        for (int y = 0; y < 3; y++) {
+          for (int dst_x = 0; dst_x < 3; dst_x++) {
+            for (int dst_y = 0; dst_y < 3; dst_y++) {
+              for (int pattern = 0; pattern < 3; pattern++) {
+                TestCropScaleReadbackAndCleanTexture(sizes[x],
+                                                     sizes[y],
+                                                     sizes[dst_x],
+                                                     sizes[dst_y],
+                                                     pattern,
+                                                     color_types[color_type],
+                                                     false,
+                                                     q);
+                if (HasFailure())
+                  return;
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
+
 // Validate that all scaling generates valid pipelines.
 TEST_F(GLHelperTest, ValidateScalerPipelines) {
   int sizes[] = {7, 99, 128, 256, 512, 719, 720, 721, 1920, 2011, 3217, 4096};
   for (size_t q = 0; q < arraysize(kQualities); q++) {
     for (size_t x = 0; x < arraysize(sizes); x++) {
       for (size_t y = 0; y < arraysize(sizes); y++) {
         for (size_t dst_x = 0; dst_x < arraysize(sizes); dst_x++) {
           for (size_t dst_y = 0; dst_y < arraysize(sizes); dst_y++) {
             TestScalerPipeline(
                 q, sizes[x], sizes[y], sizes[dst_x], sizes[dst_y]);
@@ skipping 63 matching lines @@
   base::CommandLine::Init(argc, argv);
   base::TestSuite* suite = new content::ContentTestSuite(argc, argv);
 #if defined(OS_MACOSX)
   base::mac::ScopedNSAutoreleasePool pool;
 #endif
 
   content::UnitTestTestSuite runner(suite);
   base::MessageLoop message_loop;
   return runner.Run();
 }