-
Notifications
You must be signed in to change notification settings - Fork 79
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
Comments
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... |
Great, thanks for pointing me in the right direction! I definitely have more than enough to go on now. |
@postspectacular One question/clarification before I refine a concrete implementation further: the current implementation of |
@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! |
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 lettersmlc
), and the specification mentions several other commands:Hh
- horizontal linesVv
- vertical linesSs
- smooth/shorthand cubic Bézier curveQq
- quadratic Bézier curveTt
- smooth/shorthand quadratic Bézier curveAa
- elliptical arcsIn 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
These paths should be equivalent, but they aren't:
Proposal
I am interested in better understanding the SVG spec and
thi.ng/geom
, so I would like to develop a PR that extendsthi.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?
The text was updated successfully, but these errors were encountered: