From 97fd2d42b97104fa32a58d7e7a5b7255913f9c9d Mon Sep 17 00:00:00 2001 From: bsalomon Date: Mon, 9 May 2016 13:02:01 -0700 Subject: [PATCH] Incorporate scale into GrStyle and GrShape GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1952383003 Review-Url: https://codereview.chromium.org/1952383003 --- src/gpu/GrShape.cpp | 32 +++++++++++----- src/gpu/GrShape.h | 15 ++++++-- src/gpu/GrStyle.cpp | 26 +++++++++---- src/gpu/GrStyle.h | 24 +++++++----- tests/GrShapeTest.cpp | 86 ++++++++++++++++++++++++++++++++++++++----- 5 files changed, 143 insertions(+), 40 deletions(-) diff --git a/src/gpu/GrShape.cpp b/src/gpu/GrShape.cpp index 84e14e8925..8462d4d41d 100644 --- a/src/gpu/GrShape.cpp +++ b/src/gpu/GrShape.cpp @@ -83,7 +83,7 @@ void GrShape::writeUnstyledKey(uint32_t* key) const { SkASSERT(key - origKey == this->unstyledKeySize()); } -void GrShape::setInheritedKey(const GrShape &parent, GrStyle::Apply apply) { +void GrShape::setInheritedKey(const GrShape &parent, GrStyle::Apply apply, SkScalar scale) { SkASSERT(!fInheritedKey.count()); // If the output shape turns out to be simple, then we will just use its geometric key if (Type::kPath == fType) { @@ -124,7 +124,8 @@ void GrShape::setInheritedKey(const GrShape &parent, GrStyle::Apply apply) { parentCnt * sizeof(uint32_t)); } // Now turn (geo,path_effect) or (geo) into (geo,path_effect,stroke) - GrStyle::WriteKey(fInheritedKey.get() + parentCnt, parent.fStyle, apply, styleKeyFlags); + GrStyle::WriteKey(fInheritedKey.get() + parentCnt, parent.fStyle, apply, scale, + styleKeyFlags); } } @@ -144,7 +145,11 @@ GrShape::GrShape(const GrShape& that) : fType(that.fType), fStyle(that.fStyle) { sizeof(uint32_t) * fInheritedKey.count()); } -GrShape::GrShape(const GrShape& parent, GrStyle::Apply apply) { +GrShape::GrShape(const GrShape& parent, GrStyle::Apply apply, SkScalar scale) { + // TODO: Add some quantization of scale for better cache performance here or leave that up + // to caller? + // TODO: For certain shapes and stroke params we could ignore the scale. (e.g. miter or bevel + // stroke of a rect). if (!parent.style().applies() || (GrStyle::Apply::kPathEffectOnly == apply && !parent.style().pathEffect())) { fType = Type::kEmpty; @@ -169,6 +174,7 @@ GrShape::GrShape(const GrShape& parent, GrStyle::Apply apply) { // Should we consider bounds? Would have to include in key, but it'd be nice to know // if the bounds actually modified anything before including in key. SkStrokeRec strokeRec = parent.fStyle.strokeRec(); + strokeRec.setResScale(scale); if (!pe->filterPath(fPath.get(), *srcForPathEffect, &strokeRec, nullptr)) { // Make an empty unstyled shape if filtering fails. fType = Type::kEmpty; @@ -176,6 +182,9 @@ GrShape::GrShape(const GrShape& parent, GrStyle::Apply apply) { fPath.reset(); return; } + // A path effect has access to change the res scale but we aren't expecting it to and it + // would mess up our key computation. + SkASSERT(scale == strokeRec.getResScale()); if (GrStyle::Apply::kPathEffectAndStrokeRec == apply) { if (strokeRec.needToApply()) { // The intermediate shape may not be a general path. If we we're just applying @@ -207,17 +216,20 @@ GrShape::GrShape(const GrShape& parent, GrStyle::Apply apply) { fStyle = GrStyle(strokeRec, nullptr); } } else { - const SkPath* srcForStrokeRec; + const SkPath* srcForParentStyle; if (parent.fType == Type::kPath) { - srcForStrokeRec = parent.fPath.get(); + srcForParentStyle = parent.fPath.get(); } else { - srcForStrokeRec = tmpPath.init(); + srcForParentStyle = tmpPath.init(); parent.asPath(tmpPath.get()); } - SkASSERT(parent.fStyle.strokeRec().needToApply()); - SkAssertResult(parent.fStyle.strokeRec().applyToPath(fPath.get(), *srcForStrokeRec)); - fStyle.resetToInitStyle(SkStrokeRec::kFill_InitStyle); + SkStrokeRec::InitStyle fillOrHairline; + SkASSERT(parent.fStyle.applies()); + SkASSERT(!parent.fStyle.pathEffect()); + SkAssertResult(parent.fStyle.applyToPath(fPath.get(), &fillOrHairline, *srcForParentStyle, + scale)); + fStyle.resetToInitStyle(fillOrHairline); } this->attemptToReduceFromPath(); - this->setInheritedKey(*parentForKey, apply); + this->setInheritedKey(*parentForKey, apply, scale); } diff --git a/src/gpu/GrShape.h b/src/gpu/GrShape.h index f4bd7e581a..d72b17965c 100644 --- a/src/gpu/GrShape.h +++ b/src/gpu/GrShape.h @@ -106,7 +106,14 @@ public: const GrStyle& style() const { return fStyle; } - GrShape applyStyle(GrStyle::Apply apply) { return GrShape(*this, apply); } + /** + * Returns a shape that has either applied the path effect or path effect and stroking + * information from this shape's style to its geometry. Scale is used when approximating the + * output geometry and typically is computed from the view matrix + */ + GrShape applyStyle(GrStyle::Apply apply, SkScalar scale) { + return GrShape(*this, apply, scale); + } bool asRRect(SkRRect* rrect) const { if (Type::kRRect != fType) { @@ -170,14 +177,14 @@ private: }; - /** Constructor used by Apply* functions */ - GrShape(const GrShape& parentShape, GrStyle::Apply); + /** Constructor used by the applyStyle() function */ + GrShape(const GrShape& parentShape, GrStyle::Apply, SkScalar scale); /** * Determines the key we should inherit from the input shape's geometry and style when * we are applying the style to create a new shape. */ - void setInheritedKey(const GrShape& parentShape, GrStyle::Apply); + void setInheritedKey(const GrShape& parentShape, GrStyle::Apply, SkScalar scale); void attemptToReduceFromPath() { SkASSERT(Type::kPath == fType); diff --git a/src/gpu/GrStyle.cpp b/src/gpu/GrStyle.cpp index 68238dded9..3698c31ff8 100644 --- a/src/gpu/GrStyle.cpp +++ b/src/gpu/GrStyle.cpp @@ -11,8 +11,8 @@ int GrStyle::KeySize(const GrStyle &style, Apply apply, uint32_t flags) { GR_STATIC_ASSERT(sizeof(uint32_t) == sizeof(SkScalar)); int size = 0; if (style.isDashed()) { - // One scalar for dash phase and one for each dash value. - size += 1 + style.dashIntervalCnt(); + // One scalar for scale, one for dash phase, and one for each dash value. + size += 2 + style.dashIntervalCnt(); } else if (style.pathEffect()) { // No key for a generic path effect. return -1; @@ -23,21 +23,29 @@ int GrStyle::KeySize(const GrStyle &style, Apply apply, uint32_t flags) { } if (style.strokeRec().needToApply()) { - // One for style/cap/join, 2 for miter and width. - size += 3; + // One for res scale, one for style/cap/join, one for miter limit, and one for width. + size += 4; } return size; } -void GrStyle::WriteKey(uint32_t *key, const GrStyle &style, Apply apply, uint32_t flags) { +void GrStyle::WriteKey(uint32_t *key, const GrStyle &style, Apply apply, SkScalar scale, + uint32_t flags) { SkASSERT(key); SkASSERT(KeySize(style, apply) >= 0); GR_STATIC_ASSERT(sizeof(uint32_t) == sizeof(SkScalar)); int i = 0; + // The scale can influence both the path effect and stroking. We want to preserve the + // property that the following two are equal: + // 1. WriteKey with apply == kPathEffectAndStrokeRec + // 2. WriteKey with apply == kPathEffectOnly followed by WriteKey of a GrStyle made + // from SkStrokeRec output by the the path effect (and no additional path effect). + // Since the scale can affect both parts of 2 we write it into the key twice. if (style.isDashed()) { GR_STATIC_ASSERT(sizeof(style.dashPhase()) == sizeof(uint32_t)); SkScalar phase = style.dashPhase(); + memcpy(&key[i++], &scale, sizeof(SkScalar)); memcpy(&key[i++], &phase, sizeof(SkScalar)); int32_t count = style.dashIntervalCnt(); @@ -52,6 +60,7 @@ void GrStyle::WriteKey(uint32_t *key, const GrStyle &style, Apply apply, uint32_ } if (Apply::kPathEffectAndStrokeRec == apply && style.strokeRec().needToApply()) { + memcpy(&key[i++], &scale, sizeof(SkScalar)); enum { kStyleBits = 2, kJoinBits = 2, @@ -123,9 +132,10 @@ static inline bool apply_path_effect(SkPath* dst, SkStrokeRec* strokeRec, } bool GrStyle::applyPathEffectToPath(SkPath *dst, SkStrokeRec *remainingStroke, - const SkPath &src) const { + const SkPath &src, SkScalar resScale) const { SkASSERT(dst); SkStrokeRec strokeRec = fStrokeRec; + strokeRec.setResScale(resScale); if (!apply_path_effect(dst, &strokeRec, fPathEffect, src)) { return false; } @@ -133,10 +143,12 @@ bool GrStyle::applyPathEffectToPath(SkPath *dst, SkStrokeRec *remainingStroke, return true; } -bool GrStyle::applyToPath(SkPath* dst, SkStrokeRec::InitStyle* style, const SkPath& src) const { +bool GrStyle::applyToPath(SkPath* dst, SkStrokeRec::InitStyle* style, const SkPath& src, + SkScalar resScale) const { SkASSERT(style); SkASSERT(dst); SkStrokeRec strokeRec = fStrokeRec; + strokeRec.setResScale(resScale); const SkPath* pathForStrokeRec = &src; if (apply_path_effect(dst, &strokeRec, fPathEffect, src)) { pathForStrokeRec = dst; diff --git a/src/gpu/GrStyle.h b/src/gpu/GrStyle.h index 2ca037a463..6166b56fb7 100644 --- a/src/gpu/GrStyle.h +++ b/src/gpu/GrStyle.h @@ -60,7 +60,7 @@ public: * either reflect just the path effect (if one) or the path effect and the strokerec. Note * that a simple fill has a zero sized key. */ - static int KeySize(const GrStyle& , Apply, uint32_t flags = 0); + static int KeySize(const GrStyle&, Apply, uint32_t flags = 0); /** * Writes a unique key for the style into the provided buffer. This function assumes the buffer @@ -69,7 +69,7 @@ public: * for just dash application followed by the key for the remaining SkStrokeRec is the same as * the key for applying dashing and SkStrokeRec all at once. */ - static void WriteKey(uint32_t*, const GrStyle&, Apply, uint32_t flags = 0); + static void WriteKey(uint32_t*, const GrStyle&, Apply, SkScalar scale, uint32_t flags = 0); GrStyle() : GrStyle(SkStrokeRec::kFill_InitStyle) {} @@ -135,18 +135,22 @@ public: /** * Applies just the path effect and returns remaining stroke information. This will fail if - * there is no path effect. dst may or may not have been overwritten on failure. + * there is no path effect. dst may or may not have been overwritten on failure. Scale controls + * geometric approximations made by the path effect. It is typically computed from the view + * matrix. */ bool SK_WARN_UNUSED_RESULT applyPathEffectToPath(SkPath* dst, SkStrokeRec* remainingStoke, - const SkPath& src) const; + const SkPath& src, SkScalar scale) const; - /** If this succeeds then the result path should be filled or hairlined as indicated by the - returned SkStrokeRec::InitStyle value. Will fail if there is no path effect and the - strokerec doesn't change the geometry. When this fails the outputs may or may not have - been overwritten. - */ + /** + * If this succeeds then the result path should be filled or hairlined as indicated by the + * returned SkStrokeRec::InitStyle value. Will fail if there is no path effect and the + * strokerec doesn't change the geometry. When this fails the outputs may or may not have + * been overwritten. Scale controls geometric approximations made by the path effect and + * stroker. It is typically computed from the view matrix. + */ bool SK_WARN_UNUSED_RESULT applyToPath(SkPath* dst, SkStrokeRec::InitStyle* fillOrHairline, - const SkPath& src) const; + const SkPath& src, SkScalar scale) const; /** Given bounds of a path compute the bounds of path with the style applied. */ void adjustBounds(SkRect* dst, const SkRect& src) const { diff --git a/tests/GrShapeTest.cpp b/tests/GrShapeTest.cpp index f0173de2a4..75f27eda35 100644 --- a/tests/GrShapeTest.cpp +++ b/tests/GrShapeTest.cpp @@ -32,8 +32,9 @@ namespace { class TestCase { public: template - TestCase(const GEO& geo, const SkPaint& paint, skiatest::Reporter* r) : fBase(geo, paint) { - this->init(r); + TestCase(const GEO& geo, const SkPaint& paint, skiatest::Reporter* r, + SkScalar scale = SK_Scalar1) : fBase(geo, paint) { + this->init(r, scale); } struct SelfExpectations { @@ -64,10 +65,11 @@ public: const Key& appliedPathEffectThenStrokeKey() const { return fAppliedPEThenStrokeKey; } private: - void init(skiatest::Reporter* r) { - fAppliedPE = fBase.applyStyle(GrStyle::Apply::kPathEffectOnly); - fAppliedPEThenStroke = fAppliedPE.applyStyle(GrStyle::Apply::kPathEffectAndStrokeRec); - fAppliedFull = fBase.applyStyle(GrStyle::Apply::kPathEffectAndStrokeRec); + void init(skiatest::Reporter* r, SkScalar scale) { + fAppliedPE = fBase.applyStyle(GrStyle::Apply::kPathEffectOnly, scale); + fAppliedPEThenStroke = fAppliedPE.applyStyle(GrStyle::Apply::kPathEffectAndStrokeRec, + scale); + fAppliedFull = fBase.applyStyle(GrStyle::Apply::kPathEffectAndStrokeRec, scale); make_key(&fBaseKey, fBase); make_key(&fAppliedPEKey, fAppliedPE); @@ -89,7 +91,8 @@ private: fBase.asPath(&preStyle); SkStrokeRec postPEStrokeRec(SkStrokeRec::kFill_InitStyle); - if (fBase.style().applyPathEffectToPath(&postPathEffect, &postPEStrokeRec, preStyle)) { + if (fBase.style().applyPathEffectToPath(&postPathEffect, &postPEStrokeRec, preStyle, + scale)) { // run postPathEffect through GrShape to get any geometry reductions that would have // occurred to fAppliedPE. GrShape(postPathEffect, GrStyle(postPEStrokeRec, nullptr)).asPath(&postPathEffect); @@ -100,7 +103,7 @@ private: REPORTER_ASSERT(r, postPEStrokeRec.hasEqualEffect(fAppliedPE.style().strokeRec())); } SkStrokeRec::InitStyle fillOrHairline; - if (fBase.style().applyToPath(&postAllStyle, &fillOrHairline, preStyle)) { + if (fBase.style().applyToPath(&postAllStyle, &fillOrHairline, preStyle, scale)) { // run postPathEffect through GrShape to get any reductions that would have occurred // to fAppliedFull. GrShape(postAllStyle, GrStyle(fillOrHairline)).asPath(&postAllStyle); @@ -125,7 +128,6 @@ private: Key fAppliedPEKey; Key fAppliedPEThenStrokeKey; Key fAppliedFullKey; - }; void TestCase::testExpectations(skiatest::Reporter* reporter, SelfExpectations expectations) const { @@ -296,6 +298,70 @@ static void test_basic(skiatest::Reporter* reporter, const GEO& geo) { REPORTER_ASSERT(reporter, hairlineCase.appliedPathEffectShape().style().isSimpleHairline()); } +template +static void test_scale(skiatest::Reporter* reporter, const GEO& geo) { + sk_sp dashPE = make_dash(); + + static const SkScalar kS1 = 1.f; + static const SkScalar kS2 = 2.f; + + SkPaint fill; + TestCase fillCase1(geo, fill, reporter, kS1); + TestCase fillCase2(geo, fill, reporter, kS2); + // Scale doesn't affect fills. + fillCase1.compare(reporter, fillCase2, TestCase::kAllSame_ComparisonExpecation); + + SkPaint hairline; + hairline.setStyle(SkPaint::kStroke_Style); + hairline.setStrokeWidth(0.f); + TestCase hairlineCase1(geo, hairline, reporter, kS1); + TestCase hairlineCase2(geo, hairline, reporter, kS2); + // Scale doesn't affect hairlines. + hairlineCase1.compare(reporter, hairlineCase2, TestCase::kAllSame_ComparisonExpecation); + + SkPaint stroke; + stroke.setStyle(SkPaint::kStroke_Style); + stroke.setStrokeWidth(2.f); + TestCase strokeCase1(geo, stroke, reporter, kS1); + TestCase strokeCase2(geo, stroke, reporter, kS2); + // Scale affects the stroke. + strokeCase1.compare(reporter, strokeCase2, TestCase::kSameUpToStroke_ComparisonExpecation); + + SkPaint strokeDash = stroke; + strokeDash.setPathEffect(make_dash()); + TestCase strokeDashCase1(geo, strokeDash, reporter, kS1); + TestCase strokeDashCase2(geo, strokeDash, reporter, kS2); + // Scale affects the dash and the stroke. + strokeDashCase1.compare(reporter, strokeDashCase2, TestCase::kSameUpToPE_ComparisonExpecation); + + // Stroke and fill cases + SkPaint strokeAndFill = stroke; + strokeAndFill.setStyle(SkPaint::kStrokeAndFill_Style); + TestCase strokeAndFillCase1(geo, strokeAndFill, reporter, kS1); + TestCase strokeAndFillCase2(geo, strokeAndFill, reporter, kS2); + // Scale affects the stroke. Though, this can wind up creating a rect when the input is a rect. + // In that case we wind up with a pure geometry key and the geometries are the same. + SkRRect rrect; + if (strokeAndFillCase1.appliedFullStyleShape().asRRect(&rrect)) { + // We currently only expect to get here in the rect->rect case. + REPORTER_ASSERT(reporter, rrect.isRect()); + REPORTER_ASSERT(reporter, strokeAndFillCase1.baseShape().asRRect(&rrect) && rrect.isRect()); + strokeAndFillCase1.compare(reporter, strokeAndFillCase2, + TestCase::kAllSame_ComparisonExpecation); + } else { + strokeAndFillCase1.compare(reporter, strokeAndFillCase2, + TestCase::kSameUpToStroke_ComparisonExpecation); + } + + SkPaint strokeAndFillDash = strokeDash; + strokeAndFillDash.setStyle(SkPaint::kStrokeAndFill_Style); + TestCase strokeAndFillDashCase1(geo, strokeAndFillDash, reporter, kS1); + TestCase strokeAndFillDashCase2(geo, strokeAndFillDash, reporter, kS2); + // Scale affects the path effect and stroke. + strokeAndFillDashCase1.compare(reporter, strokeAndFillDashCase2, + TestCase::kSameUpToPE_ComparisonExpecation); +} + template static void test_stroke_param_impl(skiatest::Reporter* reporter, const GEO& geo, std::function setter, T a, T b, @@ -748,6 +814,7 @@ DEF_TEST(GrShape, reporter) { for (auto rr : { SkRRect::MakeRect(SkRect::MakeWH(10, 10)), SkRRect::MakeRectXY(SkRect::MakeWH(10, 10), 3, 4)}) { test_basic(reporter, rr); + test_scale(reporter, rr); test_dash_fill(reporter, rr); test_null_dash(reporter, rr); // Test modifying various stroke params. @@ -809,6 +876,7 @@ DEF_TEST(GrShape, reporter) { test_null_dash(reporter, path); test_path_effect_makes_rrect(reporter, path); } + test_scale(reporter, path); // This test uses a stroking paint, hence use of fIsRRectForStroke test_volatile_path(reporter, path, testPath.fIsRRectForStroke); test_dash_fill(reporter, path);