Add PointArray.transform by analogy to Point.transform

[edemaine]

I recently had occasion to want to transform the points in an SVG.Polygon according to a transformation matrix. (The motivation is to "inline" transform attributes into the point coordinates themselves, for software that supports SVG but not the transform attribute.) SVG.Polygon.array reveals the SVG.PointArray that represents the coordinates, and SVG.Polygon.plot lets me put a transformed PointArray back into the polygon. What's missing is an SVG.PointArray.transform (essentially a loop over SVG.Point.transform) to transform the PointArray according to a given matrix.

This PR adds this functionality. If you like the proposed addition in principle, I can write tests and documentation.

[Fuzzyma]

Actually a cool feature. The question is if we should have it for PathArray, too. Seems a bit inconsistent to only have it on PointArray

[Fuzzyma]

comma needed!

[edemaine]

Fixed the missing comma, as well as a critical bug encountered while testing. Added a few simple tests.

Transforming a path would be a cool feature too, but I think it's not nearly as easy. In particular, transforming an arc (a/A) command according to an arbitrary matrix would be quite challenging -- perhaps not even possible? (I'm not sure whether an elliptical arc could transform to an hyperbolic arc...)

Anyway, it's inconsistent to have just SVG.Point.transform, so this at least corrects one such inconsistency. 😄 Maybe a future PR can target SVG.PathArray.transform.

[Fuzzyma]

I actually once created such a feature for paths. You can take a look at svg.screenbbox.js. As you already mentioned, it seems to be hard to transform arc commands. But then I figured, that I only transform the start and end points of arcs. Yes, the arc might not be correct because you cant really draw skewed arcs, but it was better than nothing^^.

To your implementation: I just saw, you used for...of loops. While this is not really a problem, it is anyway.
PointArray is an Array in modern browsers but its not in old Browsers. In old Browsers its an array-like object. As hard as it is, please use a normal for loop to make sure, it works in old Browsers, too.

For your inconsisteny argument: I dont really agree. But thats just my opinion^^. Its a useful feature, so why not...

[edemaine]

@Fuzzyma Thanks for pointing out the for...of issue; I'm not familiar with this codebase (I'm just a frequent user), so this is especially helpful. I've fixed that.

Fair enough, I agree that a PathArray.transform for just vertex coordinates would be useful even if it isn't correct in all cases (provided it's in the documentation). Would you like that addition in this PR or another one?

[edemaine]

Hmm, I also just realized that all the other similar methods of PointArray, like PointArray.move, are in-place modification operations (while also returning this), so PointArray.transform should probably also modify in-place?

On the other hand, Point.transform makes a copy of the point (which is why I made a copy transform), so I'm not totally sure which is best. Any preference?

[Fuzzyma]

No problem, it IS really a hack. If you take a look into ArrayPolyfill.js you will see how ugly that hack is. I even use the Function constructor. But I found it worth the effort because having native arrays where it is supported is awesome!

I would prefer to have it as a seperate PR. This one is fine already :)

[Fuzzyma]

Hmm, I also just realized that all the other similar methods of PointArray, like PointArray.move, are in-place modification operations (while also returning this), so PointArray.transform should probably also modify in-place?

Good question... I dont really have a preference because of your mentioned inconsistency. However, Matrix also returns a new one on transformation. So lets stay consistent here.
I see there is much to fix in v4.0 lol