Add code to lua paths to get the fill path and get lists of verbs and

Add code to lua paths to get the fill path and get lists of verbs and
points

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2199823002

Committed: https://skia.googlesource.com/skia/+/0b401ce663ef80222e0eb3a54bdd4c42af8e48e1

[Harry Stern, 2016-08-01 16:21:22]

Description was changed from

==========
Add code to lua paths to get the fill path and get lists of verbs and
points

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

to

==========
Add code to lua paths to get the fill path and get lists of verbs and
points

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2199823002
==========

[Harry Stern, 2016-08-01 19:03:26]

Description was changed from

==========
Add code to lua paths to get the fill path and get lists of verbs and
points

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2199823002
==========

to

==========
Add code to lua paths to get the fill path and get lists of verbs and
points

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2199823002
==========

[Harry Stern, 2016-08-01 19:03:26]

hstern@google.com changed reviewers:
+ caryclark@google.com, reed@google.com

[reed1, 2016-08-01 19:36:29]

https://codereview.chromium.org/2199823002/diff/80001/src/utils/SkLua.cpp
File src/utils/SkLua.cpp (right):

https://codereview.chromium.org/2199823002/diff/80001/src/utils/SkLua.cpp#new...
src/utils/SkLua.cpp:1618: switch (iter.next(pts, true)) {
this indent looks too big. "switch" should be 4 spaces to the right of "do"

https://codereview.chromium.org/2199823002/diff/80001/src/utils/SkLua.cpp#new...
src/utils/SkLua.cpp:1620: setarray_string(L, i, "move"); i++;
different option to consider

int i = 0;

    setarray_string(L, ++i, "move");

https://codereview.chromium.org/2199823002/diff/80001/tools/lua/count_dashes.lua
File tools/lua/count_dashes.lua (right):

https://codereview.chromium.org/2199823002/diff/80001/tools/lua/count_dashes....
tools/lua/count_dashes.lua:26: local dash = pe:asADash()
possibly check for drawPath before creating the dash object?

https://codereview.chromium.org/2199823002/diff/80001/tools/lua/count_dashes....
tools/lua/count_dashes.lua:36: if verb == "move" or verb == "close" then

why do you look for close?

[Harry Stern, 2016-08-01 20:58:23]

https://codereview.chromium.org/2199823002/diff/80001/src/utils/SkLua.cpp
File src/utils/SkLua.cpp (right):

https://codereview.chromium.org/2199823002/diff/80001/src/utils/SkLua.cpp#new...
src/utils/SkLua.cpp:1618: switch (iter.next(pts, true)) {
On 2016/08/01 19:36:29, reed1 wrote:
> this indent looks too big. "switch" should be 4 spaces to the right of "do"

Done.

https://codereview.chromium.org/2199823002/diff/80001/src/utils/SkLua.cpp#new...
src/utils/SkLua.cpp:1620: setarray_string(L, i, "move"); i++;
On 2016/08/01 19:36:29, reed1 wrote:
> different option to consider
> 
> int i = 0;
> 
>     setarray_string(L, ++i, "move");

Any time I can index starting at 0 is a good time. Done.

https://codereview.chromium.org/2199823002/diff/80001/tools/lua/count_dashes.lua
File tools/lua/count_dashes.lua (right):

https://codereview.chromium.org/2199823002/diff/80001/tools/lua/count_dashes....
tools/lua/count_dashes.lua:26: local dash = pe:asADash()
On 2016/08/01 19:36:29, reed1 wrote:
> possibly check for drawPath before creating the dash object?

I'll just do them at the same time, there's no reason not to. Done.

https://codereview.chromium.org/2199823002/diff/80001/tools/lua/count_dashes....
tools/lua/count_dashes.lua:36: if verb == "move" or verb == "close" then
On 2016/08/01 19:36:29, reed1 wrote:
> 
> why do you look for close?

(Mike and I discussed this and he told me that we aren't necessarily doing a
close for each segment of the dash)

I will just fix the count at the end. Done.

[reed1, 2016-08-01 21:17:21]

lgtm w/ nits

https://codereview.chromium.org/2199823002/diff/120001/src/utils/SkLua.cpp
File src/utils/SkLua.cpp (right):

https://codereview.chromium.org/2199823002/diff/120001/src/utils/SkLua.cpp#ne...
src/utils/SkLua.cpp:225: lua_pushstring(L, str);
indent is too far

https://codereview.chromium.org/2199823002/diff/120001/src/utils/SkLua.cpp#ne...
src/utils/SkLua.cpp:1615: bool done = false;
indent?

https://codereview.chromium.org/2199823002/diff/120001/src/utils/SkLua.cpp#ne...
src/utils/SkLua.cpp:1641: }
indent?

https://codereview.chromium.org/2199823002/diff/120001/tools/lua/count_dashes...
File tools/lua/count_dashes.lua (right):

https://codereview.chromium.org/2199823002/diff/120001/tools/lua/count_dashes...
tools/lua/count_dashes.lua:29: if dash and t.verb == "drawPath" then
we're still creating the dash object even when the verb does not need it...

you can leave it as is, but I think branching on drawPath before calling asADash
would be more efficient

[Harry Stern, 2016-08-01 22:07:33]

https://codereview.chromium.org/2199823002/diff/120001/src/utils/SkLua.cpp
File src/utils/SkLua.cpp (right):

https://codereview.chromium.org/2199823002/diff/120001/src/utils/SkLua.cpp#ne...
src/utils/SkLua.cpp:225: lua_pushstring(L, str);
On 2016/08/01 21:17:21, reed1 wrote:
> indent is too far

Done.

https://codereview.chromium.org/2199823002/diff/120001/src/utils/SkLua.cpp#ne...
src/utils/SkLua.cpp:1615: bool done = false;
On 2016/08/01 21:17:21, reed1 wrote:
> indent?

Done.

https://codereview.chromium.org/2199823002/diff/120001/src/utils/SkLua.cpp#ne...
src/utils/SkLua.cpp:1641: }
On 2016/08/01 21:17:21, reed1 wrote:
> indent?

Done.

https://codereview.chromium.org/2199823002/diff/120001/tools/lua/count_dashes...
File tools/lua/count_dashes.lua (right):

https://codereview.chromium.org/2199823002/diff/120001/tools/lua/count_dashes...
tools/lua/count_dashes.lua:29: if dash and t.verb == "drawPath" then
On 2016/08/01 21:17:21, reed1 wrote:
> we're still creating the dash object even when the verb does not need it...
> 
> you can leave it as is, but I think branching on drawPath before calling
asADash
> would be more efficient

Oh, I see. I thought you were just saying that I had extra ifs when I didn't
need to.

How about 'if t.verb == "drawPath" and pe:asADash()' since I don't actually need
the dash variable? Assuming lua uses short-circuit ifs it shouldn't call the
function if the verb is not a drawPath.

[reed1, 2016-08-02 15:22:51]

lgtm

[Harry Stern, 2016-08-02 15:48:55]

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

[commit-bot: I haz the power, 2016-08-02 15:49:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 16:16:14]

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

[commit-bot: I haz the power, 2016-08-02 16:16:15]

Dry run: This issue passed the CQ dry run.

[Harry Stern, 2016-08-02 16:16:57]

The CQ bit was checked by hstern@google.com

[commit-bot: I haz the power, 2016-08-02 16:17:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 16:18:02]

Description was changed from

==========
Add code to lua paths to get the fill path and get lists of verbs and
points

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2199823002
==========

to

==========
Add code to lua paths to get the fill path and get lists of verbs and
points

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2199823002

Committed:
https://skia.googlesource.com/skia/+/0b401ce663ef80222e0eb3a54bdd4c42af8e48e1
==========

[commit-bot: I haz the power, 2016-08-02 16:18:03]

Committed patchset #8 (id:140001) as
https://skia.googlesource.com/skia/+/0b401ce663ef80222e0eb3a54bdd4c42af8e48e1