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..34c9f0de516fbb41c30d0a45ecd832d14771fc27 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"
@@ -48,6 +49,13 @@ 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,
@@ -77,6 +85,13 @@ protected:
     }
 
     /*
+     * Get the destination row number corresponsing to the encoded row number.
+     * For kTopDown, we simply return y, but for kBottomUp, the rows will be
+     * decoded in reverse order.
+     */
+    int32_t getDstRow(int32_t y);
+
+    /*
      * Get the destination row to start filling from
      * Used to fill the remainder of the image on incomplete input for bmps
      * This is tricky since bmps may be kTopDown or kBottomUp.  For kTopDown,
@@ -104,8 +119,35 @@ private:
      */
     static SkCodec* NewFromStream(SkStream*, bool inIco);
 
-    const uint16_t fBitsPerPixel;
-    const RowOrder fRowOrder;
+    /*
+     * To be overriden by bmp subclasses, which provide unique implementations.
+     * Performs subclass specific setup.
+     */
+    virtual SkCodec::Result prepareToDecode(const SkImageInfo& dstInfo,

[scroggo, 2015/08/26 22:40:09]

Can you add comments explaining the parameters? Does dstInfo still encapsulate
the rows we will decode?

nit: I think we typically make virtual functions protected, rather than private.

[msarett, 2015/08/27 15:00:27]

Added comments and made this protected.  Is there a reason to make this
protected other than skia convention?  I generally just try to make functions as
hidden as possible.

[scroggo, 2015/08/27 20:14:22]

Good question. This was just my memory of how we do things; I'm not sure whether
it is Skia-specific.

One answer I've found online (see
http://stackoverflow.com/questions/2170688/private-virtual-method-in-c and
https://isocpp.org/wiki/faq/strange-inheritance#private-virtuals) is that people
find private virtual functions confusing. Apparently there was a recommendation
to use protected virtuals in order to not confuse people, but that has been
reversed.

The confusion comes from the fact that a subclass can override a private virtual
function, but cannot call it (according to that second link).

So the latest recommendation seems to be making a virtual function private,
unless a subclass needs to call it, to follow the general rule of providing the
least access necessary.

That said, I personally do not see how that does any good, at least for a pure
virtual function. If the base class makes a virtual method private, the derived
class can *change* the access level of its overridden version (even to public),
so we didn't really hide anything from the subclass. The one case where we did
hide something is if the derived class is overriding a function with an
implementation - then the subclass cannot call it. Sometimes you want the
subclass to be able to call its INHERITED version, in which case it needs to be
protected (or public).

+            const SkCodec::Options& options, SkPMColor inputColorPtr[],
+            int* inputColorCount) = 0;
+
+    /*
+     * Decodes the next dstInfo.height() lines.
+     *
+     * onGetPixels() uses this for full image decodes.
+     * SkScaledCodec::onGetPixels() uses the scanline decoder to call this with
+     * dstInfo.height() = 1, in order to implement sampling.
+     * A potential future use is to allow the caller to decode a subset of the
+     * lines in the image.
+     *
+     * @param dstInfo     Contains output information.  Height specifies the
+     *                    number of rows to decode at this time.
+     * @param dst         Memory location to store output pixels
+     * @param dstRowBytes Bytes in a row of the destination
+     */
+    virtual Result decode(const SkImageInfo& dstInfo, void* dst, size_t dstRowBytes,
+            const Options& opts) = 0;
+
+    const uint16_t       fBitsPerPixel;
+    const RowOrder       fRowOrder;
+
+    friend class SkBmpScanlineDecoder;
 
     typedef SkCodec INHERITED;
 };