add convex and concave polygons GM test

Non-rect polygons are not covered in GM cases, such as triangle, trapezoid, diamond, polygons with lots of edges, concave polygons, etc, especially for stroke-style and stroke-and-fill style painters. So add a GM case to avoid potential rendering errors.

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

[robertphillips, 2013-11-21 13:59:01]

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp
File gm/polygons.cpp (right):

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode13
gm/polygons.cpp:13: 
I think you should remove this (SkOnce) and just switch "makePolygons" to be an
override of "onOnceBeforeDraw"

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode42
gm/polygons.cpp:42: 
Would it be feasible to make this computation live (rather then a comment)?
Potentially replace 8 with kNumPolygons?
Potentially replace 100 with kCellSize (or something)?
3 -> kNumWidths; 3 -> kNumJoins; 2 -> kNumExtraStyles; ?

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode57
gm/polygons.cpp:57: SkPoint p3[] = {{10, 0}, {50, 0}, {60, 10}, {60, 30}, {50,
40},
octagon

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode83
gm/polygons.cpp:83: };
Maybe statically assert SK_ARRAY_COUNT(pgs) == kNumPolygons?

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode98
gm/polygons.cpp:98: static void SetLocation(SkCanvas* canvas, int counter, int
lineNum) {
Replace 100's with kCellSize?

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode116
gm/polygons.cpp:116: static const int kStrokeWidth[] = {0, 10, 40};
Maybe rename "numWidths" to "numStrokeWidths"? For clarity.

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode118
gm/polygons.cpp:118: 
Maybe rename "kJoin" to "kJoins"?

https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode169
gm/polygons.cpp:169: private:
Maybe add kNumPolygons?
The INHERITED line should go last?

[yunchao, 2013-11-22 07:02:04]

On 2013/11/21 13:59:01, robertphillips wrote:
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp
> File gm/polygons.cpp (right):
> 
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode13
> gm/polygons.cpp:13: 
> I think you should remove this (SkOnce) and just switch "makePolygons" to be
an
> override of "onOnceBeforeDraw"
> 
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode42
> gm/polygons.cpp:42: 
> Would it be feasible to make this computation live (rather then a comment)?
> Potentially replace 8 with kNumPolygons?
> Potentially replace 100 with kCellSize (or something)?
> 3 -> kNumWidths; 3 -> kNumJoins; 2 -> kNumExtraStyles; ?
> 
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode57
> gm/polygons.cpp:57: SkPoint p3[] = {{10, 0}, {50, 0}, {60, 10}, {60, 30}, {50,
> 40},
> octagon
> 
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode83
> gm/polygons.cpp:83: };
> Maybe statically assert SK_ARRAY_COUNT(pgs) == kNumPolygons?
> 
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode98
> gm/polygons.cpp:98: static void SetLocation(SkCanvas* canvas, int counter, int
> lineNum) {
> Replace 100's with kCellSize?
> 
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode116
> gm/polygons.cpp:116: static const int kStrokeWidth[] = {0, 10, 40};
> Maybe rename "numWidths" to "numStrokeWidths"? For clarity.
> 
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode118
> gm/polygons.cpp:118: 
> Maybe rename "kJoin" to "kJoins"?
> 
> https://codereview.chromium.org/80483002/diff/60001/gm/polygons.cpp#newcode169
> gm/polygons.cpp:169: private:
> Maybe add kNumPolygons?
> The INHERITED line should go last?

Hi Rob, thanks for your suggestions. I have updated the code accordingly. Please
help to review it.

[robertphillips, 2013-11-22 13:27:04]

Looking good - just 2 more nits. Could you e-mail us (Brian & me) an image?

https://codereview.chromium.org/80483002/diff/110001/gm/polygons.cpp
File gm/polygons.cpp (right):

https://codereview.chromium.org/80483002/diff/110001/gm/polygons.cpp#newcode67
gm/polygons.cpp:67: SkASSERT(SK_ARRAY_COUNT(pgs) == kNumPolygons);
These two don't seem to be used after the following nested loop so we might as
well move them into the for loops (e.g., "for (size_t pgIndex ...")

https://codereview.chromium.org/80483002/diff/110001/gm/polygons.cpp#newcode139
gm/polygons.cpp:139: paint.setStyle(kStyles[style]);
The "setStrokeWidth" call could be pulled out of the loop, right?

[yunchao, 2013-11-22 15:22:26]

On 2013/11/22 13:27:04, robertphillips wrote:
> Looking good - just 2 more nits. Could you e-mail us (Brian & me) an image?
> 
> https://codereview.chromium.org/80483002/diff/110001/gm/polygons.cpp
> File gm/polygons.cpp (right):
> 
> https://codereview.chromium.org/80483002/diff/110001/gm/polygons.cpp#newcode67
> gm/polygons.cpp:67: SkASSERT(SK_ARRAY_COUNT(pgs) == kNumPolygons);
> These two don't seem to be used after the following nested loop so we might as
> well move them into the for loops (e.g., "for (size_t pgIndex ...")
> 
>
https://codereview.chromium.org/80483002/diff/110001/gm/polygons.cpp#newcode139
> gm/polygons.cpp:139: paint.setStyle(kStyles[style]);
> The "setStrokeWidth" call could be pulled out of the loop, right?

Thank you Rob. I have updated the code according to your suggestions. Images of
this GM case will be sent to you and Brian tomorrow or next Monday.

[yunchao, 2013-11-25 17:41:52]

Hi Brian and Rob, do you know why the trybot failed?

[robertphillips, 2013-11-25 18:00:51]

polygons.cpp(35): error C2146: syntax error : missing ';' before identifier
'override'
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
polygons.cpp(35): error C2182: 'onOnceBeforeDraw' : illegal use of type 'void'
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
polygons.cpp(35): error C2433: 'skiagm::PolygonsGM::onOnceBeforeDraw' :
'virtual' not permitted on data declarations
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
polygons.cpp(35): error C4430: missing type specifier - int assumed. Note: C++
does not support default-int
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
polygons.cpp(77): warning C4183: 'override': missing return type; assumed to be
a member function returning 'int'
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]

[robertphillips, 2013-11-25 18:02:11]

On 2013/11/25 18:00:51, robertphillips wrote:
> polygons.cpp(35): error C2146: syntax error : missing ';' before identifier
> 'override'
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> polygons.cpp(35): error C2182: 'onOnceBeforeDraw' : illegal use of type 'void'
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> polygons.cpp(35): error C2433: 'skiagm::PolygonsGM::onOnceBeforeDraw' :
> 'virtual' not permitted on data declarations
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> polygons.cpp(35): error C4430: missing type specifier - int assumed. Note: C++
> does not support default-int
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> polygons.cpp(77): warning C4183: 'override': missing return type; assumed to
be
> a member function returning 'int'
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]

SK_OVERRIDE should go after the parens

[yunchao, 2013-11-26 05:13:57]

On 2013/11/25 18:02:11, robertphillips wrote:
> On 2013/11/25 18:00:51, robertphillips wrote:
> > polygons.cpp(35): error C2146: syntax error : missing ';' before identifier
> > 'override'
> >
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> > polygons.cpp(35): error C2182: 'onOnceBeforeDraw' : illegal use of type
'void'
> >
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> > polygons.cpp(35): error C2433: 'skiagm::PolygonsGM::onOnceBeforeDraw' :
> > 'virtual' not permitted on data declarations
> >
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> > polygons.cpp(35): error C4430: missing type specifier - int assumed. Note:
C++
> > does not support default-int
> >
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> > polygons.cpp(77): warning C4183: 'override': missing return type; assumed to
> be
> > a member function returning 'int'
> >
>
[C:\0\Test-Win7-ShuttleA-HD2000-x86-Release-ANGLE-Trybot\build\skia\out\gyp\SampleApp.vcxproj]
> 
> SK_OVERRIDE should go after the parens

Yeah, you are right, Rob. Thank you. 

BTW, seems that I have no access to skia trybot. Is it possible to open trybot
to non-committers?

[robertphillips, 2013-11-26 15:50:16]

lgtm

[robertphillips, 2013-11-26 15:51:44]

We are (hopefully) going to review/update/formalize all the permissions & how to
get those permissions soon - so I will keep you posted.

[yunchao, 2013-11-26 15:53:57]

On 2013/11/26 15:51:44, robertphillips wrote:
> We are (hopefully) going to review/update/formalize all the permissions & how
to
> get those permissions soon - so I will keep you posted.

OK. Thank you, Rob.

[commit-bot: I haz the power, 2013-11-27 00:58:48]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/yunchao.he@intel.com/80483002/260001

[commit-bot: I haz the power, 2013-11-27 01:24:56]

Change committed as 12413

[bungeman-skia, 2013-11-27 16:30:22]

See also follow-up https://code.google.com/p/skia/source/detail?r=12418 which
replaces 'cos' and 'sin' with 'SkScalarCos' and 'SkScalarSin'.

[yunchao, 2013-11-28 01:12:03]

On 2013/11/27 16:30:22, bungeman1 wrote:
> See also follow-up https://code.google.com/p/skia/source/detail?r=12418 which
> replaces 'cos' and 'sin' with 'SkScalarCos' and 'SkScalarSin'.

Thank you, I will be aware of this.