From 466310dbd3073add2ec934e336c30deaaf702eae Mon Sep 17 00:00:00 2001 From: "robertphillips@google.com" Date: Tue, 3 Dec 2013 16:43:54 +0000 Subject: Move fIsOval from SkPath to SkPathRef https://codereview.chromium.org/89123002/ git-svn-id: http://skia.googlecode.com/svn/trunk@12463 2bbb7eff-a529-9590-31e7-b0007b416f81 --- include/core/SkPath.h | 12 ++++++---- include/core/SkPathRef.h | 31 ++++++++++++++++++++++++-- include/core/SkPicture.h | 6 ++++- src/core/SkPath.cpp | 58 ++++++++++-------------------------------------- src/core/SkPathRef.cpp | 43 +++++++++++++++++++++++++---------- src/core/SkPicture.cpp | 4 ++++ tests/PathTest.cpp | 2 +- 7 files changed, 90 insertions(+), 66 deletions(-) diff --git a/include/core/SkPath.h b/include/core/SkPath.h index ac4dd3bf56..0388739cff 100644 --- a/include/core/SkPath.h +++ b/include/core/SkPath.h @@ -151,7 +151,7 @@ public: * optimization for performance and so some paths that are in * fact ovals can report false. */ - bool isOval(SkRect* rect) const; + bool isOval(SkRect* rect) const { return fPathRef->isOval(rect); } /** Clear any lines and curves from the path, making it empty. This frees up internal storage associated with those segments. @@ -931,6 +931,9 @@ public: private: enum SerializationOffsets { +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO + kNewFormat2_SerializationShift = 29, // requires 1 bit +#endif #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO kNewFormat_SerializationShift = 28, // requires 1 bit #endif @@ -939,7 +942,9 @@ private: // rename to kUnused_SerializationShift kOldIsFinite_SerializationShift = 25, // 1 bit #endif - kIsOval_SerializationShift = 24, // requires 1 bit +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO + kOldIsOval_SerializationShift = 24, // requires 1 bit +#endif kConvexity_SerializationShift = 16, // requires 8 bits kFillType_SerializationShift = 8, // requires 8 bits kSegmentMask_SerializationShift = 0 // requires 4 bits @@ -952,7 +957,6 @@ private: uint8_t fSegmentMask; mutable uint8_t fConvexity; mutable uint8_t fDirection; - mutable SkBool8 fIsOval; #ifdef SK_BUILD_FOR_ANDROID const SkPath* fSourcePath; #endif @@ -1008,7 +1012,7 @@ private: fPathRef->setBounds(rect); } -#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO friend class SkPathRef; // just for SerializationOffsets #endif friend class SkAutoPathBoundsUpdate; diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h index aea0a91275..fd9b339302 100644 --- a/include/core/SkPathRef.h +++ b/include/core/SkPathRef.h @@ -23,7 +23,7 @@ class SkWBuffer; * Holds the path verbs and points. It is versioned by a generation ID. None of its public methods * modify the contents. To modify or append to the verbs/points wrap the SkPathRef in an * SkPathRef::Editor object. Installing the editor resets the generation ID. It also performs - * copy-on-write if the SkPathRef is shared by multipls SkPaths. The caller passes the Editor's + * copy-on-write if the SkPathRef is shared by multiple SkPaths. The caller passes the Editor's * constructor a SkAutoTUnref, which may be updated to point to a new SkPathRef after the editor's * constructor returns. * @@ -100,6 +100,8 @@ public: */ SkPathRef* pathRef() { return fPathRef; } + void setIsOval(bool isOval) { fPathRef->setIsOval(isOval); } + private: SkPathRef* fPathRef; }; @@ -121,6 +123,24 @@ public: return SkToBool(fIsFinite); } + /** Returns true if the path is an oval. + * + * @param rect returns the bounding rect of this oval. It's a circle + * if the height and width are the same. + * + * @return true if this path is an oval. + * Tracking whether a path is an oval is considered an + * optimization for performance and so some paths that are in + * fact ovals can report false. + */ + bool isOval(SkRect* rect) const { + if (fIsOval && NULL != rect) { + *rect = getBounds(); + } + + return SkToBool(fIsOval); + } + bool hasComputedBounds() const { return !fBoundsIsDirty; } @@ -152,7 +172,7 @@ public: const SkMatrix& matrix); static SkPathRef* CreateFromBuffer(SkRBuffer* buffer -#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO , bool newFormat, int32_t oldPacked #endif ); @@ -237,6 +257,7 @@ public: private: enum SerializationOffsets { kIsFinite_SerializationShift = 25, // requires 1 bit + kIsOval_SerializationShift = 24, // requires 1 bit }; SkPathRef() { @@ -247,6 +268,7 @@ private: fPoints = NULL; fFreeSpace = 0; fGenerationID = kEmptyGenID; + fIsOval = false; SkDEBUGCODE(fEditorsAttached = 0;) SkDEBUGCODE(this->validate();) } @@ -289,6 +311,8 @@ private: fBoundsIsDirty = true; // this also invalidates fIsFinite fGenerationID = 0; + fIsOval = false; + size_t newSize = sizeof(uint8_t) * verbCount + sizeof(SkPoint) * pointCount; size_t newReserve = sizeof(uint8_t) * reserveVerbs + sizeof(SkPoint) * reservePoints; size_t minSize = newSize + newReserve; @@ -394,6 +418,8 @@ private: */ static void CreateEmptyImpl(SkPathRef** empty); + void setIsOval(bool isOval) { fIsOval = isOval; } + enum { kMinSize = 256, }; @@ -401,6 +427,7 @@ private: mutable SkRect fBounds; mutable uint8_t fBoundsIsDirty; mutable SkBool8 fIsFinite; // only meaningful if bounds are valid + mutable SkBool8 fIsOval; SkPoint* fPoints; // points to begining of the allocation uint8_t* fVerbs; // points just past the end of the allocation (verbs grow backwards) diff --git a/include/core/SkPicture.h b/include/core/SkPicture.h index 42566eddbd..552a1f9ad5 100644 --- a/include/core/SkPicture.h +++ b/include/core/SkPicture.h @@ -219,13 +219,17 @@ protected: // parameterize blurs by sigma rather than radius // V14: Add flags word to PathRef serialization // V15: Remove A1 bitmpa config (and renumber remaining configs) + // V16: Move SkPath's isOval flag to SkPathRef #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V13_AND_ALL_OTHER_INSTANCES_TOO static const uint32_t PRIOR_PRIOR_PICTURE_VERSION = 12; // TODO: remove when .skps regenerated #endif #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO static const uint32_t PRIOR_PICTURE_VERSION2 = 13; // TODO: remove when .skps regenerated #endif - static const uint32_t PICTURE_VERSION = 15; +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO + static const uint32_t PRIOR_PICTURE_VERSION3 = 15; // TODO: remove when .skps regenerated +#endif + static const uint32_t PICTURE_VERSION = 16; // fPlayback, fRecord, fWidth & fHeight are protected to allow derived classes to // install their own SkPicturePlayback-derived players,SkPictureRecord-derived diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index d25ec3c1df..571452a7d9 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -42,22 +42,6 @@ static bool is_degenerate(const SkPath& path) { return SkPath::kDone_Verb == iter.next(pts); } -class SkAutoDisableOvalCheck { -public: - SkAutoDisableOvalCheck(SkPath* path) : fPath(path) { - fSaved = fPath->fIsOval; - } - - ~SkAutoDisableOvalCheck() { - fPath->fIsOval = fSaved; - } - -private: - SkPath* fPath; - bool fSaved; -}; -#define SkAutoDisableOvalCheck(...) SK_REQUIRE_LOCAL_VAR(SkAutoDisableOvalCheck) - class SkAutoDisableDirectionCheck { public: SkAutoDisableDirectionCheck(SkPath* path) : fPath(path) { @@ -84,7 +68,7 @@ private: It also notes if the path was originally degenerate, and if so, sets isConvex to true. Thus it can only be used if the contour being added is - convex (which is always true since we only allow the addition of rects). + convex. */ class SkAutoPathBoundsUpdate { public: @@ -164,7 +148,6 @@ void SkPath::resetFields() { fSegmentMask = 0; fConvexity = kUnknown_Convexity; fDirection = kUnknown_Direction; - fIsOval = false; // 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. @@ -204,7 +187,6 @@ void SkPath::copyFields(const SkPath& that) { fSegmentMask = that.fSegmentMask; fConvexity = that.fConvexity; fDirection = that.fDirection; - fIsOval = that.fIsOval; } bool operator==(const SkPath& a, const SkPath& b) { @@ -230,7 +212,6 @@ void SkPath::swap(SkPath& that) { SkTSwap(fSegmentMask, that.fSegmentMask); SkTSwap(fConvexity, that.fConvexity); SkTSwap(fDirection, that.fDirection); - SkTSwap(fIsOval, that.fIsOval); #ifdef SK_BUILD_FOR_ANDROID SkTSwap(fSourcePath, that.fSourcePath); #endif @@ -631,7 +612,6 @@ void SkPath::setLastPt(SkScalar x, SkScalar y) { if (count == 0) { this->moveTo(x, y); } else { - fIsOval = false; SkPathRef::Editor ed(&fPathRef); ed.atPoint(count-1)->set(x, y); } @@ -650,7 +630,6 @@ void SkPath::setConvexity(Convexity c) { do { \ fConvexity = kUnknown_Convexity; \ fDirection = kUnknown_Direction; \ - fIsOval = false; \ } while (0) void SkPath::incReserve(U16CPU inc) { @@ -1263,14 +1242,13 @@ void SkPath::addOval(const SkRect& oval, Direction dir) { We can't simply check isEmpty() in this case, as additional moveTo() would mark the path non empty. */ - fIsOval = hasOnlyMoveTos(); - if (fIsOval) { + bool isOval = hasOnlyMoveTos(); + if (isOval) { fDirection = dir; } else { fDirection = kUnknown_Direction; } - SkAutoDisableOvalCheck adoc(this); SkAutoDisableDirectionCheck addc(this); SkAutoPathBoundsUpdate apbu(this, oval); @@ -1318,14 +1296,10 @@ void SkPath::addOval(const SkRect& oval, Direction dir) { this->quadTo( R, cy - sy, R, cy ); } this->close(); -} -bool SkPath::isOval(SkRect* rect) const { - if (fIsOval && rect) { - *rect = getBounds(); - } + SkPathRef::Editor ed(&fPathRef); - return fIsOval; + ed.setIsOval(isOval); } void SkPath::addCircle(SkScalar x, SkScalar y, SkScalar r, Direction dir) { @@ -1459,8 +1433,6 @@ void SkPath::addPath(const SkPath& path, SkScalar dx, SkScalar dy) { void SkPath::addPath(const SkPath& path, const SkMatrix& matrix) { SkPathRef::Editor(&fPathRef, path.countVerbs(), path.countPoints()); - fIsOval = false; - RawIter iter(path); SkPoint pts[4]; Verb verb; @@ -1525,8 +1497,6 @@ void SkPath::reversePathTo(const SkPath& path) { SkPathRef::Editor(&fPathRef, vcount, path.countPoints()); - fIsOval = false; - const uint8_t* verbs = path.fPathRef->verbs(); const SkPoint* pts = path.fPathRef->points(); const SkScalar* conicWeights = path.fPathRef->conicWeights(); @@ -1574,8 +1544,6 @@ void SkPath::reverseAddPath(const SkPath& src) { const uint8_t* verbsEnd = src.fPathRef->verbs(); // points just past the first verb const SkScalar* conicWeights = src.fPathRef->conicWeightsEnd(); - fIsOval = false; - bool needMove = true; bool needClose = false; while (verbs < verbsEnd) { @@ -1725,9 +1693,6 @@ void SkPath::transform(const SkMatrix& matrix, SkPath* dst) const { } } - // It's an oval only if it stays a rect. - dst->fIsOval = fIsOval && matrix.rectStaysRect(); - SkDEBUGCODE(dst->validate();) } } @@ -2080,13 +2045,15 @@ size_t SkPath::writeToMemory(void* storage) const { SkWBuffer buffer(storage); - int32_t packed = ((fIsOval & 1) << kIsOval_SerializationShift) | - (fConvexity << kConvexity_SerializationShift) | + int32_t packed = (fConvexity << kConvexity_SerializationShift) | (fFillType << kFillType_SerializationShift) | (fSegmentMask << kSegmentMask_SerializationShift) | (fDirection << kDirection_SerializationShift) #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO | (0x1 << kNewFormat_SerializationShift) +#endif +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO + | (0x1 << kNewFormat2_SerializationShift) #endif ; @@ -2106,17 +2073,16 @@ size_t SkPath::readFromMemory(const void* storage, size_t length) { return 0; } - fIsOval = (packed >> kIsOval_SerializationShift) & 1; fConvexity = (packed >> kConvexity_SerializationShift) & 0xFF; fFillType = (packed >> kFillType_SerializationShift) & 0xFF; fSegmentMask = (packed >> kSegmentMask_SerializationShift) & 0xF; fDirection = (packed >> kDirection_SerializationShift) & 0x3; -#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO - bool newFormat = (packed >> kNewFormat_SerializationShift) & 1; +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO + bool newFormat = (packed >> kNewFormat2_SerializationShift) & 1; #endif SkPathRef* pathRef = SkPathRef::CreateFromBuffer(&buffer -#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO , newFormat, packed #endif ); diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp index c66d75b3b2..3ad5ae7e02 100644 --- a/src/core/SkPathRef.cpp +++ b/src/core/SkPathRef.cpp @@ -26,6 +26,7 @@ SkPathRef::Editor::Editor(SkAutoTUnref* pathRef, } fPathRef = *pathRef; fPathRef->fGenerationID = 0; + fPathRef->fIsOval = false; SkDEBUGCODE(sk_atomic_inc(&fPathRef->fEditorsAttached);) } @@ -100,28 +101,35 @@ void SkPathRef::CreateTransformedCopy(SkAutoTUnref* dst, (*dst)->fBoundsIsDirty = true; } + // It's an oval only if it stays a rect. + (*dst)->fIsOval = src.fIsOval && matrix.rectStaysRect(); + SkDEBUGCODE((*dst)->validate();) } SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer -#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO , bool newFormat, int32_t oldPacked #endif ) { SkPathRef* ref = SkNEW(SkPathRef); -#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO + bool isOval; + + int32_t packed; + if (!buffer->readS32(&packed)) { + SkDELETE(ref); + return NULL; + } + + ref->fIsFinite = (packed >> kIsFinite_SerializationShift) & 1; + +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO if (newFormat) { #endif - int32_t packed; - if (!buffer->readS32(&packed)) { - SkDELETE(ref); - return NULL; - } - - ref->fIsFinite = (packed >> kIsFinite_SerializationShift) & 1; -#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO + isOval = (packed >> kIsOval_SerializationShift) & 1; +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO } else { - ref->fIsFinite = (oldPacked >> SkPath::kOldIsFinite_SerializationShift) & 1; + isOval = (oldPacked >> SkPath::kOldIsOval_SerializationShift) & 1; } #endif @@ -147,6 +155,7 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer return NULL; } ref->fBoundsIsDirty = false; + ref->fIsOval = isOval; return ref; } @@ -159,6 +168,7 @@ void SkPathRef::Rewind(SkAutoTUnref* pathRef) { (*pathRef)->fFreeSpace = (*pathRef)->currSize(); (*pathRef)->fGenerationID = 0; (*pathRef)->fConicWeights.rewind(); + (*pathRef)->fIsOval = false; SkDEBUGCODE((*pathRef)->validate();) } else { int oldVCnt = (*pathRef)->countVerbs(); @@ -216,7 +226,8 @@ void SkPathRef::writeToBuffer(SkWBuffer* buffer) { // and fIsFinite are computed. const SkRect& bounds = this->getBounds(); - int32_t packed = ((fIsFinite & 1) << kIsFinite_SerializationShift); + int32_t packed = ((fIsFinite & 1) << kIsFinite_SerializationShift) | + ((fIsOval & 1) << kIsOval_SerializationShift); buffer->write32(packed); // TODO: write gen ID here. Problem: We don't know if we're cross process or not from @@ -258,15 +269,18 @@ void SkPathRef::copy(const SkPathRef& ref, fBounds = ref.fBounds; fIsFinite = ref.fIsFinite; } + fIsOval = ref.fIsOval; SkDEBUGCODE(this->validate();) } SkPoint* SkPathRef::growForVerb(int /* SkPath::Verb*/ verb) { SkDEBUGCODE(this->validate();) int pCnt; + bool dirtyAfterEdit = true; switch (verb) { case SkPath::kMove_Verb: pCnt = 1; + dirtyAfterEdit = false; break; case SkPath::kLine_Verb: pCnt = 1; @@ -281,12 +295,14 @@ SkPoint* SkPathRef::growForVerb(int /* SkPath::Verb*/ verb) { break; case SkPath::kClose_Verb: pCnt = 0; + dirtyAfterEdit = false; break; case SkPath::kDone_Verb: SkDEBUGFAIL("growForVerb called for kDone"); // fall through default: SkDEBUGFAIL("default is not reached"); + dirtyAfterEdit = false; pCnt = 0; } size_t space = sizeof(uint8_t) + pCnt * sizeof (SkPoint); @@ -297,6 +313,9 @@ SkPoint* SkPathRef::growForVerb(int /* SkPath::Verb*/ verb) { fPointCnt += pCnt; fFreeSpace -= space; fBoundsIsDirty = true; // this also invalidates fIsFinite + if (dirtyAfterEdit) { + fIsOval = false; + } SkDEBUGCODE(this->validate();) return ret; } diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp index 2520e6b9c9..a5faf4d0a0 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp @@ -292,6 +292,10 @@ bool SkPicture::StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) { #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO // V14 is backwards compatible with V13 && PRIOR_PICTURE_VERSION2 != info.fVersion // TODO: remove when .skps regenerated +#endif +#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V16_AND_ALL_OTHER_INSTANCES_TOO + // V16 is backwards compatible with V15 + && PRIOR_PICTURE_VERSION3 != info.fVersion // TODO: remove when .skps regenerated #endif ) { return false; diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp index 131e40ba45..00e4e5f020 100644 --- a/tests/PathTest.cpp +++ b/tests/PathTest.cpp @@ -2477,7 +2477,7 @@ static void check_for_circle(skiatest::Reporter* reporter, const SkPath& path, bool expectedCircle, SkPath::Direction expectedDir) { - SkRect rect; + SkRect rect = SkRect::MakeEmpty(); REPORTER_ASSERT(reporter, path.isOval(&rect) == expectedCircle); REPORTER_ASSERT(reporter, path.cheapIsDirection(expectedDir)); -- cgit v1.2.3