make RRect and Oval first-class drawing primitives in SkCanvas.

make RRect and Oval first-class drawing primitives in SkCanvas.
add RRect as a first-class clip primitive.
Committed: https://code.google.com/p/skia/source/detail?r=6762

[robertphillips, 2012-12-12 15:44:57]

LGTM modulo removing the fancy save/restore path. Comments & questions
throughout.

https://codereview.appspot.com/6923058/diff/2001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.appspot.com/6923058/diff/2001/include/core/SkCanvas.h#newc...
include/core/SkCanvas.h:354: /**
rectangle -> round rectangle

https://codereview.appspot.com/6923058/diff/2001/include/core/SkCanvas.h#newc...
include/core/SkCanvas.h:600: *  based on the Style in the paint.
Don't we need two @params here?

https://codereview.appspot.com/6923058/diff/2001/include/core/SkPicture.h
File include/core/SkPicture.h (right):

https://codereview.appspot.com/6923058/diff/2001/include/core/SkPicture.h#new...
include/core/SkPicture.h:161: // V10: add drawRRect, drawOval, clipRRect
Actually change the version number?

https://codereview.appspot.com/6923058/diff/2001/include/core/SkRRect.h
File include/core/SkRRect.h (right):

https://codereview.appspot.com/6923058/diff/2001/include/core/SkRRect.h#newco...
include/core/SkRRect.h:231: 
If this is useful I think we should have a "const SkVector* radii() const" entry
point and no friends.

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:636: #ifdef SPEW_CLIP_SKIPPING
Shouldn't we have a skipRRect here?

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:752: #ifdef SPEW_CLIP_SKIPPING
and use skipRRect here?

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPictureRecord.cpp
File src/core/SkPictureRecord.cpp (right):

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPictureRecord.cpp...
src/core/SkPictureRecord.cpp:451: if (rrect.isRect()) {
call drawRect here?

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPictureRecord.cpp...
src/core/SkPictureRecord.cpp:455: } else if (rrect.isOval()) {
call drawOval here?

https://codereview.appspot.com/6923058/diff/2001/src/utils/SkDumpCanvas.cpp
File src/utils/SkDumpCanvas.cpp (right):

https://codereview.appspot.com/6923058/diff/2001/src/utils/SkDumpCanvas.cpp#n...
src/utils/SkDumpCanvas.cpp:42: } else if (rrect.isSimple()) {
const SkVector& rad?

https://codereview.appspot.com/6923058/diff/2001/src/utils/SkDumpCanvas.cpp#n...
src/utils/SkDumpCanvas.cpp:45: } else if (rrect.isComplex()) {
else {
 SkASSERT(rrect.isComplex())
 ...
?

https://codereview.appspot.com/6923058/diff/2001/src/utils/SkDumpCanvas.cpp#n...
src/utils/SkDumpCanvas.cpp:46: SkVector radii[4] = {
If we add the other radii accessor we should replace this with a "const
SkVector* radii"

[reed1, 2012-12-12 16:37:55]

https://codereview.appspot.com/6923058/diff/2001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.appspot.com/6923058/diff/2001/include/core/SkCanvas.h#newc...
include/core/SkCanvas.h:354: /**
On 2012/12/12 15:44:57, robertphillips wrote:
> rectangle -> round rectangle

Done.

https://codereview.appspot.com/6923058/diff/2001/include/core/SkCanvas.h#newc...
include/core/SkCanvas.h:600: *  based on the Style in the paint.
On 2012/12/12 15:44:57, robertphillips wrote:
> Don't we need two @params here?

Done.

https://codereview.appspot.com/6923058/diff/2001/include/core/SkPicture.h
File include/core/SkPicture.h (right):

https://codereview.appspot.com/6923058/diff/2001/include/core/SkPicture.h#new...
include/core/SkPicture.h:161: // V10: add drawRRect, drawOval, clipRRect
On 2012/12/12 15:44:57, robertphillips wrote:
> Actually change the version number?

Done.

https://codereview.appspot.com/6923058/diff/2001/include/core/SkRRect.h
File include/core/SkRRect.h (right):

https://codereview.appspot.com/6923058/diff/2001/include/core/SkRRect.h#newco...
include/core/SkRRect.h:231: 
On 2012/12/12 15:44:57, robertphillips wrote:
> If this is useful I think we should have a "const SkVector* radii() const"
entry
> point and no friends.

If we're going to guarantee that (1) we store the contiguously, and (2) in a
specific order... 
We don't make the promise on SkPath, though it would help some clients. I'm not
convinced we should yet. Certainly SkPath can be a trusted friend :)

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:636: #ifdef SPEW_CLIP_SKIPPING
On 2012/12/12 15:44:57, robertphillips wrote:
> Shouldn't we have a skipRRect here?

Done.

https://codereview.appspot.com/6923058/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:752: #ifdef SPEW_CLIP_SKIPPING
On 2012/12/12 15:44:57, robertphillips wrote:
> and use skipRRect here?

Done.

https://codereview.appspot.com/6923058/diff/2001/src/utils/SkDumpCanvas.cpp
File src/utils/SkDumpCanvas.cpp (right):

https://codereview.appspot.com/6923058/diff/2001/src/utils/SkDumpCanvas.cpp#n...
src/utils/SkDumpCanvas.cpp:45: } else if (rrect.isComplex()) {
On 2012/12/12 15:44:57, robertphillips wrote:
> else {
>  SkASSERT(rrect.isComplex())
>  ...
> ?

isEmpty and isRect are all folded into the missing } else {, since they just
want the shared [%g %g %g %] at the beginning.

[reed1, 2012-12-12 17:17:43]

now with all flattening/unflattening going through readFromMemory/writeToMemory,
similar to SkMatrix.
added helpers for that to SkReader32 and SkWriter32 (similar to matrix)

[robertphillips, 2012-12-12 18:29:00]

Two suggestions

Note: the debugger will need to be patched up for this too.

https://codereview.appspot.com/6923058/diff/9024/include/core/SkRRect.h
File include/core/SkRRect.h (right):

https://codereview.appspot.com/6923058/diff/9024/include/core/SkRRect.h#newco...
include/core/SkRRect.h:176: 
// Return the radii when the rrect is simple (i.e., all x radii are equal and
all y radii are equal)

https://codereview.appspot.com/6923058/diff/9024/src/core/SkRRect.cpp
File src/core/SkRRect.cpp (right):

https://codereview.appspot.com/6923058/diff/9024/src/core/SkRRect.cpp#newcode238
src/core/SkRRect.cpp:238: SkScalar storage[12];
"&fRadii[4]-&fRect==kSizeInMemory" somewhere?

https://codereview.appspot.com/6923058/diff/9024/src/utils/SkDumpCanvas.cpp
File src/utils/SkDumpCanvas.cpp (right):

https://codereview.appspot.com/6923058/diff/9024/src/utils/SkDumpCanvas.cpp#n...
src/utils/SkDumpCanvas.cpp:45: } else if (rrect.isComplex()) {
How about:
str->appendf("(");
for (int i = 0; i < 4; ++) {
  str->appendf("%g,%g ", rrect.radii(i).x(), rrect.radii(i).y());
}
str->appendf(")");
?

[reed1, 2012-12-12 18:43:29]

https://codereview.appspot.com/6923058/diff/9024/include/core/SkRRect.h
File include/core/SkRRect.h (right):

https://codereview.appspot.com/6923058/diff/9024/include/core/SkRRect.h#newco...
include/core/SkRRect.h:176: 
On 2012/12/12 18:29:00, robertphillips wrote:
> // Return the radii when the rrect is simple (i.e., all x radii are equal and
> all y radii are equal)

Done.

https://codereview.appspot.com/6923058/diff/9024/src/core/SkRRect.cpp
File src/core/SkRRect.cpp (right):

https://codereview.appspot.com/6923058/diff/9024/src/core/SkRRect.cpp#newcode238
src/core/SkRRect.cpp:238: SkScalar storage[12];
On 2012/12/12 18:29:00, robertphillips wrote:
> "&fRadii[4]-&fRect==kSizeInMemory" somewhere?

Done.

https://codereview.appspot.com/6923058/diff/9024/src/utils/SkDumpCanvas.cpp
File src/utils/SkDumpCanvas.cpp (right):

https://codereview.appspot.com/6923058/diff/9024/src/utils/SkDumpCanvas.cpp#n...
src/utils/SkDumpCanvas.cpp:45: } else if (rrect.isComplex()) {
On 2012/12/12 18:29:00, robertphillips wrote:
> How about:
> str->appendf("(");
> for (int i = 0; i < 4; ++) {
>   str->appendf("%g,%g ", rrect.radii(i).x(), rrect.radii(i).y());
> }
> str->appendf(")");
> ?

Doesn't necessarily guarantee the order over time, and these explicit enum
references do.

[robertphillips, 2012-12-12 18:57:52]

lgtm