DM: add --pipe

DM: add --pipe

internal_links is failing across all three versions.  One of the rects doesn't draw.

BUG=

Committed: http://code.google.com/p/skia/source/detail?r=11985

[mtklein, 2013-10-27 21:55:54]

Any idea why internal_links would be failing in pipe mode?  One of the two blue
boxes doesn't draw.

It's failing in GM too.  Do we not run with --pipe on the bots?

[epoger, 2013-10-28 17:32:34]

On 2013/10/27 21:55:54, mtklein wrote:
> Any idea why internal_links would be failing in pipe mode?  One of the two
blue
> boxes doesn't draw.
> 
> It's failing in GM too.  Do we not run with --pipe on the bots?

First to answer this question: currently, we do NOT run with --pipe on the bots,
to speed things up.  Certainly this is a problem-- we want to run it SOMETIMES,
at least, or perhaps on certain platforms.  Tracked as
https://code.google.com/p/skia/issues/detail?id=1641 ('Run GM tests with all
rendering modes enabled, SOMETIMES')

I will review the code this afternoon...

[epoger, 2013-10-28 17:42:15]

Seems good for the most part, a couple of quibbles...

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.cpp
File dm/DMPipeTask.cpp (right):

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.cpp#newcode13
dm/DMPipeTask.cpp:13: static const char* kNames[] = {
I see that the indices of these correspond to the values returned by
get_flags(); do we have some way of making sure they stay in sync?

If nothing else, maybe some asserts nearby with a comment indicating why they
are there?  Like so...

// Make sure that the indices of kNames are correct
SkASSERT(1 == SkGPipeWriter::kCrossProcess_Flag);
SkASSERT(2 == SkGPipeWriter::kSharedAddressSpace_Flag);

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.h
File dm/DMPipeTask.h (right):

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.h#newcode17
dm/DMPipeTask.h:17: PipeTask(const Task& parent,
please document the parameters

[epoger, 2013-10-28 18:48:24]

On 2013/10/28 17:32:34, epoger wrote:
> On 2013/10/27 21:55:54, mtklein wrote:
> > Any idea why internal_links would be failing in pipe mode?  One of the two
> blue
> > boxes doesn't draw.
> > 
> > It's failing in GM too.  Do we not run with --pipe on the bots?
> 
> First to answer this question: currently, we do NOT run with --pipe on the
bots,
> to speed things up.  Certainly this is a problem-- we want to run it
SOMETIMES,
> at least, or perhaps on certain platforms.  Tracked as
> https://code.google.com/p/skia/issues/detail?id=1641 ('Run GM tests with all
> rendering modes enabled, SOMETIMES')

I changed gm to run the alternative rendering modes on most buildbots, in
https://code.google.com/p/skia/source/detail?r=11983 ('gm on bots: exercise
alternative renderModes, except on Android and ChromeOS builders')

Now we are seeing the internal_links failures, which I have filed as
https://code.google.com/p/skia/issues/detail?id=1761 ('RenderModeMismatch:
internal_links_comparison-pipe')

Thanks for calling it to my attention.

> 
> I will review the code this afternoon...

[mtklein, 2013-10-28 19:03:40]

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.cpp
File dm/DMPipeTask.cpp (right):

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.cpp#newcode13
dm/DMPipeTask.cpp:13: static const char* kNames[] = {
On 2013/10/28 17:42:15, epoger wrote:
> I see that the indices of these correspond to the values returned by
> get_flags(); do we have some way of making sure they stay in sync?
> 
> If nothing else, maybe some asserts nearby with a comment indicating why they
> are there?  Like so...
> 
> // Make sure that the indices of kNames are correct
> SkASSERT(1 == SkGPipeWriter::kCrossProcess_Flag);
> SkASSERT(2 == SkGPipeWriter::kSharedAddressSpace_Flag);

Added get_name().

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.h
File dm/DMPipeTask.h (right):

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.h#newcode17
dm/DMPipeTask.h:17: PipeTask(const Task& parent,
On 2013/10/28 17:42:15, epoger wrote:
> please document the parameters

Done.

[epoger, 2013-10-28 19:36:25]

LGTM with or without one thing

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.h
File dm/DMPipeTask.h (right):

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.h#newcode17
dm/DMPipeTask.h:17: PipeTask(const Task& parent,
On 2013/10/28 19:03:41, mtklein wrote:
> On 2013/10/28 17:42:15, epoger wrote:
> > please document the parameters
> 
> Done.

Thanks.

I would prefer javadoc-style comments (as directed in 
https://sites.google.com/site/skiadocs/developer-documentation/contributing-c...
), but I don't think it's a big deal either way.

[commit-bot: I haz the power, 2013-10-28 19:40:39]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@google.com/47773002/80001

[mtklein, 2013-10-28 19:41:48]

Thanks!

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.h
File dm/DMPipeTask.h (right):

https://codereview.chromium.org/47773002/diff/1/dm/DMPipeTask.h#newcode17
dm/DMPipeTask.h:17: PipeTask(const Task& parent,
On 2013/10/28 19:36:25, epoger wrote:
> On 2013/10/28 19:03:41, mtklein wrote:
> > On 2013/10/28 17:42:15, epoger wrote:
> > > please document the parameters
> > 
> > Done.
> 
> Thanks.
> 
> I would prefer javadoc-style comments (as directed in 
>
https://sites.google.com/site/skiadocs/developer-documentation/contributing-c...
> ), but I don't think it's a big deal either way.
> 

This not being a library, seems like overkill.

[commit-bot: I haz the power, 2013-10-28 19:51:29]

Change committed as 11985