Combine native sampling with sampling

src/codec/SkSampledCodec.cpp

Index: src/codec/SkSampledCodec.cpp
diff --git a/src/codec/SkSampledCodec.cpp b/src/codec/SkSampledCodec.cpp
index 229555b9bf549998a434b2339fac2eb5618586a6..f78bb87626f428758ece9d2f912e380c548e3822 100644
--- a/src/codec/SkSampledCodec.cpp
+++ b/src/codec/SkSampledCodec.cpp
@@ -7,37 +7,67 @@
 
 #include "SkCodec.h"
 #include "SkCodecPriv.h"
+#include "SkMath.h"
 #include "SkSampledCodec.h"
 
+#include <initializer_list>
+
 SkSampledCodec::SkSampledCodec(SkCodec* codec)
     : INHERITED(codec->getInfo())
     , fCodec(codec)
 {}
 
-SkISize SkSampledCodec::onGetSampledDimensions(int sampleSize) const {
-    // Fast path for when we are not scaling.
-    if (1 == sampleSize) {
-        return fCodec->getInfo().dimensions();
+SkISize SkSampledCodec::accountForNativeScaling(int* sampleSizePtr, int* nativeSampleSize) const {
+    SkISize preSampledSize = fCodec->getInfo().dimensions();
+    int sampleSize = *sampleSizePtr;
+    SkASSERT(sampleSize > 1);
+
+    if (nativeSampleSize) {
+        *nativeSampleSize = 1;
     }
 
-    const int width = fCodec->getInfo().width();
-    const int height = fCodec->getInfo().height();
-
-    // Check if the codec can provide the scaling natively.
-    float scale = get_scale_from_sample_size(sampleSize);
-    SkSize idealSize = SkSize::Make(scale * (float) width, scale * (float) height);
-    SkISize nativeSize = fCodec->getScaledDimensions(scale);
-    float widthDiff = SkTAbs(((float) nativeSize.width()) - idealSize.width());
-    float heightDiff = SkTAbs(((float) nativeSize.height()) - idealSize.height());
-    // Native scaling is preferred to sampling.  If we can scale natively to
-    // within one of the ideal value, we should choose to scale natively.
-    if (widthDiff < 1.0f && heightDiff < 1.0f) {
-        return nativeSize;
+    // Only JPEG supports native downsampling.
+    if (fCodec->getEncodedFormat() == kJPEG_SkEncodedFormat) {
+        // See if libjpeg supports this scale directly
+        switch (sampleSize) {
+            case 2:
+            case 4:
+            case 8:
+                // This class does not need to do any sampling.
+                *sampleSizePtr = 1;
+                return fCodec->getScaledDimensions(get_scale_from_sample_size(sampleSize));
+            default:
+                break;
+        }
+
+        // Check if sampleSize is a multiple of something libjpeg can support.
+        int remainder;
+        for (int supportedSampleSize : { 8 , 4 , 2 }) {
+            int actualSampleSize;
+            SkTDivMod(sampleSize, supportedSampleSize, &actualSampleSize, &remainder);
+            if (0 == remainder) {
+                float scale = get_scale_from_sample_size(supportedSampleSize);
+
+                // fCodec will scale to this size.
+                preSampledSize = fCodec->getScaledDimensions(scale);
+
+                // And then this class will sample it.
+                *sampleSizePtr = actualSampleSize;
+                if (nativeSampleSize) {
+                    *nativeSampleSize = supportedSampleSize;
+                }
+                break;
+            }
+        }
     }
 
-    // Provide the scaling by sampling.
-    return SkISize::Make(get_scaled_dimension(width, sampleSize),
-            get_scaled_dimension(height, sampleSize));
+    return preSampledSize;
+}
+
+SkISize SkSampledCodec::onGetSampledDimensions(int sampleSize) const {
+    const SkISize size = this->accountForNativeScaling(&sampleSize);
+    return SkISize::Make(get_scaled_dimension(size.width(), sampleSize),
+                         get_scaled_dimension(size.height(), sampleSize));
 }
 
 SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void* pixels,
@@ -59,7 +89,7 @@ SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void
 
     // We are performing a subset decode.
     int sampleSize = options.fSampleSize;
-    SkISize scaledSize = this->onGetSampledDimensions(sampleSize);
+    SkISize scaledSize = this->getSampledDimensions(sampleSize);
     if (!fCodec->dimensionsSupported(scaledSize)) {
         // If the native codec does not support the requested scale, scale by sampling.
         return this->sampledDecode(info, pixels, rowBytes, options);
@@ -107,31 +137,46 @@ SkCodec::Result SkSampledCodec::onGetAndroidPixels(const SkImageInfo& info, void
 
 SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pixels,
         size_t rowBytes, AndroidOptions& options) {
+    // We should only call this function when sampling.
+    SkASSERT(options.fSampleSize > 1);
+
     // Create options struct for the codec.
     SkCodec::Options sampledOptions;
     sampledOptions.fZeroInitialized = options.fZeroInitialized;
 
+    // FIXME: This was already called by onGetAndroidPixels. Can we reduce that?
+    int sampleSize = options.fSampleSize;
+    int nativeSampleSize;
+    SkISize nativeSize = this->accountForNativeScaling(&sampleSize, &nativeSampleSize);
+
     // Check if there is a subset.
     SkIRect subset;
     int subsetY = 0;
-    int subsetWidth = fCodec->getInfo().width();
-    int subsetHeight = fCodec->getInfo().height();
+    int subsetWidth = nativeSize.width();
+    int subsetHeight = nativeSize.height();
     if (options.fSubset) {
         // We will need to know about subsetting in the y-dimension in order to use the
         // scanline decoder.
+        // Update the subset to account for scaling done by fCodec.
         SkIRect* subsetPtr = options.fSubset;
-        subsetY = subsetPtr->y();
-        subsetWidth = subsetPtr->width();
-        subsetHeight = subsetPtr->height();
+
+        // Do the divide ourselves, instead of calling get_scaled_dimension. If
+        // X and Y are 0, they should remain 0, rather than being upgraded to 1
+        // due to being smaller than the sampleSize.
+        const int subsetX = subsetPtr->x() / nativeSampleSize;
+        subsetY = subsetPtr->y() / nativeSampleSize;
+
+        subsetWidth = get_scaled_dimension(subsetPtr->width(), nativeSampleSize);

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

Ahh this is where it doesn't work.  Because the native scaling may not match the
behavior of get_scaled_dimension().

I think I can convince myself that this does work properly when the subset is
not the entire image.  Because getSampledSubsetDimensions() also calls
get_scaled_dimension().

[scroggo, 2015/11/03 15:17:37]

I think there's another wrinkle here, too. Our sampling method is pretty
flexible. We recompute the sampleX and sampleY down below. I could imagine that
if we tried to use the sampleSize suggested by the caller (divided by whatever
native sampling was done e.g. for a sampleSize of 24, libjpeg does 8, so if we
set sampleX and sampleY to 3) the math may not work out.

[msarett, 2015/11/03 15:23:29]

Acknowledged.

+        subsetHeight = get_scaled_dimension(subsetPtr->height(), nativeSampleSize);
 
         // The scanline decoder only needs to be aware of subsetting in the x-dimension.
-        subset.setXYWH(subsetPtr->x(), 0, subsetWidth, fCodec->getInfo().height());
+        subset.setXYWH(subsetX, 0, subsetWidth, nativeSize.height());
         sampledOptions.fSubset = ⊂
     }
 
     // Start the scanline decode.
     SkCodec::Result result = fCodec->startScanlineDecode(
-            info.makeWH(fCodec->getInfo().width(), fCodec->getInfo().height()), &sampledOptions,
+            info.makeWH(nativeSize.width(), nativeSize.height()), &sampledOptions,
             options.fColorPtr, options.fColorCount);
     if (SkCodec::kSuccess != result) {
         return result;