ungerik / go3d

A performance oriented 2D/3D math package for Go
MIT License
310 stars 49 forks source link

Extension/bugfixes on my fork #3

Closed panmari closed 10 years ago

panmari commented 10 years ago

Hey there!

I'm using a fork of your package for a ray tracer I'm implementing right now. In the process of developing it, I found some bugs, wrote some tests and implemented additional stuff (determinant of a mat4, inverse of a mat4...) Would you be interested in merging some of the changes? I must admit though that my tests/commits are not too clean...

see https://github.com/panmari/go3d

ungerik commented 10 years ago

Sure, I would be grateful for all bug fixes.

-Erik

On Thu, Jul 3, 2014 at 4:00 PM, panmari notifications@github.com wrote:

Hey there!

I'm using a fork of your package for a ray tracer I'm implementing right now. In the process of developing it, I found some bugs, wrote some tests and implemented additional stuff (determinant of a mat4, inverse of a mat4...) Would you be interested in merging some of the changes? I must admit though that my tests/commits are not too clean...

— Reply to this email directly or view it on GitHub https://github.com/ungerik/go3d/issues/3.

+43 650 58 33055 erik@erikunger.com Skype: ungerik http://erikunger.com http://github.com/ungerik http://twitter.com/ungerik http://facebook.com/ungerik http://www.linkedin.com/in/ungerik http://www.xing.com/profile/Erik_Unger2

panmari commented 10 years ago

One thing before I make a pull request:

Your scale method for matrices does seem to scale only the diagonal entries, which is pretty unusual (eg. java vecmath multiplies all entries). May i adapt the behaviour of this method to scale all values? On Jul 3, 2014 5:48 PM, "Erik Unger" notifications@github.com wrote:

Sure, I would be grateful for all bug fixes.

-Erik

On Thu, Jul 3, 2014 at 4:00 PM, panmari notifications@github.com wrote:

Hey there!

I'm using a fork of your package for a ray tracer I'm implementing right now. In the process of developing it, I found some bugs, wrote some tests and implemented additional stuff (determinant of a mat4, inverse of a mat4...) Would you be interested in merging some of the changes? I must admit though that my tests/commits are not too clean...

— Reply to this email directly or view it on GitHub https://github.com/ungerik/go3d/issues/3.

+43 650 58 33055 erik@erikunger.com Skype: ungerik http://erikunger.com http://github.com/ungerik http://twitter.com/ungerik http://facebook.com/ungerik http://www.linkedin.com/in/ungerik http://www.xing.com/profile/Erik_Unger2

— Reply to this email directly or view it on GitHub https://github.com/ungerik/go3d/issues/3#issuecomment-47947369.

ungerik commented 10 years ago

On Thu, Jul 3, 2014 at 6:08 PM, panmari notifications@github.com wrote:

Your scale method for matrices does seem to scale only the diagonal entries, which is pretty unusual

I don't think so. See http://mathforum.org/mathimages/index.php/Transformation_Matrix#Scaling

It has also always worked for me.

-Erik

+43 650 58 33055 erik@erikunger.com Skype: ungerik http://erikunger.com http://github.com/ungerik http://twitter.com/ungerik http://facebook.com/ungerik http://www.linkedin.com/in/ungerik http://www.xing.com/profile/Erik_Unger2

panmari commented 10 years ago

Fair enough. I created a new method (Mul) for multiplying a matrix with a scalar. What is your stance about tests? I did some unit testing/benchmarking for my stuff, but haven't added it in the pull request