Refactoring code in SkPictureRecord that touches fRestoreOffsetStack

[reed1, 2012-06-29 20:46:47]

http://codereview.appspot.com/6350059/diff/1/src/core/SkPictureRecord.cpp
File src/core/SkPictureRecord.cpp (left):

http://codereview.appspot.com/6350059/diff/1/src/core/SkPictureRecord.cpp#old...
src/core/SkPictureRecord.cpp:191: addInt(ClipParams_pack(op, doAA));
small, but I think I prefer the old name, so it doesn't look like another
"add.." function, all of which add something to the opcode stream.

http://codereview.appspot.com/6350059/diff/1/src/core/SkPictureRecord.cpp
File src/core/SkPictureRecord.cpp (right):

http://codereview.appspot.com/6350059/diff/1/src/core/SkPictureRecord.cpp#new...
src/core/SkPictureRecord.cpp:173: void
SkPictureRecord::addRestoreOffset(SkRegion::Op op) {
checking for underflow (too many restores) seems unrelated to the refactoring
part of this CL. Is it related (i.e. am I missing something).

[junov1, 2012-07-04 17:46:00]

On 2012/06/29 20:46:47, reed1 wrote:
http://codereview.appspot.com/6350059/diff/1/src/core/SkPictureRecord.cpp#old...
> src/core/SkPictureRecord.cpp:191: addInt(ClipParams_pack(op, doAA));
> small, but I think I prefer the old name, so it doesn't look like another
> "add.." function, all of which add something to the opcode stream.
> 

Technically speaking this function is adding something to the opcode stream, but
I get what you mean. The old name confused me when I first read this code, so I
would like to give it a clearer name.  In the latest patch set, I named it
recordRestoreOffsetPlaceholder.  What do you think?

[reed1, 2012-07-09 12:46:01]

lgtm

[junov1, 2012-07-16 19:03:38]

Fixed with r4472