Thread picture version through to SkReadBuffer.

Thread picture version through to SkReadBuffer.

This will let code outside SkPicture* fork its read code based on the picture version.

BUG=skia:

Committed: http://code.google.com/p/skia/source/detail?r=13984

[mtklein, 2014-03-27 20:51:43]

Mike/Robert: review?
Greg/Alexis: see if this helps you?

[reed1, 2014-03-27 20:58:13]

passing the info to the construct feels really good.

https://codereview.chromium.org/214413008/diff/1/include/core/SkReadBuffer.h
File include/core/SkReadBuffer.h (right):

https://codereview.chromium.org/214413008/diff/1/include/core/SkReadBuffer.h#...
include/core/SkReadBuffer.h:43: 
/** Return the current picture version, or 0 if this has not been set */

https://codereview.chromium.org/214413008/diff/1/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

https://codereview.chromium.org/214413008/diff/1/src/core/SkPicture.cpp#newco...
src/core/SkPicture.cpp:426: SkPictInfo header;
not strictly part of this change, but it would be much easier to handle all of
the parallelism in this file if we always called this info, instead of info or
header.

https://codereview.chromium.org/214413008/diff/1/src/core/SkPicturePlayback.h
File src/core/SkPicturePlayback.h (right):

https://codereview.chromium.org/214413008/diff/1/src/core/SkPicturePlayback.h...
src/core/SkPicturePlayback.h:274: SkPictInfo fInfo;
can this guy be const?

[reed1, 2014-03-27 20:58:41]

lgtm w/ comments/questions

[mtklein, 2014-03-27 21:02:48]

https://codereview.chromium.org/214413008/diff/1/include/core/SkReadBuffer.h
File include/core/SkReadBuffer.h (right):

https://codereview.chromium.org/214413008/diff/1/include/core/SkReadBuffer.h#...
include/core/SkReadBuffer.h:43: 
On 2014/03/27 20:58:14, reed1 wrote:
> /** Return the current picture version, or 0 if this has not been set */

Done.

https://codereview.chromium.org/214413008/diff/1/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

https://codereview.chromium.org/214413008/diff/1/src/core/SkPicture.cpp#newco...
src/core/SkPicture.cpp:426: SkPictInfo header;
On 2014/03/27 20:58:14, reed1 wrote:
> not strictly part of this change, but it would be much easier to handle all of
> the parallelism in this file if we always called this info, instead of info or
> header.

Done.

https://codereview.chromium.org/214413008/diff/1/src/core/SkPicturePlayback.h
File src/core/SkPicturePlayback.h (right):

https://codereview.chromium.org/214413008/diff/1/src/core/SkPicturePlayback.h...
src/core/SkPicturePlayback.h:274: SkPictInfo fInfo;
On 2014/03/27 20:58:14, reed1 wrote:
> can this guy be const?

Yep, done.

[sugoi, 2014-03-28 14:35:07]

On 2014/03/27 20:51:43, mtklein wrote:
> Greg/Alexis: see if this helps you?

I think being able to call SkReadBuffer::pictureVersion() is good enough for me.

I only have 1 question (nit): would there be a way to make
SkReadBuffer::setPictureVersion() a bit less accessible? Only SkPicturePlayback
will be allowed to use it, I think. I find it a little odd/unsafe that every
function that reads also has the ability to change the picture version of the
reader. Maybe we could use friend classes to limit this, or maybe we could have
a debug assert that we're not setting the picture version twice by making sure
the the version is at its default value before setting it, if that makes sense?

[mtklein, 2014-03-28 14:38:36]

On 2014/03/28 14:35:07, sugoi wrote:
> On 2014/03/27 20:51:43, mtklein wrote:
> > Greg/Alexis: see if this helps you?
> 
> I think being able to call SkReadBuffer::pictureVersion() is good enough for
me.
> 
> I only have 1 question (nit): would there be a way to make
> SkReadBuffer::setPictureVersion() a bit less accessible? Only
SkPicturePlayback
> will be allowed to use it, I think. I find it a little odd/unsafe that every
> function that reads also has the ability to change the picture version of the
> reader. Maybe we could use friend classes to limit this, or maybe we could
have
> a debug assert that we're not setting the picture version twice by making sure
> the the version is at its default value before setting it, if that makes
sense?

Ah, right, forgot we're necessarily working with a non-const SkReadBuffer&. 
I'll make it set-once.

[mtklein, 2014-03-28 15:05:08]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-03-28 15:05:12]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/214413008/40001

[mtklein, 2014-03-28 16:47:32]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2014-03-28 16:47:33]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/mtklein@chromium.org/214413008/60001

[commit-bot: I haz the power, 2014-03-28 18:05:50]

Change committed as 13984