Clean up DashPathEffect modulo math from r3761

Review URL: https://codereview.appspot.com/6124048

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

2 files changed, 30 insertions, 17 deletions

diff --git a/include/effects/SkDashPathEffect.h b/include/effects/SkDashPathEffect.h
index 300edcd535..0c9e53cc31 100644
--- a/include/effects/SkDashPathEffect.h
+++ b/include/effects/SkDashPathEffect.h
@@ -18,11 +18,25 @@
 */
 class SK_API SkDashPathEffect : public SkPathEffect {
 public:
-    /** The intervals array must contain an even number of entries (>=2), with the even
-        indices specifying the "on" intervals, and the odd indices specifying the "off"
-        intervals. phase is an offset into the intervals array (mod the sum of all of the
-        intervals).
-        Note: only affects framed paths
+    /** intervals: array containing an even number of entries (>=2), with
+         the even indices specifying the length of "on" intervals, and the odd
+         indices specifying the length of "off" intervals.
+        count: number of elements in the intervals array 
+        phase: offset into the intervals array (mod the sum of all of the
+         intervals).
+
+        For example: if intervals[] = {10, 20}, count = 2, and phase = 25,
+         this will set up a dashed path like so:
+         5 pixels off
+         10 pixels on
+         20 pixels off
+         10 pixels on
+         20 pixels off
+         ...
+        A phase of -5, 25, 55, 85, etc. would all result in the same path,
+         because the sum of all the intervals is 30.
+
+        Note: only affects stroked paths.
     */
     SkDashPathEffect(const SkScalar intervals[], int count, SkScalar phase, bool scaleToFit = false);
     virtual ~SkDashPathEffect();
diff --git a/src/effects/SkDashPathEffect.cpp b/src/effects/SkDashPathEffect.cpp
index d511ca69f7..0cc97b6b15 100644
--- a/src/effects/SkDashPathEffect.cpp
+++ b/src/effects/SkDashPathEffect.cpp
@@ -43,36 +43,35 @@ SkDashPathEffect::SkDashPathEffect(const SkScalar intervals[], int count,
     }
     fIntervalLength = len;
 
-    if (len > 0) {  // we don't handle 0 length dash arrays
+    // watch out for values that might make us go out of bounds
+    if ((len > 0) && SkScalarIsFinite(phase) && SkScalarIsFinite(len)) {
+
+        // Adjust phase to be between 0 and len, "flipping" phase if negative.
+        // e.g., if len is 100, then phase of -20 (or -120) is equivalent to 80
         if (phase < 0) {
             phase = -phase;
             if (phase > len) {
                 phase = SkScalarMod(phase, len);
             }
             phase = len - phase;
-            // Due to finite precision, its possible that phase == len, even after the
-            // subtract (if len >>> phase), so we need to detect that here.
+
+            // Due to finite precision, it's possible that phase == len,
+            // even after the subtract (if len >>> phase), so fix that here.
+            // This fixes http://crbug.com/124652 .
             SkASSERT(phase <= len);
             if (phase == len) {
                 phase = 0;
             }
-        }
-        if (phase >= len) {
+        } else if (phase >= len) {
             phase = SkScalarMod(phase, len);
         }
-
-        // got to watch out for values that might make us go out of bounds
-        if (!SkScalarIsFinite(phase) || !SkScalarIsFinite(len)) {
-            goto BAD_DASH;
-        }
-
         SkASSERT(phase >= 0 && phase < len);
+
         fInitialDashLength = FindFirstInterval(intervals, phase, &fInitialDashIndex);
 
         SkASSERT(fInitialDashLength >= 0);
         SkASSERT(fInitialDashIndex >= 0 && fInitialDashIndex < fCount);
     } else {
-        BAD_DASH:
         fInitialDashLength = -1;    // signal bad dash intervals
     }
 }