gfx::Transform::IsBackVisible compares

(cofactor33*determinant) with 0 to determine if a transform
is back face visible. In the bug, one of the layer's
transform ends up with an extremely small close to
zero negative number though the actual value should be 0.
This results in the function returning the wrong value.

BUG=538290
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1421603002

Cr-Commit-Position: refs/heads/master@{#355574}

2 files changed, 26 insertions, 4 deletions

diff --git a/ui/gfx/transform.cc b/ui/gfx/transform.cc
index c42c961..a7cf86e 100644
--- a/ui/gfx/transform.cc
+++ b/ui/gfx/transform.cc
@@ -24,8 +24,7 @@ namespace gfx {
 
 namespace {
 
-// Taken from SkMatrix44.
-const SkMScalar kEpsilon = 1e-8f;
+const SkMScalar kEpsilon = std::numeric_limits<float>::epsilon();
 
 SkMScalar TanDegrees(double degrees) {
   double radians = degrees * M_PI / 180;
@@ -270,7 +269,7 @@ bool Transform::IsBackFaceVisible() const {
   double determinant = matrix_.determinant();
 
   // If matrix was not invertible, then just assume back face is not visible.
-  if (std::abs(determinant) <= kEpsilon)
+  if (determinant == 0)
     return false;
 
   // Compute the cofactor of the 3rd row, 3rd column.
@@ -303,7 +302,7 @@ bool Transform::IsBackFaceVisible() const {
   // Technically the transformed z component is cofactor33 / determinant.  But
   // we can avoid the costly division because we only care about the resulting
   // +/- sign; we can check this equivalently by multiplication.
-  return cofactor33 * determinant < 0;
+  return cofactor33 * determinant < -kEpsilon;
 }
 
 bool Transform::GetInverse(Transform* transform) const {
diff --git a/ui/gfx/transform_unittest.cc b/ui/gfx/transform_unittest.cc
index 81333a3..2d881bc 100644
--- a/ui/gfx/transform_unittest.cc
+++ b/ui/gfx/transform_unittest.cc
@@ -2691,6 +2691,29 @@ TEST(XFormTest, RoundTranslationComponents) {
   EXPECT_EQ(expected.ToString(), translation.ToString());
 }
 
+TEST(XFormTest, BackFaceVisiblilityTolerance) {
+  Transform backface_invisible;
+  backface_invisible.matrix().set(0, 3, 1.f);
+  backface_invisible.matrix().set(3, 0, 1.f);
+  backface_invisible.matrix().set(2, 0, 1.f);
+  backface_invisible.matrix().set(3, 2, 1.f);
+
+  // The transformation matrix has a determinant = 1 and cofactor33 = 0. So,
+  // IsBackFaceVisible should return false.
+  EXPECT_EQ(backface_invisible.matrix().determinant(), 1.f);
+  EXPECT_FALSE(backface_invisible.IsBackFaceVisible());
+
+  // Adding a noise to the transformsation matrix that is within the tolerance
+  // (machine epsilon) should not change the result.
+  float noise = std::numeric_limits<float>::epsilon();
+  backface_invisible.matrix().set(0, 3, 1.f + noise);
+  EXPECT_FALSE(backface_invisible.IsBackFaceVisible());
+
+  // A noise that is more than the tolerance should change the result.
+  backface_invisible.matrix().set(0, 3, 1.f + (2 * noise));
+  EXPECT_TRUE(backface_invisible.IsBackFaceVisible());
+}
+
 }  // namespace
 
 }  // namespace gfx