Add nanobench tests for BitmapRegionDecoder

bench/nanobench.cpp

Index: bench/nanobench.cpp
diff --git a/bench/nanobench.cpp b/bench/nanobench.cpp
index e7019dcb8c97c0713ce653a7b3a35865e6696ef7..dff732fe8df58bcf31c1a40fd3e12bf008b72e79 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"
@@ -26,6 +27,7 @@
 #include "Stats.h"
 #include "Timer.h"
 
+#include "SkBitmapRegionDecoderInterface.h"
 #include "SkBBoxHierarchy.h"
 #include "SkCanvas.h"
 #include "SkCodec.h"
@@ -557,6 +559,33 @@ 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, SkIRect* subset) {
+    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;
+    }
+
+    // We can set the subset rect here, since we now know the height of the image.  We will choose

[scroggo, 2015/09/17 20:05:38]

Since we're only decoding one subset, and we're comparing
SkImageDecoder::decodeSubset to other implementations, it seems like that
version is going to look bad regardless, since it also needs to call
buildTileIndex.

[msarett, 2015/09/18 13:22:30]

Yeah you're right, this is unfair.  I should give SkImageDecoder a real chance. 
My original rationale was that I am more focused on the comparison between
BitmapRegionCanvas and BitmapRegionScaledCodec.

Also, I saw skbug.com/4357 and I agree with you.  We should probably add a test
to the bench/subset folder, but I'm thinking we might as well also use this
test.

I was going to make a change to decode multiple subsets, but instead, I will
keep this as a single subset test and move buildTileIndex to the predraw step.

[scroggo, 2015/09/18 15:07:23]

I'm starting to wonder whether we want to include SkImageDecoder in this
comparison at all, if we've already decided that SkCodec is faster, unless we
think scaling tilts the scale? If we want to check for that it seems like we
would want to run our other subset benchmarks with scaling? (Maybe we already
do?)

This might be an interesting case for skbug.com/4357, and I suppose it couldn't
hurt to check, but I wonder if this is the most meaningful case to check that.
It seems like we really want to compare the cases where BRD is used, and I'm not
sure this particular subset is an interesting one.

[msarett, 2015/09/18 17:41:17]

I think we do want to include SkImageDecoder.  I fully expect SkImageDecoder to
lose, but, as I think you've mentioned before, we haven't really tested with all
of the outer layers on SkCodec.  Before replacing the BitmapRegionDecoder API, I
think we want to do the most fair comparison we can, testing both
implementations from the top level.

My original implementation didn't really give us a fair comparison, so I'm glad
you pointed that out.

If we change our minds, or finally end up deleting SkImageDecoder, turning off
this test is trivial (we just need to remove kOriginal_Strategy from the list of
strategies in nanobench.cpp).
We absolutely should check the other subset benchmarks with scaling.  I will do
this once I finish implementing scaling in SkScaledCodec.
I agree that this is not the only case for skbug/4357.  But I think it doesn't
hurt to be overly pessimistic when trying to justify replacing the old code with
the new code.

When I rewrite the SubsetBenches to test scaling, I will address 4357 again
there.

In general, I don't completely agree that this subset isn't the most meaningful
to test.  Certainly, it's not as exhaustive as the SubsetBenches.

IMO, this test actually covers a use case that we see quite frequently (but
couldn't test when we didn't have scaling).  Apps like Gallery/Photos/Maps will
scale large original images into many square thumbnails to view on the same
page.

Full disclaimer: Maps only has a TODO to use BitmapRegionDecoder for this.  I
*think* Photos uses BitmapRegionDecoder for this, but I'm not 100% sure.  If
they don't use BRD for this, I think they should.

It's also nice to test scaling performance when scaling a large subset - I
imagine that the influence of other factors is greater when the subsets are
small.

A downfall of this test might be that we don't fix the size of the output (ex:
to 512x512).  This would require cropping differently depending on the scale.  I
preferred not to add this complexity, but now I'm think that this test should be
much more complex...

+    // the maximum size square subset centered at the center of the image.
+    uint32_t dimension = SkTMin(brd->width(), brd->height());
+    uint32_t left = (brd->width() - dimension) / 2;
+    uint32_t top = (brd->height() - dimension) / 2;
+    *subset = SkIRect::MakeXYWH(left, top, dimension, dimension);
+    return true;
+}
+
 static void cleanup_run(Target* target) {
     delete target;
 #if SK_SUPPORT_GPU
@@ -580,9 +609,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 +908,44 @@ public:
             fUseCodec++;
         }
 
+        // Run the BRDBenches
+        // We will benchmark multiple BRD strategies.
+        const SkBitmapRegionDecoderInterface::Strategy strategies[] = {
+                SkBitmapRegionDecoderInterface::kOriginal_Strategy,
+                SkBitmapRegionDecoderInterface::kCanvas_Strategy,
+        };
+        // We will focus on these sample sizes, since they are most often used in real code.
+        const uint32_t sampleSizes[] = { 1, 2, 4, 8 };
+        while (fCurrentBRDImage < fImages.count()) {
+            while (fCurrentBRDStrategy < (int) SK_ARRAY_COUNT(strategies)) {
+                while (fCurrentColorType < fColorTypes.count()) {
+                    while (fCurrentBRDSampleSize < (int) SK_ARRAY_COUNT(sampleSizes)) {
+                        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];
+                        fCurrentBRDSampleSize++;
+
+                        SkIRect subset;
+                        if (valid_brd_bench(encoded.get(), strategy, colorType, &subset)) {
+                            return new BitmapRegionDecoderBench(SkOSPath::Basename(path.c_str()),
+                                    encoded.get(), strategy, colorType, sampleSize, subset);
+                        } else {
+                            break;
+                        }
+                    }
+                    fCurrentBRDSampleSize = 0;
+                    fCurrentColorType++;
+                }
+                fCurrentColorType = 0;
+                fCurrentBRDStrategy++;
+            }
+            fCurrentBRDStrategy = 0;
+            fCurrentBRDImage++;
+        }
+
         return nullptr;
     }
 
@@ -932,9 +1002,12 @@ private:
     int fCurrentCodec;
     int fCurrentImage;
     int fCurrentSubsetImage;
+    int fCurrentBRDImage;
     int fCurrentColorType;
     int fCurrentSubsetType;
     int fUseCodec;
+    int fCurrentBRDStrategy;
+    int fCurrentBRDSampleSize;
     int fCurrentAnimSKP;
 };