SkScaledCodec class

src/codec/SkScaledCodec.cpp

Index: src/codec/SkScaledCodec.cpp
diff --git a/src/codec/SkScaledCodec.cpp b/src/codec/SkScaledCodec.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..30b048c06a97729881e048a28e9c598ddd62523b
--- /dev/null
+++ b/src/codec/SkScaledCodec.cpp
@@ -0,0 +1,158 @@
+/*
+ * 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"
+
+
+SkCodec* SkScaledCodec::NewFromStream(SkStream* stream) {
+	SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream));

[scroggo, 2015/07/27 15:09:19]

This should be indented only four characters.

A lot of text editors let you customize how many character indents it should
use. I'm not familiar with Sublime (which I believe you are using), but it
should have a way to set it (I'm guessing it is fighting with you to choose the
spacing?).

[emmaleer, 2015/07/27 18:31:38]

Yes, it's usually set to 4 spaced per indent.. I think because I created a new
file it defaulted to 8. I've fixed the spacing issues now. 

+	if (NULL == codec) {
+		return NULL;
+	}
+	return SkNEW_ARGS(SkScaledCodec, (codec.detach()));

[scroggo, 2015/07/27 15:09:19]

Since SkWebpCodec supports subsets and scaling natively, I think it would make
sense to check the encoded type, and return the codec itself if it is webp.

[emmaleer, 2015/07/27 18:31:38]

Acknowledged.

+}
+
+SkCodec* SkScaledCodec::NewFromData(SkData* data) {
+    if (!data) {
+        return NULL;
+    }
+    return NewFromStream(SkNEW_ARGS(SkMemoryStream, (data)));
+}
+
+SkScaledCodec::SkScaledCodec(SkCodec* codec)
+    : INHERITED(codec->getInfo(), NULL)
+    , fCodec(codec)
+{}
+
+SkScaledCodec::~SkScaledCodec() {}
+
+/*
+ * Return a valid set of output dimensions for this decoder, given an input scale
+ * TODO: This should be the default implementation once the other codecs support this.

[scroggo, 2015/07/27 15:09:19]

This TODO is no longer needed.

[emmaleer, 2015/07/27 18:31:38]

Acknowledged.

+ */
+SkISize SkScaledCodec::onGetScaledDimensions(float desiredScale) const {
+    // support scaling down by integer numbers. Ex: 1/2, 1/3, 1/4 ...
+
+    if (desiredScale > 0.5f) {
+        // sampleSize = 1
+        return fCodec->getInfo().dimensions();
+    }
+    // sampleSize determines the step size between samples
+    // Ex: sampleSize = 2, sample every second pixel in x and y directions
+    int sampleSize = 1 / desiredScale;
+
+    // if sampleSize > width or height, will only sample 1 pixel in that direction
+    int sampleX = SkMin32(sampleSize, this->getInfo().width());
+    int sampleY = SkMin32(sampleSize, this->getInfo().height());
+
+    int scaledWidth = this->getInfo().width() / sampleX;
+    int scaledHeight = this->getInfo().height() / sampleY;
+
+    // Return the calculated output dimensions for the given scale
+    return SkISize::Make(scaledWidth, scaledHeight);
+}
+
+// check if scaling to dstInfo size from srcInfo size is possible
+static bool scaling_supported(const SkImageInfo&  dstInfo, const SkImageInfo& srcInfo){
+    const int dstWidth = dstInfo.width();
+    const int dstHeight = dstInfo.height();
+    const int srcWidth = srcInfo.width();
+    const int srcHeight = srcInfo.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
+    int sampleSizeX = srcWidth / dstWidth;
+    if (srcWidth / sampleSizeX != dstWidth) {
+        return false;
+    }
+    // check that src height is scaled down by an integer value
+    int sampleSizeY = srcHeight / dstHeight;
+    if (srcHeight / sampleSizeY != 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 (sampleSizeX != sampleSizeY){
+        if (1 != dstWidth && 1 != dstHeight) {
+            return false;
+        }
+    }
+    return true;
+}
+
+SkCodec::Result SkScaledCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
+                                           size_t rowBytes, const Options& options,
+                                           SkPMColor ctable[], int* ctableCount) {
+	
+	Result result = fCodec->getPixels(requestedInfo, dst, rowBytes, &options, ctable, ctableCount);
+	if (kInvalidScale != result) {

[scroggo, 2015/07/27 15:09:19]

I wonder if there are other cases this should handle in the future.

Once we add subset decoding to this class, it needs to consider a return value
that indicated that fCodec could not handle the subset. Currently, we return
kUnimplemented. (I think it is currently only used for subsets, but if we wanted
to use it for something else, we may need to distinguish between types of
unimplemented.)

On the other hand, if the factory returns an SkWebpCodec directly, then fCodec
will never support subsetting, so we could skip calling fCodec->getPixels if
options.fSubset is not NULL (not sure whether adding that check would be better
than just waiting for getPixels to return kUnimplemented, though).

[emmaleer, 2015/07/27 18:31:38]

Maybe we should add a return value of kInvalidSubset
We could check if options.fSubset == NULL before calling getPixels
Calling getPixels is not very expensive if it fails, as it only check a bunch of
if statements at the beginning.
That being said, it would still be faster without those if statements.

I'm wondering about the case where we are sampling and request a subset (is this
going be be supported?)
Maybe we should wait until implementing Subset Decoding to worry about this. 

[scroggo, 2015/07/27 19:29:57]

Yes. This is currently supported by BitmapRegionDecoder, so we need to support
it somehow.
Agreed, but it would be nice to have a TODO/FIXME.

[emmaleer, 2015/07/28 14:19:16]

Acknowledged.

+        // no scaling requested
+		return result;
+	}
+	// scaling requested
+
+	if (!scaling_supported(requestedInfo, fCodec->getInfo())) {
+		return kInvalidScale;
+	}
+
+    bool isInterlaced = fCodec->isInterlaced();
+    Options scaledOptions = options;
+    scaledOptions.fScaled = true;
+
+    int dstHeight = requestedInfo.height();
+    int srcHeight = fCodec->getInfo().height();
+    
+    SkImageInfo info = requestedInfo;
+    if (isInterlaced){
+        // must decode entire height for interlaced, then scale after
+        info = info.makeWH(requestedInfo.width(), srcHeight);
+    }
+
+	SkAutoTDelete<SkScanlineDecoder> scanlineDecoder(fCodec->getScanlineDecoder(info, 
+													 &scaledOptions, ctable, ctableCount));
+    if (NULL == scanlineDecoder) {
+        SkCodecPrintf("failed to create scanline decoder.\n");
+        return kInvalidInput;
+    }
+
+    // set sample values in x and y directions
+    fSampleY = srcHeight / dstHeight;
+    fY0 = fSampleY >> 1;
+    int sampleX = fCodec->getInfo().width() / requestedInfo.width();
+    if (!scanlineDecoder->setSampleX(sampleX)) {
+        return kInvalidScale;
+    }
+    
+    if (!isInterlaced) {

[scroggo, 2015/07/27 15:09:19]

nit: I tend to prefer:

if (condition) {

} else {
  // !condition
}

over:

if (!condition) {

} else {
  // condition
}

Not sure whether others have this preference, but I find it more readable that
way.

[emmaleer, 2015/07/27 18:31:38]

Acknowledged.

+        scanlineDecoder->skipScanlines(fY0);
+        for (int y = 0; y < dstHeight; y++) {
+            scanlineDecoder->getScanlines(dst, 1, rowBytes);
+            if (y < dstHeight - 1) {
+                scanlineDecoder->skipScanlines(fSampleY - 1);
+            }
+            dst += rowBytes;
+        }
+    } else {
+        // interlaced
+        SkAutoMalloc storage(srcHeight * rowBytes);
+        void* storagePtr = storage.get();
+        result = scanlineDecoder->getScanlines(storagePtr, srcHeight, rowBytes);
+
+        storagePtr += fY0 * rowBytes;
+        for (int y = 0; y < dstHeight; y++) {
+            memcpy(dst, storagePtr, rowBytes);
+            storagePtr += fSampleY * rowBytes;
+            dst += rowBytes;
+        }
+    }
+    return kSuccess;
+}