Restore SkPath(const SkPath&) to copy the generation ID on Android.

BUG=
R=bsalomon@google.com, bungeman@google.com, reed@google.com

Review URL: https://codereview.chromium.org/22471002

git-svn-id: http://skia.googlecode.com/svn/trunk@10622 2bbb7eff-a529-9590-31e7-b0007b416f81

include/core/SkPath.h

@@ -40,11 +40,10 @@ public:
     SK_DECLARE_INST_COUNT_ROOT(SkPath);
 
     SkPath();
-    SkPath(const SkPath&);
+    SkPath(const SkPath&);  // Copies fGenerationID on Android.
     ~SkPath();
 
-    SkPath& operator=(const SkPath&);
+    SkPath& operator=(const SkPath&);  // Increments fGenerationID on Android.
-
     friend  SK_API bool operator==(const SkPath&, const SkPath&);
     friend bool operator!=(const SkPath& a, const SkPath& b) {
         return !(a == b);
@@ -167,14 +166,12 @@ public:
 
     /** Clear any lines and curves from the path, making it empty. This frees up
         internal storage associated with those segments.
-        This does NOT change the fill-type setting nor isConvex
     */
     void reset();
 
     /** Similar to reset(), in that all lines and curves are removed from the
         path. However, any internal storage for those lines/curves is retained,
         making reuse of the path potentially faster.
-        This does NOT change the fill-type setting nor isConvex
     */
     void rewind();
 
@@ -965,8 +962,7 @@ private:
 
     /** Sets all fields other than fPathRef to the values in 'that'.
      *  Assumes the caller has already set fPathRef.
-     *  On Android increments fGenerationID without copying it.
+     *  Doesn't change fGenerationID or fSourcePath on Android.
-     *  On Android sets fSourcePath to NULL.
      */
     void copyFields(const SkPath& that);
 

src/core/SkPath.cpp

@@ -185,12 +185,12 @@ void SkPath::resetFields() {
 }
 
 SkPath::SkPath(const SkPath& that)
-    : fPathRef(SkRef(that.fPathRef.get()))
+    : fPathRef(SkRef(that.fPathRef.get())) {
-#ifdef SK_BUILD_FOR_ANDROID
-    , fGenerationID(0)
-#endif
-{
     this->copyFields(that);
+#ifdef SK_BUILD_FOR_ANDROID
+    fGenerationID = that.fGenerationID;
+    fSourcePath   = NULL;  // TODO(mtklein): follow up with Android: do we want to copy this too?
+#endif
     SkDEBUGCODE(that.validate();)
 }
 
@@ -204,6 +204,10 @@ SkPath& SkPath::operator=(const SkPath& that) {
     if (this != &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 = NULL;  // TODO(mtklein): follow up with Android: do we want to copy this too?
+#endif
     }
     SkDEBUGCODE(this->validate();)
     return *this;
@@ -220,10 +224,6 @@ void SkPath::copyFields(const SkPath& that) {
     fDirection       = that.fDirection;
     fIsFinite        = that.fIsFinite;
     fIsOval          = that.fIsOval;
-#ifdef SK_BUILD_FOR_ANDROID
-    GEN_ID_INC;
-    fSourcePath      = NULL;
-#endif
 }
 
 SK_API bool operator==(const SkPath& a, const SkPath& b) {
@@ -253,8 +253,13 @@ void SkPath::swap(SkPath& that) {
         SkTSwap<uint8_t>(fDirection, that.fDirection);
         SkTSwap<SkBool8>(fIsFinite, that.fIsFinite);
         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
     }
 }
 

tests/PathTest.cpp

@@ -15,8 +15,9 @@
 #include "SkRandom.h"
 #include "SkReader32.h"
 #include "SkSize.h"
-#include "SkWriter32.h"
 #include "SkSurface.h"
+#include "SkTypes.h"
+#include "SkWriter32.h"
 
 #if defined(WIN32)
     #define SUPPRESS_VISIBILITY_WARNING
@@ -31,6 +32,23 @@ static SkSurface* new_surface(int w, int h) {
     return SkSurface::NewRaster(info);
 }
 
+static void test_android_specific_behavior(skiatest::Reporter* reporter) {
+#ifdef SK_BUILD_FOR_ANDROID
+    // Copy constructor should preserve generation ID, but assignment shouldn't.
+    SkPath original;
+    original.moveTo(0, 0);
+    original.lineTo(1, 1);
+    REPORTER_ASSERT(reporter, original.getGenerationID() > 0);
+
+    const SkPath copy(original);
+    REPORTER_ASSERT(reporter, copy.getGenerationID() == original.getGenerationID());
+
+    SkPath assign;
+    assign = original;
+    REPORTER_ASSERT(reporter, assign.getGenerationID() != original.getGenerationID());
+#endif
+}
+
 // This used to assert in the debug build, as the edges did not all line-up.
 static void test_bad_cubic_crbug234190() {
     SkPath path;
@@ -2450,6 +2468,7 @@ static void TestPath(skiatest::Reporter* reporter) {
     test_crbug_170666();
     test_bad_cubic_crbug229478();
     test_bad_cubic_crbug234190();
+    test_android_specific_behavior(reporter);
 }
 
 #include "TestClassDef.h"