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

a more involved path is rect bug

Browse Source 
This is bug number ten in the series, and is the
most interesting. It exploits that the code tracks
corners 0, 2, and 3 but not corner 1.

Changing the code to track all corners is the biggest
so far, and while it (hopefully) simplifies things,
the presence of new code may signify more bugs to come.

R=robertphillips@google.com

Bug: 824145,skia::7792
Change-Id: Ia18e4d80fbed06ae6d9c89dcb4c462c5610213cc
Reviewed-on: https://skia-review.googlesource.com/121487
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Cary Clark <caryclark@google.com>
This commit is contained in:
Cary Clark 2018-04-16 12:06:07 -04:00 committed by Skia Commit-Bot
parent 93b4ddd347
commit 48c464a3c4
3 changed files with 41 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
gm/pathfill.cpp
View File

@ -433,7 +433,7 @@ DEF_SIMPLE_GM(rotatedcubicpath, canvas, 200, 200) {
DEF_GM( return new PathFillGM; ) DEF_GM( return new PathFillGM; )
DEF_GM( return new PathInverseFillGM; ) DEF_GM( return new PathInverseFillGM; )
DEF_SIMPLE_GM(bug7792, canvas, 600, 600) { DEF_SIMPLE_GM(bug7792, canvas, 600, 800) {
// from skbug.com/7792 bug description // from skbug.com/7792 bug description
SkPaint p; SkPaint p;
SkPath path; SkPath path;
@ -536,4 +536,15 @@ DEF_SIMPLE_GM(bug7792, canvas, 600, 600) {
path.lineTo(75, 150); path.lineTo(75, 150);
path.close(); path.close();
canvas->drawPath(path, p); canvas->drawPath(path, p);
// from skbug.com/7792#c29
canvas->translate(-200 * 2, 200);
path.reset();
path.moveTo(75, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(75, 150);
path.lineTo(75, 250);
path.moveTo(75, 75);
path.close();
canvas->drawPath(path, p);
} }

46
src/core/SkPath.cpp
View File

@ -435,7 +435,7 @@ FIXME: Allow colinear quads and cubics to be treated like lines.
FIXME: If the API passes fill-only, return true if the filled stroke FIXME: If the API passes fill-only, return true if the filled stroke
is a rectangle, though the caller failed to close the path. is a rectangle, though the caller failed to close the path.
first,last,next direction state-machine: directions values:
0x1 is set if the segment is horizontal 0x1 is set if the segment is horizontal
0x2 is set if the segment is moving to the right or down 0x2 is set if the segment is moving to the right or down
thus: thus:
@ -456,9 +456,7 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
const SkPoint* pts = *ptsPtr; const SkPoint* pts = *ptsPtr;
const SkPoint* savePts = nullptr; // used to allow caller to iterate through a pair of rects const SkPoint* savePts = nullptr; // used to allow caller to iterate through a pair of rects
lineStart.set(0, 0); lineStart.set(0, 0);
int firstDirection = 0; signed char directions[] = {-1, -1, -1, -1}; // -1 to 3; -1 is uninitialized
int lastDirection = 0;
int nextDirection = 0;
bool closedOrMoved = false; bool closedOrMoved = false;
bool addedLine = false; bool addedLine = false;
bool autoClose = false; bool autoClose = false;
@ -486,9 +484,9 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
break; // single point on side OK break; // single point on side OK
} }
addedLine = true; addedLine = true;
nextDirection = rect_make_dir(lineDelta.fX, lineDelta.fY); int nextDirection = rect_make_dir(lineDelta.fX, lineDelta.fY); // 0 to 3
if (0 == corners) { if (0 == corners) {
firstDirection = nextDirection; directions[0] = nextDirection;
lineStart = lineEnd; lineStart = lineEnd;
corners = 1; corners = 1;
closedOrMoved = false; closedOrMoved = false;
@ -497,26 +495,27 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
if (closedOrMoved) { if (closedOrMoved) {
return false; // closed followed by a line return false; // closed followed by a line
} }
if (autoClose && nextDirection == firstDirection) { if (autoClose && nextDirection == directions[0]) {
break; // colinear with first break; // colinear with first
} }
closedOrMoved = autoClose; closedOrMoved = autoClose;
if (lastDirection != nextDirection) {
if (++corners > 4) {
return false; // too many direction changes
}
}
lineStart = lineEnd; lineStart = lineEnd;
if (lastDirection == nextDirection) { if (directions[corners - 1] == nextDirection) {
break; // colinear segment break; // colinear segment
} }
// Possible values for corners are 2, 3, and 4. if (corners >= 4) {
// When corners == 3, nextDirection opposes firstDirection. return false; // too many direction changes
// Otherwise, nextDirection at corner 2 opposes corner 4. }
int turn = firstDirection ^ (corners - 1); directions[corners++] = nextDirection;
int directionCycle = 3 == corners ? 0 : nextDirection ^ turn; // opposite lines must point in opposite directions; xoring them should equal 2
if ((directionCycle ^ turn) != nextDirection) { if (corners == 3) {
return false; // direction didn't follow cycle if ((directions[0] ^ directions[2]) != 2) {
return false;
}
} else if (corners == 4) {
if ((directions[1] ^ directions[3]) != 2) {
return false;
}
} }
break; break;
} }
@ -525,7 +524,7 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
case kCubic_Verb: case kCubic_Verb:
return false; // quadratic, cubic not allowed return false; // quadratic, cubic not allowed
case kMove_Verb: case kMove_Verb:
if (allowPartial && !autoClose && firstDirection) { if (allowPartial && !autoClose && directions[0] >= 0) {
insertClose = true; insertClose = true;
*currVerb -= 1; // try move again afterwards *currVerb -= 1; // try move again afterwards
goto addMissingClose; goto addMissingClose;
@ -544,7 +543,6 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
break; break;
} }
*currVerb += 1; *currVerb += 1;
lastDirection = nextDirection;
addMissingClose: addMissingClose:
; ;
} }
@ -567,7 +565,7 @@ addMissingClose:
} }
int closeDirection = rect_make_dir(closeXY.fX, closeXY.fY); int closeDirection = rect_make_dir(closeXY.fX, closeXY.fY);
// make sure the close-segment doesn't double-back on itself // make sure the close-segment doesn't double-back on itself
if (3 == corners || closeDirection == lastDirection) { if (3 == corners || (closeDirection ^ directions[1]) == 2) {
result = true; result = true;
autoClose = false; // we are not closed autoClose = false; // we are not closed
} }
@ -583,7 +581,7 @@ addMissingClose:
*isClosed = autoClose; *isClosed = autoClose;
} }
if (result && direction) { if (result && direction) {
*direction = firstDirection == ((lastDirection + 1) & 3) ? kCCW_Direction : kCW_Direction; *direction = directions[0] == ((directions[1] + 1) & 3) ? kCW_Direction : kCCW_Direction;
} }
return result; return result;
} }

7
tests/PathTest.cpp
View File

@ -4996,4 +4996,11 @@ DEF_TEST(Path_isRect, reporter) {
REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr)); REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
compare.set(&points23[0], SK_ARRAY_COUNT(points23)); compare.set(&points23[0], SK_ARRAY_COUNT(points23));
REPORTER_ASSERT(reporter, rect == compare); REPORTER_ASSERT(reporter, rect == compare);
// isolated from skbug.com/7792#c29
SkPath::Verb verbs29[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kMove_Verb,
SkPath::kClose_Verb };
SkPoint points29[] = { {75, 75}, {150, 75}, {150, 150}, {75, 150}, {75, 250}, {75, 75} };
path = makePath2(points29, verbs29, SK_ARRAY_COUNT(verbs29));
REPORTER_ASSERT(reporter, !path.isRect(&rect, nullptr, nullptr));
} }