Fixing libpng transform use

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 17 matching lines @@
     // 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;
     png_get_IHDR(png_ptr, info_ptr, &origWidth, &origHeight, &bitDepth,
                  &colorType, int_p_NULL, int_p_NULL, int_p_NULL);
 
+    if (bitDepthPtr) {
+        *bitDepthPtr = bitDepth;
+    }
+
     // 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:

[scroggo, 2015/07/01 20:26:01]

Don't we need to update the swizzler in order to support both?

[emmaleer, 2015/07/01 20:56:58]

Yes, I changed this to only support N32, and will update the swizzler to support
both in a later CL

+            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;
+    fNumberPasses = png_set_interlace_handling(fPng_ptr);

[scroggo, 2015/07/01 20:26:01]

I think you mentioned this to me before: png_set_interlace_handling returns 1
for a non-interlaced image?

[emmaleer, 2015/07/01 20:56:58]

Yes, if the image is not interlaced it returns 1, and doesn't turn on the
interlaced flag. 

+    png_read_update_info(fPng_ptr, fInfo_ptr);  
 
     // 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();
+
+    switch (srcColorType) {
+        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 kN32_SkColorType:
+            if (this->getInfo().alphaType() == kOpaque_SkAlphaType) {
+                    fSrcConfig = SkSwizzler::kRGBX;
+                } else {
+                    fSrcConfig = SkSwizzler::kRGBA;
+            }
+            break;
+        default:
+            //would have excited before now if the colorType was supported by png

[scroggo, 2015/07/01 20:26:01]

exited*

[emmaleer, 2015/07/01 20:56:58]

Acknowledged.

+            SkASSERT(false);
+    }  
 
     // 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() {
     switch (this->rewindIfNeeded()) {
         case kNoRewindNecessary_RewindState:
             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;
                 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()) {
+        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
         if (fRewindNeeded) {
             if(false == fCodec->handleRewind()) {
                 return SkImageGenerator::kCouldNotRewind;
@@ skipping 51 matching lines @@
 
 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 (!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 (!this->handleRewind()) {
         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");
-        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));
 }