From b3eb687f8a89eb1eacd1afb4016401eb392f66ab Mon Sep 17 00:00:00 2001 From: jvanverth Date: Fri, 24 Oct 2014 07:12:51 -0700 Subject: [PATCH] Set temporary paths volatile so we don't cache them. Any path that is generated frame-to-frame should not be rendered by using the DistanceFieldPathRenderer, because generating the initial distance field, uploading it and rendering it takes longer than the SoftwarePathRenderer. BUG=skia:2935 Review URL: https://codereview.chromium.org/677463002 --- expectations/gm/ignored-tests.txt | 9 +++++++++ include/core/SkPath.h | 20 +++++++++++++++++++- src/core/SkDraw.cpp | 8 ++++++-- src/core/SkPath.cpp | 8 +++++++- src/core/SkStroke.cpp | 2 ++ src/gpu/GrAADistanceFieldPathRenderer.cpp | 20 ++++++++++++++++++-- src/gpu/GrClipMaskManager.cpp | 1 + src/gpu/SkGpuDevice.cpp | 11 +++++++++++ 8 files changed, 73 insertions(+), 6 deletions(-) diff --git a/expectations/gm/ignored-tests.txt b/expectations/gm/ignored-tests.txt index 1fb5937..25d6545 100644 --- a/expectations/gm/ignored-tests.txt +++ b/expectations/gm/ignored-tests.txt @@ -81,3 +81,12 @@ image-surface scaled_tilemode_bitmap tilemode_bitmap verylargebitmap + +# jvanverth https://codereview.chromium.org/677463002/ +strokes_round +strokes3 +nonclosedpaths +polygons +roundrects +shadertext +shadertext2 diff --git a/include/core/SkPath.h b/include/core/SkPath.h index 57da959..f5a6d4a 100644 --- a/include/core/SkPath.h +++ b/include/core/SkPath.h @@ -185,6 +185,23 @@ public: return fPathRef->isFinite(); } + /** Returns true if the path is volatile (i.e. should not be cached by devices.) + */ + bool isVolatile() const { + return SkToBool(fIsVolatile); + } + + /** Specify whether this path is volatile. Paths are not volatile by + default. Temporary paths that are discarded or modified after use should be + marked as volatile. This provides a hint to the device that the path + should not be cached. Providing this hint when appropriate can + improve performance by avoiding unnecessary overhead and resource + consumption on the device. + */ + void setIsVolatile(bool isVolatile) { + fIsVolatile = isVolatile; + } + /** Test a line for zero length @return true if the line is of zero length; otherwise false. @@ -971,7 +988,7 @@ private: // 1 free bit at 29 kUnused1_SerializationShift = 28, // 1 free bit kDirection_SerializationShift = 26, // requires 2 bits - kUnused2_SerializationShift = 25, // 1 free bit + kIsVolatile_SerializationShift = 25, // requires 1 bit // 1 free bit at 24 kConvexity_SerializationShift = 16, // requires 8 bits kFillType_SerializationShift = 8, // requires 8 bits @@ -984,6 +1001,7 @@ private: uint8_t fFillType; mutable uint8_t fConvexity; mutable uint8_t fDirection; + mutable SkBool8 fIsVolatile; #ifdef SK_BUILD_FOR_ANDROID const SkPath* fSourcePath; #endif diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp index 929088d..67fd277 100644 --- a/src/core/SkDraw.cpp +++ b/src/core/SkDraw.cpp @@ -595,12 +595,13 @@ void SkDraw::drawPoints(SkCanvas::PointMode mode, size_t count, if (newPaint.getStrokeCap() == SkPaint::kRound_Cap) { SkPath path; SkMatrix preMatrix; - + path.addCircle(0, 0, radius); for (size_t i = 0; i < count; i++) { preMatrix.setTranslate(pts[i].fX, pts[i].fY); // pass true for the last point, since we can modify // then path then + path.setIsVolatile((count-1) == i); if (fDevice) { fDevice->drawPath(*this, path, newPaint, &preMatrix, (count-1) == i); @@ -719,6 +720,7 @@ void SkDraw::drawPoints(SkCanvas::PointMode mode, size_t count, SkPaint p(paint); p.setStyle(SkPaint::kStroke_Style); size_t inc = (SkCanvas::kLines_PointMode == mode) ? 2 : 1; + path.setIsVolatile(true); for (size_t i = 0; i < count; i += inc) { path.moveTo(pts[i]); path.lineTo(pts[i+1]); @@ -1011,6 +1013,7 @@ void SkDraw::drawPath(const SkPath& origSrcPath, const SkPaint& origPaint, SkPath tmpPath; SkMatrix tmpMatrix; const SkMatrix* matrix = fMatrix; + tmpPath.setIsVolatile(true); if (prePathMatrix) { if (origPaint.getPathEffect() || origPaint.getStyle() != SkPaint::kFill_Style || @@ -1962,7 +1965,8 @@ void SkDraw::drawTextOnPath(const char text[], size_t byteLength, if (iterPath) { SkPath tmp; SkMatrix m(scaledMatrix); - + + tmp.setIsVolatile(true); m.postTranslate(xpos + hOffset, 0); if (matrix) { m.postConcat(*matrix); diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp index f756ce3..5288b85 100644 --- a/src/core/SkPath.cpp +++ b/src/core/SkPath.cpp @@ -132,6 +132,7 @@ SkPath::SkPath() #endif { this->resetFields(); + fIsVolatile = false; } void SkPath::resetFields() { @@ -178,6 +179,7 @@ void SkPath::copyFields(const SkPath& that) { fFillType = that.fFillType; fConvexity = that.fConvexity; fDirection = that.fDirection; + fIsVolatile = that.fIsVolatile; } bool operator==(const SkPath& a, const SkPath& b) { @@ -196,6 +198,7 @@ void SkPath::swap(SkPath& that) { SkTSwap(fFillType, that.fFillType); SkTSwap(fConvexity, that.fConvexity); SkTSwap(fDirection, that.fDirection); + SkTSwap(fIsVolatile, that.fIsVolatile); #ifdef SK_BUILD_FOR_ANDROID SkTSwap(fSourcePath, that.fSourcePath); #endif @@ -1606,6 +1609,7 @@ void SkPath::transform(const SkMatrix& matrix, SkPath* dst) const { if (this != dst) { dst->fFillType = fFillType; dst->fConvexity = fConvexity; + dst->fIsVolatile = fIsVolatile; } if (kUnknown_Direction == fDirection) { @@ -1978,7 +1982,8 @@ size_t SkPath::writeToMemory(void* storage) const { int32_t packed = (fConvexity << kConvexity_SerializationShift) | (fFillType << kFillType_SerializationShift) | - (fDirection << kDirection_SerializationShift); + (fDirection << kDirection_SerializationShift) | + (fIsVolatile << kIsVolatile_SerializationShift); buffer.write32(packed); @@ -1999,6 +2004,7 @@ size_t SkPath::readFromMemory(const void* storage, size_t length) { fConvexity = (packed >> kConvexity_SerializationShift) & 0xFF; fFillType = (packed >> kFillType_SerializationShift) & 0xFF; fDirection = (packed >> kDirection_SerializationShift) & 0x3; + fIsVolatile = (packed >> kIsVolatile_SerializationShift) & 0x1; SkPathRef* pathRef = SkPathRef::CreateFromBuffer(&buffer); size_t sizeRead = 0; diff --git a/src/core/SkStroke.cpp b/src/core/SkStroke.cpp index a9cc6cd..c562c2e 100644 --- a/src/core/SkStroke.cpp +++ b/src/core/SkStroke.cpp @@ -380,7 +380,9 @@ SkPathStroker::SkPathStroker(const SkPath& src, // 3x for result == inner + outer + join (swag) // 1x for inner == 'wag' (worst contour length would be better guess) fOuter.incReserve(src.countPoints() * 3); + fOuter.setIsVolatile(true); fInner.incReserve(src.countPoints()); + fInner.setIsVolatile(true); #if QUAD_STROKE_APPROXIMATION #ifdef SK_DEBUG if (!gDebugStrokerErrorSet) { diff --git a/src/gpu/GrAADistanceFieldPathRenderer.cpp b/src/gpu/GrAADistanceFieldPathRenderer.cpp index 95e4e7a..e2cee65 100755 --- a/src/gpu/GrAADistanceFieldPathRenderer.cpp +++ b/src/gpu/GrAADistanceFieldPathRenderer.cpp @@ -31,6 +31,11 @@ SK_CONF_DECLARE(bool, c_DumpPathCache, "gpu.dumpPathCache", false, "Dump the contents of the path cache before every purge."); +#ifdef DF_PATH_TRACKING +static int g_NumCachedPaths = 0; +static int g_NumFreedPaths = 0; +#endif + //////////////////////////////////////////////////////////////////////////////// GrAADistanceFieldPathRenderer::~GrAADistanceFieldPathRenderer() { PathDataList::Iter iter; @@ -43,6 +48,10 @@ GrAADistanceFieldPathRenderer::~GrAADistanceFieldPathRenderer() { } SkDELETE(fAtlas); + +#ifdef DF_PATH_TRACKING + SkDebugf("Cached paths: %d, freed paths: %d\n", g_NumCachedPaths, g_NumFreedPaths); +#endif } //////////////////////////////////////////////////////////////////////////////// @@ -50,10 +59,11 @@ bool GrAADistanceFieldPathRenderer::canDrawPath(const SkPath& path, const SkStrokeRec& stroke, const GrDrawTarget* target, bool antiAlias) const { + // TODO: Support inverse fill // TODO: Support strokes if (!target->caps()->shaderDerivativeSupport() || !antiAlias || path.isInverseFillType() - || SkStrokeRec::kFill_Style != stroke.getStyle()) { + || path.isVolatile() || SkStrokeRec::kFill_Style != stroke.getStyle()) { return false; } @@ -230,7 +240,10 @@ HAS_ATLAS: fPathCache.add(pathData); fPathList.addToTail(pathData); - +#ifdef DF_PATH_TRACKING + ++g_NumCachedPaths; +#endif + return pathData; } @@ -252,6 +265,9 @@ bool GrAADistanceFieldPathRenderer::freeUnusedPlot() { fPathCache.remove(pathData->fGenID); fPathList.remove(pathData); SkDELETE(pathData); +#ifdef DF_PATH_TRACKING + ++g_NumFreedPaths; +#endif } } diff --git a/src/gpu/GrClipMaskManager.cpp b/src/gpu/GrClipMaskManager.cpp index 414bbab..349d51e 100644 --- a/src/gpu/GrClipMaskManager.cpp +++ b/src/gpu/GrClipMaskManager.cpp @@ -412,6 +412,7 @@ bool GrClipMaskManager::drawElement(GrTexture* target, default: { SkPath path; element->asPath(&path); + path.setIsVolatile(true); if (path.isInverseFillType()) { path.toggleInverseFillType(); } diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 02766b6..07b7f75 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -343,6 +343,7 @@ void SkGpuDevice::drawPoints(const SkDraw& draw, SkCanvas::PointMode mode, GrPaint grPaint; SkPaint2GrPaintShader(this->context(), paint, true, &grPaint); SkPath path; + path.setIsVolatile(true); path.moveTo(pts[0]); path.lineTo(pts[1]); fContext->drawPath(grPaint, path, strokeInfo); @@ -419,6 +420,7 @@ void SkGpuDevice::drawRect(const SkDraw& draw, const SkRect& rect, if (usePath) { SkPath path; + path.setIsVolatile(true); path.addRect(rect); this->drawPath(draw, path, paint, NULL, true); return; @@ -485,6 +487,7 @@ void SkGpuDevice::drawRRect(const SkDraw& draw, const SkRRect& rect, if (usePath) { SkPath path; + path.setIsVolatile(true); path.addRRect(rect); this->drawPath(draw, path, paint, NULL, true); return; @@ -511,6 +514,7 @@ void SkGpuDevice::drawDRRect(const SkDraw& draw, const SkRRect& outer, } SkPath path; + path.setIsVolatile(true); path.addRRect(outer); path.addRRect(inner); path.setFillType(SkPath::kEvenOdd_FillType); @@ -542,6 +546,7 @@ void SkGpuDevice::drawOval(const SkDraw& draw, const SkRect& oval, if (usePath) { SkPath path; + path.setIsVolatile(true); path.addOval(oval); this->drawPath(draw, path, paint, NULL, true); return; @@ -691,6 +696,8 @@ void SkGpuDevice::drawPath(const SkDraw& draw, const SkPath& origSrcPath, CHECK_SHOULD_DRAW(draw, false); GR_CREATE_TRACE_MARKER_CONTEXT("SkGpuDevice::drawPath", fContext); + SkASSERT(!pathIsMutable || origSrcPath.isVolatile()); + GrPaint grPaint; SkPaint2GrPaintShader(this->context(), paint, true, &grPaint); @@ -706,6 +713,7 @@ void SkGpuDevice::drawPath(const SkDraw& draw, const SkPath& origSrcPath, if (!pathIsMutable) { result = tmpPath.init(); + result->setIsVolatile(true); pathIsMutable = true; } // should I push prePathMatrix on our MV stack temporarily, instead @@ -740,6 +748,9 @@ void SkGpuDevice::drawPath(const SkDraw& draw, const SkPath& origSrcPath, // avoid possibly allocating a new path in transform if we can SkPath* devPathPtr = pathIsMutable ? pathPtr : tmpPath.init(); + if (!pathIsMutable) { + devPathPtr->setIsVolatile(true); + } // transform the path into device space pathPtr->transform(fContext->getMatrix(), devPathPtr); -- 2.7.4