Fixed from_matrix according to the suggestion in #32. #33

Merged
merged 2 commits into from May 5, 2014

Projects

None yet

2 participants

@ColdenCullen

No description provided.

@Dav1dde Dav1dde and 1 other commented on an outdated diff May 5, 2014
gl3n/linalg.d
if(trace > 0) {
- real s = 0.5 / sqrt(trace);
+ qt s = 0.5 / sqrt(trace + 1.0f);
@Dav1dde
Dav1dde May 5, 2014

This should still be a real, since sqrt returns real and qt might be float or double.
Same for the ones below.

@Dav1dde
Owner

Thanks your a saviour, I somehow forgot about #32. Also your code differs from the one in #32

--- #33       2014-05-05 11:33:09.207655600 +0200
+++ #32      2014-05-05 11:33:27.886734500 +0200
@@ -2,36 +2,36 @@
         Quaternion ret;

         auto mat = matrix.matrix;
-        qt trace = mat[0][0] + mat[1][1] + mat[2][2];
+        qt trace = mat[0][0] + mat[1][1] + mat[2][2] + 1.0;

         if(trace > 0) {
-            qt s = 0.5 / sqrt(trace + 1.0f);
+            real s = 0.5 / sqrt(trace);

             ret.w = to!qt(0.25 / s);
             ret.x = to!qt((mat[2][1] - mat[1][2]) * s);
             ret.y = to!qt((mat[0][2] - mat[2][0]) * s);
             ret.z = to!qt((mat[1][0] - mat[0][1]) * s);
         } else if((mat[0][0] > mat[1][1]) && (mat[0][0] > mat[2][2])) {
-            qt s = 2.0 * sqrt(1.0 + mat[0][0] - mat[1][1] - mat[2][2]);
+            real s = 2.0 * sqrt(1 + mat[0][0] - mat[1][1] - mat[2][2]);

-            ret.w = to!qt((mat[2][1] - mat[1][2]) / s);
-            ret.x = to!qt(0.25f * s);
+            ret.w = to!qt((mat[2][1] + mat[1][2]) / s);
+            ret.x = to!qt(0.5 / s);
             ret.y = to!qt((mat[0][1] + mat[1][0]) / s);
             ret.z = to!qt((mat[0][2] + mat[2][0]) / s);
         } else if(mat[1][1] > mat[2][2]) {
-            qt s = 2.0 * sqrt(1 + mat[1][1] - mat[0][0] - mat[2][2]);
+            real s = 2.0 * sqrt(1 + mat[1][1] - mat[0][0] - mat[2][2]);

-            ret.w = to!qt((mat[0][2] - mat[2][0]) / s);
+            ret.w = to!qt((mat[0][2] + mat[2][0]) / s);
             ret.x = to!qt((mat[0][1] + mat[1][0]) / s);
-            ret.y = to!qt(0.25f * s);
+            ret.y = to!qt(0.5 / s);
             ret.z = to!qt((mat[1][2] + mat[2][1]) / s);
         } else {
-            qt s = 2.0 * sqrt(1 + mat[2][2] - mat[0][0] - mat[1][1]);
+            real s = 2.0 * sqrt(1 + mat[2][2] - mat[0][0] - mat[1][1]);

-            ret.w = to!qt((mat[1][0] - mat[0][1]) / s);
+            ret.w = to!qt((mat[1][0] + mat[0][1]) / s);
             ret.x = to!qt((mat[0][2] + mat[2][0]) / s);
             ret.y = to!qt((mat[1][2] + mat[2][1]) / s);
-            ret.z = to!qt(0.25f * s);
+            ret.z = to!qt(0.5f / s);
         }

         return ret;
@ColdenCullen

I was using the code from this comment as my starting point, and it appears to work (at least, all tests now pass).

@Dav1dde
Owner

Not sure what I was thinking... Saw the wrong thing. Thank you very much,

@Dav1dde Dav1dde merged commit 7580a75 into Dav1dde:master May 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment