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

m3.multiply returns the premultiplied rather than postmultiplied result #96

Closed
bnham opened this issue Jan 26, 2021 · 3 comments
Closed

Comments

@bnham
Copy link

bnham commented Jan 26, 2021

Describe the bug
Here is the definition of m3.multiply in https://webgl2fundamentals.org/webgl/lessons/webgl-2d-matrices.html:


var m3 = {
  multiply: function(a, b) {
    var a00 = a[0 * 3 + 0];
    // ...
    var a12 = a[1 * 3 + 2];
    // ...
    var b22 = b[2 * 3 + 2];
 
    return [
      b00 * a00 + b01 * a10 + b02 * a20,
      b00 * a01 + b01 * a11 + b02 * a21,
      b00 * a02 + b01 * a12 + b02 * a22,
      b10 * a00 + b11 * a10 + b12 * a20,
      b10 * a01 + b11 * a11 + b12 * a21,
      b10 * a02 + b11 * a12 + b12 * a22,
      b20 * a00 + b21 * a10 + b22 * a20,
      b20 * a01 + b21 * a11 + b22 * a21,
      b20 * a02 + b21 * a12 + b22 * a22,
    ];
  }
}

From the code and variable names:

  • The matrices are defined as being row-major, as you'd expect in JS.
  • The result returned is actually BA (A premultiplied by B) rather than AB (A postmultiplied by B) as you would naively expect. Maybe this is what you intended, but it isn't documented anywhere in the code or called out in the prose.

Example:

let a = [1, 2, 3, 4, 5, 6, 7, 8, 9];
let b = [11, 12, 13, 14, 15, 16, 17, 18, 19];
console.log(m3.multiply(a, b));

// returns:
// [150,186,222,
//  186,231,276,
//  222,276,330]

vs.

>>> import numpy as np
>>> a = np.array([[1, 2, 3], [4, 5, 6], [7, 8, 9]])
>>> b = np.array([[11, 12, 13], [14, 15, 16], [17, 18, 19]])
>>> np.matmul(a, b)
array([[ 90,  96, 102],
       [216, 231, 246],
       [342, 366, 390]])
>>> np.matmul(b, a)
array([[150, 186, 222],
       [186, 231, 276],
       [222, 276, 330]])

The tricky thing is that the examples all work as written, because everything else in the code is written to handle this correctly. For instance, given this snippet from that webpage:

var matrix = m3.multiply(translationMatrix, rotationMatrix);
matrix = m3.multiply(matrix, scaleMatrix);

Let's simplify that snippet for notation purposes:

var M = m3.multiply(T, R);
M = m3.multiply(M, S);

From reading this naively, you would expect M = TRS. But given the way m3.multiply is implemented, you actually have M = SRT, which is actually the desired result, so the code works.

I say that M = SRT is the desired result because once you feed M into the fragment shader, WebGL transposes the matrix (since it treats the matrix as column major). So the frag shader is actually doing this:

position = M_transpose * a_position;

and if M = SRT, then M_transpose = T_transpose * R_transpose * S_transpose, so:

position = T_transpose * R_transpose * S_transpose * a_position;

which is actually correct: first the point is scaled, then it's rotated, and then it's translated; translation should be the last step since generally you want the rotate the model point around the model origin rather than the world origin.

I think this is part of what is contributing to the confusion in #33 and #84.

@greggman
Copy link
Member

greggman commented Jan 26, 2021

The code in the article is the same as glmatrix

https://github.com/toji/gl-matrix/blob/21b745f3a8b095c4a5304978fa8ab131b76a5f9e/src/mat3.js#L294

which is the same as glm

https://github.com/g-truc/glm

which is the same as opengl

https://www.khronos.org/registry/OpenGL-Refpages/gl2.1/xhtml/glMultMatrix.xml

It's not a bug. It's what pretty much every graphics library calls "multiply"

In glm (C++) your example of M = TRS is written as

matrix = translation * rotation * scale

The math applied is the same as in this article. The order of storage is the same as well, translation is in offsets 12,13,14 in the matrix (well, offsets 7,8 for a 3x3 matrix)

Also, the confusing part is WebGL consider rows to be columns as mentioned in the this article. No transposing is going on. Only how the data is interpreted

@bnham
Copy link
Author

bnham commented Jan 27, 2021

Yeah, I realize now that in multiply(a, b), both a and b are treated as column-major rather than row-major. That makes the result a*b (a postmultiplied by b) as expected.

For the next person that gets tripped up here, I was thrown off because the naming of the variables is different than what you'd expect when using standard math notation, e.g. this series of variables:

    var a00 = a[0 * 3 + 0];
    var a01 = a[0 * 3 + 1];
    var a02 = a[0 * 3 + 2];
    ...

Which I assumed referenced a matrix layout like this (row-major), because usually the first subscript denotes the row and the second subscript denotes the column in math. For instance, naively you might expect a02 to select the first row and the third column:

a00 a01 a02
a10 a11 a12
a20 a21 a22

But actually the variables in the code reference a matrix layout like this (column-major):

a00 a10 a20
a01 a11 a21
a02 a12 a22

I was also thrown off because the diagrams in the articles have the rows where columns would normally be and vice versa, as you wrote about here: https://webgl2fundamentals.org/webgl/lessons/webgl-matrix-vs-math.html

Anyway closing this and thanks for the response. Hopefully the comment helps the next person who gets confused.

@bnham bnham closed this as completed Jan 27, 2021
@greggman
Copy link
Member

greggman commented Jan 27, 2021

Thanks. If/when I write WebGPU articles I'll try to use more "math"y diagrams and explanations and point out the "rows are columns"

In another github issue I pointed out if you want to see the rows as columns you can write it like this

const xaxis = [xx, xy, xz, 0];
const yaxis = [yx, yy, yz, 0];
const zaxis = [zx, zy, zz, 0];
const trans = [tx, ty, tz, 1];

const matrix = [...xaxis, ...yaxis, ...zaxis, ...trans];

or maybe clearer

const column1 = [
  xx,
  xy,
  xz,
  0,
];
const column2 = [
  yx,
  yy,
  yz,
  0,
];
const column3 = [
  zx,
  zy,
  zz,
  0,
];
const column4 = [
  tx,
  ty,
  tz,
  1,
];

const matrix = [...column1, ...column2, ...column3, ...column4];

which if you squint can look like columns 😅

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