Move more of SkPath into SkPathRef

include/core/SkPath.h

Index: include/core/SkPath.h
===================================================================
--- include/core/SkPath.h	(revision 11578)
+++ include/core/SkPath.h	(working copy)
@@ -93,12 +93,12 @@
     void toggleInverseFillType() {
         fFillType ^= 2;
         GEN_ID_INC;
-     }
+    }
 
     enum Convexity {
-        kUnknown_Convexity,
-        kConvex_Convexity,
-        kConcave_Convexity
+         kUnknown_Convexity = SkPathRef::kUnknown_Convexity,
+         kConvex_Convexity  = SkPathRef::kConvex_Convexity,
+         kConcave_Convexity = SkPathRef::kConcave_Convexity

[caryclark, 2013/10/03 20:36:15]

I don't agree with exposing the implementation in path ref unnecessarily here. I
would prefer to see compile time asserts in the implementation of path ref that
ensures that the two enums have identical values.

[bsalomon, 2013/10/03 20:40:23]

Would it make sense to promote Convexity to SkConvexity (that is, make it global
in some public header)? Then we wouldn't need two versions of the same enum.

Same for the other enums, though I'm less enthusiastic about promoting Verb out
of SkPath.

     };
 
     /**
@@ -106,11 +106,7 @@
      *  then this function will attempt to compute the convexity (and cache the result).
      */
     Convexity getConvexity() const {
-        if (kUnknown_Convexity != fConvexity) {
-            return static_cast<Convexity>(fConvexity);
-        } else {
-            return this->internalGetConvexity();
-        }
+        return static_cast<Convexity>(fPathRef->getConvexity());
     }
 
     /**
@@ -119,7 +115,9 @@
      *  ComputeConvexity and cache its return value if the current setting is
      *  kUnknown.
      */
-    Convexity getConvexityOrUnknown() const { return (Convexity)fConvexity; }
+    Convexity getConvexityOrUnknown() const { 
+        return static_cast<Convexity>(fPathRef->getConvexityOrUnknown());
+    }
 
     /**
      *  Store a convexity setting in the path. There is no automatic check to
@@ -138,7 +136,7 @@
      *  confirmed by any analysis, it is just the value set earlier.
      */
     bool isConvex() const {
-        return kConvex_Convexity == this->getConvexity();
+        return SkPathRef::kConvex_Convexity == this->getConvexity();

[caryclark, 2013/10/03 20:36:15]

why is this change needed? Isn't the contract that SkPath convexity enums must
equal SkPathRef convexity enums?

     }
 
     /**
@@ -162,7 +160,7 @@
      *              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.
@@ -457,8 +455,8 @@
         @param dy3   The amount to add to the y-coordinate of the last point on
                      this contour, to specify the end point of a cubic curve
     */
-    void    rCubicTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2,
-                     SkScalar x3, SkScalar y3);
+    void rCubicTo(SkScalar x1, SkScalar y1, SkScalar x2, SkScalar y2,
+                  SkScalar x3, SkScalar y3);
 
     /** Append the specified arc to the path as a new contour. If the start of
         the path is different from the path's current last point, then an
@@ -472,8 +470,8 @@
                           treated mod 360.
         @param forceMoveTo If true, always begin a new contour with the arc
     */
-    void    arcTo(const SkRect& oval, SkScalar startAngle, SkScalar sweepAngle,
-                  bool forceMoveTo);
+    void arcTo(const SkRect& oval, SkScalar startAngle, SkScalar sweepAngle,
+               bool forceMoveTo);
 
     /** Append a line and arc to the current path. This is the same as the
         PostScript call "arct".
@@ -495,11 +493,11 @@
 
     enum Direction {
         /** Direction either has not been or could not be computed */
-        kUnknown_Direction,
+        kUnknown_Direction = SkPathRef::kUnknown_Direction,
         /** clockwise direction for adding closed contours */
-        kCW_Direction,
+        kCW_Direction      = SkPathRef::kCW_Direction,
         /** counter-clockwise direction for adding closed contours */
-        kCCW_Direction,
+        kCCW_Direction     = SkPathRef::kCCW_Direction,

[caryclark, 2013/10/03 20:36:15]

same feedback as convexity change above; would prefer to see compile time
asserts verifying equality

     };
 
     /**
@@ -507,10 +505,7 @@
      *  opposite.
      */
     static Direction OppositeDirection(Direction dir) {
-        static const Direction gOppositeDir[] = {
-            kUnknown_Direction, kCCW_Direction, kCW_Direction
-        };
-        return gOppositeDir[dir];
+        return (Direction) SkPathRef::OppositeDirection((SkPathRef::Direction)dir);

[caryclark, 2013/10/03 20:36:15]

you have static casts in similar changes above

     }
 
     /**
@@ -552,7 +547,9 @@
      *  the dir parameter. If the direction was determined, it is cached to make
      *  subsequent calls return quickly.
      */
-    bool cheapComputeDirection(Direction* dir) const;
+    bool cheapComputeDirection(Direction* dir) const {
+        return fPathRef->cheapComputeDirection((SkPathRef::Direction*) dir);

[caryclark, 2013/10/03 20:36:15]

static cast

+    }
 
     /**
      *  Returns true if the path's direction can be computed via
@@ -778,10 +775,10 @@
     }
 
     enum SegmentMask {
-        kLine_SegmentMask   = 1 << 0,
-        kQuad_SegmentMask   = 1 << 1,
-        kConic_SegmentMask  = 1 << 2,
-        kCubic_SegmentMask  = 1 << 3,
+        kLine_SegmentMask   = SkPathRef::kLine_SegmentMask,
+        kQuad_SegmentMask   = SkPathRef::kQuad_SegmentMask,
+        kConic_SegmentMask  = SkPathRef::kConic_SegmentMask,
+        kCubic_SegmentMask  = SkPathRef::kCubic_SegmentMask,

[caryclark, 2013/10/03 20:36:15]

Additionally, this change makes Skia harder to use. I have to look in two places
to associate debugging values with what they do.

     };
 
     /**
@@ -789,16 +786,16 @@
      *  set if the path contains 1 or more segments of that type.
      *  Returns 0 for an empty path (no segments).
      */
-    uint32_t getSegmentMasks() const { return fSegmentMask; }
+    uint32_t getSegmentMasks() const { return fPathRef->getSegmentMasks(); }
 
     enum Verb {
-        kMove_Verb,     //!< iter.next returns 1 point
-        kLine_Verb,     //!< iter.next returns 2 points
-        kQuad_Verb,     //!< iter.next returns 3 points
-        kConic_Verb,    //!< iter.next returns 3 points + iter.conicWeight()
-        kCubic_Verb,    //!< iter.next returns 4 points
-        kClose_Verb,    //!< iter.next returns 1 point (contour's moveTo pt)
-        kDone_Verb,     //!< iter.next returns 0 points
+        kMove_Verb  = SkPathRef::kMove_Verb,  //!< iter.next returns 1 point
+        kLine_Verb  = SkPathRef::kLine_Verb,  //!< iter.next returns 2 points
+        kQuad_Verb  = SkPathRef::kQuad_Verb,  //!< iter.next returns 3 points
+        kConic_Verb = SkPathRef::kConic_Verb, //!< iter.next returns 3 points + iter.conicWeight()
+        kCubic_Verb = SkPathRef::kCubic_Verb, //!< iter.next returns 4 points
+        kClose_Verb = SkPathRef::kClose_Verb, //!< iter.next returns 1 point (contour's moveTo pt)
+        kDone_Verb  = SkPathRef::kDone_Verb,  //!< iter.next returns 0 points

[caryclark, 2013/10/03 20:36:15]

another place to have compile time asserts

     };
 
     /** Iterate through all of the segments (lines, quadratics, cubics) of
@@ -814,8 +811,10 @@
     public:
         Iter();
         Iter(const SkPath&, bool forceClose);
+        Iter(const SkPathRef*, bool forceClose);
 
         void setPath(const SkPath&, bool forceClose);
+        void setPathRef(const SkPathRef*, bool forceClose);
 
         /** Return the next verb in this iteration of the path. When all
             segments have been visited, return kDone_Verb.
@@ -932,26 +931,21 @@
     enum SerializationOffsets {
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
         kNewFormat_SerializationShift = 28, // requires 1 bit
-#endif
-        kDirection_SerializationShift = 26, // requires 2 bits
-#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
-        // rename to kUnused_SerializationShift
+        kOldDirection_SerializationShift = 26, // requires 2 bits
         kOldIsFinite_SerializationShift = 25,    // 1 bit
-#endif
-        kIsOval_SerializationShift = 24,    // requires 1 bit
-        kConvexity_SerializationShift = 16, // requires 8 bits
+        kOldIsOval_SerializationShift = 24,    // requires 1 bit
+        kOldConvexity_SerializationShift = 16, // requires 8 bits
+#endif  
+        // TODO: make this shift be 0
         kFillType_SerializationShift = 8,   // requires 8 bits
-        kSegmentMask_SerializationShift = 0 // requires 4 bits
+#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
+        kOldSegmentMask_SerializationShift = 0 // requires 4 bits
+#endif
     };
 
     SkAutoTUnref<SkPathRef> fPathRef;
 
-    int                 fLastMoveToIndex;
     uint8_t             fFillType;
-    uint8_t             fSegmentMask;
-    mutable uint8_t     fConvexity;
-    mutable uint8_t     fDirection;
-    mutable SkBool8     fIsOval;
 #ifdef SK_BUILD_FOR_ANDROID
     uint32_t            fGenerationID;
     const SkPath*       fSourcePath;
@@ -984,18 +978,8 @@
     */
     void reversePathTo(const SkPath&);
 
-    // called before we add points for lineTo, quadTo, cubicTo, checking to see
-    // if we need to inject a leading moveTo first
-    //
-    //  SkPath path; path.lineTo(...);   <--- need a leading moveTo(0, 0)
-    // SkPath path; ... path.close(); path.lineTo(...) <-- need a moveTo(previous moveTo)
-    //
-    inline void injectMoveToIfNeeded();
-
     inline bool hasOnlyMoveTos() const;
 
-    Convexity internalGetConvexity() const;
-
     bool isRectContour(bool allowPartial, int* currVerb, const SkPoint** pts,
                        bool* isClosed, Direction* direction) const;
 
@@ -1013,12 +997,19 @@
         fPathRef->setBounds(rect);
     }
 
+    Direction getDirection() const {
+        return static_cast<Direction>(fPathRef->getDirection());
+    }
+    void setDirection(Direction dir) {
+        fPathRef->setDirection(static_cast<SkPathRef::Direction>(dir));
+    }
+
+    void setIsOval(bool isOval) { fPathRef->setIsOval(isOval); }
+
 #ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V14_AND_ALL_OTHER_INSTANCES_TOO
     friend class SkPathRef;     // just for SerializationOffsets
 #endif
     friend class SkAutoPathBoundsUpdate;
-    friend class SkAutoDisableOvalCheck;
-    friend class SkAutoDisableDirectionCheck;
     friend class SkBench_AddPathTest; // perf test pathTo/reversePathTo
 };