Combine native sampling with sampling

src/codec/SkAndroidCodec.cpp

Index: src/codec/SkAndroidCodec.cpp
diff --git a/src/codec/SkAndroidCodec.cpp b/src/codec/SkAndroidCodec.cpp
index 43ed90f6c0be67cea075f4c23f578065afae1b5e..086caf2d8129f30e00de197802e2358ff24a2a99 100644
--- a/src/codec/SkAndroidCodec.cpp
+++ b/src/codec/SkAndroidCodec.cpp
@@ -53,6 +53,11 @@ SkISize SkAndroidCodec::getSampledDimensions(int sampleSize) const {
         return SkISize::Make(0, 0);
     }
 
+    // Fast path for when we are not scaling.
+    if (1 == sampleSize) {
+        return fInfo.dimensions();
+    }
+
     return this->onGetSampledDimensions(sampleSize);
 }
 
@@ -77,9 +82,9 @@ SkISize SkAndroidCodec::getSampledSubsetDimensions(int sampleSize, const SkIRect
         return SkISize::Make(0, 0);
     }
 
-    // If the subset is the entire image, for consistency, use onGetSampledDimensions().
+    // If the subset is the entire image, for consistency, use getSampledDimensions().
     if (fInfo.dimensions() == subset.size()) {
-        return onGetSampledDimensions(sampleSize);
+        return this->getSampledDimensions(sampleSize);
     }
 
     // This should perhaps call a virtual function, but currently both of our subclasses
@@ -104,6 +109,15 @@ SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void*
         if (!is_valid_subset(*options->fSubset, fInfo.dimensions())) {
             return SkCodec::kInvalidParameters;
         }
+
+        if (SkIRect::MakeSize(fInfo.dimensions()) == *options->fSubset) {
+            // The caller wants the whole thing, rather than a subset. Modify

[scroggo_chromium, 2015/11/02 21:22:10]

This is an important fix: without it, I end up reporting failure for some of the
images we test on the bots.

The problem is that we didn't match the dimensions just right, due to rounding
differences.

When I get down into the code that checks whether setSampleX(sampleX)_==
info.height(), they do not match, so we return kInvalidScale. (FWIW, even with
your change to use options.fSampleX, modified to be the right
post-native-scaling sample size, the two do not match.)

[msarett, 2015/11/02 22:31:57]

Acknowledged.

+            // the AndroidOptions passed to onGetAndroidPixels to not specify
+            // a subset.
+            defaultOptions = *options;

[scroggo_chromium, 2015/11/02 21:22:10]

"options" is not const, so I could have modified it directly. I think it should
be const, but didn't want that to slow down this change, so I'm treating it as
const, which is safer than modifying it behind the client's back. SkCodec's
options are also passed via a const pointer (was it deliberate that this one was
not?).

[msarett, 2015/11/02 22:31:57]

Yes I think you are right, it should be const.

+            defaultOptions.fSubset = nullptr;
+            options = &defaultOptions;
+        }
     }
 
     return this->onGetAndroidPixels(info, pixels, rowBytes, *options);