Change SkFlatData to have a sentinel value, allowing the Compare function to

Change SkFlatData to have a sentinel value, allowing the Compare function to
not need a loop-end-test.
Committed: https://code.google.com/p/skia/source/detail?r=4517

[junov1, 2012-07-10 17:32:18]

https://codereview.appspot.com/6355086/diff/4001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.appspot.com/6355086/diff/4001/src/core/SkPictureFlat.h#new...
src/core/SkPictureFlat.h:171: SkASSERT(b->dataStop() == b_ptr);
Is this condition valid? You can have objects of the same type that have
different sizes once flattened.  For example look at SkPaint::flatten: there are
fields that are written conditionally.  Also, in the case of FlatData objects of
different sizes the sentinel may not be reliable.

https://codereview.appspot.com/6355086/diff/4001/src/core/SkPictureFlat.h#new...
src/core/SkPictureFlat.h:194: return sizeof(SkFlatData) + fFlatSize;
fFlatSize does not include the sentinel.

[reed1, 2012-07-10 17:46:17]

patch #4 should help document the questions raised.

https://codereview.appspot.com/6355086/diff/4001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.appspot.com/6355086/diff/4001/src/core/SkPictureFlat.h#new...
src/core/SkPictureFlat.h:171: SkASSERT(b->dataStop() == b_ptr);
On 2012/07/10 17:32:18, junov1 wrote:
> Is this condition valid? You can have objects of the same type that have
> different sizes once flattened.  For example look at SkPaint::flatten: there
are
> fields that are written conditionally.  Also, in the case of FlatData objects
of
> different sizes the sentinel may not be reliable.

We compare the fFlatSize *before* we get to the sentinel. Therefore if we hit
the sentinel of either, we *must* have have equal lengths.

https://codereview.appspot.com/6355086/diff/4001/src/core/SkPictureFlat.h#new...
src/core/SkPictureFlat.h:194: return sizeof(SkFlatData) + fFlatSize;
On 2012/07/10 17:32:18, junov1 wrote:
> fFlatSize does not include the sentinel.

Yes, but I thought the caller just wanted the "data", not the allocation size. I
will go hunt the caller(s) down and confirm.

[junov1, 2012-07-10 17:57:57]

https://codereview.appspot.com/6355086/diff/6002/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.appspot.com/6355086/diff/6002/src/core/SkPictureFlat.h#new...
src/core/SkPictureFlat.h:176: const uint32_t* b_ptr = b->dataToCompare() - 1;
Why the -1?

[reed1, 2012-07-10 18:23:23]

On 2012/07/10 17:57:57, junov1 wrote:
> https://codereview.appspot.com/6355086/diff/6002/src/core/SkPictureFlat.h
> File src/core/SkPictureFlat.h (right):
> 
>
https://codereview.appspot.com/6355086/diff/6002/src/core/SkPictureFlat.h#new...
> src/core/SkPictureFlat.h:176: const uint32_t* b_ptr = b->dataToCompare() - 1;
> Why the -1?

documented in patch #6

[junov1, 2012-07-10 18:47:59]

lgtm