Fix tilt by using custom normalize impl to avoid strange skia normali…

…ze behavior (flutter#6106)
TheDauntless · Aug 29, 2018 · c765bee · c765bee
1 parent 9fde6e0
commit c765bee
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 5 deletions.
diff --git a/flow/matrix_decomposition.cc b/flow/matrix_decomposition.cc
@@ -28,6 +28,14 @@ static inline SkVector3 SkVector3Cross(const SkVector3& a, const SkVector3& b) {
 MatrixDecomposition::MatrixDecomposition(const SkMatrix& matrix)
     : MatrixDecomposition(SkMatrix44{matrix}) {}
 
+// TODO(garyq): use skia row[x].normalize() when skia fixes it
+static inline void SkVector3Normalize(SkVector3& v) {
+  float mag = sqrt(v.fX * v.fX + v.fY * v.fY + v.fZ * v.fZ);
+  v.fX /= mag;
+  v.fY /= mag;
+  v.fZ /= mag;
+}
+
 MatrixDecomposition::MatrixDecomposition(SkMatrix44 matrix) : valid_(false) {
   if (matrix.get(3, 3) == 0) {
     return;
@@ -82,13 +90,16 @@ MatrixDecomposition::MatrixDecomposition(SkMatrix44 matrix) : valid_(false) {
   }
 
   scale_.fX = row[0].length();
-  row[0].normalize();
+
+  SkVector3Normalize(row[0]);
 
   shear_.fX = row[0].dot(row[1]);
   row[1] = SkVector3Combine(row[1], 1.0, row[0], -shear_.fX);
 
   scale_.fY = row[1].length();
-  row[1].normalize();
+
+  SkVector3Normalize(row[1]);
+
   shear_.fX /= scale_.fY;
 
   shear_.fY = row[0].dot(row[2]);
@@ -97,7 +108,8 @@ MatrixDecomposition::MatrixDecomposition(SkMatrix44 matrix) : valid_(false) {
   row[2] = SkVector3Combine(row[2], 1.0, row[1], -shear_.fZ);
 
   scale_.fZ = row[2].length();
-  row[2].normalize();
+
+  SkVector3Normalize(row[2]);
 
   shear_.fY /= scale_.fZ;
   shear_.fZ /= scale_.fZ;

diff --git a/flow/matrix_decomposition_unittests.cc b/flow/matrix_decomposition_unittests.cc
@@ -94,19 +94,63 @@ TEST(MatrixDecomposition, Combination) {
   ASSERT_FLOAT_EQ(cos(rotation * 0.5), decomposition.rotation().fData[3]);
 }
 
-TEST(MatrixDecomposition, DISABLED_ScaleFloatError) {
-  SkMatrix44 matrix = SkMatrix44::I();
+TEST(MatrixDecomposition, ScaleFloatError) {
+  for (float scale = 0.0001f; scale < 2.0f; scale += 0.000001f) {
+    SkMatrix44 matrix = SkMatrix44::I();
+    matrix.setScale(scale, scale, 1.0f);
+
+    flow::MatrixDecomposition decomposition3(matrix);
+    ASSERT_TRUE(decomposition3.IsValid());
+
+    ASSERT_FLOAT_EQ(scale, decomposition3.scale().fX);
+    ASSERT_FLOAT_EQ(scale, decomposition3.scale().fY);
+    ASSERT_FLOAT_EQ(1.f, decomposition3.scale().fZ);
+    ASSERT_FLOAT_EQ(0, decomposition3.rotation().fData[0]);
+    ASSERT_FLOAT_EQ(0, decomposition3.rotation().fData[1]);
+    ASSERT_FLOAT_EQ(0, decomposition3.rotation().fData[2]);
+  }
 
+  SkMatrix44 matrix = SkMatrix44::I();
   const auto scale = 1.7734375f;
   matrix.setScale(scale, scale, 1.f);
 
+  // Bug upper bound (empirical)
+  const auto scale2 = 1.773437559603f;
+  SkMatrix44 matrix2 = SkMatrix44::I();
+  matrix2.setScale(scale2, scale2, 1.f);
+
+  // Bug lower bound (empirical)
+  const auto scale3 = 1.7734374403954f;
+  SkMatrix44 matrix3 = SkMatrix44::I();
+  matrix3.setScale(scale3, scale3, 1.f);
+
   flow::MatrixDecomposition decomposition(matrix);
   ASSERT_TRUE(decomposition.IsValid());
 
+  flow::MatrixDecomposition decomposition2(matrix2);
+  ASSERT_TRUE(decomposition2.IsValid());
+
+  flow::MatrixDecomposition decomposition3(matrix3);
+  ASSERT_TRUE(decomposition3.IsValid());
+
   ASSERT_FLOAT_EQ(scale, decomposition.scale().fX);
   ASSERT_FLOAT_EQ(scale, decomposition.scale().fY);
   ASSERT_FLOAT_EQ(1.f, decomposition.scale().fZ);
   ASSERT_FLOAT_EQ(0, decomposition.rotation().fData[0]);
   ASSERT_FLOAT_EQ(0, decomposition.rotation().fData[1]);
   ASSERT_FLOAT_EQ(0, decomposition.rotation().fData[2]);
+
+  ASSERT_FLOAT_EQ(scale2, decomposition2.scale().fX);
+  ASSERT_FLOAT_EQ(scale2, decomposition2.scale().fY);
+  ASSERT_FLOAT_EQ(1.f, decomposition2.scale().fZ);
+  ASSERT_FLOAT_EQ(0, decomposition2.rotation().fData[0]);
+  ASSERT_FLOAT_EQ(0, decomposition2.rotation().fData[1]);
+  ASSERT_FLOAT_EQ(0, decomposition2.rotation().fData[2]);
+
+  ASSERT_FLOAT_EQ(scale3, decomposition3.scale().fX);
+  ASSERT_FLOAT_EQ(scale3, decomposition3.scale().fY);
+  ASSERT_FLOAT_EQ(1.f, decomposition3.scale().fZ);
+  ASSERT_FLOAT_EQ(0, decomposition3.rotation().fData[0]);
+  ASSERT_FLOAT_EQ(0, decomposition3.rotation().fData[1]);
+  ASSERT_FLOAT_EQ(0, decomposition3.rotation().fData[2]);
 }