diff --git a/src/core/SkPathPriv.h b/src/core/SkPathPriv.h index 60b0f2a155..029cb759de 100644 --- a/src/core/SkPathPriv.h +++ b/src/core/SkPathPriv.h @@ -98,6 +98,29 @@ public: */ static void CreateDrawArcPath(SkPath* path, const SkRect& oval, SkScalar startAngle, SkScalar sweepAngle, bool useCenter, bool isFillNoPathEffect); + + /** + * Returns a pointer to the verb data. Note that the verbs are stored backwards in memory and + * thus the returned pointer is the last verb. + */ + static const uint8_t* VerbData(const SkPath& path) { + return path.fPathRef->verbsMemBegin(); + } + + /** Returns a raw pointer to the path points */ + static const SkPoint* PointData(const SkPath& path) { + return path.fPathRef->points(); + } + + /** Returns the number of conic weights in the path */ + static int ConicWeightCnt(const SkPath& path) { + return path.fPathRef->countWeights(); + } + + /** Returns a raw pointer to the path conic weights. */ + static const SkScalar* ConicWeightData(const SkPath& path) { + return path.fPathRef->conicWeights(); + } }; #endif diff --git a/src/gpu/GrPath.cpp b/src/gpu/GrPath.cpp index 27fbf21a9d..836cc5ed55 100644 --- a/src/gpu/GrPath.cpp +++ b/src/gpu/GrPath.cpp @@ -8,132 +8,21 @@ #include "GrPath.h" #include "GrShape.h" -// Verb count limit for generating path key from content of a volatile path. -// The value should accomodate at least simple rects and rrects. -static const int kSimpleVolatilePathVerbLimit = 10; +static inline void write_style_key(uint32_t* key, const GrStyle& style) { + // Pass 1 for the scale since the GPU will apply the style not GrStyle::applyToPath(). + GrStyle::WriteKey(key, style, GrStyle::Apply::kPathEffectAndStrokeRec, SK_Scalar1); +} -static inline int style_data_cnt(const GrStyle& style) { - int cnt = GrStyle::KeySize(style, GrStyle::Apply::kPathEffectAndStrokeRec); + +void GrPath::ComputeKey(const GrShape& shape, GrUniqueKey* key, bool* outIsVolatile) { + int geoCnt = shape.unstyledKeySize(); + int styleCnt = GrStyle::KeySize(shape.style(), GrStyle::Apply::kPathEffectAndStrokeRec); // This should only fail for an arbitrary path effect, and we should not have gotten // here with anything other than a dash path effect. - SkASSERT(cnt >= 0); - return cnt; -} - -static inline void write_style_key(uint32_t* dst, const GrStyle& style) { - // Pass 1 for the scale since the GPU will apply the style not GrStyle::applyToPath(). - GrStyle::WriteKey(dst, style, GrStyle::Apply::kPathEffectAndStrokeRec, SK_Scalar1); -} - -// Encodes the full path data to the unique key for very small paths that wouldn't otherwise have a -// key. This is typically hit when clipping stencils the clip stack. -inline static bool compute_key_for_simple_path(const GrShape& shape, GrUniqueKey* key) { - if (shape.hasUnstyledKey()) { - return false; - } - SkPath path; - shape.asPath(&path); - // 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) { - 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 = conicWeights.count(); - - // Construct counts that align as uint32_t counts. -#define ARRAY_DATA32_COUNT(array_type, count) \ - static_cast((((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 - - // The unique key data is a "message" with following fragments: - // 0) domain, key length, uint32_t for fill type and uint32_t for verbCnt - // (fragment 0, fixed size) - // 1) verb, point data and conic weights (varying size) - // 2) stroke data (varying size) - - const int baseData32Cnt = 2 + verbData32Cnt + pointData32Cnt + conicWeightData32Cnt; - const int styleDataCnt = style_data_cnt(shape.style()); - static const GrUniqueKey::Domain kSimpleVolatilePathDomain = GrUniqueKey::GenerateDomain(); - GrUniqueKey::Builder builder(key, kSimpleVolatilePathDomain, baseData32Cnt + styleDataCnt); - int i = 0; - builder[i++] = path.getFillType(); - - // Serialize the verbCnt to make the whole message unambiguous. - // We serialize two variable length fragments to the message: - // * verbs, point data and conic weights (fragment 1) - // * stroke data (fragment 2) - // "Proof:" - // Verb count establishes unambiguous verb data. - // Verbs encode also point data size and conic weight size. - // Thus the fragment 1 is unambiguous. - // Unambiguous fragment 1 establishes unambiguous fragment 2, since the length of the message - // has been established. - - builder[i++] = SkToU32(verbCnt); // The path limit is compile-asserted above, so the cast is ok. - - // Fill the last uint32_t with 0 first, since the last uint8_ts of the uint32_t may be - // uninitialized. This does not produce ambiguous verb data, since we have serialized the exact - // verb count. - if (verbData32Cnt != static_cast((verbCnt * sizeof(uint8_t) / sizeof(uint32_t)))) { - builder[i + verbData32Cnt - 1] = 0; - } - path.getVerbs(reinterpret_cast(&builder[i]), verbCnt); - i += verbData32Cnt; - - static_assert(((sizeof(SkPoint) % sizeof(uint32_t)) == 0) && sizeof(SkPoint) > sizeof(uint32_t), - "skpoint_array_needs_padding"); - - // Here we assume getPoints does a memcpy, so that we do not need to worry about the alignment. - path.getPoints(reinterpret_cast(&builder[i]), pointCnt); - i += pointData32Cnt; - - if (conicWeightCnt > 0) { - if (conicWeightData32Cnt != static_cast( - (conicWeightCnt * sizeof(SkScalar) / sizeof(uint32_t)))) { - builder[i + conicWeightData32Cnt - 1] = 0; - } - memcpy(&builder[i], conicWeights.begin(), conicWeightCnt * sizeof(SkScalar)); - SkDEBUGCODE(i += conicWeightData32Cnt); - } - SkASSERT(i == baseData32Cnt); - if (styleDataCnt > 0) { - write_style_key(&builder[baseData32Cnt], shape.style()); - } - return true; -} - -inline static bool compute_key_for_general_shape(const GrShape& shape, GrUniqueKey* key) { - int geoCnt = shape.unstyledKeySize(); - int styleCnt = style_data_cnt(shape.style()); - if (styleCnt < 0 || geoCnt < 0) { - return false; + SkASSERT(styleCnt >= 0); + if (geoCnt < 0) { + *outIsVolatile = true; + return; } static const GrUniqueKey::Domain kGeneralPathDomain = GrUniqueKey::GenerateDomain(); GrUniqueKey::Builder builder(key, kGeneralPathDomain, geoCnt + styleCnt); @@ -141,16 +30,7 @@ inline static bool compute_key_for_general_shape(const GrShape& shape, GrUniqueK if (styleCnt) { write_style_key(&builder[geoCnt], shape.style()); } - return true; -} - -void GrPath::ComputeKey(const GrShape& shape, GrUniqueKey* key, bool* outIsVolatile) { - - if (compute_key_for_simple_path(shape, key)) { - *outIsVolatile = false; - return; - } - *outIsVolatile = !compute_key_for_general_shape(shape, key); + *outIsVolatile = false; } #ifdef SK_DEBUG @@ -170,13 +50,6 @@ bool GrPath::isEqualTo(const SkPath& path, const GrStyle& style) const { return false; } } - // We treat same-rect ovals as identical - but only when not dashing. - SkRect ovalBounds; - if (!fStyle.isDashed() && fSkPath.isOval(&ovalBounds)) { - SkRect otherOvalBounds; - return path.isOval(&otherOvalBounds) && ovalBounds == otherOvalBounds; - } - return fSkPath == path; } #endif diff --git a/src/gpu/GrShape.cpp b/src/gpu/GrShape.cpp index 582858e1d9..b3c1e0a4cc 100644 --- a/src/gpu/GrShape.cpp +++ b/src/gpu/GrShape.cpp @@ -72,6 +72,49 @@ SkRect GrShape::styledBounds() const { return bounds; } +// If the path is small enough to be keyed from its data this returns key length, otherwise -1. +static int path_key_from_data_size(const SkPath& path) { + const int verbCnt = path.countVerbs(); + if (verbCnt > GrShape::kMaxKeyFromDataVerbCnt) { + return -1; + } + const int pointCnt = path.countPoints(); + const int conicWeightCnt = SkPathPriv::ConicWeightCnt(path); + + GR_STATIC_ASSERT(sizeof(SkPoint) == 2 * sizeof(uint32_t)); + GR_STATIC_ASSERT(sizeof(SkScalar) == sizeof(uint32_t)); + // 2 is for the verb cnt and a fill type. Each verb is a byte but we'll pad the verb data out to + // a uint32_t length. + return 2 + (SkAlign4(verbCnt) >> 2) + 2 * pointCnt + conicWeightCnt; +} + +// Writes the path data key into the passed pointer. +static void write_path_key_from(const SkPath& path, uint32_t* origKey) { + uint32_t* key = origKey; + // The check below should take care of negative values casted positive. + const int verbCnt = path.countVerbs(); + const int pointCnt = path.countPoints(); + const int conicWeightCnt = SkPathPriv::ConicWeightCnt(path); + SkASSERT(verbCnt <= GrShape::kMaxKeyFromDataVerbCnt); + SkASSERT(pointCnt && verbCnt); + *key++ = path.getFillType(); + *key++ = verbCnt; + memcpy(key, SkPathPriv::VerbData(path), verbCnt * sizeof(uint8_t)); + int verbKeySize = SkAlign4(verbCnt); + // pad out to uint32_t alignment using value that will stand out when debugging. + uint8_t* pad = reinterpret_cast(key)+ verbCnt; + memset(pad, 0xDE, verbKeySize - verbCnt); + key += verbKeySize >> 2; + + memcpy(key, SkPathPriv::PointData(path), sizeof(SkPoint) * pointCnt); + GR_STATIC_ASSERT(sizeof(SkPoint) == 2 * sizeof(uint32_t)); + key += 2 * pointCnt; + memcpy(key, SkPathPriv::ConicWeightData(path), sizeof(SkScalar) * conicWeightCnt); + GR_STATIC_ASSERT(sizeof(SkScalar) == sizeof(uint32_t)); + SkDEBUGCODE(key += conicWeightCnt); + SkASSERT(key - origKey == path_key_from_data_size(path)); +} + int GrShape::unstyledKeySize() const { if (fInheritedKey.count()) { return fInheritedKey.count(); @@ -88,13 +131,17 @@ int GrShape::unstyledKeySize() const { GR_STATIC_ASSERT(2 * sizeof(uint32_t) == sizeof(SkPoint)); // 4 for the end points and 1 for the inverseness return 5; - case Type::kPath: + case Type::kPath: { + int dataKeySize = path_key_from_data_size(fPathData.fPath); + if (dataKeySize >= 0) { + return dataKeySize; + } if (0 == fPathData.fGenID) { return -1; - } else { - // The key is the path ID and fill type. - return 2; } + // The key is the path ID and fill type. + return 2; + } } SkFAIL("Should never get here."); return 0; @@ -124,13 +171,19 @@ void GrShape::writeUnstyledKey(uint32_t* key) const { key += 4; *key++ = fLineData.fInverted ? 1 : 0; break; - case Type::kPath: + case Type::kPath: { + int dataKeySize = path_key_from_data_size(fPathData.fPath); + if (dataKeySize >= 0) { + write_path_key_from(fPathData.fPath, key); + return; + } SkASSERT(fPathData.fGenID); *key++ = fPathData.fGenID; // We could canonicalize the fill rule for paths that don't differentiate between // even/odd or winding fill (e.g. convex). *key++ = this->path().getFillType(); break; + } } } SkASSERT(key - origKey == this->unstyledKeySize()); diff --git a/src/gpu/GrShape.h b/src/gpu/GrShape.h index 8f3be58702..074278cd86 100644 --- a/src/gpu/GrShape.h +++ b/src/gpu/GrShape.h @@ -34,6 +34,10 @@ */ class GrShape { public: + // Keys for paths may be extracted from the path data for small paths. Clients aren't supposed + // to have to worry about this. This value is exposed for unit tests. + static constexpr int kMaxKeyFromDataVerbCnt = 10; + GrShape() { this->initType(Type::kEmpty); } explicit GrShape(const SkPath& path) : GrShape(path, GrStyle::SimpleFill()) {} diff --git a/tests/GrShapeTest.cpp b/tests/GrShapeTest.cpp index 36ebd7b21e..4f96a038e3 100644 --- a/tests/GrShapeTest.cpp +++ b/tests/GrShapeTest.cpp @@ -1078,14 +1078,16 @@ void test_unknown_path_effect(skiatest::Reporter* reporter, const Geo& geo) { bool filterPath(SkPath* dst, const SkPath& src, SkStrokeRec*, const SkRect* cullR) const override { *dst = src; - dst->lineTo(0, 0); - dst->lineTo(10, 10); + // To avoid triggering data-based keying of paths with few verbs we add many segments. + for (int i = 0; i < 100; ++i) { + dst->lineTo(SkIntToScalar(i), SkIntToScalar(i)); + } return true; } void computeFastBounds(SkRect* dst, const SkRect& src) const override { *dst = src; dst->growToInclude(0, 0); - dst->growToInclude(10, 10); + dst->growToInclude(100, 100); } static sk_sp Make() { return sk_sp(new AddLineTosPathEffect); } Factory getFactory() const override { return nullptr; } @@ -1156,8 +1158,17 @@ void test_make_hairline_path_effect(skiatest::Reporter* reporter, const Geo& geo a.setFillType(b.getFillType()); REPORTER_ASSERT(reporter, a == b); REPORTER_ASSERT(reporter, a == c); - REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey().empty()); - REPORTER_ASSERT(reporter, peCase.appliedFullStyleKey().empty()); + // If the resulting path is small enough then it will have a key. + REPORTER_ASSERT(reporter, paths_fill_same(a, b)); + REPORTER_ASSERT(reporter, paths_fill_same(a, c)); + if (c.countVerbs() <= GrShape::kMaxKeyFromDataVerbCnt) { + REPORTER_ASSERT(reporter, !peCase.appliedPathEffectKey().empty()); + REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey() == + peCase.appliedFullStyleKey()); + } else { + REPORTER_ASSERT(reporter, peCase.appliedPathEffectKey().empty()); + REPORTER_ASSERT(reporter, peCase.appliedFullStyleKey().empty()); + } } REPORTER_ASSERT(reporter, peCase.appliedPathEffectShape().style().isSimpleHairline()); REPORTER_ASSERT(reporter, peCase.appliedFullStyleShape().style().isSimpleHairline()); @@ -1173,8 +1184,8 @@ void test_volatile_path(skiatest::Reporter* reporter, const Geo& geo) { dashAndStroke.setStyle(SkPaint::kStroke_Style); TestCase volatileCase(reporter, vPath, dashAndStroke); // We expect a shape made from a volatile path to have a key iff the shape is recognized - // as a specialized geometry. - if (geo.isNonPath(dashAndStroke)) { + // as a specialized geometry or it has a small verb count. + if (geo.isNonPath(dashAndStroke) || vPath.countVerbs() <= GrShape::kMaxKeyFromDataVerbCnt) { REPORTER_ASSERT(reporter, SkToBool(volatileCase.baseKey().count())); // In this case all the keys should be identical to the non-volatile case. TestCase nonVolatileCase(reporter, geo.path(), dashAndStroke); @@ -1768,6 +1779,70 @@ static void test_stroked_lines(skiatest::Reporter* r) { TestCase::kAllSame_ComparisonExpecation); } +static void test_short_path_keys(skiatest::Reporter* r) { + SkPaint paints[4]; + paints[1].setStyle(SkPaint::kStroke_Style); + paints[1].setStrokeWidth(5.f); + paints[2].setStyle(SkPaint::kStroke_Style); + paints[2].setStrokeWidth(0.f); + paints[3].setStyle(SkPaint::kStrokeAndFill_Style); + paints[3].setStrokeWidth(5.f); + + auto compare = [r, &paints] (SkPath* pathA, SkPath* pathB, + TestCase::ComparisonExpecation expectation) { + for (const SkPaint& paint : paints) { + for (PathGeo::Invert invert : {PathGeo::Invert::kNo, PathGeo::Invert::kYes}) { + for (bool aIsVolatile : {false, true}) { + for (bool bIsVolatile : {false, true}) { + pathA->setIsVolatile(aIsVolatile); + pathB->setIsVolatile(bIsVolatile); + TestCase caseA(PathGeo(*pathA, invert), paint, r); + TestCase caseB(PathGeo(*pathB, invert), paint, r); + caseA.compare(r, caseB, expectation); + } + } + } + } + }; + + SkPath pathA; + SkPath pathB; + + // Two identical paths + pathA.lineTo(10.f, 10.f); + pathA.conicTo(20.f, 20.f, 20.f, 30.f, 0.7f); + + pathB.lineTo(10.f, 10.f); + pathB.conicTo(20.f, 20.f, 20.f, 30.f, 0.7f); + compare(&pathA, &pathB, TestCase::kAllSame_ComparisonExpecation); + + // Give path b a different point + pathB.reset(); + pathB.lineTo(10.f, 10.f); + pathB.conicTo(21.f, 20.f, 20.f, 30.f, 0.7f); + compare(&pathA, &pathB, TestCase::kAllDifferent_ComparisonExpecation); + + // Give path b a different conic weight + pathB.reset(); + pathB.lineTo(10.f, 10.f); + pathB.conicTo(20.f, 20.f, 20.f, 30.f, 0.6f); + compare(&pathA, &pathB, TestCase::kAllDifferent_ComparisonExpecation); + + // Give path b an extra lineTo verb + pathB.reset(); + pathB.lineTo(10.f, 10.f); + pathB.conicTo(20.f, 20.f, 20.f, 30.f, 0.6f); + pathB.lineTo(50.f, 50.f); + compare(&pathA, &pathB, TestCase::kAllDifferent_ComparisonExpecation); + + // Give path b a close + pathB.reset(); + pathB.lineTo(10.f, 10.f); + pathB.conicTo(20.f, 20.f, 20.f, 30.f, 0.7f); + pathB.close(); + compare(&pathA, &pathB, TestCase::kAllDifferent_ComparisonExpecation); +} + DEF_TEST(GrShape, reporter) { SkTArray> geos; SkTArray> rrectPathGeos; @@ -1894,6 +1969,8 @@ DEF_TEST(GrShape, reporter) { test_lines(reporter); test_stroked_lines(reporter); + + test_short_path_keys(reporter); } #endif