aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--include/core/SkPath.h26
-rw-r--r--include/core/SkPathRef.h17
-rw-r--r--src/core/SkPath.cpp43
-rw-r--r--src/core/SkPathRef.cpp5
-rw-r--r--tests/PathTest.cpp71
5 files changed, 95 insertions, 67 deletions
diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index a6674d9afb..9b5dc4bf65 100644
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -16,14 +16,6 @@
#include "SkTDArray.h"
#include "SkRefCnt.h"
-#ifdef SK_BUILD_FOR_ANDROID
-#define GEN_ID_INC fGenerationID++
-#define GEN_ID_PTR_INC(ptr) (ptr)->fGenerationID++
-#else
-#define GEN_ID_INC
-#define GEN_ID_PTR_INC(ptr)
-#endif
-
class SkReader32;
class SkWriter32;
class SkAutoPathBoundsUpdate;
@@ -40,10 +32,10 @@ public:
SK_DECLARE_INST_COUNT_ROOT(SkPath);
SkPath();
- SkPath(const SkPath&); // Copies fGenerationID on Android.
+ SkPath(const SkPath&);
~SkPath();
- SkPath& operator=(const SkPath&); // Increments fGenerationID on Android.
+ SkPath& operator=(const SkPath&);
friend SK_API bool operator==(const SkPath&, const SkPath&);
friend bool operator!=(const SkPath& a, const SkPath& b) {
return !(a == b);
@@ -80,7 +72,6 @@ public:
*/
void setFillType(FillType ft) {
fFillType = SkToU8(ft);
- GEN_ID_INC;
}
/** Returns true if the filltype is one of the Inverse variants */
@@ -92,7 +83,6 @@ public:
*/
void toggleInverseFillType() {
fFillType ^= 2;
- GEN_ID_INC;
}
enum Convexity {
@@ -914,16 +904,25 @@ public:
* If buffer is NULL, it still returns the number of bytes.
*/
uint32_t writeToMemory(void* buffer) const;
+
/**
* Initialized the region from the buffer, returning the number
* of bytes actually read.
*/
uint32_t readFromMemory(const void* buffer);
-#ifdef SK_BUILD_FOR_ANDROID
+ /** Returns a non-zero, globally unique value corresponding to the set of verbs
+ and points in the path (but not the fill type [except on Android skbug.com/1762]).
+ Each time the path is modified, a different generation ID will be returned.
+ */
uint32_t getGenerationID() const;
+
+#ifdef SK_BUILD_FOR_ANDROID
+ static const int kPathRefGenIDBitCnt = 30; // leave room for the fill type (skbug.com/1762)
const SkPath* getSourcePath() const;
void setSourcePath(const SkPath* path);
+#else
+ static const int kPathRefGenIDBitCnt = 32;
#endif
SkDEBUGCODE(void validate() const;)
@@ -953,7 +952,6 @@ private:
mutable uint8_t fDirection;
mutable SkBool8 fIsOval;
#ifdef SK_BUILD_FOR_ANDROID
- uint32_t fGenerationID;
const SkPath* fSourcePath;
#endif
diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h
index d832944ec3..aea0a91275 100644
--- a/include/core/SkPathRef.h
+++ b/include/core/SkPathRef.h
@@ -227,6 +227,13 @@ public:
*/
uint32_t writeSize();
+ /**
+ * Gets an ID that uniquely identifies the contents of the path ref. If two path refs have the
+ * same ID then they have the same verbs and points. However, two path refs may have the same
+ * contents but different genIDs.
+ */
+ uint32_t genID() const;
+
private:
enum SerializationOffsets {
kIsFinite_SerializationShift = 25, // requires 1 bit
@@ -380,14 +387,6 @@ private:
return reinterpret_cast<intptr_t>(fVerbs) - reinterpret_cast<intptr_t>(fPoints);
}
- /**
- * Gets an ID that uniquely identifies the contents of the path ref. If two path refs have the
- * same ID then they have the same verbs and points. However, two path refs may have the same
- * contents but different genIDs. Zero is reserved and means an ID has not yet been determined
- * for the path ref.
- */
- int32_t genID() const;
-
SkDEBUGCODE(void validate() const;)
/**
@@ -413,7 +412,7 @@ private:
enum {
kEmptyGenID = 1, // GenID reserved for path ref with zero points and zero verbs.
};
- mutable int32_t fGenerationID;
+ mutable uint32_t fGenerationID;
SkDEBUGCODE(int32_t fEditorsAttached;) // assert that only one editor in use at any time.
typedef SkRefCnt INHERITED;
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 8f79fbe370..83f481a91b 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -147,7 +147,7 @@ private:
SkPath::SkPath()
: fPathRef(SkPathRef::CreateEmpty())
#ifdef SK_BUILD_FOR_ANDROID
- , fGenerationID(0)
+ , fSourcePath(NULL)
#endif
{
this->resetFields();
@@ -161,18 +161,15 @@ void SkPath::resetFields() {
fConvexity = kUnknown_Convexity;
fDirection = kUnknown_Direction;
fIsOval = false;
-#ifdef SK_BUILD_FOR_ANDROID
- GEN_ID_INC;
- // We don't touch fSourcePath. It's used to track texture garbage collection, so we don't
- // want to muck with it if it's been set to something non-NULL.
-#endif
+
+ // We don't touch Android's fSourcePath. It's used to track texture garbage collection, so we
+ // don't want to muck with it if it's been set to something non-NULL.
}
SkPath::SkPath(const SkPath& that)
: fPathRef(SkRef(that.fPathRef.get())) {
this->copyFields(that);
#ifdef SK_BUILD_FOR_ANDROID
- fGenerationID = that.fGenerationID;
fSourcePath = that.fSourcePath;
#endif
SkDEBUGCODE(that.validate();)
@@ -189,7 +186,6 @@ SkPath& SkPath::operator=(const SkPath& that) {
fPathRef.reset(SkRef(that.fPathRef.get()));
this->copyFields(that);
#ifdef SK_BUILD_FOR_ANDROID
- GEN_ID_INC; // Similar to swap, we can't just copy this or it could go back in time.
fSourcePath = that.fSourcePath;
#endif
}
@@ -232,10 +228,6 @@ void SkPath::swap(SkPath& that) {
SkTSwap<uint8_t>(fDirection, that.fDirection);
SkTSwap<SkBool8>(fIsOval, that.fIsOval);
#ifdef SK_BUILD_FOR_ANDROID
- // It doesn't really make sense to swap the generation IDs here, because they might go
- // backwards. To be safe we increment both to mark them both as changed.
- GEN_ID_INC;
- GEN_ID_PTR_INC(&that);
SkTSwap<const SkPath*>(fSourcePath, that.fSourcePath);
#endif
}
@@ -328,11 +320,16 @@ bool SkPath::conservativelyContainsRect(const SkRect& rect) const {
return check_edge_against_rect(prevPt, firstPt, rect, direction);
}
-#ifdef SK_BUILD_FOR_ANDROID
uint32_t SkPath::getGenerationID() const {
- return fGenerationID;
+ uint32_t genID = fPathRef->genID();
+#ifdef SK_BUILD_FOR_ANDROID
+ SkASSERT((unsigned)fFillType < (1 << (32 - kPathRefGenIDBitCnt)));
+ genID |= static_cast<uint32_t>(fFillType) << kPathRefGenIDBitCnt;
+#endif
+ return genID;
}
+#ifdef SK_BUILD_FOR_ANDROID
const SkPath* SkPath::getSourcePath() const {
return fSourcePath;
}
@@ -633,14 +630,12 @@ void SkPath::setLastPt(SkScalar x, SkScalar y) {
fIsOval = false;
SkPathRef::Editor ed(&fPathRef);
ed.atPoint(count-1)->set(x, y);
- GEN_ID_INC;
}
}
void SkPath::setConvexity(Convexity c) {
if (fConvexity != c) {
fConvexity = c;
- GEN_ID_INC;
}
}
@@ -669,8 +664,6 @@ void SkPath::moveTo(SkScalar x, SkScalar y) {
fLastMoveToIndex = ed.pathRef()->countPoints();
ed.growForVerb(kMove_Verb)->set(x, y);
-
- GEN_ID_INC;
}
void SkPath::rMoveTo(SkScalar x, SkScalar y) {
@@ -702,7 +695,6 @@ void SkPath::lineTo(SkScalar x, SkScalar y) {
ed.growForVerb(kLine_Verb)->set(x, y);
fSegmentMask |= kLine_SegmentMask;
- GEN_ID_INC;
DIRTY_AFTER_EDIT;
}
@@ -724,7 +716,6 @@ void SkPath::quadTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2) {
pts[1].set(x2, y2);
fSegmentMask |= kQuad_SegmentMask;
- GEN_ID_INC;
DIRTY_AFTER_EDIT;
}
@@ -756,7 +747,6 @@ void SkPath::conicTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2,
pts[1].set(x2, y2);
fSegmentMask |= kConic_SegmentMask;
- GEN_ID_INC;
DIRTY_AFTER_EDIT;
}
}
@@ -782,7 +772,6 @@ void SkPath::cubicTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2,
pts[2].set(x3, y3);
fSegmentMask |= kCubic_SegmentMask;
- GEN_ID_INC;
DIRTY_AFTER_EDIT;
}
@@ -808,7 +797,6 @@ void SkPath::close() {
case kMove_Verb: {
SkPathRef::Editor ed(&fPathRef);
ed.growForVerb(kClose_Verb);
- GEN_ID_INC;
break;
}
case kClose_Verb:
@@ -894,7 +882,6 @@ void SkPath::addPoly(const SkPoint pts[], int count, bool close) {
vb[~count] = kClose_Verb;
}
- GEN_ID_INC;
DIRTY_AFTER_EDIT;
SkDEBUGCODE(this->validate();)
}
@@ -1720,12 +1707,6 @@ void SkPath::transform(const SkMatrix& matrix, SkPath* dst) const {
dst->fConvexity = fConvexity;
}
-#ifdef SK_BUILD_FOR_ANDROID
- if (!matrix.isIdentity() && !dst->hasComputedBounds()) {
- GEN_ID_PTR_INC(dst);
- }
-#endif
-
if (kUnknown_Direction == fDirection) {
dst->fDirection = kUnknown_Direction;
} else {
@@ -2134,8 +2115,6 @@ uint32_t SkPath::readFromMemory(const void* storage) {
buffer.skipToAlign4();
- GEN_ID_INC;
-
SkDEBUGCODE(this->validate();)
return SkToU32(buffer.pos());
}
diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index f635c2a3e5..f811b245ec 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -289,8 +289,9 @@ SkPoint* SkPathRef::growForVerb(int /* SkPath::Verb*/ verb) {
return ret;
}
-int32_t SkPathRef::genID() const {
+uint32_t SkPathRef::genID() const {
SkASSERT(!fEditorsAttached);
+ static const uint32_t kMask = (static_cast<int64_t>(1) << SkPath::kPathRefGenIDBitCnt) - 1;
if (!fGenerationID) {
if (0 == fPointCnt && 0 == fVerbCnt) {
fGenerationID = kEmptyGenID;
@@ -299,7 +300,7 @@ int32_t SkPathRef::genID() const {
// do a loop in case our global wraps around, as we never want to return a 0 or the
// empty ID
do {
- fGenerationID = sk_atomic_inc(&gPathRefGenerationID) + 1;
+ fGenerationID = (sk_atomic_inc(&gPathRefGenerationID) + 1) & kMask;
} while (fGenerationID <= kEmptyGenID);
}
}
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index c47b2c4858..6544c0b8d7 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -127,7 +127,6 @@ static void test_android_specific_behavior(skiatest::Reporter* reporter) {
original.setSourcePath(&source);
original.moveTo(0, 0);
original.lineTo(1, 1);
- REPORTER_ASSERT(reporter, original.getGenerationID() > 0);
REPORTER_ASSERT(reporter, original.getSourcePath() == &source);
uint32_t copyID, assignID;
@@ -137,37 +136,88 @@ static void test_android_specific_behavior(skiatest::Reporter* reporter) {
REPORTER_ASSERT(reporter, copy.getGenerationID() == original.getGenerationID());
REPORTER_ASSERT(reporter, copy.getSourcePath() == original.getSourcePath());
- // Test assigment operator. Increment generation ID, copy source path.
+ // Test assigment operator. Change generation ID, copy source path.
SkPath assign;
assignID = assign.getGenerationID();
assign = original;
- REPORTER_ASSERT(reporter, assign.getGenerationID() > assignID);
+ REPORTER_ASSERT(reporter, assign.getGenerationID() != assignID);
REPORTER_ASSERT(reporter, assign.getSourcePath() == original.getSourcePath());
- // Test rewind. Increment generation ID, don't touch source path.
+ // Test rewind. Change generation ID, don't touch source path.
copyID = copy.getGenerationID();
copy.rewind();
- REPORTER_ASSERT(reporter, copy.getGenerationID() > copyID);
+ REPORTER_ASSERT(reporter, copy.getGenerationID() != copyID);
REPORTER_ASSERT(reporter, copy.getSourcePath() == original.getSourcePath());
- // Test reset. Increment generation ID, don't touch source path.
+ // Test reset. Change generation ID, don't touch source path.
assignID = assign.getGenerationID();
assign.reset();
- REPORTER_ASSERT(reporter, assign.getGenerationID() > assignID);
+ REPORTER_ASSERT(reporter, assign.getGenerationID() != assignID);
REPORTER_ASSERT(reporter, assign.getSourcePath() == original.getSourcePath());
- // Test swap. Increment both generation IDs, swap source paths.
+ // Test swap. Swap the generation IDs, swap source paths.
+ copy.reset();
+ copy.moveTo(2, 2);
copy.setSourcePath(&anotherSource);
copyID = copy.getGenerationID();
+ assign.moveTo(3, 3);
assignID = assign.getGenerationID();
copy.swap(assign);
- REPORTER_ASSERT(reporter, copy.getGenerationID() > copyID);
- REPORTER_ASSERT(reporter, assign.getGenerationID() > assignID);
+ REPORTER_ASSERT(reporter, copy.getGenerationID() != copyID);
+ REPORTER_ASSERT(reporter, assign.getGenerationID() != assignID);
REPORTER_ASSERT(reporter, copy.getSourcePath() == original.getSourcePath());
REPORTER_ASSERT(reporter, assign.getSourcePath() == &anotherSource);
#endif
}
+static void test_gen_id(skiatest::Reporter* reporter) {
+ SkPath a, b;
+ REPORTER_ASSERT(reporter, a.getGenerationID() == b.getGenerationID());
+
+ a.moveTo(0, 0);
+ const uint32_t z = a.getGenerationID();
+ REPORTER_ASSERT(reporter, z != b.getGenerationID());
+
+ a.reset();
+ REPORTER_ASSERT(reporter, a.getGenerationID() == b.getGenerationID());
+
+ a.moveTo(1, 1);
+ const uint32_t y = a.getGenerationID();
+ REPORTER_ASSERT(reporter, z != y);
+
+ b.moveTo(2, 2);
+ const uint32_t x = b.getGenerationID();
+ REPORTER_ASSERT(reporter, x != y && x != z);
+
+ a.swap(b);
+ REPORTER_ASSERT(reporter, b.getGenerationID() == y && a.getGenerationID() == x);
+
+ b = a;
+ REPORTER_ASSERT(reporter, b.getGenerationID() == x);
+
+ SkPath c(a);
+ REPORTER_ASSERT(reporter, c.getGenerationID() == x);
+
+ c.lineTo(3, 3);
+ const uint32_t w = c.getGenerationID();
+ REPORTER_ASSERT(reporter, b.getGenerationID() == x);
+ REPORTER_ASSERT(reporter, a.getGenerationID() == x);
+ REPORTER_ASSERT(reporter, w != x);
+
+#ifdef SK_BUILD_FOR_ANDROID
+ static bool kExpectGenIDToIgnoreFill = false;
+#else
+ static bool kExpectGenIDToIgnoreFill = true;
+#endif
+
+ c.toggleInverseFillType();
+ const uint32_t v = c.getGenerationID();
+ REPORTER_ASSERT(reporter, (v == w) == kExpectGenIDToIgnoreFill);
+
+ c.rewind();
+ REPORTER_ASSERT(reporter, v != c.getGenerationID());
+}
+
// This used to assert in the debug build, as the edges did not all line-up.
static void test_bad_cubic_crbug234190() {
SkPath path;
@@ -2620,6 +2670,7 @@ static void TestPath(skiatest::Reporter* reporter) {
test_bad_cubic_crbug229478();
test_bad_cubic_crbug234190();
test_android_specific_behavior(reporter);
+ test_gen_id(reporter);
test_path_close_issue1474(reporter);
test_path_to_region(reporter);
}