Keep track of the number of skia operations

Moved SkPictureContentInfo into its own file.  It now tracks the number of skia 
operations (maybe, I'm not 100% sure I'm doing this right yet)  and the number 
of text draws.  I also moved some of the gpu logic out of SkPictureRecord and 
into SkPictureContentInfo,

http://code.google.com/p/chromium/issues/detail?id=396908
http://code.google.com/p/chromium/issues/detail?id=397198
http://code.google.com/p/chromium/issues/detail?id=399728

BUG=396908
Committed: https://skia.googlesource.com/skia/+/afdada2eabdef47a59e08c884b2beb1af3f31aa5

[nduca, 2014-08-02 00:59:30]

https://codereview.chromium.org/435093003/diff/1/src/core/SkPictureContentInfo.h
File src/core/SkPictureContentInfo.h (right):

https://codereview.chromium.org/435093003/diff/1/src/core/SkPictureContentInf...
src/core/SkPictureContentInfo.h:13: class SkPictureContentInfo {
it seems like a SkPictureContentInfo and SkPictureContentInfoImpl would help ---
the the onDrawPoitns, etc is stuff thats part of the implementation of the
interface.

class  SkPictureContentInfo {
public:
numOps()
hasText()
suitableFor()

private:
  int fNumBlah.
}

class  SkPictureContentInfoImpl : public SkPictureContentInfo {
 addOperation
  onDraw*
  onAddPaintPtr
}

class SkPictureContent {
   const SkPictureContentInfo& contnt_info();
private:
  SkPictureContentInfoimpl_ info_;
}

[hendrikw, 2014-08-02 01:33:13]

Hi, please let me know what you think.

http://code.google.com/p/chromium/issues/detail?id=396908

[mtklein, 2014-08-02 14:06:28]

As a refactor, this looks great to me.

Before we get too far into this plan to expose SkPictureContentInfo, let's make
sure Robert's on board too.  Personally I think this is a fine interface for us
to support, and I like keeping the stats a little off to the side like this.

https://codereview.chromium.org/435093003/diff/20001/src/core/SkPictureConten...
File src/core/SkPictureContentInfo.cpp (right):

https://codereview.chromium.org/435093003/diff/20001/src/core/SkPictureConten...
src/core/SkPictureContentInfo.cpp:2: * Copyright 2011 Google Inc.
2014?

[mtklein, 2014-08-02 14:09:35]

On 2014/08/02 00:59:30, nduca wrote:
>
https://codereview.chromium.org/435093003/diff/1/src/core/SkPictureContentInfo.h
> File src/core/SkPictureContentInfo.h (right):
> 
>
https://codereview.chromium.org/435093003/diff/1/src/core/SkPictureContentInf...
> src/core/SkPictureContentInfo.h:13: class SkPictureContentInfo {
> it seems like a SkPictureContentInfo and SkPictureContentInfoImpl would help
---
> the the onDrawPoitns, etc is stuff thats part of the implementation of the
> interface.
> 
> class  SkPictureContentInfo {
> public:
> numOps()
> hasText()
> suitableFor()
> 
> private:
>   int fNumBlah.
> }
> 
> class  SkPictureContentInfoImpl : public SkPictureContentInfo {
>  addOperation
>   onDraw*
>   onAddPaintPtr
> }
> 
> class SkPictureContent {
>    const SkPictureContentInfo& contnt_info();
> private:
>   SkPictureContentInfoimpl_ info_;
> }

I'm generally okay with pimpls, but it does prevent inlining, which could be a
bother here for tiny frequent things like addOperation().  If we think of the
const methods as the public interface and the non-const methods as the impl,
seems fine to keep them all right here together.

[robertphillips, 2014-08-04 14:31:24]

https://codereview.chromium.org/435093003/diff/40001/gyp/core.gypi
File gyp/core.gypi (right):

https://codereview.chromium.org/435093003/diff/40001/gyp/core.gypi#newcode132
gyp/core.gypi:132: '<(skia_src_path)/core/SkPicture.cpp',
Add .h in here somewhere too?

https://codereview.chromium.org/435093003/diff/40001/src/core/SkPictureConten...
File src/core/SkPictureContentInfo.cpp (right):

https://codereview.chromium.org/435093003/diff/40001/src/core/SkPictureConten...
src/core/SkPictureContentInfo.cpp:61: 
Can this be all on one line ?

https://codereview.chromium.org/435093003/diff/40001/src/core/SkPictureConten...
src/core/SkPictureContentInfo.cpp:95: SkTSwap(fNumTexts, other->fNumTexts);
fNumPaintWithPathEffectUses ?

https://codereview.chromium.org/435093003/diff/40001/src/core/SkPictureConten...
File src/core/SkPictureContentInfo.h (right):

https://codereview.chromium.org/435093003/diff/40001/src/core/SkPictureConten...
src/core/SkPictureContentInfo.h:34: private:
// Raw count of operations in the picture ?

https://codereview.chromium.org/435093003/diff/40001/src/core/SkPictureConten...
src/core/SkPictureContentInfo.h:35: int fNumOperations;
// This includes all forms of drawText ?

https://codereview.chromium.org/435093003/diff/40001/src/core/SkPictureData.cpp
File src/core/SkPictureData.cpp (right):

https://codereview.chromium.org/435093003/diff/40001/src/core/SkPictureData.c...
src/core/SkPictureData.cpp:671: bool
SkPictureData::suitableForGpuRasterization(GrContext* context, const char
**reason,
line this guy up ?

[hendrikw, 2014-08-04 19:24:33]

I've fixed the issues that Mike and Robert pointed out, thanks!

Can someone verify that I'm counting the number of operations correctly (i.e.
does addDraw capture all skia operations, and only skia operations?)

[robertphillips, 2014-08-07 14:18:13]

lgtm. To the best of my knowledge addDraw is the correct location for counting
the operations.

https://codereview.chromium.org/435093003/diff/60001/src/core/SkPictureData.cpp
File src/core/SkPictureData.cpp (right):

https://codereview.chromium.org/435093003/diff/60001/src/core/SkPictureData.c...
src/core/SkPictureData.cpp:10: #include "SkPictureData.h"
Copyright?

[hendrikw, 2014-08-08 16:57:44]

The CQ bit was checked by hendrikw@chromium.org

[hendrikw, 2014-08-08 16:57:45]

The CQ bit was unchecked by hendrikw@chromium.org

[hendrikw, 2014-08-08 17:04:57]

The CQ bit was checked by hendrikw@chromium.org

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

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/hendrikw@chromium.org/435093003/60001

[hendrikw, 2014-08-08 17:08:23]

The CQ bit was unchecked by hendrikw@chromium.org

[hendrikw, 2014-08-08 17:23:22]

The CQ bit was checked by hendrikw@chromium.org

[commit-bot: I haz the power, 2014-08-08 17:23:50]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/hendrikw@chromium.org/435093003/80001

[commit-bot: I haz the power, 2014-08-08 17:31:10]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  Build-Win-VS2013-x86-Debug-Trybot on tryserver.skia
(http://108.170.220.76:10117/builders/Build-Win-VS2013-x86-Debug-Trybot/builds...)

[hendrikw, 2014-08-08 17:32:39]

The CQ bit was unchecked by hendrikw@chromium.org

[hendrikw, 2014-08-08 17:37:37]

The CQ bit was checked by hendrikw@chromium.org

[commit-bot: I haz the power, 2014-08-08 17:38:02]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/hendrikw@chromium.org/435093003/100001

[commit-bot: I haz the power, 2014-08-08 17:44:39]

Change committed as afdada2eabdef47a59e08c884b2beb1af3f31aa5