Add abilitly to query audit trail for batches by draw op

Add abilitly to query audit trail for batches by draw op

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1745513002

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

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

[joshua.litt, 2016-02-26 19:33:18]

Description was changed from

==========
Add abilitly to query audit trail for batches by draw op

BUG=skia:
==========

to

==========
Add abilitly to query audit trail for batches by draw op

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[joshualitt, 2016-02-26 20:01:24]

joshualitt@google.com changed reviewers:
+ ethannicholas@google.com, jcgregorio@google.com, joshualitt@google.com

[joshualitt, 2016-02-26 20:01:25]

This is a ground up rethinking of how GrAuditTrail works, and gives us the
ability to cross reference between the GrAuditTrail and SkCanvas level draw ops.

How it works is the client supplies a 'ClientID,' which we use to collect all of
the batches associated with that draw(see AutoCollectBatches).

We also store a flat list of BatchNodes, each of which has a bunch of Batches. 
The final JSON we return for both the BatchList and for a given skia draw op,
has a ClientID(the skia draw of), a BatchListID(ID of the BatchNode in the
BatchList which stores the batch), and a ChildID(offset into the BatchNode).

Using all of this information we can cross reference.  I had to dump the old
stack frame code, now in the JSON we return just a flat list of the batches the
Skia draw op created, but I think that is clearer, and we can always wire up the
stack frame stuff in a more sensible way later on.

Thoughts?

[ethannicholas, 2016-02-26 20:55:32]

https://codereview.chromium.org/1745513002/diff/40001/include/private/GrAudit...
File include/private/GrAuditTrail.h (right):

https://codereview.chromium.org/1745513002/diff/40001/include/private/GrAudit...
include/private/GrAuditTrail.h:114: // can be 'trusted'
I don't fully understand this comment.

https://codereview.chromium.org/1745513002/diff/40001/src/gpu/GrAuditTrail.cpp
File src/gpu/GrAuditTrail.cpp (right):

https://codereview.chromium.org/1745513002/diff/40001/src/gpu/GrAuditTrail.cp...
src/gpu/GrAuditTrail.cpp:159: json->appendf("\"Left\": %f,", rect.fLeft);
I know it predates this change, but while we're at it we should unify the rect
format here with that used by SkDrawCommand (one way or the other).

[joshualitt, 2016-02-26 21:08:00]

https://codereview.chromium.org/1745513002/diff/40001/include/private/GrAudit...
File include/private/GrAuditTrail.h (right):

https://codereview.chromium.org/1745513002/diff/40001/include/private/GrAudit...
include/private/GrAuditTrail.h:114: // can be 'trusted'
On 2016/02/26 20:55:32, ethannicholas wrote:
> I don't fully understand this comment.

Sorry, try the comment now.  Basically, you need to know what you are doing when
you call toJson because the results might only be valid for that point in time. 
However, this is algorithm dependent.  The second call below in particular, the
one which takes a clientID, is actually valid for all M where M >= N draws.  I
can explain why this is so, sorry its really confusing and probably a bit
fragile, but I also don't anticipate our reordering algorithm changing very much
if ever.

https://codereview.chromium.org/1745513002/diff/40001/src/gpu/GrAuditTrail.cpp
File src/gpu/GrAuditTrail.cpp (right):

https://codereview.chromium.org/1745513002/diff/40001/src/gpu/GrAuditTrail.cp...
src/gpu/GrAuditTrail.cpp:159: json->appendf("\"Left\": %f,", rect.fLeft);
On 2016/02/26 20:55:32, ethannicholas wrote:
> I know it predates this change, but while we're at it we should unify the rect
> format here with that used by SkDrawCommand (one way or the other).

Can we do this as a follow on? I think we should factor all all of the JSON
helpers into some kind of helper namespace.

[ethannicholas, 2016-02-26 21:42:14]

On 2016/02/26 21:08:00, joshualitt wrote:
>
https://codereview.chromium.org/1745513002/diff/40001/include/private/GrAudit...
> File include/private/GrAuditTrail.h (right):
> 
>
https://codereview.chromium.org/1745513002/diff/40001/include/private/GrAudit...
> include/private/GrAuditTrail.h:114: // can be 'trusted'
> On 2016/02/26 20:55:32, ethannicholas wrote:
> > I don't fully understand this comment.
> 
> Sorry, try the comment now.  Basically, you need to know what you are doing
when
> you call toJson because the results might only be valid for that point in
time. 
> However, this is algorithm dependent.  The second call below in particular,
the
> one which takes a clientID, is actually valid for all M where M >= N draws.  I
> can explain why this is so, sorry its really confusing and probably a bit
> fragile, but I also don't anticipate our reordering algorithm changing very
much
> if ever.
> 
> https://codereview.chromium.org/1745513002/diff/40001/src/gpu/GrAuditTrail.cpp
> File src/gpu/GrAuditTrail.cpp (right):
> 
>
https://codereview.chromium.org/1745513002/diff/40001/src/gpu/GrAuditTrail.cp...
> src/gpu/GrAuditTrail.cpp:159: json->appendf("\"Left\": %f,", rect.fLeft);
> On 2016/02/26 20:55:32, ethannicholas wrote:
> > I know it predates this change, but while we're at it we should unify the
rect
> > format here with that used by SkDrawCommand (one way or the other).
> 
> Can we do this as a follow on? I think we should factor all all of the JSON
> helpers into some kind of helper namespace.

Sounds good.

lgtm

[joshualitt, 2016-02-29 13:11:35]

The CQ bit was checked by joshualitt@google.com

[commit-bot: I haz the power, 2016-02-29 13:11:42]

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

[commit-bot: I haz the power, 2016-02-29 13:20:42]

Description was changed from

==========
Add abilitly to query audit trail for batches by draw op

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Add abilitly to query audit trail for batches by draw op

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/9b48a6e3f862076018cc7d63b180b6340f4873cd
==========

[commit-bot: I haz the power, 2016-02-29 13:20:46]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/9b48a6e3f862076018cc7d63b180b6340f4873cd

[joshualitt, 2016-02-29 14:32:03]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1747893002/ by joshualitt@google.com.

The reason for reverting is: kInvalidID conflicts with an older xCode #define.

[joshualitt, 2016-02-29 15:26:27]

Description was changed from

==========
Add abilitly to query audit trail for batches by draw op

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/9b48a6e3f862076018cc7d63b180b6340f4873cd
==========

to

==========
Add abilitly to query audit trail for batches by draw op

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/9b48a6e3f862076018cc7d63b180b6340f4873cd
==========

[joshualitt, 2016-02-29 15:26:31]

The CQ bit was checked by joshualitt@google.com

[joshualitt, 2016-02-29 15:26:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
ethannicholas@google.com
Link to the patchset: https://codereview.chromium.org/1745513002/#ps80001
(title: "tweaks")

[commit-bot: I haz the power, 2016-02-29 15:26:33]

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

[commit-bot: I haz the power, 2016-02-29 15:44:04]

Description was changed from

==========
Add abilitly to query audit trail for batches by draw op

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/9b48a6e3f862076018cc7d63b180b6340f4873cd
==========

to

==========
Add abilitly to query audit trail for batches by draw op

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

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

Committed:
https://skia.googlesource.com/skia/+/b95c772d45efc98f0159fa65c7aa06314cecb9de
==========

[commit-bot: I haz the power, 2016-02-29 15:44:05]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/b95c772d45efc98f0159fa65c7aa06314cecb9de