From da6d0720300a29a4deb5dd4c433a92a3ec41286e Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Wed, 3 Jan 2018 13:54:35 -0500 Subject: [PATCH] Make GrShape lazily initialize an original path for gen id change listeners Change-Id: I3a1cb400190cf18241436b7e655a4a267bb2e22d Reviewed-on: https://skia-review.googlesource.com/90482 Commit-Queue: Brian Salomon Reviewed-by: Brian Osman --- src/gpu/GrShape.cpp | 40 +++++++++++++++++++++++++++++++++------- src/gpu/GrShape.h | 14 +++++++++----- tests/GrShapeTest.cpp | 20 ++++++++++++++------ 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/gpu/GrShape.cpp b/src/gpu/GrShape.cpp index b9bc00d1aa..8c0a0aec8e 100644 --- a/src/gpu/GrShape.cpp +++ b/src/gpu/GrShape.cpp @@ -9,7 +9,6 @@ GrShape& GrShape::operator=(const GrShape& that) { fStyle = that.fStyle; - fOriginalPath = that.fOriginalPath; this->changeType(that.fType, Type::kPath == that.fType ? &that.path() : nullptr); switch (fType) { case Type::kEmpty: @@ -29,6 +28,11 @@ GrShape& GrShape::operator=(const GrShape& that) { fInheritedKey.reset(that.fInheritedKey.count()); sk_careful_memcpy(fInheritedKey.get(), that.fInheritedKey.get(), sizeof(uint32_t) * fInheritedKey.count()); + if (that.fInheritedPathForListeners.isValid()) { + fInheritedPathForListeners.set(*that.fInheritedPathForListeners.get()); + } else { + fInheritedPathForListeners.reset(); + } return *this; } @@ -67,7 +71,9 @@ GrShape GrShape::MakeFilled(const GrShape& original, FillInversion inversion) { return original; } GrShape result; - result.fOriginalPath = original.fOriginalPath; + if (original.fInheritedPathForListeners.isValid()) { + result.fInheritedPathForListeners.set(*original.fInheritedPathForListeners.get()); + } switch (original.fType) { case Type::kRRect: result.fType = original.fType; @@ -326,11 +332,24 @@ void GrShape::setInheritedKey(const GrShape &parent, GrStyle::Apply apply, SkSca } } -void GrShape::addGenIDChangeListener(SkPathRef::GenIDChangeListener* listener) const { - SkPathPriv::AddGenIDChangeListener(fOriginalPath, listener); +const SkPath* GrShape::originalPathForListeners() const { + if (fInheritedPathForListeners.isValid()) { + return fInheritedPathForListeners.get(); + } else if (Type::kPath == fType && !fPathData.fPath.isVolatile()) { + return &fPathData.fPath; + } + return nullptr; } -GrShape::GrShape(const GrShape& that) : fStyle(that.fStyle), fOriginalPath(that.fOriginalPath) { +void GrShape::addGenIDChangeListener(SkPathRef::GenIDChangeListener* listener) const { + if (const auto* lp = this->originalPathForListeners()) { + SkPathPriv::AddGenIDChangeListener(*lp, listener); + } else { + delete listener; + } +} + +GrShape::GrShape(const GrShape& that) : fStyle(that.fStyle) { const SkPath* thatPath = Type::kPath == that.fType ? &that.fPathData.fPath : nullptr; this->initType(that.fType, thatPath); switch (fType) { @@ -351,6 +370,9 @@ GrShape::GrShape(const GrShape& that) : fStyle(that.fStyle), fOriginalPath(that. fInheritedKey.reset(that.fInheritedKey.count()); sk_careful_memcpy(fInheritedKey.get(), that.fInheritedKey.get(), sizeof(uint32_t) * fInheritedKey.count()); + if (that.fInheritedPathForListeners.isValid()) { + fInheritedPathForListeners.set(*that.fInheritedPathForListeners.get()); + } } GrShape::GrShape(const GrShape& parent, GrStyle::Apply apply, SkScalar scale) { @@ -436,7 +458,11 @@ GrShape::GrShape(const GrShape& parent, GrStyle::Apply apply, SkScalar scale) { scale)); fStyle.resetToInitStyle(fillOrHairline); } - fOriginalPath = parent.fOriginalPath; + if (parent.fInheritedPathForListeners.isValid()) { + fInheritedPathForListeners.set(*parent.fInheritedPathForListeners.get()); + } else if (Type::kPath == parent.fType && !parent.fPathData.fPath.isVolatile()) { + fInheritedPathForListeners.set(parent.fPathData.fPath); + } this->attemptToSimplifyPath(); this->setInheritedKey(*parentForKey, apply, scale); } @@ -500,7 +526,7 @@ void GrShape::attemptToSimplifyPath() { // Whenever we simplify to a non-path, break the chain so we no longer refer to the // original path. This prevents attaching genID listeners to temporary paths created when // drawing simple shapes. - fOriginalPath.reset(); + fInheritedPathForListeners.reset(); if (Type::kRRect == fType) { this->attemptToSimplifyRRect(); } else if (Type::kLine == fType) { diff --git a/src/gpu/GrShape.h b/src/gpu/GrShape.h index 07cccb8a83..99579ba4c3 100644 --- a/src/gpu/GrShape.h +++ b/src/gpu/GrShape.h @@ -46,7 +46,7 @@ public: explicit GrShape(const SkRect& rect) : GrShape(rect, GrStyle::SimpleFill()) {} - GrShape(const SkPath& path, const GrStyle& style) : fStyle(style), fOriginalPath(path) { + GrShape(const SkPath& path, const GrStyle& style) : fStyle(style) { this->initType(Type::kPath, &path); this->attemptToSimplifyPath(); } @@ -91,7 +91,7 @@ public: this->attemptToSimplifyRRect(); } - GrShape(const SkPath& path, const SkPaint& paint) : fStyle(paint), fOriginalPath(path) { + GrShape(const SkPath& path, const SkPaint& paint) : fStyle(paint) { this->initType(Type::kPath, &path); this->attemptToSimplifyPath(); } @@ -375,13 +375,14 @@ public: /** * Helpers that are only exposed for unit tests, to determine if the shape is a path, and get - * the generation ID of the *original* path. + * the generation ID of the *original* path. This is the path that will receive + * GenIDChangeListeners added to this shape. */ uint32_t testingOnly_getOriginalGenerationID() const; bool testingOnly_isPath() const; + bool testingOnly_isNonVolatilePath() const; private: - enum class Type { kEmpty, kInvertedEmpty, @@ -440,6 +441,9 @@ private: bool attemptToSimplifyStrokedLineToRRect(); + /** Gets the path that gen id listeners should be added to. */ + const SkPath* originalPathForListeners() const; + // Defaults to use when there is no distinction between even/odd and winding fills. static constexpr SkPath::FillType kDefaultPathFillType = SkPath::kEvenOdd_FillType; static constexpr SkPath::FillType kDefaultPathInverseFillType = @@ -509,7 +513,7 @@ private: } fLineData; }; GrStyle fStyle; - SkPath fOriginalPath; + SkTLazy fInheritedPathForListeners; SkAutoSTArray<8, uint32_t> fInheritedKey; }; #endif diff --git a/tests/GrShapeTest.cpp b/tests/GrShapeTest.cpp index 2b80a84d63..261a7d2500 100644 --- a/tests/GrShapeTest.cpp +++ b/tests/GrShapeTest.cpp @@ -18,13 +18,20 @@ #include "SkClipOpPriv.h" uint32_t GrShape::testingOnly_getOriginalGenerationID() const { - return fOriginalPath.getGenerationID(); + if (const auto* lp = this->originalPathForListeners()) { + return lp->getGenerationID(); + } + return SkPath().getGenerationID(); } bool GrShape::testingOnly_isPath() const { return Type::kPath == fType; } +bool GrShape::testingOnly_isNonVolatilePath() const { + return Type::kPath == fType && !fPathData.fPath.isVolatile(); +} + using Key = SkTArray; static bool make_key(Key* key, const GrShape& shape) { @@ -219,7 +226,7 @@ static void check_equivalence(skiatest::Reporter* r, const GrShape& a, const GrS static void check_original_path_ids(skiatest::Reporter* r, const GrShape& base, const GrShape& pe, const GrShape& peStroke, const GrShape& full) { - bool baseIsPath = base.testingOnly_isPath(); + bool baseIsNonVolatilePath = base.testingOnly_isNonVolatilePath(); bool peIsPath = pe.testingOnly_isPath(); bool peStrokeIsPath = peStroke.testingOnly_isPath(); bool fullIsPath = full.testingOnly_isPath(); @@ -235,8 +242,9 @@ static void check_original_path_ids(skiatest::Reporter* r, const GrShape& base, uint32_t emptyID = SkPath().getGenerationID(); // If we started with a real path, then our genID should match that path's gen ID (and not be - // empty). If we started with a simple shape, our original path should have been reset. - REPORTER_ASSERT(r, baseIsPath == (baseID != emptyID)); + // empty). If we started with a simple shape or a volatile path, our original path should have + // been reset. + REPORTER_ASSERT(r, baseIsNonVolatilePath == (baseID != emptyID)); // For the derived shapes, if they're simple types, their original paths should have been reset REPORTER_ASSERT(r, peIsPath || (peID == emptyID)); @@ -250,7 +258,7 @@ static void check_original_path_ids(skiatest::Reporter* r, const GrShape& base, // From here on, we know that the path effect produced a shape that was a "real" path - if (baseIsPath) { + if (baseIsNonVolatilePath) { REPORTER_ASSERT(r, baseID == peID); } @@ -259,7 +267,7 @@ static void check_original_path_ids(skiatest::Reporter* r, const GrShape& base, REPORTER_ASSERT(r, peStrokeID == fullID); } - if (baseIsPath && peStrokeIsPath) { + if (baseIsNonVolatilePath && peStrokeIsPath) { REPORTER_ASSERT(r, baseID == peStrokeID); REPORTER_ASSERT(r, baseID == fullID); }