SkJpegCodec

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"
@@ skipping 79 matching lines @@
     kCMYK4BitRLE_BitmapCompressionMethod =   13
 };
 
 /*
  *
  * Checks the start of the stream to see if the image is a bitmap
  *
  */
 bool SkBmpCodec::IsBmp(SkStream* stream) {
     // TODO: Support "IC", "PT", "CI", "CP", "BA"
-    // TODO: ICO files may contain a BMP and need to use this decoder
     const char bmpSig[] = { 'B', 'M' };
     char buffer[sizeof(bmpSig)];
     return stream->read(buffer, sizeof(bmpSig)) == sizeof(bmpSig) &&
             !memcmp(buffer, bmpSig, sizeof(bmpSig));
 }
 
 /*
  *
  * Assumes IsBmp was called and returned true
  * Creates a bmp decoder
@@ skipping 496 matching lines @@
     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, opts);

[scroggo, 2015/04/14 13:10:32]

nit: Rebasing between uploads can make it harder to tell what has changed
between revisions. (This line changed in a different CL.)

If I need to rebase, I'll often upload the rebase without any other changes, to
isolate the changes that were made by me for this code review.

[msarett, 2015/04/14 19:30:36]

Sorry about that.  Good to know.

         case kRLE_BitmapInputFormat:
             return decodeRLE(dstInfo, dst, dstRowBytes, opts);
         case kStandard_BitmapInputFormat:
             return decode(dstInfo, dst, dstRowBytes, opts);
         default:
             SkASSERT(false);
             return kInvalidInput;
     }
 }
 
@@ skipping 138 matching lines @@
             SkMaskSwizzler::CreateMaskSwizzler(dstInfo, dst, dstRowBytes,
             fMasks, fBitsPerPixel));
 
     // Iterate over rows of the image
     bool transparent = true;
     for (int y = 0; y < height; y++) {
         // Read a row of the input
         if (stream()->read(srcRow, rowBytes) != rowBytes) {
             SkCodecPrintf("Warning: incomplete input stream.\n");
             // Fill the destination image on failure
-            // By using zero as the fill value, we will fill with transparent
-            // pixels for non-opaque images and white for opaque images.
-            // These are arbitrary choices but allow for consistent behavior.
+            SkPMColor fillColor = dstInfo.alphaType() == kOpaque_SkAlphaType ?
+                    SK_ColorBLACK : SK_ColorTRANSPARENT;
             if (kNo_ZeroInitialized == opts.fZeroInitialized) {
                 void* dstStart = get_dst_start_row(dst, dstRowBytes, y, fRowOrder);
-                SkSwizzler::Fill(dstStart, dstInfo, dstRowBytes, dstInfo.height() - y, 0, NULL);
+                SkSwizzler::Fill(dstStart, dstInfo, dstRowBytes, dstInfo.height() - y, fillColor,
+                        NULL);
             }
             return kIncompleteInput;
         }
 
         // Decode the row in destination format
         int row = kBottomUp_RowOrder == fRowOrder ? height - 1 - y : y;
         SkSwizzler::ResultAlpha r = maskSwizzler->next(srcRow, row);
         transparent &= SkSwizzler::IsTransparent(r);
 
         // Move to the next row
@@ skipping 291 matching lines @@
  *
  */
 SkCodec::Result SkBmpCodec::decode(const SkImageInfo& dstInfo,
                                    void* dst, size_t dstRowBytes,
                                    const Options& opts) {
     // Set constant values
     const int width = dstInfo.width();
     const int height = dstInfo.height();
     const size_t rowBytes = SkAlign4(compute_row_bytes(width, fBitsPerPixel));
 
-    // Get swizzler configuration
+    // Get swizzler configuration and choose the fill value for failures.  We will use
+    // zero as the default palette index, black for opaque images, and transparent for
+    // non-opaque images.
     SkSwizzler::SrcConfig config;
+    uint32_t fillColorOrIndex;
     switch (fBitsPerPixel) {
         case 1:
             config = SkSwizzler::kIndex1;
+            fillColorOrIndex = 0;
             break;
         case 2:
             config = SkSwizzler::kIndex2;
+            fillColorOrIndex = 0;
             break;
         case 4:
             config = SkSwizzler::kIndex4;
+            fillColorOrIndex = 0;
             break;
         case 8:
             config = SkSwizzler::kIndex;
+            fillColorOrIndex = 0;
             break;
         case 24:
             config = SkSwizzler::kBGR;
+            fillColorOrIndex = SK_ColorBLACK;
             break;
         case 32:
             if (kOpaque_SkAlphaType == dstInfo.alphaType()) {
                 config = SkSwizzler::kBGRX;
+                fillColorOrIndex = SK_ColorBLACK;
             } else {
                 config = SkSwizzler::kBGRA;
+                fillColorOrIndex = SK_ColorTRANSPARENT;
             }
             break;
         default:
             SkASSERT(false);
             return kInvalidInput;
     }
 
     // Get a pointer to the color table if it exists
     const SkPMColor* colorPtr = NULL != fColorTable.get() ? fColorTable->readColors() : NULL;
 
     // Create swizzler
     SkAutoTDelete<SkSwizzler> swizzler(SkSwizzler::CreateSwizzler(config,
             colorPtr, dstInfo, dst, dstRowBytes,
             SkImageGenerator::kNo_ZeroInitialized));
 
     // Allocate space for a row buffer and a source for the swizzler
     SkAutoTDeleteArray<uint8_t> srcBuffer(SkNEW_ARRAY(uint8_t, rowBytes));
 
     // Iterate over rows of the image
     // FIXME: bool transparent = true;
     for (int y = 0; y < height; y++) {
         // Read a row of the input
         if (stream()->read(srcBuffer.get(), rowBytes) != rowBytes) {
             SkCodecPrintf("Warning: incomplete input stream.\n");
             // Fill the destination image on failure
-            // By using zero as the fill value, we will fill with the first
-            // color in the color table for palette images, transparent
-            // pixels for non-opaque images, and white for opaque images.
-            // These are arbitrary choices but allow for consistent behavior.
             if (kNo_ZeroInitialized == opts.fZeroInitialized) {

[scroggo, 2015/04/14 13:10:32]

Since fillColorOrIndex may not be 0, being zero initialized does not necessarily
mean we can skip this.

We either need to do it all the time, or we need to make extra checks to know
whether we can skip (maybe included in the switch statement above).

[msarett, 2015/04/14 19:30:36]

Yes you are right!

                 void* dstStart = get_dst_start_row(dst, dstRowBytes, y, fRowOrder);
-                SkSwizzler::Fill(dstStart, dstInfo, dstRowBytes, dstInfo.height() - y, 0,
-                        colorPtr);
+                SkSwizzler::Fill(dstStart, dstInfo, dstRowBytes, dstInfo.height() - y,
+                        fillColorOrIndex, colorPtr);
             }
             return kIncompleteInput;
         }
 
         // Decode the row in destination format
         uint32_t row;
         if (kTopDown_RowOrder == fRowOrder) {
             row = y;
         } else {
             row = height - 1 - y;
@@ skipping 62 matching lines @@
                 uint32_t alphaBit =
                         (srcBuffer.get()[quotient] >> shift) & 0x1;
                 dstRow[x] &= alphaBit - 1;
             }
         }
     }
 
     // Finished decoding the entire image
     return kSuccess;
 }