Scanline decoding for wbmp

src/codec/SkCodec_wbmp.cpp

Index: src/codec/SkCodec_wbmp.cpp
diff --git a/src/codec/SkCodec_wbmp.cpp b/src/codec/SkCodec_wbmp.cpp
index 35ac808219c625ad1bacf045d6414fb4b8e40b76..3360274a79ac2caebd10bab6645db760f958358e 100644
--- a/src/codec/SkCodec_wbmp.cpp
+++ b/src/codec/SkCodec_wbmp.cpp
@@ -68,8 +68,6 @@ static uint16_t bit_to_rgb565(U8CPU bit) {
     return bit ? RGB565_WHITE : RGB565_BLACK;
 }
 
-typedef void (*ExpandProc)(uint8_t*, const uint8_t*, int);
-
 // TODO(halcanary): Add this functionality (grayscale and indexed output) to
 //                  SkSwizzler and use it here.
 template <typename T, T (*TRANSFORM)(U8CPU)>
@@ -93,6 +91,28 @@ static void expand_bits_to_T(uint8_t* dstptr, const uint8_t* src, int bits) {
     }
 }
 
+bool SkWbmpCodec::setExpandProc(SkColorType colorType, SkPMColor* ctable, int* ctableCount) {
+    switch (colorType) {
+        case kGray_8_SkColorType:
+            fProc = expand_bits_to_T<uint8_t, bit_to_grayscale>;
+            return true;
+        case kN32_SkColorType:
+            fProc = expand_bits_to_T<SkPMColor, bit_to_pmcolor>;
+            return true;
+        case kIndex_8_SkColorType:
+            ctable[0] = BLACK;
+            ctable[1] = WHITE;
+            *ctableCount = 2;
+            fProc = expand_bits_to_T<uint8_t, bit_to_bit>;
+            return true;
+        case kRGB_565_SkColorType:
+            fProc = expand_bits_to_T<uint16_t, bit_to_rgb565>;
+            return true;
+        default:
+            return false;
+    }
+}
+
 SkWbmpCodec::SkWbmpCodec(const SkImageInfo& info, SkStream* stream)
     : INHERITED(info, stream) {}
 
@@ -119,26 +139,12 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
     if (info.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
-    ExpandProc proc = NULL;
-    switch (info.colorType()) {
-        case kGray_8_SkColorType:
-            proc = expand_bits_to_T<uint8_t, bit_to_grayscale>;
-            break;
-        case kN32_SkColorType:
-            proc = expand_bits_to_T<SkPMColor, bit_to_pmcolor>;
-            break;
-        case kIndex_8_SkColorType:
-            ctable[0] = BLACK;
-            ctable[1] = WHITE;
-            *ctableCount = 2;
-            proc = expand_bits_to_T<uint8_t, bit_to_bit>;
-            break;
-        case kRGB_565_SkColorType:
-            proc = expand_bits_to_T<uint16_t, bit_to_rgb565>;
-            break;
-        default:
-            return kInvalidConversion;
+
+    // Choose the expand routine based on the requested color type
+    if (!setExpandProc(info.colorType(), ctable, ctableCount)) {
+        return kInvalidConversion;
     }
+

[emmaleer, 2015/07/27 14:41:37]

nit: added extra line

[msarett, 2015/07/27 23:42:30]

Acknowledged.

     SkISize size = info.dimensions();
     uint8_t* dst = static_cast<uint8_t*>(pixels);
     size_t srcRowBytes = SkAlign8(size.width()) >> 3;
@@ -147,7 +153,7 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
         if (this->stream()->read(src.get(), srcRowBytes) != srcRowBytes) {
             return kIncompleteInput;
         }
-        proc(dst, src.get(), size.width());
+        fProc(dst, src.get(), size.width());
         dst += rowBytes;
     }
     return kSuccess;
@@ -168,3 +174,59 @@ SkCodec* SkWbmpCodec::NewFromStream(SkStream* stream) {
                               kOpaque_SkAlphaType);
     return SkNEW_ARGS(SkWbmpCodec, (info, streamDeleter.detach()));
 }
+
+class SkWbmpScanlineDecoder : public SkScanlineDecoder {
+public:
+    SkWbmpScanlineDecoder(const SkImageInfo& dstInfo, SkWbmpCodec* codec)
+        : INHERITED(dstInfo)
+        , fCodec(codec)
+        , fSrcRowBytes(SkAlign8(fCodec->getInfo().width()) >> 3)

[scroggo, 2015/07/27 14:16:45]

Maybe add a static helper function for this, to share with the codec?

[emmaleer, 2015/07/27 14:41:37]

Could you add a comment describing why the rowBytes is width shifted right by 3?

[msarett, 2015/07/27 23:42:30]

Done.

+        , fSrcBuffer(fSrcRowBytes)
+    {}
+
+    SkCodec::Result onGetScanlines(void* dst, int count, size_t dstRowBytes) override {
+        for (int y = 0; y < count; ++y) {
+            if (fCodec->stream()->read(fSrcBuffer.get(), fSrcRowBytes) != fSrcRowBytes) {
+                return SkCodec::kIncompleteInput;
+            }
+            fCodec->fProc((uint8_t*) dst, fSrcBuffer.get(), dstInfo().width());

[scroggo, 2015/07/27 14:16:45]

The scanline decoder will need to store fProc, but it's not clear to me that it
needs to be set on the codec.

[emmaleer, 2015/07/27 14:41:37]

Why isn't the Swizzler being used? Does fProc do something different than the
swizzler?
We will need to use the swizzler soon for SkScaledCodec
If fProc does something different than the swizzler, let me know so I can update
the swizzler to do the same 
Or, if the swizzler does the same thing, maybe we should just use the swizzler
here so we don't have to change it in the future

[msarett, 2015/07/27 23:42:30]

fSwizzler is now a field of only the scanline decoder.

+            dst += dstRowBytes;
+        }
+        return SkCodec::kSuccess;
+    }
+
+private:
+    SkAutoTDelete<SkWbmpCodec>   fCodec;
+    const size_t                 fSrcRowBytes;
+    SkAutoTMalloc<uint8_t>       fSrcBuffer;
+
+    typedef SkScanlineDecoder INHERITED;
+};
+
+
+SkScanlineDecoder* SkWbmpCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
+        const Options& options, SkPMColor ctable[], int* ctableCount) {
+    if (options.fSubset) {
+        // Subsets are not supported.
+        return NULL;
+    }
+    if (dstInfo.dimensions() != this->getInfo().dimensions()) {
+        return NULL;
+    }
+    // Create a new SkWbmpCodec, to be owned by the scanline decoder.
+    SkStream* stream = this->stream()->duplicate();
+    if (!stream) {
+        return NULL;
+    }
+    SkAutoTDelete<SkWbmpCodec> codec(static_cast<SkWbmpCodec*>(SkWbmpCodec::NewFromStream(stream)));
+    if (!codec) {
+        return NULL;
+    }
+
+    // Choose the expand routine based on the requested color type
+    if (!codec->setExpandProc(dstInfo.colorType(), ctable, ctableCount)) {
+        return NULL;
+    }
+
+    return SkNEW_ARGS(SkWbmpScanlineDecoder, (dstInfo, codec.detach()));
+}