Fix scanline decoding of rare RLE bmps

Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

This should fix testcase7.bmp on Gold.

The image that I am adding to resources is in the
public domain.

BUG=skia:4730
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1689953002

Committed: https://skia.googlesource.com/skia/+/4946b9419f650ef7a2dc684ce98bb771e1ce8f40

[msarett, 2016-02-11 15:07:22]

Description was changed from

==========
Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

BUG=skia:4730
==========

to

==========
Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

BUG=skia:4730
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[msarett, 2016-02-11 15:22:01]

Description was changed from

==========
Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

BUG=skia:4730
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

This should fix testcase7.bmp on Gold.

BUG=skia:4730
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[msarett, 2016-02-11 15:22:59]

Description was changed from

==========
Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

This should fix testcase7.bmp on Gold.

BUG=skia:4730
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

This should fix testcase7.bmp on Gold.

The image that I am adding to resources is in the
public domain.

BUG=skia:4730
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[msarett, 2016-02-11 15:23:09]

msarett@google.com changed reviewers:
+ scroggo@google.com

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

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp
File src/codec/SkBmpRLECodec.cpp (right):

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp...
src/codec/SkBmpRLECodec.cpp:292: fRLEBytes = fOrigRLEBytes;
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.

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

lgtm

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp
File src/codec/SkBmpRLECodec.cpp (right):

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp...
src/codec/SkBmpRLECodec.cpp:275: // Scanline decodes allow the client to ask for
a single scanline at a time.
Maybe this comment should go in the header?

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp...
src/codec/SkBmpRLECodec.cpp:389: SkCodecPrintf("Warning: invalid RLE input.\n");
Is this still correct if we consider a subset (in width) decode?

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

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp
File src/codec/SkBmpRLECodec.cpp (right):

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp...
src/codec/SkBmpRLECodec.cpp:275: // Scanline decodes allow the client to ask for
a single scanline at a time.
On 2016/02/11 15:56:52, scroggo wrote:
> Maybe this comment should go in the header?

Done.

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp...
src/codec/SkBmpRLECodec.cpp:389: SkCodecPrintf("Warning: invalid RLE input.\n");
On 2016/02/11 15:56:52, scroggo wrote:
> Is this still correct if we consider a subset (in width) decode?

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:48]

lgtm at patch set 2

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp
File src/codec/SkBmpRLECodec.cpp (right):

https://codereview.chromium.org/1689953002/diff/1/src/codec/SkBmpRLECodec.cpp...
src/codec/SkBmpRLECodec.cpp:389: SkCodecPrintf("Warning: invalid RLE input.\n");
On 2016/02/11 16:03:51, msarett wrote:
> On 2016/02/11 15:56:52, scroggo wrote:
> > Is this still correct if we consider a subset (in width) decode?
> 
> 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.
> 

Ah, of course. Doesn't matter yet.

[msarett, 2016-02-11 16:09:57]

The CQ bit was checked by msarett@google.com

[commit-bot: I haz the power, 2016-02-11 16:10:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1689953002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1689953002/20001

[commit-bot: I haz the power, 2016-02-11 16:35:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-11 16:35:01]

Failed to commit the patch.

[msarett, 2016-02-11 16:40:16]

The CQ bit was checked by msarett@google.com

[commit-bot: I haz the power, 2016-02-11 16:40:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1689953002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1689953002/20001

[commit-bot: I haz the power, 2016-02-11 16:41:10]

Description was changed from

==========
Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

This should fix testcase7.bmp on Gold.

The image that I am adding to resources is in the
public domain.

BUG=skia:4730
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Fix scanline decoding of rare RLE bmps

This also exposed a bug in rewinding RLE bmps, which I have also
fixed in this CL.

This should fix testcase7.bmp on Gold.

The image that I am adding to resources is in the
public domain.

BUG=skia:4730
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/4946b9419f650ef7a2dc684ce98bb771e1ce8f40
==========

[commit-bot: I haz the power, 2016-02-11 16:41:10]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/4946b9419f650ef7a2dc684ce98bb771e1ce8f40