Merge pull request #10579 from AndrewMeadows/fix-generateBasisVectors

Fix generateBasisVectors() helper

libraries/animation/src/Rig.cpp

@@ -1135,9 +1135,9 @@ void Rig::updateEyeJoint(int index, const glm::vec3& modelTranslation, const glm
         }
 
         glm::vec3 headUp = headQuat * Vectors::UNIT_Y;
-        glm::vec3 z, y, x;
-        generateBasisVectors(lookAtVector, headUp, z, y, x);
-        glm::mat3 m(glm::cross(y, z), y, z);
+        glm::vec3 z, y, zCrossY;
+        generateBasisVectors(lookAtVector, headUp, z, y, zCrossY);
+        glm::mat3 m(-zCrossY, y, z);
         glm::quat desiredQuat = glm::normalize(glm::quat_cast(m));
 
         glm::quat deltaQuat = desiredQuat * glm::inverse(headQuat);

libraries/render-utils/src/AnimDebugDraw.cpp

@@ -260,7 +260,7 @@ static void addLink(const AnimPose& rootPose, const AnimPose& pose, const AnimPo
         // there is room, so lets draw a nice bone
 
         glm::vec3 uAxis, vAxis, wAxis;
-        generateBasisVectors(boneAxis0, glm::vec3(1, 0, 0), uAxis, vAxis, wAxis);
+        generateBasisVectors(boneAxis0, glm::vec3(1.0f, 0.0f, 0.0f), uAxis, vAxis, wAxis);
 
         glm::vec3 boneBaseCorners[NUM_BASE_CORNERS];
         boneBaseCorners[0] = pose0 * ((uAxis * radius) + (vAxis * radius) + (wAxis * radius));

libraries/shared/src/GLMHelpers.cpp

@@ -538,17 +538,16 @@ void generateBasisVectors(const glm::vec3& primaryAxis, const glm::vec3& seconda
     uAxisOut = glm::normalize(primaryAxis);
     glm::vec3 normSecondary = glm::normalize(secondaryAxis);
 
-    // if secondaryAxis is parallel with the primaryAxis, pick another axis.
+    // if normSecondary is parallel with the primaryAxis, pick another secondary.
     const float EPSILON = 1.0e-4f;
-    if (fabsf(fabsf(glm::dot(uAxisOut, secondaryAxis)) - 1.0f) > EPSILON) {
-        // pick a better secondaryAxis.
-        normSecondary = glm::vec3(1.0f, 0.0f, 0.0f);
-        if (fabsf(fabsf(glm::dot(uAxisOut, secondaryAxis)) - 1.0f) > EPSILON) {
-            normSecondary = glm::vec3(0.0f, 1.0f, 0.0f);
+    if (fabsf(fabsf(glm::dot(uAxisOut, normSecondary)) - 1.0f) < EPSILON) {
+        normSecondary = Vectors::UNIT_X;
+        if (fabsf(fabsf(glm::dot(uAxisOut, normSecondary)) - 1.0f) < EPSILON) {
+            normSecondary = Vectors::UNIT_Y;
         }
     }
 
-    wAxisOut = glm::normalize(glm::cross(uAxisOut, secondaryAxis));
+    wAxisOut = glm::normalize(glm::cross(uAxisOut, normSecondary));
     vAxisOut = glm::cross(wAxisOut, uAxisOut);
 }
 

tests/shared/src/GLMHelpersTests.cpp

@@ -140,3 +140,77 @@ void GLMHelpersTests::testSimd() {
     }
     qDebug() << "Done ";
 }
+
+void GLMHelpersTests::testGenerateBasisVectors() {
+    { // very simple case: primary along X, secondary is linear combination of X and Y
+        glm::vec3 u(1.0f, 0.0f, 0.0f);
+        glm::vec3 v(1.0f, 1.0f, 0.0f);
+        glm::vec3 w;
+
+        generateBasisVectors(u, v, u, v, w);
+
+        QCOMPARE_WITH_ABS_ERROR(u, Vectors::UNIT_X, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(v, Vectors::UNIT_Y, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(w, Vectors::UNIT_Z, EPSILON);
+    }
+
+    { // point primary along Y instead of X
+        glm::vec3 u(0.0f, 1.0f, 0.0f);
+        glm::vec3 v(1.0f, 1.0f, 0.0f);
+        glm::vec3 w;
+
+        generateBasisVectors(u, v, u, v, w);
+
+        QCOMPARE_WITH_ABS_ERROR(u, Vectors::UNIT_Y, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(v, Vectors::UNIT_X, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(w, -Vectors::UNIT_Z, EPSILON);
+    }
+
+    { // pass bad data (both vectors along Y).  The helper will guess X for secondary.
+        glm::vec3 u(0.0f, 1.0f, 0.0f);
+        glm::vec3 v(0.0f, 1.0f, 0.0f);
+        glm::vec3 w;
+
+        generateBasisVectors(u, v, u, v, w);
+
+        QCOMPARE_WITH_ABS_ERROR(u, Vectors::UNIT_Y, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(v, Vectors::UNIT_X, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(w, -Vectors::UNIT_Z, EPSILON);
+    }
+
+    { // pass bad data (both vectors along X).  The helper will guess X for secondary, fail, then guess Y.
+        glm::vec3 u(1.0f, 0.0f, 0.0f);
+        glm::vec3 v(1.0f, 0.0f, 0.0f);
+        glm::vec3 w;
+
+        generateBasisVectors(u, v, u, v, w);
+
+        QCOMPARE_WITH_ABS_ERROR(u, Vectors::UNIT_X, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(v, Vectors::UNIT_Y, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(w, Vectors::UNIT_Z, EPSILON);
+    }
+
+    { // general case for arbitrary rotation
+        float angle = 1.234f;
+        glm::vec3 axis = glm::normalize(glm::vec3(1.0f, 2.0f, 3.0f));
+        glm::quat rotation = glm::angleAxis(angle, axis);
+
+        // expected values
+        glm::vec3 x = rotation * Vectors::UNIT_X;
+        glm::vec3 y = rotation * Vectors::UNIT_Y;
+        glm::vec3 z = rotation * Vectors::UNIT_Z;
+
+        // primary is along x
+        // secondary is linear combination of x and y
+        // tertiary is unknown
+        glm::vec3 u  = 1.23f * x;
+        glm::vec3 v = 2.34f * x + 3.45f * y;
+        glm::vec3 w;
+
+        generateBasisVectors(u, v, u, v, w);
+
+        QCOMPARE_WITH_ABS_ERROR(u, x, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(v, y, EPSILON);
+        QCOMPARE_WITH_ABS_ERROR(w, z, EPSILON);
+    }
+}

tests/shared/src/GLMHelpersTests.h

@@ -21,6 +21,7 @@ private slots:
     void testEulerDecomposition();
     void testSixByteOrientationCompression();
     void testSimd();
+    void testGenerateBasisVectors();
 };
 
 float getErrorDifference(const float& a, const float& b);

tests/shared/src/StorageTests.cpp

@@ -8,6 +8,8 @@
 
 #include "StorageTests.h"
 
+#include <memory>
+
 QTEST_MAIN(StorageTests)
 
 using namespace storage;
@@ -32,8 +34,8 @@ void StorageTests::testConversion() {
         QFileInfo fileInfo(_testFile);
         QCOMPARE(fileInfo.exists(), false);
     }
-    StoragePointer storagePointer = std::make_unique<MemoryStorage>(_testData.size(), _testData.data());
-    QCOMPARE(storagePointer->size(), (quint64)_testData.size());
+    StoragePointer storagePointer = std::unique_ptr<MemoryStorage>(new MemoryStorage(_testData.size(), _testData.data()));
+    QCOMPARE(storagePointer->size(), _testData.size());
     QCOMPARE(memcmp(_testData.data(), storagePointer->data(), _testData.size()), 0);
     // Convert to a file
     storagePointer = storagePointer->toFileStorage(_testFile);
@@ -42,12 +44,12 @@ void StorageTests::testConversion() {
         QCOMPARE(fileInfo.exists(), true);
         QCOMPARE(fileInfo.size(), (qint64)_testData.size());
     }
-    QCOMPARE(storagePointer->size(), (quint64)_testData.size());
+    QCOMPARE(storagePointer->size(), _testData.size());
     QCOMPARE(memcmp(_testData.data(), storagePointer->data(), _testData.size()), 0);
 
     // Convert to memory
     storagePointer = storagePointer->toMemoryStorage();
-    QCOMPARE(storagePointer->size(), (quint64)_testData.size());
+    QCOMPARE(storagePointer->size(), _testData.size());
     QCOMPARE(memcmp(_testData.data(), storagePointer->data(), _testData.size()), 0);
     {
         // ensure the file is unaffected
@@ -58,13 +60,13 @@ void StorageTests::testConversion() {
 
     // truncate the data as a new memory object
     auto newSize = _testData.size() / 2;
-    storagePointer = std::make_unique<MemoryStorage>(newSize, storagePointer->data());
-    QCOMPARE(storagePointer->size(), (quint64)newSize);
+    storagePointer = std::unique_ptr<Storage>(new MemoryStorage(newSize, storagePointer->data()));
+    QCOMPARE(storagePointer->size(), newSize);
     QCOMPARE(memcmp(_testData.data(), storagePointer->data(), newSize), 0);
 
     // Convert back to file
     storagePointer = storagePointer->toFileStorage(_testFile);
-    QCOMPARE(storagePointer->size(), (quint64)newSize);
+    QCOMPARE(storagePointer->size(), newSize);
     QCOMPARE(memcmp(_testData.data(), storagePointer->data(), newSize), 0);
     {
         // ensure the file is truncated