Sketch splitting SkPicture into an interface and SkBigPicture.

Sketch splitting SkPicture into an interface and SkBigPicture.

Adds small pictures for drawRect(), drawTextBlob(), and drawPath().
These cover about 89% of draw calls from Blink SKPs,
and about 25% of draw calls from our GMs.

SkPicture handles:
  - serialization and deserialization
  - unique IDs

Everything else is left to the subclasses:
  - playback(), cullRect()
  - hasBitmap(), hasText(), suitableForGPU(), etc.
  - LayerInfo / AccelData if applicable.

The time to record a 1-op picture improves a good chunk
(2 mallocs to 1), and the time to record a 0-op picture 
greatly improves (2 mallocs to none):

    picture_overhead_draw:   450ns -> 350ns
    picture_overhead_nodraw: 300ns -> 90ns

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/c92c129ff85b05a714bd1bf921c02d5e14651f8b

Latest blink_linux_rel:

http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/61248

Committed: https://skia.googlesource.com/skia/+/15877b6eae33a9282458bdb904a6d00440eca0ec

http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/62015

Committed: https://skia.googlesource.com/skia/+/9db912c2ac2ab53bc24f2d50a3e5a80162051dcc

[mtklein, 2015-04-29 20:11:48]

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

[mtklein, 2015-04-29 20:11:48]

Just thought I'd get this stewing in your minds.

The thing to imagine coming next (really, hacking at it in parallel) is one or
more `class SkTinyPicture : public SkPicture` that specialize for a small number
of draw ops (e.g. 1,2, maybe 3?).  These tiny pictures wouldn't go through most
of the usual stuff for pictures: little to no optimization, no BBH, no GPU
analysis, etc.  They'd be meant to be the 80%+ case for the first phase of
slimming paint where we record a zillion little pictures, with SkBigPicture
still handling the big linearized picture at the end.  They'd be small and
faster to record (one malloc seems plausible so far).

There's a lot here that's "Well, I can't just in good conscience copy _that_ to
a new file, so let me fix it up a bit," and a good bit of "I have no idea how
Robert's layer hoisting works."  This CL has that all stewed together, but I'm
more than happy to start reverting bits or pulling them out to land on their own
first.

All the tests pass, things all draw the same, and SKP serialization still works.

[mtklein, 2015-04-29 20:11:57]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-04-29 20:12:23]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1112523006/80001

[mtklein, 2015-04-29 20:14:59]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-04-29 20:15:07]

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

[mtklein, 2015-04-29 20:16:48]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-04-29 20:17:12]

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

[reed1, 2015-04-29 20:21:53]

1. I think this is an exciting idea/direction
2. I wonder if we should try to implement "1" tiny picture, just so we can see
how it will fit into the PictureRecorder flow (even if its the empty-picture
specialization).

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture.h
File include/core/SkPicture.h (right):

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture...
include/core/SkPicture.h:78: struct SK_API AbortCallback {
this change *may* generate warnings for existing clients/subclasses. why from
class to struct?

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture...
include/core/SkPicture.h:90: virtual void playback(SkCanvas*, AbortCallback* =
NULL) const = 0;
general comment (not important in the early stages per-se) -- wrapping the
onVirtuals with public nonVirtuals is a zero-cost trick, but allows us in the
future to add common debug checks/stats/caching w/o needing to rev the
subclasses.

e.g.

void playback(...) {
    this->onPlayback(...);
}

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture...
include/core/SkPicture.h:151: virtual const SkBigPicture* asSkBigPicture() const
{ return NULL; }
does this guy need to be public?

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkTemplat...
File include/core/SkTemplates.h (right):

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkTemplat...
include/core/SkTemplates.h:135: operator T*() const { return fObj; }
happy to land this separately

https://codereview.chromium.org/1112523006/diff/100001/src/core/SkPictureCont...
File src/core/SkPictureContentInfo.h (right):

https://codereview.chromium.org/1112523006/diff/100001/src/core/SkPictureCont...
src/core/SkPictureContentInfo.h:11: #include "SkTDArray.h"
happy to land this separately

[commit-bot: I haz the power, 2015-04-29 20:21:58]

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

[commit-bot: I haz the power, 2015-04-29 20:21:59]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-04-29 20:46:22]

Yep, agree, I'm shooting for tomorrow morning to have a small picture class
plumbed into SkPictureRecorder.  I would like to land this with at least
specializations for 1x DrawRect and for 1x DrawTextBlob.

I've been hacking back and forth about how to generalize to 1x DrawWhatever, but
I think that'll probably want to push off for another CL after some profiling.

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture.h
File include/core/SkPicture.h (right):

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture...
include/core/SkPicture.h:78: struct SK_API AbortCallback {
On 2015/04/29 20:21:53, reed1 wrote:
> this change *may* generate warnings for existing clients/subclasses. why from
> class to struct?

Just seemed struct-y, given its all-pubic nature.

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture...
include/core/SkPicture.h:90: virtual void playback(SkCanvas*, AbortCallback* =
NULL) const = 0;
On 2015/04/29 20:21:53, reed1 wrote:
> general comment (not important in the early stages per-se) -- wrapping the
> onVirtuals with public nonVirtuals is a zero-cost trick, but allows us in the
> future to add common debug checks/stats/caching w/o needing to rev the
> subclasses.
> 
> e.g.
> 
> void playback(...) {
>     this->onPlayback(...);
> }

Right.  I figured it'd not be very helpful here, given that all subclasses must
live in the Skia codebase, so can be changed in the same CL.

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture...
include/core/SkPicture.h:151: virtual const SkBigPicture* asSkBigPicture() const
{ return NULL; }
On 2015/04/29 20:21:53, reed1 wrote:
> does this guy need to be public?

No, we can friend a bunch of Gr classes instead.  Was easier refactoring things
public, but I'll hide it.

[mtklein, 2015-04-30 16:17:03]

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture.h
File include/core/SkPicture.h (right):

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture...
include/core/SkPicture.h:78: struct SK_API AbortCallback {
On 2015/04/29 20:46:22, mtklein wrote:
> On 2015/04/29 20:21:53, reed1 wrote:
> > this change *may* generate warnings for existing clients/subclasses. why
from
> > class to struct?
> 
> Just seemed struct-y, given its all-pubic nature.

Undone.

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkPicture...
include/core/SkPicture.h:151: virtual const SkBigPicture* asSkBigPicture() const
{ return NULL; }
On 2015/04/29 20:46:21, mtklein wrote:
> On 2015/04/29 20:21:53, reed1 wrote:
> > does this guy need to be public?
> 
> No, we can friend a bunch of Gr classes instead.  Was easier refactoring
things
> public, but I'll hide it.

I take it back.  It's really hideous to try to make this private.

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkTemplat...
File include/core/SkTemplates.h (right):

https://codereview.chromium.org/1112523006/diff/100001/include/core/SkTemplat...
include/core/SkTemplates.h:135: operator T*() const { return fObj; }
On 2015/04/29 20:21:53, reed1 wrote:
> happy to land this separately

Done.

https://codereview.chromium.org/1112523006/diff/100001/src/core/SkPictureCont...
File src/core/SkPictureContentInfo.h (right):

https://codereview.chromium.org/1112523006/diff/100001/src/core/SkPictureCont...
src/core/SkPictureContentInfo.h:11: #include "SkTDArray.h"
On 2015/04/29 20:21:53, reed1 wrote:
> happy to land this separately

Done.

[mtklein, 2015-05-01 22:23:10]

I think it'd look something like this.

[reed1, 2015-05-04 13:26:34]

it may be time for a src/picture/... directory to group these many files (as we
did for gradients... effects/gradients/...)

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorde...
File src/core/SkMiniRecorder.cpp (right):

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorde...
src/core/SkMiniRecorder.cpp:99: }
return NULL; ?

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorder.h
File src/core/SkMiniRecorder.h (right):

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorde...
src/core/SkMiniRecorder.h:38: void flushToCanvas(SkCanvas*);
detachToCanvas?
detachAndPlayback?
detachAndDraw?

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorde...
src/core/SkMiniRecorder.h:44: char  fBuffer[1024];  // TODO: size/align this
carefully
do we already have an Aligned buffer helper template?

[mtklein, 2015-05-04 15:52:45]

Adding another src/ directory is a pain in the butt to roll.

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorde...
File src/core/SkMiniRecorder.cpp (right):

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorde...
src/core/SkMiniRecorder.cpp:99: }
On 2015/05/04 13:26:34, reed1 wrote:
> return NULL; ?

Done.

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorder.h
File src/core/SkMiniRecorder.h (right):

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorde...
src/core/SkMiniRecorder.h:38: void flushToCanvas(SkCanvas*);
On 2015/05/04 13:26:34, reed1 wrote:
> detachToCanvas?
> detachAndPlayback?
> detachAndDraw?

Removed the whole method for now, given that it's called once and not really
specialized to be efficient yet.

https://codereview.chromium.org/1112523006/diff/300001/src/core/SkMiniRecorde...
src/core/SkMiniRecorder.h:44: char  fBuffer[1024];  // TODO: size/align this
carefully
On 2015/05/04 13:26:34, reed1 wrote:
> do we already have an Aligned buffer helper template?

Yep, done.

[mtklein, 2015-05-06 17:56:29]

Have another look guys?  out/Debug/dm --config sp-8888 now shows no diffs
against head.

[reed1, 2015-05-06 18:13:49]

lgtm

[mtklein, 2015-05-07 18:58:08]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-05-07 18:58:38]

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

[commit-bot: I haz the power, 2015-05-07 18:58:40]

COMMIT=false detected. CQ is abandoning the patch.

[mtklein, 2015-05-07 19:09:19]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-05-07 19:09:51]

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

[commit-bot: I haz the power, 2015-05-07 19:11:52]

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

[commit-bot: I haz the power, 2015-05-07 19:11:54]

Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)

[jbroman, 2015-05-07 19:38:26]

https://codereview.chromium.org/1112523006/diff/360001/src/core/SkMiniRecorder.h
File src/core/SkMiniRecorder.h (right):

https://codereview.chromium.org/1112523006/diff/360001/src/core/SkMiniRecorde...
src/core/SkMiniRecorder.h:34: SkAlignedSStorage<sizeof(SkRecords::DrawPath)>
fBuffer;
Lacking constexpr std::max, maybe a trick like this:

template <typename... Records>
struct MaxSize {};

template <typename Record, typename... Records>
struct MaxSize<Record, Records...> {
  static const size_t value = sizeof(Record) >= MaxSize<Records...>::value ?
sizeof(Record) : MaxSize<Records...>::value;
};

template <>
struct MaxSize<> {
  static const size_t value = 0;
};

SkAlignedSStorage<MaxSize<SkRecords::DrawPath, SkRecords::DrawRect,
SkRecords::DrawTextBlob>::value> fBuffer;

[mtklein, 2015-05-07 19:47:45]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-05-07 19:47:46]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1112523006/#ps380001
(title: "size")

[commit-bot: I haz the power, 2015-05-07 19:47:59]

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

[mtklein, 2015-05-07 19:48:04]

On 2015/05/07 19:38:26, jbroman wrote:
>
https://codereview.chromium.org/1112523006/diff/360001/src/core/SkMiniRecorder.h
> File src/core/SkMiniRecorder.h (right):
> 
>
https://codereview.chromium.org/1112523006/diff/360001/src/core/SkMiniRecorde...
> src/core/SkMiniRecorder.h:34: SkAlignedSStorage<sizeof(SkRecords::DrawPath)>
> fBuffer;
> Lacking constexpr std::max, maybe a trick like this:
> 
> template <typename... Records>
> struct MaxSize {};
> 
> template <typename Record, typename... Records>
> struct MaxSize<Record, Records...> {
>   static const size_t value = sizeof(Record) >= MaxSize<Records...>::value ?
> sizeof(Record) : MaxSize<Records...>::value;
> };
> 
> template <>
> struct MaxSize<> {
>   static const size_t value = 0;
> };
> 
> SkAlignedSStorage<MaxSize<SkRecords::DrawPath, SkRecords::DrawRect,
> SkRecords::DrawTextBlob>::value> fBuffer;

Hmm, will give this a shot.

[commit-bot: I haz the power, 2015-05-07 19:51:20]

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

[commit-bot: I haz the power, 2015-05-07 19:51:22]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[mtklein, 2015-05-07 19:52:36]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-05-07 19:52:36]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1112523006/#ps400001
(title: "Max<A,B>")

[commit-bot: I haz the power, 2015-05-07 19:52:59]

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

[commit-bot: I haz the power, 2015-05-07 19:56:37]

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

[commit-bot: I haz the power, 2015-05-07 19:56:38]

Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[mtklein, 2015-05-07 20:33:52]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-05-07 20:33:53]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1112523006/#ps420001
(title: "unit test fixes")

[commit-bot: I haz the power, 2015-05-07 20:34:16]

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

[commit-bot: I haz the power, 2015-05-07 20:41:13]

Committed patchset #22 (id:420001) as
https://skia.googlesource.com/skia/+/c92c129ff85b05a714bd1bf921c02d5e14651f8b

[reed2, 2015-05-08 00:29:32]

A revert of this CL (patchset #22 id:420001) has been created in
https://codereview.chromium.org/1130333002/ by reed@chromium.org.

The reason for reverting is: speculative revert to fix failures in DEPS roll.

[mtklein, 2015-05-11 14:33:46]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[mtklein, 2015-05-11 14:33:47]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1112523006/#ps440001
(title: "implement suitable-for-gpu for small pictures")

[commit-bot: I haz the power, 2015-05-11 14:33:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1112523006/440001

[commit-bot: I haz the power, 2015-05-11 14:40:16]

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

[commit-bot: I haz the power, 2015-05-11 14:40:18]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-05-11 16:06:55]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[mtklein, 2015-05-11 16:06:56]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1112523006/#ps460001
(title: "Don't assume detachAsPicture() is called.")

[commit-bot: I haz the power, 2015-05-11 16:07:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1112523006/460001

[commit-bot: I haz the power, 2015-05-11 18:21:09]

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

[commit-bot: I haz the power, 2015-05-11 18:21:11]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-05-18 17:13:41]

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

[mtklein, 2015-05-18 18:30:48]

The CQ bit was checked by mtklein@google.com to run a CQ dry run

[mtklein, 2015-05-18 18:30:49]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1112523006/#ps480001
(title: "Rebase")

[commit-bot: I haz the power, 2015-05-18 18:31:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1112523006/480001

[commit-bot: I haz the power, 2015-05-18 20:01:01]

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

[commit-bot: I haz the power, 2015-05-18 20:01:02]

Dry run: This issue passed the CQ dry run.

[mtklein, 2015-05-18 20:45:48]

The CQ bit was checked by mtklein@google.com

[commit-bot: I haz the power, 2015-05-18 20:46:22]

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

[commit-bot: I haz the power, 2015-05-18 20:47:23]

Committed patchset #25 (id:480001) as
https://skia.googlesource.com/skia/+/15877b6eae33a9282458bdb904a6d00440eca0ec

[mtklein, 2015-05-18 21:53:07]

A revert of this CL (patchset #25 id:480001) has been created in
https://codereview.chromium.org/1130283004/ by mtklein@google.com.

The reason for reverting is: win_chromium_compile_dbg_ng

FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma/gomacc
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo
/showIncludes /FC @obj\third_party\skia\src\core\skia.SkBitmapHeap.obj.rsp /c
..\..\third_party\skia\src\core\SkBitmapHeap.cpp
/Foobj\third_party\skia\src\core\skia.SkBitmapHeap.obj /Fdobj\skia\skia.cc.pdb 
e:\b\build\slave\win\build\src\third_party\skia\include\core\skpicture.h(176) :
error C2487: 'CURRENT_PICTURE_VERSION' : member of dll interface class may not
be declared with dll interface
.

[mtklein, 2015-05-19 17:40:35]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-05-19 17:40:36]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1112523006/#ps510001
(title: "no singleton for now")

[commit-bot: I haz the power, 2015-05-19 17:41:00]

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

[mtklein, 2015-05-19 17:46:59]

The CQ bit was unchecked by mtklein@google.com

[commit-bot: I haz the power, 2015-05-19 17:47:19]

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

[mtklein, 2015-05-19 18:04:33]

The CQ bit was checked by mtklein@google.com

[mtklein, 2015-05-19 18:04:33]

The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com
Link to the patchset: https://codereview.chromium.org/1112523006/#ps570001
(title: "note")

[commit-bot: I haz the power, 2015-05-19 18:04:48]

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

[commit-bot: I haz the power, 2015-05-19 18:11:30]

Committed patchset #30 (id:570001) as
https://skia.googlesource.com/skia/+/9db912c2ac2ab53bc24f2d50a3e5a80162051dcc