search

gecko0307 / dlib

Allocators, I/O streams, math, geometry, image and audio processing for D
https://gecko0307.github.io/dlib
Boost Software License 1.0
217 stars 33 forks source link

Rewrite Matrix with column-major order #16

Closed gecko0307 closed 10 years ago

gecko0307 commented 10 years ago

...to gain better compatibility with OpenGL and address multiplication-related issues mentioned in #14.

gecko0307 commented 10 years ago

The other possible way is to keep row-major order, but define two matrix multiplication operators: the ordinary one (*) and "reverse" one (for example, tilda: ~). Row-majors are still possible in OpenGL, since trans(A) * trans(B) = trans(B * A) So we have A ~ B = trans(A) * trans(B) as it actually is in the current opMul implementation - it does column-by-row product instead of row-by-column.

minexew commented 10 years ago

Would that be all it takes? Either way, such operation should probably have a descriptive name rather than just use an arbitrary operator. My vote would still go to taking the leap (and aiming for GLSL-compatible semantics like I mentioned before), but please, if you decide not to change this, at least put a big warning somewhere in the class :) I could probably wrap my head around having to multiply in the reverse order, after all, if that's really all it takes.

gecko0307 commented 10 years ago

OK, I decided to rewrite. This would be consistent with the principle of least surprise, after all.

minexew commented 10 years ago

Yo, if you want me to help with any code related to this, just point me to the line numbers

gecko0307 commented 10 years ago

Thanks, I'm gonna do it myself :)

gecko0307 commented 10 years ago

Would it be reasonable to start square bracket indexing in opIndex(i, j) from 1 rather than 0? This would conform standard mathematical notation.

minexew commented 10 years ago

I don't think any other library in a C-style language does this, and neither should we :)

gecko0307 commented 10 years ago

OK. The most part is ready, dlib.math.affine is on the go.

gecko0307 commented 10 years ago

Just to make sure. With new matrix code, the unittest that you committed for #14 gives [0, 0, -0.577418, 1.41421] as the resulting vector. Is this correct?

minexew commented 10 years ago

Can't guarantee it's correct, but it's the same value as given by the old matrix code which works for me. You swapped the matrices in the multiplication, right?

gecko0307 commented 10 years ago

No. It turns out swapping is not needed. Nor was needed with the old code, due to "reversed" multiplication.