PDF: remove last use of SkPDFImage

src/pdf/SkPDFBitmap.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 "SkColorPriv.h"
 #include "SkDeflateWStream.h"
 #include "SkPDFBitmap.h"
 #include "SkPDFCanon.h"
 #include "SkPDFCatalog.h"
 #include "SkPDFDocument.h"
 #include "SkStream.h"
 #include "SkUnPreMultiply.h"
 
 ////////////////////////////////////////////////////////////////////////////////
 
 static void pdf_stream_begin(SkWStream* stream) {
     static const char streamBegin[] = " stream\n";
     stream->write(streamBegin, strlen(streamBegin));
 }
 
 static void pdf_stream_end(SkWStream* stream) {
     static const char streamEnd[] = "\nendstream";
     stream->write(streamEnd, strlen(streamEnd));
 }
 
-static size_t pixel_count(const SkBitmap& bm) {
-    return SkToSizeT(bm.width()) * SkToSizeT(bm.height());
-}
+////////////////////////////////////////////////////////////////////////////////
 
 // write a single byte to a stream n times.
 static void fill_stream(SkWStream* out, char value, size_t n) {
     char buffer[4096];
     memset(buffer, value, sizeof(buffer));
     while (n) {
         size_t k = SkTMin(n, sizeof(buffer));
         out->write(buffer, k);
         n -= k;
     }
 }
 
-static SkPMColor get_pmcolor_neighbor_avg_color(const SkBitmap& bitmap,
-                                                int xOrig,
-                                                int yOrig) {
-    SkASSERT(kN32_SkColorType == bitmap.colorType());
-    SkASSERT(bitmap.getPixels());
-    uint8_t count = 0;
-    unsigned r = 0;
-    unsigned g = 0;
-    unsigned b = 0;
-    for (int y = yOrig - 1; y <= yOrig + 1; ++y) {
-        if (y < 0 || y >= bitmap.height()) {
-            continue;
-        }
-        uint32_t* src = bitmap.getAddr32(0, y);
-        for (int x = xOrig - 1; x <= xOrig + 1; ++x) {
-            if (x < 0 || x >= bitmap.width()) {
-                continue;
-            }
-            SkPMColor pmColor = src[x];
+static void to_rgb24(SkPMColor color, U8CPU alpha, uint8_t* rgb) {

[mtklein, 2015/02/23 14:26:52]

This seems weird.  Why do we pass alpha twice?  It's always color's alpha,
right?

Might be nice to pass rgb as uint8_t rgb[3]?

[hal.canary, 2015/03/20 01:09:45]

I didn't want to recalculate it, but that was too much optimization. Fixed.

+    uint32_t s = SkUnPreMultiply::GetScale(alpha);
+    rgb[0] = SkUnPreMultiply::ApplyScale(s, SkGetPackedR32(color));
+    rgb[1] = SkUnPreMultiply::ApplyScale(s, SkGetPackedG32(color));
+    rgb[2] = SkUnPreMultiply::ApplyScale(s, SkGetPackedB32(color));
+}
+
+static void to_rgb24(SkPMColor color, uint8_t* rgb) {

[mtklein, 2015/02/23 14:26:52]

This is for the alpha == 0xFF case?  Let's at least assert it, maybe merge this
into the other to_rgb24?

[hal.canary, 2015/03/20 01:09:45]

this is removed.

+    rgb[0] = SkGetPackedR32(color);
+    rgb[1] = SkGetPackedG32(color);
+    rgb[2] = SkGetPackedB32(color);
+}
+
+static SkPMColor get_pmcolor(const SkBitmap& bm, int x, int y) {
+    switch (bm.colorType()) {
+        case kARGB_4444_SkColorType:
+            return SkPixel4444ToPixel32(*(bm.getAddr16(x, y)));
+        case kN32_SkColorType:
+            return *(bm.getAddr32(x, y));
+        default:
+            SkASSERT(false);

[mtklein, 2015/02/23 14:26:52]

// We only use this to sample around transparent pixels, so only 4444 and 8888
make sense.

[hal.canary, 2015/03/20 01:09:46]

Done.

+            return 0;
+    }
+}
+
+/* It is necessary to average the color component of transparent
+   pixels with their surrounding neighbors since the PDF renderer may
+   separately re-sample the alpha and color channels when the image is
+   not displayed at its native resolution. Since an alpha of zero
+   gives no information about the color component, the pathological
+   case is a white image with sharp transparency bounds - the color
+   channel goes to black, and the should-be-transparent pixels are
+   rendered as grey because of the separate soft mask and color
+   resizing. e.g.: gm/bitmappremul.cpp */
+static SkPMColor get_neighbor_avg_color(const SkBitmap& bm,
+                                        int xOrig,
+                                        int yOrig) {
+    SkASSERT(kARGB_4444_SkColorType == bm.colorType() ||
+             kN32_SkColorType == bm.colorType());
+    uint8_t n = 0;

[mtklein, 2015/02/23 14:26:53]

This seems... unnecessarily precise?  unsigned seems fine for n too, or really,
int?

[hal.canary, 2015/03/20 01:09:45]

Done.

+    unsigned r = 0, g = 0, b = 0;
+    int yrange[2] = {SkTMax(yOrig - 1, 0), SkTMin(bm.height(), yOrig + 2)};

[mtklein, 2015/02/23 14:26:53]

Might want to swap the orders of the arguments to max and min so they look like
a clamp

SkTMax(0, yOrig-1), SkTMin(yOrig+2, bm.height())

Might also want to use <= so you can write -1, +1 ?

[hal.canary, 2015/03/20 01:09:45]

<= is unexpected in C.

+    int xrange[2] = {SkTMax(xOrig - 1, 0), SkTMin(bm.width(), xOrig + 2)};
+    for (int y = yrange[0]; y < yrange[1]; ++y) {
+        for (int x = xrange[0]; x < xrange[1]; ++x) {

[mtklein, 2015/02/23 14:26:53]

Seems like when there are a lot of transparent pixels, this will examine each
pixel many times more than if we did a pass over the whole image at once.  I
guess this is still dwarfed by deflate in CPU time?

[hal.canary, 2015/03/20 01:09:45]

I thinks so.  I tried to think of how to do this in one pass, (think of doing a
blur by scattering rather than gathering), but it doesn't seem worth the
headache to make that change.

Also, I changed the algorithm a little, to weight neighbors by opacity.

+            SkPMColor pmColor = get_pmcolor(bm, x, y);
             U8CPU alpha = SkGetPackedA32(pmColor);
             if (alpha != SK_AlphaTRANSPARENT) {
-                uint32_t s = SkUnPreMultiply::GetScale(alpha);
-                r += SkUnPreMultiply::ApplyScale(s, SkGetPackedR32(pmColor));
-                g += SkUnPreMultiply::ApplyScale(s, SkGetPackedG32(pmColor));
-                b += SkUnPreMultiply::ApplyScale(s, SkGetPackedB32(pmColor));
-                ++count;
-            }
-        }
-    }
-    if (count == 0) {
-        return SkPackARGB32NoCheck(SK_AlphaOPAQUE, 0, 0, 0);
+                uint8_t rgb[3];
+                to_rgb24(pmColor, alpha, rgb);
+                r += rgb[0];
+                g += rgb[1];
+                b += rgb[2];
+                ++n;
+            }
+        }
+    }
+    return (n > 0) ? SkPackARGB32NoCheck(SK_AlphaOPAQUE, r / n, g / n, b / n)
+                   : SkPackARGB32NoCheck(SK_AlphaOPAQUE, 0, 0, 0);
+}
+
+static void convert_pixel(
+        const SkBitmap& bm, int x, int y, SkPMColor color, uint8_t* dst) {
+    U8CPU alpha = SkGetPackedA32(color);
+    if (alpha != SK_AlphaTRANSPARENT) {
+        to_rgb24(color, alpha, dst);
     } else {
-        return SkPackARGB32NoCheck(
-                SK_AlphaOPAQUE, r / count, g / count, b / count);
-    }
-}
-
-static void pmcolor_to_rgb24(const SkBitmap& bm, SkWStream* out) {
-    SkASSERT(kN32_SkColorType == bm.colorType());
+        to_rgb24(get_neighbor_avg_color(bm, x, y), dst);
+    }
+}
+
+static void convert_scanline(const SkBitmap& bm, int y, uint8_t* dst) {
+    switch (bm.colorType()) {
+        case kARGB_4444_SkColorType: {
+            const uint16_t* src = bm.getAddr16(0, y);
+            for (int x = 0; x < bm.width(); ++x) {
+                convert_pixel(bm, x, y, SkPixel4444ToPixel32(*src++), dst);
+                dst += 3;
+            }
+            return;
+        }
+        case kN32_SkColorType: {
+            const SkPMColor* src = bm.getAddr32(0, y);
+            for (int x = 0; x < bm.width(); ++x) {
+                convert_pixel(bm, x, y, *src++, dst);
+                dst += 3;
+            }
+            return;
+        }
+        case kRGB_565_SkColorType: {
+            const uint16_t* src = bm.getAddr16(0, y);
+            for (int x = 0; x < bm.width(); ++x) {
+                U16CPU color565 = *src++;
+                *dst++ = SkPacked16ToR32(color565);
+                *dst++ = SkPacked16ToG32(color565);
+                *dst++ = SkPacked16ToB32(color565);
+            }
+            return;
+        }
+        default:
+            SkASSERT(false);
+    }
+}
+
+static size_t pixel_count(const SkBitmap& bm) {
+    return SkToSizeT(bm.width()) * SkToSizeT(bm.height());
+}
+
+static void bitmap_to_rgb24(const SkBitmap& bm, SkWStream* out) {
     if (!bm.getPixels()) {
         fill_stream(out, '\xFF', 3 * pixel_count(bm));
         return;
     }
-    size_t scanlineLength = 3 * bm.width();
-    SkAutoTMalloc<uint8_t> scanline(scanlineLength);
+    switch (bm.colorType()) {
+        case kN32_SkColorType:
+        case kARGB_4444_SkColorType:
+        case kRGB_565_SkColorType: {
+            SkAutoTMalloc<uint8_t> scanline(3 * bm.width());
+            for (int y = 0; y < bm.height(); ++y) {
+                convert_scanline(bm, y, scanline.get());
+                out->write(scanline.get(), 3 * bm.width());
+            }
+            return;
+        }
+        case kAlpha_8_SkColorType:
+            fill_stream(out, '\x00', 3 * pixel_count(bm));
+            return;
+        case kIndex_8_SkColorType:
+            for (int y = 0; y < bm.height(); ++y) {
+                out->write(bm.getAddr8(0, y), bm.width());
+            }  // not actually rgb24 format.

[mtklein, 2015/02/23 14:26:53]

Let's explain what it is rather than what it's not?

[hal.canary, 2015/03/20 01:09:45]

Done.

+            return;
+        case kUnknown_SkColorType:
+        default:
+            SkASSERT(false);
+            return;
+    }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+static void convert_scanline_alpha(const SkBitmap& bm, int y, uint8_t* dst) {
+    switch (bm.colorType()) {
+        case kARGB_4444_SkColorType: {
+            const uint16_t* src = bm.getAddr16(0, y);
+            for (int x = 0; x < bm.width(); ++x) {
+                *dst++ = SkPacked4444ToA32(*src++);
+            }
+            return;
+        }
+        case kN32_SkColorType: {
+            const SkPMColor* src = bm.getAddr32(0, y);
+            for (int x = 0; x < bm.width(); ++x) {
+                *dst++ = SkGetPackedA32(*src++);
+            }
+            return;
+        }
+        default:
+            SkASSERT(false);
+    }
+}
+
+static void index8_alpha_to_a8(const SkBitmap& bm, SkWStream* out) {
+    SkASSERT(kIndex_8_SkColorType == bm.colorType());
+    SkColorTable* ct = bm.getColorTable();
+    if (!ct) {
+        fill_stream(out, '\x00', pixel_count(bm));

[mtklein, 2015/02/23 14:26:53]

It seems like it's worth promoting this up to an assertion.  Index8 doesn't make
much sense without a color table, right?  I might just use
SkBitmap::getIndex8Color()?

[hal.canary, 2015/03/20 01:09:45]

Done.

+        return;
+    }
+    SkAutoTMalloc<uint8_t> scanline(bm.width());
     for (int y = 0; y < bm.height(); ++y) {
         uint8_t* dst = scanline.get();
-        const SkPMColor* src = bm.getAddr32(0, y);
+        const uint8_t* src = bm.getAddr8(0, y);
         for (int x = 0; x < bm.width(); ++x) {
-            SkPMColor color = *src++;
-            U8CPU alpha = SkGetPackedA32(color);
-            if (alpha != SK_AlphaTRANSPARENT) {
-                uint32_t s = SkUnPreMultiply::GetScale(alpha);
-                *dst++ = SkUnPreMultiply::ApplyScale(s, SkGetPackedR32(color));
-                *dst++ = SkUnPreMultiply::ApplyScale(s, SkGetPackedG32(color));
-                *dst++ = SkUnPreMultiply::ApplyScale(s, SkGetPackedB32(color));
-            } else {
-                /* It is necessary to average the color component of
-                   transparent pixels with their surrounding neighbors
-                   since the PDF renderer may separately re-sample the
-                   alpha and color channels when the image is not
-                   displayed at its native resolution. Since an alpha
-                   of zero gives no information about the color
-                   component, the pathological case is a white image
-                   with sharp transparency bounds - the color channel
-                   goes to black, and the should-be-transparent pixels
-                   are rendered as grey because of the separate soft
-                   mask and color resizing. e.g.: gm/bitmappremul.cpp */
-                color = get_pmcolor_neighbor_avg_color(bm, x, y);
-                *dst++ = SkGetPackedR32(color);
-                *dst++ = SkGetPackedG32(color);
-                *dst++ = SkGetPackedB32(color);
-            }
-        }
-        out->write(scanline.get(), scanlineLength);
-    }
-}
-
-static void pmcolor_alpha_to_a8(const SkBitmap& bm, SkWStream* out) {
-    SkASSERT(kN32_SkColorType == bm.colorType());
+            *dst++ = SkGetPackedA32((*ct)[*src++]);
+        }
+        out->write(scanline.get(), bm.width());
+    }
+}
+
+static void bitmap_alpha_to_a8(const SkBitmap& bm, SkWStream* out) {
     if (!bm.getPixels()) {
         fill_stream(out, '\xFF', pixel_count(bm));
         return;
     }
-    size_t scanlineLength = bm.width();
-    SkAutoTMalloc<uint8_t> scanline(scanlineLength);
-    for (int y = 0; y < bm.height(); ++y) {
-        uint8_t* dst = scanline.get();
-        const SkPMColor* src = bm.getAddr32(0, y);
-        for (int x = 0; x < bm.width(); ++x) {
-            *dst++ = SkGetPackedA32(*src++);
-        }
-        out->write(scanline.get(), scanlineLength);
-    }
-}
-
+    switch (bm.colorType()) {
+        case kN32_SkColorType:
+        case kARGB_4444_SkColorType: {
+            SkAutoTMalloc<uint8_t> scanline(bm.width());
+            for (int y = 0; y < bm.height(); ++y) {
+                convert_scanline_alpha(bm, y, scanline.get());
+                out->write(scanline.get(), bm.width());
+            }
+            return;
+        }
+        case kAlpha_8_SkColorType:
+            for (int y = 0; y < bm.height(); ++y) {
+                out->write(bm.getAddr8(0, y), bm.width());
+            }
+            return;
+        case kIndex_8_SkColorType:
+            index8_alpha_to_a8(bm, out);
+            return;
+        case kRGB_565_SkColorType:
+        case kUnknown_SkColorType:
+        default:
+            SkASSERT(false);
+            return;
+    }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 namespace {
 // This SkPDFObject only outputs the alpha layer of the given bitmap.
 class PDFAlphaBitmap : public SkPDFObject {
 public:
     PDFAlphaBitmap(const SkBitmap& bm) : fBitmap(bm) {}
     ~PDFAlphaBitmap() {}
     void emitObject(SkWStream*, SkPDFCatalog*) SK_OVERRIDE;
     void addResources(SkTSet<SkPDFObject*>*, SkPDFCatalog*) const SK_OVERRIDE {}
 
 private:
     const SkBitmap fBitmap;
     void emitDict(SkWStream*, SkPDFCatalog*, size_t, bool) const;
 };
 
 void PDFAlphaBitmap::emitObject(SkWStream* stream, SkPDFCatalog* catalog) {
     SkAutoLockPixels autoLockPixels(fBitmap);
 
 #ifndef SK_NO_FLATE
     // Write to a temporary buffer to get the compressed length.
     SkDynamicMemoryWStream buffer;
     SkDeflateWStream deflateWStream(&buffer);
-    pmcolor_alpha_to_a8(fBitmap, &deflateWStream);
+    bitmap_alpha_to_a8(fBitmap, &deflateWStream);
     deflateWStream.finalize();  // call before detachAsStream().
     SkAutoTDelete<SkStreamAsset> asset(buffer.detachAsStream());
 
     this->emitDict(stream, catalog, asset->getLength(), /*deflate=*/true);
     pdf_stream_begin(stream);
     stream->writeStream(asset.get(), asset->getLength());
     pdf_stream_end(stream);
 #else
     this->emitDict(stream, catalog, pixel_count(fBitmap), /*deflate=*/false);
     pdf_stream_begin(stream);
-    pmcolor_alpha_to_a8(fBitmap, stream);
+    bitmap_alpha_to_a8(fBitmap, stream);
     pdf_stream_end(stream);
 #endif  // SK_NO_FLATE
 }
 
 void PDFAlphaBitmap::emitDict(SkWStream* stream,
                               SkPDFCatalog* catalog,
                               size_t length,
                               bool deflate) const {
     SkPDFDict pdfDict("XObject");
     pdfDict.insertName("Subtype", "Image");
@@ skipping 18 matching lines @@
     }
 }
 
 void SkPDFBitmap::emitObject(SkWStream* stream, SkPDFCatalog* catalog) {
     SkAutoLockPixels autoLockPixels(fBitmap);
 
 #ifndef SK_NO_FLATE
     // Write to a temporary buffer to get the compressed length.
     SkDynamicMemoryWStream buffer;
     SkDeflateWStream deflateWStream(&buffer);
-    pmcolor_to_rgb24(fBitmap, &deflateWStream);
+    bitmap_to_rgb24(fBitmap, &deflateWStream);
     deflateWStream.finalize();  // call before detachAsStream().
     SkAutoTDelete<SkStreamAsset> asset(buffer.detachAsStream());
 
     this->emitDict(stream, catalog, asset->getLength(), /*deflate=*/true);
     pdf_stream_begin(stream);
     stream->writeStream(asset.get(), asset->getLength());
     pdf_stream_end(stream);
 #else
     this->emitDict(stream, catalog, 3 * pixel_count(fBitmap), /*deflate=*/false);
     pdf_stream_begin(stream);
-    pmcolor_to_rgb24(fBitmap, stream);
+    bitmap_to_rgb24(fBitmap, stream);
     pdf_stream_end(stream);
     return;
 #endif  // SK_NO_FLATE
 }
 
+static SkPDFArray* make_indexed_color_space(const SkColorTable* table) {
+    SkPDFArray* result = SkNEW(SkPDFArray);
+    result->reserve(4);
+    result->appendName("Indexed");
+    result->appendName("DeviceRGB");
+    SkASSERT(table);
+    result->appendInt(table->count() - 1);
+
+    // Potentially, this could be represented in fewer bytes with a stream.
+    // Max size as a string is 1.5k.

[mtklein, 2015/02/23 14:26:53]

Some sort of assert here then?

[hal.canary, 2015/03/20 01:09:45]

Done.

+    SkString index;
+    for (int i = 0; i < table->count(); i++) {
+        SkPMColor color = (*table)[i];
+        U8CPU alpha = SkGetPackedA32(color);
+        uint8_t rgb[3];
+        to_rgb24(color, alpha, rgb);
+        index.append((char*)rgb, 3);
+    }
+    result->append(new SkPDFString(index))->unref();
+    return result;
+}
+
 void SkPDFBitmap::emitDict(SkWStream* stream,
                            SkPDFCatalog* catalog,
                            size_t length,
                            bool deflate) const {
     SkPDFDict pdfDict("XObject");
     pdfDict.insertName("Subtype", "Image");
     pdfDict.insertInt("Width", fBitmap.width());
     pdfDict.insertInt("Height", fBitmap.height());
-    pdfDict.insertName("ColorSpace", "DeviceRGB");
+    if (fBitmap.colorType() != kIndex_8_SkColorType) {
+        pdfDict.insertName("ColorSpace", "DeviceRGB");
+    } else {
+        pdfDict.insert("ColorSpace", make_indexed_color_space(
+                                             fBitmap.getColorTable()))->unref();
+    }
     pdfDict.insertInt("BitsPerComponent", 8);
     if (fSMask) {
         pdfDict.insert("SMask", new SkPDFObjRef(fSMask))->unref();
     }
     if (deflate) {
         pdfDict.insertName("Filter", "FlateDecode");
     }
     pdfDict.insertInt("Length", length);
     pdfDict.emitObject(stream, catalog);
 }
 
 SkPDFBitmap::SkPDFBitmap(SkPDFCanon* canon,
                          const SkBitmap& bm,
                          SkPDFObject* smask)
     : fCanon(canon), fBitmap(bm), fSMask(smask) {}
 
 SkPDFBitmap::~SkPDFBitmap() { fCanon->removeBitmap(this); }
 
 ////////////////////////////////////////////////////////////////////////////////
-static bool is_transparent(const SkBitmap& bm) {
-    SkAutoLockPixels autoLockPixels(bm);
-    if (NULL == bm.getPixels()) {
-        return true;
-    }
-    SkASSERT(kN32_SkColorType == bm.colorType());
+
+static bool is_transparent_pixels(const SkBitmap& bm) {

[mtklein, 2015/02/23 14:26:52]

pixels_are_transparent?

[hal.canary, 2015/03/20 01:09:45]

I've stoped this check.  Why lock the bitmap unnecessarily?

     for (int y = 0; y < bm.height(); ++y) {
         U8CPU alpha = 0;
-        const SkPMColor* src = bm.getAddr32(0, y);
-        for (int x = 0; x < bm.width(); ++x) {
-            alpha |= SkGetPackedA32(*src++);
+        switch (bm.colorType()) {
+            case kN32_SkColorType: {
+                const SkPMColor* src = bm.getAddr32(0, y);
+                for (int x = 0; x < bm.width(); ++x) {
+                    alpha |= SkGetPackedA32(*src++);

[mtklein, 2015/02/23 14:26:53]

Why don't we just bail out the first time we see a non-zero alpha?  Why wait
until the end of the scanline?

[hal.canary, 2015/03/20 01:09:45]

Acknowledged.

+                }
+                break;
+            }
+            case kARGB_4444_SkColorType: {
+                const uint16_t* src = bm.getAddr16(0, y);
+                for (int x = 0; x < bm.width(); ++x) {
+                    alpha |= SkPacked4444ToA32(*src++);
+                }
+                break;
+            }
+            case kAlpha_8_SkColorType: {
+                const uint8_t* src = bm.getAddr8(0, y);
+                for (int x = 0; x < bm.width(); ++x) {
+                    alpha |= *src++;
+                }
+                break;
+            }
+            default:
+                SkASSERT(false);
         }
         if (alpha) {
             return false;
         }
     }
     return true;
 }
 
+static bool is_transparent(const SkBitmap& bm) {
+    SkAutoLockPixels autoLockPixels(bm);
+    if (NULL == bm.getPixels()) {
+        return true;
+    }
+    switch (bm.colorType()) {
+        case kN32_SkColorType:
+        case kARGB_4444_SkColorType:
+        case kAlpha_8_SkColorType:
+            return is_transparent_pixels(bm);
+        case kIndex_8_SkColorType: {
+            const SkColorTable* ct = bm.getColorTable();
+            if (!ct) {
+                return true;
+            }
+            for (int y = 0; y < bm.height(); ++y) {
+                U8CPU alpha = 0;
+                const uint8_t* src = bm.getAddr8(0, y);
+                for (int x = 0; x < bm.width(); ++x) {
+                    alpha |= SkGetPackedA32((*ct)[*src++]);

[mtklein, 2015/02/23 14:26:52]

Same question?

[hal.canary, 2015/03/20 01:09:45]

Acknowledged.

+                }
+                if (alpha) {
+                    return false;
+                }
+            }
+            return true;
+        }
+        default:
+            SkASSERT(false);
+            return false;
+    }
+}
+
 SkPDFBitmap* SkPDFBitmap::Create(SkPDFCanon* canon,
                                  const SkBitmap& bitmap,
                                  const SkIRect& subset) {
     SkASSERT(canon);
-    if (kN32_SkColorType != bitmap.colorType()) {
-        // TODO(halcanary): support other colortypes.
-        return NULL;
+    switch (bitmap.colorType()) {
+        case kN32_SkColorType:
+        case kRGB_565_SkColorType:
+        case kARGB_4444_SkColorType:
+        case kAlpha_8_SkColorType:
+        case kIndex_8_SkColorType:
+            break;
+        case kUnknown_SkColorType:
+        default:
+            return NULL;
     }
     SkBitmap bm;
     // Should extractSubset be done by the SkPDFDevice?
     if (!bitmap.extractSubset(&bm, subset)) {
         return NULL;
     }
     if (bm.drawsNothing()) {
         return NULL;
     }
     if (!bm.isImmutable()) {
         SkBitmap copy;
         if (!bm.copyTo(&copy)) {
             return NULL;
         }
         copy.setImmutable();
         bm = copy;
     }
 
     SkPDFBitmap* pdfBitmap = canon->findBitmap(bm);
     if (pdfBitmap) {
         return SkRef(pdfBitmap);
     }
     SkPDFObject* smask = NULL;
     if (!bm.isOpaque() && !SkBitmap::ComputeIsOpaque(bm)) {
         if (is_transparent(bm)) {

[mtklein, 2015/02/23 14:26:53]

Just curious, do we really see fully transparent bitmaps?  Is this check really
worth the bother and ~70 extra lines of code?  All the normal paths will work
just fine, right?  And presumably all-zero bitmaps will compress very well.

[hal.canary, 2015/03/20 01:09:46]

Done.

             return NULL;
         }
         // PDFAlphaBitmaps do not get directly canonicalized (they
         // are refed by the SkPDFBitmap).
         smask = SkNEW_ARGS(PDFAlphaBitmap, (bm));
     }
     pdfBitmap = SkNEW_ARGS(SkPDFBitmap, (canon, bm, smask));
     canon->addBitmap(pdfBitmap);
     return pdfBitmap;
 }