Add isArc to SkPath
This remembers when a path is a simple elliptical arc (ie, it could be
rendered by a call to SkCanvas::drawArc).
For now, this only remembers perimeter arcs (when useCenter is false).
I intend to support useCenter arcs as well, but those will be more
complicated. Regardless, the new interface will support both uses.
Bug: b/330506337
Change-Id: I5c43809e0d57d2e1a4e5704d4027863412d6a739
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/841396
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index a0faea5..8936c9b 100644
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -26,6 +26,7 @@
#include <tuple>
#include <type_traits>
+struct SkArc;
class SkData;
class SkPathRef;
class SkRRect;
@@ -280,6 +281,16 @@
*/
bool isRRect(SkRRect* rrect) const;
+ /** Returns true if path is representable as an oval arc. In other words, could this
+ path be drawn using SkCanvas::drawArc.
+
+ arc receives parameters of arc
+
+ @param arc storage for arc; may be nullptr
+ @return true if SkPath contains only a single arc from an oval
+ */
+ bool isArc(SkArc* arc) const;
+
/** Sets SkPath to its initial state.
Removes verb array, SkPoint array, and weights, and sets FillType to kWinding.
Internal storage associated with SkPath is released.
diff --git a/include/private/SkPathRef.h b/include/private/SkPathRef.h
index 5bf2a39..ba8ef80 100644
--- a/include/private/SkPathRef.h
+++ b/include/private/SkPathRef.h
@@ -8,6 +8,7 @@
#ifndef SkPathRef_DEFINED
#define SkPathRef_DEFINED
+#include "include/core/SkArc.h"
#include "include/core/SkPoint.h"
#include "include/core/SkRect.h"
#include "include/core/SkRefCnt.h"
@@ -63,6 +64,7 @@
kGeneral,
kOval,
kRRect,
+ kArc,
};
SkPathRef(PointsArray points, VerbsArray verbs, ConicWeightsArray weights,
@@ -78,6 +80,9 @@
// The next two values don't matter unless fType is kOval or kRRect
fRRectOrOvalIsCCW = false;
fRRectOrOvalStartIdx = 0xAC;
+ fArcOval.setEmpty();
+ fArcStartAngle = fArcSweepAngle = 0.0f;
+ fArcUseCenter = false;
SkDEBUGCODE(fEditorsAttached.store(0);)
this->computeBounds(); // do this now, before we worry about multiple owners/threads
@@ -160,6 +165,10 @@
fPathRef->setIsRRect(isCCW, start);
}
+ void setIsArc(const SkArc& arc) {
+ fPathRef->setIsArc(arc);
+ }
+
void setBounds(const SkRect& rect) { fPathRef->setBounds(rect); }
private:
@@ -249,6 +258,19 @@
bool isRRect(SkRRect* rrect, bool* isCCW, unsigned* start) const;
+ bool isArc(SkArc* arc) const {
+ if (fType == PathType::kArc) {
+ if (arc) {
+ arc->fOval = fArcOval;
+ arc->fStartAngle = fArcStartAngle;
+ arc->fSweepAngle = fArcSweepAngle;
+ arc->fUseCenter = fArcUseCenter;
+ }
+ }
+
+ return fType == PathType::kArc;
+ }
+
bool hasComputedBounds() const {
return !fBoundsIsDirty;
}
@@ -365,6 +387,9 @@
// The next two values don't matter unless fType is kOval or kRRect
fRRectOrOvalIsCCW = false;
fRRectOrOvalStartIdx = 0xAC;
+ fArcOval.setEmpty();
+ fArcStartAngle = fArcSweepAngle = 0.0f;
+ fArcUseCenter = false;
if (numPoints > 0) {
fPoints.reserve_exact(numPoints);
}
@@ -497,6 +522,14 @@
fRRectOrOvalStartIdx = SkToU8(start);
}
+ void setIsArc(const SkArc& arc) {
+ fType = PathType::kArc;
+ fArcOval = arc.fOval;
+ fArcStartAngle = arc.fStartAngle;
+ fArcSweepAngle = arc.fSweepAngle;
+ fArcUseCenter = arc.fUseCenter;
+ }
+
// called only by the editor. Note that this is not a const function.
SkPoint* getWritablePoints() {
SkDEBUGCODE(this->validate();)
@@ -534,6 +567,12 @@
bool fRRectOrOvalIsCCW;
uint8_t fRRectOrOvalStartIdx;
uint8_t fSegmentMask;
+ // If the path is an arc, these four variables store that information.
+ // We should just store an SkArc, but alignment would cost us 8 more bytes.
+ bool fArcUseCenter;
+ SkRect fArcOval;
+ SkScalar fArcStartAngle;
+ SkScalar fArcSweepAngle;
friend class PathRefTest_Private;
friend class ForceIsRRect_Private; // unit test isRRect
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 2625cf3..27046f7 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -518,6 +518,10 @@
return SkPathPriv::IsRRect(*this, rrect, nullptr, nullptr);
}
+bool SkPath::isArc(SkArc* arc) const {
+ return fPathRef->isArc(arc);
+}
+
int SkPath::countPoints() const {
return fPathRef->countPoints();
}
@@ -1172,10 +1176,12 @@
SkPoint singlePt;
+ bool isArc = this->hasOnlyMoveTos();
+
// Adds a move-to to 'pt' if forceMoveTo is true. Otherwise a lineTo unless we're sufficiently
// 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) {
+ auto addPt = [&forceMoveTo, &isArc, this](const SkPoint& pt) {
SkPoint lastPt;
if (forceMoveTo) {
this->moveTo(pt);
@@ -1183,6 +1189,7 @@
!SkScalarNearlyEqual(lastPt.fX, pt.fX) ||
!SkScalarNearlyEqual(lastPt.fY, pt.fY)) {
this->lineTo(pt);
+ isArc = false;
}
};
@@ -1212,6 +1219,10 @@
for (int i = 0; i < count; ++i) {
this->conicTo(conics[i].fPts[1], conics[i].fPts[2], conics[i].fW);
}
+ if (isArc) {
+ SkPathRef::Editor ed(&fPathRef);
+ ed.setIsArc({oval, startAngle, sweepAngle, false});
+ }
} else {
addPt(singlePt);
}
diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index f53b7cf..d2299c7 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -10,6 +10,7 @@
#include "include/core/SkMatrix.h"
#include "include/core/SkPath.h"
#include "include/core/SkRRect.h"
+#include "include/private/base/SkFloatingPoint.h"
#include "include/private/base/SkOnce.h"
#include "src/base/SkVx.h"
@@ -200,9 +201,12 @@
(*dst)->fSegmentMask = src.fSegmentMask;
- // It's an oval only if it stays a rect.
+ // It's an oval only if it stays a rect. Technically if scale is uniform, then it would stay an
+ // arc. For now, don't bother handling that (we'd also need to fixup the angles for negative
+ // scale, etc.)
bool rectStaysRect = matrix.rectStaysRect();
- const PathType newType = rectStaysRect ? src.fType : PathType::kGeneral;
+ const PathType newType =
+ (rectStaysRect && src.fType != PathType::kArc) ? src.fType : PathType::kGeneral;
(*dst)->fType = newType;
if (newType == PathType::kOval || newType == PathType::kRRect) {
unsigned start = src.fRRectOrOvalStartIdx;
@@ -286,6 +290,10 @@
fType = ref.fType;
fRRectOrOvalIsCCW = ref.fRRectOrOvalIsCCW;
fRRectOrOvalStartIdx = ref.fRRectOrOvalStartIdx;
+ fArcOval = ref.fArcOval;
+ fArcStartAngle = ref.fArcStartAngle;
+ fArcSweepAngle = ref.fArcSweepAngle;
+ fArcUseCenter = ref.fArcUseCenter;
SkDEBUGCODE(this->validate();)
}
@@ -621,6 +629,11 @@
return false;
}
break;
+ case PathType::kArc:
+ if (!(fArcOval.isFinite() && SkIsFinite(fArcStartAngle, fArcSweepAngle))) {
+ return false;
+ }
+ break;
}
if (!fBoundsIsDirty && !fBounds.isEmpty()) {
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 897583a..c289189 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -2438,6 +2438,84 @@
}
+static void test_isArc(skiatest::Reporter* reporter) {
+ SkPath path;
+ REPORTER_ASSERT(reporter, !path.isArc(nullptr));
+
+ // One circle, one oval:
+ const SkRect kOvals[] = { SkRect::MakeWH(100, 100), SkRect::MakeWH(100, 200)};
+
+ // Various start and sweep angles. Note that we can't test with more than a full revolution,
+ // those cases are automatically converted to ovals by SkPath.
+ const SkScalar kStartAngles[] = { -270, -135, -45, 0, 10, 70, 180, 350 };
+ const SkScalar kSweepAngles[] = { -350, -190, -90, -5, 5, 89, 180, 270, 350 };
+
+ int mutator = 0;
+
+ for (SkRect oval : kOvals) {
+ for (SkScalar startAngle : kStartAngles) {
+ for (SkScalar sweepAngle : kSweepAngles) {
+ // For now, isArc only works for arcs where useCenter is false!
+ // TODO: When that's fixed, add more tests cases here.
+ path.rewind();
+ // Include an extra moveTo at the start - this should not interfere with isArc
+ path.moveTo(oval.center());
+ path.addArc(oval, startAngle, sweepAngle);
+
+ SkArc arc;
+ REPORTER_ASSERT(reporter, path.isArc(&arc));
+ REPORTER_ASSERT(reporter,
+ oval == arc.fOval &&
+ startAngle == arc.fStartAngle &&
+ sweepAngle == arc.fSweepAngle &&
+ !arc.fUseCenter);
+
+ // Apply some mutation. All of these should cause the path to no longer be an arc:
+ switch (mutator) {
+ case 0:
+ path.addArc(oval, startAngle, sweepAngle);
+ break;
+ case 1:
+ path.lineTo(oval.center());
+ break;
+ case 2:
+ path.lineTo(path.getPoint(0));
+ break;
+ case 3:
+ path.close();
+ break;
+ case 4:
+ path.moveTo(oval.center());
+ break;
+ default:
+ SkUNREACHABLE;
+ }
+ mutator = (mutator + 1) % 5;
+ REPORTER_ASSERT(reporter, !path.isArc(nullptr));
+ }
+ }
+ }
+
+ // Having any non-move verb before the arc should cause isArc to return false:
+ path.rewind();
+ path.lineTo(kOvals[0].center());
+ path.addArc(kOvals[0], kStartAngles[0], kSweepAngles[0]);
+ REPORTER_ASSERT(reporter, !path.isArc(nullptr));
+
+ // Finally, transforming an arc path by a non-identity should always result in a non-arc path:
+ // TODO: We could clearly preserve arcs for translation, and for scale/rotation with extra work.
+ for (SkMatrix m :
+ {SkMatrix::Translate(10, 10), SkMatrix::RotateDeg(90), SkMatrix::Scale(2, 2)}) {
+ path.rewind();
+ path.addArc(kOvals[0], kStartAngles[0], kSweepAngles[0]);
+ REPORTER_ASSERT(reporter, path.isArc(nullptr));
+ path.transform(SkMatrix::I());
+ REPORTER_ASSERT(reporter, path.isArc(nullptr));
+ path.transform(m);
+ REPORTER_ASSERT(reporter, !path.isArc(nullptr));
+ }
+}
+
static void test_isNestedFillRects(skiatest::Reporter* reporter) {
// passing tests (all moveTo / lineTo...
SkPoint r1[] = {{0, 0}, {1, 0}, {1, 1}, {0, 1}}; // CW
@@ -5014,6 +5092,7 @@
test_isRect(reporter);
test_is_closed_rect(reporter);
test_isNestedFillRects(reporter);
+ test_isArc(reporter);
test_zero_length_paths(reporter);
test_direction(reporter);
test_convexity(reporter);
