MultiPictureDraw is taskgroup aware.

MultiPictureDraw is taskgroup aware.
SampleApp is multipicturedraw aware.

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/89889b69391a730f0ba2a1efb549864b7762263f

[reed1, 2014-10-28 17:43:47]

cloned from https://codereview.chromium.org/641943005/

[reed1, 2014-10-28 20:35:30]

reed@google.com changed reviewers:
+ bsalomon@google.com, mtklein@google.com, robertphillips@google.com

[reed1, 2014-10-28 20:35:30]

MPD now with two buckets.

Still want to clean-up the calling code in sampleapp

[robertphillips, 2014-10-28 20:44:58]

https://codereview.chromium.org/684923002/diff/20001/src/core/SkMultiPictureD...
File src/core/SkMultiPictureDraw.cpp (right):

https://codereview.chromium.org/684923002/diff/20001/src/core/SkMultiPictureD...
src/core/SkMultiPictureDraw.cpp:49: void SkMultiPictureDraw::reset() {
DrawData::Reset(fGPUDrawData); ?

https://codereview.chromium.org/684923002/diff/20001/src/core/SkMultiPictureD...
src/core/SkMultiPictureDraw.cpp:110: if (0 == count) {
add:

group.wait();
this->reset();

to fulfill contract?

[reed1, 2014-10-28 20:45:54]

On 2014/10/28 20:44:58, robertphillips wrote:
>
https://codereview.chromium.org/684923002/diff/20001/src/core/SkMultiPictureD...
> File src/core/SkMultiPictureDraw.cpp (right):
> 
>
https://codereview.chromium.org/684923002/diff/20001/src/core/SkMultiPictureD...
> src/core/SkMultiPictureDraw.cpp:49: void SkMultiPictureDraw::reset() {
> DrawData::Reset(fGPUDrawData); ?
> 
>
https://codereview.chromium.org/684923002/diff/20001/src/core/SkMultiPictureD...
> src/core/SkMultiPictureDraw.cpp:110: if (0 == count) {
> add:
> 
> group.wait();
> this->reset();
> 
> to fulfill contract?

Doh! Yes indeed. Will fix.

[reed1, 2014-10-29 13:29:52]

ptal

[reed1, 2014-10-29 13:29:58]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-10-29 13:30:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/684923002/40001

[commit-bot: I haz the power, 2014-10-29 13:30:19]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-10-29 19:30 UTC

[commit-bot: I haz the power, 2014-10-29 13:36:48]

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

[commit-bot: I haz the power, 2014-10-29 13:36:50]

Try jobs failed on following builders:
  Build-Win-VS2013-x86-Debug-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Win-VS2013-x86...)

[reed1, 2014-10-29 13:48:27]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-10-29 13:49:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/684923002/60001

[commit-bot: I haz the power, 2014-10-29 13:49:34]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2014-10-29 19:49 UTC

[commit-bot: I haz the power, 2014-10-29 13:54:15]

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

[commit-bot: I haz the power, 2014-10-29 13:54:16]

Try jobs failed on following builders:
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Release-Shared-Trybot on client.skia
(http://build.chromium.org/p/client.skia/builders/Test-Ubuntu13.10-GCE-NoGPU-x...)

[bsalomon, 2014-10-29 14:01:51]

Def more Rob's area than mine, but lgtm

[mtklein, 2014-10-29 14:29:04]

https://codereview.chromium.org/684923002/diff/60001/include/core/SkMultiPict...
File include/core/SkMultiPictureDraw.h (right):

https://codereview.chromium.org/684923002/diff/60001/include/core/SkMultiPict...
include/core/SkMultiPictureDraw.h:58: struct DrawData {
Might simplify things to just make DrawData implement SkRunnable rather than
adding PictureDrawRunnable?

https://codereview.chromium.org/684923002/diff/60001/src/core/SkMultiPictureD...
File src/core/SkMultiPictureDraw.cpp (right):

https://codereview.chromium.org/684923002/diff/60001/src/core/SkMultiPictureD...
src/core/SkMultiPictureDraw.cpp:103: // objects until after we're finished the
tasks (which have pointers to the data).
If we're worried about scope timing, can't hurt to scope the SkTaskGroup in a
block, or make an explicit call to wait().

https://codereview.chromium.org/684923002/diff/60001/src/core/SkMultiPictureD...
src/core/SkMultiPictureDraw.cpp:108: if (fThreadSafeDrawData.count() > 0) {
My instinct is to drop these extra checks (this and 0 == count below) until a
profile wants us to put them back.  The code should all work fine without them.

[reed1, 2014-10-29 17:05:13]

ptal

https://codereview.chromium.org/684923002/diff/60001/include/core/SkMultiPict...
File include/core/SkMultiPictureDraw.h (right):

https://codereview.chromium.org/684923002/diff/60001/include/core/SkMultiPict...
include/core/SkMultiPictureDraw.h:58: struct DrawData {
On 2014/10/29 14:29:03, mtklein wrote:
> Might simplify things to just make DrawData implement SkRunnable rather than
> adding PictureDrawRunnable?

using function-ptrs instead

https://codereview.chromium.org/684923002/diff/60001/src/core/SkMultiPictureD...
File src/core/SkMultiPictureDraw.cpp (right):

https://codereview.chromium.org/684923002/diff/60001/src/core/SkMultiPictureD...
src/core/SkMultiPictureDraw.cpp:103: // objects until after we're finished the
tasks (which have pointers to the data).
On 2014/10/29 14:29:03, mtklein wrote:
> If we're worried about scope timing, can't hurt to scope the SkTaskGroup in a
> block, or make an explicit call to wait().

deliberately want to not force the wait until the gpu work was done.

https://codereview.chromium.org/684923002/diff/60001/src/core/SkMultiPictureD...
src/core/SkMultiPictureDraw.cpp:108: if (fThreadSafeDrawData.count() > 0) {
On 2014/10/29 14:29:03, mtklein wrote:
> My instinct is to drop these extra checks (this and 0 == count below) until a
> profile wants us to put them back.  The code should all work fine without
them.

Done.

[reed1, 2014-10-29 17:05:18]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-10-29 17:05:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/684923002/100001

[commit-bot: I haz the power, 2014-10-29 17:07:25]

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

[commit-bot: I haz the power, 2014-10-29 17:07:27]

Try jobs failed on following builders:
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Debug-Trybot on client.skia
(http://build.chromium.org/p/client.skia/builders/Test-Ubuntu13.10-GCE-NoGPU-x...)
  Test-Ubuntu13.10-GCE-NoGPU-x86_64-Release-Shared-Trybot on client.skia
(http://build.chromium.org/p/client.skia/builders/Test-Ubuntu13.10-GCE-NoGPU-x...)
  Build-Mac10.8-Clang-x86_64-Release-Trybot on client.skia.compile
(http://build.chromium.org/p/client.skia.compile/builders/Build-Mac10.8-Clang-...)

[reed1, 2014-10-29 17:18:51]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-10-29 17:19:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/684923002/120001

[reed1, 2014-10-29 17:26:41]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-10-29 17:26:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/684923002/140001

[mtklein, 2014-10-29 17:28:03]

lgtm

[commit-bot: I haz the power, 2014-10-29 19:36:51]

Committed patchset #8 (id:140001) as 89889b69391a730f0ba2a1efb549864b7762263f