Swizzler changes Index8 and 565

dm/DMSrcSink.cpp

Index: dm/DMSrcSink.cpp
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 929ff0556b453de2a5786595fb107e69fdc4febe..e614a1eef767956eb1e320af139bf4666b83089e 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -72,21 +72,36 @@ Error CodecSrc::draw(SkCanvas* canvas) const {
         return SkStringPrintf("Couldn't decode %s.", fPath.c_str());
     }
 
-    SkImageInfo decodeInfo = codec->getInfo().makeColorType(canvasInfo.colorType());

[scroggo, 2015/04/02 19:20:30]

By removing this, we no longer have a way to test 565.

It also means that if an image decodes to Index8 by default, we no longer test
to 8888.

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

Oh ok, I think I misunderstood this line.  So if we keep it, we will try to
decode to the config passed on the command line?  Is it valid to pass Index8
there?

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

Essentially. This means to decode to the config of the Sink. We determine which
Sinks to use based on --config on the command line. (By default, one run will
include a few different Sinks.)
No. None of our Sinks are Index8, since Skia does not allow you to create an
SkCanvas with Index8 color type - Skia does not have a way to draw to Index8.

However, if an SkBitmap (or SkImage) is Index8, Skia has code to draw that to
multiple color types.

+    // Process the decode info to prepare for the decode
+    SkImageInfo decodeInfo = codec->getInfo();
+
+    // Construct a color table for the decode if necessary
+    SkAutoTUnref<SkColorTable> colorTable(NULL);

[msarett, 2015/04/02 17:26:29]

I saw what you did for nanobench, but I decided to continue to use SkColorTable
and SkBitmap because of the convenience of drawing an SkBitmap at the end of the
function.  Let me know if I am missing a better alternative, I'm not too
familiar with canvas->draw.

I don't think this is too bad as is, but one way to get rid of some complexity
would be to always pass in a color table.  But I think it's  a better idea to
pass NULL (when it might be NULL) to catch bugs early.

[scroggo, 2015/04/02 19:20:30]

This may be as good as anything. If you took my approach in nanobench (allocate
a buffer based on the SkImageInfo - for posterity, it's here:
https://codereview.chromium.org/1051973002/, patch set 3), you could create an
SkBitmap later. It would be ugly, but you could create an SkMallocPixelRef using
NewDirect (maybe by using a custom SkBitmap::Allocator, which I also use in that
CL...).

I was looking to see how you could use SkImage. In theory, you could use
SkImage::NewFromGenerator, but that does not allow us to specify a different
SkImageInfo or use the scanline decoder, which we want to do.
sgtm

+    SkPMColor* colorPtr = NULL;
+    int* colorCountPtr = NULL;
+    int maxColors = 256;
+    if (kIndex_8_SkColorType == decodeInfo.colorType()) {
+        SkPMColor colors[maxColors];
+        colorTable.reset(SkNEW_ARGS(SkColorTable, (colors, maxColors)));
+        colorPtr = const_cast<SkPMColor*>(colorTable->readColors());
+        colorCountPtr = &maxColors;
+    }
+
+    // FIXME: Currently we cannot draw unpremultiplied sources.
     if (decodeInfo.alphaType() == kUnpremul_SkAlphaType) {
-        // FIXME: Currently we cannot draw unpremultiplied sources.
         decodeInfo = decodeInfo.makeAlphaType(kPremul_SkAlphaType);
     }
 
     SkBitmap bitmap;
-    if (!bitmap.tryAllocPixels(decodeInfo)) {
+    if (!bitmap.tryAllocPixels(decodeInfo, NULL, colorTable.get())) {
         return SkStringPrintf("Image(%s) is too large (%d x %d)\n", fPath.c_str(),
                               decodeInfo.width(), decodeInfo.height());
     }
 
     switch (fMode) {
         case kNormal_Mode:
-            switch (codec->getPixels(decodeInfo, bitmap.getPixels(), bitmap.rowBytes())) {
+            switch (codec->getPixels(decodeInfo, bitmap.getPixels(), bitmap.rowBytes(), NULL,
+                    colorPtr, colorCountPtr)) {
                 case SkImageGenerator::kSuccess:
                     // We consider incomplete to be valid, since we should still decode what is
                     // available.