dwmkerr / glmnet

GlmNet is a .NET version of the excellent OpenGL Mathematics library (GLM).
MIT License
53 stars 20 forks source link

Matrix Multiplication is not associative #5

Closed Diramorius closed 5 years ago

Diramorius commented 8 years ago

vec4 case1 = translationM * rotationM * projectionMatrix * new vec4(0,1,0,1); vec4 case2 = projectionMatrix * (translationM * (rotationM * new vec4(0,1,0,1)));

It is very surprising to find that these two method produces the same outcome. What's is the multiplication rules of glmNet? it does not seems associative.

if i were to place brackets around case1, the answer will be different.

CamecaGeoffS commented 8 years ago

I believe this is the correct code for the "mat4" class.

public static mat4 operator * (mat4 lhs, mat4 rhs)
        {
            return new mat4(new []
            {
                rhs[0][0] * lhs[0] + rhs[0][1] * lhs[1] + rhs[0][2] * lhs[2] + rhs[0][3] * lhs[3],
        rhs[1][0] * lhs[0] + rhs[1][1] * lhs[1] + rhs[1][2] * lhs[2] + rhs[1][3] * lhs[3],
        rhs[2][0] * lhs[0] + rhs[2][1] * lhs[1] + rhs[2][2] * lhs[2] + rhs[2][3] * lhs[3],
        rhs[3][0] * lhs[0] + rhs[3][1] * lhs[1] + rhs[3][2] * lhs[2] + rhs[3][3] * lhs[3]
            });
        }
CamecaGeoffS commented 8 years ago

Here is a patch with my changes based on the current HEAD (0125dd6). 0001-Partial-Fix-for-Issue-5-Matrix-Multiplication-is-not.zip

Diramorius commented 8 years ago

Great. its working fine now. I can now multiply the projection and modelview matrix first and sending it as a full transform matrix to the vertex shader. Thanks!

ejannink commented 7 years ago

I found the multiplication to transopose the result. I came to the same solution CamecaGeoffS suggested and hope the sources can be updated to save others some time.

Phoenix138 commented 5 years ago

Hi Dave, rather than starting a new thread I think this seems to be the same issue I had mentioned over email. The brief summary is when I attempt to modify your ModernOpenGL sample project from your SharpGL repository by replacing the individual matrices with a single matrix constructed with:

var mvp = projectionMatrix viewMatrix modelMatrix; shaderProgram.SetUniformMatrix4(gl, "mvp", mvp.to_array());

The onscreen results differ from what I get if I just let the shader perform the multiplication itself.

Also likely related: if I call glm.project followed immediately by glm.unproject the value returned from unproject does not match the original input to project.

dwmkerr commented 5 years ago

Sorry for the delay on this - I only realised I wasn't watching updates on this project till I got the comment from @Phoenix138!

OK this fix is in, and published as 0.6.0, available now!

I've also fully updated the build tooling to use AppVeyor, greatly simplifying the build and release process. Codecov.io has also been integrated. @Phoenix138 - this should fix your issue, I'll update GlmNet in SharpGL now!

Phoenix138 commented 5 years ago

Terrific! Thanks again Dave!

On Fri, Apr 26, 2019 at 7:20 AM Dave Kerr notifications@github.com wrote:

Sorry for the delay on this - I only realised I wasn't watching updates on this project till I got the comment from @Phoenix138 https://github.com/Phoenix138!

OK this fix is in, and published as 0.6.0, available now!

I've also fully updated the build tooling to use AppVeyor, greatly simplifying the build and release process. Codecov.io has also been integrated. @Phoenix138 https://github.com/Phoenix138 - this should fix your issue, I'll update GlmNet in SharpGL now!

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/dwmkerr/glmnet/issues/5#issuecomment-487074384, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7WYYK7DEZPDZ5QZS2TOY3PSMFTBANCNFSM4B27YP2Q .

dwmkerr commented 5 years ago

No problems! I've also updated the core SharpGL project to use the latest GlmNet. BTW did it solve your issue?

Phoenix138 commented 5 years ago

I haven't had a chance to run a full test yet but so far it looks good. Thanks again!

On Thu, May 2, 2019 at 9:08 AM Dave Kerr notifications@github.com wrote:

No problems! I've also updated the core SharpGL project to use the latest GlmNet. BTW did it solve your issue?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dwmkerr/glmnet/issues/5#issuecomment-488733634, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7WYYLN3ASDZYII2JUZ5LDPTMGXDANCNFSM4B27YP2Q .