SkPNGImageEncoder encodes all SkColorTypes

src/images/SkImageDecoder_libpng.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 "SkImageDecoder.h"
 #include "SkImageEncoder.h"
 #include "SkColor.h"
@@ skipping 809 matching lines @@
     bool onEncode(SkWStream* stream, const SkBitmap& bm, int quality) override;
 private:
     bool doEncode(SkWStream* stream, const SkBitmap& bm,
                   const bool& hasAlpha, int colorType,
                   int bitDepth, SkColorType ct,
                   png_color_8& sig_bit);
 
     typedef SkImageEncoder INHERITED;
 };
 
-bool SkPNGImageEncoder::onEncode(SkWStream* stream, const SkBitmap& bitmap, int /*quality*/) {
+bool SkPNGImageEncoder::onEncode(SkWStream* stream,
+                                 const SkBitmap& originalBitmap,
+                                 int /*quality*/) {
+    SkBitmap copy;
+    bool useCopy = false;

[scroggo, 2015/12/07 17:05:45]

I don't think this variable is necessary. Instead you could say something like:

SkBitmap bmStorage;
const SkBitmap* bitmap;

switch (ct) {
  // All your cases:
    bitmap = &originalBitmap;
    break;
  default:
    copyTo();
    bitmap = &bitmapStorage;
    break;
}

[hal.canary, 2015/12/07 21:34:38]

done.

+    switch (originalBitmap.colorType()) {
+        case kIndex_8_SkColorType:
+        case kN32_SkColorType:
+        case kARGB_4444_SkColorType:
+        case kRGB_565_SkColorType:
+            break;
+        default:

[scroggo, 2015/12/07 17:05:45]

So basically, what we are now supporting are:

- The other swizzle for N32 (the ironic thing here is that we might copy to the
opposite of what libpng wants - maybe add a FIXME?)
- Gray (again, this probably deserves a FIXME - we could actually encoded it to
gray, rather than copying)
- Alpha8 (not really sure what to do here, but maybe copying to N32 is as good
as anything? Android uses ALPHA_8 to mean gray, since gray did not exist when
they wanted it; maybe it would be interesting to abuse gray here? not sure...)

[hal.canary, 2015/12/07 21:34:38]

I added TODOs and assigned them to you.

+            if (originalBitmap.copyTo(&copy, kN32_SkColorType)) {
+                useCopy = true;
+            }
+    }
+    const SkBitmap& bitmap = useCopy ? copy : originalBitmap;
+
     SkColorType ct = bitmap.colorType();
 
     const bool hasAlpha = !bitmap.isOpaque();
     int colorType = PNG_COLOR_MASK_COLOR;
     int bitDepth = 8;   // default for color
     png_color_8 sig_bit;
 
     switch (ct) {
         case kIndex_8_SkColorType:
             colorType |= PNG_COLOR_MASK_PALETTE;
@@ skipping 155 matching lines @@
     return SkImageDecoder::kUnknown_Format;
 }
 
 SkImageEncoder* sk_libpng_efactory(SkImageEncoder::Type t) {
     return (SkImageEncoder::kPNG_Type == t) ? new SkPNGImageEncoder : nullptr;
 }
 
 static SkImageDecoder_DecodeReg gDReg(sk_libpng_dfactory);
 static SkImageDecoder_FormatReg gFormatReg(get_format_png);
 static SkImageEncoder_EncodeReg gEReg(sk_libpng_efactory);