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>
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index c3a7c6b..a460990 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -175,10 +175,11 @@
//fPathRef is assumed to have been set by the caller.
fLastMoveToIndex = that.fLastMoveToIndex;
fFillType = that.fFillType;
- fConvexity = that.fConvexity;
- // Simulate fFirstDirection = that.fFirstDirection;
- fFirstDirection.store(that.fFirstDirection.load());
fIsVolatile = that.fIsVolatile;
+
+ // Non-atomic assignment of atomic values.
+ fConvexity .store(that.fConvexity .load());
+ fFirstDirection.store(that.fFirstDirection.load());
}
bool operator==(const SkPath& a, const SkPath& b) {
@@ -193,12 +194,16 @@
fPathRef.swap(that.fPathRef);
SkTSwap<int>(fLastMoveToIndex, that.fLastMoveToIndex);
SkTSwap<uint8_t>(fFillType, that.fFillType);
- SkTSwap<uint8_t>(fConvexity, that.fConvexity);
- // Simulate SkTSwap<uint8_t>(fFirstDirection, that.fFirstDirection);
- uint8_t temp = fFirstDirection;
- fFirstDirection.store(that.fFirstDirection.load());
- that.fFirstDirection.store(temp);
SkTSwap<SkBool8>(fIsVolatile, that.fIsVolatile);
+
+ // Non-atomic swaps of atomic values.
+ Convexity c = fConvexity.load();
+ fConvexity.store(that.fConvexity.load());
+ that.fConvexity.store(c);
+
+ uint8_t fd = fFirstDirection.load();
+ fFirstDirection.store(that.fFirstDirection.load());
+ that.fFirstDirection.store(fd);
}
}
@@ -1742,7 +1747,7 @@
if (this != dst) {
dst->fFillType = fFillType;
- dst->fConvexity = fConvexity;
+ dst->fConvexity.store(fConvexity);
dst->fIsVolatile = fIsVolatile;
}
@@ -2158,7 +2163,7 @@
return 0;
}
- fConvexity = (packed >> kConvexity_SerializationShift) & 0xFF;
+ fConvexity.store( (Convexity)((packed >> kConvexity_SerializationShift) & 0xFF) );
fFillType = fillType;
fIsVolatile = (packed >> kIsVolatile_SerializationShift) & 0x1;
SkPathRef* pathRef = SkPathRef::CreateFromBuffer(&buffer);
@@ -3550,7 +3555,7 @@
if (this->getSegmentMasks() == SkPath::kLine_SegmentMask) {
return this->getBounds();
}
-
+
SkPoint extremas[5]; // big enough to hold worst-case curve type (cubic) extremas + 1
SkPoint pts[4];
SkPath::RawIter iter(*this);