Re: change matrix represenation about gama

[gilbox]

I believe this fixes the bug I mentioned, but still need to update UTs.

[honzabrecka]

I changed the signature as you proposed.

I don't think it is a bug - matrices are multiplied in b * a order, which may be confusing. I changed it to a * b to make it clear. Does the subtract function behave as you expected? Should it be changed to a - b too? What do you think?

[gilbox]

Interesting, I hadn't even thought of that. I agree that a * b and a - b do seem more intuitive.

However, I am not using those functions directly. I have a small app which does a series of transformations with gama. Some of those transformations (translateMatrix, scaleMatrix) use multiplyMatrix internally. When I upgraded to this commit my app broke. However, when I made this change it fixed it.

I guess I'm trying to figure out if gama's matrix transformations were incorrect before, but correct now.

[honzabrecka]

I know what you mean, it's expected BC break - transformations were correct all the time, but the matrix representation has changed (from columns to rows). Here's the proof:

function multiply(a, b) {
  return [
    a[0] * b[0] + a[1] * b[2],
    a[0] * b[1] + a[1] * b[3],
    a[2] * b[0] + a[3] * b[2],
    a[2] * b[1] + a[3] * b[3]
  ];
}

// then
console.log(multiply([1, 3, 2, 4], [5, 7, 6, 8]));// -> [23, 31, 34, 46]
console.log(multiply([5, 7, 6, 8], [1, 3, 2, 4]));// -> [19, 43, 22, 50]

// now
console.log(multiply([1, 2, 3, 4], [5, 6, 7, 8]));// -> [19, 22, 43, 50]
console.log(multiply([5, 6, 7, 8], [1, 2, 3, 4]));// -> [23, 34, 31, 46]

Anyway, gama has more intuitive API now, which outweights introduced BC break. Try to update your app to the latest changes. :)

[gilbox]

Sure, I don't mind updating my app at all. However, my expectation was that certain operations would not require any change. For example, consider the following file (index.js):

const gama = require('gama');
const {Point} = gama;
const R = require('ramda');

const m = gama.EmptyMatrix();
const point = Point(2,3);

const xf1 = gama.translateMatrix(Point(5,10));
const xf2 = gama.scaleMatrix(Point(3,3));
const xf3 = gama.translateMatrix(Point(3,9));

demo(xf1);
demo(xf2, xf1);
demo(xf3, xf2, xf1);

function demo(...xf) {
  const matrix = R.compose(...xf)(m);
  const p = gama.transformPoint(matrix, point);
  console.log(p);
}

Now I execute the file like this, (this is copy+pasted from my terminal):

gbirman@gama-demo$ npm install gama
[email protected] node_modules/gama
└── [email protected]
gbirman@gama-demo$ babel-node ./
{ x: 7, y: 13 }
{ x: 11, y: 19 }
{ x: 20, y: 46 }
gbirman@gama-demo$ npm install https://github.com/honzabrecka/gama.git
[email protected] node_modules/gama
└── [email protected]
gbirman@gama-demo$ babel-node ./
{ x: 7, y: 13 }
{ x: 21, y: 39 }
{ x: 24, y: 48 }

Is that what you expected?

[honzabrecka]

You were right, there was a bug which is actually fixed: 2 + 5 = 7; 7 * 3 = 21; 21 + 3 = 24

Fixed version will be released soon.