Matrix4.Inverse fails to calculate the inverse of almost any matrix

Description

Matrix4's Inverse function is severely defective. It will fail to calculate the inverse for almost any matrix passed to it.

I don't have time to supply at patch for the fix, but here is what goes wrong and how to fix it.

Matrix4.Inverse uses the Adjoint method to calculate the inverse, however, Matrix4.Adjoint has a bug, which prevents it from calculating the adjoint matrix correctly.

Adjoint uses the method 'Minor' to produce matrices where one row and one column has been removed. It does this in a 4x4 matrix, where row 4 and column 4 are left as zero vectors. It then proceeds to calculate the full 4x4 matrix determinant on that reduced matrix.

This - of course - yields zero, since the reduced - minor - matrix has a dimensionality of 3. It should instead use Determinant3x3, which was clearly designed for this purpose.

I.e.
line 109 in Matrix4 needs to be changed from:
adjointMatrix[i,j] = (float)(Math.Pow(-1, i + j) * ((Minor(matrix, i, j)).Determinant()));
to:
adjointMatrix[i,j] = (float)(Math.Pow(-1, i + j) * ((Minor(matrix, i, j)).Determinant3x3()));

Unfortunately this isn't sufficient, since there is a bug in 'Determinant3x3' as well. I doubt it will produce the correct determinant for any matrix 3x3 with a dimensionality of 3.

In line 165 of the same file the statement:
{{ float diag2 = M12 * M32 * M31;}}
needs to be changed to:
{{ float diag2 = M12 * M23 * M31;}}

Given that this bug must have been in there for quite a while, I doubt many people use this feature.

Steps to Reproduce

Write a unit test:
Assert(Matrix4.Identity == Matrix4.Inverse(Matrix.Identity));

That unit test will fail, since the 'Inverse' method fails to calculate the correct inverse.

Activity

Show:
Frederick Martian
July 3, 2015, 12:11 PM

This patch adds some simple unit tests for Matrix4 to the unit test framework, Fixes the aforementioned bugs and also fixes a stack overrun error in the Equals() evalution where the == operator uses the Equals() method and vice versa.

Assignee

Unassigned

Reporter

Anton Lauridsen

Severity

High

Environment

All

Fixed in Revision

None

Components

Affects versions

Priority

Minor
Configure