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

3 files changed, 47 insertions, 58 deletions

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<uint8_t>(fSegmentMask, that.fSegmentMask);
         SkTSwap<uint8_t>(fConvexity, that.fConvexity);
         SkTSwap<uint8_t>(fDirection, that.fDirection);
-        SkTSwap<SkBool8>(fIsOval, that.fIsOval);
 #ifdef SK_BUILD_FOR_ANDROID
         SkTSwap<const SkPath*>(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,14 +2045,16 @@ 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
                      ;
 
     buffer.write32(packed);
@@ -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<SkPathRef>* pathRef,
     }
     fPathRef = *pathRef;
     fPathRef->fGenerationID = 0;
+    fPathRef->fIsOval = false;
     SkDEBUGCODE(sk_atomic_inc(&fPathRef->fEditorsAttached);)
 }
 
@@ -100,28 +101,35 @@ void SkPathRef::CreateTransformedCopy(SkAutoTUnref<SkPathRef>* 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<SkPathRef>* 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
@@ -293,6 +293,10 @@ bool SkPicture::StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
         // 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;
     }