diff options
-rw-r--r-- | include/core/SkPath.h | 26 | ||||
-rw-r--r-- | include/core/SkPathRef.h | 17 | ||||
-rw-r--r-- | src/core/SkPath.cpp | 43 | ||||
-rw-r--r-- | src/core/SkPathRef.cpp | 5 | ||||
-rw-r--r-- | tests/PathTest.cpp | 71 |
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); } |