svgdotjs / svg.js

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

Something wrong with relative transform #1142

Closed moayuisuda closed 3 years ago

moayuisuda commented 4 years ago

I'm following the code example below to transform relatively:

element.transform({ rotate: 125 }).transform({ rotate: 37.5, relative: true })
// or
element.transform({ rotate: 125 }).transform({ rotate: 37.5 }, true)

But the code only work with element.transform({ rotate: 125 }).transform({ rotate: 37.5 }, true) element.transform({ rotate: 125 }).transform({ rotate: 37.5, relative: true }) will transform absolutely codepen

Fuzzyma commented 4 years ago

If this is true, this is a bug. However, your codepen is not working for me!

edemaine commented 3 years ago

I just noticed this bug (relative: true has no effect in first argument to transform) while reading the source code. Specifically, the following line looks at the second argument (relative) but does not look at o.relative as claimed in the documentation:

https://github.com/svgdotjs/svg.js/blob/4524c20b48abe08e69c25a7382a289592d147496/src/modules/optional/transform.js#L67-L68

Actually, the documentation for transform() as setter also mentions that relative can be a point-like object, and new Matrix follows this claim:

https://github.com/svgdotjs/svg.js/blob/4524c20b48abe08e69c25a7382a289592d147496/src/types/Matrix.js#L61-L63

Given this conflict, I wonder if the documentation should be changed, to remove the claim that relative: true works, and instead encourage use of the second argument...

Fuzzyma commented 3 years ago

Good catch. Yes, we introduced a different relative-thing for transforms without noticing the overlap. I will remove it from the docs since it just doesn't work