AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

If we lose precision computing sum of the dash intervals, then we can have the

Browse Source 
same error when we subtract-in-a-loop with the phase. The result is that we can
read past the end of the array.

To fix this, we just pin the loop counter, and if we exhaust our intervals, we
just treat the phase as 0. Not precisely the exact answer, but we aren't going
to draw this dash correctly anyway, since it contains massive interval values
that will be imprecise given our current float implementation.

Fixes http://code.google.com/p/chromium/issues/detail?id=140642
Review URL: https://codereview.appspot.com/6458088

git-svn-id: http://skia.googlecode.com/svn/trunk@4959 2bbb7eff-a529-9590-31e7-b0007b416f81
This commit is contained in:
reed@google.com 2012-08-06 14:58:35 +00:00
parent 157d94465a
commit d9ee348720
2 changed files with 39 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/effects/SkDashPathEffect.cpp
View File

@ -16,15 +16,22 @@ 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++) {
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;
}
}
// 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,
SkScalar phase, bool scaleToFit)
@ -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);

23
tests/DrawPathTest.cpp
View File

@ -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);
}