Fix handling of infinite bounds during "fast transforms".

http://codereview.appspot.com/6449125/

git-svn-id: http://skia.googlecode.com/svn/trunk@5042 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 4483388..1127e95 100644 (file)
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -1341,13 +1341,30 @@ void SkPath::transform(const SkMatrix& matrix, SkPath* dst) const {
         dst->swap(tmp);
         matrix.mapPoints(dst->fPts.begin(), dst->fPts.count());
     } else {
-        // remember that dst might == this, so be sure to check
-        // fBoundsIsDirty before we set it
+        /*
+         *  If we're not in perspective, we can transform all of the points at
+         *  once.
+         *
+         *  Here we also want to optimize bounds, by noting if the bounds are
+         *  already known, and if so, we just transform those as well and mark
+         *  them as "known", rather than force the transformed path to have to
+         *  recompute them.
+         *
+         *  Special gotchas if the path is effectively empty (<= 1 point) or
+         *  if it is non-finite. In those cases bounds need to stay empty,
+         *  regardless of the matrix.
+         */
         if (!fBoundsIsDirty && matrix.rectStaysRect() && fPts.count() > 1) {
-            // if we're empty, fastbounds should not be mapped
-            matrix.mapRect(&dst->fBounds, fBounds);
             dst->fBoundsIsDirty = false;
-            dst->fIsFinite = dst->fBounds.isFinite();
+            if (fIsFinite) {
+                matrix.mapRect(&dst->fBounds, fBounds);
+                if (!(dst->fIsFinite = dst->fBounds.isFinite())) {
+                    dst->fBounds.setEmpty();
+                }
+            } else {
+                dst->fIsFinite = false;
+                dst->fBounds.setEmpty();
+            }
         } else {
             GEN_ID_PTR_INC(dst);
             dst->fBoundsIsDirty = true;
@@ -1795,7 +1812,10 @@ void SkPath::validate() const {
 
     if (!fBoundsIsDirty) {
         SkRect bounds;
-        compute_pt_bounds(&bounds, fPts);
+
+        bool isFinite = compute_pt_bounds(&bounds, fPts);
+        SkASSERT(fIsFinite == isFinite);
+
         if (fPts.count() <= 1) {
             // if we're empty, fBounds may be empty but translated, so we can't
             // necessarily compare to bounds directly

diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 8c00efb..0053640 100644 (file)
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -16,6 +16,34 @@
 #include "SkSize.h"
 #include "SkWriter32.h"
 
+// Inspired by http://code.google.com/p/chromium/issues/detail?id=141651
+//
+static void test_isfinite_after_transform(skiatest::Reporter* reporter) {
+    SkPath path;
+    path.quadTo(157, 366, 286, 208);
+    path.arcTo(37, 442, 315, 163, 957494590897113.0f);
+    
+    SkMatrix matrix;
+    matrix.setScale(1000*1000, 1000*1000);
+
+    // Be sure that path::transform correctly updates isFinite and the bounds
+    // if the transformation overflows. The previous bug was that isFinite was
+    // set to true in this case, but the bounds were not set to empty (which
+    // they should be).
+    while (path.isFinite()) {
+        REPORTER_ASSERT(reporter, path.getBounds().isFinite());
+        REPORTER_ASSERT(reporter, !path.getBounds().isEmpty());
+        path.transform(matrix);
+    }
+    REPORTER_ASSERT(reporter, path.getBounds().isEmpty());
+
+    matrix.setTranslate(SK_Scalar1, SK_Scalar1);
+    path.transform(matrix);
+    // we need to still be non-finite
+    REPORTER_ASSERT(reporter, !path.isFinite());
+    REPORTER_ASSERT(reporter, path.getBounds().isEmpty());
+}
+
 static void test_rect_isfinite(skiatest::Reporter* reporter) {
     const SkScalar inf = SK_ScalarInfinity;
     const SkScalar nan = SK_ScalarNaN;
@@ -1561,6 +1589,7 @@ static void TestPath(skiatest::Reporter* reporter) {
     test_strokerec(reporter);
     test_addPoly(reporter);
     test_isfinite(reporter);
+    test_isfinite_after_transform(reporter);
 }
 
 #include "TestClassDef.h"