Add CodecZeroInit test to DMSrcSink

Add CodecZeroInit test to DMSrcSink

This should not cause any diffs on Gold.

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

Committed: https://skia.googlesource.com/skia/+/bb25b5324965d53253fc3ad9f43b7d6faa2f9100

[msarett, 2016-01-12 14:58:48]

Description was changed from

==========
Add CodecZeroInit test to DMSrcSink

This should not cause any diffs on Gold.

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

to

==========
Add CodecZeroInit test to DMSrcSink

This should not cause any diffs on Gold.

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

[msarett, 2016-01-12 15:02:33]

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

[msarett, 2016-01-12 15:02:34]

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.h
File dm/DMSrcSink.h (right):

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.h#newcode103
dm/DMSrcSink.h:103: class CodecSrc : public Src {
I think you could make the argument that fZeroInit should be a bool field on
CodecSrc (and AndroidCodecSrc and BRDSrc), so that we test all of the modes on
zero init memory.

However, I don't think we want to double the time it takes to test.  The
interesting code to test is the swizzle functions, which will be exercised by
SkCodec::getPixels().

[scroggo, 2016-01-12 22:27:46]

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.cpp
File dm/DMSrcSink.cpp (right):

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.cpp#newcode328
dm/DMSrcSink.cpp:328: bitmap.eraseColor(0);
This won't work for index8 bitmaps. Instead, we can use an allocator which calls
calloc.

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.h
File dm/DMSrcSink.h (right):

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.h#newcode103
dm/DMSrcSink.h:103: class CodecSrc : public Src {
On 2016/01/12 15:02:33, msarett wrote:
> I think you could make the argument that fZeroInit should be a bool field on
> CodecSrc (and AndroidCodecSrc and BRDSrc), so that we test all of the modes on
> zero init memory.
> 
> However, I don't think we want to double the time it takes to test.  The
> interesting code to test is the swizzle functions, which will be exercised by
> SkCodec::getPixels().

sgtm. Maybe add a comment explaining that in the code along with the enum?

[mtklein, 2016-01-12 22:51:22]

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.cpp
File dm/DMSrcSink.cpp (right):

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.cpp#newcode328
dm/DMSrcSink.cpp:328: bitmap.eraseColor(0);
On 2016/01/12 22:27:46, scroggo wrote:
> This won't work for index8 bitmaps. Instead, we can use an allocator which
calls
> calloc.

e.g. SkMallocPixelRef::NewZeroed() / SkMallocPixelRef::ZeroedPRFactory.

[msarett, 2016-01-13 14:58:16]

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.cpp
File dm/DMSrcSink.cpp (right):

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.cpp#newcode328
dm/DMSrcSink.cpp:328: bitmap.eraseColor(0);
On 2016/01/12 22:51:22, mtklein wrote:
> On 2016/01/12 22:27:46, scroggo wrote:
> > This won't work for index8 bitmaps. Instead, we can use an allocator which
> calls
> > calloc.
> 
> e.g. SkMallocPixelRef::NewZeroed() / SkMallocPixelRef::ZeroedPRFactory.

Done.

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.h
File dm/DMSrcSink.h (right):

https://codereview.chromium.org/1577853004/diff/1/dm/DMSrcSink.h#newcode103
dm/DMSrcSink.h:103: class CodecSrc : public Src {
On 2016/01/12 22:27:46, scroggo wrote:
> On 2016/01/12 15:02:33, msarett wrote:
> > I think you could make the argument that fZeroInit should be a bool field on
> > CodecSrc (and AndroidCodecSrc and BRDSrc), so that we test all of the modes
on
> > zero init memory.
> > 
> > However, I don't think we want to double the time it takes to test.  The
> > interesting code to test is the swizzle functions, which will be exercised
by
> > SkCodec::getPixels().
> 
> sgtm. Maybe add a comment explaining that in the code along with the enum?

Done.

[scroggo, 2016-01-13 17:03:32]

lgtm

https://codereview.chromium.org/1577853004/diff/20001/dm/DMSrcSink.h
File dm/DMSrcSink.h (right):

https://codereview.chromium.org/1577853004/diff/20001/dm/DMSrcSink.h#newcode108
dm/DMSrcSink.h:108: // This should exercise all of the interesting cases in
SkSwizzler
nit: should -> will

[msarett, 2016-01-13 17:16:57]

The CQ bit was checked by msarett@google.com

[msarett, 2016-01-13 17:16:57]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com
Link to the patchset: https://codereview.chromium.org/1577853004/#ps40001
(title: "Fix comment")

[commit-bot: I haz the power, 2016-01-13 17:17:01]

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

[commit-bot: I haz the power, 2016-01-13 17:31:41]

Description was changed from

==========
Add CodecZeroInit test to DMSrcSink

This should not cause any diffs on Gold.

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

to

==========
Add CodecZeroInit test to DMSrcSink

This should not cause any diffs on Gold.

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

Committed:
https://skia.googlesource.com/skia/+/bb25b5324965d53253fc3ad9f43b7d6faa2f9100
==========

[commit-bot: I haz the power, 2016-01-13 17:31:42]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/bb25b5324965d53253fc3ad9f43b7d6faa2f9100