Support color xforms for kIndex8 pngs

src/codec/SkPngCodec.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 "SkBitmap.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
@@ skipping 74 matching lines @@
         fPng_ptr = nullptr;
         fInfo_ptr = nullptr;
     }
 
 private:
     png_structp     fPng_ptr;
     png_infop       fInfo_ptr;
 };
 #define AutoCleanPng(...) SK_REQUIRE_LOCAL_VAR(AutoCleanPng)
 
+static inline SkAlphaType xform_alpha_type(SkAlphaType dstAlphaType, SkAlphaType srcAlphaType) {
+    return (kOpaque_SkAlphaType == srcAlphaType) ? kOpaque_SkAlphaType : dstAlphaType;
+}
+
 // Note: SkColorTable claims to store SkPMColors, which is not necessarily the case here.
-bool SkPngCodec::createColorTable(SkColorType dstColorType, bool premultiply, int* ctableCount) {
+bool SkPngCodec::createColorTable(const SkImageInfo& dstInfo, int* ctableCount) {
 
     int numColors;
     png_color* palette;
     if (!png_get_PLTE(fPng_ptr, fInfo_ptr, &palette, &numColors)) {
         return false;
     }
 
     // Note: These are not necessarily SkPMColors.
     SkPMColor colorPtr[256];

[mtklein, 2016/08/22 14:22:51]

Just noticed it's a little odd to have an array named ...Ptr.

Given how you always call it the color table, maybe, colorTable.

Instead of noting what these are not, you might want to explain what these can
be:

// Contents depend on tableColorType: {premul,unpremul} x {bgra,rgba} x ...
uint32_t colorTable[256];

[msarett, 2016/08/22 15:01:35]

Done.

+    SkColorType tableColorType = fColorXform ? kRGBA_8888_SkColorType : dstInfo.colorType();
 
     png_bytep alphas;
     int numColorsWithAlpha = 0;
     if (png_get_tRNS(fPng_ptr, fInfo_ptr, &alphas, &numColorsWithAlpha, nullptr)) {
+        // If we are performing a color xform, it will handle the premultiply.

[mtklein, 2016/08/22 14:22:51]

... otherwise we'll do it here.

[msarett, 2016/08/22 15:01:35]

Done.

+        bool needsPremul = needs_premul(dstInfo, this->getInfo());
+        bool doPremul =  needsPremul && !fColorXform;

[mtklein, 2016/08/22 14:22:51]

Might be just as clear as

   bool premultiply = !fColorXform && needs_premul(dstInfo, this->getInfo());

[msarett, 2016/08/22 15:01:35]

Done.

+
         // Choose which function to use to create the color table. If the final destination's
         // colortype is unpremultiplied, the color table will store unpremultiplied colors.
-        PackColorProc proc = choose_pack_color_proc(premultiply, dstColorType);
+        PackColorProc proc = choose_pack_color_proc(doPremul, tableColorType);
 
         for (int i = 0; i < numColorsWithAlpha; i++) {
             // We don't have a function in SkOpts that combines a set of alphas with a set
             // of RGBs.  We could write one, but it's hardly worth it, given that this
             // is such a small fraction of the total decode time.
             colorPtr[i] = proc(alphas[i], palette->red, palette->green, palette->blue);
             palette++;
         }
     }
 
     if (numColorsWithAlpha < numColors) {
         // The optimized code depends on a 3-byte png_color struct with the colors
         // in RGB order.  These checks make sure it is safe to use.
         static_assert(3 == sizeof(png_color), "png_color struct has changed.  Opts are broken.");
 #ifdef SK_DEBUG
         SkASSERT(&palette->red < &palette->green);
         SkASSERT(&palette->green < &palette->blue);
 #endif
 
-        if (is_rgba(dstColorType)) {
+        if (is_rgba(tableColorType)) {
             SkOpts::RGB_to_RGB1(colorPtr + numColorsWithAlpha, palette,
                     numColors - numColorsWithAlpha);
         } else {
             SkOpts::RGB_to_BGR1(colorPtr + numColorsWithAlpha, palette,
                     numColors - numColorsWithAlpha);
         }
     }
 
+    // If we are not decoding to F16, we can color xform now and store the results
+    // in the color table.
+    if (fColorXform && kRGBA_F16_SkColorType != dstInfo.colorType()) {
+        SkColorType xformColorType = is_rgba(dstInfo.colorType()) ?
+                kRGBA_8888_SkColorType : kBGRA_8888_SkColorType;
+        SkAlphaType xformAlphaType = xform_alpha_type(dstInfo.alphaType(),
+                                                      this->getInfo().alphaType());
+        fColorXform->apply(colorPtr, colorPtr, numColors, xformColorType, xformAlphaType);
+
+        // We have already applied the color xform.  Set it to nullptr so we don't try to
+        // use it again.
+        fColorXform = nullptr;

[mtklein, 2016/08/22 14:22:51]

Is there not some sort of early return we can make instead?  Code can get
confusing when variables are conditionally reassigned.

[msarett, 2016/08/22 15:01:35]

Yeah this is confusing, I'll try this another way.

+    }
+
     // Pad the color table with the last color in the table (or black) in the case that
     // invalid pixel indices exceed the number of colors in the table.
     const int maxColors = 1 << fBitDepth;
     if (numColors < maxColors) {
         SkPMColor lastColor = numColors > 0 ? colorPtr[numColors - 1] : SK_ColorBLACK;
         sk_memset32(colorPtr + numColors, lastColor, maxColors - numColors);
     }
 
     // Set the new color count.
     if (ctableCount != nullptr) {
@@ skipping 248 matching lines @@
             return y;
         }
 
         void* swizzlerDstRow = dst;
         size_t swizzlerDstRowBytes = rowBytes;
         if (fColorXform) {
             swizzlerDstRow = fColorXformSrcRow;
             swizzlerDstRowBytes = 0;
         }
 
-        SkAlphaType xformAlphaType = (kOpaque_SkAlphaType == this->getInfo().alphaType()) ?
-                kOpaque_SkAlphaType : dstInfo.alphaType();
+        SkAlphaType xformAlphaType = xform_alpha_type(dstInfo.alphaType(),
+                                                      this->getInfo().alphaType());
         for (; y < count; y++) {
             png_read_row(fPng_ptr, fSwizzlerSrcRow, nullptr);
             fSwizzler->swizzle(swizzlerDstRow, fSwizzlerSrcRow);
 
             if (fColorXform) {
                 fColorXform->apply(dst, (const uint32_t*) swizzlerDstRow, fSwizzler->swizzleWidth(),
                                    dstInfo.colorType(), xformAlphaType);
                 dst = SkTAddOffset<void>(dst, rowBytes);
             }
 
@@ skipping 78 matching lines @@
         }
 
         // Swizzle and xform the rows we care about
         void* swizzlerDstRow = dst;
         size_t swizzlerDstRowBytes = rowBytes;
         if (fColorXform) {
             swizzlerDstRow = fColorXformSrcRow;
             swizzlerDstRowBytes = 0;
         }
 
-        SkAlphaType xformAlphaType = (kOpaque_SkAlphaType == this->getInfo().alphaType()) ?
-                kOpaque_SkAlphaType : dstInfo.alphaType();
+        SkAlphaType xformAlphaType = xform_alpha_type(dstInfo.alphaType(),
+                                                      this->getInfo().alphaType());
         srcRow = storage.get();
         for (int y = 0; y < count; y++) {
             fSwizzler->swizzle(swizzlerDstRow, srcRow);
             srcRow = SkTAddOffset<uint8_t>(srcRow, fSrcRowBytes);
 
             if (fColorXform) {
                 if (fColorXform) {
                     fColorXform->apply(dst, (const uint32_t*) swizzlerDstRow,
                                        fSwizzler->swizzleWidth(), dstInfo.colorType(),
                                        xformAlphaType);
@@ skipping 264 matching lines @@
     if (needsColorXform) {
         switch (dstInfo.colorType()) {
             case kRGBA_8888_SkColorType:
             case kBGRA_8888_SkColorType:
             case kRGBA_F16_SkColorType:
                 swizzlerInfo = swizzlerInfo.makeColorType(kRGBA_8888_SkColorType);
                 if (kPremul_SkAlphaType == dstInfo.alphaType()) {
                     swizzlerInfo = swizzlerInfo.makeAlphaType(kUnpremul_SkAlphaType);
                 }
                 break;
+            case kIndex_8_SkColorType:
+                break;
             default:
                 return false;
         }
 
         fColorXform = SkColorSpaceXform::New(sk_ref_sp(this->getInfo().colorSpace()),
                                              sk_ref_sp(dstInfo.colorSpace()));
 
         if (!fColorXform && kRGBA_F16_SkColorType == dstInfo.colorType()) {
             return false;
         }
     }
 
     if (SkEncodedInfo::kPalette_Color == this->getEncodedInfo().color()) {
-        if (!this->createColorTable(swizzlerInfo.colorType(),
-                                    kPremul_SkAlphaType == swizzlerInfo.alphaType(), ctableCount)) {
+        if (!this->createColorTable(dstInfo, ctableCount)) {
             return false;
         }
     }
 
     // Copy the color table to the client if they request kIndex8 mode
     copy_color_table(swizzlerInfo, fColorTable, ctable, ctableCount);
 
     // Create the swizzler.  SkPngCodec retains ownership of the color table.
     const SkPMColor* colors = get_color_ptr(fColorTable.get());
     fSwizzler.reset(SkSwizzler::CreateSwizzler(this->getEncodedInfo(), colors, swizzlerInfo,
@@ skipping 59 matching lines @@
     SkCodec* outCodec;
     if (read_header(stream, chunkReader, &outCodec, nullptr, nullptr)) {
         // Codec has taken ownership of the stream.
         SkASSERT(outCodec);
         streamDeleter.release();
         return outCodec;
     }
 
     return nullptr;
 }