Add NOOP "command" to picture format

This is the next step in being able to arbitrarily remove random commands from the skp byte stream in PictureRecord.

This CL relies on the addition of the chunk size so looks a bit weird without it. In particular, the "size" in SkPicturePlayback will be filled in by the read_op_and_size method.

The "noClips" method will not be checked in but I wanted some example of how the NOOP command will be used and why the poke8 method is necessary.

[reed1, 2013-02-15 09:52:48]

Not sure why we need to land this before the size-in-op CL. Or, perhaps even
here we could embedded the size just in this op.

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

https://codereview.appspot.com/7343044/diff/1/src/core/SkPicturePlayback.cpp#...
src/core/SkPicturePlayback.cpp:696: #endif
Hard to understand why we have 'size' here...

https://codereview.appspot.com/7343044/diff/1/src/core/SkPicturePlayback.cpp#...
src/core/SkPicturePlayback.cpp:699: if (NOOP == type) {
"Not supposed to exist"?

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

https://codereview.appspot.com/7343044/diff/1/src/core/SkPictureRecord.cpp#ne...
src/core/SkPictureRecord.cpp:212: #if 0
// What this function does

https://codereview.appspot.com/7343044/diff/1/src/core/SkPictureRecord.cpp#ne...
src/core/SkPictureRecord.cpp:234: if (CLIP_RECT == op || CLIP_RRECT == op) {
Very endian specific it seems.

Can we instead:

uint32_t* ptr = writer->peek(offset);
*ptr = (*ptr & ~0xFF) | NOOP;

[robertphillips, 2013-02-15 14:22:00]

I never intended to deliver this without the chunk patch but wanted to get the
review process started.

With the removal of the poke8 calls there isn't much left.

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

https://codereview.appspot.com/7343044/diff/1/src/core/SkPicturePlayback.cpp#...
src/core/SkPicturePlayback.cpp:696: #endif
On 2013/02/15 09:52:48, reed1 wrote:
> Hard to understand why we have 'size' here...

Done.

https://codereview.appspot.com/7343044/diff/1/src/core/SkPicturePlayback.cpp#...
src/core/SkPicturePlayback.cpp:699: if (NOOP == type) {
On 2013/02/15 09:52:48, reed1 wrote:
> "Not supposed to exist"?

Done.

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

https://codereview.appspot.com/7343044/diff/1/src/core/SkPictureRecord.cpp#ne...
src/core/SkPictureRecord.cpp:212: #if 0
This file will not be delivered as part of this CL - it is just to show how the
NOOP will be used.

https://codereview.appspot.com/7343044/diff/1/src/core/SkPictureRecord.cpp#ne...
src/core/SkPictureRecord.cpp:234: if (CLIP_RECT == op || CLIP_RRECT == op) {
Done - that's better.

https://codereview.appspot.com/7343044/diff/1/src/core/SkPictureRecord.cpp#ne...
src/core/SkPictureRecord.cpp:234: if (CLIP_RECT == op || CLIP_RRECT == op) {
Done - that's better.

[reed1, 2013-02-15 15:50:32]

Where is the dox for read_op_and_size? Does it document what 'size' means the
number of bytes *after* the op-code word? i.e. for a no argument op, the size
would be zero, not 4.

lgtm

[robertphillips, 2013-02-15 17:19:46]

Upgraded read_op_and_size's comment

committed as r7752