Scanline decoding for bmp

src/codec/SkBmpStandardCodec.cpp

Index: src/codec/SkBmpStandardCodec.cpp
diff --git a/src/codec/SkBmpStandardCodec.cpp b/src/codec/SkBmpStandardCodec.cpp
index 08146fc3e48f9a75f528044ade27e7fb497df13b..d74f7f03286414b191d0a5ffb037cd29c342b8f6 100644
--- a/src/codec/SkBmpStandardCodec.cpp
+++ b/src/codec/SkBmpStandardCodec.cpp
@@ -45,10 +45,11 @@ static bool conversion_possible(const SkImageInfo& dst,
  * Called only by NewFromStream
  */
 SkBmpStandardCodec::SkBmpStandardCodec(const SkImageInfo& info, SkStream* stream,
+                                       SkBmpCodec::BmpInputFormat inputFormat,
                                        uint16_t bitsPerPixel, uint32_t numColors,
                                        uint32_t bytesPerColor, uint32_t offset,
                                        SkBmpCodec::RowOrder rowOrder, bool inIco)
-    : INHERITED(info, stream, bitsPerPixel, rowOrder)
+    : INHERITED(info, stream, inputFormat, bitsPerPixel, rowOrder)
     , fColorTable(NULL)
     , fNumColors(this->computeNumColors(numColors))
     , fBytesPerColor(bytesPerColor)
@@ -280,8 +281,8 @@ static uint32_t get_fill_color_or_index(uint16_t bitsPerPixels, SkAlphaType alph
  * Performs the bitmap decoding for standard input format
  */
 SkCodec::Result SkBmpStandardCodec::decode(const SkImageInfo& dstInfo,
-                                   void* dst, size_t dstRowBytes,
-                                   const Options& opts) {
+                                           void* dst, size_t dstRowBytes,
+                                           const Options& opts) {
     // Set constant values
     const int width = dstInfo.width();
     const int height = dstInfo.height();
@@ -359,3 +360,59 @@ SkCodec::Result SkBmpStandardCodec::decode(const SkImageInfo& dstInfo,
     // Finished decoding the entire image
     return kSuccess;
 }
+
+SkBmpStandardScanlineDecoder::SkBmpStandardScanlineDecoder(SkBmpStandardCodec* codec)
+    : INHERITED(codec->getInfo())
+    , fCodec(codec)
+{}
+
+SkCodec::Result SkBmpStandardScanlineDecoder::onStart(const SkImageInfo& dstInfo,

[scroggo, 2015/08/13 21:25:36]

Here's how I see these. Let me know if I'm wrong:

1. All three scanline decoders have the same implementation of onGetScanlines.
2. They have differences in onStart, which are particular to the subclass of
SkBmpCodec.

What if we do the following instead?
- Add a virtual function to SkBmpCodec which does onStart
- Write one scanline decoder which does the following:

SkBmpScanlineDecoder::onStart(...) {
  return fCodec->onStart(...);
}

SkBmpScanlineDecoder::onGetScanlines(...) {
  return fCodec->decode(...);
}

[msarett, 2015/08/14 15:44:48]

This is clever!

+        const SkCodec::Options& options, SkPMColor inputColorPtr[], int* inputColorCount) {
+    // Rewind the stream if needed
+    if (!fCodec->rewindIfNeeded()) {
+        return SkCodec::kCouldNotRewind;
+    }
+
+    if (options.fSubset) {
+        // Subsets are not supported.
+        return SkCodec::kUnimplemented;
+    }
+    if (dstInfo.dimensions() != fCodec->getInfo().dimensions()) {
+        SkCodecPrintf("Error: scaling not supported.\n");
+        return SkCodec::kInvalidScale;
+    }
+    if (!conversion_possible(dstInfo, fCodec->getInfo())) {
+        SkCodecPrintf("Error: cannot convert input type to output type.\n");
+        return SkCodec::kInvalidConversion;
+    }
+
+    // Create the color table if necessary and prepare the stream for decode
+    // Note that if it is non-NULL, inputColorCount will be modified
+    if (!fCodec->createColorTable(dstInfo.alphaType(), inputColorCount)) {
+        SkCodecPrintf("Error: could not create color table.\n");
+        return SkCodec::kInvalidInput;
+    }
+
+    // Copy the color table to the client if necessary
+    copy_color_table(dstInfo, fCodec->fColorTable, inputColorPtr, inputColorCount);
+
+    // Initialize a swizzler if necessary
+    if (!fCodec->initializeSwizzler(dstInfo, options)) {
+        SkCodecPrintf("Error: cannot initialize swizzler.\n");
+        return SkCodec::kInvalidConversion;
+    }
+
+    fDstInfo = dstInfo;
+    fOpts = options;
+
+    return SkCodec::kSuccess;
+}
+
+SkCodec::Result SkBmpStandardScanlineDecoder::onGetScanlines(void* dst,
+        int count, size_t rowBytes) {
+    // Create a new image info representing the portion of the image to decode
+    SkImageInfo rowInfo = fDstInfo.makeWH(dstInfo().width(), count);
+
+    // Decode the requested rows
+    SkCodec::Result r = fCodec->decode(rowInfo, dst, rowBytes, fOpts);

[scroggo, 2015/08/13 21:25:36]

This needs to be returned. (probably true for the others)

[msarett, 2015/08/14 15:44:48]

Hmm yes.  Thanks for being a better error checker than my compiler.

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

I just filed skbug.com/4208, where I proposed doing some sort of automatic
check. The only thing I've found is -fsanitize=return, which does a runtime
check.

+}