[SVGDom/experimental] Initial SVGLength support

[SVGDom] Initial SVGLength support

Mostly plumb new length types, but also introduce a stateful parser,
rect shape and named color support.

R=reed@google.com,robertphillips@google.com,stephana@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2202053002

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

[f(malita), 2016-08-01 23:06:22]

Description was changed from

==========
[SVGDom] Initial SVGLength support

Mostly plumb new length types, but also introduce a stateful parser,
rect shape and named color support.

R=reed@google.com,robertphillips@google.com,stephana@google.com
==========

to

==========
[SVGDom] Initial SVGLength support

Mostly plumb new length types, but also introduce a stateful parser,
rect shape and named color support.

R=reed@google.com,robertphillips@google.com,stephana@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2202053002
==========

[f(malita), 2016-08-01 23:12:52]

Description was changed from

==========
[SVGDom] Initial SVGLength support

Mostly plumb new length types, but also introduce a stateful parser,
rect shape and named color support.

R=reed@google.com,robertphillips@google.com,stephana@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2202053002
==========

to

==========
[SVGDom] Initial SVGLength support

Mostly plumb new length types, but also introduce a stateful parser,
rect shape and named color support.

R=reed@google.com,robertphillips@google.com,stephana@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2202053002
==========

[f(malita), 2016-08-02 18:20:21]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

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

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 18:39:06]

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

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

Dry run: This issue passed the CQ dry run.

[robertphillips, 2016-08-02 22:25:57]

lgtm

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGAttributeParser.cpp (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGAttributeParser.cpp:58: 
consumeExpectedToken ?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGAttributeParser.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGAttributeParser.h:38: 
fCurPos ?

// The current position in the input string 

?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGAttributeParser.h:40: 
everyone else uses a typedef here

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGRenderContext.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGRenderContext.h:19: public:
Can viewports ever be not based at the origin?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGRenderContext.h:22: enum class LengthType {
k prefixes ?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGShape.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGShape.h:19: 
Do we want to assert that this appendChild is never called ?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGTypes.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGTypes.h:16: public:
Make these two one line each ?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGTypes.h:48: 
single line this guy ?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGTypes.h:65: public:
single lines ?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGValue.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGValue.h:19: enum class Type {
k prefixes ?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGValue.h:38: 
typedef ?

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGValue.h:55: 
here too ?

[stephana, 2016-08-03 12:54:58]

On 2016/08/02 22:25:57, robertphillips wrote:
> lgtm
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> File experimental/svg/model/SkSVGAttributeParser.cpp (right):
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGAttributeParser.cpp:58: 
> consumeExpectedToken ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> File experimental/svg/model/SkSVGAttributeParser.h (right):
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGAttributeParser.h:38: 
> fCurPos ?
> 
> // The current position in the input string 
> 
> ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGAttributeParser.h:40: 
> everyone else uses a typedef here
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> File experimental/svg/model/SkSVGRenderContext.h (right):
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGRenderContext.h:19: public:
> Can viewports ever be not based at the origin?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGRenderContext.h:22: enum class LengthType {
> k prefixes ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> File experimental/svg/model/SkSVGShape.h (right):
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGShape.h:19: 
> Do we want to assert that this appendChild is never called ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> File experimental/svg/model/SkSVGTypes.h (right):
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGTypes.h:16: public:
> Make these two one line each ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGTypes.h:48: 
> single line this guy ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGTypes.h:65: public:
> single lines ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> File experimental/svg/model/SkSVGValue.h (right):
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGValue.h:19: enum class Type {
> k prefixes ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGValue.h:38: 
> typedef ?
> 
>
https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
> experimental/svg/model/SkSVGValue.h:55: 
> here too ?
lgtm 
Are there tests that show what it can parse at this point ?

[reed1, 2016-08-03 13:18:20]

perhaps we start adding (in a private storage) svg test cases, so ravi can help
us auto-test them (looking for regressions etc.)

lgtm

[f(malita), 2016-08-03 16:50:52]

On 2016/08/03 12:54:58, stephana wrote:
> Are there tests that show what it can parse at this point ?

At the moment just local testing with SampleApp and random SVGs.

We definitely need to start adding unit tests at some point, but it doesn't feel
like the APIs are fleshed out enough just yet :)

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGAttributeParser.cpp (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGAttributeParser.cpp:58: 
On 2016/08/02 22:25:56, robertphillips wrote:
> consumeExpectedToken ?

Done (parseExpectedStringToken for consistency with the other private methods,
also renamed to parseFooToken).

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGAttributeParser.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGAttributeParser.h:38: 
On 2016/08/02 22:25:56, robertphillips wrote:
> fCurPos ?
> 
> // The current position in the input string 
> 
> ?

Done.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGAttributeParser.h:40: 
On 2016/08/02 22:25:56, robertphillips wrote:
> everyone else uses a typedef here

I thought 'using' is preferred since c++11, but not sure whether the style guide
is updated.  Done.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGRenderContext.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGRenderContext.h:19: public:
On 2016/08/02 22:25:56, robertphillips wrote:
> Can viewports ever be not based at the origin?

Logically, yes, but I don't think the location matters for length resolution
purposes: viewports also establish new coordinate systems with the origin ==
viewport location.  Dependent lengths are resolved within this local coordinate
system, so for them the viewport location is always (0,0).

https://www.w3.org/TR/SVG/coords.html#EstablishingANewViewport

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGRenderContext.h:22: enum class LengthType {
On 2016/08/02 22:25:56, robertphillips wrote:
> k prefixes ?

Done.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGShape.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGShape.h:19: 
On 2016/08/02 22:25:56, robertphillips wrote:
> Do we want to assert that this appendChild is never called ?

Hmm, since this would be a public API it seems a bit harsh to crash if someone
attempts to append to the wrong node type.

But we should definitely signal the failure somehow - maybe add a result type at
some point?

Added an SkDebugf message for now.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGTypes.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGTypes.h:16: public:
On 2016/08/02 22:25:56, robertphillips wrote:
> Make these two one line each ?

Done.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGTypes.h:48: 
On 2016/08/02 22:25:56, robertphillips wrote:
> single line this guy ?

Done.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGTypes.h:65: public:
On 2016/08/02 22:25:56, robertphillips wrote:
> single lines ?

Done.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
File experimental/svg/model/SkSVGValue.h (right):

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGValue.h:19: enum class Type {
On 2016/08/02 22:25:56, robertphillips wrote:
> k prefixes ?

Done.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGValue.h:38: 
On 2016/08/02 22:25:56, robertphillips wrote:
> typedef ?

Done.

https://codereview.chromium.org/2202053002/diff/40001/experimental/svg/model/...
experimental/svg/model/SkSVGValue.h:55: 
On 2016/08/02 22:25:56, robertphillips wrote:
> here too ?

Done.

[f(malita), 2016-08-03 16:52:07]

The CQ bit was checked by fmalita@chromium.org

[f(malita), 2016-08-03 16:52:08]

The patchset sent to the CQ was uploaded after l-g-t-m from stephana@google.com,
robertphillips@google.com, reed@google.com
Link to the patchset: https://codereview.chromium.org/2202053002/#ps60001
(title: "review")

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

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-03 17:21:14]

Description was changed from

==========
[SVGDom] Initial SVGLength support

Mostly plumb new length types, but also introduce a stateful parser,
rect shape and named color support.

R=reed@google.com,robertphillips@google.com,stephana@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2202053002
==========

to

==========
[SVGDom] Initial SVGLength support

Mostly plumb new length types, but also introduce a stateful parser,
rect shape and named color support.

R=reed@google.com,robertphillips@google.com,stephana@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2202053002

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

[commit-bot: I haz the power, 2016-08-03 17:21:15]

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