Obtain conic weights for GrPath cache keys from SkPathRef

src/gpu/GrPath.cpp

 /*
  * Copyright 2012 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "GrPath.h"
 
 namespace {
@@ skipping 40 matching lines @@
     memcpy(&builder[1], &rect, sizeof(rect));
     if (strokeDataCnt > 0) {
         stroke.asUniqueKeyFragment(&builder[kBaseData32Cnt]);
     }
     return true;
 }
 
 // Encodes the full path data to the unique key for very small, volatile paths. This is typically
 // hit when clipping stencils the clip stack. Intention is that this handles rects too, since
 // SkPath::isRect seems to do non-trivial amount of work.
-inline static bool compute_key_for_simple_path(const SkPath& path, const GrStrokeInfo& stroke,
-                                               GrUniqueKey* key) {
+inline static bool compute_key_for_simple_path(const SkPath& path, const SkPathRef* pathRef,
+                                               const GrStrokeInfo& stroke, GrUniqueKey* key) {
     if (!path.isVolatile()) {
         return false;
     }
     // The check below should take care of negative values casted positive.
     const int verbCnt = path.countVerbs();
     if (verbCnt > kSimpleVolatilePathVerbLimit) {
         return false;
     }
 
     // If somebody goes wild with the constant, it might cause an overflow.
     static_assert(kSimpleVolatilePathVerbLimit <= 100,
                   "big_simple_volatile_path_verb_limit_may_cause_overflow");
 
     const int pointCnt = path.countPoints();
     if (pointCnt < 0) {

[reed1, 2015/11/30 15:36:39]

Same question here: why this check? (tho I know its not part of your change).

         SkASSERT(false);
         return false;
     }
-    SkSTArray<16, SkScalar, true> conicWeights(16);
-    if ((path.getSegmentMasks() & SkPath::kConic_SegmentMask) != 0) {
-        SkPath::RawIter iter(path);
-        SkPath::Verb verb;
-        SkPoint points[4];
-        while ((verb = iter.next(points)) != SkPath::kDone_Verb) {
-            if (verb == SkPath::kConic_Verb) {
-                conicWeights.push_back(iter.conicWeight());
-            }
-        }
+
+    const int conicWeightCnt = pathRef ? pathRef->countWeights() : 0;
+    if (conicWeightCnt < 0) {

[reed1, 2015/11/30 15:36:39]

Why this check?

[Kimmo Kinnunen, 2015/12/01 06:35:14]

Because it's an int and used as a thing below that would overflow a buffer if
the int would be negative?

[reed1, 2015/12/01 14:37:21]

Yes, but can it ever be negative? This is an internal value, and typically we
don't spend runtime checking internal structures for self-consistency. Is there
a scenario where this can happen?

+        SkASSERT(false);
+        return false;
     }
 
-    const int conicWeightCnt = conicWeights.count();
-
     // Construct counts that align as uint32_t counts.
 #define ARRAY_DATA32_COUNT(array_type, count) \
     static_cast<int>((((count) * sizeof(array_type) + sizeof(uint32_t) - 1) / sizeof(uint32_t)))
 
     const int verbData32Cnt = ARRAY_DATA32_COUNT(uint8_t, verbCnt);
     const int pointData32Cnt = ARRAY_DATA32_COUNT(SkPoint, pointCnt);
     const int conicWeightData32Cnt = ARRAY_DATA32_COUNT(SkScalar, conicWeightCnt);
 
 #undef ARRAY_DATA32_COUNT
 
@@ skipping 37 matching lines @@
 
     // Here we assume getPoints does a memcpy, so that we do not need to worry about the alignment.
     path.getPoints(reinterpret_cast<SkPoint*>(&builder[i]), pointCnt);
     i += pointData32Cnt;
 
     if (conicWeightCnt > 0) {
         if (conicWeightData32Cnt != static_cast<int>(
                 (conicWeightCnt * sizeof(SkScalar) / sizeof(uint32_t)))) {
             builder[i + conicWeightData32Cnt - 1] = 0;
         }
-        memcpy(&builder[i], conicWeights.begin(), conicWeightCnt * sizeof(SkScalar));
+        memcpy(&builder[i], pathRef->conicWeights(), conicWeightCnt * sizeof(SkScalar));
         SkDEBUGCODE(i += conicWeightData32Cnt);
     }
     SkASSERT(i == baseData32Cnt);
     if (strokeDataCnt > 0) {
         stroke.asUniqueKeyFragment(&builder[baseData32Cnt]);
     }
     return true;
 }
 
 inline static void compute_key_for_general_path(const SkPath& path, const GrStrokeInfo& stroke,
@@ skipping 16 matching lines @@
     if (compute_key_for_line_path(path, stroke, key)) {
         *outIsVolatile = false;
         return;
     }
 
     if (compute_key_for_oval_path(path, stroke, key)) {
         *outIsVolatile = false;
         return;
     }
 
-    if (compute_key_for_simple_path(path, stroke, key)) {
+    if (compute_key_for_simple_path(path, path.fPathRef, stroke, key)) {
         *outIsVolatile = false;
         return;
     }
 
     compute_key_for_general_path(path, stroke, key);
     *outIsVolatile = path.isVolatile();
 }
 
 #ifdef SK_DEBUG
 bool GrPath::isEqualTo(const SkPath& path, const GrStrokeInfo& stroke) const {
     if (!fStroke.hasEqualEffect(stroke)) {
         return false;
     }
 
     // We treat same-rect ovals as identical - but only when not dashing.
     SkRect ovalBounds;
     if (!fStroke.isDashed() && fSkPath.isOval(&ovalBounds)) {
         SkRect otherOvalBounds;
         return path.isOval(&otherOvalBounds) && ovalBounds == otherOvalBounds;
     }
 
     return fSkPath == path;
 }
 #endif