Fix BMP RLE bug: attempt 2

src/codec/SkBmpRLECodec.cpp

Index: src/codec/SkBmpRLECodec.cpp
diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index 2e04b8b7e6cbc59ea830f4a369939d33ccc44621..4cee274ffecc31534acb8d56380727ed3967463c 100644
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -349,7 +349,7 @@ int SkBmpRLECodec::decodeRows(const SkImageInfo& info, void* dst, size_t dstRowB
                     y++;
                     break;
                 case RLE_EOF:
-                    return y;
+                    return height;

[msarett, 2015/11/19 15:16:56]

We will often hit the EOF marker when x=width, y=height-1.  This should indicate
that we have finished a successful decode.

If we return "y" here, it will indicate that the decode was incomplete (since y
< height), which is not correct.

One fix to this might be:
if (x >= width && y < height)
  return y + 1;
else
  return y

I'm not sure whether this is more correct than just returning "height".  RLE is
allowed to skip/ignore pixels.  So no matter when we hit an EOF, I think we can
conclude "decode completed successfully" and not worry about any pixels that we
haven't reached.  This is what I've decided to do.

BTW, all of this is coming up because of another bmp related "bug". 
SkBmpRLECodec::onGetPixels() (correctly) never fills on incomplete images
because we start the RLE decode by filling the memory.

Unfortunately, SkBmpRLECodec::getScanlines() does (incorrectly) fill on
incomplete images because the code is shared in SkCodec::getScanlines().  So, on
incomplete scanline decodes, some memory will get filled twice.  I'm not sure
how bad this is.  It has allowed us to catch this bug because we are filling
over pixels that we successfully decoded.

[scroggo, 2015/11/19 15:55:51]

Maybe file a bug and/or add a FIXME? It seems unfortunate (we could be faster),
but not terrible. I think BMP is relatively rare (and RLE even more so - the old
implementation did not even support it, IIRC), and incomplete ones should be
even more rare.

I'm confused why an incomplete image would hit this case though; won't an
incomplete image return without finding an EOF?

[msarett, 2015/11/19 16:23:29]

Will file a bug.
That's exactly the goal of this CL.  If we hit an EOF marker, we should never
consider the image incomplete.  That's why we always want to return height.  If
we return some y that is less than height, the image will be interpreted as
incomplete.

The buggy case that still exists is double filling actual incomplete RLE bmps.

                 case RLE_DELTA: {
                     // Two bytes are needed to specify delta
                     if ((int) fRLEBytes - fCurrRLEByte < 2) {