Initial support for turning Skia draws into a JSON document and vice versa.

Initial support for turning Skia draws into a JSON document and vice versa.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1636563002

Committed: https://skia.googlesource.com/skia/+/3cb582f688822461efa5a034e18008bf2f11e4f8

Committed: https://skia.googlesource.com/skia/+/978d08a4a90d69961bd53811ed3ab222b88e2d30

[ethannicholas, 2016-01-25 18:23:35]

Description was changed from

==========
Initial support for turning Skia draws into a JSON document and vice versa.
==========

to

==========
Initial support for turning Skia draws into a JSON document and vice versa.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[ethannicholas, 2016-01-25 18:23:55]

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

[commit-bot: I haz the power, 2016-01-25 18:24:01]

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

[commit-bot: I haz the power, 2016-01-25 18:24:53]

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

[commit-bot: I haz the power, 2016-01-25 18:24:54]

Dry run: Try jobs failed on following builders:
  Build-Mac10.9-Clang-Arm7-Debug-iOS-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Mac10.9-Clang-...)
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[ethannicholas, 2016-01-25 18:28:23]

Patchset #2 (id:20001) has been deleted

[ethannicholas, 2016-01-25 18:29:55]

Patchset #1 (id:1) has been deleted

[ethannicholas, 2016-01-25 18:30:09]

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

[commit-bot: I haz the power, 2016-01-25 18:30:19]

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

[commit-bot: I haz the power, 2016-01-25 18:32:39]

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

[commit-bot: I haz the power, 2016-01-25 18:32:39]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)
  Build-Win-MSVC-x86-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86-D...)
  Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)

[ethannicholas, 2016-01-25 19:11:55]

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

[commit-bot: I haz the power, 2016-01-25 19:12:00]

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

[commit-bot: I haz the power, 2016-01-25 19:30:19]

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

[commit-bot: I haz the power, 2016-01-25 19:30:19]

Dry run: This issue passed the CQ dry run.

[ethannicholas, 2016-01-25 19:31:23]

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

[jcgregorio, 2016-01-25 19:37:32]

Do you have an example of the generated output?

https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.cpp
File src/json/SkJSONCanvas.cpp (right):

https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.c...
src/json/SkJSONCanvas.cpp:18: fOut.writeText("{\"" SKJSONCANVAS_VERSION "\":1,
\"" SKJSONCANVAS_COMMANDS
You use the json library for parsing the JSON, why don't use also use it to
generate the JSON?

[joshualitt, 2016-01-25 19:41:04]

https://codereview.chromium.org/1636563002/diff/60001/gyp/json.gyp
File gyp/json.gyp (right):

https://codereview.chromium.org/1636563002/diff/60001/gyp/json.gyp#newcode8
gyp/json.gyp:8: 'target_name': 'json',
hmm, this name could be a bit clearer json_canvas?  I guess it also has the
renderer.

https://codereview.chromium.org/1636563002/diff/60001/gyp/json.gyp#newcode14
gyp/json.gyp:14: 'jsoncpp.gyp:jsoncpp',
do you actually use jsoncpp?

https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.cpp
File src/json/SkJSONCanvas.cpp (right):

https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.c...
src/json/SkJSONCanvas.cpp:75: rect.left(), rect.top(), rect.right(),
rect.bottom(), corner1.x(), corner1.y(),
\n

https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.c...
src/json/SkJSONCanvas.cpp:76: corner2.x(), corner2.y(), corner3.x(),
corner3.y(), corner4.x(), corner4.y());
\n

https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.c...
src/json/SkJSONCanvas.cpp:132: this->writef("\"" SKJSONCANVAS_ATTRIBUTE_COLOR
"\":[%d,%d,%d,%d]", SkColorGetA(color),
\n

[ethannicholas, 2016-01-25 19:48:25]

On 2016/01/25 19:37:32, jcgregorio wrote:
> Do you have an example of the generated output?

{"version":1,
"commands":[{"command":"Matrix","matrix":[[1.000000,0.000000,20.000000],[0.000000,1.000000,20.000000],[0.000000,0.000000,1.000000]]},{"command":"Path","path":[{"move":[200.103638,900.077698]},{"conic":[[-199.974060,600.181335],[99.922302,200.103638],0.707107]},{"conic":[[399.818665,-199.974060],[799.896362,99.922302],0.707107]},{"conic":[[1199.974121,399.818665],[900.077698,799.896362],0.707107]},{"conic":[[669.959167,1106.886719],[313.870209,964.064331],0.793353]}],"paint":{"color":[255,148,65,165],"style":"stroke","strokeWidth":15.000000,"antiAlias":true}}]}

> 
>
https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.cpp
> File src/json/SkJSONCanvas.cpp (right):
> 
>
https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.c...
> src/json/SkJSONCanvas.cpp:18: fOut.writeText("{\"" SKJSONCANVAS_VERSION "\":1,
> \"" SKJSONCANVAS_COMMANDS
> You use the json library for parsing the JSON, why don't use also use it to
> generate the JSON?

I've just generally found JSON generators to be more trouble than they are worth
-- the format is so simple that there's really nothing to it. But I don't feel
strongly about it, and am happy to change if you think I should.

[jcgregorio, 2016-01-25 19:51:54]

On 2016/01/25 at 19:48:25, ethannicholas wrote:
> On 2016/01/25 19:37:32, jcgregorio wrote:
> > Do you have an example of the generated output?
> 
> {"version":1,
"commands":[{"command":"Matrix","matrix":[[1.000000,0.000000,20.000000],[0.000000,1.000000,20.000000],[0.000000,0.000000,1.000000]]},{"command":"Path","path":[{"move":[200.103638,900.077698]},{"conic":[[-199.974060,600.181335],[99.922302,200.103638],0.707107]},{"conic":[[399.818665,-199.974060],[799.896362,99.922302],0.707107]},{"conic":[[1199.974121,399.818665],[900.077698,799.896362],0.707107]},{"conic":[[669.959167,1106.886719],[313.870209,964.064331],0.793353]}],"paint":{"color":[255,148,65,165],"style":"stroke","strokeWidth":15.000000,"antiAlias":true}}]}
> 
> > 
> >
https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.cpp
> > File src/json/SkJSONCanvas.cpp (right):
> > 
> >
https://codereview.chromium.org/1636563002/diff/60001/src/json/SkJSONCanvas.c...
> > src/json/SkJSONCanvas.cpp:18: fOut.writeText("{\"" SKJSONCANVAS_VERSION
"\":1,
> > \"" SKJSONCANVAS_COMMANDS
> > You use the json library for parsing the JSON, why don't use also use it to
> > generate the JSON?
> 
> I've just generally found JSON generators to be more trouble than they are
worth -- the format is so simple that there's really nothing to it. But I don't
feel strongly about it, and am happy to change if you think I should.

One encoding bug could cause the frontend to choke on the input and the debugger
won't work. I'm fine with landing it like it is now to get something working,
but would really like the next CL to move to using jsoncpp to do the encoding.

[mtklein, 2016-01-25 19:56:24]

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

[mtklein, 2016-01-25 19:56:24]

Have you guys seem SkDebugCanvas?  It should be a big accelerator for all the
non-JSON aspects of this.  It implements SkCanvas and gives you back a bunch of
SkDrawCommand objects representing each draw.

[mtklein, 2016-01-25 19:56:33]

On 2016/01/25 19:56:24, mtklein wrote:
> Have you guys seem SkDebugCanvas?  It should be a big accelerator for all the
> non-JSON aspects of this.  It implements SkCanvas and gives you back a bunch
of
> SkDrawCommand objects representing each draw.

*seen

[ethannicholas, 2016-01-25 19:59:17]

On 2016/01/25 19:56:33, mtklein wrote:
> On 2016/01/25 19:56:24, mtklein wrote:
> > Have you guys seem SkDebugCanvas?  It should be a big accelerator for all
the
> > non-JSON aspects of this.  It implements SkCanvas and gives you back a bunch
> of
> > SkDrawCommand objects representing each draw.
> 
> *seen

I took a look at it, but didn't feel that it would simplify things -- seemed
like the effort to iterate through the list of SkDrawCommands would be at least
as great as the effort to just intercept the draw commands myself, plus it would
add the cost of building the list.

[ethannicholas, 2016-01-25 20:52:19]

Josh pointed out that jsoncpp is enormous and should be gated behind a build
variable -- seemed like it was best to just put it behind the skia_build_server
variable he had already created, since these two things will go together anyway.

https://codereview.chromium.org/1636563002/diff/60001/gyp/json.gyp
File gyp/json.gyp (right):

https://codereview.chromium.org/1636563002/diff/60001/gyp/json.gyp#newcode8
gyp/json.gyp:8: 'target_name': 'json',
On 2016/01/25 19:41:04, joshualitt wrote:
> hmm, this name could be a bit clearer json_canvas?  I guess it also has the
> renderer. 

That was my reasoning. I'm not sure from your message how strongly you feel that
it should change...?

https://codereview.chromium.org/1636563002/diff/60001/gyp/json.gyp#newcode14
gyp/json.gyp:14: 'jsoncpp.gyp:jsoncpp',
On 2016/01/25 19:41:04, joshualitt wrote:
> do you actually use jsoncpp?

Yes, it's the parser in SkJSONRenderer.

[mtklein, 2016-01-25 20:53:15]

On 2016/01/25 20:52:19, ethannicholas wrote:
> Josh pointed out that jsoncpp is enormous and should be gated behind a build
> variable -- seemed like it was best to just put it behind the
skia_build_server
> variable he had already created, since these two things will go together
anyway.

That's probably not necessary.  Most of our tools already depend on it.

[ethannicholas, 2016-01-25 21:06:49]

On 2016/01/25 20:53:15, mtklein wrote:
> On 2016/01/25 20:52:19, ethannicholas wrote:
> > Josh pointed out that jsoncpp is enormous and should be gated behind a build
> > variable -- seemed like it was best to just put it behind the
> skia_build_server
> > variable he had already created, since these two things will go together
> anyway.
> 
> That's probably not necessary.  Most of our tools already depend on it.

It's to keep Skia itself (as opposed to tools) from depending on it.

[joshualitt, 2016-01-25 21:16:16]

On 2016/01/25 21:06:49, ethannicholas wrote:
> On 2016/01/25 20:53:15, mtklein wrote:
> > On 2016/01/25 20:52:19, ethannicholas wrote:
> > > Josh pointed out that jsoncpp is enormous and should be gated behind a
build
> > > variable -- seemed like it was best to just put it behind the
> > skia_build_server
> > > variable he had already created, since these two things will go together
> > anyway.
> > 
> > That's probably not necessary.  Most of our tools already depend on it.
> 
> It's to keep Skia itself (as opposed to tools) from depending on it.

lgtm, this should be fine as we iterate.

[ethannicholas, 2016-01-25 21:17:19]

The CQ bit was checked by ethannicholas@google.com

[commit-bot: I haz the power, 2016-01-25 21:17:26]

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

[mtklein, 2016-01-25 21:18:42]

None of this code so far is part of Skia-the-library.  Why don't you move it
somewhere under tools/ so you don't get confused?

[commit-bot: I haz the power, 2016-01-25 21:28:30]

Description was changed from

==========
Initial support for turning Skia draws into a JSON document and vice versa.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Initial support for turning Skia draws into a JSON document and vice versa.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/3cb582f688822461efa5a034e18008bf2f11e4f8
==========

[commit-bot: I haz the power, 2016-01-25 21:28:32]

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

[msarett, 2016-01-25 21:57:14]

A revert of this CL (patchset #3 id:80001) has been created in
https://codereview.chromium.org/1637643002/ by msarett@google.com.

The reason for reverting is: Breaking the CMake build..

[mtklein, 2016-01-25 21:58:03]

On 2016/01/25 21:57:14, msarett wrote:
> A revert of this CL (patchset #3 id:80001) has been created in
> https://codereview.chromium.org/1637643002/ by mailto:msarett@google.com.
> 
> The reason for reverting is: Breaking the CMake build..

(moving this stuff to tools/ will fix CMake + Google3)

[ethannicholas, 2016-01-26 15:15:24]

Description was changed from

==========
Initial support for turning Skia draws into a JSON document and vice versa.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/3cb582f688822461efa5a034e18008bf2f11e4f8
==========

to

==========
Initial support for turning Skia draws into a JSON document and vice versa.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/3cb582f688822461efa5a034e18008bf2f11e4f8
==========

[dogben, 2016-01-26 15:35:13]

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

[dogben, 2016-01-26 15:35:14]

lgtm

[ethannicholas, 2016-01-26 15:36:01]

The CQ bit was checked by ethannicholas@google.com

[ethannicholas, 2016-01-26 15:36:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1636563002/#ps100001
(title: "moved to /tools")

[commit-bot: I haz the power, 2016-01-26 15:36:12]

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

[commit-bot: I haz the power, 2016-01-26 15:48:00]

Description was changed from

==========
Initial support for turning Skia draws into a JSON document and vice versa.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/3cb582f688822461efa5a034e18008bf2f11e4f8
==========

to

==========
Initial support for turning Skia draws into a JSON document and vice versa.
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/3cb582f688822461efa5a034e18008bf2f11e4f8

Committed:
https://skia.googlesource.com/skia/+/978d08a4a90d69961bd53811ed3ab222b88e2d30
==========

[commit-bot: I haz the power, 2016-01-26 15:48:01]

Committed patchset #4 (id:100001) as
https://skia.googlesource.com/skia/+/978d08a4a90d69961bd53811ed3ab222b88e2d30