Fix scanline decoding of rare RLE bmps

src/codec/SkBmpRLECodec.cpp

Index: src/codec/SkBmpRLECodec.cpp
diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index b01012644bfded083a27c95deb41bff5be0f96be..f49c1a46b53e79aeb60df3ccaa3bdbb892f59738 100644
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -26,6 +26,7 @@ SkBmpRLECodec::SkBmpRLECodec(const SkImageInfo& info, SkStream* stream,
     , fOffset(offset)
     , fStreamBuffer(new uint8_t[RLEBytes])
     , fRLEBytes(RLEBytes)
+    , fOrigRLEBytes(RLEBytes)
     , fCurrRLEByte(0)
     , fSampleX(1)
 {}
@@ -270,6 +271,13 @@ SkCodec::Result SkBmpRLECodec::prepareToDecode(const SkImageInfo& dstInfo,
     // Reset fSampleX. If it needs to be a value other than 1, it will get modified by
     // the sampler.
     fSampleX = 1;
+
+    // Scanline decodes allow the client to ask for a single scanline at a time.

[scroggo, 2016/02/11 15:56:52]

Maybe this comment should go in the header?

[msarett, 2016/02/11 16:03:51]

Done.

+    // This can be tricky when the RLE encoding instructs the decoder to jump down
+    // multiple lines.  This field keeps track of lines that need to be skipped
+    // on subsequent calls to getScanlines().
+    fLinesToSkip = 0;
+
     // Create the color table if necessary and prepare the stream for decode
     // Note that if it is non-NULL, inputColorCount will be modified
     if (!this->createColorTable(inputColorCount)) {
@@ -281,6 +289,7 @@ SkCodec::Result SkBmpRLECodec::prepareToDecode(const SkImageInfo& dstInfo,
     copy_color_table(dstInfo, this->fColorTable, inputColorPtr, inputColorCount);
 
     // Initialize a buffer for encoded RLE data
+    fRLEBytes = fOrigRLEBytes;

[msarett, 2016/02/11 15:23:10]

On bmps where we need to "check for more data", we change the value of
fRLEBytes.

We need to reset this value to the original value after rewinding.

     if (!this->initializeStreamBuffer()) {
         SkCodecPrintf("Error: cannot initialize stream buffer.\n");
         return SkCodec::kInvalidConversion;
@@ -301,17 +310,12 @@ int SkBmpRLECodec::decodeRows(const SkImageInfo& info, void* dst, size_t dstRowB
     static const uint8_t RLE_EOF = 1;
     static const uint8_t RLE_DELTA = 2;
 
-    // Set constant values
     const int width = this->getInfo().width();
-    const int height = info.height();
+    int height = info.height();
 
     // Account for sampling.
     SkImageInfo dstInfo = info.makeWH(get_scaled_dimension(width, fSampleX), height);
 
-    // Destination parameters
-    int x = 0;
-    int y = 0;
-
     // Set the background as transparent.  Then, if the RLE code skips pixels,
     // the skipped pixels will be transparent.
     // Because of the need for transparent pixels, kN32 is the only color
@@ -319,6 +323,21 @@ int SkBmpRLECodec::decodeRows(const SkImageInfo& info, void* dst, size_t dstRowB
     SkASSERT(kN32_SkColorType == dstInfo.colorType());
     SkSampler::Fill(dstInfo, dst, dstRowBytes, SK_ColorTRANSPARENT, opts.fZeroInitialized);
 
+    // Adjust the height and the dst if the previous call to decodeRows() left us
+    // with lines that need to be skipped.
+    if (height > fLinesToSkip) {
+        height -= fLinesToSkip;
+        dst = SkTAddOffset<void>(dst, fLinesToSkip * dstRowBytes);
+        fLinesToSkip = 0;
+    } else {
+        fLinesToSkip -= height;
+        return height;
+    }
+
+    // Destination parameters
+    int x = 0;
+    int y = 0;
+
     while (true) {
         // If we have reached a row that is beyond the requested height, we have
         // succeeded.
@@ -366,9 +385,12 @@ int SkBmpRLECodec::decodeRows(const SkImageInfo& info, void* dst, size_t dstRowB
                     const uint8_t dy = fStreamBuffer.get()[fCurrRLEByte++];
                     x += dx;
                     y += dy;
-                    if (x > width || y > height) {
+                    if (x > width) {
                         SkCodecPrintf("Warning: invalid RLE input.\n");

[scroggo, 2016/02/11 15:56:52]

Is this still correct if we consider a subset (in width) decode?

[msarett, 2016/02/11 16:03:51]

If width == originalWidth, this is fine.  But if width == subsetWidth, this is
wrong.

If we were ever to try to support bmp rle subsets, I think it might make sense
to use originalWidth in this function and then handle subsetting in the
setPixel() functions.  Similar to how we handle scaling.

[scroggo, 2016/02/11 16:08:47]

Ah, of course. Doesn't matter yet.

                         return y - dy;
+                    } else if (y > height) {
+                        fLinesToSkip = y - height;
+                        return height;
                     }
                     break;
                 }