Check for identity before doing matrix math in more places in gfx::Transform

Check for identity before doing matrix math in more places in gfx::Transform

This should be a good speedup once checking for if a matrix is identity is faster  (i.e. https://codereview.appspot.com/6854113/)- otherwise it's a bit of a wash.

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170485

[jamesr, 2012-11-30 00:24:59]

Hey Shawn - this saves ~1.3% on cc_perftests with
http://code.google.com/p/skia/source/detail?r=6620 applied.  Not huge, but real.
 I'm not sure this is the right place to apply this optimization - what do you
think?

[shawnsingh, 2012-11-30 00:45:00]

On 2012/11/30 00:24:59, jamesr wrote:
> Hey Shawn - this saves ~1.3% on cc_perftests with
> http://code.google.com/p/skia/source/detail?r=6620 applied.  Not huge, but
real.
>  I'm not sure this is the right place to apply this optimization - what do you
> think?

Yeah, all of these should be great in my opinon.  unofficial non-owner LGTM on
my side

[danakj, 2012-11-30 01:29:26]

OWNERS LGTM with one nit

https://codereview.chromium.org/11434027/diff/1/ui/gfx/transform.cc
File ui/gfx/transform.cc (right):

https://codereview.chromium.org/11434027/diff/1/ui/gfx/transform.cc#newcode211
ui/gfx/transform.cc:211: *this = transform;
matrix_ = transform.matrix_; like below?

[jamesr, 2012-11-30 02:04:30]

Fixed style and added checks to TransformPoint...() since with the math_util
changes these show up now.  PTAL

[danakj, 2012-11-30 02:06:35]

thanks, LGTM

[commit-bot: I haz the power, 2012-11-30 02:08:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11434027/3002

[commit-bot: I haz the power, 2012-11-30 03:29:25]

Retried try job too often on win_rel for step(s) browser_tests

[commit-bot: I haz the power, 2012-11-30 03:30:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11434027/3002

[commit-bot: I haz the power, 2012-11-30 07:27:28]

Failed to apply patch for ui/gfx/transform.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ui/gfx/transform.cc
  Hunk #1 succeeded at 206 (offset -1 lines).
  Hunk #2 succeeded at 226 (offset -1 lines).
  Hunk #3 succeeded at 249 (offset -1 lines).
  Hunk #4 succeeded at 268 (offset -1 lines).
  Hunk #5 FAILED at 281.
  Hunk #6 succeeded at 385 (offset 46 lines).
  Hunk #7 succeeded at 395 (offset 46 lines).
  Hunk #8 succeeded at 432 (offset 46 lines).
  Hunk #9 succeeded at 469 (offset 46 lines).
  1 out of 9 hunks FAILED -- saving rejects to file ui/gfx/transform.cc.rej

Patch:       ui/gfx/transform.cc
Index: ui/gfx/transform.cc
diff --git a/ui/gfx/transform.cc b/ui/gfx/transform.cc
index
b75bb7fb419113197228c69c6e699521fb904188..74860ba7b7eae6ee318cb1747e602a49c90565ab
100644
--- a/ui/gfx/transform.cc
+++ b/ui/gfx/transform.cc
@@ -207,13 +207,17 @@ void Transform::ApplyPerspectiveDepth(double depth) {
 }
 
 void Transform::PreconcatTransform(const Transform& transform) {
-  if (!transform.matrix_.isIdentity()) {
+  if (matrix_.isIdentity()) {
+    matrix_ = transform.matrix_;
+  } else if (!transform.matrix_.isIdentity()) {
     matrix_.preConcat(transform.matrix_);
   }
 }
 
 void Transform::ConcatTransform(const Transform& transform) {
-  if (!transform.matrix_.isIdentity()) {
+  if (matrix_.isIdentity()) {
+    matrix_ = transform.matrix_;
+  } else if (!transform.matrix_.isIdentity()) {
     matrix_.postConcat(transform.matrix_);
   }
 }
@@ -223,6 +227,9 @@ bool Transform::IsIdentity() const {
 }
 
 bool Transform::IsIdentityOrTranslation() const {
+  if (matrix_.isIdentity())
+    return true;
+
   bool has_no_perspective = !matrix_.getDouble(3, 0) &&
                             !matrix_.getDouble(3, 1) &&
                             !matrix_.getDouble(3, 2) &&
@@ -243,6 +250,9 @@ bool Transform::IsIdentityOrTranslation() const {
 }
 
 bool Transform::IsScaleOrTranslation() const {
+  if (matrix_.isIdentity())
+    return true;
+
   bool has_no_perspective = !matrix_.getDouble(3, 0) &&
                             !matrix_.getDouble(3, 1) &&
                             !matrix_.getDouble(3, 2) &&
@@ -259,10 +269,11 @@ bool Transform::IsScaleOrTranslation() const {
 }
 
 bool Transform::HasPerspective() const {
-  return matrix_.getDouble(3, 0) ||
-         matrix_.getDouble(3, 1) ||
-         matrix_.getDouble(3, 2) ||
-         (matrix_.getDouble(3, 3) != 1);
+  return !matrix_.isIdentity() &&
+         (matrix_.getDouble(3, 0) ||
+          matrix_.getDouble(3, 1) ||
+          matrix_.getDouble(3, 2) ||
+          (matrix_.getDouble(3, 3) != 1));
 }
 
 bool Transform::IsInvertible() const {
@@ -270,6 +281,9 @@ bool Transform::IsInvertible() const {
 }
 
 bool Transform::IsBackFaceVisible() const {
+  if (matrix_.isIdentity())
+    return false;
+
   // Compute whether a layer with a forward-facing normal of (0, 0, 1) would
   // have its back face visible after applying the transform.
   //
@@ -328,6 +342,9 @@ bool Transform::TransformPointReverse(Point3F& point) const
{
 }
 
 void Transform::TransformRect(RectF* rect) const {
+  if (matrix_.isIdentity())
+    return;
+
   SkRect src = RectFToSkRect(*rect);
   const SkMatrix& matrix = matrix_;
   matrix.mapRect(&src);
@@ -335,9 +352,13 @@ void Transform::TransformRect(RectF* rect) const {
 }
 
 bool Transform::TransformRectReverse(RectF* rect) const {
+  if (matrix_.isIdentity())
+    return true;
+
   SkMatrix44 inverse;
   if (!matrix_.invert(&inverse))
     return false;
+
   const SkMatrix& matrix = inverse;
   SkRect src = RectFToSkRect(*rect);
   matrix.mapRect(&src);
@@ -368,18 +389,25 @@ bool Transform::Blend(const Transform& from, double
progress) {
 }
 
 Transform Transform::operator*(const Transform& other) const {
+  if (matrix_.isIdentity())
+    return other;
+  if (other.matrix_.isIdentity())
+    return *this;
   Transform to_return;
   to_return.matrix_.setConcat(matrix_, other.matrix_);
   return to_return;
 }
 
 Transform& Transform::operator*=(const Transform& other) {
-  matrix_.preConcat(other.matrix_);
+  PreconcatTransform(other);
   return *this;
 }
 
 void Transform::TransformPointInternal(const SkMatrix44& xform,
                                        Point3F& point) const {
+  if (xform.isIdentity())
+    return;
+
   SkMScalar p[4] = {
     SkDoubleToMScalar(point.x()),
     SkDoubleToMScalar(point.y()),
@@ -398,6 +426,9 @@ void Transform::TransformPointInternal(const SkMatrix44&
xform,
 
 void Transform::TransformPointInternal(const SkMatrix44& xform,
                                        Point& point) const {
+  if (xform.isIdentity())
+    return;
+
   SkMScalar p[4] = {
     SkDoubleToMScalar(point.x()),
     SkDoubleToMScalar(point.y()),

[commit-bot: I haz the power, 2012-11-30 07:48:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11434027/5003

[commit-bot: I haz the power, 2012-11-30 11:30:38]

Retried try job too often on win_rel for step(s) chrome_frame_net_tests,
sync_integration_tests

[commit-bot: I haz the power, 2012-11-30 17:15:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11434027/5003

[commit-bot: I haz the power, 2012-11-30 18:20:41]

Change committed as 170485