Add fast path in arcTo and addArc for 0==sweep && 0|360==sweepAngle

This appears to be one of the problems affecting http://code.google.com/p/chromium/issues/detail?id=125490. The width & height computations are prone to numerical instability and prevent the circle path from being taken.

Here is the speed up on the new Chrome-emulating circle test:

bench_compare.py -o w-o-fix.log -n w-fix.log
 circles GPU  c         17.78      16.30      +1.48    +8.3%
 circles GPU  g         16.35      13.07      +3.28   +20.1%

This change will require rebaselining of bigmatrix_gpu & bigmatrix_msaa16 both of which use circles.

[bsalomon, 2012-08-20 13:35:58]

On 2012/08/20 13:03:58, robertphillips wrote:

I thought that we would set the path bounds based on the rect rather than the
control points for ovals. Is that not correct, or is there something particular
about either the picture/pipe path or the way chrome adds the circle to path
that takes a different code path?

[bungeman, 2012-08-20 14:19:06]

Not sure if appropriate in this case, but also take a look at
src/ports/SkFloatUtils.h if it's just some ULPs that need to be ignored.

[robertphillips, 2012-08-20 14:30:24]

So ... addOval does stuff the original rect back into the path - but only when
the path is empty. Chrome adds a moveTo before and after every oval and thus
addOval doesn't preserve the original rectangle. I attempted to fix this by
extending addOval's "hasOnlyMoveTos" logic to the SkAutoPathBoundsUpdate class.
This does preserve the rectangle but we start getting a lot of asserts in the
SkPath::validate path since the computed bound is no longer enclosed by
SkPath::fBound.

Because the problem is actually with different bound computation methods I don't
think the SkFloatUtils.h code is applicable.

[bsalomon, 2012-08-20 14:33:15]

The other thing I meant to ask is why don't the x/y extreme control points
form a square. It seems like they should (or could be made to).


On Mon, Aug 20, 2012 at 10:30 AM, <robertphillips@google.com> wrote:

> So ... addOval does stuff the original rect back into the path - but
> only when the path is empty. Chrome adds a moveTo before and after every
> oval and thus addOval doesn't preserve the original rectangle. I
> attempted to fix this by extending addOval's "hasOnlyMoveTos" logic to
> the SkAutoPathBoundsUpdate class. This does preserve the rectangle but
> we start getting a lot of asserts in the SkPath::validate path since the
> computed bound is no longer enclosed by SkPath::fBound.
>
> Because the problem is actually with different bound computation methods
> I don't think the SkFloatUtils.h code is applicable.
>
>
http://codereview.appspot.com/**6463071/<http://codereview.appspot.com/6463071/>
>

[robertphillips, 2012-08-20 14:51:45]

The actual oval/circle points don't appear to be at fault. Chrome adds
the initial and final moveTo's by calling arcTo with startAngle=0 and
360 and sweepAngle=0. This creates points that are slightly outside of
the oval's bounding box thus messing up the bounding box for the
entire path.

On Mon, Aug 20, 2012 at 10:33 AM, Brian Salomon <bsalomon@google.com> wrote:
> The other thing I meant to ask is why don't the x/y extreme control points
> form a square. It seems like they should (or could be made to).
>
>
> On Mon, Aug 20, 2012 at 10:30 AM, <robertphillips@google.com> wrote:
>>
>> So ... addOval does stuff the original rect back into the path - but
>> only when the path is empty. Chrome adds a moveTo before and after every
>> oval and thus addOval doesn't preserve the original rectangle. I
>> attempted to fix this by extending addOval's "hasOnlyMoveTos" logic to
>> the SkAutoPathBoundsUpdate class. This does preserve the rectangle but
>> we start getting a lot of asserts in the SkPath::validate path since the
>> computed bound is no longer enclosed by SkPath::fBound.
>>
>> Because the problem is actually with different bound computation methods
>> I don't think the SkFloatUtils.h code is applicable.
>>
>> http://codereview.appspot.com/6463071/
>
>

[robertphillips, 2012-08-20 16:36:39]

Even faster:

bench\bench_compare.py -o w-o-fix.log -n w-fix.log
 circles 8888 c         24.34      22.46      +1.88    +7.7%
 circles GPU  c         18.33      13.26      +5.07   +27.7%
 circles GPU  g         16.65      10.70      +5.95   +35.7%

[bsalomon, 2012-08-20 16:50:44]

Great! LGTM.

http://codereview.appspot.com/6463071/diff/7002/src/core/SkPath.cpp
File src/core/SkPath.cpp (right):

http://codereview.appspot.com/6463071/diff/7002/src/core/SkPath.cpp#newcode937
src/core/SkPath.cpp:937: if (0 == sweepAngle && (0 == startAngle || 360 ==
startAngle)) {
Probably should be SkIntToScalar(360).

[robertphillips, 2012-08-20 17:25:00]

committed as r5190

http://codereview.appspot.com/6463071/diff/7002/src/core/SkPath.cpp
File src/core/SkPath.cpp (right):

http://codereview.appspot.com/6463071/diff/7002/src/core/SkPath.cpp#newcode937
src/core/SkPath.cpp:937: if (0 == sweepAngle && (0 == startAngle || 360 ==
startAngle)) {
On 2012/08/20 16:50:44, bsalomon wrote:
> Probably should be SkIntToScalar(360).

Done.