Update Subset benches to support interlacing and fix bugs

bench/subset/SubsetTranslateBench.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "CodecBenchPriv.h"
 #include "SubsetTranslateBench.h"
 #include "SubsetBenchPriv.h"
@@ skipping 44 matching lines @@
 
 void SubsetTranslateBench::onDraw(int n, SkCanvas* canvas) {
     // When the color type is kIndex8, we will need to store the color table.  If it is
     // used, it will be initialized by the codec.
     int colorCount;
     SkPMColor colors[256];
     if (fUseCodec) {
         for (int count = 0; count < n; count++) {
             SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(fStream->duplicate()));
             const SkImageInfo info = codec->getInfo().makeColorType(fColorType);
-            SkAutoTDeleteArray<uint8_t> row(new uint8_t[info.minRowBytes()]);
-            codec->startScanlineDecode(info, nullptr, colors, &colorCount);
+            SkAutoTDeleteArray<uint8_t> row(nullptr);
+            if (codec->getScanlineOrder() == SkCodec::kTopDown_SkScanlineOrder) {
+                row.reset(new uint8_t[info.minRowBytes()]);
+            }
 
             SkBitmap bitmap;
             // Note that we use the same bitmap for all of the subsets.
             // It might be larger than necessary for the end subsets.
             SkImageInfo subsetInfo = info.makeWH(fSubsetWidth, fSubsetHeight);
             alloc_pixels(&bitmap, subsetInfo, colors, colorCount);
 
+            const uint32_t bpp = info.bytesPerPixel();
+
             for (int x = 0; x < info.width(); x += fSubsetWidth) {
                 for (int y = 0; y < info.height(); y += fSubsetHeight) {
+                    codec->startScanlineDecode(info, nullptr, colors, &colorCount);

[scroggo, 2015/10/06 20:36:04]

Here's the bug (unless I'm missing something?): without calling startScanlines
again, we're just attempting to read off the end of the image, which will return
some failure case (which we never read).

On that note, perhaps we should read the Results and assert them. (I also wonder
if we should make it so kNonrendering tests in nanobench still get a canvas, at
least when you use the flag -w to write the output images, so we can verify the
results somehow.)

Taking it a step further, maybe we ultimately don't need these tests - instead,
we should be using some version of SkBitmapRegionDecoderInterface.

[msarett, 2015/10/06 21:58:52]

Ugh this is an embarrassing bug.

Theoretically, we test correctness in dm, so we can test performance here
without worrying about checking correctness.  But I guess the code got
complicated enough that I made a mistake.  I'm not sure if that is a bad
oversight on my part or an indication that we need more ways of error checking
here.

Certainly asserting the results as a first step can't hurt.

I do think that the SkBitmapRegionDecoderInterface tests are more important at
this point...these will be tests we look at when replacing BRD.  But I think
these still have value, especially now that we need to redo them to make valid
conclusions.

[scroggo, 2015/10/07 19:50:58]

Done.
That is probably ultimately true, although we cannot look at those until we
settle on how to do subsets in SkScaledCodec (and of course, the hard part is
sampling combined with subsets).

In the meantime, I think this gives us something interesting to look at. (Also,
if we find the straightforward code here is better, but not so in SkScaledCodec,
we can know that we've probably done something wrong in SkScaledCodec...)

                     codec->skipScanlines(y);
+
                     const uint32_t currSubsetWidth =
                             x + (int) fSubsetWidth > info.width() ?
                             info.width() - x : fSubsetWidth;
                     const uint32_t currSubsetHeight =
                             y + (int) fSubsetHeight > info.height() ?
                             info.height() - y : fSubsetHeight;
-                    const uint32_t bpp = info.bytesPerPixel();
-                    for (uint32_t y = 0; y < currSubsetHeight; y++) {
-                        codec->getScanlines(row.get(), 1, 0);
-                        memcpy(bitmap.getAddr(0, y), row.get() + x * bpp,
-                                currSubsetWidth * bpp);
+
+                    switch (codec->getScanlineOrder()) {
+                        case SkCodec::kTopDown_SkScanlineOrder:
+                            for (uint32_t y = 0; y < currSubsetHeight; y++) {

[scroggo, 2015/10/06 20:36:04]

Again, I think this loop is unchanged.

[msarett, 2015/10/06 21:58:52]

Acknowledged.

+                                codec->getScanlines(row.get(), 1, 0);
+                                memcpy(bitmap.getAddr(0, y), row.get() + x * bpp,
+                                        currSubsetWidth * bpp);
+                            }
+                            break;
+                        case SkCodec::kNone_SkScanlineOrder: {
+                            // decode all scanlines that intersect the subset, and copy the subset
+                            // into the output.
+                            SkImageInfo stripeInfo = info.makeWH(info.width(), currSubsetHeight);
+                            SkBitmap stripeBm;
+                            alloc_pixels(&stripeBm, stripeInfo, colors, colorCount);
+
+                            codec->getScanlines(stripeBm.getPixels(), currSubsetHeight,
+                                    stripeBm.rowBytes());
+                            for (uint32_t subsetY = 0; subsetY < currSubsetHeight; subsetY++) {
+                                memcpy(bitmap.getAddr(0, subsetY), stripeBm.getAddr(x, subsetY),
+                                        currSubsetWidth * bpp);
+                            }
+                            break;
+                        }
+                        default:
+                            // We currently are only testing kTopDown and kNone, which are the only
+                            // two used by the subsets we care about. skbug.com/4428
+                            SkASSERT(false);
                     }
                 }
             }
         }
     } else {
         // We create a color table here to satisfy allocPixels() when the output
         // type is kIndex8.  It's okay that this is uninitialized since we never
         // use it.
         SkColorTable* colorTable = new SkColorTable(colors, 0);
         for (int count = 0; count < n; count++) {
@@ skipping 18 matching lines @@
                     const uint32_t currSubsetHeight = y + (int) fSubsetHeight > height ?
                             height - y : fSubsetHeight;
                     SkIRect rect = SkIRect::MakeXYWH(x, y, currSubsetWidth,
                             currSubsetHeight);
                     decoder->decodeSubset(&bitmap, rect, fColorType);
                 }
             }
         }
     }
 }