Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incomplete SVG path specfication #97

Open
respatialized opened this issue Feb 19, 2023 · 4 comments
Open

Incomplete SVG path specfication #97

respatialized opened this issue Feb 19, 2023 · 4 comments

Comments

@respatialized
Copy link

respatialized commented Feb 19, 2023

First, let me start off by congratulating you on the 1.0 release and thanking you for thi.ng/geom, which has been a consistent source of inspiration for me in my Clojure endeavors ever since I got started with the language seven (maybe more?) years back. I would be more than happy to contribute some time and effort back to a library I've found useful over the years.

Context

Right now, it is possible to parse a subset of SVG path strings using thi.ng.geom.path/parse-svg-path into geometry objects. The supported commands are absolute move (M), absolute line (L), absolute curve to (C), and close (Z). However, each of these except for close has an equivalent relative version (specified by lowercase letters mlc), and the specification mentions several other commands:

  • Hh - horizontal lines
  • Vv - vertical lines
  • Ss - smooth/shorthand cubic Bézier curve
  • Qq - quadratic Bézier curve
  • Tt - smooth/shorthand quadratic Bézier curve
  • Aa - elliptical arcs

In addition to this, the current regex-based implementation runs into errors on this subset of path commands, including inconsistent handling commas and whitespace between coordinates.

Examples

(thi.ng.geom.path/parse-svg-path "M275,175 v-150 a150,150 0 0,0 -150,150 z")
;; => ({:type :close, :points [[275.0 175.0] [275.0 175.0]]})
(thi.ng.geom.path/parse-svg-path "M200,300 Q400,50 600,300 T1000,300")
;; => nil

These paths should be equivalent, but they aren't:

(thi.ng.geom.path/parse-svg-path "M 10 80 C 40 10 65 10 95 80 S 150 150 180 80")
;; => ({:type :bezier, :points ([10.0 80.0] [40.0 10.0])})
(thi.ng.geom.path/parse-svg-path "M 10,80 C 40,10 65,10 95,80 S 150 150 180 80")
;; => ({:type :bezier, :points ([10.0 80.0] [40.0 10.0] [65.0 10.0] [95.0 80.0])})

Proposal

I am interested in better understanding the SVG spec and thi.ng/geom, so I would like to develop a PR that extends thi.ng.geom.types.Path2 to include all of the operations in the SVG path data specification. I will begin by adding tests of path definitions that currently cause incorrect parsing behavior and fix them by extending the Path2 geometry type and parser function to support all of them.

Once I have an implementation that I am confident in, I will submit it as a PR.

Questions

Parsing - grammar vs regex

I think that the most faithful way of ensuring consistent parser behavior would be to translate the BNF grammar of the SVG path data specification to an Instaparse grammar, but I do not know if you want to introduce another library as a dependency for this project. Instaparse itself does not have any transitive dependencies, but I am not sure whether that by itself outweighs the additional complexity of taking on a parsing library for thi.ng/geom. Speed is also another consideration; Instaparse may not be as fast as a regex-based approach. I do not have a strong opinion on the speed vs correctness tradeoff. I may experiment with using Instaparse as a dev-time dependency to generate test cases to get some of its guarantees while checking the updated regex implementation, which may not require it as a run-time dependency.

Testing

I plan to test this by including examples from the SVG spec. Is there a preferred way to do "visual" testing to compare the output of rendered geometry types to known examples?

@postspectacular
Copy link
Member

postspectacular commented Feb 20, 2023

Hi @respatialized - thank you so much for taking the time to create such a detailed issue & proposal to extend that current (half-baked) functionality! Most (if not all) of these things mentioned are already addressed by the newer TypeScript version of this library (not really the same, more of a rewrite) and should be pretty trivial to port to Clojure:

https://github.com/thi-ng/umbrella/blob/develop/packages/geom/src/path-from-svg.ts

That path parser implementation (~150 lines) too has no extra dependencies other than some constants, small utils and various vector ops (the latter should all be avail in the Clojure version too), all from the same thi.ng/umbrella monorepo:

I'd maybe start with this. The parser is currently undocumented (can prioritize that if it'd help you to get started), but it's been used in production since 2019 and was tested this with hundreds of SVG production files created by different tools (Illustrator, Inkscape, svgo optimizer, handwritten files, W3C)... FWIW I too started with a BNF, but had lots of issues dealing with most of the optimized SVG files and so ended up with this handwritten parser instead...

@respatialized
Copy link
Author

Great, thanks for pointing me in the right direction! I definitely have more than enough to go on now.

@respatialized
Copy link
Author

@postspectacular One question/clarification before I refine a concrete implementation further: the current implementation of parse-svg-coords is lazy, yielding a sequence of segments. For API compatibility, I assume this means that my new implementation should also be lazy, correct?

@respatialized
Copy link
Author

@postspectacular I have a draft PR that implements more complete parsing of SVG paths, but I think there are a few unresolved questions about scope that I would appreciate some thoughts on!

#98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants