Split SkBmpCodec into three separate classes

src/codec/SkBmpMaskCodec.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.
+ */
+
+#include "SkBmpCodec.h"
+#include "SkCodec.h"
+#include "SkColorTable.h"

[scroggo, 2015/07/31 15:05:43]

I think this is not needed?

[msarett, 2015/08/03 22:52:35]

Done.

+#include "SkImageInfo.h"
+#include "SkMaskSwizzler.h"
+#include "SkStream.h"

[scroggo, 2015/07/31 15:05:43]

This can be forward declared.

[msarett, 2015/08/03 22:52:35]

We actually don't need to include it at all.

Out of curiosity, what is the advantage of a forward declaration vs an include?

[scroggo, 2015/08/04 16:16:46]

Google's style guide has a nice succinct section on the advantages and
disadvantages of forward declarations:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Forward_Decla...

+#include "SkSwizzler.h"

[scroggo, 2015/07/31 15:05:43]

Is this necessary?

[msarett, 2015/08/03 22:52:35]

Nope.

+#include "SkTypes.h"
+
+/*
+ * This class implements the decoding for bmp images

[scroggo, 2015/07/31 15:05:43]

For a specific type, right?

[msarett, 2015/08/03 22:52:35]

Yes

+ */
+class SkBmpMaskCodec : public SkCodec {
+protected:
+
+    /*
+     * Initiates the bmp decode
+     */
+    Result onGetPixels(const SkImageInfo& dstInfo, void* dst,
+                       size_t dstRowBytes, const Options&, SkPMColor*,
+                       int*) override;
+
+    SkEncodedFormat onGetEncodedFormat() const override { return kBMP_SkEncodedFormat; }

[scroggo, 2015/07/31 15:05:43]

If these all inherited from SkBmpCodec, that class could implement this, and the
others would not have to.

[msarett, 2015/08/03 22:52:35]

Done.

+
+private:
+
+    /*
+     * Used to define the input format of the bmp
+     */
+    enum BitmapInputFormat {

[scroggo, 2015/07/31 15:05:43]

Isn't this the same as in SkBmpCodec? Why the duplication?

[msarett, 2015/08/03 22:52:35]

This was accidental.  Sorry.

+        kStandard_BitmapInputFormat,
+        kRLE_BitmapInputFormat,
+        kBitMask_BitmapInputFormat,
+        kUnknown_BitmapInputFormat
+    };
+
+    /*
+     * Initialize swizzler

[scroggo, 2015/07/31 15:05:43]

These comments don't really add much, do they?

[msarett, 2015/08/03 22:52:35]

No they don't.  Removing them.

+     */
+    bool initializeSwizzler(const SkImageInfo& dstInfo);
+
+    /*
+     * Performs the decoding
+     */
+    Result decode(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes,
+                  const Options& opts);
+
+    /*
+     * Creates an instance of the decoder
+     * Called only by NewFromStream

[scroggo, 2015/07/31 15:05:43]

nit: SkBmpCodec::NewFromStream.

[msarett, 2015/08/03 22:52:35]

Done.

+     *
+     * @param srcInfo contains the source width and height
+     * @param stream the stream of image data
+     * @param bitsPerPixel the number of bits used to store each pixel
+     * @param masks color masks for certain bmp formats
+     * @param rowOrder indicates whether rows are ordered top-down or bottom-up
+     */
+    SkBmpMaskCodec(const SkImageInfo& srcInfo, SkStream* stream,
+                   uint16_t bitsPerPixel, SkMasks* masks,
+                   SkBmpCodec::RowOrder rowOrder);
+
+    // Fields
+    const uint16_t                      fBitsPerPixel;
+    SkAutoTDelete<SkMasks>              fMasks;        // owned
+    const SkBmpCodec::RowOrder          fRowOrder;
+    SkAutoTDelete<SkMaskSwizzler>       fMaskSwizzler;
+    SkAutoTDeleteArray<uint8_t>         fSrcBuffer;
+    bool                                fIsTransparent;
+
+    friend class SkBmpCodec;
+
+    typedef SkCodec INHERITED;
+};