Add viewer mode to VisualBench.

Add viewer mode to VisualBench.

Displays each benchmark/skp with a graph showing a series of frame times.
Use the space bar to skip to the next benchmark.

Adds an option to hit ESC to quit VisualBench. Useful in fullscreen mode.

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

[jvanverth1, 2015-09-11 18:35:44]

jvanverth@google.com changed reviewers:
+ joshualitt@google.com

[joshualitt, 2015-09-14 14:07:45]

minor feedback

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
File tools/VisualBench/VisualBench.cpp (right):

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualBench.cpp:113: if (27 == unichar) {
static const kEscKey or something similar?

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
File tools/VisualBench/VisualInteractiveModule.cpp (right):

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:65: canvas->drawLine(rect.fLeft,
rect.fBottom - 16.f*kPixelPerMs,
I wonder if we want to make this 1000/60 'exactly.'  also might be kind of nice
to have a GetFrameTimeMs(int FramesPerSecond).

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:71: int x =
SkScalarTruncToInt(rect.fLeft) + 3;
Is this some kind of padding?

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:76: int endY = startY -
(int)(fMeasurements[i] * kPixelPerMs + 0.5);
0.5?

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:100: canvas->clear(0xffffffff);
Hmm, might be nice to have a static function on VisualBench which can do this
clear.  Possibly also taking an N for single, double, or triple buffering.

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:183: inline void
VisualInteractiveModule::tuneLoops(SkCanvas* canvas) {
it'd be nice to figure out how to share this code, but its not really necessary
for this change.

[jvanverth1, 2015-09-14 16:21:39]

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
File tools/VisualBench/VisualBench.cpp (right):

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualBench.cpp:113: if (27 == unichar) {
On 2015/09/14 14:07:45, joshualitt wrote:
> static const kEscKey or something similar?

Done.

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
File tools/VisualBench/VisualInteractiveModule.cpp (right):

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:65: canvas->drawLine(rect.fLeft,
rect.fBottom - 16.f*kPixelPerMs,
On 2015/09/14 14:07:45, joshualitt wrote:
> I wonder if we want to make this 1000/60 'exactly.'  also might be kind of
nice
> to have a GetFrameTimeMs(int FramesPerSecond).

Done.

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:71: int x =
SkScalarTruncToInt(rect.fLeft) + 3;
On 2015/09/14 14:07:45, joshualitt wrote:
> Is this some kind of padding?

Done.

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:76: int endY = startY -
(int)(fMeasurements[i] * kPixelPerMs + 0.5);
On 2015/09/14 14:07:45, joshualitt wrote:
> 0.5?

Added comment

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:100: canvas->clear(0xffffffff);
On 2015/09/14 14:07:45, joshualitt wrote:
> Hmm, might be nice to have a static function on VisualBench which can do this
> clear.  Possibly also taking an N for single, double, or triple buffering.

How do I know what the buffer setup is?

https://codereview.chromium.org/1336043003/diff/60001/tools/VisualBench/Visua...
tools/VisualBench/VisualInteractiveModule.cpp:183: inline void
VisualInteractiveModule::tuneLoops(SkCanvas* canvas) {
On 2015/09/14 14:07:45, joshualitt wrote:
> it'd be nice to figure out how to share this code, but its not really
necessary
> for this change.

Acknowledged.

[jvanverth1, 2015-09-14 18:25:45]

Added a clear() method to VisualBench(). I tried to add it to VisualModule, but
there was a dependency loop that I couldn't seem to sort out. PTAL.

[joshualitt, 2015-09-15 13:38:13]

lgtm with a question

https://codereview.chromium.org/1336043003/diff/90001/tools/VisualBench/Visua...
File tools/VisualBench/VisualLightweightBenchModule.cpp (right):

https://codereview.chromium.org/1336043003/diff/90001/tools/VisualBench/Visua...
tools/VisualBench/VisualLightweightBenchModule.cpp:130: fOwner->clear(canvas,
SK_ColorWHITE, 1);
I think this should also be 2??

[jvanverth1, 2015-09-15 14:22:31]

On 2015/09/15 13:38:13, joshualitt wrote:
> lgtm with a question
> 
>
https://codereview.chromium.org/1336043003/diff/90001/tools/VisualBench/Visua...
> File tools/VisualBench/VisualLightweightBenchModule.cpp (right):
> 
>
https://codereview.chromium.org/1336043003/diff/90001/tools/VisualBench/Visua...
> tools/VisualBench/VisualLightweightBenchModule.cpp:130: fOwner->clear(canvas,
> SK_ColorWHITE, 1);
> I think this should also be 2??

Done.

[jvanverth1, 2015-09-15 14:22:36]

The CQ bit was checked by jvanverth@google.com

[jvanverth1, 2015-09-15 14:22:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1336043003/#ps110001
(title: "Use double clear w/ lightweight bench")

[commit-bot: I haz the power, 2015-09-15 14:22:45]

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

[commit-bot: I haz the power, 2015-09-15 14:24:07]

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

[commit-bot: I haz the power, 2015-09-15 14:24:08]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[jvanverth1, 2015-09-15 14:33:41]

The CQ bit was checked by jvanverth@google.com

[jvanverth1, 2015-09-15 14:33:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1336043003/#ps130001
(title: "Fix init order")

[commit-bot: I haz the power, 2015-09-15 14:33:48]

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

[commit-bot: I haz the power, 2015-09-15 14:41:01]

Committed patchset #8 (id:130001) as
https://skia.googlesource.com/skia/+/f5d1b2de031b4ae2130280dd09eadcccb001ba11