Adding a tool to get images from skps

Adding a tool to get images from skps

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

Committed: https://skia.googlesource.com/skia/+/3478f753ffc28a6f0f0877cc06be7373f960c527

[msarett, 2016-02-12 16:17:53]

Description was changed from

==========
Adding a tool to get from images from skps

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

to

==========
Adding a tool to get from images from skps

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

[msarett, 2016-02-12 16:29:47]

Patchset #1 (id:1) has been deleted

[msarett, 2016-02-12 16:33:48]

msarett@google.com changed reviewers:
+ scroggo@google.com

[msarett, 2016-02-12 16:33:49]

Once this tool (or something like it) is ready, I'm going to talk to Ravi about
generating a new set of skps (without reencoding any images).

I've run this on a small set of skps to test.  So far I'm a little surprised
that all of the images seem quite small.

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
File tools/get_images_from_skps.cpp (right):

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:130: SkAutoTUnref<SkPicture>
picture(SkPicture::CreateFromStream(stream, store_encoded_to_file));
Originally, I wanted to write code that actually parsed the SkPicture.  But I
think this is a nice way to take advantage of the fact that someone has already
written that code :).

[scroggo, 2016-02-12 17:18:39]

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
File tools/get_images_from_skps.cpp (right):

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:24: SkDebugf("Usage: get_images_from_skps -s <dir
of skps> -o <dir for output images>\n");
SkCommandLineFlags has a method to specify usage (SetUsage), which gets printed
when you use -h/--help. Maybe we should add a method to print that usage, and
you can use that? Then this message will also be printed when you use -h.

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:31: path.append("/");
Should these use SkOSPath::Join so that it will work on Windows?

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:130: SkAutoTUnref<SkPicture>
picture(SkPicture::CreateFromStream(stream, store_encoded_to_file));
On 2016/02/12 16:33:49, msarett wrote:
> Originally, I wanted to write code that actually parsed the SkPicture.  But I
> think this is a nice way to take advantage of the fact that someone has
already
> written that code :).RWBuffer_threadSafe,

Yep! This is similar to a feature we used to have in the long dead
render_pictures program, which was subsumed into DM. (Although I just did a git
grep of "render_pictures" and it turns out we have two python scripts that use
it, plus some documentation that refers to it. So we have more cleanups to do
there...)

Why not put this inside DM also?

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:136: SkDebugf("Saved %d images to %s\n",
gSuccessCtr, gOutputDir);
Should we also report the number of images that failed?

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:137: return 1;
FWIW, I think our other testing tools return 0 and success and 1 on failure.

[mtklein, 2016-02-12 17:21:41]

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

[mtklein, 2016-02-12 17:21:42]

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
File tools/get_images_from_skps.cpp (right):

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:130: SkAutoTUnref<SkPicture>
picture(SkPicture::CreateFromStream(stream, store_encoded_to_file));
> Why not put this inside DM also?

Unlike render_pictures, this program has multiple outputs per input, which
doesn't fit well into DM's model.  Let's just leave it stand-alone.

[scroggo, 2016-02-12 18:50:27]

lgtm

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
File tools/get_images_from_skps.cpp (right):

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:130: SkAutoTUnref<SkPicture>
picture(SkPicture::CreateFromStream(stream, store_encoded_to_file));
On 2016/02/12 17:21:41, mtklein wrote:
> > Why not put this inside DM also?
> 
> Unlike render_pictures, this program has multiple outputs per input, which
> doesn't fit well into DM's model.  Let's just leave it stand-alone.

sgtm

[msarett, 2016-02-12 21:30:29]

Using SkOSPath::Join() and added a SkCommandLineFlags::PrintUsage().

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
File tools/get_images_from_skps.cpp (right):

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:24: SkDebugf("Usage: get_images_from_skps -s <dir
of skps> -o <dir for output images>\n");
On 2016/02/12 17:18:38, scroggo wrote:
> SkCommandLineFlags has a method to specify usage (SetUsage), which gets
printed
> when you use -h/--help. Maybe we should add a method to print that usage, and
> you can use that? Then this message will also be printed when you use -h.

Done.

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:31: path.append("/");
On 2016/02/12 17:18:38, scroggo wrote:
> Should these use SkOSPath::Join so that it will work on Windows?

Done.

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:136: SkDebugf("Saved %d images to %s\n",
gSuccessCtr, gOutputDir);
On 2016/02/12 17:18:38, scroggo wrote:
> Should we also report the number of images that failed?

Yes that sounds good.

https://codereview.chromium.org/1696763002/diff/20001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:137: return 1;
On 2016/02/12 17:18:38, scroggo wrote:
> FWIW, I think our other testing tools return 0 and success and 1 on failure.

Haha yes.  Fixed.

[msarett, 2016-02-12 21:33:29]

Description was changed from

==========
Adding a tool to get from images from skps

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

to

==========
Adding a tool to get images from skps

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

[scroggo, 2016-02-12 21:53:27]

https://codereview.chromium.org/1696763002/diff/40001/tools/get_images_from_s...
File tools/get_images_from_skps.cpp (right):

https://codereview.chromium.org/1696763002/diff/40001/tools/get_images_from_s...
tools/get_images_from_skps.cpp:98: gFailureCtr++;
I guess "number of failures" is ambiguous - I was thinking the images for which
we did not create a codec (or the "default" case).

This number tells us the number of times the file system gave us trouble, which
may be interesting (the data we retrieved is not complete). But I think failures
to create codec (of one of the types we expect) is a sign of a deeper problem.

[msarett, 2016-02-12 22:09:48]

Hmmm yeah.  I think I'll break up successes into "known" and "unknown" images.

[msarett, 2016-02-12 22:19:54]

Now keeps track of recognized and unrecognized images.

[scroggo, 2016-02-12 22:21:03]

lgtm

[msarett, 2016-02-12 22:21:34]

The CQ bit was checked by msarett@google.com

[commit-bot: I haz the power, 2016-02-12 22:21:44]

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

[commit-bot: I haz the power, 2016-02-12 22:47:11]

Description was changed from

==========
Adding a tool to get images from skps

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

to

==========
Adding a tool to get images from skps

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/+/3478f753ffc28a6f0f0877cc06be7373f960c527
==========

[commit-bot: I haz the power, 2016-02-12 22:47:12]

Committed patchset #3 (id:60001) as
https://skia.googlesource.com/skia/+/3478f753ffc28a6f0f0877cc06be7373f960c527