From e4e4cb01103d5fe89852fd4fa75791e412d76681 Mon Sep 17 00:00:00 2001
From: "Anthony J. Thibault" <tony@highfidelity.io>
Date: Mon, 11 Jan 2016 12:03:44 -0800
Subject: [PATCH] Bug fix for Transform.postMult() with non-uniform scale.

Previously it would not flag the matrix as non-uniform, this would
cause some operations (such as inverse) to be incorrect.
---
 libraries/shared/src/Transform.h    | 13 +++---------
 tests/shared/src/TransformTests.cpp | 31 +++++++++++++++++++----------
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/libraries/shared/src/Transform.h b/libraries/shared/src/Transform.h
index ec81513f3e..310e2d0409 100644
--- a/libraries/shared/src/Transform.h
+++ b/libraries/shared/src/Transform.h
@@ -104,8 +104,6 @@ public:
     const Vec3& getScale() const;
     Transform& setScale(float scale);
     Transform& setScale(const Vec3& scale);  // [new this] = [this.translation] * [this.rotation] * [scale]
-    Transform& preScale(float scale);
-    Transform& preScale(const Vec3& scale);
     Transform& postScale(float scale);       // [new this] = [this] * [scale] equivalent to:glScale
     Transform& postScale(const Vec3& scale); // [new this] = [this] * [scale] equivalent to:glScale
 
@@ -350,14 +348,6 @@ inline Transform& Transform::setScale(const Vec3& scale) {
     return *this;
 }
 
-inline Transform& Transform::preScale(float scale) {
-    return setScale(getScale() * scale);
-}
-
-inline Transform& Transform::preScale(const Vec3& scale) {
-    return setScale(getScale() * scale);
-}
-
 inline Transform& Transform::postScale(float scale) {
     if (!isValidScale(scale) || scale == 1.0f) {
         return *this;
@@ -376,6 +366,9 @@ inline Transform& Transform::postScale(const Vec3& scale) {
         return *this;
     }
     invalidCache();
+    if ((scale.x != scale.y) || (scale.x != scale.z)) {
+        flagNonUniform();
+    }
     if (isScaling()) {
         _scale *= scale;
     } else {
diff --git a/tests/shared/src/TransformTests.cpp b/tests/shared/src/TransformTests.cpp
index b936d45555..ca55bbf873 100644
--- a/tests/shared/src/TransformTests.cpp
+++ b/tests/shared/src/TransformTests.cpp
@@ -11,19 +11,21 @@
 #include "TransformTests.h"
 
 #include <algorithm>
-#include <glm/glm.hpp>
 
 #include <SharedLogging.h>
-#include <Transform.h>
 
 #include "../QTestExtensions.h"
+#include <QtCore/QDebug>
+#include <Transform.h>
+#include <StreamUtils.h>
+#include <glm/glm.hpp>
 
-using namespace glm;
+//using namespace glm;
 
 const vec3 xAxis(1.0f, 0.0f, 0.0f);
 const vec3 yAxis(0.0f, 1.0f, 0.0f);
 const vec3 zAxis(0.0f, 0.0f, 1.0f);
-const quat rot90 = angleAxis((float)M_PI / 2.0f, yAxis);
+const quat rot90 = glm::angleAxis((float)M_PI / 2.0f, yAxis);
 
 QTEST_MAIN(TransformTests)
 
@@ -71,19 +73,26 @@ void TransformTests::getInverseMatrix() {
                        vec4( 0.0f, 1.0f, 0.0f, 0.0f),
                        vec4( 0.0f, 0.0f, 1.0f, 0.0f),
                        vec4( 0.0f, 0.0f, 0.0f, 1.0f));
-    const mat4 result_a = inverse(m * mirrorX);
+    const mat4 result_a = glm::inverse(m * mirrorX);
 
     Transform xform;
     xform.setTranslation(t);
     xform.setRotation(rot90);
-
-    //
-    // change postScale to preScale and the test will pass...
-    //
-
     xform.postScale(vec3(-1.0f, 1.0f, 1.0f));
+
     mat4 result_b;
     xform.getInverseMatrix(result_b);
 
-    QCOMPARE_WITH_ABS_ERROR(result_a, result_b, EPSILON);
+    // don't check elements directly, instead compare each axis transformed by the matrix.
+    auto xa = transformPoint(result_a, xAxis);
+    auto ya = transformPoint(result_a, yAxis);
+    auto za = transformPoint(result_a, zAxis);
+
+    auto xb = transformPoint(result_b, xAxis);
+    auto yb = transformPoint(result_b, yAxis);
+    auto zb = transformPoint(result_b, zAxis);
+
+    QCOMPARE_WITH_ABS_ERROR(xa, xb, EPSILON);
+    QCOMPARE_WITH_ABS_ERROR(ya, yb, EPSILON);
+    QCOMPARE_WITH_ABS_ERROR(za, zb, EPSILON);
 }