Swizzler changes Index8 and 565

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"
 #include "SkColorTable.h"
 #include "SkBitmap.h"
 #include "SkMath.h"
 #include "SkScanlineDecoder.h"
 #include "SkSize.h"
 #include "SkStream.h"
 #include "SkSwizzler.h"
+#include "SkUtils.h"
 
 ///////////////////////////////////////////////////////////////////////////////
 // Helper macros
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifndef png_jmpbuf
 #  define png_jmpbuf(png_ptr) ((png_ptr)->jmpbuf)
 #endif
 
 /* These were dropped in libpng >= 1.4 */
@@ skipping 84 matching lines @@
     png_get_tRNS(png_ptr, info_ptr, &trans, &num_trans, NULL);
     return num_trans > 0;
 }
 
 // Method for coverting to either an SkPMColor or a similarly packed
 // unpremultiplied color.
 typedef uint32_t (*PackColorProc)(U8CPU a, U8CPU r, U8CPU g, U8CPU b);
 
 // Note: SkColorTable claims to store SkPMColors, which is not necessarily
 // the case here.
-bool SkPngCodec::decodePalette(bool premultiply) {
+bool SkPngCodec::decodePalette(bool premultiply, int bitDepth, int* ctableCount) {
     int numPalette;
     png_colorp palette;
     png_bytep trans;
 
     if (!png_get_PLTE(fPng_ptr, fInfo_ptr, &palette, &numPalette)) {
         return false;
     }
 
-    /*  BUGGY IMAGE WORKAROUND
-
-        We hit some images (e.g. fruit_.png) who contain bytes that are == colortable_count
-        which is a problem since we use the byte as an index. To work around this we grow
-        the colortable by 1 (if its < 256) and duplicate the last color into that slot.
-    */
-    const int colorCount = numPalette + (numPalette < 256);
-    // Note: These are not necessarily SkPMColors.
+    // Note: These are not necessarily SkPMColors
     SkPMColor colorStorage[256];    // worst-case storage
     SkPMColor* colorPtr = colorStorage;
 
     int numTrans;
     if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {
         png_get_tRNS(fPng_ptr, fInfo_ptr, &trans, &numTrans, NULL);
     } else {
         numTrans = 0;
     }
 
@@ skipping 19 matching lines @@
         palette++;
     }
 
     fReallyHasAlpha = transLessThanFF < 0;
 
     for (; index < numPalette; index++) {
         *colorPtr++ = SkPackARGB32(0xFF, palette->red, palette->green, palette->blue);
         palette++;
     }
 
-    // see BUGGY IMAGE WORKAROUND comment above
-    if (numPalette < 256) {
-        *colorPtr = colorPtr[-1];
+    /*  BUGGY IMAGE WORKAROUND
+        Invalid images could contain pixel values that are greater than the number of palette
+        entries. Since we use pixel values as indices into the palette this could result in reading
+        beyond the end of the palette which could leak the contents of uninitialized memory. To
+        ensure this doesn't happen, we grow the colortable to the maximum size that can be
+        addressed by the bitdepth of the image and fill it with the last palette color or black if
+        the palette is empty (really broken image).
+    */
+    int colorCount = SkTMax(numPalette, 1 << SkTMin(bitDepth, 8));
+    SkPMColor lastColor = index > 0 ? colorPtr[-1] : SkPackARGB32(0xFF, 0, 0, 0);
+    for (; index < colorCount; index++) {
+        *colorPtr++ = lastColor;
+    }
+
+    // Set the new color count
+    if (ctableCount != NULL) {
+        SkASSERT(256 == *ctableCount);
+        *ctableCount = colorCount;
     }
 
     fColorTable.reset(SkNEW_ARGS(SkColorTable, (colorStorage, colorCount)));
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // Creation
 ///////////////////////////////////////////////////////////////////////////////
 
@@ skipping 78 matching lines @@
     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.
     SkColorType skColorType;
     SkAlphaType skAlphaType;
     switch (colorType) {
         case PNG_COLOR_TYPE_PALETTE:
-            // Technically, this is true of the data, but I don't think we want
-            // to support it.
-            // skColorType = kIndex8_SkColorType;
-            skColorType = kN32_SkColorType;
+            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;
@@ skipping 101 matching lines @@
     }
     if (dst.alphaType() == src.alphaType()) {
         return true;
     }
     return kPremul_SkAlphaType == dst.alphaType() &&
             kUnpremul_SkAlphaType == src.alphaType();
 }
 
 SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
                                                void* dst, size_t rowBytes,
-                                               const Options& options) {
+                                               const Options& options,
+                                               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())) {
+        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 {
@@ skipping 48 matching lines @@
     if (!this->handleRewind()) {
         return kCouldNotRewind;
     }
     if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
     if (!conversion_possible(requestedInfo, this->getInfo())) {
         return kInvalidConversion;
     }
 
+    // Note that ctableCount will be modified if there is a color table
     const Result result = this->initializeSwizzler(requestedInfo, dst, rowBytes,
-                                                   options);
+                                                   options, ctableCount);
+
+    // Copy the color table to the client if necessary
+    if (kIndex_8_SkColorType == requestedInfo.colorType()) {
+        SkASSERT(NULL != ctable);
+        SkASSERT(NULL != ctableCount);
+        SkASSERT(NULL != fColorTable.get());
+        sk_memcpy32(ctable, fColorTable->readColors(), *ctableCount);

[msarett, 2015/04/03 18:01:32]

Is this better than using regular memcpy?

[scroggo, 2015/04/06 14:55:11]

We have MemoryBench which compares the two, and it looks like we use SSE2 if we
have it. I'm guessing that if MemoryBench did not show it to be faster, we'd
just make it call memcpy.

[msarett, 2015/04/06 18:10:55]

Acknowledged.

+    }
+
     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;
     }
@@ skipping 117 matching lines @@
         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.
     Options opts;
     // FIXME: Pass this in to getScanlineDecoder?
     opts.fZeroInitialized = kNo_ZeroInitialized;
-    if (this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), opts) != kSuccess) {
+    if (this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), opts, NULL) != kSuccess) {

[scroggo, 2015/04/06 14:55:11]

Maybe add a FIXME acknowledging that getScanlineDecoder currently does not have
a way to get the color table information for a kIndex8 image. (It seems like the
caller cannot reasonably use a kIndex8 image from scanline decoding, since they
won't know what the colors are.)

[msarett, 2015/04/06 18:10:55]

Done.

         SkCodecPrintf("failed to initialize the swizzler.\n");
         return NULL;
     }
 
     SkASSERT(fNumberPasses != INVALID_NUMBER_PASSES);
     if (fNumberPasses > 1) {
         // We cannot efficiently do scanline decoding.
         return NULL;
     }
 
     return SkNEW_ARGS(SkPngScanlineDecoder, (dstInfo, this));
 }