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

Inconsistent transform functions #33

Closed
thednp opened this issue May 11, 2019 · 3 comments
Closed

Inconsistent transform functions #33

thednp opened this issue May 11, 2019 · 3 comments

Comments

@thednp
Copy link

thednp commented May 11, 2019

I noticed using transform functions in other order different to the spec, the script crashes at line 296, something related to toFixed(). Here is a regular use case:

var SvgPath = require(svgpath);
var pathData = 'ANY_VALID_PATH_DATA';

var newData = new SvgPath(pathData).scale(1,1.5).translate(0,150).round(3).toString();

Just to be clear, I have some knowledge on SVG, as you will know in a sec, I'm going to try and briefly explain my best what's wrong and hopefully offer a solution.

According to the spec, in short, the SVG coordinate system expects us to compute path/transform coordinates in a specific order, translate, rotate (rotate/skew), scale, despite some people who would argue otherwise. I mean we all want to have full control over these coordinates despite the severe inconsistencies with CSS and other specific issues, we all want to have consistent coordinate values.

Only when working with the transform attribute it's actually recommended to use translate after rotation for instance when you want to rotate a shape around a certain coordinate (transform origin), take for instance rotate the shape around the center of the parent SVG itself.

<svg viewBox="0 0 100 100">
  <path d="SOME_VALID_PATH" transform="translate(50,50) rotate(45) translate(-50,-50)">
</svg>

Back to our issue, I would suggest to advise developers with your documentation to always use methods in this order: 1 translate, 2a rotate and/or 2b skew, 3 scale, 4 round, 5 toString.

As for you, I suggest to focus on consistency. Allow the transform methods to be used in any order, but build an object to store transform functions and compute them always in the order I mentioned above. I mean what's the point of using toString/round in the middle?

Have a look at my SVG plugin, see how I apply transform functions in animation and see how consistent looks in the demo. As you can imagine I've updated the code in my development (with transform functions order) and I never gotten any crashes.

Let me know if you like what you see, if I find some time I can give a hand.

@puzrin
Copy link
Member

puzrin commented May 11, 2019

I suggest to focus on consistency. Allow the transform methods to be used in any order, but build an object to store transform functions and compute them always in the order I mentioned above.

IMHO that's not acceptable. We implement pure math lib, it can not know better than users, what he needs.

If you use it to apply html properties, and those have additional ordering requiremets - that's related to outer system (may be properties parser), not to this lib.

@puzrin puzrin closed this as completed May 11, 2019
@thednp
Copy link
Author

thednp commented May 11, 2019

IMHO that's not acceptable. We implement pure math lib, it can not know better than users, what he needs.

OK I don't want to paste links from w3c here, but both transform functions and path commands use the exact same coordinate system as drafted, something that math lab doesn't care about.

@puzrin
Copy link
Member

puzrin commented May 11, 2019

There are no need to paste such links because this library is not about w3c. It's a low level lib, and it applies operations exactly in the same order as been requested.

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