SkPath::getLastPt() to return optional
- clarifies to the caller that this can fail
- start updating internal call-sites
Change-Id: I1357dafcd473d1ba920e9044aa110a03a9a34a30
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1061316
Owners-Override: Kaylee Lubick <kjlubick@google.com>
Reviewed-by: Kaylee Lubick <kjlubick@google.com>
Reviewed-by: Daniel Dilan <danieldilan@google.com>
Commit-Queue: Mike Reed <mike@reedtribe.org>
diff --git a/docs/examples/Path_getLastPt.cpp b/docs/examples/Path_getLastPt.cpp
index c1de446..2b3b286 100644
--- a/docs/examples/Path_getLastPt.cpp
+++ b/docs/examples/Path_getLastPt.cpp
@@ -3,14 +3,14 @@
#include "tools/fiddle/examples.h"
REG_FIDDLE(Path_getLastPt, 256, 256, true, 0) {
void draw(SkCanvas* canvas) {
- SkPath path;
- path.moveTo(100, 100);
- path.quadTo(100, 20, 20, 100);
+ SkPath path = SkPathBuilder()
+ .moveTo(100, 100)
+ .quadTo(100, 20, 20, 100)
+ .detach();
SkMatrix matrix;
matrix.setRotate(36, 100, 100);
- path.transform(matrix);
- SkPoint last;
- path.getLastPt(&last);
+ path = path.makeTransform(matrix);
+ SkPoint last = path.getLastPt().value_or(SkPoint{0, 0});
SkDebugf("last point: %g, %g\n", last.fX, last.fY);
}
} // END FIDDLE
diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index e9d3210..857d62a 100644
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -611,15 +611,27 @@
}
#endif
- /** Returns last point on SkPath in lastPt. Returns false if SkPoint array is empty,
- storing (0, 0) if lastPt is not nullptr.
+ /** Return the last point, or {}
- @param lastPt storage for final SkPoint in SkPoint array; may be nullptr
- @return true if SkPoint array contains one or more SkPoint
+ @return The last if the path contains one or more SkPoint, else returns {}
example: https://fiddle.skia.org/c/@Path_getLastPt
*/
- bool getLastPt(SkPoint* lastPt) const;
+ std::optional<SkPoint> getLastPt() const;
+
+ // DEPRECATED
+ bool getLastPt(SkPoint* lastPt) const {
+ if (auto lp = this->getLastPt()) {
+ if (lastPt) {
+ *lastPt = *lp;
+ }
+ return true;
+ }
+ if (lastPt) {
+ *lastPt = {0, 0};
+ }
+ return false;
+ }
/** \enum SkPath::SegmentMask
SegmentMask constants correspond to each drawing Verb type in SkPath; for
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index a85c85c..17a68f5 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -507,20 +507,13 @@
return size;
}
-bool SkPath::getLastPt(SkPoint* lastPt) const {
+std::optional<SkPoint> SkPath::getLastPt() const {
SkDEBUGCODE(this->validate();)
- int count = fPathRef->countPoints();
- if (count > 0) {
- if (lastPt) {
- *lastPt = fPathRef->atPoint(count - 1);
- }
- return true;
+ if (const int count = fPathRef->countPoints()) {
+ return fPathRef->atPoint(count - 1);
}
- if (lastPt) {
- lastPt->set(0, 0);
- }
- return false;
+ return {};
}
void SkPath::setPt(int index, SkScalar x, SkScalar y) {
@@ -665,8 +658,7 @@
SkPath& SkPath::rLineTo(SkScalar x, SkScalar y) {
this->injectMoveToIfNeeded(); // This can change the result of this->getLastPt().
- SkPoint pt;
- this->getLastPt(&pt);
+ SkPoint pt = this->getLastPt().value_or(SkPoint{0, 0});
return this->lineTo(pt.fX + x, pt.fY + y);
}
@@ -685,8 +677,7 @@
SkPath& SkPath::rQuadTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2) {
this->injectMoveToIfNeeded(); // This can change the result of this->getLastPt().
- SkPoint pt;
- this->getLastPt(&pt);
+ SkPoint pt = this->getLastPt().value_or(SkPoint{0, 0});
return this->quadTo(pt.fX + x1, pt.fY + y1, pt.fX + x2, pt.fY + y2);
}
@@ -718,8 +709,7 @@
SkPath& SkPath::rConicTo(SkScalar dx1, SkScalar dy1, SkScalar dx2, SkScalar dy2,
SkScalar w) {
this->injectMoveToIfNeeded(); // This can change the result of this->getLastPt().
- SkPoint pt;
- this->getLastPt(&pt);
+ SkPoint pt = this->getLastPt().value_or(SkPoint{0, 0});
return this->conicTo(pt.fX + dx1, pt.fY + dy1, pt.fX + dx2, pt.fY + dy2, w);
}
@@ -741,8 +731,7 @@
SkPath& SkPath::rCubicTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2,
SkScalar x3, SkScalar y3) {
this->injectMoveToIfNeeded(); // This can change the result of this->getLastPt().
- SkPoint pt;
- this->getLastPt(&pt);
+ SkPoint pt = this->getLastPt().value_or(SkPoint{0, 0});
return this->cubicTo(pt.fX + x1, pt.fY + y1, pt.fX + x2, pt.fY + y2,
pt.fX + x3, pt.fY + y3);
}
@@ -1049,13 +1038,15 @@
// close to 'pt' currently. This prevents spurious lineTos when adding a series of contiguous
// arcs from the same oval.
auto addPt = [&forceMoveTo, this](const SkPoint& pt) {
- SkPoint lastPt;
if (forceMoveTo) {
this->moveTo(pt);
- } else if (!this->getLastPt(&lastPt) ||
- !SkScalarNearlyEqual(lastPt.fX, pt.fX) ||
- !SkScalarNearlyEqual(lastPt.fY, pt.fY)) {
- this->lineTo(pt);
+ } else {
+ auto lastPt = this->getLastPt();
+ if (!lastPt ||
+ !SkScalarNearlyEqual(lastPt->fX, pt.fX) ||
+ !SkScalarNearlyEqual(lastPt->fY, pt.fY)) {
+ this->lineTo(pt);
+ }
}
};
@@ -1101,7 +1092,7 @@
SkPathDirection arcSweep, SkScalar x, SkScalar y) {
this->injectMoveToIfNeeded();
SkPoint srcPts[2];
- this->getLastPt(&srcPts[0]);
+ srcPts[0] = *this->getLastPt();
// If rx = 0 or ry = 0 then this arc is treated as a straight line segment (a "lineto")
// joining the endpoints.
// http://www.w3.org/TR/SVG/implnote.html#ArcOutOfRangeParameters
@@ -1227,8 +1218,7 @@
SkPath& SkPath::rArcTo(SkScalar rx, SkScalar ry, SkScalar xAxisRotate, SkPath::ArcSize largeArc,
SkPathDirection sweep, SkScalar dx, SkScalar dy) {
- SkPoint currentPoint;
- this->getLastPt(¤tPoint);
+ SkPoint currentPoint = this->getLastPt().value_or(SkPoint{0, 0});
return this->arcTo(rx, ry, xAxisRotate, largeArc, sweep,
currentPoint.fX + dx, currentPoint.fY + dy);
}
@@ -1269,8 +1259,7 @@
}
// need to know our prev pt so we can construct tangent vectors
- SkPoint start;
- this->getLastPt(&start);
+ SkPoint start = this->getLastPt().value_or(SkPoint{0, 0});
// need double precision for these calcs.
skvx::double2 befored = normalize(skvx::double2{x1 - start.fX, y1 - start.fY});
@@ -1355,9 +1344,9 @@
mapPtsProc(matrix, mappedPts, &pts[0], 1);
if (firstVerb && mode == kExtend_AddPathMode && !isEmpty()) {
injectMoveToIfNeeded(); // In case last contour is closed
- SkPoint lastPt;
+ auto lastPt = this->getLastPt();
// don't add lineTo if it is degenerate
- if (!this->getLastPt(&lastPt) || lastPt != mappedPts[0]) {
+ if (!lastPt.has_value() || *lastPt != mappedPts[0]) {
this->lineTo(mappedPts[0]);
}
} else {
diff --git a/src/pathops/SkOpBuilder.cpp b/src/pathops/SkOpBuilder.cpp
index a923b15..0510134 100644
--- a/src/pathops/SkOpBuilder.cpp
+++ b/src/pathops/SkOpBuilder.cpp
@@ -41,13 +41,13 @@
}
void SkOpBuilder::ReversePath(SkPath* path) {
- SkPath temp;
- SkPoint lastPt;
- SkAssertResult(path->getLastPt(&lastPt));
- temp.moveTo(lastPt);
- temp.reversePathTo(*path);
+ auto lastPt = path->getLastPt();
+ SkASSERT(lastPt.has_value());
+ SkPathBuilder temp;
+ temp.moveTo(*lastPt);
+ SkPathPriv::ReversePathTo(&temp, *path);
temp.close();
- *path = temp;
+ *path = temp.detach();
}
bool SkOpBuilder::FixWinding(SkPath* path) {
diff --git a/src/pathops/SkPathWriter.cpp b/src/pathops/SkPathWriter.cpp
index 4c6a550..0092cc4a 100644
--- a/src/pathops/SkPathWriter.cpp
+++ b/src/pathops/SkPathWriter.cpp
@@ -361,7 +361,9 @@
if (forward) {
next = contour.getPoint(0);
} else {
- SkAssertResult(contour.getLastPt(&next));
+ auto lastPt = contour.getLastPt();
+ SkASSERT(lastPt.has_value());
+ next = *lastPt;
}
if (*prior != next) {
/* TODO: if there is a gap between open path written so far and path to come,
diff --git a/tools/viewer/VariableWidthStrokerSlide.cpp b/tools/viewer/VariableWidthStrokerSlide.cpp
index d92c50b..9bbe375 100644
--- a/tools/viewer/VariableWidthStrokerSlide.cpp
+++ b/tools/viewer/VariableWidthStrokerSlide.cpp
@@ -382,7 +382,7 @@
break;
}
- fCurrVerb.getLastPt(&fPreviousPoint);
+ fPreviousPoint = fCurrVerb.getLastPt().value_or(SkPoint{0, 0});
fMeas.setPath(&fCurrVerb, false);
}
@@ -748,8 +748,7 @@
fOuter.close();
} else {
// Inner last pt == first pt when appending in reverse
- SkPoint innerLastPt;
- fInner.getLastPt(&innerLastPt);
+ SkPoint innerLastPt = fInner.getLastPt().value_or(SkPoint{0, 0});
fOuter.lineTo(innerLastPt);
}
};
