Add R-Tree record flag to SkPicture, plus some cleanup/fixes in associated classes.

Add R-Tree record flag to SkPicture, plus some cleanup/fixes in associated classes.
Committed: https://code.google.com/p/skia/source/detail?r=5537

[reed1, 2012-09-12 14:38:11]

Perhaps a more general name for the flag?

- OptimizeForClippedPlayback?
- AddBBoxInfo?
- ...

[rileya, 2012-09-12 20:46:42]

On 2012/09/12 14:38:11, reed1 wrote:
> Perhaps a more general name for the flag?
> 
> - OptimizeForClippedPlayback?
> - AddBBoxInfo?
> - ...

I like OptimizeForClippedPlayback, makes it more evident what it's meant for,
changed to that.

[rileya, 2012-09-13 18:53:58]

I increased the padding on text bounds (single character draws weren't getting
large enough boxes). Apart from the tiny differences in lcd text (appears to be
benign), and the re-record losing draws problem (which is a general picture
record problem), we seem to get identical results in the pages I've tested.

[bsalomon, 2012-09-13 21:17:16]

LGTM

http://codereview.appspot.com/6506103/diff/5003/include/core/SkPicture.h
File include/core/SkPicture.h (right):

http://codereview.appspot.com/6506103/diff/5003/include/core/SkPicture.h#newc...
include/core/SkPicture.h:84: after a frame or two (and complex pages that induce
the worst record
let's say pictures rather than pages.

http://codereview.appspot.com/6506103/diff/5003/src/core/SkPictureStateTree.cpp
File src/core/SkPictureStateTree.cpp (right):

http://codereview.appspot.com/6506103/diff/5003/src/core/SkPictureStateTree.c...
src/core/SkPictureStateTree.cpp:94: SkASSERT(isValid());
this->

[robertphillips, 2012-09-14 11:55:32]

LGTM + nits

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPicture.cpp#newcod...
src/core/SkPicture.cpp:191: SkRTree* tree = SkRTree::Create(6, 11, aspectRatio);
6? 11? magic numbers?

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPicturePlayback.cpp
File src/core/SkPicturePlayback.cpp (right):

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPicturePlayback.cp...
src/core/SkPicturePlayback.cpp:588: if (fStateTree && fBoundingHierarchy) {
NULL != 's

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPicturePlayback.cp...
src/core/SkPicturePlayback.cpp:594: if (results.count() == 0) { return; }
if ... {
   return;
}

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPicturePlayback.cp...
src/core/SkPicturePlayback.cpp:607: if (off == SK_MaxU32) { return; }
if ... {
   return;
}

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPictureStateTree.h
File src/core/SkPictureStateTree.h (right):

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPictureStateTree.h...
src/core/SkPictureStateTree.h:73: Iterator() : fValid(false) { }
isValid() const?

[rileya, 2012-09-14 13:00:07]

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

http://codereview.appspot.com/6506103/diff/7005/src/core/SkPicture.cpp#newcod...
src/core/SkPicture.cpp:191: SkRTree* tree = SkRTree::Create(6, 11, aspectRatio);
On 2012/09/14 11:55:32, robertphillips wrote:
> 6? 11? magic numbers?

They're the min/max child counts that seemed to perform best, would it make more
sense to make these constants on SkPicture, or to just to explain them here?

[rileya, 2012-09-14 15:53:25]

Landed fixes for the nits in r5547.