Move all knowledge of X sampling into SkScaledCodec

src/codec/SkScaledCodec.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCodecPriv.h"
 #include "SkScaledCodec.h"
 #include "SkStream.h"
@@ skipping 82 matching lines @@
     int scaledHeight = get_scaled_dimension(this->getInfo().height(), sampleSize);
 
     // Return the calculated output dimensions for the given scale
     scaledCodecDimensions = SkISize::Make(scaledWidth, scaledHeight);
 
     return best_scaled_dimensions(this->getInfo().dimensions(), nativeDimensions,
                                   scaledCodecDimensions, desiredScale);
 }
 
 // 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;
     }
     // check that srcWidth is scaled down by an integer value
     if (get_scaled_dimension(srcWidth, *sampleX) != dstWidth) {
         return false;
     }
     // check that src height is scaled down by an integer value
     if (get_scaled_dimension(srcHeight, *sampleY) != dstHeight) {
         return false;
     }
     // sampleX and sampleY should be equal unless the original sampleSize requested was larger
     // than srcWidth or srcHeight. If so, the result of this is dstWidth or dstHeight = 1.
     // This functionality allows for tall thin images to still be scaled down by scaling factors.
     if (*sampleX != *sampleY){
         if (1 != dstWidth && 1 != dstHeight) {
             return false;
         }
     }
     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;
 
     // only support down sampling, not up sampling
     SkASSERT(dstWidth <= srcWidth);
     SkASSERT(dstHeight <= srcHeight);
 
     // sampleX and sampleY should be equal unless the original sampleSize requested was
     // larger than srcWidth or srcHeight.
@@ skipping 26 matching lines @@
 
 // TODO: Implement subsetting in onGetPixels which works when and when not sampling 
 
 SkCodec::Result SkScaledCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
                                            size_t rowBytes, const Options& options,
                                            SkPMColor ctable[], int* ctableCount) {
 
     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);
+    const 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);
+    const int dstHeight = requestedInfo.height();
+    const int srcWidth = fCodec->getInfo().width();
+    const int srcHeight = fCodec->getInfo().height();
 
-    // 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 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);
             if (kSuccess != result && kIncompleteInput != result) {
                 return result;
             }
             for (int y = 0; y < dstHeight; y++) {
                 result = fCodec->getScanlines(dst, 1, rowBytes);
                 if (kSuccess != result && kIncompleteInput != result) {
                     return result;
@@ skipping 40 matching lines @@
                 storagePtr += sampleY * rowBytes;
                 dst = SkTAddOffset<void>(dst, rowBytes);
             }
             return result;
         }
         default:
             SkASSERT(false);
             return kUnimplemented;
     }
 }