don't trust stored segment mask

Bug: skia:7604
Change-Id: I508bbdc006e1c6edce2006be0c43b037038c876b
Reviewed-on: https://skia-review.googlesource.com/108360
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>

1 files changed, 25 insertions, 40 deletions

diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index c185abaeb7..c710f71ba9 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -277,7 +277,6 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer) {
     }
 
     ref->fIsFinite = (packed >> kIsFinite_SerializationShift) & 1;
-    uint8_t segmentMask = (packed >> kSegmentMask_SerializationShift) & 0xF;
 
     int32_t verbCount, pointCount, conicCount;
     if (!buffer->readU32(&(ref->fGenerationID)) ||
@@ -338,12 +337,13 @@ SkPathRef* SkPathRef::CreateFromBuffer(SkRBuffer* buffer) {
         if (ComputePtBounds(&bounds, *ref) != SkToBool(ref->fIsFinite) || bounds != ref->fBounds) {
             return nullptr;
         }
+
+        // call this after validate_verb_sequence, since it relies on valid verbs
+        ref->fSegmentMask = ref->computeSegmentMask();
     }
 
     ref->fBoundsIsDirty = false;
 
-    // resetToSize clears fSegmentMask and fIsOval
-    ref->fSegmentMask = segmentMask;
     return ref.release();
 }
 
@@ -424,6 +424,8 @@ void SkPathRef::writeToBuffer(SkWBuffer* buffer) const {
     // and fIsFinite are computed.
     const SkRect& bounds = this->getBounds();
 
+    // We store fSegmentMask for older readers, but current readers can't trust it, so they
+    // don't read it.
     int32_t packed = ((fIsFinite & 1) << kIsFinite_SerializationShift) |
                      (fSegmentMask << kSegmentMask_SerializationShift);
     buffer->write32(packed);
@@ -472,6 +474,20 @@ void SkPathRef::copy(const SkPathRef& ref,
     SkDEBUGCODE(this->validate();)
 }
 
+unsigned SkPathRef::computeSegmentMask() const {
+    const uint8_t* verbs = this->verbsMemBegin();
+    unsigned mask = 0;
+    for (int i = this->countVerbs() - 1; i >= 0; --i) {
+        switch (verbs[i]) {
+            case SkPath::kLine_Verb:  mask |= SkPath::kLine_SegmentMask; break;
+            case SkPath::kQuad_Verb:  mask |= SkPath::kQuad_SegmentMask; break;
+            case SkPath::kConic_Verb: mask |= SkPath::kConic_SegmentMask; break;
+            case SkPath::kCubic_Verb: mask |= SkPath::kCubic_SegmentMask; break;
+            default: break;
+        }
+    }
+    return mask;
+}
 
 void SkPathRef::interpolate(const SkPathRef& ending, SkScalar weight, SkPathRef* out) const {
     const SkScalar* inValues = &ending.getPoints()->fX;
@@ -569,25 +585,26 @@ SkPoint* SkPathRef::growForVerb(int /* SkPath::Verb*/ verb, SkScalar weight) {
     SkDEBUGCODE(this->validate();)
     int pCnt;
     bool dirtyAfterEdit = true;
+    unsigned mask = 0;
     switch (verb) {
         case SkPath::kMove_Verb:
             pCnt = 1;
             dirtyAfterEdit = false;
             break;
         case SkPath::kLine_Verb:
-            fSegmentMask |= SkPath::kLine_SegmentMask;
+            mask = SkPath::kLine_SegmentMask;
             pCnt = 1;
             break;
         case SkPath::kQuad_Verb:
-            fSegmentMask |= SkPath::kQuad_SegmentMask;
+            mask = SkPath::kQuad_SegmentMask;
             pCnt = 2;
             break;
         case SkPath::kConic_Verb:
-            fSegmentMask |= SkPath::kConic_SegmentMask;
+            mask = SkPath::kConic_SegmentMask;
             pCnt = 2;
             break;
         case SkPath::kCubic_Verb:
-            fSegmentMask |= SkPath::kCubic_SegmentMask;
+            mask = SkPath::kCubic_SegmentMask;
             pCnt = 3;
             break;
         case SkPath::kClose_Verb:
@@ -614,6 +631,7 @@ SkPoint* SkPathRef::growForVerb(int /* SkPath::Verb*/ verb, SkScalar weight) {
     SkPoint* ret = fPoints + fPointCnt;
     fVerbCnt = newVerbCnt;
     fPointCnt = newPointCnt;
+    fSegmentMask |= mask;
     fFreeSpace -= space;
     fBoundsIsDirty = true;  // this also invalidates fIsFinite
     if (dirtyAfterEdit) {
@@ -859,38 +877,5 @@ bool SkPathRef::isValid() const {
             return false;
         }
     }
-
-#ifdef SK_DEBUG_PATH
-    uint32_t mask = 0;
-    for (int i = 0; i < fVerbCnt; ++i) {
-        switch (fVerbs[~i]) {
-            case SkPath::kMove_Verb:
-                break;
-            case SkPath::kLine_Verb:
-                mask |= SkPath::kLine_SegmentMask;
-                break;
-            case SkPath::kQuad_Verb:
-                mask |= SkPath::kQuad_SegmentMask;
-                break;
-            case SkPath::kConic_Verb:
-                mask |= SkPath::kConic_SegmentMask;
-                break;
-            case SkPath::kCubic_Verb:
-                mask |= SkPath::kCubic_SegmentMask;
-                break;
-            case SkPath::kClose_Verb:
-                break;
-            case SkPath::kDone_Verb:
-                SkDEBUGFAIL("Done verb shouldn't be recorded.");
-                break;
-            default:
-                SkDEBUGFAIL("Unknown Verb");
-                break;
-        }
-    }
-    if (mask != fSegmentMask) {
-        return false;
-    }
-#endif // SK_DEBUG_PATH
     return true;
 }