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

path string parsing fails for "super"-optimized strings #1145

Closed
stranskyjan opened this issue Aug 14, 2020 · 11 comments
Closed

path string parsing fails for "super"-optimized strings #1145

stranskyjan opened this issue Aug 14, 2020 · 11 comments

Comments

@stranskyjan
Copy link

stranskyjan commented Aug 14, 2020

Consider a path string M2,0a2 2 0 00-2 2a2 2 0 002 2a.5.5 0 011 0z, which seems to be valid (browsers natively renders it correctly), but is "super"-optimized:

  • using 00 (instead of "standard" 0 0 with space) symbol for the arc flags.
  • using 0-2 (no delimiter in front of -)
  • using a.5.5 (no delimiter in front of decimal separator)

Using this in svg.js renders correctly, but path.array() method returns

[
   ["M", 2, 0],
   ["A", 2, 2, 0, 0, -2, 4, NaN],
   ["A", 2, 2, 0, 2, 2, NaN, NaN],
   ["A", 0.5, 0, 11, 0, NaN, NaN, NaN],
]

but should return

[
   ["M", 2, 0],
   ["A", 2, 2, 0, 0, 0, -2, 2],
   ["A", 2, 2, 0, 0, 0, 2, 2],
   ["A", 0.5, 0.5, 0, 0, 1, 1, 0],
   ["Z"],
]

Bug report based on this stackoverflow discussion

jsfiddle

@Fuzzyma
Copy link
Member

Fuzzyma commented Aug 14, 2020

Wow I didn't know that 00 is allowed. All other cases are covered. Good catch!

@stranskyjan
Copy link
Author

@Fuzzyma a.5.5 also seems to be a problem, resulting in ["A", 0.5, 0, ... but should be ["A", 0.5, 0.5, ...

@Fuzzyma
Copy link
Member

Fuzzyma commented Aug 14, 2020

strange, thought I caught that. But it was 5 years ago - so maybe my memory is just bad :D.
I guess at that point I just copy the solution from stackoverflow -.-

@Fuzzyma
Copy link
Member

Fuzzyma commented Aug 15, 2020

hm ok, because of the 011 part in the flags, we cannot use regex here anymore because no regex knows if 111 is a valid number or 3 flags glued together. Unfortunate... time to write a parser

@Fuzzyma
Copy link
Member

Fuzzyma commented Aug 15, 2020

@stranskyjan can 00 also occure in other path commands (for coodinates for example (e.g. M00)) or only for the flags in arc?
Is 11 also allowed as arc flags?

@thednp
Copy link

thednp commented Aug 15, 2020

@stranskyjan try this

@stranskyjan
Copy link
Author

@Fuzzyma according to browser test and the grammar for path data, M00 and similar is not allowed, as 00 is treated as a single number ([0-9])+, not a pair of coordinates. The flags are required to be either 0 or 1, so they can be "shrinked"

@Fuzzyma
Copy link
Member

Fuzzyma commented Aug 16, 2020

Cool, in that case I have a working parser now. However, I want to combine it with changing the coordinates to absolute so I don't have 2 loops hanging around :)

@Fuzzyma
Copy link
Member

Fuzzyma commented Sep 5, 2020

@stranskyjan just realized that your "correct" example is wrong. You test against absolute coordinates but use the relative coordinates in your example

@stranskyjan
Copy link
Author

@Fuzzyma true, sorry, I put wrong numbers. But the main idea that there are numbers instead of NaNs remains :-)

@Fuzzyma
Copy link
Member

Fuzzyma commented Sep 5, 2020

Yes, ofc! I just wondered why my parser put out the wrong values :D

Fuzzyma added a commit that referenced this issue Sep 5, 2020
@Fuzzyma Fuzzyma closed this as completed Sep 5, 2020
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

3 participants