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

path is rect track corners

Browse Source 
This was triggered by an exploit that started the first
edge well outside the final rectangle, causing the captured
to exceed the correct result.

Ivan observes that we really only want the first and third
corners to compute the bounds, so remove the tracking code
that looks for a valid range of points, and record the
corners instead.

R=robertphillips@google.com
Bug: 824145,skia:7792
Change-Id: If228573d0f05c7158dba8142c144d13834e691ec
Reviewed-on: https://skia-review.googlesource.com/122081
Commit-Queue: Cary Clark <caryclark@skia.org>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Auto-Submit: Cary Clark <caryclark@skia.org>
This commit is contained in:
Cary Clark 2018-04-19 07:37:29 -04:00 committed by Skia Commit-Bot
parent 9599b0fd91
commit dbc59ba23b
3 changed files with 80 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
gm/pathfill.cpp
View File

@ -588,4 +588,26 @@ DEF_SIMPLE_GM(bug7792, canvas, 800, 800) {
path.lineTo(150, 75);
path.close();
canvas->drawPath(path, p);
// from skbug.com/7792#c41
canvas->translate(0, 200);
path.reset();
path.moveTo(75, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(140, 150);
path.lineTo(140, 75);
path.moveTo(75, 75);
path.close();
canvas->drawPath(path, p);
// from skbug.com/7792#c53
canvas->translate(0, 200);
path.reset();
path.moveTo(75, 75);
path.lineTo(150, 75);
path.lineTo(150, 150);
path.lineTo(140, 150);
path.lineTo(140, 75);
path.moveTo(75, 75);
path.close();
canvas->drawPath(path, p);
}

88
src/core/SkPath.cpp
View File

@ -454,16 +454,15 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
SkPoint lineStart; // 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* lastCountedPt = nullptr; // point creating 3rd corner
SkPoint firstCorner;
SkPoint thirdCorner;
const SkPoint* pts = *ptsPtr;
const SkPoint* savePts = nullptr; // used to allow caller to iterate through a pair of rects
lineStart.set(0, 0);
signed char directions[] = {-1, -1, -1, -1}; // -1 to 3; -1 is uninitialized
signed char directions[] = {-1, -1, -1, -1, -1}; // -1 to 3; -1 is uninitialized
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);
@ -472,11 +471,7 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
savePts = pts;
autoClose = true;
insertClose = false;
accumulatingRect = false;
case kLine_Verb: {
if (accumulatingRect) {
lastCountedPt = pts;
}
if (kClose_Verb != verb) {
lastPt = pts;
}
@ -488,13 +483,12 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
if (lineStart == lineEnd) {
break; // single point on side OK
}
addedLine = true;
int nextDirection = rect_make_dir(lineDelta.fX, lineDelta.fY); // 0 to 3
if (0 == corners) {
directions[0] = nextDirection;
lineStart = lineEnd;
corners = 1;
closedOrMoved = false;
lineStart = lineEnd;
break;
}
if (closedOrMoved) {
@ -504,28 +498,34 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
break; // colinear with first
}
closedOrMoved = autoClose;
lineStart = lineEnd;
if (directions[corners - 1] == nextDirection) {
if (3 == corners && kLine_Verb == verb) {
lastCountedPt = lastPt;
thirdCorner = lineEnd;
}
lineStart = lineEnd;
break; // colinear segment
}
if (corners >= 4) {
return false; // too many direction changes
}
directions[corners++] = nextDirection;
// opposite lines must point in opposite directions; xoring them should equal 2
if (3 == corners) {
if ((directions[0] ^ directions[2]) != 2) {
return false;
}
accumulatingRect = false;
} else if (4 == corners) {
if ((directions[1] ^ directions[3]) != 2) {
return false;
}
switch (corners) {
case 2:
firstCorner = lineStart;
break;
case 3:
if ((directions[0] ^ directions[2]) != 2) {
return false;
}
thirdCorner = lineEnd;
break;
case 4:
if ((directions[1] ^ directions[3]) != 2) {
return false;
}
break;
default:
return false; // too many direction changes
}
lineStart = lineEnd;
break;
}
case kQuad_Verb:
@ -538,15 +538,13 @@ bool SkPath::isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts
*currVerb -= 1; // try move again afterwards
goto addMissingClose;
}
if (!addedLine) {
if (!corners) {
firstPt = pts;
accumulatingRect = true;
} else {
closeXY = *firstPt - *lastPt;
if (closeXY.fX && closeXY.fY) {
return false; // we're diagonal, abort
}
accumulatingRect = false;
}
lineStart = *pts++;
closedOrMoved = true;
@ -563,40 +561,24 @@ addMissingClose:
if (corners < 3 || corners > 4) {
return false;
}
closeXY = *firstPt - *lastPt;
// If autoClose, check if close generates diagonal
bool result = 4 == corners && (closeXY.isZero() || (autoClose && (!closeXY.fX || !closeXY.fY)));
if (!result) {
if (closeXY.fX && closeXY.fY) {
return false; // we're diagonal, abort
}
// check if we are just an incomplete rectangle, in which case we can
// return true, but not claim to be closed.
// e.g.
// 3 sided rectangle
// 4 sided but the last edge is not long enough to reach the start
//
int closeDirection = rect_make_dir(closeXY.fX, closeXY.fY);
// make sure the close-segment doesn't double-back on itself
if (3 == corners || 2 == (closeDirection ^ directions[1])) {
result = true;
autoClose = false; // we are not closed
}
}
if (savePts) {
*ptsPtr = savePts;
}
if (result && rect) {
ptrdiff_t count = lastCountedPt - firstPt + 1;
rect->set(firstPt, (int) count);
// check if close generates diagonal
closeXY = *firstPt - *lastPt;
if (closeXY.fX && closeXY.fY) {
return false;
}
if (result && isClosed) {
if (rect) {
rect->set(firstCorner, thirdCorner);
}
if (isClosed) {
*isClosed = autoClose;
}
if (result && direction) {
if (direction) {
*direction = directions[0] == ((directions[1] + 1) & 3) ? kCW_Direction : kCCW_Direction;
}
return result;
return true;
}
bool SkPath::isRect(SkRect* rect, bool* isClosed, Direction* direction) const {

27
tests/PathTest.cpp
View File

@ -2033,7 +2033,7 @@ static void test_isRect(skiatest::Reporter* reporter) {
{ c3, SK_ARRAY_COUNT(c3), false, true },
{ d1, SK_ARRAY_COUNT(d1), false, false },
{ d2, SK_ARRAY_COUNT(d2), false, false },
{ d2, SK_ARRAY_COUNT(d2), false, true },
{ d3, SK_ARRAY_COUNT(d3), false, false },
};
@ -2055,7 +2055,8 @@ static void test_isRect(skiatest::Reporter* reporter) {
bool isClosed;
SkPath::Direction direction;
SkPathPriv::FirstDirection cheapDirection;
expected.set(tests[testIndex].fPoints, tests[testIndex].fPointCount);
int pointCount = tests[testIndex].fPointCount - (d2 == tests[testIndex].fPoints);
expected.set(tests[testIndex].fPoints, pointCount);
REPORTER_ASSERT(reporter, SkPathPriv::CheapComputeFirstDirection(path, &cheapDirection));
REPORTER_ASSERT(reporter, path.isRect(&computed, &isClosed, &direction));
REPORTER_ASSERT(reporter, expected == computed);
@ -5020,11 +5021,11 @@ DEF_TEST(Path_isRect, reporter) {
REPORTER_ASSERT(reporter, !path.isRect(&rect));
// isolated from skbug.com/7792#c39
SkPath::Verb verbs39[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb };
SkPath::kLine_Verb };
SkPoint points39[] = { {150, 75}, {150, 150}, {75, 150}, {75, 100} };
path = makePath2(points39, verbs39, SK_ARRAY_COUNT(verbs39));
REPORTER_ASSERT(reporter, !path.isRect(&rect));
// from zero_length_paths_aa
// isolated from zero_length_paths_aa
SkPath::Verb verbsAA[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb, SkPath::kClose_Verb };
@ -5034,4 +5035,22 @@ DEF_TEST(Path_isRect, reporter) {
REPORTER_ASSERT(reporter, path.isRect(&rect));
compare.set(&pointsAA[0], SK_ARRAY_COUNT(pointsAA));
REPORTER_ASSERT(reporter, rect == compare);
// isolated from skbug.com/7792#c41
SkPath::Verb verbs41[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kMove_Verb,
SkPath::kClose_Verb };
SkPoint points41[] = { {75, 75}, {150, 75}, {150, 150}, {140, 150}, {140, 75}, {75, 75} };
path = makePath2(points41, verbs41, SK_ARRAY_COUNT(verbs41));
REPORTER_ASSERT(reporter, path.isRect(&rect));
compare.set(&points41[1], 4);
REPORTER_ASSERT(reporter, rect == compare);
// isolated from skbug.com/7792#c53
SkPath::Verb verbs53[] = { SkPath::kMove_Verb, SkPath::kLine_Verb, SkPath::kLine_Verb,
SkPath::kLine_Verb, SkPath::kLine_Verb, SkPath::kMove_Verb,
SkPath::kClose_Verb };
SkPoint points53[] = { {75, 75}, {150, 75}, {150, 150}, {140, 150}, {140, 75}, {75, 75} };
path = makePath2(points53, verbs53, SK_ARRAY_COUNT(verbs53));
REPORTER_ASSERT(reporter, path.isRect(&rect));
compare.set(&points53[1], 4);
REPORTER_ASSERT(reporter, rect == compare);
}