Add implicit hairline conic rendering to GPU

Add implicit hairline conic rendering to GPU

R=bsalomon@google.com

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

[egdaniel, 2013-07-10 14:35:36]

Straight implicit version of conic hairlines for gpu

Not ready for being patched in since it only handles hairlines at the moment and
I'm not sure how it will merge/work with Mike's conic code as of now.

[bsalomon, 2013-07-10 15:14:36]

I think it's OK to land hairlines first

https://codereview.chromium.org/18258005/diff/1/include/core/SkGeometry.h
File include/core/SkGeometry.h (right):

https://codereview.chromium.org/18258005/diff/1/include/core/SkGeometry.h#new...
include/core/SkGeometry.h:79: float SkGetTQuadAtMaxCurvature(const SkPoint
src[3]);
It read don't right.

SkGetQuadTAtMaxCurvature?
SkGetQuadMaxCurvatureT?

https://codereview.chromium.org/18258005/diff/1/src/core/SkGeometry.cpp
File src/core/SkGeometry.cpp (right):

https://codereview.chromium.org/18258005/diff/1/src/core/SkGeometry.cpp#newco...
src/core/SkGeometry.cpp:421: //  F(t)    = a (1 - t) ^ 2 + 2 b t (1 - t) + c t ^
2
Maybe these comments belong with the new function that does all the work?

https://codereview.chromium.org/18258005/diff/1/src/gpu/GrAAHairLinePathRende...
File src/gpu/GrAAHairLinePathRenderer.cpp (right):

https://codereview.chromium.org/18258005/diff/1/src/gpu/GrAAHairLinePathRende...
src/gpu/GrAAHairLinePathRenderer.cpp:105: !push_quad_index_data(cIdxBuf)) {
can't we just use one idx buffer then?

https://codereview.chromium.org/18258005/diff/1/src/gpu/GrAAHairLinePathRende...
src/gpu/GrAAHairLinePathRenderer.cpp:166: int is_degen_quad_conic(const SkPoint
p[3]) {
quad_or_conic?

https://codereview.chromium.org/18258005/diff/1/src/gpu/GrAAHairLinePathRende...
src/gpu/GrAAHairLinePathRenderer.cpp:270: SkScalar t =
SkGetTQuadAtMaxCurvature(pathPts);
maybe this whole bit up to the for should be added to a chop conic helper.

https://codereview.chromium.org/18258005/diff/1/src/gpu/GrAAHairLinePathRende...
src/gpu/GrAAHairLinePathRenderer.cpp:274: {
move {s up to prev line

https://codereview.chromium.org/18258005/diff/1/src/gpu/GrAAHairLinePathRende...
src/gpu/GrAAHairLinePathRenderer.cpp:298: // when in perspective keep conics in
src space ???
should test this... but yes. The later chopping that is done to reduce over fill
has to chop in src space if the matrix has perspective.

[reed1, 2013-07-10 16:00:36]

https://codereview.chromium.org/18258005/diff/1/include/core/SkGeometry.h
File include/core/SkGeometry.h (right):

https://codereview.chromium.org/18258005/diff/1/include/core/SkGeometry.h#new...
include/core/SkGeometry.h:79: float SkGetTQuadAtMaxCurvature(const SkPoint
src[3]);
On 2013/07/10 15:14:36, bsalomon wrote:
> It read don't right.
> 
> SkGetQuadTAtMaxCurvature?
> SkGetQuadMaxCurvatureT?

SkFindQuadMaxCurvature

[egdaniel, 2013-07-10 20:34:18]

https://codereview.chromium.org/18258005/diff/1/src/gpu/GrAAHairLinePathRende...
File src/gpu/GrAAHairLinePathRenderer.cpp (right):

https://codereview.chromium.org/18258005/diff/1/src/gpu/GrAAHairLinePathRende...
src/gpu/GrAAHairLinePathRenderer.cpp:105: !push_quad_index_data(cIdxBuf)) {
I'm assuming you mean for the quad and conic correct?
I just separated them in case at some point we don't use the same setup of 5
vertices (and 3 triangles) that are used in quads when we render conics. But as
of now the two can definitely be combined into 1.

On 2013/07/10 15:14:36, bsalomon wrote:
> can't we just use one idx buffer then?

[egdaniel, 2013-07-11 15:22:38]

Updated code based on comments, and removed my local test files from CL

[egdaniel, 2013-07-11 17:58:26]

Moved chop_conic back into Renderer as helper function instead of in skgeometry.
Also finished merging conic and quad index buffer and const variables

[bsalomon, 2013-07-11 18:48:38]

lgtm with some minor nits. Any before/after perf numbers? (There is a sample
called HairCurves that might be worth updating and using to compare perf).

https://codereview.chromium.org/18258005/diff/12001/src/gpu/GrAAHairLinePathR...
File src/gpu/GrAAHairLinePathRenderer.cpp (right):

https://codereview.chromium.org/18258005/diff/12001/src/gpu/GrAAHairLinePathR...
src/gpu/GrAAHairLinePathRenderer.cpp:301: break;
I think we put the break inside the last }

https://codereview.chromium.org/18258005/diff/12001/src/gpu/GrAAHairLinePathR...
src/gpu/GrAAHairLinePathRenderer.cpp:1168: kVertsPerLineSeg * lineCnt +
kVertsPerQuad*quads,  // startV
can we realign the comments (here and below)?

[egdaniel, 2013-07-12 14:28:51]

I used the HairCurves sample to draw 600 random quads/conics. Just using sample
apps 'f key' redraw feature, both quads and conics were drawing in virtually the
same amount of time (between 3.01 ms - 3.12 ms). So I don't see a how lot of
performance decrease, at least relative to quad drawing, in this version of
conics.

[jvanverth1, 2013-07-12 15:09:43]

If we don't have one, we should probably add a bench and a gm to track perf and
rendering changes. rrect_bw_gpu might cover the gm side, once Mike gets his
change in.

On 2013/07/12 14:28:51, egdaniel wrote:
> I used the HairCurves sample to draw 600 random quads/conics. Just using
sample
> apps 'f key' redraw feature, both quads and conics were drawing in virtually
the
> same amount of time (between 3.01 ms - 3.12 ms). So I don't see a how lot of
> performance decrease, at least relative to quad drawing, in this version of
> conics.

[egdaniel, 2013-07-12 20:15:37]

Committed patchset #4 manually as r10055 (presubmit successful).