testcase and fix for issue #1388 (Quaternion javadoc is misleading) (#1451)

* testcase and fix for issue #1388 (Quaternion javadoc is misleading)

* privatize 2 methods to please Codacy

* Quaternion: distinguish intrinsic-vs-extrinsic rotation order in cmts

* TestIssue1388: verify intrinsic rotation order

jme3-core/src/main/java/com/jme3/math/Quaternion.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009-2020 jMonkeyEngine
+ * Copyright (c) 2009-2021 jMonkeyEngine
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -185,12 +185,13 @@ public final class Quaternion implements Savable, Cloneable, java.io.Serializabl
     }
 
     /**
-     * Constructor instantiates a new <code>Quaternion</code> object from a
-     * collection of rotation angles.
+     * Instantiate a new <code>Quaternion</code> from Tait-Bryan angles,
+     * applying the rotations in x-z-y extrinsic order or y-z'-x" intrinsic
+     * order.
      *
-     * @param angles
-     *            the angles of rotation (x, y, z) that will define the
-     *            <code>Quaternion</code>.
+     * @param angles an array of Tait-Bryan angles (in radians, exactly 3
+     * elements, the X angle in angles[0], the Y angle in angles[1], and the Z
+     * angle in angles[2], unaffected)
      */
     public Quaternion(float[] angles) {
         fromAngles(angles);
@@ -244,11 +245,13 @@ public final class Quaternion implements Savable, Cloneable, java.io.Serializabl
     }
 
     /**
-     * <code>fromAngles</code> builds a quaternion from the Euler rotation
-     * angles (x,y,z) aka (pitch, yaw, roll).
+     * Reconfigure this <code>Quaternion</code> based on Tait-Bryan angles,
+     * applying the rotations in x-z-y extrinsic order or y-z'-x" intrinsic
+     * order.
      *
-     * @param angles
-     *            the Euler angles of rotation (in radians).
+     * @param angles an array of Tait-Bryan angles (in radians, exactly 3
+     * elements, the X angle in angles[0], the Y angle in angles[1], and the Z
+     * angle in angles[2], unaffected)
      * @return this
      */
     public Quaternion fromAngles(float[] angles) {
@@ -261,22 +264,15 @@ public final class Quaternion implements Savable, Cloneable, java.io.Serializabl
     }
 
     /**
-     * <code>fromAngles</code> builds a Quaternion from the Euler rotation
-     * angles (x,y,z) aka (pitch, yaw, roll)).
-     * Note that we are applying in order: (y, x, z) aka (yaw, pitch, roll)
-     * but we've ordered them in x, y, and z for convenience.
+     * Reconfigure this Quaternion based on Tait-Bryan rotations, applying the
+     * rotations in x-z-y extrinsic order or y-z'-x" intrinsic order.
      *
-     * @see <a href="http://www.euclideanspace.com/maths/geometry/rotations/conversions/eulerToQuaternion/index.htm">http://www.euclideanspace.com/maths/geometry/rotations/conversions/eulerToQuaternion/index.htm</a>
+     * @see
+     * <a href="http://www.euclideanspace.com/maths/geometry/rotations/conversions/eulerToQuaternion/index.htm">http://www.euclideanspace.com/maths/geometry/rotations/conversions/eulerToQuaternion/index.htm</a>
      *
-     * @param xAngle
-     *            the Euler pitch of rotation (in radians). (aka Attitude, often rot
-     *            around x)
-     * @param yAngle
-     *            the Euler yaw of rotation (in radians). (aka Heading, often
-     *            rot around y)
-     * @param zAngle
-     *            the Euler roll of rotation (in radians). (aka Bank, often
-     *            rot around z)
+     * @param xAngle the X angle (in radians)
+     * @param yAngle the Y angle (in radians)
+     * @param zAngle the Z angle (in radians)
      * @return this
      */
     public Quaternion fromAngles(float xAngle, float yAngle, float zAngle) {
@@ -308,16 +304,20 @@ public final class Quaternion implements Savable, Cloneable, java.io.Serializabl
     }
 
     /**
-     * <code>toAngles</code> returns this quaternion converted to Euler rotation
-     * angles (x,y,z) aka (pitch, yaw, roll).
+     * Convert this <code>Quaternion</code> to Tait-Bryan angles, to be applied
+     * in x-z-y intrinsic order or y-z'-x" extrinsic order, for instance by
+     * {@link #fromAngles(float[])}.
      *
-     * Note that the result is not always 100% accurate due to the implications of euler angles.
-     * @see <a href="http://www.euclideanspace.com/maths/geometry/rotations/conversions/quaternionToEuler/index.htm">http://www.euclideanspace.com/maths/geometry/rotations/conversions/quaternionToEuler/index.htm</a>
+     * Note that the result is not always 100% accurate due to the implications
+     * of Tait-Bryan angles.
      *
-     * @param angles
-     *            the float[] in which the angles should be stored, or null if
-     *            you want a new float[] to be created
-     * @return the float[] in which the angles are stored.
+     * @see
+     * <a href="http://www.euclideanspace.com/maths/geometry/rotations/conversions/quaternionToEuler/index.htm">http://www.euclideanspace.com/maths/geometry/rotations/conversions/quaternionToEuler/index.htm</a>
+     *
+     * @param angles an array of 3 floats in which to store the result, or else
+     * null (If null, a new array will be allocated.)
+     * @return an array of 3 angles (in radians, the X angle in angles[0], the Y
+     * angle in angles[1], and the Z angle in angles[2])
      */
     public float[] toAngles(float[] angles) {
         if (angles == null) {

jme3-core/src/test/java/com/jme3/math/TestIssue1388.java

@@ -0,0 +1,126 @@
+/*
+ * Copyright (c) 2020-2021 jMonkeyEngine
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of 'jMonkeyEngine' nor the names of its contributors
+ *   may be used to endorse or promote products derived from this software
+ *   without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package com.jme3.math;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Verify the order in which Tait-Bryan angles are applied by the Quaternion
+ * class. This was issue #1388 at GitHub.
+ *
+ * @author Stephen Gold
+ */
+public class TestIssue1388 {
+
+    @Test
+    public void testIssue1388() {
+        Vector3f in = new Vector3f(4f, 6f, 9f); // test vector, never modified
+        Vector3f saveIn = in.clone();
+        /*
+         * Three arbitrary rotation angles between -PI/2 and +PI/2
+         */
+        final float xAngle = 1.23f;
+        final float yAngle = 0.765f;
+        final float zAngle = -0.456f;
+        float[] angles = new float[]{xAngle, yAngle, zAngle};
+        float[] saveAngles = new float[]{xAngle, yAngle, zAngle};
+        /*
+         * Part 1: verify that the extrinsic rotation order is x-z-y
+         *
+         * Apply extrinsic rotations to the "in" vector in x-z-y order.
+         */
+        Quaternion qx = new Quaternion().fromAngleAxis(xAngle, Vector3f.UNIT_X);
+        Quaternion qy = new Quaternion().fromAngleAxis(yAngle, Vector3f.UNIT_Y);
+        Quaternion qz = new Quaternion().fromAngleAxis(zAngle, Vector3f.UNIT_Z);
+        Vector3f outXZY = qx.mult(in);
+        qz.mult(outXZY, outXZY);
+        qy.mult(outXZY, outXZY);
+        /*
+         * Construct a Quaternion using fromAngles(float, float, float),
+         * use it to rotate the "in" vector, and compare.
+         */
+        Quaternion q1 = new Quaternion().fromAngles(xAngle, yAngle, zAngle);
+        Vector3f out1 = q1.mult(in);
+        assertEquals(outXZY, out1, 1e-5f);
+        /*
+         * Construct a Quaternion using fromAngles(float[]),
+         * use it to rotate the "in" vector, and compare.
+         */
+        Quaternion q2 = new Quaternion().fromAngles(angles);
+        Vector3f out2 = q2.mult(in);
+        assertEquals(outXZY, out2, 1e-5f);
+        /*
+         * Construct a Quaternion using only the constructor,
+         * use it to rotate the "in" vector, and compare.
+         */
+        Quaternion q3 = new Quaternion(angles);
+        Vector3f out3 = q3.mult(in);
+        assertEquals(outXZY, out3, 1e-5f);
+        /*
+         * Verify that fromAngles() reverses toAngles() for the chosen angles.
+         */
+        float[] out4 = q1.toAngles(null);
+        assertEquals(angles, out4, 1e-5f);
+        float[] out5 = q2.toAngles(null);
+        assertEquals(angles, out5, 1e-5f);
+        float[] out6 = q3.toAngles(null);
+        assertEquals(angles, out6, 1e-5f);
+        /*
+         * Part 2: verify intrinsic rotation order
+         *
+         * Apply intrinsic rotations to the "in" vector in y-z'-x" order.
+         */
+        Quaternion q4 = qy.mult(qz).mult(qx);
+        Vector3f out7 = q4.mult(in);
+        assertEquals(outXZY, out7, 1e-5f);
+        /*
+         * Verify that the values of "saveAngles" and "in" haven't changed.
+         */
+        assertEquals(saveAngles, angles, 0f);
+        assertEquals(saveIn, in, 0f);
+    }
+
+    private void assertEquals(float[] expected, float[] actual,
+            float tolerance) {
+        Assert.assertEquals(expected[0], actual[0], tolerance);
+        Assert.assertEquals(expected[1], actual[1], tolerance);
+        Assert.assertEquals(expected[2], actual[2], tolerance);
+    }
+
+    private void assertEquals(Vector3f expected, Vector3f actual,
+            float tolerance) {
+        Assert.assertEquals(expected.x, actual.x, tolerance);
+        Assert.assertEquals(expected.y, actual.y, tolerance);
+        Assert.assertEquals(expected.z, actual.z, tolerance);
+    }
+}