Swizzler changes Index8 and 565

src/codec/SkCodec_libbmp.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 "SkCodec_libbmp.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
 #include "SkStream.h"
+#include "SkUtils.h"
 
 /*
  *
  * Checks if the conversion between the input image and the requested output
  * image has been implemented
  *
  */
 static bool conversion_possible(const SkImageInfo& dst,
                                 const SkImageInfo& src) {
     // Ensure that the profile type is unchanged
     if (dst.profileType() != src.profileType()) {
         return false;
     }
 
-    // Check for supported color and alpha types
+    // Check for supported alpha types
+    if (src.alphaType() != dst.alphaType() &&
+            (kPremul_SkAlphaType != dst.alphaType() ||
+            kUnpremul_SkAlphaType == src.alphaType())) {

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

I think this is not quite the ! of the original statement, but it is intended to
be, correct?

The prior patch looks like it says:

A || (B && C)

and the new patch says

!A && (!B || C)

I think that

!(A || (B && C))

should be

!A && (!B || !C)

That said, given that we have to think so hard about this multi-conditional
statement, I think we can make this easier to read:

if (src.alphaType() != dst.alphaType()) {
  if (kOpaque_SkAlpha == src.alphaType()) {
    // If the source is opaque, we cannot decode to anything but opaque
    return false;
  }
  // src is opaque.
  switch (dst.alphaType()) {
    case kPremul:
    case kUnpremul:
      // The source is not opaque, so either of these is okay.
      break;
    default:
      // We cannot decode a non-opaque image to opaque (or unknown)
      return false;
  }
}

The switch statement could be an if, and technically you could only check for
premul, since we know the source must be unpremul if it's not opaque. If we
decide to make all our codecs claim their non-opaque images are premul (which is
not correct, but maybe more useful for callers that try to use the default
info), we'd have to go back and change this code if you only check the dst for
unpremul. To catch that, we could assert that src.alphaType() is unpremul.

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

Yeah that statement was confusing and wrong.  Your suggestion is much better.

+        return false;
+    }
+
+    // Check for supported color types
     switch (dst.colorType()) {
+        // Allow output to kN32 from any type of input
         case kN32_SkColorType:
-            return src.alphaType() == dst.alphaType() ||
-                    (kPremul_SkAlphaType == dst.alphaType() &&
-                    kUnpremul_SkAlphaType == src.alphaType());
+            return true;
+        // Allow output to kIndex_8 from compatible inputs
+        case kIndex_8_SkColorType:
+            return kIndex_8_SkColorType == src.colorType();
         default:
             return false;
     }
 }
 
 /*
  *
  * Defines the version and type of the second bitmap header
  *
  */
@@ skipping 370 matching lines @@
         // Header types are matched based on size.  If the header is
         // V3+, we are guaranteed to be able to read at least this size.
         SkASSERT(infoBytesRemaining > 52);
         inputMasks.alpha = get_int(iBuffer.get(), 48);
         if (inputMasks.alpha != 0) {
             alphaType = kUnpremul_SkAlphaType;
         }
     }
     iBuffer.free();
 
-    // Additionally, 32 bit bmp-in-icos use the alpha channel
-    if (isIco && 32 == bitsPerPixel) {
+    // Additionally, 32 bit bmp-in-icos use the alpha channel.
+    // And, RLE inputs may skip pixels, leaving them as transparent.  This
+    // is uncommon, but we cannot be certain that an RLE bmp will be opaque.
+    if ((isIco && 32 == bitsPerPixel) || (kRLE_BitmapInputFormat == inputFormat)) {
         alphaType = kUnpremul_SkAlphaType;
     }
 
-    // Check for valid bits per pixel input
+    // Check for valid bits per pixel.
+    // At the same time, use this information to choose a suggested color type
+    // and to set default masks.
+    SkColorType colorType = kN32_SkColorType;
     switch (bitsPerPixel) {
         // In addition to more standard pixel compression formats, bmp supports
         // the use of bit masks to determine pixel components.  The standard
         // format for representing 16-bit colors is 555 (XRRRRRGGGGGBBBBB),
         // which does not map well to any Skia color formats.  For this reason,
         // we will always enable mask mode with 16 bits per pixel.
         case 16:
             if (kBitMask_BitmapInputFormat != inputFormat) {
                 inputMasks.red = 0x7C00;
                 inputMasks.green = 0x03E0;
                 inputMasks.blue = 0x001F;
                 inputFormat = kBitMask_BitmapInputFormat;
             }
             break;
+        // We want to decode to kIndex_8 for input formats that are already
+        // designed in index format.
         case 1:
         case 2:
         case 4:
         case 8:
+            // However, we cannot in RLE format since we may need to leave some
+            // pixels as transparent.  Similarly, we also cannot for ICO images
+            // since we may need to apply a transparent mask.
+            if (kRLE_BitmapInputFormat != inputFormat && !isIco) {
+                colorType = kIndex_8_SkColorType;
+            }
         case 24:
         case 32:
             break;
         default:
             SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
             return false;
     }
 
     // Check that input bit masks are valid and create the masks object
     SkAutoTDelete<SkMasks>
@@ skipping 12 matching lines @@
 
     // Calculate the number of bytes read so far
     const uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes;
     if (!isIco && offset < bytesRead) {
         SkCodecPrintf("Error: pixel data offset less than header size.\n");
         return false;
     }
 
     if (codecOut) {
         // Return the codec
-        // We will use ImageInfo to store width, height, and alpha type. We
-        // will set color type to kN32_SkColorType because that should be the
-        // default output.
+        // We will use ImageInfo to store width, height, suggested color type, and
+        // suggested alpha type.
         const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
-                kN32_SkColorType, alphaType);
+                colorType, alphaType);
         *codecOut = SkNEW_ARGS(SkBmpCodec, (imageInfo, stream, bitsPerPixel,
                                             inputFormat, masks.detach(),
                                             numColors, bytesPerColor,
                                             offset - bytesRead, rowOrder,
                                             RLEBytes, isIco));
     }
     return true;
 }
 
 /*
@@ skipping 35 matching lines @@
 
 {}
 
 /*
  *
  * Initiates the bitmap decode
  *
  */
 SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         void* dst, size_t dstRowBytes,
-                                        const Options&,
-                                        SkPMColor*, int*) {
+                                        const Options& opts,
+                                        SkPMColor* inputColorPtr,
+                                        int* inputColorCount) {
     // Check for proper input and output formats
     SkCodec::RewindState rewindState = this->rewindIfNeeded();
     if (rewindState == kCouldNotRewind_RewindState) {
         return kCouldNotRewind;
     } else if (rewindState == kRewound_RewindState) {
         if (!ReadHeader(this->stream(), fIsIco, NULL)) {
             return kCouldNotRewind;
         }
     }
     if (dstInfo.dimensions() != this->getInfo().dimensions()) {
         SkCodecPrintf("Error: scaling not supported.\n");
         return kInvalidScale;
     }
     if (!conversion_possible(dstInfo, this->getInfo())) {
         SkCodecPrintf("Error: cannot convert input type to output type.\n");
         return kInvalidConversion;
     }
 
     // Create the color table if necessary and prepare the stream for decode
-    if (!createColorTable(dstInfo.alphaType())) {
+    // Note that if it is non-NULL, inputColorCount will be modified
+    if (!createColorTable(dstInfo.alphaType(), inputColorCount)) {
         SkCodecPrintf("Error: could not create color table.\n");
         return kInvalidInput;
     }
 
+    // Copy the color table to the client if necessary
+    if (kIndex_8_SkColorType == dstInfo.colorType()) {
+        SkASSERT(NULL != inputColorPtr);
+        SkASSERT(NULL != inputColorCount);
+        SkASSERT(NULL != fColorTable.get());
+        sk_memcpy32(inputColorPtr, fColorTable->readColors(), *inputColorCount);
+    }
+
     // Perform the decode
     switch (fInputFormat) {
         case kBitMask_BitmapInputFormat:
             return decodeMask(dstInfo, dst, dstRowBytes);
         case kRLE_BitmapInputFormat:
-            return decodeRLE(dstInfo, dst, dstRowBytes);
+            return decodeRLE(dstInfo, dst, dstRowBytes, opts);
         case kStandard_BitmapInputFormat:
             return decode(dstInfo, dst, dstRowBytes);
         default:
             SkASSERT(false);
             return kInvalidInput;
     }
 }
 
 /*
  *
  * Process the color table for the bmp input
  *
  */
- bool SkBmpCodec::createColorTable(SkAlphaType alphaType) {
+ bool SkBmpCodec::createColorTable(SkAlphaType alphaType, int* numColors) {
     // Allocate memory for color table
     uint32_t colorBytes = 0;
     uint32_t maxColors = 0;
     SkPMColor colorTable[256];
     if (fBitsPerPixel <= 8) {
         // Zero is a default for maxColors
         // Also set fNumColors to maxColors when it is too large
         maxColors = 1 << fBitsPerPixel;
         if (fNumColors == 0 || fNumColors >= maxColors) {
             fNumColors = maxColors;
         }
 
+        // Inform the caller of the number of colors
+        if (NULL != numColors) {
+            SkASSERT(256 == *numColors);
+            *numColors = maxColors;

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

Maybe add a note that we set it to maxColors in case the image file has errors
(i.e. fNumColors was wrong).

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

Done.

+        }
+
         // Read the color table from the stream
         colorBytes = fNumColors * fBytesPerColor;
         SkAutoTDeleteArray<uint8_t> cBuffer(SkNEW_ARRAY(uint8_t, colorBytes));
         if (stream()->read(cBuffer.get(), colorBytes) != colorBytes) {
             SkCodecPrintf("Error: unable to read color table.\n");
             return false;
         }
 
         // Choose the proper packing function
         SkPMColor (*packARGB) (uint32_t, uint32_t, uint32_t, uint32_t);
@@ skipping 121 matching lines @@
 
     // Finished decoding the entire image
     return kSuccess;
 }
 
 /*
  *
  * Set an RLE pixel using the color table
  *
  */
-void SkBmpCodec::setRLEPixel(SkPMColor* dst, size_t dstRowBytes,
+void SkBmpCodec::setRLEPixel(void* dst, size_t dstRowBytes,
                              const SkImageInfo& dstInfo, uint32_t x, uint32_t y,
                              uint8_t index) {
     // Set the row
     int height = dstInfo.height();
     int row;
     if (kBottomUp_RowOrder == fRowOrder) {
         row = height - y - 1;
     } else {
         row = y;
     }
 
     // Set the pixel based on destination color type
     switch (dstInfo.colorType()) {
         case kN32_SkColorType: {
-            SkPMColor* dstRow = SkTAddOffset<SkPMColor>(dst,
+            SkPMColor* dstRow = SkTAddOffset<SkPMColor>((SkPMColor*) dst,
                     row * (int) dstRowBytes);
             dstRow[x] = fColorTable->operator[](index);
             break;
         }
-        case kRGB_565_SkColorType: {
-            uint16_t* dstRow = SkTAddOffset<uint16_t>(dst,
-                    row * (int) dstRowBytes);
-            dstRow[x] = SkPixel32ToPixel16(fColorTable->operator[](index));
-            break;
-        }
         default:
             // This case should not be reached.  We should catch an invalid
             // color type when we check that the conversion is possible.
             SkASSERT(false);
             break;
     }
 }
 
 /*
  *
  * Set an RLE pixel from R, G, B values
  *
  */
-void SkBmpCodec::setRLE24Pixel(SkPMColor* dst, size_t dstRowBytes,
+void SkBmpCodec::setRLE24Pixel(void* dst, size_t dstRowBytes,
                                const SkImageInfo& dstInfo, uint32_t x,
                                uint32_t y, uint8_t red, uint8_t green,
                                uint8_t blue) {
     // Set the row
     int height = dstInfo.height();
     int row;
     if (kBottomUp_RowOrder == fRowOrder) {
         row = height - y - 1;
     } else {
         row = y;
     }
 
     // Set the pixel based on destination color type
     switch (dstInfo.colorType()) {
         case kN32_SkColorType: {
-            SkPMColor* dstRow = SkTAddOffset<SkPMColor>(dst,
+            SkPMColor* dstRow = SkTAddOffset<SkPMColor>((SkPMColor*) dst,
                     row * (int) dstRowBytes);
             dstRow[x] = SkPackARGB32NoCheck(0xFF, red, green, blue);
             break;
         }
-        case kRGB_565_SkColorType: {
-            uint16_t* dstRow = SkTAddOffset<uint16_t>(dst,
-                    row * (int) dstRowBytes);
-            dstRow[x] = SkPack888ToRGB16(red, green, blue);
-            break;
-        }
         default:
             // This case should not be reached.  We should catch an invalid
             // color type when we check that the conversion is possible.
             SkASSERT(false);
             break;
     }
 }
 
 /*
  *
  * Performs the bitmap decoding for RLE input format
  * RLE decoding is performed all at once, rather than a one row at a time
  *
  */
 SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo,
-                                      void* dst, size_t dstRowBytes) {
+                                      void* dst, size_t dstRowBytes,
+                                      const Options& opts) {
     // Set RLE flags
     static const uint8_t RLE_ESCAPE = 0;
     static const uint8_t RLE_EOL = 0;
     static const uint8_t RLE_EOF = 1;
     static const uint8_t RLE_DELTA = 2;
 
     // Set constant values
     const int width = dstInfo.width();
     const int height = dstInfo.height();
 
     // Input buffer parameters
     uint32_t currByte = 0;
     SkAutoTDeleteArray<uint8_t> buffer(SkNEW_ARRAY(uint8_t, fRLEBytes));
     size_t totalBytes = stream()->read(buffer.get(), fRLEBytes);
     if (totalBytes < fRLEBytes) {
         SkCodecPrintf("Warning: incomplete RLE file.\n");
     } else if (totalBytes <= 0) {
         SkCodecPrintf("Error: could not read RLE image data.\n");
         return kInvalidInput;
     }
 
     // Destination parameters
     int x = 0;
     int y = 0;
-    // If the code skips pixels, remaining pixels are transparent or black
-    // TODO: Skip this if memory was already zeroed.
-    memset(dst, 0, dstRowBytes * height);
-    SkPMColor* dstPtr = (SkPMColor*) dst;
+
+    // Set the background as transparent.  Then, if the RLE code skips pixels,
+    // the skipped pixels will be transparent.
+    // Because of the need for transparent pixels, kN32 is the only color
+    // type that makes sense for the destination format.
+    SkASSERT(kN32_SkColorType == dstInfo.colorType());
+    if (kNo_ZeroInitialized == opts.fZeroInitialized) {
+        SkSwizzler::Fill(dst, dstInfo, dstRowBytes, 0, SK_ColorTRANSPARENT, NULL);
+    }
 
     while (true) {
         // Every entry takes at least two bytes
         if ((int) totalBytes - currByte < 2) {
             SkCodecPrintf("Warning: incomplete RLE input.\n");
             return kIncompleteInput;
         }
 
         // Read the next two bytes.  These bytes have different meanings
         // depending on their values.  In the first interpretation, the first
@@ skipping 50 matching lines @@
                             (int) totalBytes - currByte < SkAlign2(rowBytes)) {
                         SkCodecPrintf("Warning: invalid RLE input.\n");
                         return kIncompleteInput;
                     }
                     // Set numPixels number of pixels
                     while (numPixels > 0) {
                         switch(fBitsPerPixel) {
                             case 4: {
                                 SkASSERT(currByte < totalBytes);
                                 uint8_t val = buffer.get()[currByte++];
-                                setRLEPixel(dstPtr, dstRowBytes, dstInfo, x++,
+                                setRLEPixel(dst, dstRowBytes, dstInfo, x++,
                                         y, val >> 4);
                                 numPixels--;
                                 if (numPixels != 0) {
-                                    setRLEPixel(dstPtr, dstRowBytes, dstInfo,
+                                    setRLEPixel(dst, dstRowBytes, dstInfo,
                                             x++, y, val & 0xF);
                                     numPixels--;
                                 }
                                 break;
                             }
                             case 8:
                                 SkASSERT(currByte < totalBytes);
-                                setRLEPixel(dstPtr, dstRowBytes, dstInfo, x++,
+                                setRLEPixel(dst, dstRowBytes, dstInfo, x++,
                                         y, buffer.get()[currByte++]);
                                 numPixels--;
                                 break;
                             case 24: {
                                 SkASSERT(currByte + 2 < totalBytes);
                                 uint8_t blue = buffer.get()[currByte++];
                                 uint8_t green = buffer.get()[currByte++];
                                 uint8_t red = buffer.get()[currByte++];
-                                setRLE24Pixel(dstPtr, dstRowBytes, dstInfo,
+                                setRLE24Pixel(dst, dstRowBytes, dstInfo,
                                             x++, y, red, green, blue);
                                 numPixels--;
                             }
                             default:
                                 SkASSERT(false);
                                 return kInvalidInput;
                         }
                     }
                     // Skip a byte if necessary to maintain alignment
                     if (!SkIsAlign2(rowBytes)) {
@@ skipping 15 matching lines @@
                 if ((int) totalBytes - currByte < 2) {
                     SkCodecPrintf("Warning: incomplete RLE input\n");
                     return kIncompleteInput;
                 }
 
                 // Fill the pixels up to endX with the specified color
                 uint8_t blue = task;
                 uint8_t green = buffer.get()[currByte++];
                 uint8_t red = buffer.get()[currByte++];
                 while (x < endX) {
-                    setRLE24Pixel(dstPtr, dstRowBytes, dstInfo, x++, y, red,
+                    setRLE24Pixel(dst, dstRowBytes, dstInfo, x++, y, red,
                             green, blue);
                 }
             } else {
                 // In RLE8 or RLE4, the second byte read gives the index in the
                 // color table to look up the pixel color.
                 // RLE8 has one color index that gets repeated
                 // RLE4 has two color indexes in the upper and lower 4 bits of
                 // the bytes, which are alternated
                 uint8_t indices[2] = { task, task };
                 if (4 == fBitsPerPixel) {
                     indices[0] >>= 4;
                     indices[1] &= 0xf;
                 }
 
                 // Set the indicated number of pixels
                 for (int which = 0; x < endX; x++) {
-                    setRLEPixel(dstPtr, dstRowBytes, dstInfo, x, y,
+                    setRLEPixel(dst, dstRowBytes, dstInfo, x, y,
                             indices[which]);
                     which = !which;
                 }
             }
         }
     }
 }
 
 /*
  *
@@ skipping 69 matching lines @@
     }
 
     // FIXME: This code exists to match the behavior in the chromium decoder
     // and to follow the bmp specification as it relates to alpha masks.  It is
     // commented out because we have yet to discover a test image that provides
     // an alpha mask and uses this decode mode.
 
     // Now we adjust the output image with some additional behavior that
     // SkSwizzler does not support.  Firstly, all bmp images that contain
     // alpha are masked by the alpha mask.  Secondly, many fully transparent
-    // bmp images are intended to be opaque.  Here, we make those corrections.
+    // bmp images are intended to be opaque.  Here, we make those corrections
+    // in the kN32 case.
     /*
     SkPMColor* dstRow = (SkPMColor*) dst;
     if (SkSwizzler::kBGRA == config) {
         for (int y = 0; y < height; y++) {
             for (int x = 0; x < width; x++) {
                 if (transparent) {
                     dstRow[x] |= 0xFF000000;
                 } else {
                     dstRow[x] &= alphaMask;
                 }
@@ skipping 34 matching lines @@
                 uint32_t alphaBit =
                         (srcBuffer.get()[quotient] >> shift) & 0x1;
                 dstRow[x] &= alphaBit - 1;
             }
         }
     }
 
     // Finished decoding the entire image
     return kSuccess;
 }