fix contour-measure for move-line-move
Use RawIter, so we can dependably peek() (since consumeDegenerates in
Iter() make peeking unreliable), which caused us to think there were two
real contours in the test case.
Bug: oss-fuzz:13013
Change-Id: I0d85f3e6a83cb972c4d801dd9b17f0e388b926d0
Reviewed-on: https://skia-review.googlesource.com/c/192025
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
diff --git a/include/core/SkContourMeasure.h b/include/core/SkContourMeasure.h
index f3ec519..7e4a95d 100644
--- a/include/core/SkContourMeasure.h
+++ b/include/core/SkContourMeasure.h
@@ -121,7 +121,7 @@
sk_sp<SkContourMeasure> next();
private:
- SkPath::Iter fIter;
+ SkPath::RawIter fIter;
SkPath fPath;
SkScalar fTolerance;
bool fForceClosed;
@@ -132,6 +132,7 @@
SkContourMeasure* buildSegments();
+ SkScalar compute_line_seg(SkPoint p0, SkPoint p1, SkScalar distance, unsigned ptIndex);
SkScalar compute_quad_segs(const SkPoint pts[3], SkScalar distance,
int mint, int maxt, unsigned ptIndex);
SkScalar compute_conic_segs(const SkConic& conic, SkScalar distance,
diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index f19bae4..183f1c3 100644
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -1485,9 +1485,6 @@
*/
bool isClosedContour() const;
- // Returns the next verb, or kDone, without changing the state of the iterator
- Verb peekVerb() const;
-
private:
const SkPoint* fPts;
const uint8_t* fVerbs;
diff --git a/src/core/SkContourMeasure.cpp b/src/core/SkContourMeasure.cpp
index 79b0ca8..cd4dc93 100644
--- a/src/core/SkContourMeasure.cpp
+++ b/src/core/SkContourMeasure.cpp
@@ -181,6 +181,7 @@
SkScalar prevD = distance;
distance += d;
if (distance > prevD) {
+ SkASSERT(ptIndex < (unsigned)fPts.count());
SkContourMeasure::Segment* seg = fSegments.append();
seg->fDistance = distance;
seg->fPtIndex = ptIndex;
@@ -208,6 +209,7 @@
SkScalar prevD = distance;
distance += d;
if (distance > prevD) {
+ SkASSERT(ptIndex < (unsigned)fPts.count());
SkContourMeasure::Segment* seg = fSegments.append();
seg->fDistance = distance;
seg->fPtIndex = ptIndex;
@@ -232,6 +234,7 @@
SkScalar prevD = distance;
distance += d;
if (distance > prevD) {
+ SkASSERT(ptIndex < (unsigned)fPts.count());
SkContourMeasure::Segment* seg = fSegments.append();
seg->fDistance = distance;
seg->fPtIndex = ptIndex;
@@ -242,13 +245,29 @@
return distance;
}
+SkScalar SkContourMeasureIter::compute_line_seg(SkPoint p0, SkPoint p1, SkScalar distance,
+ unsigned ptIndex) {
+ SkScalar d = SkPoint::Distance(p0, p1);
+ SkASSERT(d >= 0);
+ SkScalar prevD = distance;
+ distance += d;
+ if (distance > prevD) {
+ SkASSERT((unsigned)ptIndex < (unsigned)fPts.count());
+ SkContourMeasure::Segment* seg = fSegments.append();
+ seg->fDistance = distance;
+ seg->fPtIndex = ptIndex;
+ seg->fType = kLine_SegType;
+ seg->fTValue = kMaxTValue;
+ }
+ return distance;
+}
+
SkContourMeasure* SkContourMeasureIter::buildSegments() {
- SkPoint pts[4];
- int ptIndex = -1;
- SkScalar distance = 0;
- bool isClosed = fForceClosed;
- bool firstMoveTo = true;
- SkContourMeasure::Segment* seg;
+ SkPoint pts[4];
+ int ptIndex = -1;
+ SkScalar distance = 0;
+ bool haveSeenClose = fForceClosed;
+ bool haveSeenMoveTo = false;
/* Note:
* as we accumulate distance, we have to check that the result of +=
@@ -257,37 +276,35 @@
*
* We do this check below, and in compute_quad_segs and compute_cubic_segs
*/
+
fSegments.reset();
+ fPts.reset();
+
bool done = false;
do {
- if (!firstMoveTo && fIter.peekVerb() == SkPath::kMove_Verb) {
+ if (haveSeenMoveTo && fIter.peek() == SkPath::kMove_Verb) {
break;
}
switch (fIter.next(pts)) {
case SkPath::kMove_Verb:
ptIndex += 1;
fPts.append(1, pts);
- SkASSERT(firstMoveTo);
- firstMoveTo = false;
+ SkASSERT(!haveSeenMoveTo);
+ haveSeenMoveTo = true;
break;
case SkPath::kLine_Verb: {
- SkScalar d = SkPoint::Distance(pts[0], pts[1]);
- SkASSERT(d >= 0);
+ SkASSERT(haveSeenMoveTo);
SkScalar prevD = distance;
- distance += d;
+ distance = this->compute_line_seg(pts[0], pts[1], distance, ptIndex);
if (distance > prevD) {
- seg = fSegments.append();
- seg->fDistance = distance;
- seg->fPtIndex = ptIndex;
- seg->fType = kLine_SegType;
- seg->fTValue = kMaxTValue;
fPts.append(1, pts + 1);
ptIndex++;
}
} break;
case SkPath::kQuad_Verb: {
+ SkASSERT(haveSeenMoveTo);
SkScalar prevD = distance;
distance = this->compute_quad_segs(pts, distance, 0, kMaxTValue, ptIndex);
if (distance > prevD) {
@@ -297,6 +314,7 @@
} break;
case SkPath::kConic_Verb: {
+ SkASSERT(haveSeenMoveTo);
const SkConic conic(pts, fIter.conicWeight());
SkScalar prevD = distance;
distance = this->compute_conic_segs(conic, distance, 0, conic.fPts[0],
@@ -312,6 +330,7 @@
} break;
case SkPath::kCubic_Verb: {
+ SkASSERT(haveSeenMoveTo);
SkScalar prevD = distance;
distance = this->compute_cubic_segs(pts, distance, 0, kMaxTValue, ptIndex);
if (distance > prevD) {
@@ -321,7 +340,7 @@
} break;
case SkPath::kClose_Verb:
- isClosed = true;
+ haveSeenClose = true;
break;
case SkPath::kDone_Verb:
@@ -338,6 +357,16 @@
return nullptr;
}
+ // Handle the close segment ourselves, since we're using RawIter
+ if (haveSeenClose) {
+ SkScalar prevD = distance;
+ SkPoint firstPt = fPts[0];
+ distance = this->compute_line_seg(fPts[ptIndex], firstPt, distance, ptIndex);
+ if (distance > prevD) {
+ *fPts.append() = firstPt;
+ }
+ }
+
#ifdef SK_DEBUG
{
const SkContourMeasure::Segment* seg = fSegments.begin();
@@ -366,7 +395,7 @@
}
#endif
- return new SkContourMeasure(std::move(fSegments), std::move(fPts), distance, isClosed);
+ return new SkContourMeasure(std::move(fSegments), std::move(fPts), distance, haveSeenClose);
}
static void compute_pos_tan(const SkPoint pts[], unsigned segType,
@@ -419,7 +448,7 @@
fTolerance = CHEAP_DIST_LIMIT * SkScalarInvert(resScale);
fForceClosed = forceClosed;
- fIter.setPath(fPath, forceClosed);
+ fIter.setPath(fPath);
}
SkContourMeasureIter::~SkContourMeasureIter() {}
@@ -434,13 +463,13 @@
}
fForceClosed = forceClosed;
- fIter.setPath(fPath, forceClosed);
+ fIter.setPath(fPath);
fSegments.reset();
fPts.reset();
}
sk_sp<SkContourMeasure> SkContourMeasureIter::next() {
- while (fIter.peekVerb() != SkPath::kDone_Verb) {
+ while (fIter.peek() != SkPath::kDone_Verb) {
auto cm = this->buildSegments();
if (cm) {
return sk_sp<SkContourMeasure>(cm);
@@ -541,6 +570,7 @@
return false;
}
+ SkASSERT((unsigned)seg->fPtIndex < (unsigned)fPts.count());
compute_pos_tan(&fPts[seg->fPtIndex], seg->fType, t, pos, tangent);
return true;
}
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 9f87e5a..23a479d 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -1959,13 +1959,6 @@
return false;
}
-SkPath::Verb SkPath::Iter::peekVerb() const {
- if (fVerbs == nullptr || fVerbs == fVerbStop) {
- return kDone_Verb;
- }
- return (Verb)fVerbs[-1];
-}
-
SkPath::Verb SkPath::Iter::autoClose(SkPoint pts[2]) {
SkASSERT(pts);
if (fLastPt != fMoveTo) {
diff --git a/tests/PathMeasureTest.cpp b/tests/PathMeasureTest.cpp
index c944f86..166fe44 100644
--- a/tests/PathMeasureTest.cpp
+++ b/tests/PathMeasureTest.cpp
@@ -265,6 +265,20 @@
REPORTER_ASSERT(reporter, !fact.next());
}
+static void test_MLM_contours(skiatest::Reporter* reporter) {
+ SkPath path;
+
+ // This odd sequence (with a trailing moveTo) used to return a 2nd contour, which is
+ // wrong, since the contract for a measure is to only return non-zero length contours.
+ path.moveTo(10, 10).lineTo(20, 20).moveTo(30, 30);
+
+ for (bool forceClosed : {false, true}) {
+ SkContourMeasureIter fact(path, forceClosed);
+ REPORTER_ASSERT(reporter, fact.next());
+ REPORTER_ASSERT(reporter, !fact.next());
+ }
+}
+
DEF_TEST(contour_measure, reporter) {
SkPath path;
path.addCircle(0, 0, 100);
@@ -290,4 +304,5 @@
REPORTER_ASSERT(reporter, !cm2);
test_empty_contours(reporter);
+ test_MLM_contours(reporter);
}
