Have SkPicture::willPlayBackBitmaps() count SkImages too.

Have SkPicture::willPlayBackBitmaps() count SkImages too.

New unit test fails at head but passes with this patch.

BUG=skia:4225
Committed: https://skia.googlesource.com/skia/+/a16af21b17885c517a587482e9062efb99c19306

[mtklein_C, 2015-08-26 14:28:19]

The CQ bit was checked by mtklein@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-26 14:28:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1319723002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1319723002/1

[mtklein_C, 2015-08-26 14:35:59]

mtklein@chromium.org changed reviewers:
+ fmalita@chromium.org

[commit-bot: I haz the power, 2015-08-26 14:37:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-26 14:37:45]

Dry run: This issue passed the CQ dry run.

[f(malita), 2015-08-26 14:56:01]

lgtm

https://codereview.chromium.org/1319723002/diff/1/tests/PictureTest.cpp
File tests/PictureTest.cpp (right):

https://codereview.chromium.org/1319723002/diff/1/tests/PictureTest.cpp#newco...
tests/PictureTest.cpp:54: SkAutoTUnref<SkImage>
image(SkImage::NewFromBitmap(SkBitmap()));
I was kinda surprised that this works: we never instantiate zero-sized images =>
we're drawing a null image below.

I guess the member detector doesn't care, and in practice I think we're
discarding null images earlier.  So likely not worth filtering.

[mtklein, 2015-08-26 15:13:56]

mtklein@google.com changed reviewers:
+ mtklein@google.com

[mtklein, 2015-08-26 15:13:57]

https://codereview.chromium.org/1319723002/diff/1/tests/PictureTest.cpp
File tests/PictureTest.cpp (right):

https://codereview.chromium.org/1319723002/diff/1/tests/PictureTest.cpp#newco...
tests/PictureTest.cpp:54: SkAutoTUnref<SkImage>
image(SkImage::NewFromBitmap(SkBitmap()));
On 2015/08/26 14:56:00, f(malita) wrote:
> I was kinda surprised that this works: we never instantiate zero-sized images
=>
> we're drawing a null image below.
> 
> I guess the member detector doesn't care, and in practice I think we're
> discarding null images earlier.  So likely not worth filtering.

Yeah, I was just sort of looking for the simplest thing that worked.  I also
thought this might be _too_ simple, but hey, it works.

You're right.  The real key is that we record a DrawImage struct into the
SkRecord.

[mtklein, 2015-08-26 15:14:06]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-08-26 15:14:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1319723002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1319723002/1

[commit-bot: I haz the power, 2015-08-26 15:14:55]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/a16af21b17885c517a587482e9062efb99c19306

[vmpstr, 2015-08-26 17:36:18]

On 2015/08/26 15:14:55, commit-bot: I haz the power wrote:
> Committed patchset #1 (id:1) as
> https://skia.googlesource.com/skia/+/a16af21b17885c517a587482e9062efb99c19306

I was wondering about willPlaybackBitmaps in general and maybe you peeps know
this off the top of your head: Is this always constant time operation or does it
sometimes do some work? In Chromium, we use this pretty aggressively as a "cheap
early out" check, but if it's not always cheap then we should probably reduce
some usages. (sorry about hijacking a review thread :P)

[mtklein, 2015-08-26 17:49:57]

On 2015/08/26 17:36:18, vmpstr wrote:
> On 2015/08/26 15:14:55, commit-bot: I haz the power wrote:
> > Committed patchset #1 (id:1) as
> >
https://skia.googlesource.com/skia/+/a16af21b17885c517a587482e9062efb99c19306
> 
> I was wondering about willPlaybackBitmaps in general and maybe you peeps know
> this off the top of your head: Is this always constant time operation or does
it
> sometimes do some work? In Chromium, we use this pretty aggressively as a
"cheap
> early out" check, but if it's not always cheap then we should probably reduce
> some usages. (sorry about hijacking a review thread :P)

The first time it's called, it's O(N) for N calls made to the original recording
canvas that generated the picture.  It's cached from there on, so it becomes
O(1) if you call it many times per picture.

Right now all of hasText(), willPlayBackBitmaps() and numSlowPaths() follow this
same lazy pattern.  In fact, they run together off the same lazy bit, so the
first time you call one of them, you pre-calculate the other two.

hasText() and willPlayBackBitmaps() may scale sub-O(N) in practice, in that they
can bail out early when they know the answer is 'true'.  numSlowPaths() always
triggers an operation that runs over the whole picture.

These operations are probably faster than can be done with an external SkCanvas
subclass, but they scale pretty much the same way.