Bmp Image Decoding

src/codec/SkCodec_libbmp.h

Index: src/codec/SkCodec_libbmp.h
diff --git a/src/codec/SkCodec_libbmp.h b/src/codec/SkCodec_libbmp.h
new file mode 100644
index 0000000000000000000000000000000000000000..5cf9018eb3a1642d89a36a20d10f1046edea2af2
--- /dev/null
+++ b/src/codec/SkCodec_libbmp.h
@@ -0,0 +1,111 @@
+/*
+ * 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.h"
+#include "SkImageInfo.h"
+#include "SkStream.h"
+#include "SkSwizzler.h"
+#include "SkTypes.h"
+
+#ifdef SK_SUPPORT_LEGACY_IMAGE_GENERATOR_RETURN
+    #define onGetPixels onGetPixelsEnum
+#endif
+
+/*
+ *
+ * This class implements the decoding for bmp images
+ *
+ */
+class SkBmpCodec : public SkCodec {
+public:
+
+    /*
+     *
+     * Checks the start of the stream to see if the image is a bitmap
+     *
+     */
+    static bool IsBmp(SkStream* stream);

[scroggo, 2015/02/27 17:04:03]

Nit: typically I think we leave out the parameter name from a declaration if it
does not provide any information. (Sorry, I could've been more clear when I
requested you use names. Names are useful when it's something like

size_t dstRowBytes

but not needed when it's something like

ClassName className.)

+
+    /*
+     *
+     * Assumes IsBmp was called and returned true
+     * Creates a bitmap decoder
+     * Reads enough of the stream to determine the image format
+     *
+     */
+    static SkCodec* NewFromStream(SkStream* stream);
+
+protected:
+
+    /*
+     *
+     * Initiates the bitmap decode
+     *
+     */
+    virtual Result onGetPixels(const SkImageInfo& dstInfo, void* dst,
+                               size_t dstRowBytes, SkPMColor* colorTable,
+                               int*) SK_OVERRIDE;
+
+private:
+
+    /*
+     *
+     * Used to define the input format of the bitmap
+     *
+     */
+    enum BitmapInputFormat {
+        kStandard_BitmapInputFormat,
+        k4BitRLE_BitmapInputFormat,
+        k8BitRLE_BitmapInputFormat,
+        kBitMask_BitmapInputFormat,
+        kUnknown_BitmapInputFormat
+    };
+
+    /*
+     *
+     * Set an RLE pixel using the color table
+     *
+     */
+    void setPixel(SkPMColor* dst, uint32_t dstRowBytes, int height,
+                  uint32_t x, uint32_t y, uint8_t index);
+
+    /*
+     *
+     * Performs the bitmap decoding for standard and bit masks input format
+     *
+     */
+    Result decode(const SkImageInfo& dstInfo, void* dst, uint32_t dstRowBytes);
+
+    /*
+     *
+     * Performs the bitmap decoding for RLE input format
+     *
+     */
+    Result decodeRLE(const SkImageInfo& dstInfo, void* dst,
+                     uint32_t dstRowBytes);
+
+    /*
+     *
+     * Creates an instance of the decoder
+     * Called only by NewFromStream
+     *
+     */
+    SkBmpCodec(const SkImageInfo& srcInfo, SkStream* stream,
+               uint16_t bitsPerPixel, BitmapInputFormat format,
+               SkSwizzler::ColorMasks* bitMasks, SkPMColor* colorTable,

[scroggo, 2015/02/27 17:04:03]

Could you add some comments describing which of these parameters pass ownership
to the SkBmpCodec?

Also some comments describing what they mean? In particular, inverted, which I
think means in y (and maybe it should be an enum, in which case the names should
serve as documentation), and remainingBytes (in the stream? How do we know?).

+               bool inverted, uint32_t remainingBytes);
+
+    // Fields
+    const uint16_t                              fBitsPerPixel;
+    const BitmapInputFormat                     fInputFormat;
+    const SkAutoTDelete<SkSwizzler::ColorMasks> fBitMasks;
+    const SkAutoTDeleteArray<SkPMColor>         fColorTable;
+    const bool                                  fInverted;
+    const uint32_t                              fRemainingBytes;
+
+    typedef SkCodec INHERITED;
+};