Scanline decoding for bmp

src/codec/SkBmpCodec.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 SkBmpCodec_DEFINED
 #define SkBmpCodec_DEFINED
 
 #include "SkCodec.h"
 #include "SkColorTable.h"
 #include "SkImageInfo.h"
 #include "SkMaskSwizzler.h"
+#include "SkScanlineDecoder.h"
 #include "SkStream.h"
 #include "SkSwizzler.h"
 #include "SkTypes.h"
 
 /*
  * This class enables code sharing between its bmp codec subclasses.  The
  * subclasses actually do the work.
  */
 class SkBmpCodec : public SkCodec {
 public:
@@ skipping 17 matching lines @@
      * Reads enough of the stream to determine the image format
      */
     static SkCodec* NewFromStream(SkStream*);
 
     /*
      * Creates a bmp decoder for a bmp embedded in ico
      * Reads enough of the stream to determine the image format
      */
     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);
 
     SkEncodedFormat onGetEncodedFormat() const override { return kBMP_SkEncodedFormat; }
 
     /*
      * Read enough of the stream to initialize the SkBmpCodec. Returns a bool
      * representing success or failure. If it returned true, and codecOut was
      * not NULL, it will be set to a new SkBmpCodec.
      * Does *not* take ownership of the passed in SkStream.
      */
     static bool ReadHeader(SkStream*, bool inIco, SkCodec** codecOut);
 
     bool onRewind() override;
 
     /*
      * Returns whether this BMP is part of an ICO image.
      */
     bool inIco() const {
         return this->onInIco();
     }
 
     virtual bool onInIco() const {
         return false;
     }
 
     /*
+     * 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,
      * we start filling from where we left off, but for kBottomUp we start
      * filling at the top of the image.
      */
     void* getDstStartRow(void* dst, size_t dstRowBytes, int32_t y) const;
 
     /*
      * Compute the number of colors in the color table
      */
     uint32_t computeNumColors(uint32_t numColors);
 
     /*
      * Accessors used by subclasses
      */
     uint16_t bitsPerPixel() const { return fBitsPerPixel; }
     RowOrder rowOrder() const { return fRowOrder; }
 
 private:
 
     /*
      * Creates a bmp decoder
      * Reads enough of the stream to determine the image format
      */
     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;
 };
 
 #endif