Cleanup isRect variants

Cleanup isRect variants

Committed: https://skia.googlesource.com/skia/+/91b0a3587342c48a1a6e1c4687686688737359da

[robertphillips, 2015-01-02 21:02:40]

robertphillips@google.com changed reviewers:
+ reed@google.com

[reed1, 2015-01-05 14:25:50]

Do we really need the rect parameter ever?

https://codereview.chromium.org/832083002/diff/20001/src/core/SkPath.cpp
File src/core/SkPath.cpp (right):

https://codereview.chromium.org/832083002/diff/20001/src/core/SkPath.cpp#newc...
src/core/SkPath.cpp:534: return (PathAsRect) (this->isRect(NULL, &isClosed,
direction) + isClosed);
now that is a compact line of code. Can we somehow make it easier to
glance/understand?

[robertphillips, 2015-01-05 16:53:48]

On 2015/01/05 14:25:50, reed1 wrote:
> Do we really need the rect parameter ever?
> 
> https://codereview.chromium.org/832083002/diff/20001/src/core/SkPath.cpp
> File src/core/SkPath.cpp (right):
> 
>
https://codereview.chromium.org/832083002/diff/20001/src/core/SkPath.cpp#newc...
> src/core/SkPath.cpp:534: return (PathAsRect) (this->isRect(NULL, &isClosed,
> direction) + isClosed);
> now that is a compact line of code. Can we somehow make it easier to
> glance/understand?

The rect parameter is needed (rather than just calling getBounds if isRect
returns true) because isRect will ignore trailing moveTo points while getBounds
will not.

[reed1, 2015-01-05 17:03:20]

ugh. ok. lets update the dox to make this very clear (both for this API and (I
presume) for path::getBounds as well.

lgtm

https://codereview.chromium.org/832083002/diff/20001/include/core/SkPath.h
File include/core/SkPath.h (right):

https://codereview.chromium.org/832083002/diff/20001/include/core/SkPath.h#ne...
include/core/SkPath.h:592: *  @param rect If not null, set to the bounds of the
rectangle
rect: Note : this bounds may be smaller than the path's bounds, since it is just
the bounds of the "drawable" parts of the path. e.g. a trailing MoveTo would be
ignored by this rect, but not by the path's bounds

[robertphillips, 2015-01-05 17:34:17]

re the asRect entry point, what do you think about removing it from SkPath and
moving it to SkClipStack.cpp (its only caller AFAICT).

[robertphillips, 2015-01-05 18:03:42]

The CQ bit was checked by robertphillips@google.com

[commit-bot: I haz the power, 2015-01-05 18:04:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/832083002/40001

[commit-bot: I haz the power, 2015-01-05 18:13:49]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/91b0a3587342c48a1a6e1c4687686688737359da