Add option to dump images from nanobench.

Add option to dump images from nanobench.

Add option to set the repeat count to any number, replacs the --runOnce flag.

Committed: https://skia.googlesource.com/skia/+/6eb03cc06d0bc60da5277a83aa0251a475794b04

[mtklein, 2014-08-07 18:00:47]

Here's it's friend: https://codereview.chromium.org/452493005

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp
File bench/nanobench.cpp (right):

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:51: DEFINE_int32(loops, kDefaultLoops,
loops_help_txt().c_str());
This seems somewhat unsafe unless DEFINE_int32 immediate copies the help string.
 I think Valgrind's going to yell at us if we ever pass --help. :P

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:538: SkDebugf(" Ignoring: ", FLAGS_writePath[0]);
bogus extra arg to SkDebugf?

Maybe it's just me being cavalier, but maybe you can just drop this whole
too-many-args section?

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:544: if (!sk_mkdir(FLAGS_writePath[0])) {
Don't we do this inside write_canvas_png?

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:613: canvas->clear(SK_ColorWHITE);
Perhaps we should always clear canvas to white?  and move the rest of this down
to where we call write_canvas_png?

[bsalomon, 2014-08-07 20:08:10]

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp
File bench/nanobench.cpp (right):

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:51: DEFINE_int32(loops, kDefaultLoops,
loops_help_txt().c_str());
On 2014/08/07 18:00:47, mtklein wrote:
> This seems somewhat unsafe unless DEFINE_int32 immediate copies the help
string.
>  I think Valgrind's going to yell at us if we ever pass --help. :P

It does copy it into an SkString.

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:538: SkDebugf(" Ignoring: ", FLAGS_writePath[0]);
On 2014/08/07 18:00:47, mtklein wrote:
> bogus extra arg to SkDebugf?
> 
> Maybe it's just me being cavalier, but maybe you can just drop this whole
> too-many-args section?

Done.

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:544: if (!sk_mkdir(FLAGS_writePath[0])) {
On 2014/08/07 18:00:47, mtklein wrote:
> Don't we do this inside write_canvas_png?

It seems that sk_mkdir doesn't mkdirs recursively. So this attempts to make the
top level dir and that one makes the per-config dirs. It also prevents lots of
spew when the root dir can't be created.

https://codereview.chromium.org/450743002/diff/40001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:613: canvas->clear(SK_ColorWHITE);
On 2014/08/07 18:00:47, mtklein wrote:
> Perhaps we should always clear canvas to white?  and move the rest of this
down
> to where we call write_canvas_png?

Done.

[mtklein, 2014-08-07 20:13:12]

lgtm

https://codereview.chromium.org/450743002/diff/80001/bench/nanobench.cpp
File bench/nanobench.cpp (right):

https://codereview.chromium.org/450743002/diff/80001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:84: static double time(int loops, Benchmark* bench,
SkCanvas* canvas, SkGLContextHelper* gl) {
might put the if (canvas) { canvas->clear(white); } here?

[bsalomon, 2014-08-07 20:18:07]

https://codereview.chromium.org/450743002/diff/80001/bench/nanobench.cpp
File bench/nanobench.cpp (right):

https://codereview.chromium.org/450743002/diff/80001/bench/nanobench.cpp#newc...
bench/nanobench.cpp:84: static double time(int loops, Benchmark* bench,
SkCanvas* canvas, SkGLContextHelper* gl) {
On 2014/08/07 20:13:11, mtklein wrote:
> might put the if (canvas) { canvas->clear(white); } here?

Done.

[bsalomon, 2014-08-07 20:23:55]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-08-07 20:24:43]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/bsalomon@google.com/450743002/100001

[commit-bot: I haz the power, 2014-08-07 21:29:03]

Change committed as 6eb03cc06d0bc60da5277a83aa0251a475794b04