Bmp Image Decoding

src/codec/SkSwizzler.h

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef SkSwizzler_DEFINED
 #define SkSwizzler_DEFINED
 
 #include "SkTypes.h"
 #include "SkColor.h"
 #include "SkImageInfo.h"
 
 class SkSwizzler : public SkNoncopyable {
 public:
     /**
      *  Enum describing the config of the source data.
      */
     enum SrcConfig {
-        kGray,  // 1 byte per pixel
-        kIndex, // 1 byte per pixel
-        kRGB,   // 3 bytes per pixel
-        kRGBX,  // 4 byes per pixel (ignore 4th)
-        kRGBA,  // 4 bytes per pixel
-        kRGB_565 // 2 bytes per pixel
+        kGray,
+        kIndex1,

[scroggo, 2015/02/25 17:22:43]

Alternatively, we could make the caller specify how big a pixel is, and just
have kIndex.

+        kIndex2,
+        kIndex4,
+        kIndex8,
+        kRGB,
+        kBGR,
+        kRGBX,
+        kBGRX,
+        kRGBA,
+        kBGRA,
+        kRGB_565,
+        kMask16,
+        kMask24,
+        kMask32
     };
 
     static int BytesPerPixel(SrcConfig sc) {
         switch (sc) {
+            case kIndex1:
+            case kIndex2:
+            case kIndex4:
+                return 0; // Flag used when pixels are less than a byte

[scroggo, 2015/02/25 17:22:43]

I don't like having both BitsPerPixel and BytesPerPixel. Can we convert all
callers to use BitsPerPixel? (Or maybe there's another solution?)

             case kGray:
-            case kIndex:
+            case kIndex8:
                 return 1;
             case kRGB:
+            case kBGR:
+            case kMask24:
                 return 3;
             case kRGBX:
+            case kBGRX:
             case kRGBA:
+            case kBGRA:
+            case kMask32:
                 return 4;
             case kRGB_565:
+            case kMask16:
                 return 2;
         }
     }
+    
+    static int BitsPerPixel(SrcConfig sc) {
+        switch (sc) {
+            case kIndex1:
+                return 1;
+            case kIndex2:
+                return 2;
+            case kIndex4:
+                return 4;
+            default:
+                return BytesPerPixel(sc) * 8;
+        }
+    }
 
     /**
      *  Create a new SkSwizzler.
      *  @param sc SrcConfig
      *  @param info dimensions() describe both the src and the dst.
      *              Other fields describe the dst.
      *  @param dst Destination to write pixels. Must match info and dstRowBytes
      *  @param dstRowBytes rowBytes for dst.
      *  @param skipZeroes Whether to skip writing zeroes. Useful if dst is
      *              zero-initialized. The implementation may or may not respect this.
+     *  @param bitMasks Custom bit masks used to interpret pixels

[scroggo, 2015/02/25 17:22:43]

What does custom mean?

[scroggo, 2015/02/25 17:22:43]

What does this mean? Does the swizzler always do the same thing with it? If so,
it should be described here

+     *  @param fixAlpha Whether to monitor for fully transparent images and set
+     *              them as opaque
      *  @return A new SkSwizzler or NULL on failure.
      */
     static SkSwizzler* CreateSwizzler(SrcConfig sc, const SkPMColor* ctable,
                                       const SkImageInfo& info, void* dst,
-                                      size_t dstRowBytes, bool skipZeroes);
+                                      size_t dstRowBytes, bool skipZeroes,
+                                      const uint32_t* bitMasks = NULL,
+                                      const bool fixAlpha = false,
+                                      const bool inverted = false);

[scroggo, 2015/02/25 17:22:43]

Can you add a comment explaining "inverted"?

I shouldn't have made skipZeroes a boolean. We tend to encourage using enums so
the call site is clearer. With three booleans, now, it's hard to tell what's
what.

     /**
      *  Swizzle the next line. Call height times, once for each row of source.
      *  @param src The next row of the source data.
      *  @return Whether the row had non-opaque alpha.
      */
     bool next(const uint8_t* SK_RESTRICT src);
 private:
     /**
      *  Method for converting raw data to Skia pixels.
      *  @param dstRow Row in which to write the resulting pixels.
      *  @param src Row of src data, in format specified by SrcConfig
      *  @param width Width in pixels
      *  @param bpp bytes per pixel of the source.
      *  @param y Line of source.
      *  @param ctable Colors (used for kIndex source).
      */
     typedef bool (*RowProc)(void* SK_RESTRICT dstRow,
                             const uint8_t* SK_RESTRICT src,
-                            int width, int bpp, int y,
-                            const SkPMColor ctable[]);
+                            int width, int bpp, int bitsPerPixel, int y,
+                            const SkPMColor ctable[], const uint32_t* masks,

[scroggo, 2015/02/25 17:22:43]

please add comments for all of these.

+                            const bool fixAlpha, bool* fSeenNonZeroAlphaPtr,

[scroggo, 2015/02/25 17:22:43]

fCamelCase is reserved for member fields. Parameter names should be camelCase
(e.g. seenNonZeroAlphaPtr).

[msarett, 2015/02/26 23:58:18]

I was trying to draw attention to the fact that I was using ptrs to fields,
which I think is pretty hacky.  Any thoughts on this?

+                            bool* fZeroPrevRowsPtr);
 
     const RowProc       fRowProc;
     const SkPMColor*    fColorTable;    // Unowned pointer
+    const uint32_t*     fBitMasks;      // Unowned pointer
     const int           fSrcPixelSize;
+    const int           fSrcBitsPerPixel;
     const SkImageInfo   fDstInfo;
     void*               fDstRow;
     const size_t        fDstRowBytes;
     int                 fCurrY;
+    const bool          fFixAlpha;
+    const bool          fInverted;
+    bool                fSeenNonZeroAlpha;
+    bool                fZeroPrevRows;
 
-    SkSwizzler(RowProc proc, const SkPMColor* ctable, int srcBpp,
-               const SkImageInfo& info, void* dst, size_t rowBytes);
+    SkSwizzler(RowProc proc, const SkPMColor* ctable, int srcBpp, int srcBitsPP,
+               const SkImageInfo& info, void* dst, size_t rowBytes,
+               const uint32_t* bitMasks, const bool fixAlpha,
+               const bool inverted);
 
 };
 #endif // SkSwizzler_DEFINED