SkScaledCodec class

src/codec/SkCodec_libpng.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 "SkCodec_libpng.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
@@ skipping 406 matching lines @@
     }
     // Check for supported color types
     switch (dst.colorType()) {
         case kN32_SkColorType:
             return true;
         default:
             return dst.colorType() == src.colorType();
     }
 }
 
+bool SkPngCodec::onIsInterlaced() {
+    if (1 == png_set_interlace_handling(fPng_ptr)){

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

You actually do not need a conditional here. You can just say:

return 1 != png_set_interlace_handling(fPng_ptr);

Maybe add a comment that we can not depend on fNumberPasses, since it may not
have been set yet.

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

Acknowledged.

+        return false;
+    } else {
+        return true;
+    }
+}
+
 SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
                                                void* dst, size_t rowBytes,
                                                const Options& options,
                                                SkPMColor ctable[],
                                                int* ctableCount) {
     // FIXME: Could we use the return value of setjmp to specify the type of
     // error?
     if (setjmp(png_jmpbuf(fPng_ptr))) {
         SkCodecPrintf("setjmp long jump!\n");
         return kInvalidInput;
@@ skipping 149 matching lines @@
     }
 
     // read rest of file, and get additional comment and time chunks in info_ptr
     png_read_end(fPng_ptr, fInfo_ptr);
     return kSuccess;
 }
 
 class SkPngScanlineDecoder : public SkScanlineDecoder {
 public:
     SkPngScanlineDecoder(const SkImageInfo& dstInfo, SkPngCodec* codec)
-        : INHERITED(dstInfo)
+        : INHERITED(dstInfo, codec->getInfo())
         , fCodec(codec)
         , fHasAlpha(false)
     {
-        fStorage.reset(dstInfo.width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig));
+        fStorage.reset(fCodec->getInfo().width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig));
         fSrcRow = static_cast<uint8_t*>(fStorage.get());
     }
 
+    bool onSetSampleX(int sampleX) override {
+        fCodec->fSwizzler->setSampleX(sampleX);
+        return true;
+    }
+
     SkCodec::Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
             return SkCodec::kInvalidInput;
         }
 
         for (int i = 0; i < count; i++) {
             png_read_rows(fCodec->fPng_ptr, &fSrcRow, png_bytepp_NULL, 1);
             fCodec->fSwizzler->setDstRow(dst);
             fHasAlpha |= !SkSwizzler::IsOpaque(fCodec->fSwizzler->next(fSrcRow));
@@ skipping 26 matching lines @@
     SkAutoMalloc                fStorage;
     uint8_t*                    fSrcRow;
 
     typedef SkScanlineDecoder INHERITED;
 };
 
 
 class SkPngInterlacedScanlineDecoder : public SkScanlineDecoder {
 public:
     SkPngInterlacedScanlineDecoder(const SkImageInfo& dstInfo, SkPngCodec* codec)
-        : INHERITED(dstInfo)
+        : INHERITED(dstInfo, codec->getInfo())
         , fCodec(codec)
         , fHasAlpha(false)
         , fCurrentRow(0)
         , fHeight(dstInfo.height())
     {
-        fSrcRowBytes = dstInfo.width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig);
+        fSrcRowBytes = codec->getInfo().width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig);
         fGarbageRow.reset(fSrcRowBytes);
         fGarbageRowPtr = static_cast<uint8_t*>(fGarbageRow.get());
     }
 
+    bool onSetSampleX(int sampleX) override {
+        fCodec->fSwizzler->setSampleX(sampleX);
+        return true;
+    }
+
     SkCodec::Result onGetScanlines(void* dst, int count, size_t dstRowBytes) override {
         //rewind stream if have previously called onGetScanlines, 
         //since we need entire progressive image to get scanlines
         if (!fCodec->handleRewind()) {
             return SkCodec::kCouldNotRewind;
         }
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
             return SkCodec::kInvalidInput;
         }
@@ skipping 50 matching lines @@
 };
 
 
 SkScanlineDecoder* SkPngCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
         const Options& options, SkPMColor ctable[], int* ctableCount) {
     if (!conversion_possible(dstInfo, this->getInfo())) {
         SkCodecPrintf("no conversion possible\n");
         return NULL;
     }
     // Check to see if scaling was requested.
-    if (dstInfo.dimensions() != this->getInfo().dimensions()) {
-        return NULL;
+    if(options.fScaled == false) {

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

Please add a comment explaining this. Something like:

// If the caller is sampling, it is okay for the dimensions to not match.

(Which makes me wonder if the variable should be called fSampled?)

That said, IIUC, the width may be smaller (due to sampling), but it needs to be
a supported width, and the height should be the same?

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

Changed to fSampled.

For normal images the dstHeight and dstWidth can both be smaller. These are the
dimensions the user requests. They are checked in SkScaledCodec.cpp in
onGetPixels() by calling scaling_supported()

For interlaced images, the dstHeight must be the same. (as we do the y sampling
after)
In SkScaledCodec we set the dstHeight = srcHeight if the image is interlaced,
before creating the scanline decoder.

I could add a check saying: If interlaced and sampled, make sure dstHeight and
srcHeight are equal.
But since SkScaledCodec previously set them equal, I don't think it's really
necessary.
Let me know if you think it's still beneficial, and I can add the check. 

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

Does the SkCodec need to know that dstHeight is smaller though? It seems like
from this SkCodec's perspective, the dstHeight is unchanged - the caller needs
to call skip/get scanline for each scanline in the original height, right?
If there is a condition that we assume in a function, then that is a place we
can add an assert. It checks that you were right about your assumption, and
allows a reader to know you assume that.

That said, the info passed in comes from a public function, so someone could
call with an invalid value, right?

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

Yes, I have changed the height to be the original height
I added a check that returns NULL if the heights are no equal when scaling

+        if (dstInfo.dimensions() != this->getInfo().dimensions()) {
+            return NULL;
+        }
     }
     // Create a new SkPngCodec, to be owned by the scanline decoder.
     SkStream* stream = this->stream()->duplicate();
     if (!stream) {
         return NULL;
     }
     SkAutoTDelete<SkPngCodec> codec (static_cast<SkPngCodec*>(SkPngCodec::NewFromStream(stream)));
     if (!codec) {
         return NULL;
     }
 
     // Note: We set dst to NULL since we do not know it yet. rowBytes is not needed,
     // since we'll be manually updating the dstRow, but the SkSwizzler requires it to
     // be at least dstInfo.minRowBytes.
     if (codec->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options, ctable,
             ctableCount) != kSuccess) {
         SkCodecPrintf("failed to initialize the swizzler.\n");
         return NULL;
     }
 
     SkASSERT(codec->fNumberPasses != INVALID_NUMBER_PASSES);
     if (codec->fNumberPasses > 1) {
         // interlaced image
         return SkNEW_ARGS(SkPngInterlacedScanlineDecoder, (dstInfo, codec.detach()));
     }
 
     return SkNEW_ARGS(SkPngScanlineDecoder, (dstInfo, codec.detach()));
 }