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

more path is rect bugs

Browse Source 
More edge cases found; clean up the logic a bit
to make more clear where the rectangle points
start and stop.

R=robertphillips@google.com,brianosman@google.com
Bug: 824145,skia:7792
Change-Id: Ie24dfd1519f30875f44ffac68e20d777490b00b9
Reviewed-on: https://skia-review.googlesource.com/120422
Commit-Queue: Cary Clark <caryclark@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This commit is contained in:
Cary Clark 2018-04-11 14:30:27 -04:00 committed by Skia Commit-Bot
parent ac78c7f415
commit 8540e110f9
3 changed files with 164 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
gm/pathfill.cpp
View File

@ -432,3 +432,70 @@ DEF_SIMPLE_GM(rotatedcubicpath, canvas, 200, 200) {
DEF_GM( return new PathFillGM; )
DEF_GM( return new PathInverseFillGM; )
DEF_SIMPLE_GM(bug7792, canvas, 800, 200) {
// from skbug.com/7792 bug description
SkPaint p;
SkPath path;
path.moveTo(10, 10);
path.moveTo(75, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(75, 150);
canvas->drawPath(path, p);
// from skbug.com/7792 comment 3
canvas->translate(200, 0);
path.reset();
path.moveTo(75, 50);
path.moveTo(100, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(75, 150);
path.lineTo(75, 50);
path.close();
canvas->drawPath(path, p);
// from skbug.com/7792 comment 9
canvas->translate(200, 0);
path.reset();
path.moveTo(10, 10);
path.moveTo(75, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(75, 150);
path.close();
canvas->drawPath(path, p);
// from skbug.com/7792 comment 11
canvas->translate(-200 * 2, 200);
path.reset();
path.moveTo(75, 150);
path.lineTo(75, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(75, 150);
path.moveTo(75, 150);
canvas->drawPath(path, p);
// from skbug.com/7792 comment 14
canvas->translate(200, 0);
path.reset();
path.moveTo(250, 75);
path.moveTo(250, 75);
path.moveTo(250, 75);
path.moveTo(100, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(75, 150);
path.lineTo(75, 75);
path.close();
path.lineTo(0, 0);
path.close();
canvas->drawPath(path, p);
// from skbug.com/7792 comment 15
canvas->translate(200, 0);
path.reset();
path.moveTo(75, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(75, 150);
path.moveTo(250, 75);
canvas->drawPath(path, p);
}

49
src/core/SkPath.cpp
View File

@ -447,18 +447,20 @@ static int rect_make_dir(SkScalar dx, SkScalar dy) {
bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** ptsPtr,
bool* isClosed, Direction* direction, SkRect* rect) const {
int corners = 0;
SkPoint first, last;
const SkPoint* firstPt = nullptr;
SkPoint previous; // used to construct line from previous point
const SkPoint* firstPt = nullptr; // first point in the rect (last of first moves)
const SkPoint* lastPt = nullptr; // last point in the rect (last of lines or first if closed)
const SkPoint* pts = *ptsPtr;
const SkPoint* savePts = nullptr;
first.set(0, 0);
last.set(0, 0);
const SkPoint* savePts = nullptr; // used to allow caller to iterate through a pair of rects
previous.set(0, 0);
int firstDirection = 0;
int lastDirection = 0;
int nextDirection = 0;
bool closedOrMoved = false;
bool addedLine = false;
bool autoClose = false;
bool insertClose = false;
bool accumulatingRect = false;
int verbCnt = fPathRef->countVerbs();
while (*currVerb < verbCnt && (!allowPartial || !autoClose)) {
uint8_t verb = insertClose ? (uint8_t) kClose_Verb : fPathRef->atVerb(*currVerb);
@ -468,23 +470,27 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
pts = *ptsPtr;
autoClose = true;
insertClose = false;
accumulatingRect = false;
case kLine_Verb: {
SkScalar left = last.fX;
SkScalar top = last.fY;
SkScalar left = previous.fX;
SkScalar top = previous.fY;
SkScalar right = pts->fX;
SkScalar bottom = pts->fY;
if (accumulatingRect) {
lastPt = pts;
}
++pts;
if (left != right && top != bottom) {
return false; // diagonal
}
addedLine = true;
if (left == right && top == bottom) {
break; // single point on side OK
}
nextDirection = rect_make_dir(right - left, bottom - top);
if (0 == corners) {
firstDirection = nextDirection;
first = last;
last = pts[-1];
previous = pts[-1];
corners = 1;
closedOrMoved = false;
break;
@ -501,7 +507,7 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
return false; // too many direction changes
}
}
last = pts[-1];
previous = pts[-1];
if (lastDirection == nextDirection) {
break; // colinear segment
}
@ -525,8 +531,13 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
*currVerb -= 1; // try move again afterwards
goto addMissingClose;
}
if (!addedLine) {
firstPt = pts;
last = *pts++;
accumulatingRect = true;
} else {
accumulatingRect = false;
}
previous = *pts++;
closedOrMoved = true;
break;
default:
@ -539,10 +550,12 @@ addMissingClose:
;
}
// Success if 4 corners and first point equals last
SkScalar closeX = first.x() - last.x();
SkScalar closeY = first.y() - last.y();
if (corners < 3 || corners > 4) {
return false;
}
SkPoint closeXY = *firstPt - *lastPt;
// If autoClose, check if close generates diagonal
bool result = 4 == corners && (first == last || (autoClose && (!closeX || !closeY)));
bool result = 4 == corners && (closeXY.isZero() || (autoClose && (!closeXY.fX || !closeXY.fY)));
if (!result) {
// check if we are just an incomplete rectangle, in which case we can
// return true, but not claim to be closed.
@ -550,12 +563,12 @@ addMissingClose:
// 3 sided rectangle
// 4 sided but the last edge is not long enough to reach the start
//
if (closeX && closeY) {
if (closeXY.fX && closeXY.fY) {
return false; // we're diagonal, abort (can we ever reach this?)
}
int closeDirection = rect_make_dir(closeX, closeY);
int closeDirection = rect_make_dir(closeXY.fX, closeXY.fY);
// make sure the close-segment doesn't double-back on itself
if (3 == corners || (4 == corners && closeDirection == lastDirection)) {
if (3 == corners || closeDirection == lastDirection) {
result = true;
autoClose = false; // we are not closed
}
@ -564,7 +577,7 @@ addMissingClose:
*ptsPtr = savePts;
}
if (result && rect) {
ptrdiff_t count = (savePts ? savePts : pts) - firstPt;
ptrdiff_t count = lastPt - firstPt + 1;
rect->set(firstPt, (int) count);
}
if (result && isClosed) {

70
tests/PathTest.cpp
View File

@ -4901,21 +4901,75 @@ DEF_TEST(ClipPath_nonfinite, reporter) {
// skbug.com/7792
DEF_TEST(Path_isRect, reporter) {
auto makePath = [](const SkPoint* points, size_t count, bool close) -> SkPath {
SkPath path;
SkPoint points[] = { {10, 10}, {75, 75}, {150, 75}, {150, 150}, {75, 150} };
for (size_t index = 0; index < SK_ARRAY_COUNT(points); ++index) {
for (size_t index = 0; index < count; ++index) {
index < 2 ? path.moveTo(points[index]) : path.lineTo(points[index]);
}
return path;
};
auto makePath2 = [](const SkPoint* points, const SkPath::Verb* verbs, size_t count) -> SkPath {
SkPath path;
for (size_t index = 0; index < count; ++index) {
switch (verbs[index]) {
case SkPath::kMove_Verb:
path.moveTo(*points++);
break;
case SkPath::kLine_Verb:
path.lineTo(*points++);
break;
case SkPath::kClose_Verb:
path.close();
break;
default:
SkASSERT(0);
}
}
return path;
};
// isolated from skbug.com/7792 bug description
SkRect rect;
SkPoint points[] = { {10, 10}, {75, 75}, {150, 75}, {150, 150}, {75, 150} };
SkPath path = makePath(points, SK_ARRAY_COUNT(points), false);
REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
SkRect compare;
compare.set(&points[1], SK_ARRAY_COUNT(points) - 1);
REPORTER_ASSERT(reporter, rect == compare);
path.reset();
SkPoint points2[] = { {75, 50}, {100, 75}, {150, 75}, {150, 150}, {75, 150}, {75, 50} };
for (size_t index = 0; index < SK_ARRAY_COUNT(points); ++index) {
index < 2 ? path.moveTo(points2[index]) : path.lineTo(points2[index]);
}
path.close();
// isolated from skbug.com/7792 comment 3
SkPoint points3[] = { {75, 50}, {100, 75}, {150, 75}, {150, 150}, {75, 150}, {75, 50} };
path = makePath(points3, SK_ARRAY_COUNT(points3), true);
REPORTER_ASSERT(reporter, !path.isRect(&rect, nullptr, nullptr));
// isolated from skbug.com/7792 comment 9
SkPoint points9[] = { {10, 10}, {75, 75}, {150, 75}, {150, 150}, {75, 150} };
path = makePath(points9, SK_ARRAY_COUNT(points9), true);
REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
compare.set(&points9[1], SK_ARRAY_COUNT(points9) - 1);
REPORTER_ASSERT(reporter, rect == compare);
// isolated from skbug.com/7792 comment 11
SkPath::Verb verbs11[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kMove_Verb };
SkPoint points11[] = { {75, 150}, {75, 75}, {150, 75}, {150, 150}, {75, 150}, {75, 150} };
path = makePath2(points11, verbs11, SK_ARRAY_COUNT(verbs11));
REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
compare.set(&points11[0], SK_ARRAY_COUNT(points11));
REPORTER_ASSERT(reporter, rect == compare);
// isolated from skbug.com/7792 comment 14
SkPath::Verb verbs14[] = { SkPath::kMove_Verb, SkPath::kMove_Verb, SkPath::kMove_Verb,
SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kClose_Verb,
SkPath::kLine_Verb, SkPath::kClose_Verb };
SkPoint points14[] = { {250, 75}, {250, 75}, {250, 75}, {100, 75},
{150, 75}, {150, 150}, {75, 150}, {75, 75}, {0, 0} };
path = makePath2(points14, verbs14, SK_ARRAY_COUNT(verbs14));
REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
compare.set(&points14[3], 5);
REPORTER_ASSERT(reporter, rect == compare);
// isolated from skbug.com/7792 comment 15
SkPath::Verb verbs15[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb, SkPath::kMove_Verb };
SkPoint points15[] = { {75, 75}, {150, 75}, {150, 150}, {75, 150}, {250, 75} };
path = makePath2(points15, verbs15, SK_ARRAY_COUNT(verbs15));
REPORTER_ASSERT(reporter, path.isRect(&rect, nullptr, nullptr));
compare.set(&points15[0], SK_ARRAY_COUNT(points15) - 1);
REPORTER_ASSERT(reporter, rect == compare);
}