search

svgdotjs / svg.js

The lightweight library for manipulating and animating SVG
https://svgjs.dev
Other
11.04k stars 1.08k forks source link

Path.array() cannot read the data if the PathCommands have shortened sign-separated coordinates #1165

Closed adaliszk closed 1 year ago

adaliszk commented 3 years ago

Bug report

Path.array() fails to parse the data into PathCommand correctly when I work with an SVG document that has + signs instead of , signs between the coordinates. There is a workaround to go through all of the places and replace the problematic character, but that is very expensive with large documents and it should not be necessary.

Fiddle

https://jsfiddle.net/vmq83e6z/2/

Explanation

UPDATE:

Further investigating why is it working within the browser it turns out that this is an intended hidden perk of the data description because the spec defines that coordinates can have signs between them in order to have negative coordinates. They allow to put the positive end there, and they also allow to shorten the path command description by removing the white-space characters which are in this case defined to be command and other characters like #x9 #x20 #xA #xC #xD

So the whole story is that a command's coordinate can be defined as follows:
https://www.w3.org/TR/SVG/paths.html#PathDataBNF

coordinate_pair::= coordinate comma_wsp? coordinate
coordinate::= sign? number

sign::= "+"|"-"
number ::= ([0-9])+
comma_wsp::=(wsp+ ","? wsp*) | ("," wsp*)
wsp ::= (#x9 | #x20 | #xA | #xC | #xD)

and you can remove the wsp and the comma_wsp if there is a command or a sign to separate the attributes (https://www.w3.org/TR/SVG/paths.html#PathDataGeneralInformation)

This whole set then makes it possible to define the path data like any of the lines:

Fuzzyma commented 3 years ago

Please show me the part of the spec which allows plus signs in path data because that is completely new to me :D

BrianHanechak commented 3 years ago

I hadn't seen this before either. It's not that "+" is a divider, it's that it's a sign.. from https://www.w3.org/TR/SVG/paths.html ...

coordinate::= sign? number

sign::= "+"|"-"

So, M+5+5 is just as legal as M5 5 or M5,5. However M+5+-5 would probably not be legal, because I think it would be interpreted as three coordinates, 5, 0, and -5. (This is based on my reading of the paragraph just below the grammar.)

adaliszk commented 3 years ago

@Fuzzyma, I will have to go deep in the if you really want that, but the fact is that the format works in your main platform the browser and I would assume that the browsers did implement the spec and that kind of way to describe a path is just a hidden perk somewhere in the spec just like @BrianHanechak described.

And yes, the M+5+-5 is not working, it does not even have to be a double sign. I'll update the title because I was wrong about divider, it's actually a sign that allows you to not put a divider character between the coordinates, thanks for the clarification Brian!

adaliszk commented 3 years ago

One thing that caught my eye now that technically the comma separation should be not the default by the spec either but it is working because they define the comma as a whitespace character alongside the space:

https://www.w3.org/TR/SVG/paths.html#PathDataBNF

coordinate_pair::= coordinate comma_wsp? coordinate
coordinate::= sign? number

sign::= "+"|"-"
number ::= ([0-9])+
comma_wsp::=(wsp+ ","? wsp*) | ("," wsp*)
wsp ::= (#x9 | #x20 | #xA | #xC | #xD)

They also specify that you can shorten the data by removing the white spaces if there is a clear separation point between the commands in: https://www.w3.org/TR/SVG/paths.html#PathDataGeneralInformation

adaliszk commented 3 years ago

@Fuzzyma I've updated the description with the spec related parts, I hope it will clear things out for you in order to fix it.

Fuzzyma commented 3 years ago

Thanks for diving into this. Will fix it as soon as I find some time

Fuzzyma commented 1 year ago

I am incredibly sorry that it took 3 years. Just pushed the fix

adaliszk commented 1 year ago

No need to apologize; I could have opened a PR with it, but since I've moved on from my role where I used the library, I have not done so.

Fuzzyma commented 2 months ago

I am sorry - again, I somehow forgot to publish all the fixes I made. I just released it: https://github.com/svgdotjs/svg.js/releases/tag/3.2.1