Revert of r12450 (Move fIsOval from SkPath to SkPathRef)
authorrobertphillips@google.com <robertphillips@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 3 Dec 2013 00:23:39 +0000 (00:23 +0000)
committerrobertphillips@google.com <robertphillips@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 3 Dec 2013 00:23:39 +0000 (00:23 +0000)
git-svn-id: http://skia.googlecode.com/svn/trunk@12452 2bbb7eff-a529-9590-31e7-b0007b416f81

include/core/SkPath.h
include/core/SkPathRef.h
include/core/SkPicture.h
src/core/SkPath.cpp
src/core/SkPathRef.cpp
tests/PathTest.cpp

index 5c4860e..ac4dd3b 100644 (file)
@@ -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 { return fPathRef->isOval(rect); }
+    bool isOval(SkRect* rect) const;
 
     /** Clear any lines and curves from the path, making it empty. This frees up
         internal storage associated with those segments.
@@ -938,8 +938,8 @@ private:
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
         // rename to kUnused_SerializationShift
         kOldIsFinite_SerializationShift = 25,    // 1 bit
-        kOldIsOval_SerializationShift = 24,    // requires 1 bit
 #endif
+        kIsOval_SerializationShift = 24,    // requires 1 bit
         kConvexity_SerializationShift = 16, // requires 8 bits
         kFillType_SerializationShift = 8,   // requires 8 bits
         kSegmentMask_SerializationShift = 0 // requires 4 bits
@@ -952,6 +952,7 @@ private:
     uint8_t             fSegmentMask;
     mutable uint8_t     fConvexity;
     mutable uint8_t     fDirection;
+    mutable SkBool8     fIsOval;
 #ifdef SK_BUILD_FOR_ANDROID
     const SkPath*       fSourcePath;
 #endif
index 1b3251e..aea0a91 100644 (file)
@@ -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 multiple SkPaths. The caller passes the Editor's
+ * copy-on-write if the SkPathRef is shared by multipls 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,8 +100,6 @@ public:
          */
         SkPathRef* pathRef() { return fPathRef; }
 
-        void setIsOval(bool isOval) { fPathRef->setIsOval(isOval); }
-
     private:
         SkPathRef* fPathRef;
     };
@@ -123,24 +121,6 @@ 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;
     }
@@ -257,7 +237,6 @@ public:
 private:
     enum SerializationOffsets {
         kIsFinite_SerializationShift = 25,  // requires 1 bit
-        kIsOval_SerializationShift = 24,    // requires 1 bit
     };
 
     SkPathRef() {
@@ -268,7 +247,6 @@ private:
         fPoints = NULL;
         fFreeSpace = 0;
         fGenerationID = kEmptyGenID;
-        fIsOval = false;
         SkDEBUGCODE(fEditorsAttached = 0;)
         SkDEBUGCODE(this->validate();)
     }
@@ -311,8 +289,6 @@ 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;
@@ -418,8 +394,6 @@ private:
      */
     static void CreateEmptyImpl(SkPathRef** empty);
 
-    void setIsOval(bool isOval) { fIsOval = isOval; }
-
     enum {
         kMinSize = 256,
     };
@@ -427,7 +401,6 @@ 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)
index 42d9d93..42566ed 100644 (file)
@@ -219,14 +219,13 @@ 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 = 16;
+    static const uint32_t PICTURE_VERSION = 15;
 
     // fPlayback, fRecord, fWidth & fHeight are protected to allow derived classes to
     // install their own SkPicturePlayback-derived players,SkPictureRecord-derived
index a670157..d25ec3c 100644 (file)
@@ -42,6 +42,22 @@ 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) {
@@ -68,7 +84,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.
+    convex (which is always true since we only allow the addition of rects).
  */
 class SkAutoPathBoundsUpdate {
 public:
@@ -148,6 +164,7 @@ 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.
@@ -187,6 +204,7 @@ 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) {
@@ -212,6 +230,7 @@ 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
@@ -612,6 +631,7 @@ 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);
     }
@@ -630,6 +650,7 @@ void SkPath::setConvexity(Convexity c) {
     do {                                 \
         fConvexity = kUnknown_Convexity; \
         fDirection = kUnknown_Direction; \
+        fIsOval = false;                 \
     } while (0)
 
 void SkPath::incReserve(U16CPU inc) {
@@ -1242,13 +1263,14 @@ 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.
      */
-    bool isOval = hasOnlyMoveTos();
-    if (isOval) {
+    fIsOval = hasOnlyMoveTos();
+    if (fIsOval) {
         fDirection = dir;
     } else {
         fDirection = kUnknown_Direction;
     }
 
+    SkAutoDisableOvalCheck adoc(this);
     SkAutoDisableDirectionCheck addc(this);
 
     SkAutoPathBoundsUpdate apbu(this, oval);
@@ -1296,10 +1318,14 @@ void SkPath::addOval(const SkRect& oval, Direction dir) {
         this->quadTo(      R, cy - sy,       R, cy     );
     }
     this->close();
+}
 
-    SkPathRef::Editor ed(&fPathRef);
+bool SkPath::isOval(SkRect* rect) const {
+    if (fIsOval && rect) {
+        *rect = getBounds();
+    }
 
-    ed.setIsOval(isOval);
+    return fIsOval;
 }
 
 void SkPath::addCircle(SkScalar x, SkScalar y, SkScalar r, Direction dir) {
@@ -1433,6 +1459,8 @@ 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;
@@ -1497,6 +1525,8 @@ 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();
@@ -1544,6 +1574,8 @@ 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) {
@@ -1693,6 +1725,9 @@ 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();)
     }
 }
@@ -2045,7 +2080,8 @@ size_t SkPath::writeToMemory(void* storage) const {
 
     SkWBuffer   buffer(storage);
 
-    int32_t packed = (fConvexity << kConvexity_SerializationShift) |
+    int32_t packed = ((fIsOval & 1) << kIsOval_SerializationShift) |
+                     (fConvexity << kConvexity_SerializationShift) |
                      (fFillType << kFillType_SerializationShift) |
                      (fSegmentMask << kSegmentMask_SerializationShift) |
                      (fDirection << kDirection_SerializationShift)
@@ -2070,6 +2106,7 @@ 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;
index b3a9759..c66d75b 100644 (file)
@@ -26,7 +26,6 @@ SkPathRef::Editor::Editor(SkAutoTUnref<SkPathRef>* pathRef,
     }
     fPathRef = *pathRef;
     fPathRef->fGenerationID = 0;
-    fPathRef->fIsOval = false;
     SkDEBUGCODE(sk_atomic_inc(&fPathRef->fEditorsAttached);)
 }
 
@@ -101,9 +100,6 @@ 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();)
 }
 
@@ -113,7 +109,6 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer
 #endif
     ) {
     SkPathRef* ref = SkNEW(SkPathRef);
-    bool isOval;
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
     if (newFormat) {
 #endif
@@ -124,11 +119,9 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer
         }
 
         ref->fIsFinite = (packed >> kIsFinite_SerializationShift) & 1;
-        isOval  = (packed >> kIsOval_SerializationShift) & 1;
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
     } else {
         ref->fIsFinite = (oldPacked >> SkPath::kOldIsFinite_SerializationShift) & 1;
-        isOval  = (oldPacked >> SkPath::kOldIsOval_SerializationShift) & 1;
     }
 #endif
 
@@ -154,7 +147,6 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer
         return NULL;
     }
     ref->fBoundsIsDirty = false;
-    ref->fIsOval = isOval;
     return ref;
 }
 
@@ -167,7 +159,6 @@ 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();
@@ -225,8 +216,7 @@ void SkPathRef::writeToBuffer(SkWBuffer* buffer) {
     // and fIsFinite are computed.
     const SkRect& bounds = this->getBounds();
 
-    int32_t packed = ((fIsFinite & 1) << kIsFinite_SerializationShift) |
-                     ((fIsOval & 1) << kIsOval_SerializationShift);
+    int32_t packed = ((fIsFinite & 1) << kIsFinite_SerializationShift);
     buffer->write32(packed);
 
     // TODO: write gen ID here. Problem: We don't know if we're cross process or not from
@@ -268,18 +258,15 @@ 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;
@@ -294,14 +281,12 @@ 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);
@@ -312,9 +297,6 @@ 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;
 }
index 00e4e5f..131e40b 100644 (file)
@@ -2477,7 +2477,7 @@ static void check_for_circle(skiatest::Reporter* reporter,
                              const SkPath& path,
                              bool expectedCircle,
                              SkPath::Direction expectedDir) {
-    SkRect rect = SkRect::MakeEmpty();
+    SkRect rect;
     REPORTER_ASSERT(reporter, path.isOval(&rect) == expectedCircle);
     REPORTER_ASSERT(reporter, path.cheapIsDirection(expectedDir));