make SkPath::fConvexity atomic

The TSAN bots fail regularly with races on fConvexity.  Very annoying.

We used to have this very same problem with SkPath::fFirstDirection
until we made it atomic.  This does the same to fConvexity.

This makes the field as lightly atomic as possible, with all operations
using a relaxed memory order.  The value of fConvexity isn't guarding
any other non-atomic memory or implying any other writes have happened
so I don't think we need anything beyond relaxed here.

Change-Id: I0da1f892dc2b7072d692ce8b460fb1862aebef77
Reviewed-on: https://skia-review.googlesource.com/52180
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>

1 files changed, 10 insertions, 11 deletions

diff --git a/include/core/SkPath.h b/include/core/SkPath.h
index 0eb626bd42..2a84c753bd 100644
--- a/include/core/SkPath.h
+++ b/include/core/SkPath.h
@@ -221,7 +221,7 @@ enum Direction {
         Initially SkPath Convexity is kUnknown_Convexity. SkPath Convexity is computed
         if needed by destination SkSurface.
     */
-    enum Convexity {
+    enum Convexity : uint8_t {
         kUnknown_Convexity, //!< Indicates Convexity has not been determined.
 
         /** SkPath has one contour made of a simple geometry without indentations. */
@@ -236,11 +236,10 @@ enum Direction {
         @return  computed or stored SkPath::Convexity
     */
     Convexity getConvexity() const {
-        if (kUnknown_Convexity != fConvexity) {
-            return static_cast<Convexity>(fConvexity);
-        } else {
-            return this->internalGetConvexity();
+        for (Convexity convexity = fConvexity.load(); kUnknown_Convexity != convexity; ) {
+            return convexity;
         }
+        return this->internalGetConvexity();
     }
 
     /** Returns last computed SkPath::Convexity, or kUnknown_Convexity if
@@ -1703,12 +1702,12 @@ private:
         kRRect = 1
     };
 
-    sk_sp<SkPathRef>                                   fPathRef;
-    int                                                fLastMoveToIndex;
-    uint8_t                                            fFillType;
-    mutable uint8_t                                    fConvexity;
-    mutable SkAtomic<uint8_t, sk_memory_order_relaxed> fFirstDirection;// SkPathPriv::FirstDirection
-    SkBool8                                            fIsVolatile;
+    sk_sp<SkPathRef>                                     fPathRef;
+    int                                                  fLastMoveToIndex;
+    uint8_t                                              fFillType;
+    mutable SkAtomic<Convexity, sk_memory_order_relaxed> fConvexity;
+    mutable SkAtomic<uint8_t, sk_memory_order_relaxed>   fFirstDirection;// SkPathPriv::FirstDirection
+    SkBool8                                              fIsVolatile;
 
     /** Resets all fields other than fPathRef to their initial 'empty' values.
      *  Assumes the caller has already emptied fPathRef.