diff options
| -rw-r--r-- | src/effects/SkDashPathEffect.cpp | 24 | ||||
| -rw-r--r-- | tests/DrawPathTest.cpp | 23 |
2 files changed, 39 insertions, 8 deletions
diff --git a/src/effects/SkDashPathEffect.cpp b/src/effects/SkDashPathEffect.cpp index e6f6afaf07..c1bb705818 100644 --- a/src/effects/SkDashPathEffect.cpp +++ b/src/effects/SkDashPathEffect.cpp @@ -16,14 +16,21 @@ static inline int is_even(int x) { } static SkScalar FindFirstInterval(const SkScalar intervals[], SkScalar phase, - int32_t* index) { - int i; - - for (i = 0; phase > intervals[i]; i++) { - phase -= intervals[i]; + int32_t* index, int count) { + for (int i = 0; i < count; ++i) { + if (phase > intervals[i]) { + phase -= intervals[i]; + } else { + *index = i; + return intervals[i] - phase; + } } - *index = i; - return intervals[i] - phase; + // If we get here, phase "appears" to be larger than our length. This + // shouldn't happen with perfect precision, but we can accumulate errors + // during the initial length computation (rounding can make our sum be too + // big or too small. In that event, we just have to eat the error here. + *index = 0; + return intervals[0]; } SkDashPathEffect::SkDashPathEffect(const SkScalar intervals[], int count, @@ -67,7 +74,8 @@ SkDashPathEffect::SkDashPathEffect(const SkScalar intervals[], int count, } SkASSERT(phase >= 0 && phase < len); - fInitialDashLength = FindFirstInterval(intervals, phase, &fInitialDashIndex); + fInitialDashLength = FindFirstInterval(intervals, phase, + &fInitialDashIndex, count); SkASSERT(fInitialDashLength >= 0); SkASSERT(fInitialDashIndex >= 0 && fInitialDashIndex < fCount); diff --git a/tests/DrawPathTest.cpp b/tests/DrawPathTest.cpp index 947bf88029..f421ee87b6 100644 --- a/tests/DrawPathTest.cpp +++ b/tests/DrawPathTest.cpp @@ -137,6 +137,28 @@ static void test_bug533(skiatest::Reporter* reporter) { #endif } +static void test_crbug_140642(skiatest::Reporter* reporter) { + /* + * We used to see this construct, and due to rounding as we accumulated + * our length, the loop where we apply the phase would run off the end of + * the array, since it relied on just -= each interval value, which did not + * behave as "expected". Now the code explicitly checks for walking off the + * end of that array. + + * A different (better) fix might be to rewrite dashing to do all of its + * length/phase/measure math using double, but this may need to be + * coordinated with SkPathMeasure, to be consistent between the two. + + <path stroke="mintcream" stroke-dasharray="27734 35660 2157846850 247" + stroke-dashoffset="-248.135982067"> + */ + +#ifdef SK_SCALAR_IS_FLOAT + const SkScalar vals[] = { 27734, 35660, 2157846850.0f, 247 }; + SkDashPathEffect dontAssert(vals, 4, -248.135982067f); +#endif +} + static void test_crbug_124652(skiatest::Reporter* reporter) { #ifdef SK_SCALAR_IS_FLOAT /* @@ -185,6 +207,7 @@ static void TestDrawPath(skiatest::Reporter* reporter) { test_bug533(reporter); test_bigcubic(reporter); test_crbug_124652(reporter); + test_crbug_140642(reporter); test_inversepathwithclip(reporter); // test_crbug131181(reporter); } |