Better method for rendering ellipses

Better method for rendering AA ellipses.

This uses the standard ellipse equation as a signed distance 
test, and adjusts the result by the length of the gradient 
at that point to get a better approximation of the distance 
to the ellipse. It replaces the standard ellipse and 
roundrect shader renderers.

Also adds a check to see if the curvature extrema of the 
stroke are less than the curvature extrema of the ellipse (i.e. 
the radius of curvature is larger). In this case, it's no 
longer an ellipse and we can't use this renderer.

Only supports stroking for near-circular ellipses.

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

[jvanverth1, 2013-05-06 14:41:41]

Bench times on Mac:          
         path_fill_big_oval GPU            0.60       1.00      -0.40   -66.7% 
       path_stroke_big_oval GPU g          0.53       0.77      -0.24   -45.3% 
         path_fill_big_oval GPU g          0.45       0.58      -0.13   -28.9% 
     path_stroke_small_oval GPU g          0.80       0.82      -0.02    -2.5% 
       path_fill_small_oval GPU            2.00       2.00      +0.00    +0.0% 
       path_stroke_big_oval GPU            1.00       1.00      +0.00    +0.0% 
     path_stroke_small_oval GPU            2.00       2.00      +0.00    +0.0% 
       path_fill_small_oval GPU g          0.84       0.82      +0.02    +2.4% 
          rrects_3_stroke_4 GPU           13.02      12.02      +1.00    +7.7% 
                   rrects_1 GPU g          1.12       0.99      +0.13   +11.6% 
                    ovals_1 GPU g          1.08       0.95      +0.13   +12.0% 
          rrects_3_stroke_4 GPU g          5.29       4.58      +0.71   +13.4% 
                   rrects_3 GPU g          0.90       0.63      +0.27   +30.0% 
                    ovals_3 GPU            2.00       1.00      +1.00   +50.0% 
                    ovals_1 GPU            2.01       1.00      +1.01   +50.2% 
          rrects_1_stroke_4 GPU           18.96       9.00      +9.96   +52.5% 
                    ovals_3 GPU g          0.85       0.38      +0.47   +55.3% 
                   rrects_3 GPU            2.80       1.00      +1.80   +64.3% 
                   rrects_1 GPU            3.00       1.01      +1.99   +66.3% 
          rrects_1_stroke_4 GPU g         15.39       3.17     +12.22   +79.4% 

The regressions are due to the fact that the new shader is slower for the
near-circular case. However, it's also correct. rrects_3_stroke and
rrects_1_stroke didn't get as much speedup as expected because they have a lot
of degenerate cases, which this algorithm doesn't handle.

[bsalomon, 2013-05-06 15:17:23]

lgtm

[commit-bot: I haz the power, 2013-05-06 16:37:31]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/jvanverth@google.com/14938004/11002

[commit-bot: I haz the power, 2013-05-06 16:44:05]

Change committed as 9016

[jvanverth1, 2013-05-07 20:59:51]

Second try, which looks like it fixes the Nexus-7 bug. The issue appeared to be
that using the form of the equation that I did led to some floating point error
in larger shapes. Using the standard form appears to work better. 

You'll want to compare patch 6 to patch 9 for this. 

And since it gets rid of a couple of multiplies, it's slightly faster. Updated
times (from Mac): 

rrects_3_stroke_4 GPU           13.04      12.02      +1.02    +7.8% 
          ovals_1 GPU g          1.03       0.94      +0.09    +8.7% 
         rrects_1 GPU g          1.13       0.98      +0.15   +13.3% 
rrects_3_stroke_4 GPU g          5.32       4.58      +0.74   +13.9% 
 ovals_3_stroke_4 GPU           13.00       9.00      +4.00   +30.8% 
         rrects_3 GPU g          0.88       0.60      +0.28   +31.8% 
 ovals_3_stroke_4 GPU g          5.31       3.38      +1.93   +36.3% 
          ovals_1 GPU            2.00       1.00      +1.00   +50.0% 
          ovals_3 GPU            2.00       1.00      +1.00   +50.0% 
rrects_1_stroke_4 GPU           18.51       8.95      +9.56   +51.6% 
          ovals_3 GPU g          0.84       0.38      +0.46   +54.8% 
         rrects_3 GPU            2.66       1.00      +1.66   +62.4% 
 ovals_1_stroke_4 GPU           18.93       6.38     +12.55   +66.3% 
         rrects_1 GPU            3.00       1.00      +2.00   +66.7% 
rrects_1_stroke_4 GPU g         15.30       3.16     +12.14   +79.3% 
 ovals_1_stroke_4 GPU g         15.64       2.50     +13.14   +84.0% 

And the regressions:

path_stroke_big_oval GPU g          0.57       0.76      -0.19   -33.3% 
  path_fill_big_oval GPU            0.85       1.00      -0.15   -17.6% 
  path_fill_big_oval GPU g          0.52       0.53      -0.01    -1.9% 
path_stroke_big_oval GPU            1.01       1.01      +0.00    +0.0%

[bsalomon, 2013-05-07 21:01:56]

lgtm

[commit-bot: I haz the power, 2013-05-08 13:53:23]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/jvanverth@google.com/14938004/28001

[commit-bot: I haz the power, 2013-05-08 13:59:38]

Change committed as 9055

[jvanverth1, 2013-05-14 21:09:36]

This time for sure...

I've added a check to ensure we only use 1+ width strokes on near-circular
ellipses, otherwise it'll drop back to the path renderer. I also changed how the
ellipse calculation is done, to potentially avoid some floating point issues --
so now I multiply the offset by 1/radii, and then square the results.

Because it's not using the fast path for more strokes, there's not really a win
in general stroking (though there will be for hairlines), in fact there's a
slight regression. I'd still argue for it as the anti-aliasing is visibly better
with this method. Also, the old GPU method wasn't rendering two roundrects in
rrects_3_stroke_4, which explains some of the regression.

          rrects_3_stroke_4 GPU           13.09      13.71      -0.62    -4.7% 
     path_stroke_small_oval GPU            2.00       2.06      -0.06    -3.0% 
         path_fill_big_oval GPU            0.98       1.00      -0.02    -2.0% 
           ovals_1_stroke_4 GPU           18.91      19.00      -0.09    -0.5% 
           ovals_3_stroke_4 GPU           13.00      13.01      -0.01    -0.1% 
         path_fill_small_oval GPU            2.00       2.00      +0.00    +0.0%

       path_stroke_big_oval GPU            1.00       1.00      +0.00    +0.0% 
          rrects_1_stroke_4 GPU           19.00      19.00      +0.00    +0.0% 
                     ovals_3 GPU            2.00       1.00      +1.00   +50.0% 
                    ovals_1 GPU            2.01       1.00      +1.01   +50.2% 
                   rrects_1 GPU            3.00       1.01      +1.99   +66.3% 
                   rrects_3 GPU            2.99       1.00      +1.99   +66.6%

[robertphillips, 2013-05-14 22:44:30]

lgtm + nits

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cpp
File src/gpu/GrOvalRenderer.cpp (right):

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:477: // we don't handle it if curvature of the stroke
is less than curvature of the ellipse
The compiler should do this - but rejigger to remove /'s?

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:483: // this is legit only if scale & translation
(which should be the case at the moment)
isn't this if test redundant since we're in this block?

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:484: if (SkStrokeRec::kStroke_Style == style ||
SkStrokeRec::kHairline_Style == style) {
Do innerXRadius first to match xRadius/yRadius order after this block?

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:523: // Compute the reciprocals of the squares of the
radii here to save time in the shader
I think there is a scalar macro for this

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:525: SkScalar yRadRecip = SK_Scalar1/yRadius;
What happens when innerXRadius is zero?

[bsalomon, 2013-05-15 13:42:41]

https://codereview.chromium.org/14938004/diff/42001/src/gpu/GrOvalRenderer.cpp
File src/gpu/GrOvalRenderer.cpp (right):

https://codereview.chromium.org/14938004/diff/42001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:462: bool isStroked = (SkStrokeRec::kStroke_Style ==
style || SkStrokeRec::kHairline_Style == style);
Is it possible to get here with kStrokeAndFill? Or is that handled upstack
somewhere?

[jvanverth1, 2013-05-15 18:34:12]

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cpp
File src/gpu/GrOvalRenderer.cpp (right):

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:477: // we don't handle it if curvature of the stroke
is less than curvature of the ellipse
On 2013/05/14 22:44:30, robertphillips wrote:
> The compiler should do this - but rejigger to remove /'s?

Good catch, thanks.

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:483: // this is legit only if scale & translation
(which should be the case at the moment)
On 2013/05/14 22:44:30, robertphillips wrote:
> isn't this if test redundant since we're in this block?

No, because it could also be stroked and filled. However, I did already compute
this boolean (for isStroked).

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:484: if (SkStrokeRec::kStroke_Style == style ||
SkStrokeRec::kHairline_Style == style) {
On 2013/05/14 22:44:30, robertphillips wrote:
> Do innerXRadius first to match xRadius/yRadius order after this block?

Done.

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:523: // Compute the reciprocals of the squares of the
radii here to save time in the shader
On 2013/05/14 22:44:30, robertphillips wrote:
> I think there is a scalar macro for this

Done.

https://codereview.chromium.org/14938004/diff/39001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:525: SkScalar yRadRecip = SK_Scalar1/yRadius;
On 2013/05/14 22:44:30, robertphillips wrote:
> What happens when innerXRadius is zero?

Added a check above. If it's zero, then we'll use the fill shader, and that
won't use the inner parameters.

https://codereview.chromium.org/14938004/diff/42001/src/gpu/GrOvalRenderer.cpp
File src/gpu/GrOvalRenderer.cpp (right):

https://codereview.chromium.org/14938004/diff/42001/src/gpu/GrOvalRenderer.cp...
src/gpu/GrOvalRenderer.cpp:462: bool isStroked = (SkStrokeRec::kStroke_Style ==
style || SkStrokeRec::kHairline_Style == style);
On 2013/05/15 13:42:42, bsalomon wrote:
> Is it possible to get here with kStrokeAndFill? Or is that handled upstack
> somewhere?

We can -- for near-round ellipses it expands the boundrect a little. Otherwise
it'll get caught by line 474 and sent back to the path renderer.

[bsalomon, 2013-05-15 20:01:36]

lgtm

[commit-bot: I haz the power, 2013-05-16 13:07:53]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/jvanverth@google.com/14938004/45001

[commit-bot: I haz the power, 2013-05-16 13:14:48]

Change committed as 9162