search

benjamminf / warpjs

Warp, distort, bend, twist and smudge your SVG’s directly in the browser
https://benjamminf.github.io/warpjs
MIT License
488 stars 34 forks source link

Error handling failure with too few arguments in path command #17

Open bgmort opened 4 years ago

bgmort commented 4 years ago

There are two related issues here. One is a quick fix, and I'll send a pull request for it with tests. The second is more complicated, and I see that you've started a rewrite, so I'll just point it out for you to be aware of.

Apparently the A arc path's two flag arguments, which must be either 0 or 1, do not have to have any spaces after them. So, the following command: a 110 110 0 0 1 55 -7 could be written like this: a 110 110 0 0155-7. You'll get output like that from an optimizer like https://jakearchibald.github.io/svgomg/.

The current parser doesn't take that into account, and parses the latter command as having only 5 arguments. I couldn't find where it was described in the svg spec, but it's clearly widely supported as any SVG renderer I tried doesn't have a problem with it. <path d="M0 0a 100 100 0 00200-0z" /> would be a valid path to test.

Second, this would have been easier to diagnose, but the line of code that should spit out the error has a typo in it, so produces a reference error instead. I'll have a pull request ready to fix that shortly.

benjamminf commented 4 years ago

@bgmort thanks for finding and fixing! There's a lot of corners I cut, particularly with parsing, that I look back on now and wonder what I was thinking. I've merged your PR and will put out a new release this weekend. Really appreciate that you took the time to surface the issue and put out a fix for it :)

I'm actually looking at using a proper parser for the path in the rewrite. Either by forking https://github.com/hughsk/svg-path-parser/blob/master/grammar.peg (which looks like it does parse arc commands correctly) or writing my own, not sure yet. But I'll ensure I support exactly what the spec defines in https://www.w3.org/TR/SVG/paths.html

bgmort commented 4 years ago

No problem, thank you for the library! I'm a developer that does my own graphic design sometimes, and was disappointed to find that Affinity Designer has no support for warping text, but I was able to solve my problem using warp.js.