Update how SkPath handles fGenerationID and fSourcePath, and add tests to cover.
BUG=
R=bungeman@google.com, reed@google.com
Review URL: https://codereview.chromium.org/22911002
git-svn-id: http://skia.googlecode.com/svn/trunk@10756 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index be15a63..cc4dfcb 100644
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -166,12 +166,14 @@
/** Clear any lines and curves from the path, making it empty. This frees up
internal storage associated with those segments.
+ On Android, does not change fSourcePath.
*/
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.
+ On Android, does not change fSourcePath.
*/
void rewind();
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index ed2f555..47fa552 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -180,7 +180,8 @@
fIsOval = false;
#ifdef SK_BUILD_FOR_ANDROID
GEN_ID_INC;
- fSourcePath = NULL;
+ // We don't touch 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.
#endif
}
@@ -189,7 +190,7 @@
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?
+ fSourcePath = that.fSourcePath;
#endif
SkDEBUGCODE(that.validate();)
}
@@ -206,7 +207,7 @@
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?
+ fSourcePath = that.fSourcePath;
#endif
}
SkDEBUGCODE(this->validate();)
diff --git a/tests/PathTest.cpp b/tests/PathTest.cpp
index 4637e92..e6795d6 100644
--- a/tests/PathTest.cpp
+++ b/tests/PathTest.cpp
@@ -84,18 +84,50 @@
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;
+ // Make sure we treat fGenerationID and fSourcePath correctly for each of
+ // copy, assign, rewind, reset, and swap.
+ SkPath original, source, anotherSource;
+ original.setSourcePath(&source);
original.moveTo(0, 0);
original.lineTo(1, 1);
REPORTER_ASSERT(reporter, original.getGenerationID() > 0);
+ REPORTER_ASSERT(reporter, original.getSourcePath() == &source);
- const SkPath copy(original);
+ uint32_t copyID, assignID;
+
+ // Test copy constructor. Copy generation ID, copy source path.
+ SkPath copy(original);
REPORTER_ASSERT(reporter, copy.getGenerationID() == original.getGenerationID());
+ REPORTER_ASSERT(reporter, copy.getSourcePath() == original.getSourcePath());
+ // Test assigment operator. Increment generation ID, copy source path.
SkPath assign;
+ assignID = assign.getGenerationID();
assign = original;
- REPORTER_ASSERT(reporter, assign.getGenerationID() != original.getGenerationID());
+ REPORTER_ASSERT(reporter, assign.getGenerationID() > assignID);
+ REPORTER_ASSERT(reporter, assign.getSourcePath() == original.getSourcePath());
+
+ // Test rewind. Increment generation ID, don't touch source path.
+ copyID = copy.getGenerationID();
+ copy.rewind();
+ REPORTER_ASSERT(reporter, copy.getGenerationID() > copyID);
+ REPORTER_ASSERT(reporter, copy.getSourcePath() == original.getSourcePath());
+
+ // Test reset. Increment generation ID, don't touch source path.
+ assignID = assign.getGenerationID();
+ assign.reset();
+ REPORTER_ASSERT(reporter, assign.getGenerationID() > assignID);
+ REPORTER_ASSERT(reporter, assign.getSourcePath() == original.getSourcePath());
+
+ // Test swap. Increment both generation IDs, swap source paths.
+ copy.setSourcePath(&anotherSource);
+ copyID = copy.getGenerationID();
+ assignID = assign.getGenerationID();
+ copy.swap(assign);
+ REPORTER_ASSERT(reporter, copy.getGenerationID() > copyID);
+ REPORTER_ASSERT(reporter, assign.getGenerationID() > assignID);
+ REPORTER_ASSERT(reporter, copy.getSourcePath() == original.getSourcePath());
+ REPORTER_ASSERT(reporter, assign.getSourcePath() == &anotherSource);
#endif
}