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 35 matching lines @@
 }
 
 const char* SubsetTranslateBench::onGetName() {
     return fName.c_str();
 }
 
 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.
             // 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->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);
                     }
                 }
             }
         }
     } 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++) {
             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.
             // If we do not include this step, decodeSubset() would allocate space
             // for the pixels automatically, but this would not allow us to reuse the
             // same bitmap as the other subsets.  We want to reuse the same bitmap
             // because it gives a more fair comparison with SkCodec and is a common
             // use case of BitmapRegionDecoder.
             bitmap.allocPixels(SkImageInfo::Make(fSubsetWidth, fSubsetHeight,
                     fColorType, kOpaque_SkAlphaType), nullptr, colorTable);
 
             for (int x = 0; x < width; x += fSubsetWidth) {
                 for (int y = 0; y < height; y += fSubsetHeight) {
                     const uint32_t currSubsetWidth = x + (int) fSubsetWidth > width ?
                             width - x : fSubsetWidth;
                     const uint32_t currSubsetHeight = y + (int) fSubsetHeight > height ?
                             height - y : fSubsetHeight;
                     SkIRect rect = SkIRect::MakeXYWH(x, y, currSubsetWidth,
                             currSubsetHeight);
-                    decoder->decodeSubset(&bitmap, rect, fColorType);
+                    SkAssertResult(decoder->decodeSubset(&bitmap, rect, fColorType));
                 }
             }
         }
     }
 }