RoundRect contains

This only adds and exercises the SkRRect::contains(const SkRect&) method.

[reed1, 2013-04-24 15:22:44]

Hmmm

1. rect contains does not treat RB differently from TL (all are inclusive)
2. "loose" as a default bool arg.... ick. If we *really* need this in the public
api, then
    1. I think I would vote for a separate method or an enum
    2. I think we should treat SkRect and SkIRect similarly (if we must)

[jvanverth1, 2013-04-24 15:29:30]

https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp
File src/core/SkRRect.cpp (right):

https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp#newco...
src/core/SkRRect.cpp:200: //      a^2     b^2
This can be rewritten as b^2*x^2 + a^2*y^2 <= (ab)^2, which would be faster. I
guess the only precision issue might be if a and b are large, but I wouldn't
expect them to be larger than 1024 -- and in that case you might have problems
dividing by a large number.

https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp#newco...
src/core/SkRRect.cpp:217: 
You're checking to see if the corners are in the boundrect twice -- once above
(implictly in getBounds().contains(rect)) and then once for each point below. 
It'd be faster if you just check the  corners against the ovals once you know
the whole rect is in the boundrect. I think you could get rid of the loose
contains as well in that case.

https://codereview.chromium.org/14200044/diff/4001/tests/RoundRectTest.cpp
File tests/RoundRectTest.cpp (right):

https://codereview.chromium.org/14200044/diff/4001/tests/RoundRectTest.cpp#ne...
tests/RoundRectTest.cpp:343: static const SkVector gRadii[kNumRRects][4] = {
Super nitty nit: the commas aren't aligned :)

[robertphillips, 2013-04-24 16:11:57]

PTAL

https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp
File src/core/SkRRect.cpp (right):

https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp#newco...
src/core/SkRRect.cpp:200: //      a^2     b^2
On 2013/04/24 15:29:30, JimVV wrote:
> This can be rewritten as b^2*x^2 + a^2*y^2 <= (ab)^2, which would be faster. I
> guess the only precision issue might be if a and b are large, but I wouldn't
> expect them to be larger than 1024 -- and in that case you might have problems
> dividing by a large number.

Done.

https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp#newco...
src/core/SkRRect.cpp:217: 
On 2013/04/24 15:29:30, JimVV wrote:
> You're checking to see if the corners are in the boundrect twice -- once above
> (implictly in getBounds().contains(rect)) and then once for each point below. 
> It'd be faster if you just check the  corners against the ovals once you know
> the whole rect is in the boundrect. I think you could get rid of the loose
> contains as well in that case.

Done.

https://codereview.chromium.org/14200044/diff/4001/tests/RoundRectTest.cpp
File tests/RoundRectTest.cpp (right):

https://codereview.chromium.org/14200044/diff/4001/tests/RoundRectTest.cpp#ne...
tests/RoundRectTest.cpp:343: static const SkVector gRadii[kNumRRects][4] = {
On 2013/04/24 15:29:30, JimVV wrote:
> Super nitty nit: the commas aren't aligned :)

Done.

[jvanverth1, 2013-04-24 16:17:05]

LGTM. Especially the "Jim fixed everything" part.

On 2013/04/24 16:11:57, robertphillips wrote:
> PTAL
> 
> https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp
> File src/core/SkRRect.cpp (right):
> 
>
https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp#newco...
> src/core/SkRRect.cpp:200: //      a^2     b^2
> On 2013/04/24 15:29:30, JimVV wrote:
> > This can be rewritten as b^2*x^2 + a^2*y^2 <= (ab)^2, which would be faster.
I
> > guess the only precision issue might be if a and b are large, but I wouldn't
> > expect them to be larger than 1024 -- and in that case you might have
problems
> > dividing by a large number.
> 
> Done.
> 
>
https://codereview.chromium.org/14200044/diff/4001/src/core/SkRRect.cpp#newco...
> src/core/SkRRect.cpp:217: 
> On 2013/04/24 15:29:30, JimVV wrote:
> > You're checking to see if the corners are in the boundrect twice -- once
above
> > (implictly in getBounds().contains(rect)) and then once for each point
below. 
> > It'd be faster if you just check the  corners against the ovals once you
know
> > the whole rect is in the boundrect. I think you could get rid of the loose
> > contains as well in that case.
> 
> Done.
> 
> https://codereview.chromium.org/14200044/diff/4001/tests/RoundRectTest.cpp
> File tests/RoundRectTest.cpp (right):
> 
>
https://codereview.chromium.org/14200044/diff/4001/tests/RoundRectTest.cpp#ne...
> tests/RoundRectTest.cpp:343: static const SkVector gRadii[kNumRRects][4] = {
> On 2013/04/24 15:29:30, JimVV wrote:
> > Super nitty nit: the commas aren't aligned :)
> 
> Done.

[reed1, 2013-04-24 19:48:32]

lgtm

[robertphillips, 2013-04-25 12:23:17]

committed as r8854