mjs513 / monoxna

Automatically exported from code.google.com/p/monoxna
Other
0 stars 0 forks source link

Implement the Matrix class #27

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Implement the Matrix class.

More details coming as the problems arises..

Original issue reported on code.google.com by xeethra...@gmail.com on 29 Apr 2009 at 5:41

GoogleCodeExporter commented 9 years ago
-Implemented Inverse()
-Implemented Divide()
-Fixed Multiply()

Original comment by lav...@gmail.com on 19 Sep 2009 at 2:58

GoogleCodeExporter commented 9 years ago
started work on the NUnit tests

Original comment by lav...@gmail.com on 20 Sep 2009 at 9:26

GoogleCodeExporter commented 9 years ago
I've added Matrix.CreateWorld() implementation and tests

Original comment by lav...@gmail.com on 11 Jan 2010 at 11:57

GoogleCodeExporter commented 9 years ago
This patch implements:
* CreateFromQuaternion
* CreateLookAt
* CreatePerspectiveFieldOfView

There's a TODO where an ArgumentException should be thrown.
There are no tests yet.

Original comment by saltyho...@gmail.com on 5 Mar 2010 at 4:10

Attachments:

GoogleCodeExporter commented 9 years ago
Had a small error in the previous patch.

Original comment by saltyho...@gmail.com on 5 Mar 2010 at 4:18

Attachments:

GoogleCodeExporter commented 9 years ago
these changes look good enough, but the tests need to be implemented as well. 
If you
don't provide them, I will need to write them myself before including the 
patch. 

Original comment by lav...@gmail.com on 18 Mar 2010 at 7:21

GoogleCodeExporter commented 9 years ago
Applied Matrix.patch and added NUnit tests. The methods implemented in
the patch did not pass the tests. Now fixed. 

CreatePerspectiveFieldOfView now throws exceptions.

Original comment by magne.gertner on 26 May 2010 at 7:54

GoogleCodeExporter commented 9 years ago
Signed this issue over to magne.gertner.

Original comment by lav...@gmail.com on 26 May 2010 at 8:49

GoogleCodeExporter commented 9 years ago
Here is a patch with tests that should implement:
* CreateOrthographic
* CreateOrthographicOffCenter

Original comment by jrh2...@gmail.com on 3 Aug 2010 at 1:49

Attachments:

GoogleCodeExporter commented 9 years ago
I would suggest to take look at SlimMath project, its MIT license and have nice 
math oprations, take look here http://code.google.com/p/slimmath/ . there is 
already full Matrix class.

Hope this helps,

Amer Koleci

Original comment by amerkol...@gmail.com on 27 Aug 2010 at 6:49

GoogleCodeExporter commented 9 years ago
This patch fixes the Decompose method, which was giving incorrect results. The 
fix is based on SlimMath.

Original comment by andres.traks on 19 May 2011 at 11:04

Attachments:

GoogleCodeExporter commented 9 years ago
The patch has been applied, but I haven't been able to test it yet.

Original comment by lav...@gmail.com on 21 May 2011 at 9:26

GoogleCodeExporter commented 9 years ago
Here is a simple test case:
float d90 = (float)System.Math.PI / 2;
Matrix a = Matrix.CreateRotationY(d90) * Matrix.CreateRotationZ(d90);
Vector3 scale, trans;
Quaternion rot;
a.Decompose(out scale, out rot, out trans);

Quaternion rot2 = Quaternion.CreateFromYawPitchRoll(0, d90, d90);

if (rot.ToString() != rot2.ToString())
{
    System.Diagnostics.Debugger.Break();
}

Original comment by andres.traks on 21 May 2011 at 1:31

GoogleCodeExporter commented 9 years ago
Could you implement this test in the unit test?

Original comment by lav...@gmail.com on 21 May 2011 at 2:17

GoogleCodeExporter commented 9 years ago
Yes, I should have done that right away :)

It seems like a lot of the tests are failing even though the results are the 
same as the expected values. Is it because no margin of floating-point error is 
allowed? Should TestHelper.Approximate handle this?

Original comment by andres.traks on 21 May 2011 at 3:54

Attachments:

GoogleCodeExporter commented 9 years ago
Yes, that was the idea at least. I'm not a big fan of the way the approximation 
is done.

Original comment by lav...@gmail.com on 21 May 2011 at 4:54

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
This is a separate concern, and thus a separate comment.

The Matrix class should be heavily micro-optimized, including such procedures 
as manually inlining the static method calls into the instance method calls 
(i.e., myMatrix.Multiply should NOT call Matrix.Multiply, and the op_Multiply 
should call neither of the two). This also means that Matrix.Divide should be 
manually inlined because the generated IL for the Matrix.Multiply method is 
over 32 bytes - that is, over the maximum size of IL code that is inlined by 
the JITter. Essentially, don't optimize for the code cache, optimize for 
runtime speed (since there should be nothing in the Matrix class that has 
branching).
Obviously, this should not break anything and should instead make everything a 
few percentage points faster. Profiling on several computers would demonstrate 
whether or not the intended optimization worked or did not.

Original comment by gurwinde...@gmail.com on 30 Nov 2011 at 3:58

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Alright, done.

Changes:
- Optimized basic Matrix math (add, subtract, scalar multiply, matrix multiply, 
scalar divide, inequality comparison)
- Optimized basic Vector2/3/4 math (cross product, reflect, distance and length 
calculation, inequality comparison)
    - Fails the Vector3 Clamp unit test at test #6 since #6 is a "wicked" clamp - the min and max arguments are passed backwards. Compensating for this case involves an extra "if" check in the MathHelper.Clamp function and would significantly slow down the function.
- Optimized MathHelper functions
    - Eliminated implicit casts to double in CatmullRom interpolation (basically, it said 0.5 instead of 0.5f and so on)

Recommended Changes:
- MathHelper.ToRadians and MathHelper.ToDegrees: Lose the double precision. 
Marginal benefit.
- MathHelper.WrapAngle: I suppose that this is supposed to, well, give you an 
angle between 0 and TwoPi radians. Erm... why not use a modulo and then an if 
check? You lose the expensive casting and a branch. Mind you, modulo is not a 
terribly fast instruction, but it's faster than calling a method with two casts.
    angle = angle % TwoPi;
    return angle < 0.0f ? angle + TwoPi : angle;

Suggestions for the Future:
- Implement your own Math.Sqrt function that uses floats instead of doubles. 
Save on the expensive casts. Add it to MathHelper or whatever.
    - You'd have to profile this change and see if it made a difference, since System.Math.Sqrt is actually just an extern.
- For Vector2/3/4.Transform(VectorN[] sourceArray, ref Matrix matrix, Vector2[] 
destinationArray): Erm... am I missing something here? It seems obvious to just 
loop and transform each Vector.
    - Also, inline the Transform overload calls as soon as you finalize the implementations
- For Vector2/3/4.Transform(VectorN value, Quaternion rotation): Why not use 
the Matrix.CreateFromQuaternion method to create a Matrix and then called 
VectorN.Transform(Vector2, Matrix)? It's slow, but at least it runs.
- Various other functions can be altered so they don't create new VectorN or 
Matrix or whatever and instead recycle the arguments (which are copied on the 
stack and so are safe to reuse)

Original comment by gurwinde...@gmail.com on 30 Nov 2011 at 6:02

Attachments:

GoogleCodeExporter commented 9 years ago
Sorry for the late reply. I've imported your patch and will test it later 
today. 

Original comment by lav...@gmail.com on 7 Dec 2011 at 8:33

GoogleCodeExporter commented 9 years ago
Ok, so I've finally gotten around to testing your patch. I've had some problems 
with my development setup, which I didn't have the time to look into before 
yesterday, so sorry again for the late reply.

The patch looks fine. I'll commit it later today (for real this time).

Your input regarding optimization also sounds sound.

Thanks

PS If you'd like commit rights, just send me an email.

Original comment by lav...@gmail.com on 22 Dec 2011 at 5:54

GoogleCodeExporter commented 9 years ago
Hello; sorry it's been so long since I last posted. I've spent a fair bit of 
time profiling different Math.Sqrt substitutes, from the Id fast inverse square 
root to a C# specific one; however, they all perform more slowly than 
(float)Math.Sqrt((double)myNum) [given that floating point promotion is cheap 
in a register, and the number will be in an x87 register anyhow for the sqrt, 
the casting has minimal cost]. I've also done a good deal of the work for 
recycling struct arguments passed on the stack. I'll post a patch soon after 
running it through some additional unit tests (to ensure that the passed Vector 
is not modified in the functions).

I'll ask for commit rights after this patch is finished, when I feel I can 
contribute more time to the project. Thank you!

Original comment by gurwinde...@gmail.com on 22 Mar 2012 at 3:04