Move all knowledge of X sampling into SkScaledCodec

src/codec/SkScaledCodec.cpp

Index: src/codec/SkScaledCodec.cpp
diff --git a/src/codec/SkScaledCodec.cpp b/src/codec/SkScaledCodec.cpp
index 36aeda98cf495ff2a84f1103d0612bb26d91ada1..69c72cab331db087e79ea36651d00d7a4da8b367 100644
--- a/src/codec/SkScaledCodec.cpp
+++ b/src/codec/SkScaledCodec.cpp
@@ -100,13 +100,13 @@ SkISize SkScaledCodec::onGetScaledDimensions(float desiredScale) const {
 }
 
 // check if scaling to dstInfo size from srcInfo size using sampleSize is possible
-static bool scaling_supported(const SkImageInfo& dstInfo, const SkImageInfo& srcInfo,
+static bool scaling_supported(const SkISize& dstDim, const SkISize& srcDim,
                               int* sampleX, int* sampleY) {
-    SkScaledCodec::ComputeSampleSize(dstInfo, srcInfo, sampleX, sampleY);
-    const int dstWidth = dstInfo.width();
-    const int dstHeight = dstInfo.height();
-    const int srcWidth = srcInfo.width();
-    const int srcHeight = srcInfo.height();
+    SkScaledCodec::ComputeSampleSize(dstDim, srcDim, sampleX, sampleY);
+    const int dstWidth = dstDim.width();
+    const int dstHeight = dstDim.height();
+    const int srcWidth = srcDim.width();
+    const int srcHeight = srcDim.height();
      // only support down sampling, not up sampling
     if (dstWidth > srcWidth || dstHeight  > srcHeight) {
         return false;
@@ -130,13 +130,29 @@ static bool scaling_supported(const SkImageInfo& dstInfo, const SkImageInfo& src
     return true;
 }
 
+bool SkScaledCodec::onDimensionsSupported(const SkISize& dim) {
+    // Check with fCodec first. No need to call the non-virtual version, which
+    // just checks if it matches the original, since a match means this method
+    // will not be called.
+    if (fCodec->onDimensionsSupported(dim)) {
+        return true;
+    }
+
+    // FIXME: These variables are unused, but are needed by scaling_supported.
+    // This class could also cache these values, and avoid calling this in
+    // onGetPixels (since getPixels already calls it).
+    int sampleX;
+    int sampleY;
+    return scaling_supported(dim, this->getInfo().dimensions(), &sampleX, &sampleY);
+}
+
 // calculates sampleSize in x and y direction
-void SkScaledCodec::ComputeSampleSize(const SkImageInfo& dstInfo, const SkImageInfo& srcInfo,
+void SkScaledCodec::ComputeSampleSize(const SkISize& dstDim, const SkISize& srcDim,
                                       int* sampleXPtr, int* sampleYPtr) {
-    int srcWidth = srcInfo.width();
-    int dstWidth = dstInfo.width();
-    int srcHeight = srcInfo.height();
-    int dstHeight = dstInfo.height();
+    int srcWidth = srcDim.width();
+    int dstWidth = dstDim.width();
+    int srcHeight = srcDim.height();
+    int dstHeight = dstDim.height();
 
     int sampleX = srcWidth / dstWidth;
     int sampleY = srcHeight / dstHeight;
@@ -183,62 +199,43 @@ SkCodec::Result SkScaledCodec::onGetPixels(const SkImageInfo& requestedInfo, voi
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
-    } 
-
-    // FIXME: If no scaling/subsets are requested, we can call fCodec->getPixels.
-    Result result = fCodec->startScanlineDecode(requestedInfo, &options, ctable, ctableCount);
-    if (kSuccess == result) {
-        // native decode supported
-        switch (fCodec->getScanlineOrder()) {
-            case SkCodec::kTopDown_SkScanlineOrder:
-            case SkCodec::kBottomUp_SkScanlineOrder:
-            case SkCodec::kNone_SkScanlineOrder:
-                return fCodec->getScanlines(dst, requestedInfo.height(), rowBytes);
-            case SkCodec::kOutOfOrder_SkScanlineOrder: {
-                for (int y = 0; y < requestedInfo.height(); y++) {
-                    int dstY = fCodec->nextScanline();
-                    void* dstPtr = SkTAddOffset<void>(dst, rowBytes * dstY);
-                    result = fCodec->getScanlines(dstPtr, 1, rowBytes);
-                    // FIXME (msarett): Make the SkCodec base class take care of filling
-                    // uninitialized pixels so we can return immediately on kIncompleteInput.
-                    if (kSuccess != result && kIncompleteInput != result) {
-                        return result;
-                    }
-                }
-                return result;
-            }
-        }
     }
 
-    if (kInvalidScale != result) {
-        // no scaling requested
-        return result;
+    if (fCodec->dimensionsSupported(requestedInfo.dimensions())) {
+        return fCodec->getPixels(requestedInfo, dst, rowBytes, &options, ctable, ctableCount);

[msarett, 2015/10/01 19:34:34]

Unfortunately, I think I might change this back to scanline decoding when we add
subsets here.

[scroggo, 2015/10/01 21:16:57]

Well, it will have to do scanline decoding if there's a subset that is not
supported by fCodec, but otherwise I think we should call fCodec->getPixels.

     }
 
     // scaling requested
     int sampleX;
     int sampleY;
-    if (!scaling_supported(requestedInfo, fCodec->getInfo(), &sampleX, &sampleY)) {
+    if (!scaling_supported(requestedInfo.dimensions(), fCodec->getInfo().dimensions(),
+                           &sampleX, &sampleY)) {
         return kInvalidScale;

[msarett, 2015/10/01 19:34:34]

I think we should probably assert false here, since we've already checked that
native scaling or sampling is supported.

Alternatively, we could just return true in onDimensionsSupported(), since we
are going to redo the checks in here anyway.

[scroggo, 2015/10/01 21:16:57]

Done.
As another option, we could store fSampleX and fSampleY in
onDimensionsSupported, and never need to call it here.

     }
+
     // set first sample pixel in y direction
-    int Y0 = get_start_coord(sampleY);
-
-    int dstHeight = requestedInfo.height();
-    int srcHeight = fCodec->getInfo().height();
-    
-    SkImageInfo info = requestedInfo;
-    // use original height as codec does not support y sampling natively
-    info = info.makeWH(requestedInfo.width(), srcHeight);
-
-    // update codec with new info
-    // FIXME: The previous call to start returned kInvalidScale. This call may
-    // require a rewind. (skbug.com/4284)
-    result = fCodec->startScanlineDecode(info, &options, ctable, ctableCount);
+    const int Y0 = get_start_coord(sampleY);
+
+    const int dstHeight = requestedInfo.height();
+    const int srcWidth = fCodec->getInfo().width();
+    const int srcHeight = fCodec->getInfo().height();
+
+    const SkImageInfo info = requestedInfo.makeWH(srcWidth, srcHeight);
+
+    Result result = fCodec->startScanlineDecode(info, &options, ctable, ctableCount);
+
     if (kSuccess != result) {
         return result;
     }
 
+    SkSampler* sampler = fCodec->getSampler();
+    if (!sampler) {
+        // Or maybe this should be unimplemented?
+        return kInvalidScale;

[msarett, 2015/10/01 19:34:34]

I think it should be unimplemented.

[scroggo, 2015/10/01 21:16:57]

Done.

+    }
+
+    sampler->setSampleX(sampleX);
+
     switch(fCodec->getScanlineOrder()) {
         case SkCodec::kTopDown_SkScanlineOrder: {
             result = fCodec->skipScanlines(Y0);