Alternative method signature for new Matrix()

[iamdustan]

I am reading attributes from an SVG string and passing it to the Bonsai runner.

With the relevant attribute for a Matrix from an svg node being transform='matrix(0,1,0,0,1,1)' I thought it may be useful to others to have this alternative method signature.

var matrix = new Matrix('matrix(0,1,0,0,1,1)'

Thoughts?

[iamdustan]

Not sure of the proper jsdoc style for doing alternative signatures.

Took this approach from http://usejsdoc.org/tags-param.html under the heading Multiple types

[davidaurelio]

The approach is good

[iamdustan]

Alternatively, is there a way with apply to currently pass in an array of values instead of listing them all separately?

Currently I have a line of code that looks like this:

opts.attr.matrix = new Matrix(opts.attr.matrix[0], opts.attr.matrix[1], opts.attr.matrix[2], opts.attr.matrix[3], opts.attr.matrix[4], opts.attr.matrix[5]);

I tried doing matrix = Matrix.apply(new Matrix, matrixArray) and a few variations, but all returned undefined as the result.

With this pull request I could delete some other code and just say opts.attr.matrix = new Matrix(opts.attr.matrix) where opts.attr.matrix === 'matrix(a,b,c,d,tx,ty)'.

I’m using the latest commit from github version of bonsai (I’m working on the bleeding edge rather than from the official releases)

[pvdz]

If not, you could try this:

Matrix.apply(new Matrix, '0,1,0,0,1,1'.split(','))

[iamdustan]

No worky. http://bit.ly/V7qtpZ

[davidaurelio]

We had some internal discussion, and the current mood is:

we’d prefer

new Matrix([a,b,c,d,x,y]);
new Matrix('a,b,c,d,x,y');

over

new Matrix('matrix(a,b,c,d,x,y)')

I hope that’s ok with you

[iamdustan]

The reasoning behind 'matrix(a,b,c,d,x,y)' is because that is how it is represented in the DOM and by the Matrix.prototype.toString() method. I like the array approach, but think that if the result of toString can’t be passed back in and work as is, perhaps it’s better not to accept a string argument at all.

[davidaurelio]

If not, you could try this:

Matrix.apply(new Matrix, '0,1,0,0,1,1'.split(','))

No worky. http://bit.ly/V7qtpZ

It needs to be

var matrix;
Matrix.apply(matrix = new Matrix, '0,1,0,0,1,1'.split(','));

[davidaurelio]

but think that if the result of toString can’t be passed back in and work as is, perhaps it’s better not to accept a string argument at all.

That’s a good point. Hm. I’d personally prefer something like Matrix.parse('matrix(1,2,3,4,5,6)') in that case.

[iamdustan]

and would Matrix.parse('matrix(1,2,3,4,5,6)') return a new Matrix instance with those values applied?

[iamdustan]

var matrix;
Matrix.apply(matrix = new Matrix, '0,1,0,0,1,1'.split(','));

Never would’ve known that. Any link to a javascript reference as to why that is the syntax for such a thing?

[davidaurelio]

and would Matrix.parse('matrix(1,2,3,4,5,6)') return a new Matrix instance with those values applied?

yes, like a second constructor. We just don’t have polymorphism based on signatures in JS. Would parse be explicit enough? Would fromString be better?

[pvdz]

:D I give workshops about that stuff

The short: constructors return a new instance by default. Apply calls a function as if you regularly called it, except it explicitly sets the context (new Matrix in this case) and passes on the elements of the array as if they were actual arguments to the call (see also call ;). So to map the array elements to the matrix constructor, we create a new matrix instance (with empty args) and pass that the context. Then we call Matrix with the proper args and the new matrix instance as context, which causes the constructor (which is a regular function) to initialize the instance. Of course, our Matrix doesn't return anything explicitly, so that's why my trick didn't work at first. If you assign the new instance immediately, the effect works :)

(yes, that's the short)

[davidaurelio]

var matrix;
Matrix.apply(matrix = new Matrix, '0,1,0,0,1,1'.split(','));

Never would’ve known that. Any link to a javascript reference as to why that is the syntax for such a thing?

The example is too weird.

var matrix = new Matrix();
Matrix.apply(matrix, '0,1,0,0,1,1'.split(','));

does the same thing.

The problem with Constructor.apply(new Constructor, […]) is that constructors usually don’t return. The new instance is created, altered, and effectively thrown away immediately

[iamdustan]

To the Constructor / apply / call discussion — I knew and understood the function/context part. I did not know or understand the difference in working with constructors.

So to map the array elements to the matrix constructor, we create a new matrix instance (with empty args) and pass that the context. Then we call Matrix with the proper args and the new matrix instance as context, which causes the constructor (which is a regular function) to initialize the instance.

This is what I thought I was doing with my initial code example. Apparently not :)

[iamdustan]

@davidaurelio

yes, like a second constructor. We just don’t have polymorphism based on signatures in JS. Would parse be explicit enough? Would fromString be better?

My initial thought is fromString() would be more intuitive, because I would expect parse() to return either an object or array of the values which I could then do other things with. I also would expect parse to be able to return the values from a Matrix instance or string.

[klipstein]

I would also prefer fromString() over parse().

[davidaurelio]

ok, cool. Seems we have a winner :)

[iamdustan]

Sweet. I’ll update.

End result of pull request will include the following two signatures and one additional method:

var matrix = new Matrix([a,b,c,d,e,f]);
var matrix = new Matrix(a, b, c, d, e, f);

var matrix = Matrix.fromString('a,b,c,d,e,f')
var matrix = Matrix.fromString('matrix(a,b,c,d,e,f)')

[davidaurelio]

great.

If you don’t really need new Matrix([a,b,c,d,e,f]), feel free to leave it out. Makes fromString easier, on the other hand.

[iamdustan]

Done.

[davidaurelio]

one last thing: could you use tools.isArray(a), please? And don’t forget to include the tools module ;-)

Background: The iframe runner might create sub-iframes for sub movies. Arrays from different frames might not pass this test.

Looks good otherwise. Thanks!

[iamdustan]

Check.

Good to know the background on that. 😄

[davidaurelio]

Assuming that “Check” meant “did it” – maybe you didn’t push?

[iamdustan]

Shoot! It seems I did forget to push. On the bus ride home now. Will comment again when pushed.

[davidaurelio]

Ok, I’m off for today. Will merge in the morning (CET)

[iamdustan]

For real done this time.

[nonken]

@iamdustan Thats great! Just one question, are the docs updated as well?

[nonken]

Just found out that there is no need for explanation in the overview part of the docs, so no update needed. Thx!