In bench_pictures, use a pool of tiles for multicore drawing.

In bench_pictures, use a pool of tiles for multicore drawing.

Also includes some code cleanup and code sharing.

Allow setting the number of threads on the command line.

Rename ThreadSafePipeController::playback to ::draw, to be the same
as SkPicture so DrawTileToCanvas can take a template parameter.

Disallow multithreading with GPU turned on.

Display help information with improper tiled arguments.

BUG=https://code.google.com/p/skia/issues/detail?id=871

Committed: https://code.google.com/p/skia/source/detail?r=5602

[DerekS, 2012-09-19 13:49:14]

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp
File tools/PictureRenderer.cpp (right):

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp#ne...
tools/PictureRenderer.cpp:353: , fController(controller) {}
why not just put this in the header declaration for clarity?

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp#ne...
tools/PictureRenderer.cpp:380: , fClone(clone) {}
same as above.

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp#ne...
tools/PictureRenderer.cpp:384: void TiledPictureRenderer::setup() {
do we really need setup or could that logic just be in the render method?

[Leon, 2012-09-19 13:59:12]

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp
File tools/PictureRenderer.cpp (right):

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp#ne...
tools/PictureRenderer.cpp:353: , fController(controller) {}
On 2012/09/19 13:49:14, DerekS wrote:
> why not just put this in the header declaration for clarity?

This was originally here because it referenced DrawTile, which in turn
referenced TileData. Now that that is no longer the case (which I changed mainly
because Windows gives me a warning for using a pointer to this in the
constructor), I can move it into the declaration.

Will fix with next patch.

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp#ne...
tools/PictureRenderer.cpp:384: void TiledPictureRenderer::setup() {
On 2012/09/19 13:49:14, DerekS wrote:
> do we really need setup or could that logic just be in the render method?

The reason I do it here is because PictureBenchmark times the render() method.
My goal is to time drawing from the Pipe, not writing a picture to it.

[DerekS, 2012-09-19 14:02:27]

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp
File tools/PictureRenderer.cpp (right):

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp#ne...
tools/PictureRenderer.cpp:384: void TiledPictureRenderer::setup() {
Makes perfect sense with that explanation. Can you put a comment on setup in the
header that states that? So that in the future we no if we don't want it timed
as part of rendering that we should put it in setup.

On 2012/09/19 13:59:12, scroggo-work wrote:
> On 2012/09/19 13:49:14, DerekS wrote:
> > do we really need setup or could that logic just be in the render method?
> 
> The reason I do it here is because PictureBenchmark times the render() method.
> My goal is to time drawing from the Pipe, not writing a picture to it.

[Leon, 2012-09-19 17:16:49]

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp
File tools/PictureRenderer.cpp (right):

https://codereview.appspot.com/6536050/diff/2001/tools/PictureRenderer.cpp#ne...
tools/PictureRenderer.cpp:384: void TiledPictureRenderer::setup() {
On 2012/09/19 14:02:28, DerekS wrote:
> Makes perfect sense with that explanation. Can you put a comment on setup in
the
> header that states that? So that in the future we no if we don't want it timed
> as part of rendering that we should put it in setup.
> 
> On 2012/09/19 13:59:12, scroggo-work wrote:
> > On 2012/09/19 13:49:14, DerekS wrote:
> > > do we really need setup or could that logic just be in the render method?
> > 
> > The reason I do it here is because PictureBenchmark times the render()
method.
> > My goal is to time drawing from the Pipe, not writing a picture to it.
> 

There is a comment on setup() in the base class, which this overrides. Should I
add it to this one as well?

[DerekS, 2012-09-19 18:05:12]

no as long as it is documented somewhere that should be fine.

lgtm