WIP -- separate out Parts from SkPaint

src/core/SkPaint.cpp

Index: src/core/SkPaint.cpp
diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp
index 9cb61bd9d128707299306f36f695bd2eb7f1d9df..d2b1e04dd20908d1bc1e9933364a01635126fee3 100644
--- a/src/core/SkPaint.cpp
+++ b/src/core/SkPaint.cpp
@@ -35,6 +35,32 @@
 #include "SkTypeface.h"
 #include "SkXfermode.h"
 
+bool SkPaintParts::operator==(const SkPaintParts& other) const {

[mtklein, 2014/04/15 14:32:44]

Give him his own .cpp?

[reed1, 2014/04/18 14:04:31]

Eventually yes.

+#define EQUAL(field) (this->field == other.field)
+    return EQUAL(fTypeface)
+        && EQUAL(fPathEffect)
+        && EQUAL(fShader)
+        && EQUAL(fXfermode)
+        && EQUAL(fMaskFilter)
+        && EQUAL(fColorFilter)
+        && EQUAL(fRasterizer)
+        && EQUAL(fLooper)
+        && EQUAL(fImageFilter)
+        && EQUAL(fAnnotation)
+        && EQUAL(fTextSize)
+        && EQUAL(fTextScaleX)
+        && EQUAL(fTextSkewX)
+        && EQUAL(fColor)
+        && EQUAL(fWidth)
+        && EQUAL(fMiterLimit)
+        && EQUAL(fBitfields)
+        ;
+#undef EQUAL
+}
+
+///////////
+
+
 enum {
     kColor_DirtyBit               = 1 <<  0,
     kBitfields_DirtyBit           = 1 <<  1,
@@ -69,33 +95,20 @@ enum {
 #endif
 
 SkPaint::SkPaint() {
-    fTypeface    = NULL;
-    fPathEffect  = NULL;
-    fShader      = NULL;
-    fXfermode    = NULL;
-    fMaskFilter  = NULL;
-    fColorFilter = NULL;
-    fRasterizer  = NULL;
-    fLooper      = NULL;
-    fImageFilter = NULL;
-    fAnnotation  = NULL;
-
-    fTextSize     = SkPaintDefaults_TextSize;
-    fTextScaleX   = SK_Scalar1;
-    fTextSkewX    = 0;
-    fColor        = SK_ColorBLACK;
-    fWidth        = 0;
-    fMiterLimit   = SkPaintDefaults_MiterLimit;
-
-    // Zero all bitfields, then set some non-zero defaults.
-    fBitfields    = 0;
-    fFlags        = SkPaintDefaults_Flags;
-    fCapType      = kDefault_Cap;
-    fJoinType     = kDefault_Join;
-    fTextAlign    = kLeft_Align;
-    fStyle        = kFill_Style;
-    fTextEncoding = kUTF8_TextEncoding;
-    fHinting      = SkPaintDefaults_Hinting;
+    // set any non-zero values in fParts
+
+    fParts.fTextSize     = SkPaintDefaults_TextSize;
+    fParts.fTextScaleX   = SK_Scalar1;
+    fParts.fColor        = SK_ColorBLACK;
+    fParts.fMiterLimit   = SkPaintDefaults_MiterLimit;
+
+    fParts.fFlags        = SkPaintDefaults_Flags;
+    fParts.fCapType      = SkPaintParts::kDefault_Cap;
+    fParts.fJoinType     = SkPaintParts::kDefault_Join;
+    fParts.fTextAlign    = SkPaintParts::kLeft_Align;
+    fParts.fStyle        = SkPaintParts::kFill_Style;
+    fParts.fTextEncoding = kUTF8_TextEncoding;
+    fParts.fHinting      = SkPaintDefaults_Hinting;
 
     fDirtyBits    = 0;
 #ifdef SK_BUILD_FOR_ANDROID
@@ -105,8 +118,8 @@ SkPaint::SkPaint() {
 }
 
 SkPaint::SkPaint(const SkPaint& src) {
-#define COPY(field) field = src.field
-#define REF_COPY(field) field = SkSafeRef(src.field)
+#define COPY(field) fParts.field = src.fParts.field
+#define REF_COPY(field) fParts.field = SkSafeRef(src.fParts.field)
 

[tomhudson, 2014/04/16 11:41:31]

operator= asserts that the argument isn't *(NULL). Is that worth doing here?

     REF_COPY(fTypeface);
     REF_COPY(fPathEffect);
@@ -126,7 +139,8 @@ SkPaint::SkPaint(const SkPaint& src) {
     COPY(fWidth);
     COPY(fMiterLimit);
     COPY(fBitfields);
-    COPY(fDirtyBits);
+
+    fDirtyBits = src.fDirtyBits;
 
 #ifdef SK_BUILD_FOR_ANDROID
     new (&fPaintOptionsAndroid) SkPaintOptionsAndroid(src.fPaintOptionsAndroid);
@@ -138,21 +152,21 @@ SkPaint::SkPaint(const SkPaint& src) {
 }
 
 SkPaint::~SkPaint() {
-    SkSafeUnref(fTypeface);
-    SkSafeUnref(fPathEffect);
-    SkSafeUnref(fShader);
-    SkSafeUnref(fXfermode);
-    SkSafeUnref(fMaskFilter);
-    SkSafeUnref(fColorFilter);
-    SkSafeUnref(fRasterizer);
-    SkSafeUnref(fLooper);
-    SkSafeUnref(fImageFilter);
-    SkSafeUnref(fAnnotation);
+    SkSafeUnref(fParts.fTypeface);
+    SkSafeUnref(fParts.fPathEffect);
+    SkSafeUnref(fParts.fShader);
+    SkSafeUnref(fParts.fXfermode);
+    SkSafeUnref(fParts.fMaskFilter);
+    SkSafeUnref(fParts.fColorFilter);
+    SkSafeUnref(fParts.fRasterizer);
+    SkSafeUnref(fParts.fLooper);
+    SkSafeUnref(fParts.fImageFilter);
+    SkSafeUnref(fParts.fAnnotation);
 }
 
 SkPaint& SkPaint::operator=(const SkPaint& src) {
-#define COPY(field) field = src.field
-#define REF_COPY(field) SkSafeUnref(field); field = SkSafeRef(src.field)
+#define COPY(field) fParts.field = src.fParts.field
+#define REF_COPY(field) SkSafeUnref(fParts.field); fParts.field = SkSafeRef(src.fParts.field)
 
     SkASSERT(&src);
 
@@ -174,7 +188,8 @@ SkPaint& SkPaint::operator=(const SkPaint& src) {
     COPY(fWidth);
     COPY(fMiterLimit);
     COPY(fBitfields);
-    COPY(fDirtyBits);
+    
+    fDirtyBits = src.fDirtyBits;
 
 #ifdef SK_BUILD_FOR_ANDROID
     fPaintOptionsAndroid.~SkPaintOptionsAndroid();
@@ -189,30 +204,15 @@ SkPaint& SkPaint::operator=(const SkPaint& src) {
 }
 
 bool operator==(const SkPaint& a, const SkPaint& b) {
-#define EQUAL(field) (a.field == b.field)
-    // Don't check fGenerationID or fDirtyBits, which can be different for logically equal paints.
-    return EQUAL(fTypeface)
-        && EQUAL(fPathEffect)
-        && EQUAL(fShader)
-        && EQUAL(fXfermode)
-        && EQUAL(fMaskFilter)
-        && EQUAL(fColorFilter)
-        && EQUAL(fRasterizer)
-        && EQUAL(fLooper)
-        && EQUAL(fImageFilter)
-        && EQUAL(fAnnotation)
-        && EQUAL(fTextSize)
-        && EQUAL(fTextScaleX)
-        && EQUAL(fTextSkewX)
-        && EQUAL(fColor)
-        && EQUAL(fWidth)
-        && EQUAL(fMiterLimit)
-        && EQUAL(fBitfields)
+    if (a.fParts != b.fParts) {
+        return false;
+    }
 #ifdef SK_BUILD_FOR_ANDROID
-        && EQUAL(fPaintOptionsAndroid)
+    if (fPaintOptionsAndroid != b.fPaintOptionsAndroid) {

[mtklein, 2014/04/15 14:32:44]

It makes me very happy that these are not moving into SkPaintParts.

+        return false;
+    }
 #endif
-        ;
-#undef EQUAL
+    return true;
 }
 
 void SkPaint::reset() {