Support RGBA/BGRA Premul/Unpremul from SkPNGImageEncoder

src/images/SkPNGImageEncoder.cpp

 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkImageEncoder.h"
 #include "SkColor.h"
 #include "SkColorPriv.h"
@@ skipping 40 matching lines @@
     longjmp(png_jmpbuf(png_ptr), 1);
 }
 
 static void sk_write_fn(png_structp png_ptr, png_bytep data, png_size_t len) {
     SkWStream* sk_stream = (SkWStream*)png_get_io_ptr(png_ptr);
     if (!sk_stream->write(data, len)) {
         png_error(png_ptr, "sk_write_fn Error!");
     }
 }
 
-static transform_scanline_proc choose_proc(SkColorType ct, bool hasAlpha) {
-    // we don't care about search on alpha if we're kIndex8, since only the
-    // colortable packing cares about that distinction, not the pixels
-    if (kIndex_8_SkColorType == ct) {
-        hasAlpha = false;   // we store false in the table entries for kIndex8
-    }
-
+static transform_scanline_proc choose_proc(SkColorType ct, SkAlphaType alphaType) {
     static const struct {
         SkColorType             fColorType;
-        bool                    fHasAlpha;
+        SkAlphaType             fAlphaType;
         transform_scanline_proc fProc;
     } gMap[] = {
-        { kRGB_565_SkColorType,     false,  transform_scanline_565 },
-        { kN32_SkColorType,         false,  transform_scanline_888 },
-        { kN32_SkColorType,         true,   transform_scanline_8888 },
-        { kARGB_4444_SkColorType,   false,  transform_scanline_444 },
-        { kARGB_4444_SkColorType,   true,   transform_scanline_4444 },
-        { kIndex_8_SkColorType,     false,  transform_scanline_memcpy },
-        { kGray_8_SkColorType,      false,  transform_scanline_memcpy },
+        { kRGB_565_SkColorType,   kOpaque_SkAlphaType,   transform_scanline_565    },
+        { kRGBA_8888_SkColorType, kOpaque_SkAlphaType,   transform_scanline_RGB1   },
+        { kBGRA_8888_SkColorType, kOpaque_SkAlphaType,   transform_scanline_BGR1   },
+        { kRGBA_8888_SkColorType, kPremul_SkAlphaType,   transform_scanline_rgbA   },
+        { kBGRA_8888_SkColorType, kPremul_SkAlphaType,   transform_scanline_bgrA   },
+        { kRGBA_8888_SkColorType, kUnpremul_SkAlphaType, transform_scanline_memcpy },
+        { kBGRA_8888_SkColorType, kUnpremul_SkAlphaType, transform_scanline_BGRA   },
+        { kARGB_4444_SkColorType, kOpaque_SkAlphaType,   transform_scanline_444    },
+        { kARGB_4444_SkColorType, kPremul_SkAlphaType,   transform_scanline_4444   },
+        { kIndex_8_SkColorType,   kOpaque_SkAlphaType,   transform_scanline_memcpy },
+        { kIndex_8_SkColorType,   kPremul_SkAlphaType,   transform_scanline_memcpy },
+        { kIndex_8_SkColorType,   kUnpremul_SkAlphaType, transform_scanline_memcpy },
+        { kGray_8_SkColorType,    kOpaque_SkAlphaType,   transform_scanline_memcpy },
     };
 
     for (int i = SK_ARRAY_COUNT(gMap) - 1; i >= 0; --i) {

[scroggo, 2016/09/12 17:16:49]

nit: Can you update this to a for each loop?

[msarett, 2016/09/12 19:39:19]

Done.

-        if (gMap[i].fColorType == ct && gMap[i].fHasAlpha == hasAlpha) {
+        if (gMap[i].fColorType == ct && gMap[i].fAlphaType == alphaType) {
             return gMap[i].fProc;
         }
     }
     sk_throw();
     return nullptr;
 }
 
 // return the minimum legal bitdepth (by png standards) for this many colortable
 // entries. SkBitmap always stores in 8bits per pixel, but for colorcount <= 16,
 // we can use fewer bits per in png
@@ skipping 11 matching lines @@
 
 /*  Pack palette[] with the corresponding colors, and if hasAlpha is true, also
     pack trans[] and return the number of trans[] entries written. If hasAlpha
     is false, the return value will always be 0.
 
     Note: this routine takes care of unpremultiplying the RGB values when we
     have alpha in the colortable, since png doesn't support premul colors
 */
 static inline int pack_palette(SkColorTable* ctable,
                                png_color* SK_RESTRICT palette,
-                               png_byte* SK_RESTRICT trans, bool hasAlpha) {
+                               png_byte* SK_RESTRICT trans, SkAlphaType alphaType) {

[scroggo, 2016/09/12 17:16:50]

I'm confused. Didn't you change this method differently in crrev.com/2330053002
?

[msarett, 2016/09/12 19:39:19]

Sorry I've sent my code reviews backwards.  That CL is based on this CL.  The
changes to pack_palette() here aren't super interesting since they will be
overwritten anyway.

     const SkPMColor* SK_RESTRICT colors = ctable ? ctable->readColors() : nullptr;
     const int ctCount = ctable->count();
     int i, num_trans = 0;
 
-    if (hasAlpha) {
+    if (kOpaque_SkAlphaType != alphaType) {
         /*  first see if we have some number of fully opaque at the end of the
             ctable. PNG allows num_trans < num_palette, but all of the trans
             entries must come first in the palette. If I was smarter, I'd
             reorder the indices and ctable so that all non-opaque colors came
             first in the palette. But, since that would slow down the encode,
             I'm leaving the indices and ctable order as is, and just looking
             at the tail of the ctable for opaqueness.
         */
         num_trans = ctCount;
         for (i = ctCount - 1; i >= 0; --i) {
             if (SkGetPackedA32(colors[i]) != 0xFF) {
                 break;
             }
             num_trans -= 1;
         }
 
-        const SkUnPreMultiply::Scale* SK_RESTRICT table =
-                                            SkUnPreMultiply::GetScaleTable();
+        if (kPremul_SkAlphaType == alphaType) {
+            const SkUnPreMultiply::Scale* SK_RESTRICT table = SkUnPreMultiply::GetScaleTable();
+            for (i = 0; i < num_trans; i++) {
+                const SkPMColor c = *colors++;
+                const unsigned a = SkGetPackedA32(c);
+                const SkUnPreMultiply::Scale s = table[a];
+                trans[i] = a;
+                palette[i].red = SkUnPreMultiply::ApplyScale(s, SkGetPackedR32(c));
+                palette[i].green = SkUnPreMultiply::ApplyScale(s,SkGetPackedG32(c));
+                palette[i].blue = SkUnPreMultiply::ApplyScale(s, SkGetPackedB32(c));
+            }
+        } else {
+            for (i = 0; i < num_trans; i++) {
+                const SkPMColor c = *colors++;
+                trans[i] = SkGetPackedA32(c);
+                palette[i].red = SkGetPackedR32(c);
+                palette[i].green = SkGetPackedG32(c);
+                palette[i].blue = SkGetPackedB32(c);
+            }
+        }
 
-        for (i = 0; i < num_trans; i++) {
-            const SkPMColor c = *colors++;
-            const unsigned a = SkGetPackedA32(c);
-            const SkUnPreMultiply::Scale s = table[a];
-            trans[i] = a;
-            palette[i].red = SkUnPreMultiply::ApplyScale(s, SkGetPackedR32(c));
-            palette[i].green = SkUnPreMultiply::ApplyScale(s,SkGetPackedG32(c));
-            palette[i].blue = SkUnPreMultiply::ApplyScale(s, SkGetPackedB32(c));
-        }
         // now fall out of this if-block to use common code for the trailing
         // opaque entries
     }
 
     // these (remaining) entries are opaque
     for (i = num_trans; i < ctCount; i++) {
         SkPMColor c = *colors++;
         palette[i].red = SkGetPackedR32(c);
         palette[i].green = SkGetPackedG32(c);
         palette[i].blue = SkGetPackedB32(c);
     }
     return num_trans;
 }
 
 class SkPNGImageEncoder : public SkImageEncoder {
 protected:
     bool onEncode(SkWStream* stream, const SkBitmap& bm, int quality) override;
 private:
     bool doEncode(SkWStream* stream, const SkBitmap& bm,
-                  const bool& hasAlpha, int colorType,
+                  SkAlphaType alphaType, int colorType,
                   int bitDepth, SkColorType ct,
                   png_color_8& sig_bit);
 
     typedef SkImageEncoder INHERITED;
 };
 
 bool SkPNGImageEncoder::onEncode(SkWStream* stream,
                                  const SkBitmap& originalBitmap,
                                  int /*quality*/) {
     SkBitmap copy;
     const SkBitmap* bitmap = &originalBitmap;
     switch (originalBitmap.colorType()) {
         case kIndex_8_SkColorType:
         case kGray_8_SkColorType:
-        case kN32_SkColorType:
+        case kRGBA_8888_SkColorType:
+        case kBGRA_8888_SkColorType:
         case kARGB_4444_SkColorType:
         case kRGB_565_SkColorType:
             break;
         default:
-            // TODO(scroggo): support 8888-but-not-N32 natively.
             // TODO(scroggo): support Alpha_8 as Grayscale(black)+Alpha
             if (originalBitmap.copyTo(&copy, kN32_SkColorType)) {
                 bitmap = &copy;
             }
     }
     SkColorType ct = bitmap->colorType();
 
-    const bool hasAlpha = !bitmap->isOpaque();
+    SkAlphaType alphaType = bitmap->alphaType();

[scroggo, 2016/09/12 17:16:50]

nit: could be const?

[msarett, 2016/09/12 19:39:19]

Done.

+    switch (alphaType) {
+        case kUnpremul_SkAlphaType:
+            if (kARGB_4444_SkColorType == ct) {
+                return false;
+            }
+
+            break;
+        case kOpaque_SkAlphaType:
+        case kPremul_SkAlphaType:
+            break;
+        default:
+            return false;
+    }
+
+    const bool isOpaque = (kOpaque_SkAlphaType == alphaType);
     int bitDepth = 8;   // default for color
     png_color_8 sig_bit;
     sk_bzero(&sig_bit, sizeof(png_color_8));
 
     int colorType;
     switch (ct) {
         case kIndex_8_SkColorType:
             sig_bit.red = 8;
             sig_bit.green = 8;
             sig_bit.blue = 8;
             sig_bit.alpha = 8;
             colorType = PNG_COLOR_TYPE_PALETTE;
             break;
         case kGray_8_SkColorType:
             sig_bit.gray = 8;
             colorType = PNG_COLOR_TYPE_GRAY;
-            SkASSERT(!hasAlpha);
+            SkASSERT(isOpaque);
             break;
-        case kN32_SkColorType:
+        case kRGBA_8888_SkColorType:
+        case kBGRA_8888_SkColorType:
             sig_bit.red = 8;
             sig_bit.green = 8;
             sig_bit.blue = 8;
             sig_bit.alpha = 8;
-            colorType = hasAlpha ? PNG_COLOR_TYPE_RGB_ALPHA : PNG_COLOR_TYPE_RGB;
+            colorType = isOpaque ? PNG_COLOR_TYPE_RGB : PNG_COLOR_TYPE_RGB_ALPHA;
             break;
         case kARGB_4444_SkColorType:
             sig_bit.red = 4;
             sig_bit.green = 4;
             sig_bit.blue = 4;
             sig_bit.alpha = 4;
-            colorType = hasAlpha ? PNG_COLOR_TYPE_RGB_ALPHA : PNG_COLOR_TYPE_RGB;
+            colorType = isOpaque ? PNG_COLOR_TYPE_RGB : PNG_COLOR_TYPE_RGB_ALPHA;
             break;
         case kRGB_565_SkColorType:
             sig_bit.red = 5;
             sig_bit.green = 6;
             sig_bit.blue = 5;
             colorType = PNG_COLOR_TYPE_RGB;
-            SkASSERT(!hasAlpha);
+            SkASSERT(isOpaque);
             break;
         default:
             return false;
     }
 
     SkAutoLockPixels alp(*bitmap);
     // readyToDraw checks for pixels (and colortable if that is required)
     if (!bitmap->readyToDraw()) {
         return false;
     }
 
     // we must do this after we have locked the pixels
     SkColorTable* ctable = bitmap->getColorTable();
     if (ctable) {
         if (ctable->count() == 0) {
             return false;
         }
         // check if we can store in fewer than 8 bits
         bitDepth = computeBitDepth(ctable->count());
     }
 
-    return doEncode(stream, *bitmap, hasAlpha, colorType, bitDepth, ct, sig_bit);
+    return doEncode(stream, *bitmap, alphaType, colorType, bitDepth, ct, sig_bit);
 }
 
 bool SkPNGImageEncoder::doEncode(SkWStream* stream, const SkBitmap& bitmap,
-                  const bool& hasAlpha, int colorType,
+                  SkAlphaType alphaType, int colorType,
                   int bitDepth, SkColorType ct,
                   png_color_8& sig_bit) {
 
     png_structp png_ptr;
     png_infop info_ptr;
 
     png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, nullptr, sk_error_fn,
                                       nullptr);
     if (nullptr == png_ptr) {
         return false;
@@ skipping 27 matching lines @@
     png_set_IHDR(png_ptr, info_ptr, bitmap.width(), bitmap.height(),
                  bitDepth, colorType,
                  PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_BASE,
                  PNG_FILTER_TYPE_BASE);
 
     // set our colortable/trans arrays if needed
     png_color paletteColors[256];
     png_byte trans[256];
     if (kIndex_8_SkColorType == ct) {
         SkColorTable* ct = bitmap.getColorTable();
-        int numTrans = pack_palette(ct, paletteColors, trans, hasAlpha);
+        int numTrans = pack_palette(ct, paletteColors, trans, alphaType);
         png_set_PLTE(png_ptr, info_ptr, paletteColors, ct->count());
         if (numTrans > 0) {
             png_set_tRNS(png_ptr, info_ptr, trans, numTrans, nullptr);
         }
     }
 
     png_set_sBIT(png_ptr, info_ptr, &sig_bit);
     png_write_info(png_ptr, info_ptr);
 
-    const char* srcImage = (const char*)bitmap.getPixels();
-    SkAutoSTMalloc<1024, char> rowStorage(bitmap.width() << 2);
-    char* storage = rowStorage.get();
-    transform_scanline_proc proc = choose_proc(ct, hasAlpha);
+    const uint8_t* srcImage = (const uint8_t*)bitmap.getPixels();
+    SkAutoSTMalloc<1024, uint8_t> rowStorage(bitmap.width() << 2);
+    uint8_t* storage = rowStorage.get();
+    transform_scanline_proc proc = choose_proc(ct, alphaType);
 
     for (int y = 0; y < bitmap.height(); y++) {
         png_bytep row_ptr = (png_bytep)storage;
-        proc(srcImage, bitmap.width(), storage);
+        proc(storage, srcImage, bitmap.width(), SkColorTypeBytesPerPixel(ct));
         png_write_rows(png_ptr, &row_ptr, 1);
         srcImage += bitmap.rowBytes();
     }
 
     png_write_end(png_ptr, info_ptr);
 
     /* clean up after the write, and free any memory allocated */
     png_destroy_write_struct(&png_ptr, &info_ptr);
     return true;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 DEFINE_ENCODER_CREATOR(PNGImageEncoder);
 ///////////////////////////////////////////////////////////////////////////////
 
 SkImageEncoder* sk_libpng_efactory(SkImageEncoder::Type t) {
     return (SkImageEncoder::kPNG_Type == t) ? new SkPNGImageEncoder : nullptr;
 }
 
 static SkImageEncoder_EncodeReg gEReg(sk_libpng_efactory);