handle overflow

note: gradient caller doesn't so we can still draw wrong when the caller
converts its initial fx from float->fixed. Perhaps SkClampRange should offer
a float interface as well.

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

diff --git a/include/core/SkClampRange.h b/include/core/SkClampRange.h
index 999faa0..9acf1ad 100644 (file)
--- a/include/core/SkClampRange.h
+++ b/include/core/SkClampRange.h
@@ -37,6 +37,9 @@ struct SkClampRange {
     bool fOverflowed;   // true if we had to clamp due to numerical overflow
 
     void init(SkFixed fx, SkFixed dx, int count, int v0, int v1);
+
+private:
+    void initFor1(SkFixed fx);
 };
 
 #endif

diff --git a/src/core/SkClampRange.cpp b/src/core/SkClampRange.cpp
index be41067..32ea964 100644 (file)
--- a/src/core/SkClampRange.cpp
+++ b/src/core/SkClampRange.cpp
@@ -20,7 +20,7 @@
  *  returns [0..count] for the number of steps (<= count) for which x0 <= edge
  *  given each step is followed by x0 += dx
  */
-static int chop(SkFixed x0, SkFixed edge, SkFixed x1, SkFixed dx, int count) {
+static int chop(int64_t x0, SkFixed edge, int64_t x1, int64_t dx, int count) {
     SkASSERT(dx > 0);
     SkASSERT(count >= 0);
 
@@ -30,36 +30,52 @@ static int chop(SkFixed x0, SkFixed edge, SkFixed x1, SkFixed dx, int count) {
     if (x1 <= edge) {
         return count;
     }
-    int n = (edge - x0 + dx - 1) / dx;
+    int64_t n = (edge - x0 + dx - 1) / dx;
     SkASSERT(n >= 0);
     SkASSERT(n <= count);
-    return n;
+    return (int)n;
 }
 
-void SkClampRange::init(SkFixed fx, SkFixed dx, int count, int v0, int v1) {
+static bool overflows_fixed(int64_t x) {
+    return x < -SK_FixedMax || x > SK_FixedMax;
+}
+
+void SkClampRange::initFor1(SkFixed fx) {
+    fCount0 = fCount1 = fCount2 = 0;
+    if (fx <= 0) {
+        fCount0 = 1;
+    } else if (fx < 0xFFFF) {
+        fCount1 = 1;
+        fFx1 = fx;
+    } else {
+        fCount2 = 1;
+    }
+}
+
+void SkClampRange::init(SkFixed fx0, SkFixed dx0, int count, int v0, int v1) {
     SkASSERT(count > 0);
 
     fV0 = v0;
     fV1 = v1;
+    fOverflowed = false;
 
-    // check for over/underflow
-    {
-        int64_t eex = (int64_t)fx + count * (int64_t)dx;
-        if (eex > SK_FixedMax) {
-            
-        } else if (eex < -SK_FixedMax) {
-        }
+    // special case 1 == count, as it is slightly common for skia
+    // and avoids us ever calling divide or 64bit multiply
+    if (1 == count) {
+        this->initFor1(fx0);
+        return;
     }
 
-    // remember our original fx
-    const SkFixed fx0 = fx;
+    int64_t fx = fx0;
+    int64_t dx = dx0;
     // start with ex equal to the last computed value
-    SkFixed ex = fx + (count - 1) * dx;
+    int64_t ex = fx + (count - 1) * dx;
+    fOverflowed = overflows_fixed(ex);
 
-    if ((unsigned)(fx | ex) <= 0xFFFF) {
+    if ((uint64_t)(fx | ex) <= 0xFFFF) {
         fCount0 = fCount2 = 0;
         fCount1 = count;
-        fFx1 = fx;
+        fFx1 = fx0;
         return;
     }
     if (fx <= 0 && ex <= 0) {
@@ -73,9 +89,41 @@ void SkClampRange::init(SkFixed fx, SkFixed dx, int count, int v0, int v1) {
         return;
     }
 
+    int extraCount = 0;
+
     // now make ex be 1 past the last computed value
     ex += dx;
-
+    fOverflowed = overflows_fixed(ex);
+    // now check for over/under flow
+    if (fOverflowed) {
+        int originalCount = count;
+        int64_t ccount;
+        bool swap = dx < 0;
+        if (swap) {
+            dx = -dx;
+            fx = -fx;
+        }
+        ccount = (SK_FixedMax - fx + dx - 1) / dx;
+        if (swap) {
+            dx = -dx;
+            fx = -fx;
+        }
+        SkASSERT(ccount > 0 && ccount <= SK_FixedMax);
+
+        count = (int)ccount;
+        if (0 == count) {
+            this->initFor1(fx0);
+            if (dx > 0) {
+                fCount2 += originalCount - 1;
+            } else {
+                fCount0 += originalCount - 1;
+            }
+            return;
+        }
+        extraCount = originalCount - count;
+        ex = fx + dx * count;
+    }
+    
     bool doSwap = dx < 0;
 
     if (doSwap) {
@@ -85,6 +133,7 @@ void SkClampRange::init(SkFixed fx, SkFixed dx, int count, int v0, int v1) {
         dx = -dx;
     }
 
+
     fCount0 = chop(fx, 0, ex, dx, count);
     count -= fCount0;
     fx += fCount0 * dx;
@@ -112,7 +161,13 @@ void SkClampRange::init(SkFixed fx, SkFixed dx, int count, int v0, int v1) {
     }
 
     if (fCount1 > 0) {
-        fFx1 = fx0 + fCount0 * dx;
+        fFx1 = fx0 + fCount0 * (int)dx;
+    }
+
+    if (dx > 0) {
+        fCount2 += extraCount;
+    } else {
+        fCount0 += extraCount;
     }
 }
 

diff --git a/src/effects/SkGradientShader.cpp b/src/effects/SkGradientShader.cpp
index 068913f..5af0486 100644 (file)
--- a/src/effects/SkGradientShader.cpp
+++ b/src/effects/SkGradientShader.cpp
@@ -27,7 +27,7 @@
     #define USE_DITHER_32BIT_GRADIENT
 #endif
 
-//#define SK_ENABLE_FAST_LINEAR_GRADIENTS
+#define SK_ENABLE_FAST_LINEAR_GRADIENTS
 
 #ifdef SK_ENABLE_FAST_LINEAR_GRADIENTS
 static void sk_memset32_dither(uint32_t dst[], uint32_t v0, uint32_t v1,
@@ -819,11 +819,13 @@ static inline bool no_need_for_clamp(int fx, int dx, int count) {
 #include "SkClampRange.h"
 
 #define NO_CHECK_ITER               \
-    fi = fx >> 8;                   \
+    do {                            \
+    unsigned fi = fx >> 8;          \
     SkASSERT(fi <= 0xFF);           \
     fx += dx;                       \
     *dstC++ = cache[toggle + fi];   \
-    toggle ^= TOGGLE_MASK
+    toggle ^= TOGGLE_MASK;          \
+    } while (0)
 
 
 void Linear_Gradient::shadeSpan(int x, int y, SkPMColor dstC[], int count) {
@@ -874,9 +876,9 @@ void Linear_Gradient::shadeSpan(int x, int y, SkPMColor dstC[], int count) {
                 dstC += count;
             }
             if ((count = range.fCount1) > 0) {
-                unsigned fi;
-                int i, unroll = count >> 3;
-                for (i = 0; i < unroll; i++) {
+                int unroll = count >> 3;
+                fx = range.fFx1;
+                for (int i = 0; i < unroll; i++) {
                     NO_CHECK_ITER;  NO_CHECK_ITER;
                     NO_CHECK_ITER;  NO_CHECK_ITER;
                     NO_CHECK_ITER;  NO_CHECK_ITER;
@@ -978,11 +980,13 @@ static void dither_memset16(uint16_t dst[], uint16_t value, uint16_t other,
 }
 
 #define NO_CHECK_ITER_16                \
-    fi = fx >> kCache16Shift;           \
+    do {                                \
+    unsigned fi = fx >> kCache16Shift;  \
     SkASSERT(fi <= kCache16Mask);       \
     fx += dx;                           \
     *dstC++ = cache[toggle + fi];       \
-    toggle ^= TOGGLE_MASK
+    toggle ^= TOGGLE_MASK;              \
+    } while (0)
 
 
 void Linear_Gradient::shadeSpan16(int x, int y, uint16_t dstC[], int count) {
@@ -1028,9 +1032,9 @@ void Linear_Gradient::shadeSpan16(int x, int y, uint16_t dstC[], int count) {
                 dstC += count;
             }
             if ((count = range.fCount1) > 0) {
-                unsigned fi;
-                int i, unroll = count >> 3;
-                for (i = 0; i < unroll; i++) {
+                int unroll = count >> 3;
+                fx = range.fFx1;
+                for (int i = 0; i < unroll; i++) {
                     NO_CHECK_ITER_16;  NO_CHECK_ITER_16;
                     NO_CHECK_ITER_16;  NO_CHECK_ITER_16;
                     NO_CHECK_ITER_16;  NO_CHECK_ITER_16;

diff --git a/tests/ClampRangeTest.cpp b/tests/ClampRangeTest.cpp
index b3d1e78..be9c6ec 100644 (file)
--- a/tests/ClampRangeTest.cpp
+++ b/tests/ClampRangeTest.cpp
@@ -4,6 +4,27 @@
 
 static skiatest::Reporter* gReporter;
 
+static void debug_me() {
+    if (NULL == gReporter) {
+        SkDebugf("dsfdssd\n");
+    }
+}
+
+#ifdef USE_REPORTER
+
+#define R_ASSERT(cond)                  \
+    do { if (!(cond)) {                 \
+    debug_me();                         \
+    REPORTER_ASSERT(gReporter, cond);   \
+    }} while (0)
+
+#else
+#define R_ASSERT(cond)                  \
+    do { if (!(cond)) {                 \
+    debug_me();                         \
+    }} while (0)
+#endif
+
 static int classify_value(SkFixed fx, int v0, int v1) {
     if (fx <= 0) {
         return v0;
@@ -11,7 +32,7 @@ static int classify_value(SkFixed fx, int v0, int v1) {
     if (fx >= 0xFFFF) {
         return v1;
     }
-    REPORTER_ASSERT(gReporter, false);
+    R_ASSERT(false);
     return 0;
 }
 
@@ -21,22 +42,34 @@ static int classify_value(SkFixed fx, int v0, int v1) {
 static void slow_check(const SkClampRange& range,
                        SkFixed fx, SkFixed dx, int count) {
     SkASSERT(range.fCount0 + range.fCount1 + range.fCount2 == count);
-    
+
     int i;
-    for (i = 0; i < range.fCount0; i++) {
-        int v = classify_value(fx, V0, V1);
-        REPORTER_ASSERT(gReporter, v == range.fV0);
-        fx += dx;
-    }
-    REPORTER_ASSERT(gReporter, range.fCount1 == 0 || fx == range.fFx1);
-    for (i = 0; i < range.fCount1; i++) {
-        REPORTER_ASSERT(gReporter, fx >= 0 && fx <= 0xFFFF);
-        fx += dx;
-    }
-    for (i = 0; i < range.fCount2; i++) {
-        int v = classify_value(fx, V0, V1);
-        REPORTER_ASSERT(gReporter, v == range.fV1);
-        fx += dx;
+    if (range.fOverflowed) {
+        fx = range.fFx1;
+        for (i = 0; i < range.fCount1; i++) {
+            R_ASSERT(fx >= 0 && fx <= 0xFFFF);
+            fx += dx;
+        }
+    } else {
+        for (i = 0; i < range.fCount0; i++) {
+            int v = classify_value(fx, V0, V1);
+            R_ASSERT(v == range.fV0);
+            fx += dx;
+        }
+        if (range.fCount1 > 0 && fx != range.fFx1) {
+            SkDebugf("%x %x\n", fx, range.fFx1);
+            R_ASSERT(!"bad fFx1");
+            return;
+        }
+        for (i = 0; i < range.fCount1; i++) {
+            R_ASSERT(fx >= 0 && fx <= 0xFFFF);
+            fx += dx;
+        }
+        for (i = 0; i < range.fCount2; i++) {
+            int v = classify_value(fx, V0, V1);
+            R_ASSERT(v == range.fV1);
+            fx += dx;
+        }
     }
 }
 
@@ -48,7 +81,8 @@ static void test_range(SkFixed fx, SkFixed dx, int count) {
 
 #define ff(x)   SkIntToFixed(x)
 
-static void TestClampRange(skiatest::Reporter* reporter) {
+void TestClampRange(skiatest::Reporter* reporter);
+void TestClampRange(skiatest::Reporter* reporter) {
     gReporter = reporter;
 
     test_range(0, 0, 20);
@@ -60,7 +94,12 @@ static void TestClampRange(skiatest::Reporter* reporter) {
     test_range(10, -1, 20);
     test_range(-10, 3, 20);
     test_range(10, -3, 20);
-    
+
+    test_range(ff(1),  ff(16384),  100);
+    test_range(ff(-1), ff(-16384), 100);
+    test_range(ff(1)/2, ff(16384), 100);
+    test_range(ff(1)/2, ff(-16384), 100);
+
     SkRandom rand;
     
     // test non-overflow cases
@@ -73,14 +112,17 @@ static void TestClampRange(skiatest::Reporter* reporter) {
     }
     
     // test overflow cases
-    for (int i = 0; i < 0*1000000; i++) {
+    for (int i = 0; i < 100000; i++) {
         SkFixed fx = rand.nextS();
-        SkFixed sx = rand.nextS();
+        SkFixed dx = rand.nextS();
         int count = rand.nextU() % 1000 + 1;
-        SkFixed dx = (sx - fx) / count;
         test_range(fx, dx, count);
     }
 }
 
+#ifdef USE_REPORTER
+
 #include "TestClassDef.h"
 DEFINE_TESTCLASS("ClampRange", ClampRangeClass, TestClampRange)
+
+#endif