Scanline decoding for bmp

src/codec/SkBmpCodec.h

Index: src/codec/SkBmpCodec.h
diff --git a/src/codec/SkBmpCodec.h b/src/codec/SkBmpCodec.h
index 71006f73153ea3028529b0c51daa999b498bc86e..b3cd40fa997883a23a17d7cf4633e0b5c1a3c4ff 100644
--- a/src/codec/SkBmpCodec.h
+++ b/src/codec/SkBmpCodec.h
@@ -11,6 +11,7 @@
 #include "SkColorTable.h"
 #include "SkImageInfo.h"
 #include "SkMaskSwizzler.h"
+#include "SkScanlineDecoder.h"
 #include "SkStream.h"
 #include "SkSwizzler.h"
 #include "SkTypes.h"
@@ -31,6 +32,16 @@ public:
     };
 
     /*
+     * Used to define the input format of the bmp
+     */
+    enum BmpInputFormat {
+        kStandard_BmpInputFormat,
+        kRLE_BmpInputFormat,
+        kBitMask_BmpInputFormat,
+        kUnknown_BmpInputFormat
+    };
+
+    /*
      * Checks the start of the stream to see if the image is a bmp
      */
     static bool IsBmp(SkStream*);
@@ -48,10 +59,17 @@ public:
      */
     static SkCodec* NewFromIco(SkStream*);
 
+    /*
+     * Assumes IsBmp was called and returned true
+     * Creates a bmp scanline decoder
+     * Takes ownership of the stream
+     */
+    static SkScanlineDecoder* NewSDFromStream(SkStream* stream);
+
 protected:
 
-    SkBmpCodec(const SkImageInfo& info, SkStream* stream, uint16_t bitsPerPixel,
-            RowOrder rowOrder);
+    SkBmpCodec(const SkImageInfo& info, SkStream* stream, BmpInputFormat inputFormat,
+            uint16_t bitsPerPixel, RowOrder rowOrder);
 
     SkEncodedFormat onGetEncodedFormat() const override { return kBMP_SkEncodedFormat; }
 
@@ -104,8 +122,24 @@ private:
      */
     static SkCodec* NewFromStream(SkStream*, bool inIco);
 
-    const uint16_t fBitsPerPixel;
-    const RowOrder fRowOrder;
+    /*
+     * Allow to BmpCodec subclasses to implement unique onStart() functions

[scroggo, 2015/08/14 21:53:00]

Maybe mention that this relates to scanline decoding?

[msarett, 2015/08/17 19:16:13]

This is now shared with onGetPixels().

+     */
+    virtual SkCodec::Result onStart(const SkImageInfo& dstInfo, const SkCodec::Options& options,

[scroggo, 2015/08/14 21:53:01]

I do not know whether it's good or bad that this shares the same name as
SkScanlineDecoder::onStart.

Would it make sense for onGetPixels to call onStart?

[msarett, 2015/08/17 19:16:13]

I think it makes sense to change the name because it's a bit confusing to
duplicate the name of a scanline decoding function.  With the name change, I
think it makes sense to share this function with onGetPixels().

+            SkPMColor inputColorPtr[], int* inputColorCount) = 0;
+
+    /*
+     * Performs the row by row decode
+     * Unique for each of the bmp subclasses
+     */
+    virtual Result decode(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes,

[scroggo, 2015/08/14 21:53:01]

Can you add a description for these parameters? I think it's interesting (and
not necessarily intuitive) that the height of dstInfo allows the caller to do a
subset (of lines - not in X).

Also, I assume it decodes the next dstInfo.height() lines?

[msarett, 2015/08/17 19:16:13]

Yeah I'm adding comments, this is awkward.  Ideally we would share a decodeRow()
function and do away with this capability (since we are not enabling subsets for
bmp).

decodeRow() isn't a good fit for RLE though.  So I think sticking with decode()
is the best design move.

+            const Options& opts) = 0;
+
+    const BmpInputFormat fInputFormat;
+    const uint16_t       fBitsPerPixel;
+    const RowOrder       fRowOrder;
+
+    friend class SkBmpScanlineDecoder;
 
     typedef SkCodec INHERITED;
 };