Permalink
Browse files

Fix rotation matrix decomposition.

Summary:
This PR fixes an issue with rotation decomposition matrix on android.

The issue can be illustrated with this sample code https://snack.expo.io/r1SHEJpVb

It surfaces when we have non-zero rotation in Y or X axis and when rotation Z is greater than 90deg or less than -90deg. In that case the decomposition code doesn't give a valid output and as a result the view gets rotated by 180deg in Z axis.

You may want to run the code linked above on android and iOS to see the difference. Basically the example app renders first image rotated only by 89deg and the next one by 91deg. As a result you should see the second view being pivoted just slightly more than the first image. Apparently on android the second image is completely flipped:

iOS:
![screen shot 2017-07-07 at 12 40 30](https://user-images.githubusercontent.com/726445/27954719-7cf6d02c-6311-11e7-9104-5c3cc8e9b9c1.png)

Android:
![screen shot 2017-07-07 at 12 41 21](https://user-images.githubusercontent.com/726445/27954737-981f57e8-6311-11e7-8c72-af1824426c30.png)

The bug seemed to be caused by the code that decomposes the matrix into axis angles. It seems like that whole code has been overly complicated and we've been converting matrix first into quaternion just to extract angles. Whereas it is sufficient to extract angles directly from rotation matrix as described here: http://nghiaho.com/?page_id=846

This formula produces way simpler code and also gives correct result in the aforementioned case, so I decided not to debug quaternion code any further.

sidenote: New formula's y angle output range is now -90 to 90deg hence changes in tests.
Closes #14888

Reviewed By: astreet

Differential Revision: D5414006

Pulled By: shergin

fbshipit-source-id: 2e0a68cf4b2a9e32f10f6bfff2d484867a337fa3
  • Loading branch information...
kmagiera authored and facebook-github-bot committed Jul 18, 2017
1 parent c678f9c commit b60a8dc6b90bd607353d376a962f6df05239614e
@@ -12,7 +12,6 @@
public static class MatrixDecompositionContext {
double[] perspective = new double[4];
double[] quaternion = new double[4];
double[] scale = new double[3];
double[] skew = new double[3];
double[] translation = new double[3];
@@ -65,7 +64,6 @@ public static void decomposeMatrix(double[] transformMatrix, MatrixDecomposition
// output values
final double[] perspective = ctx.perspective;
final double[] quaternion = ctx.quaternion;
final double[] scale = ctx.scale;
final double[] skew = ctx.skew;
final double[] translation = ctx.translation;
@@ -170,35 +168,11 @@ public static void decomposeMatrix(double[] transformMatrix, MatrixDecomposition
}
// Now, get the rotations out
quaternion[0] =
0.5 * Math.sqrt(Math.max(1 + row[0][0] - row[1][1] - row[2][2], 0));
quaternion[1] =
0.5 * Math.sqrt(Math.max(1 - row[0][0] + row[1][1] - row[2][2], 0));
quaternion[2] =
0.5 * Math.sqrt(Math.max(1 - row[0][0] - row[1][1] + row[2][2], 0));
quaternion[3] =
0.5 * Math.sqrt(Math.max(1 + row[0][0] + row[1][1] + row[2][2], 0));
if (row[2][1] > row[1][2]) {
quaternion[0] = -quaternion[0];
}
if (row[0][2] > row[2][0]) {
quaternion[1] = -quaternion[1];
}
if (row[1][0] > row[0][1]) {
quaternion[2] = -quaternion[2];
}
// correct for occasional, weird Euler synonyms for 2d rotation
if (quaternion[0] < 0.001 && quaternion[0] >= 0 &&
quaternion[1] < 0.001 && quaternion[1] >= 0) {
// this is a 2d rotation on the z-axis
rotationDegrees[0] = rotationDegrees[1] = 0d;
rotationDegrees[2] = roundTo3Places(Math.atan2(row[0][1], row[0][0]) * 180 / Math.PI);
} else {
quaternionToDegreesXYZ(quaternion, rotationDegrees);
}
// Based on: http://nghiaho.com/?page_id=846
double conv = 180 / Math.PI;
rotationDegrees[0] = roundTo3Places(-Math.atan2(row[2][1], row[2][2]) * conv);
rotationDegrees[1] = roundTo3Places(-Math.atan2(-row[2][0], Math.sqrt(row[2][1] * row[2][1] + row[2][2] * row[2][2])) * conv);
rotationDegrees[2] = roundTo3Places(-Math.atan2(row[1][0], row[0][0]) * conv);
}
public static double determinant(double[] matrix) {
@@ -334,50 +308,6 @@ public static double v3Dot(double[] a, double[] b) {
};
}
/**
* Based on:
* http://www.euclideanspace.com/maths/geometry/rotations/conversions/quaternionToEuler/
* and:
* http://quat.zachbennett.com/
*
* Note that this rounds degrees to the thousandth of a degree, due to
* floating point errors in the creation of the quaternion.
*
* Also note that this expects the qw value to be last, not first.
*
* Also, when researching this, remember that:
* yaw === heading === z-axis
* pitch === elevation/attitude === y-axis
* roll === bank === x-axis
*/
public static void quaternionToDegreesXYZ(double[] q, double[] result) {
double qx = q[0], qy = q[1], qz = q[2], qw = q[3];
double qw2 = qw * qw;
double qx2 = qx * qx;
double qy2 = qy * qy;
double qz2 = qz * qz;
double test = qx * qy + qz * qw;
double unit = qw2 + qx2 + qy2 + qz2;
double conv = 180 / Math.PI;
if (test > 0.49999 * unit) {
result[0] = 0;
result[1] = 2 * Math.atan2(qx, qw) * conv;
result[2] = 90;
return;
}
if (test < -0.49999 * unit) {
result[0] = 0;
result[1] = -2 * Math.atan2(qx, qw) * conv;
result[2] = -90;
return;
}
result[0] = roundTo3Places(Math.atan2(2 * qx * qw - 2 * qy * qz, 1 - 2 * qx2 - 2 * qz2) * conv);
result[1] = roundTo3Places(Math.atan2(2 * qy * qw - 2 * qx * qz, 1 - 2 * qy2 - 2 * qz2) * conv);
result[2] = roundTo3Places(Math.asin(2 * qx * qy + 2 * qz * qw) * conv);
}
public static double roundTo3Places(double n) {
return Math.round(n * 1000d) * 0.001;
}
@@ -4,7 +4,7 @@ rn_robolectric_test(
name = "uimanager",
# TODO Disabled temporarily until Yoga linking is fixed t14964130
# srcs = glob(['**/*.java']),
srcs = ["SimpleViewPropertyTest.java"],
srcs = ["SimpleViewPropertyTest.java", "MatrixMathHelperTest.java"],
# Please change the contact to the oncall of your team
contacts = ["oncall+fbandroid_sheriff@xmail.facebook.com"],
visibility = [
@@ -36,6 +36,20 @@ private void verifyXRotatedMatrix(double degrees, double rotX, double rotY, doub
assertThat(ctx.rotationDegrees).containsSequence(rotX, rotY, rotZ);
}
private void verifyRotatedMatrix(double degreesX, double degreesY, double degreesZ, double rotX, double rotY, double rotZ) {
MatrixMathHelper.MatrixDecompositionContext ctx =
new MatrixMathHelper.MatrixDecompositionContext();
double[] matrixX = createRotateX(degreesToRadians(degreesX));
double[] matrixY = createRotateY(degreesToRadians(degreesY));
double[] matrixZ = createRotateZ(degreesToRadians(degreesZ));
double[] matrix = MatrixMathHelper.createIdentityMatrix();
MatrixMathHelper.multiplyInto(matrix, matrix, matrixX);
MatrixMathHelper.multiplyInto(matrix, matrix, matrixY);
MatrixMathHelper.multiplyInto(matrix, matrix, matrixZ);
MatrixMathHelper.decomposeMatrix(matrix, ctx);
assertThat(ctx.rotationDegrees).containsSequence(rotX, rotY, rotZ);
}
@Test
public void testDecomposing4x4MatrixToProduceAccurateZaxisAngles() {
@@ -82,17 +96,17 @@ public void testDecomposing4x4MatrixToProduceAccurateZaxisAngles() {
@Test
public void testDecomposing4x4MatrixToProduceAccurateYaxisAngles() {
double[] angles = new double[]{30, 45, 60, 75, 90, 100, 110, 120, 133, 167};
double[] angles = new double[]{30, 45, 60, 75, 90};
for (double angle : angles) {
verifyYRotatedMatrix(angle, 0d, angle, 0d);
verifyYRotatedMatrix(-angle, 0d, -angle, 0d);
}
// all values are between 0 and 180;
// all values are between -90 and 90;
// change of sign and direction in the third and fourth quadrant
verifyYRotatedMatrix(222, 0d, -138d, 0d);
verifyYRotatedMatrix(222, -180d, -42d, -180d);
verifyYRotatedMatrix(270, 0d, -90d, 0d);
verifyYRotatedMatrix(270, -180d, -90d, -180d);
verifyYRotatedMatrix(360, 0d, 0d, 0d);
}
@@ -114,6 +128,13 @@ public void testDecomposing4x4MatrixToProduceAccurateXaxisAngles() {
verifyXRotatedMatrix(360, 0d, 0d, 0d);
}
@Test
public void testDecomposingComplex4x4MatrixToProduceAccurateAngles() {
verifyRotatedMatrix(10, -80, 0, 10, -80, 0);
// x and y will filp
verifyRotatedMatrix(10, -95, 0, -170, -85, -180);
}
private static double degreesToRadians(double degrees) {
return degrees * Math.PI / 180;
}

0 comments on commit b60a8dc

Please sign in to comment.