Update Subset benches to support interlacing and fix bugs

bench/subset/SubsetTranslateBench.cpp

Index: bench/subset/SubsetTranslateBench.cpp
diff --git a/bench/subset/SubsetTranslateBench.cpp b/bench/subset/SubsetTranslateBench.cpp
index 8f29ba8fa011f25ef74eee4eb09d2ebb09fcb73a..d13f31a86d1e90f7a4b099ea5a95a25aa50a4742 100644
--- a/bench/subset/SubsetTranslateBench.cpp
+++ b/bench/subset/SubsetTranslateBench.cpp
@@ -53,17 +53,29 @@ bool SubsetTranslateBench::isSuitableFor(Backend backend) {
     return kNonRendering_Backend == backend;
 }
 
+// Allows allocating the bitmap first, and then writing to them later (in startScanlineDecode)
+static SkPMColor* get_colors(SkBitmap* bm) {
+    SkColorTable* ct = bm->getColorTable();
+    if (!ct) {
+        return nullptr;
+    }
+
+    return const_cast<SkPMColor*>(ct->readColors());
+}
+
 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;
+    int colorCount = 256;

[scroggo, 2015/10/08 18:29:08]

I'm cheating a little bit here. Setting this value here allows alloc_pixels to
read from it (and expect a deterministic value), but that won't necessarily be
the value set by startScanlineDecode (on the other hand, I think we almost
always defensively set it to 256 - perhaps the only time we do not is in wbmp,
since the indices in the data, being stored in one bit per pixel, cannot be out
of range, so this might be correct in most cases).

Alternatively, I could have continued to call startScanlineDecode before calling
alloc_pixels. This can be done in one of two ways:

1. Make an extra call to startScanlineDecode
2. Do not reuse the bitmap

(1) probably does more work (it means we need to rewind and reread the stream,
and potentially recreate some internal structures), and probably does not
represent the use case.

(2) goes against the original intent, which was to reuse the same bitmap. I
wonder if it would be more realistic to use a pool of bitmaps (since the Android
client wants to have multiple tiles which can be drawn to the screen in one
frame), but that seems like too much work for us to try to reproduce their exact
use case.

This seemed like the best compromise to me.

[msarett, 2015/10/08 18:52:41]

Yes I think it is almost always 256.  Rare bmps may also sometimes have a
smaller color table when they use less than 8-bit indices.
I agree that this is the best compromise.

     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.
@@ -71,20 +83,55 @@ void SubsetTranslateBench::onDraw(int n, SkCanvas* canvas) {
             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->skipScanlines(y);
+                    SkDEBUGCODE(SkCodec::Result result =)
+                    codec->startScanlineDecode(info, nullptr, get_colors(&bitmap), &colorCount);

[scroggo, 2015/10/08 18:29:08]

Note that I'm using the colors from the bitmap's color table, which were copied
from colors. If I just used colors directly, bitmap's colors would be whatever
they were randomly initialized to. I think this only really matters if we decide
to draw the output and look at it.

[msarett, 2015/10/08 18:52:41]

Acknowledged.

+                    SkASSERT(SkCodec::kSuccess == result);
+
+                    SkDEBUGCODE(result =) codec->skipScanlines(y);
+                    SkASSERT(SkCodec::kSuccess == result);
+
                     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++) {
+                                SkDEBUGCODE(result =) codec->getScanlines(row.get(), 1, 0);
+                                SkASSERT(SkCodec::kSuccess == result);
+
+                                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);
+
+                            SkDEBUGCODE(result =) codec->getScanlines(stripeBm.getPixels(),
+                                    currSubsetHeight, stripeBm.rowBytes());
+                            SkASSERT(SkCodec::kSuccess == result);
+
+                            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);
                     }
                 }
             }
@@ -97,7 +144,7 @@ void SubsetTranslateBench::onDraw(int n, SkCanvas* canvas) {
         for (int count = 0; count < n; count++) {
             int width, height;
             SkAutoTDelete<SkImageDecoder> decoder(SkImageDecoder::Factory(fStream));
-            decoder->buildTileIndex(fStream->duplicate(), &width, &height);
+            SkAssertResult(decoder->buildTileIndex(fStream->duplicate(), &width, &height));
             SkBitmap bitmap;
             // Note that we use the same bitmap for all of the subsets.
             // It might be larger than necessary for the end subsets.
@@ -117,7 +164,7 @@ void SubsetTranslateBench::onDraw(int n, SkCanvas* canvas) {
                             height - y : fSubsetHeight;
                     SkIRect rect = SkIRect::MakeXYWH(x, y, currSubsetWidth,
                             currSubsetHeight);
-                    decoder->decodeSubset(&bitmap, rect, fColorType);
+                    SkAssertResult(decoder->decodeSubset(&bitmap, rect, fColorType));
                 }
             }
         }