optimize for paints that don't require deep copy during clone

optimize for paints that don't require deep copy during clone
Committed: https://code.google.com/p/skia/source/detail?r=6739

[reed1, 2012-12-10 16:20:35]

Not the most agressive change, but it does speed up some of our pages.

Down the road, lets re-consider how hard it will be to either
- have an optimized "clone" method on SkFlattenable
- force all our effects to be immutable

[DerekS, 2012-12-10 16:32:05]

lgtm with a nit

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

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:189: #ifdef SHOW_SHALLOW_COUNTER
do we really need to check in this debugging code?

[Leon, 2012-12-10 16:34:43]

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

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:149: static bool needDeepCopy(const SkPaint&
paint) {
need_deep_copy

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:190: static int gShalowCounter, gCounter;
gShallowCounter (two 'L's).

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:205: deepCopyInfo->paintData[i] =
SkFlatData::Create(&deepCopyInfo->controller,
Why don't we set an SkBitmapHeap on the controller? From the assert below about
fBitmapHeap's count, it looks like the intent was to use fBitmapHeap. Should it
already have the bitmaps that are needed?

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:235: if (deepCopyInfo->paintData[i]) {
if (NULL == deepCopyInfo->paintData[i]) {

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:239:
deepCopyInfo->controller.getTypefacePlayback());
100 chars

[reed1, 2012-12-10 16:41:39]

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

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:149: static bool needDeepCopy(const SkPaint&
paint) {
On 2012/12/10 16:34:43, scroggo-work wrote:
> need_deep_copy

Done.

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:189: #ifdef SHOW_SHALLOW_COUNTER
On 2012/12/10 16:32:06, DerekS wrote:
> do we really need to check in this debugging code?

Done.

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:205: deepCopyInfo->paintData[i] =
SkFlatData::Create(&deepCopyInfo->controller,
On 2012/12/10 16:34:43, scroggo-work wrote:
> Why don't we set an SkBitmapHeap on the controller? From the assert below
about
> fBitmapHeap's count, it looks like the intent was to use fBitmapHeap. Should
it
> already have the bitmaps that are needed?

Separate CL?

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:235: if (deepCopyInfo->paintData[i]) {
On 2012/12/10 16:34:43, scroggo-work wrote:
> if (NULL == deepCopyInfo->paintData[i]) {

Since we need to know both the null and not-null cases, I think this matches
existing skia pattern for checking for non-null, yes?

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:239:
deepCopyInfo->controller.getTypefacePlayback());
On 2012/12/10 16:34:43, scroggo-work wrote:
> 100 chars

Done.

[Leon, 2012-12-10 17:32:58]

lgtm, with one spelling error

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

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:205: deepCopyInfo->paintData[i] =
SkFlatData::Create(&deepCopyInfo->controller,
On 2012/12/10 16:41:39, reed1 wrote:
> On 2012/12/10 16:34:43, scroggo-work wrote:
> > Why don't we set an SkBitmapHeap on the controller? From the assert below
> about
> > fBitmapHeap's count, it looks like the intent was to use fBitmapHeap. Should
> it
> > already have the bitmaps that are needed?
> 
> Separate CL?

https://codereview.appspot.com/6903063/

https://codereview.appspot.com/6903062/diff/2001/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:235: if (deepCopyInfo->paintData[i]) {
On 2012/12/10 16:41:39, reed1 wrote:
> On 2012/12/10 16:34:43, scroggo-work wrote:
> > if (NULL == deepCopyInfo->paintData[i]) {
> 
> Since we need to know both the null and not-null cases, I think this matches
> existing skia pattern for checking for non-null, yes?

Sounds fine to me.

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

https://codereview.appspot.com/6903062/diff/5003/src/core/SkPicturePlayback.c...
src/core/SkPicturePlayback.cpp:225: // needs_deep_copy was falls, so just need
to assign
false*