Allow GMs ato be used as benchmarks. Make convex_poly_clip opt in.

Allow GMs to be used as benchmarks. Make convex_poly_clip opt in.

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

[reed1, 2014-02-03 13:20:41]

https://codereview.chromium.org/151843002/diff/30001/bench/SkGMBench.cpp
File bench/SkGMBench.cpp (right):

https://codereview.chromium.org/151843002/diff/30001/bench/SkGMBench.cpp#newc...
bench/SkGMBench.cpp:42: fGM->draw(canvas);
I think the draw == drawBackground + drawContent. Perhaps you loop should call
drawContent, to avoid the built-in backgroundraw each time...

https://codereview.chromium.org/151843002/diff/30001/bench/SkGMBench.cpp#newc...
bench/SkGMBench.cpp:46: SkIPoint SkGMBench::onGetSize() {
clearly we need to change the signature of onGetSize to return ISize instead of
IPoint... not for this CL.

https://codereview.chromium.org/151843002/diff/30001/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/151843002/diff/30001/bench/benchmain.cpp#newc...
bench/benchmain.cpp:36: class AutoUnrefArray {
Does this need a new class from scratch? Can you use/subclass SkTDArray, which
already has unrefAll()?

[bsalomon, 2014-02-03 14:02:45]

https://codereview.chromium.org/151843002/diff/30001/bench/SkGMBench.cpp
File bench/SkGMBench.cpp (right):

https://codereview.chromium.org/151843002/diff/30001/bench/SkGMBench.cpp#newc...
bench/SkGMBench.cpp:42: fGM->draw(canvas);
On 2014/02/03 13:20:42, reed1 wrote:
> I think the draw == drawBackground + drawContent. Perhaps you loop should call
> drawContent, to avoid the built-in backgroundraw each time...

Ah.. didn't realize that.

https://codereview.chromium.org/151843002/diff/30001/bench/SkGMBench.cpp#newc...
bench/SkGMBench.cpp:46: SkIPoint SkGMBench::onGetSize() {
On 2014/02/03 13:20:42, reed1 wrote:
> clearly we need to change the signature of onGetSize to return ISize instead
of
> IPoint... not for this CL.

yup

https://codereview.chromium.org/151843002/diff/30001/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/151843002/diff/30001/bench/benchmain.cpp#newc...
bench/benchmain.cpp:36: class AutoUnrefArray {
On 2014/02/03 13:20:42, reed1 wrote:
> Does this need a new class from scratch? Can you use/subclass SkTDArray, which
> already has unrefAll()?

Done. Inherited in order to override the destructor.

[reed1, 2014-02-03 14:09:56]

lgtm w/ nits/questions

https://codereview.chromium.org/151843002/diff/190001/bench/SkGMBench.cpp
File bench/SkGMBench.cpp (right):

https://codereview.chromium.org/151843002/diff/190001/bench/SkGMBench.cpp#new...
bench/SkGMBench.cpp:40: fGM->drawBackground(canvas); // necessary?
remove or clarify this comment?

// Do we care about timing the draw of the background (once) ?
// Does the GM ever rely on drawBackground to lazily compute something?

https://codereview.chromium.org/151843002/diff/190001/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/151843002/diff/190001/bench/benchmain.cpp#new...
bench/benchmain.cpp:39: RefCntArray() {}
nit: do you even need this empty constructor?

[bsalomon, 2014-02-03 14:34:29]

https://codereview.chromium.org/151843002/diff/190001/bench/SkGMBench.cpp
File bench/SkGMBench.cpp (right):

https://codereview.chromium.org/151843002/diff/190001/bench/SkGMBench.cpp#new...
bench/SkGMBench.cpp:40: fGM->drawBackground(canvas); // necessary?
On 2014/02/03 14:09:57, reed1 wrote:
> remove or clarify this comment?
> 
> // Do we care about timing the draw of the background (once) ?
> // Does the GM ever rely on drawBackground to lazily compute something?

Done.

https://codereview.chromium.org/151843002/diff/190001/bench/benchmain.cpp
File bench/benchmain.cpp (right):

https://codereview.chromium.org/151843002/diff/190001/bench/benchmain.cpp#new...
bench/benchmain.cpp:39: RefCntArray() {}
On 2014/02/03 14:09:57, reed1 wrote:
> nit: do you even need this empty constructor?

Done.

[bsalomon, 2014-02-03 14:34:34]

The CQ bit was checked by bsalomon@google.com

[commit-bot: I haz the power, 2014-02-03 14:34:41]

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

[commit-bot: I haz the power, 2014-02-03 14:48:22]

Change committed as 13279

[commit-bot: I haz the power, 2014-02-03 14:48:26]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-03 14:48:29]

CQ bit was unchecked on CL. Ignoring.