Move oval rendering code to GrOvalRenderer

Move oval rendering code to GrOvalRenderer.

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

[bsalomon, 2013-03-21 02:08:53]

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer.h
File include/gpu/GrOvalRenderer.h (right):

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer...
include/gpu/GrOvalRenderer.h:28: }
can keep empty {} on same line if you like

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer...
include/gpu/GrOvalRenderer.h:33: bool canDrawOval(const GrContext* gpu, const
GrPaint& paint, const GrRect& oval, bool* isCircle) const;
nit: wrap (BTW Rob and I both have done some variant of
http://stackoverflow.com/questions/84209/adding-a-guideline-to-the-editor-in-...
to get a 100col guide in VS2010).

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrContext.cpp#newc...
src/gpu/GrContext.cpp:1007: if (!fOvalRenderer->canDrawOval(this, paint, oval,
&isCircle)) {
Would we be able to hide the circle/oval distinction in the class (without a
perf hit) by having a draw call here that returns false if it couldn't be
handled?

Something like

drawOval(...) {
    if (!fOvalRenderer->drawOval(...)) {
        SkPath path;
        path.addOval(oval);
        this->internalDrawPath(...);
    }
}

[robertphillips, 2013-03-21 13:12:39]

lgtm + nits

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer.h
File include/gpu/GrOvalRenderer.h (right):

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer...
include/gpu/GrOvalRenderer.h:1: /*
2013

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer...
include/gpu/GrOvalRenderer.h:20: /*
not rects

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

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:1: /*
2013

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:38: 
circle_stays_circle

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:77: 
Do we even need "rt"?

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:82: 
&

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:196: GrDrawState* drawState = target->drawState();
Do we even need "rt"?

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:206: 
&

[jvanverth1, 2013-03-21 18:42:18]

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer.h
File include/gpu/GrOvalRenderer.h (right):

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer...
include/gpu/GrOvalRenderer.h:1: /*
On 2013/03/21 13:12:39, robertphillips wrote:
> 2013

Done.

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer...
include/gpu/GrOvalRenderer.h:20: /*
On 2013/03/21 13:12:39, robertphillips wrote:
> not rects

Done.

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer...
include/gpu/GrOvalRenderer.h:28: }
On 2013/03/21 02:08:53, bsalomon wrote:
> can keep empty {} on same line if you like

Done.

https://codereview.chromium.org/12657003/diff/1006/include/gpu/GrOvalRenderer...
include/gpu/GrOvalRenderer.h:33: bool canDrawOval(const GrContext* gpu, const
GrPaint& paint, const GrRect& oval, bool* isCircle) const;
On 2013/03/21 02:08:53, bsalomon wrote:
> nit: wrap (BTW Rob and I both have done some variant of
>
http://stackoverflow.com/questions/84209/adding-a-guideline-to-the-editor-in-...
> to get a 100col guide in VS2010).

Done. And thanks for the pointer.

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrContext.cpp#newc...
src/gpu/GrContext.cpp:1007: if (!fOvalRenderer->canDrawOval(this, paint, oval,
&isCircle)) {
On 2013/03/21 02:08:53, bsalomon wrote:
> Would we be able to hide the circle/oval distinction in the class (without a
> perf hit) by having a draw call here that returns false if it couldn't be
> handled?
> 
> Something like
> 
> drawOval(...) {
>     if (!fOvalRenderer->drawOval(...)) {
>         SkPath path;
>         path.addOval(oval);
>         this->internalDrawPath(...);
>     }
> }

Done. There doesn't seem to be a perf hit, which is odd. Compilers, what can you
do?

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

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:1: /*
On 2013/03/21 13:12:39, robertphillips wrote:
> 2013

Done.

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:38: 
On 2013/03/21 13:12:39, robertphillips wrote:
> circle_stays_circle

Done.

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:77: 
On 2013/03/21 13:12:39, robertphillips wrote:
> Do we even need "rt"?

Done.

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:82: 
On 2013/03/21 13:12:39, robertphillips wrote:
> &

Done.

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:196: GrDrawState* drawState = target->drawState();
On 2013/03/21 13:12:39, robertphillips wrote:
> Do we even need "rt"?

Done.

https://codereview.chromium.org/12657003/diff/1006/src/gpu/GrOvalRenderer.cpp...
src/gpu/GrOvalRenderer.cpp:206: 
On 2013/03/21 13:12:39, robertphillips wrote:
> &

Done.

[bsalomon, 2013-03-21 18:46:22]

lgtm

[commit-bot: I haz the power, 2013-03-22 18:33:59]

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

[commit-bot: I haz the power, 2013-03-22 18:34:15]

Change committed as 8345