Add nanobench tests for BitmapRegionDecoder

bench/nanobench.cpp

Index: bench/nanobench.cpp
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index e7019dcb8c97c0713ce653a7b3a35865e6696ef7..7f383f3fb175b70f0b62cdc2b046997c42b43aca 100644
--- a/bench/nanobench.cpp
+++ b/bench/nanobench.cpp
@@ -10,6 +10,7 @@
 #include "nanobench.h"
 
 #include "Benchmark.h"
+#include "BitmapRegionDecoderBench.h"
 #include "CodecBench.h"
 #include "CrashHandler.h"
 #include "DecodingBench.h"
@@ -19,16 +20,17 @@
 #include "RecordingBench.h"
 #include "SKPAnimationBench.h"
 #include "SKPBench.h"
-#include "SubsetBenchPriv.h"
 #include "SubsetSingleBench.h"
 #include "SubsetTranslateBench.h"
 #include "SubsetZoomBench.h"
 #include "Stats.h"
 #include "Timer.h"
 
+#include "SkBitmapRegionDecoderInterface.h"
 #include "SkBBoxHierarchy.h"
 #include "SkCanvas.h"
 #include "SkCodec.h"
+#include "SkCodecBenchPriv.h"
 #include "SkCommonFlags.h"
 #include "SkData.h"
 #include "SkForceLinking.h"
@@ -525,7 +527,7 @@ static bool valid_subset_bench(const SkString& path, SkColorType colorType, bool
                 colors, &colorCount) != SkCodec::kSuccess)
         {
             SkDebugf("Could not create scanline decoder for %s with color type %s.  "
-                    "Skipping bench.\n", path.c_str(), get_color_name(colorType));
+                    "Skipping bench.\n", path.c_str(), color_type_to_str(colorType));
             return false;
         }
         *width = info.width();
@@ -539,7 +541,7 @@ static bool valid_subset_bench(const SkString& path, SkColorType colorType, bool
         //FIXME: See skbug.com/3921
         if (kIndex_8_SkColorType == colorType || kGray_8_SkColorType == colorType) {
             SkDebugf("Cannot use image subset decoder for %s with color type %s.  "
-                    "Skipping bench.\n", path.c_str(), get_color_name(colorType));
+                    "Skipping bench.\n", path.c_str(), color_type_to_str(colorType));
             return false;
         }
         if (!decoder->buildTileIndex(stream.detach(), width, height)) {
@@ -557,6 +559,38 @@ static bool valid_subset_bench(const SkString& path, SkColorType colorType, bool
     return true;
 }
 
+static bool valid_brd_bench(SkData* encoded, SkBitmapRegionDecoderInterface::Strategy strategy,
+        SkColorType colorType, uint32_t sampleSize, uint32_t minOutputSize, int* width,
+        int* height) {
+    SkStreamRewindable* stream = new SkMemoryStream(encoded);
+    SkAutoTDelete<SkBitmapRegionDecoderInterface> brd(
+            SkBitmapRegionDecoderInterface::CreateBitmapRegionDecoder(stream, strategy));
+    if (nullptr == brd.get()) {
+        // This is indicates that subset decoding is not supported for a particular image format.
+        return false;
+    }
+
+    SkAutoTDelete<SkBitmap> bitmap(brd->decodeRegion(0, 0, brd->width(), brd->height(), 1,
+            colorType));
+    if (nullptr == bitmap.get() || colorType != bitmap->colorType()) {
+        // This indicates that conversion to the requested color type is not supported for the
+        // particular image.
+        return false;
+    }
+
+    if (sampleSize * minOutputSize > (uint32_t) brd->width() || sampleSize * minOutputSize >
+            (uint32_t) brd->height()) {
+        // This indicates that the image is not large enough to decode a
+        // minOutputSize x minOutputSize subset at the given sampleSize.
+        return false;
+    }
+
+    // Set the image width and height.  The calling code will use this to choose subsets to decode.
+    *width = brd->width();
+    *height = brd->height();
+    return true;
+}
+
 static void cleanup_run(Target* target) {
     delete target;
 #if SK_SUPPORT_GPU
@@ -580,9 +614,12 @@ public:
                       , fCurrentCodec(0)
                       , fCurrentImage(0)
                       , fCurrentSubsetImage(0)
+                      , fCurrentBRDImage(0)
                       , fCurrentColorType(0)
                       , fCurrentSubsetType(0)
                       , fUseCodec(0)
+                      , fCurrentBRDStrategy(0)
+                      , fCurrentBRDSampleSize(0)
                       , fCurrentAnimSKP(0) {
         for (int i = 0; i < FLAGS_skps.count(); i++) {
             if (SkStrEndsWith(FLAGS_skps[i], ".skp")) {
@@ -876,6 +913,95 @@ public:
             fUseCodec++;
         }
 
+        // Run the BRDBenches
+        // We will benchmark multiple BRD strategies.
+        const SkBitmapRegionDecoderInterface::Strategy strategies[] = {
+                SkBitmapRegionDecoderInterface::kOriginal_Strategy,
+                SkBitmapRegionDecoderInterface::kCanvas_Strategy,
+        };
+
+        // We intend to create benchmarks that model the use cases in
+        // android/libraries/social/tiledimage.  In this library, an image is decoded in 512x512
+        // tiles.  The image can be translated freely, so the location of a tile may be anywhere in
+        // the image.  For that reason, we will benchmark decodes in five representative locations
+        // in the image.  Additionally, power of two scaling is enabled, so we test on the below

[scroggo, 2015/09/22 12:13:09]

Why only power of two? Is it because BRD claims to only support powers of two,
and JPEG does the wrong thing (skbug.com/4319)?

[msarett, 2015/09/22 15:27:19]

I think there are a few good reasons to only test power of 2.
(1) As you said, SkImageDecoder JPEG does the wrong thing, and we don't want to
benchmark code that doesn't work.
(2) All of the uses of BRD I have found only use power of 2, including the
particular use case that I am trying to match.
(3) PNGs always use the same sampling strategy regardless of the sample size, so
I think we get good coverage of PNG performance with this set of sample sizes.

On the other hand, we will definitely want to compare SkScaledCodec and
SkBitmapRegionCanvas for sampling JPEGs, which means we need to test on
non-power of 2 scales.

I am adding a few non-power of two scales and disabling these tests for
SkImageDecoder.  AFAIK, these tests aren't relevant for any current uses but may
be interesting for us to look at.

[scroggo, 2015/09/22 16:57:35]

Actually, you pose a good argument for only testing powers of two. I'm fine with
sticking with only powers of two (maybe with an explanation similar to the one
in this comment).

[msarett, 2015/09/22 17:53:38]

Done.

+        // set of sampleSizes.  The output tile is always 512x512, so when a sampleSize is used,
+        // the size of the subset that is decoded is always (sampleSize*512)x(sampleSize*512).
+        const uint32_t sampleSizes[] = { 1, 2, 4, 8, 16 };
+        const uint32_t minOutputSize = 512;

[scroggo, 2015/09/22 12:13:09]

Why did you make this a variable passed to valid_brd_bench, instead of a
constant inside that function?

[msarett, 2015/09/22 15:27:19]

I use it on line 950 to calculate subsetSize and wanted to avoid defining it
twice.  Alternatively, valid_brd_bench could set the subsetSize, but I'm not
sure either approach is better.

[scroggo, 2015/09/22 16:57:35]

Thanks for the explanation. The current way seems fine.

+        while (fCurrentBRDImage < fImages.count()) {
+            while (fCurrentBRDStrategy < (int) SK_ARRAY_COUNT(strategies)) {
+                while (fCurrentColorType < fColorTypes.count()) {
+                    while (fCurrentBRDSampleSize < (int) SK_ARRAY_COUNT(sampleSizes)) {
+                        while (fCurrentSubsetType <= kLastSingle_SubsetType) {
+                            const SkString& path = fImages[fCurrentBRDImage];
+                            SkAutoTUnref<SkData> encoded(SkData::NewFromFileName(path.c_str()));
+                            const SkBitmapRegionDecoderInterface::Strategy strategy =
+                                    strategies[fCurrentBRDStrategy];
+                            const SkColorType colorType = fColorTypes[fCurrentColorType];
+                            uint32_t sampleSize = sampleSizes[fCurrentBRDSampleSize];
+                            int currentSubsetType = fCurrentSubsetType++;
+
+                            int width = 0;
+                            int height = 0;
+                            if (valid_brd_bench(encoded.get(), strategy, colorType, sampleSize,
+                                    minOutputSize, &width, &height)) {
+                                SkString basename = SkOSPath::Basename(path.c_str());
+                                const uint32_t subsetSize = sampleSize * minOutputSize;
+                                switch (currentSubsetType) {

[scroggo, 2015/09/22 12:13:09]

You can eliminate a bunch of lines of code by moving the return outside the
switch statement:

SkIRect irect;
switch (currentSubsetType) {
  case kTopLeft_SubsetType:
    basename.append("_TopLeft");
    irect = SkIRect::MakeXYWH(0, 0, subsetSize, subsetSize);
    break;
  ...
  default:
    SkASSERT(false);
    return nullptr;
}

return new BRDBench(basename, encoded, strategy, colorType, sampleSize, irect);

[msarett, 2015/09/22 15:27:19]

Yes this is much better!

+                                    case kTopLeft_SubsetType:
+                                        basename.append("_TopLeft");
+                                        return new BitmapRegionDecoderBench(basename.c_str(),
+                                                encoded.get(), strategy, colorType, sampleSize,
+                                                SkIRect::MakeXYWH(0, 0, subsetSize, subsetSize));
+                                    case kTopRight_SubsetType:
+                                        basename.append("_TopRight");
+                                        return new BitmapRegionDecoderBench(basename.c_str(),
+                                                encoded.get(), strategy, colorType, sampleSize,
+                                                SkIRect::MakeXYWH(width - subsetSize, 0, subsetSize,
+                                                        subsetSize));
+                                    case kMiddle_SubsetType:
+                                        basename.append("_Middle");
+                                        return new BitmapRegionDecoderBench(basename.c_str(),
+                                                encoded.get(), strategy, colorType, sampleSize,
+                                                SkIRect::MakeXYWH((width - subsetSize) / 2,
+                                                        (height - subsetSize) / 2, subsetSize,
+                                                        subsetSize));
+                                    case kBottomLeft_SubsetType:
+                                        basename.append("_BottomLeft");
+                                        return new BitmapRegionDecoderBench(basename.c_str(),
+                                                encoded.get(), strategy, colorType, sampleSize,
+                                                SkIRect::MakeXYWH(0, height - subsetSize,
+                                                        subsetSize, subsetSize));
+                                    case kBottomRight_SubsetType:
+                                        basename.append("_BottomRight");
+                                        return new BitmapRegionDecoderBench(basename.c_str(),
+                                                encoded.get(), strategy, colorType, sampleSize,
+                                                SkIRect::MakeXYWH(width - subsetSize,
+                                                        height - subsetSize, subsetSize,
+                                                        subsetSize));
+                                    default:
+                                        SkASSERT(false);
+                                        return nullptr;
+                                }
+
+                            } else {

[scroggo, 2015/09/22 12:13:09]

nit: This will be more readable if you do the "exit" condition first:

if (!valid_brd_bench(...)) {
  break;
}
// The code you currently have in the above block.

[msarett, 2015/09/22 15:27:19]

Done.

+                                break;
+                            }
+                        }
+                        fCurrentSubsetType = 0;
+                        fCurrentBRDSampleSize++;
+                    }
+                    fCurrentBRDSampleSize = 0;
+                    fCurrentColorType++;
+                }
+                fCurrentColorType = 0;
+                fCurrentBRDStrategy++;
+            }
+            fCurrentBRDStrategy = 0;
+            fCurrentBRDImage++;
+        }
+
         return nullptr;
     }
 
@@ -907,7 +1033,8 @@ private:
         kBottomRight_SubsetType = 4,
         kTranslate_SubsetType   = 5,
         kZoom_SubsetType        = 6,
-        kLast_SubsetType        = kZoom_SubsetType
+        kLast_SubsetType        = kZoom_SubsetType,
+        kLastSingle_SubsetType  = kBottomRight_SubsetType,
     };
 
     const BenchRegistry* fBenches;
@@ -932,9 +1059,12 @@ private:
     int fCurrentCodec;
     int fCurrentImage;
     int fCurrentSubsetImage;
+    int fCurrentBRDImage;
     int fCurrentColorType;
     int fCurrentSubsetType;
     int fUseCodec;
+    int fCurrentBRDStrategy;
+    int fCurrentBRDSampleSize;
     int fCurrentAnimSKP;
 };