Swizzling, except for premultiplying, done using libpng transforms

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 196 matching lines @@
     }
     return true;
 }
 
 // Reads the header, and initializes the passed in fields, if not NULL (except
 // stream, which is passed to the read function).
 // Returns true on success, in which case the caller is responsible for calling
 // png_destroy_read_struct. If it returns false, the passed in fields (except
 // stream) are unchanged.
 static bool read_header(SkStream* stream, png_structp* png_ptrp,
-                        png_infop* info_ptrp, SkImageInfo* imageInfo) {
+                        png_infop* info_ptrp, SkImageInfo* imageInfo, int* bitDepthPtr) {
     // The image is known to be a PNG. Decode enough to know the SkImageInfo.
     png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL,
                                                  sk_error_fn, sk_warning_fn);
     if (!png_ptr) {
         return false;
     }
 
     AutoCleanPng autoClean(png_ptr);
 
     png_infop info_ptr = png_create_info_struct(png_ptr);
@@ skipping 13 matching lines @@
 
     // FIXME: This is where the old code hooks up the Peeker. Does it need to
     // be set this early? (i.e. where are the user chunks? early in the stream,
     // potentially?)
     // If it does, we need to figure out a way to set it here.
 
     // The call to png_read_info() gives us all of the information from the
     // PNG file before the first IDAT (image data chunk).
     png_read_info(png_ptr, info_ptr);
     png_uint_32 origWidth, origHeight;
     int bitDepth, colorType;

[scroggo, 2015/06/30 20:50:03]

No need to create this variable (bitDepth). Instead, you can pass bitDepthPtr
directly to png_get_IHDR. (Come to think of it though, I think you pass NULL for
bitDepthPtr in one of the calls to this method. In that case, this might be
helpful, if png_get_IHDR does not handle NULL gracefully.)

     png_get_IHDR(png_ptr, info_ptr, &origWidth, &origHeight, &bitDepth,
                  &colorType, int_p_NULL, int_p_NULL, int_p_NULL);
 
+    bitDepthPtr = &bitDepth;

[scroggo, 2015/06/30 20:50:03]

This is going to do the wrong thing. I believe you want to assign the value of
bitDepth to the variable pointed to by bitDepthPtr. In order to do that you
would do the following:

if (bitDepthPtr) { // don't forget to check for NULL
  *bitDepthPtr = bitDepth;
}

What you've done here is made bitDepthPtr point to the memory address of
bitDepth. The variable it used to point to remains unchanged.

Let me know if this does not make sense and we can talk about it more in person.

+
     // sanity check for size
     {
         int64_t size = sk_64_mul(origWidth, origHeight);
         // now check that if we are 4-bytes per pixel, we also don't overflow
         if (size < 0 || size > (0x7FFFFFFF >> 2)) {
             return false;
         }
     }
 
     // Tell libpng to strip 16 bit/color files down to 8 bits/color
     if (bitDepth == 16) {
         png_set_strip_16(png_ptr);
     }
 #ifdef PNG_READ_PACK_SUPPORTED
     // Extract multiple pixels with bit depths of 1, 2, and 4 from a single
     // byte into separate bytes (useful for paletted and grayscale images).
     if (bitDepth < 8) {
         png_set_packing(png_ptr);
     }
 #endif
     // Expand grayscale images to the full 8 bits from 1, 2, or 4 bits/pixel.
     if (colorType == PNG_COLOR_TYPE_GRAY && bitDepth < 8) {
         png_set_expand_gray_1_2_4_to_8(png_ptr);
     }
 
-
-    // Now determine the default SkColorType and SkAlphaType.
+    // Now determine the default SkColorType and SkAlphaType and set required transforms
     SkColorType skColorType;
     SkAlphaType skAlphaType;
     switch (colorType) {
         case PNG_COLOR_TYPE_PALETTE:
             skColorType = kIndex_8_SkColorType;
             skAlphaType = has_transparency_in_palette(png_ptr, info_ptr) ?
                     kUnpremul_SkAlphaType : kOpaque_SkAlphaType;
             break;
-        case PNG_COLOR_TYPE_GRAY:
-            if (false) {
-                // FIXME: Is this the wrong default behavior? This means if the
-                // caller supplies the info we gave them, they'll get Alpha 8.
-                skColorType = kAlpha_8_SkColorType;
-                // FIXME: Strangely, the canonical type for Alpha 8 is Premul.
-                skAlphaType = kPremul_SkAlphaType;
-            } else {
-                skColorType = kN32_SkColorType;
-                skAlphaType = kOpaque_SkAlphaType;
-            }
-            break;
-        default:
-            // Note: This *almost* mimics the code in SkImageDecoder_libpng.
-            // has_transparency_in_palette makes an additional check - whether
-            // numTrans is greater than 0. Why does the other code not make that
-            // check?
-            if (has_transparency_in_palette(png_ptr, info_ptr)
-                || PNG_COLOR_TYPE_RGB_ALPHA == colorType
-                || PNG_COLOR_TYPE_GRAY_ALPHA == colorType)
-            {
+        case PNG_COLOR_TYPE_RGB:
+            if (has_transparency_in_palette(png_ptr, info_ptr)) {
+                //convert to RGBA with tranparency information in tRNS chunk if it exists
+                png_set_tRNS_to_alpha(png_ptr);
                 skAlphaType = kUnpremul_SkAlphaType;
             } else {
+                //convert to RGBA with Opaque Alpha 
+                png_set_filler(png_ptr, 0xff, PNG_FILLER_AFTER);
                 skAlphaType = kOpaque_SkAlphaType;
             }
             skColorType = kN32_SkColorType;
             break;
-    }
-
-    {
-        // FIXME: Again, this block needs to go into onGetPixels.
-        bool convertGrayToRGB = PNG_COLOR_TYPE_GRAY == colorType && skColorType != kAlpha_8_SkColorType;
-
-        // Unless the user is requesting A8, convert a grayscale image into RGB.
-        // GRAY_ALPHA will always be converted to RGB
-        if (convertGrayToRGB || colorType == PNG_COLOR_TYPE_GRAY_ALPHA) {
+        case PNG_COLOR_TYPE_GRAY:
+            if (has_transparency_in_palette(png_ptr, info_ptr)) {
+                //FIXME: support gray with alpha as a color type
+                //convert to RGBA if there is transparentcy info in the tRNS chunk
+                png_set_tRNS_to_alpha(png_ptr);
+                png_set_gray_to_rgb(png_ptr);
+                skColorType = kN32_SkColorType;
+                skAlphaType = kUnpremul_SkAlphaType;
+            } else {
+                skColorType = kGray_8_SkColorType;
+                skAlphaType = kOpaque_SkAlphaType;
+            }
+            break;
+        case PNG_COLOR_TYPE_GRAY_ALPHA:
+            //FIXME: support gray with alpha as a color type 
+            //convert to RGBA 
             png_set_gray_to_rgb(png_ptr);
-        }
-
-        // Add filler (or alpha) byte (after each RGB triplet) if necessary.
-        // FIXME: It seems like we could just use RGB as the SrcConfig here.
-        if (colorType == PNG_COLOR_TYPE_RGB || convertGrayToRGB) {
-            png_set_filler(png_ptr, 0xff, PNG_FILLER_AFTER);
-        }
+            skColorType = kN32_SkColorType;
+            skAlphaType = kUnpremul_SkAlphaType;
+            break;
+        case PNG_COLOR_TYPE_RGBA:
+            skColorType = kN32_SkColorType;
+            skAlphaType = kUnpremul_SkAlphaType;
+            break;
+        default:
+            //all the color types have been covered above
+            SkASSERT(false);
     }
 
     // FIXME: Also need to check for sRGB (skbug.com/3471).
 
     if (imageInfo) {
-        *imageInfo = SkImageInfo::Make(origWidth, origHeight, skColorType,
-                                       skAlphaType);
+        *imageInfo = SkImageInfo::Make(origWidth, origHeight, skColorType, skAlphaType);
     }
     autoClean.detach();
     if (png_ptrp) {
         *png_ptrp = png_ptr;
     }
     if (info_ptrp) {
         *info_ptrp = info_ptr;
     }
     return true;
 }
 
 SkCodec* SkPngCodec::NewFromStream(SkStream* stream) {
     SkAutoTDelete<SkStream> streamDeleter(stream);
     png_structp png_ptr;
     png_infop info_ptr;
     SkImageInfo imageInfo;
-    if (read_header(stream, &png_ptr, &info_ptr, &imageInfo)) {
-        return SkNEW_ARGS(SkPngCodec, (imageInfo, streamDeleter.detach(), png_ptr, info_ptr));
+    int bitDepth;
+    if (read_header(stream, &png_ptr, &info_ptr, &imageInfo, &bitDepth)) {
+        return SkNEW_ARGS(SkPngCodec, (imageInfo, streamDeleter.detach(), 
+                png_ptr, info_ptr, bitDepth));
     }
     return NULL;
 }
 
 #define INVALID_NUMBER_PASSES -1
 SkPngCodec::SkPngCodec(const SkImageInfo& info, SkStream* stream,
-                       png_structp png_ptr, png_infop info_ptr)
+                       png_structp png_ptr, png_infop info_ptr, int bitDepth)
     : INHERITED(info, stream)
     , fPng_ptr(png_ptr)
     , fInfo_ptr(info_ptr)
     , fSrcConfig(SkSwizzler::kUnknown)
     , fNumberPasses(INVALID_NUMBER_PASSES)
     , fReallyHasAlpha(false)
+    , fBitDepth(bitDepth)
 {}
 
 SkPngCodec::~SkPngCodec() {
     this->destroyReadStruct();
 }
 
 void SkPngCodec::destroyReadStruct() {
     if (fPng_ptr) {
         // We will never have a NULL fInfo_ptr with a non-NULL fPng_ptr
         SkASSERT(fInfo_ptr);
@@ skipping 24 matching lines @@
         switch (dst.alphaType()) {
             case kPremul_SkAlphaType:
             case kUnpremul_SkAlphaType:
                 // The source is not opaque, so either of these is okay
                 break;
             default:
                 // We cannot decode a non-opaque image to opaque (or unknown)
                 return false;
         }
     }
-
     // Check for supported color types
     switch (dst.colorType()) {
-        // Allow output to kN32 from any type of input
-        case kN32_SkColorType:
+        case kRGBA_8888_SkColorType:
+            return true;
+        case kBGRA_8888_SkColorType:
             return true;
         default:
             return dst.colorType() == src.colorType();
     }
 }
 
 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;
     }
 
-    // FIXME: We already retrieved this information. Store it in SkPngCodec?
-    png_uint_32 origWidth, origHeight;
-    int bitDepth, pngColorType, interlaceType;
-    png_get_IHDR(fPng_ptr, fInfo_ptr, &origWidth, &origHeight, &bitDepth,
-                 &pngColorType, &interlaceType, int_p_NULL, int_p_NULL);
-
-    fNumberPasses = (interlaceType != PNG_INTERLACE_NONE) ?
-            png_set_interlace_handling(fPng_ptr) : 1;
-
     // Set to the default before calling decodePalette, which may change it.
     fReallyHasAlpha = false;
-    if (PNG_COLOR_TYPE_PALETTE == pngColorType) {
-        fSrcConfig = SkSwizzler::kIndex;
-        if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType(), bitDepth,
-                ctableCount)) {
-            return kInvalidInput;
-        }
-    } else if (kAlpha_8_SkColorType == requestedInfo.colorType()) {
-        // Note: we check the destination, since otherwise we would have
-        // told png to upscale.
-        SkASSERT(PNG_COLOR_TYPE_GRAY == pngColorType);
-        fSrcConfig = SkSwizzler::kGray;
-    } else if (this->getInfo().alphaType() == kOpaque_SkAlphaType) {
-        fSrcConfig = SkSwizzler::kRGBX;
-    } else {
-        fSrcConfig = SkSwizzler::kRGBA;
-    }
+
+    //srcColorType was determined in readHeader() which determined png color type
+    const SkColorType srcColorType = this->getInfo().colorType();
+    const SkColorType dstColorType = requestedInfo.colorType();
+        //set swizzler fSrcConfig to dstColorType, as libbpng will convert from 

[scroggo, 2015/06/30 20:50:03]

nit: Maybe specify that libpng will do the conversion thanks to the transforms
we have specified?

+        //srcColorType to dstColorType.  Swizzler still needed for premultiplying

[scroggo, 2015/06/30 20:50:04]

The swizzler is only needed in one case: the client requested kPremul.

If the image is opaque (and known to be) or if the client requested unpremul, we
should skip the swizzler entirely.

+        switch (dstColorType) {

[scroggo, 2015/06/30 20:50:03]

nit: This (and the comments above) should line up with the code above it (4
spaces, rather than 8)

+        case kIndex_8_SkColorType:
+            //decode palette to Skia format
+            fSrcConfig = SkSwizzler::kIndex;
+            if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType(), fBitDepth,
+                    ctableCount)) {
+                return kInvalidInput;
+            }
+            break;
+        case kGray_8_SkColorType:
+            fSrcConfig = SkSwizzler::kGray;
+            break; 
+        case kRGBA_8888_SkColorType:
+            if (this->getInfo().alphaType() == kOpaque_SkAlphaType) {
+                    fSrcConfig = SkSwizzler::kRGBX;
+                } else {
+                    fSrcConfig = SkSwizzler::kRGBA;
+            }
+        default:
+        //dstColorType == kBGRA_8888_SkColorType

[scroggo, 2015/06/30 20:50:03]

Again, I think it would be better to make this a case, and then handle the rest
in default.

(It seems like we have not covered all the cases though. Maybe the default
should be SkASSERT(false) with a comment that we would have exited before now if
the colortype is one of the remaining cases.)

+            if (this->getInfo().alphaType() == kOpaque_SkAlphaType) {
+                fSrcConfig = SkSwizzler::kBGRX;
+            } else {
+                fSrcConfig = SkSwizzler::kBGRA;
+            }
+    }  
 
     // Copy the color table to the client if they request kIndex8 mode
     copy_color_table(requestedInfo, fColorTable, ctable, ctableCount);
 
     // Create the swizzler.  SkPngCodec retains ownership of the color table.
     const SkPMColor* colors = fColorTable ? fColorTable->readColors() : NULL;
     fSwizzler.reset(SkSwizzler::CreateSwizzler(fSrcConfig, colors, requestedInfo,
             dst, rowBytes, options.fZeroInitialized));
     if (!fSwizzler) {
         // FIXME: CreateSwizzler could fail for another reason.
         return kUnimplemented;
     }
-
-    // FIXME: Here is where we should likely insert some of the modifications
-    // made in the factory.
-    png_read_update_info(fPng_ptr, fInfo_ptr);
-
     return kSuccess;
 }
 
-bool SkPngCodec::handleRewind() {
+SkCodec::Result SkPngCodec::setDataFormat(const SkImageInfo& requestedInfo) {
+    if (setjmp(png_jmpbuf(fPng_ptr))) {
+        SkCodecPrintf("setjmp long jump in SkPngCodec::setDataFormat()\n");
+        return kInvalidInput;
+    }
+
+    fNumberPasses = png_set_interlace_handling(fPng_ptr);
+
+     //srcColorType was determined in readHeader() which determined png color type
+    const SkColorType srcColorType = this->getInfo().colorType();
+    const SkColorType dstColorType = requestedInfo.colorType();
+    if (kRGBA_8888_SkColorType == dstColorType || kBGRA_8888_SkColorType == dstColorType) {
+        if (kIndex_8_SkColorType == srcColorType) {
+            //convert paletted images to RGBA
+            png_set_palette_to_rgb(fPng_ptr);
+            if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {

[scroggo, 2015/06/30 20:50:03]

Should this call has_transparency_in_palette, too?

+                png_set_tRNS_to_alpha(fPng_ptr);
+            } else {
+                png_set_filler(fPng_ptr, 0xff, PNG_FILLER_AFTER);
+            }
+        } else if (kGray_8_SkColorType == srcColorType) {
+            //convert from Gray to RGBA (need to add filler for alpha as gray has no alpha)
+            png_set_gray_to_rgb(fPng_ptr);
+            png_set_filler(fPng_ptr, 0xff, PNG_FILLER_AFTER);
+        }
+        if (kBGRA_8888_SkColorType == dstColorType) {
+            //convert RGBA to BGRA 
+            png_set_bgr(fPng_ptr);
+        }
+    }
+    png_read_update_info(fPng_ptr, fInfo_ptr);  
+    return kSuccess;
+}
+
+bool SkPngCodec::handleRewind(const SkImageInfo& requestedInfo) {
     switch (this->rewindIfNeeded()) {
         case kNoRewindNecessary_RewindState:
+            //set transforms needed for requestedInfo format
+            if (kSuccess != this->setDataFormat(requestedInfo)) {
+                return false;
+            }
             return true;
         case kCouldNotRewind_RewindState:
             return false;
         case kRewound_RewindState: {
             // This sets fPng_ptr and fInfo_ptr to NULL. If read_header
             // succeeds, they will be repopulated, and if it fails, they will
             // remain NULL. Any future accesses to fPng_ptr and fInfo_ptr will
             // come through this function which will rewind and again attempt
             // to reinitialize them.
             this->destroyReadStruct();
             png_structp png_ptr;
             png_infop info_ptr;
-            if (read_header(this->stream(), &png_ptr, &info_ptr, NULL)) {
+            if (read_header(this->stream(), &png_ptr, &info_ptr, NULL, NULL)) {
                 fPng_ptr = png_ptr;
                 fInfo_ptr = info_ptr;
+                //set transforms needed for requestedInfo format
+                if (kSuccess != this->setDataFormat(requestedInfo)) {
+                    return false;
+                }
                 return true;
             }
             return false;
         }
         default:
             SkASSERT(false);
             return false;
     }
 }
 
 SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
                                         size_t rowBytes, const Options& options,
                                         SkPMColor ctable[], int* ctableCount) {
-    if (!this->handleRewind()) {
-        return kCouldNotRewind;
+    if (!conversion_possible(requestedInfo, this->getInfo())) {
+        return kInvalidConversion;
     }
     if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
-    if (!conversion_possible(requestedInfo, this->getInfo())) {
-        return kInvalidConversion;
+    if (!this->handleRewind(requestedInfo)) {
+        return kCouldNotRewind;
     }
 
     // Note that ctable and ctableCount may be modified if there is a color table
     const Result result = this->initializeSwizzler(requestedInfo, dst, rowBytes,
                                                    options, ctable, ctableCount);
-
     if (result != kSuccess) {
         return result;
     }
-
     // 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;
     }
 
     SkASSERT(fNumberPasses != INVALID_NUMBER_PASSES);
     SkAutoMalloc storage;
     if (fNumberPasses > 1) {
@@ skipping 105 matching lines @@
 
 
 class SkPngInterlacedScanlineDecoder : public SkScanlineDecoder {
 public:
     SkPngInterlacedScanlineDecoder(const SkImageInfo& dstInfo, SkPngCodec* codec)
         : INHERITED(dstInfo)
         , fCodec(codec)
         , fHasAlpha(false)
         , fCurrentRow(0)
         , fHeight(dstInfo.height())
-        , fSrcRowBytes(dstInfo.minRowBytes())
         , fRewindNeeded(false)
     {
+        fSrcRowBytes = dstInfo.width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig);
         fGarbageRow.reset(fSrcRowBytes);
         fGarbageRowPtr = static_cast<uint8_t*>(fGarbageRow.get());
     }
 
     SkImageGenerator::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
+        //need to reset transforms in setDataFormat if stream is rewound
         if (fRewindNeeded) {
-            if(false == fCodec->handleRewind()) {
+            if(false == fCodec->handleRewind(this->dstInfo())) {
                 return SkImageGenerator::kCouldNotRewind;
             }
         } else {
             fRewindNeeded = true;
         }
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
             return SkImageGenerator::kInvalidInput;
         }
         const int number_passes = png_set_interlace_handling(fCodec->fPng_ptr);
@@ skipping 21 matching lines @@
         for (int y = 0; y < count; y++) {
             fCodec->fSwizzler->setDstRow(dst);
             fHasAlpha |= !SkSwizzler::IsOpaque(fCodec->fSwizzler->next(srcRow));
             dst = SkTAddOffset<void>(dst, dstRowBytes);
             srcRow += fSrcRowBytes;
         }
         fCurrentRow += count;
         return SkImageGenerator::kSuccess;
     }
 
-    SkImageGenerator::Result onSkipScanlines(int count) override {
+    SkImageGenerator::Result onSkipScanlines(int count) override { 
         //when ongetScanlines is called it will skip to fCurrentRow
         fCurrentRow += count;
         return SkImageGenerator::kSuccess;
     }
 
     void onFinish() override {
         fCodec->finish();
     }
 
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
     SkPngCodec*         fCodec;     // Unowned.
     bool                fHasAlpha;
     int                 fCurrentRow;
     int                 fHeight;
     size_t              fSrcRowBytes;
     bool                fRewindNeeded;
     SkAutoMalloc        fGarbageRow;
     uint8_t*            fGarbageRowPtr;
-    
-    
-    
-    
-
     typedef SkScanlineDecoder INHERITED;
 };
 
 
 SkScanlineDecoder* SkPngCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
         const Options& options, SkPMColor ctable[], int* ctableCount) {
-    if (!this->handleRewind()) {
+    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 (!conversion_possible(dstInfo, this->getInfo())) {
-        SkCodecPrintf("no conversion possible\n");
+    if (!this->handleRewind(dstInfo)) {
         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 (this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options, ctable,
             ctableCount) != kSuccess) {
         SkCodecPrintf("failed to initialize the swizzler.\n");
         return NULL;
     }
 
     SkASSERT(fNumberPasses != INVALID_NUMBER_PASSES);
     if (fNumberPasses > 1) {
         // interlaced image
         return SkNEW_ARGS(SkPngInterlacedScanlineDecoder, (dstInfo, this));
     }
 
     return SkNEW_ARGS(SkPngScanlineDecoder, (dstInfo, this));
 }