Swizzler changes Index8 and 565

src/codec/SkSwizzler.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 "SkCodecPriv.h"
 #include "SkColorPriv.h"
 #include "SkSwizzler.h"
 #include "SkTemplates.h"
+#include "SkUtils.h"
 
 SkSwizzler::ResultAlpha SkSwizzler::GetResult(uint8_t zeroAlpha,
                                               uint8_t maxAlpha) {
     // In the transparent case, this returns 0x0000
     // In the opaque case, this returns 0xFFFF
     // If the row is neither transparent nor opaque, returns something else
     return (((uint16_t) maxAlpha) << 8) | zeroAlpha;
 }
 
 // kIndex1, kIndex2, kIndex4
 
-static SkSwizzler::ResultAlpha swizzle_small_index_to_n32(
+static SkSwizzler::ResultAlpha swizzle_small_index_to_index(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
         int bitsPerPixel, int y, const SkPMColor ctable[]) {
 
-    SkPMColor* SK_RESTRICT dst = (SkPMColor*) dstRow;
+    uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
     INIT_RESULT_ALPHA;
     const uint32_t pixelsPerByte = 8 / bitsPerPixel;
     const size_t rowBytes = compute_row_bytes_ppb(width, pixelsPerByte);
     const uint8_t mask = (1 << bitsPerPixel) - 1;
     int x = 0;
     for (uint32_t byte = 0; byte < rowBytes; byte++) {
         uint8_t pixelData = src[byte];
         for (uint32_t p = 0; p < pixelsPerByte && x < width; p++) {
             uint8_t index = (pixelData >> (8 - bitsPerPixel)) & mask;
-            SkPMColor c = ctable[index];
-            UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT);
-            dst[x] = c;
+            UPDATE_RESULT_ALPHA(ctable[index] >> SK_A32_SHIFT);
+            dst[x] = index;
             pixelData <<= bitsPerPixel;
             x++;
         }
     }
     return COMPUTE_RESULT_ALPHA;
 }
 
-static SkSwizzler::ResultAlpha swizzle_small_index_to_565(
+static SkSwizzler::ResultAlpha swizzle_small_index_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
         int bitsPerPixel, int y, const SkPMColor ctable[]) {
 
-    uint16_t* SK_RESTRICT dst = (uint16_t*) dstRow;
+    SkPMColor* SK_RESTRICT dst = (SkPMColor*) dstRow;
+    INIT_RESULT_ALPHA;
     const uint32_t pixelsPerByte = 8 / bitsPerPixel;
     const size_t rowBytes = compute_row_bytes_ppb(width, pixelsPerByte);
     const uint8_t mask = (1 << bitsPerPixel) - 1;
     int x = 0;
     for (uint32_t byte = 0; byte < rowBytes; byte++) {
         uint8_t pixelData = src[byte];
         for (uint32_t p = 0; p < pixelsPerByte && x < width; p++) {
             uint8_t index = (pixelData >> (8 - bitsPerPixel)) & mask;
-            uint16_t c = SkPixel32ToPixel16(ctable[index]);
+            SkPMColor c = ctable[index];
+            UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT);
             dst[x] = c;
             pixelData <<= bitsPerPixel;
             x++;
         }
     }
-    return SkSwizzler::kOpaque_ResultAlpha;
+    return COMPUTE_RESULT_ALPHA;
+}
+
+static SkSwizzler::ResultAlpha swizzle_index_to_index(
+        void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
+        int bytesPerPixel, int y, const SkPMColor ctable[]) {
+
+    uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
+    memcpy(dst, src, width);
+    // TODO (msarett): Should we skip the loop here and guess that the row is opaque/not opaque?
+    //                 SkScaledBitmap sampler just guesses that it is opaque.  This is dangerous
+    //                 and probably wrong since gif and bmp (rarely) may have alpha.
+    INIT_RESULT_ALPHA;
+    for (int x = 0; x < width; x++) {
+        UPDATE_RESULT_ALPHA(ctable[src[x]] >> SK_A32_SHIFT);
+    }
+    return COMPUTE_RESULT_ALPHA;
 }
 
 // kIndex
 
 static SkSwizzler::ResultAlpha swizzle_index_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
         int bytesPerPixel, int y, const SkPMColor ctable[]) {
 
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
     INIT_RESULT_ALPHA;
     for (int x = 0; x < width; x++) {
-        SkPMColor c = ctable[*src];
+        SkPMColor c = ctable[src[x]];
         UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT);
         dst[x] = c;
-        src++;
     }
     return COMPUTE_RESULT_ALPHA;
 }
 
 static SkSwizzler::ResultAlpha swizzle_index_to_n32_skipZ(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
         int bytesPerPixel, int y, const SkPMColor ctable[]) {
 
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
     INIT_RESULT_ALPHA;
     for (int x = 0; x < width; x++) {
-        SkPMColor c = ctable[*src];
+        SkPMColor c = ctable[src[x]];
         UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT);
         if (c != 0) {
             dst[x] = c;
         }
-        src++;
     }
     return COMPUTE_RESULT_ALPHA;
 }
 
-static SkSwizzler::ResultAlpha swizzle_index_to_565(
-        void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
-        int bytesPerPixel, int y, const SkPMColor ctable[]) {
-
-    uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
-    for (int x = 0; x < width; x++) {
-        uint16_t c = SkPixel32ToPixel16(ctable[*src]);
-        dst[x] = c;
-        src++;
-    }
-    return SkSwizzler::kOpaque_ResultAlpha;
-}
-
 #undef A32_MASK_IN_PLACE
 
 static SkSwizzler::ResultAlpha swizzle_bgrx_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
         int bytesPerPixel, int y, const SkPMColor ctable[]) {
 
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
     for (int x = 0; x < width; x++) {
         dst[x] = SkPackARGB32NoCheck(0xFF, src[2], src[1], src[0]);
         src += bytesPerPixel;
     }
     return SkSwizzler::kOpaque_ResultAlpha;
 }
 
-static SkSwizzler::ResultAlpha swizzle_bgrx_to_565(
-        void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
-        int bytesPerPixel, int y, const SkPMColor ctable[]) {
-
-    uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
-    for (int x = 0; x < width; x++) {
-        dst[x] = SkPack888ToRGB16(src[2], src[1], src[0]);
-        src += bytesPerPixel;
-    }
-    return SkSwizzler::kOpaque_ResultAlpha;
-}
-
 // kBGRA
 
 static SkSwizzler::ResultAlpha swizzle_bgra_to_n32_unpremul(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
         int bytesPerPixel, int y, const SkPMColor ctable[]) {
 
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
     INIT_RESULT_ALPHA;
     for (int x = 0; x < width; x++) {
         uint8_t alpha = src[3];
@@ skipping 123 matching lines @@
     }
     RowProc proc = NULL;
     switch (sc) {
         case kIndex1:
         case kIndex2:
         case kIndex4:
             switch (info.colorType()) {
                 case kN32_SkColorType:
                     proc = &swizzle_small_index_to_n32;
                     break;
-                case kRGB_565_SkColorType:
-                    proc = &swizzle_small_index_to_565;
+                case kIndex_8_SkColorType:
+                    proc = &swizzle_small_index_to_index;
                     break;
                 default:
                     break;
             }
             break;
         case kIndex:
             switch (info.colorType()) {
                 case kN32_SkColorType:
                     // We assume the color premultiplied ctable (or not) as desired.
                     if (SkImageGenerator::kYes_ZeroInitialized == zeroInit) {
                         proc = &swizzle_index_to_n32_skipZ;
                         break;
                     } else {
                         proc = &swizzle_index_to_n32;
                         break;
                     }
                     break;
-                case kRGB_565_SkColorType:
-                    proc = &swizzle_index_to_565;
+                case kIndex_8_SkColorType:
+                    proc = &swizzle_index_to_index;
                     break;
                 default:
                     break;
             }
             break;
         case kBGR:
         case kBGRX:
             switch (info.colorType()) {
                 case kN32_SkColorType:
                     proc = &swizzle_bgrx_to_n32;
                     break;
-                case kRGB_565_SkColorType:
-                    proc = &swizzle_bgrx_to_565;
-                    break;
                 default:
                     break;
             }
             break;
         case kBGRA:
             switch (info.colorType()) {
                 case kN32_SkColorType:
                     switch (info.alphaType()) {
                         case kUnpremul_SkAlphaType:
                             proc = &swizzle_bgra_to_n32_unpremul;
@@ skipping 96 matching lines @@
     SkASSERT(kConsecutive_NextMode != fNextMode);
     SkDEBUGCODE(fNextMode = kDesignateRow_NextMode);
     
     // Choose the row
     void* row = SkTAddOffset<void>(fDstRow, y*fDstRowBytes);
 
     // Decode the row
     return fRowProc(row, src, fDstInfo.width(), fDeltaSrc, fCurrY,
             fColorTable);
 }
+
+void SkSwizzler::Fill(void* dst, const SkImageInfo& dstInfo, size_t dstRowBytes, uint32_t y,
+        uint32_t input, SkPMColor* colorTable) {
+    SkASSERT(dst != NULL);
+
+    // Get dst start row
+    void* dstRow = SkTAddOffset<void*>(dst, y * dstRowBytes);
+
+    // Calculate remaining bytes.  This is tricky since the final row may not be padded.
+    size_t totalBytes = dstInfo.getSafeSize(dstRowBytes);

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

const

[msarett, 2015/04/06 18:10:56]

Done.

+    size_t bytes = totalBytes - y * dstRowBytes;

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

const.

Also, Maybe this is better described as remainingBytes?

[msarett, 2015/04/06 18:10:55]

Agreed

+
+    // Use the proper memset routine to fill the remaining bytes
+    switch(dstInfo.colorType()) {
+        case kN32_SkColorType:
+            // Assume input is an index if we have a color table
+            if (NULL != colorTable) {

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

My original version passed a bool parameter indicating whether the input was an
index into a color table or a color.  Instead, if the color table is non-NULL,
we assume it's an index.  I can't decide if this is confusing or clever.

FWIW: This function looks a lot cleaner if I implement it as non-static (doesn't
need a ton of parameters).  This version would be useful in one place in
SkGifCodec and one place in SkJpegCodec.

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

It is a little bit confusing, although I think that's better than having
redundant parameters. A comment in the header will help with any confusion
though.
What is the advantage to doing it this way?

[msarett, 2015/04/06 18:10:55]

Added the comment.

I don't think there isn't a great advantage to a non-static version, and given
that it wouldn't be usable in all cases, it probably isn't worthwhile to
consider.

+                sk_memset32((uint32_t*) dstRow, colorTable[(uint8_t) input],

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

Is this cast just in case? I think it might be interesting to assert that
(uint8_t) input == input.

[msarett, 2015/04/06 18:10:55]

Done.

+                        bytes / sizeof(SkPMColor));

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

This seems potentially dangerous. In general, I would expect rowBytes to be
divisible by 4, in which case this is safe. That said, we allow the caller to
specify, so they could supply some weird number (maybe they have their
reasons?). If the padding on the end was not a multiple of 4, we would end up
not setting our colors properly.

That said, this is likely faster than doing a loop over each row. Maybe do a
check that this is safe based on the rowBytes, and if not, have a backup option
which handles each row, with a comment that (and maybe even an SkCodecPrintf)
that this is a weird rowBytes and an unlikely case.

[msarett, 2015/04/06 18:10:55]

Done

+            // Otherwise, assume the input is a color
+            } else {
+                sk_memset32((uint32_t*) dstRow, input, bytes / sizeof(SkPMColor));
+            }
+            break;
+        // On an index destination color type, always assume the input is an index
+        case kIndex_8_SkColorType:
+            memset(dstRow, (uint8_t) input, bytes);
+            break;
+        default:
+            SkCodecPrintf("Warning: Unsupported dst color type for fill().  Doing nothing.\n");

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

Maybe SkASSERT(false);

[msarett, 2015/04/06 18:10:55]

Done.

+            break;
+    }
+}