SkScaledCodec class

include/codec/SkScaledCodec.h

+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#ifndef SkScaledCodec_DEFINED
+#define SkScaledCodec_DEFINED
+
+#include "SkCodec.h"
+#include "SkScanlineDecoder.h"
+
+class SkScanlineDecoder;
+class SkStream;
+
+/**
+ * This class implements scaling, by sampling scanlines in the y direction.
+ * x-wise sampling is implemented in the swizzler, when getScanlines() is called.
+ */
+class SkScaledCodec : public SkCodec {
+public:
+    static SkCodec* NewFromStream(SkStream*);
+    static SkCodec* NewFromData(SkData*);
+
+    virtual ~SkScaledCodec();
+
+    /**
+     * returns whether a destination's dimensions are supported for down sampling
+     */
+    static bool DimensionsSupportedForSampling(const SkImageInfo& srcInfo, 
+                                               const SkImageInfo& dstInfo) {
+        // heights must be equal as no native y sampling is supported
+        if (dstInfo.height() != srcInfo.height()) {
+            return false;
+        }
+        // only support down sampling, dstWidth cannot be larger that srcWidth
+        if(dstInfo.width() > srcInfo.width()) {
+            return false;
+        }
+        return true;   
+    }
+
+    // returns a scaled dimension based on the original dimension and the sampleSize
+    // NOTE: we round down here for scaled dimension to match the behavior of SkImageDecoder
+    static int getScaledDimension(int srcDimension, int sampleSize) {

[msarett, 2015/08/13 15:45:10]

nit: GetScaledDimension because it is static.

Leon, what is your opinion about a member function that does not use any fields?

It is here in the header file because it is needed here and also in the cpp
file.  Maybe it can moved out of the class but still in this file?  It could go
to SkCodecPriv but it seems to me that it belongs with SkScaledCodec.

[scroggo, 2015/08/13 16:10:45]

In general, that is a sign it may not need to be a member function. If this
function were used by other classes, I think it would be fine to put it here for
context (i.e. so callers will have to call SkScaledCodec::GetScaledDimensions).
That said, it looks like it is only used by SkScaledCodec, and by the SampleSize
constructor. If the SampleSize constructor were defined in the implementation
file, this would not be needed here. So I would say move that calculation to
.cpp, and move this back to cpp as a static function.

[emmaleer, 2015/08/13 17:49:11]

This is no longer a member function.

+        if (sampleSize > srcDimension) {
+            return 1;
+        }
+        return srcDimension / sampleSize;
+    }
+
+    struct SampleSize {

[scroggo, 2015/08/13 16:10:45]

I do not think this object is necessary. It has a constructor which can
potentially create bad values, and it is really only used to do some
calculations. I think it would be better to just have a function that does this:

static bool ComputeSampleSize(const SkImageInfo& srcInfo,
    const SkImageInfo& dstInfo, int* sampleX, int* sampleY)
{
  // Do calculation...
  if (/* not valid */) {
    return false;
  }

  if (sampleX) {
    *sampleX = <value computed above>;
  }
  if (sampleY) {
    *sampleY = <value computed above>;
  }

  return true;
}

[emmaleer, 2015/08/13 17:49:11]

Okay, I like that way better too.

+        int sampleX;    // sampleSize in the x direction
+        int sampleY;    // sampleSize in the y direction
+
+        // calculates sampleSize in x and y direction
+        SampleSize(const SkImageInfo& srcInfo, const SkImageInfo& dstInfo) {
+            int srcWidth = srcInfo.width();
+            int dstWidth = dstInfo.width();
+            int srcHeight = srcInfo.height();
+            int dstHeight = dstInfo.height();
+
+            sampleX = srcWidth / dstWidth;
+            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.
+            // 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) {
+
+                    // rounding during onGetScaledDimensions can cause different sampleSizes
+                    // Ex: srcWidth = 79, srcHeight = 20, sampleSize = 10
+                    // dstWidth = 7, dstHeight = 2, sampleX = 79/7 = 11, sampleY = 20/2 = 10
+                    // correct for this rounding by comparing width to sampleY and height to sampleX
+
+                    if (getScaledDimension(srcWidth, sampleY) == dstWidth) {
+                        sampleX = sampleY;
+                    } else if (getScaledDimension(srcHeight, sampleX) == dstHeight) {
+                        sampleY = sampleX;
+                    }
+                }
+            }
+        }
+    };
+
+protected:
+    /**
+     * Recommend a set of destination dimensions given a requested scale
+     */
+    SkISize onGetScaledDimensions(float desiredScale) const override;
+
+    Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, SkPMColor*, int*)
+            override;
+    SkEncodedFormat onGetEncodedFormat() const override {
+        return fScanlineDecoder->getEncodedFormat();
+    }
+
+    bool onReallyHasAlpha() const override {
+        return fScanlineDecoder->reallyHasAlpha();
+    }
+
+private:
+
+    SkAutoTDelete<SkScanlineDecoder> fScanlineDecoder;
+
+    explicit SkScaledCodec(SkScanlineDecoder*);
+
+    typedef SkCodec INHERITED;
+};
+#endif // SkScaledCodec_DEFINED