From 7bfdb1044916cbe207d6adca90158e8b854bacd7 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Fri, 18 Dec 2020 14:56:52 +0000 Subject: [PATCH] Revert "Enable GrTessellationPathRenderer by default" This reverts commit 6ea387e7c751cbfd3b0cc3267b95808550448ded. Reason for revert: valgrind failing Original change's description: > Enable GrTessellationPathRenderer by default > > Moves GrTessellationPathRenderer to the end of the chain and enables > it by default. > > Also updates nvpr to not draw volatile paths. The tessellator is much > faster at these. > > Bug: skia:10419 > Change-Id: I97ca7d4d1dff65fc9d4040c267f9808c8c33b548 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344377 > Commit-Queue: Chris Dalton > Reviewed-by: Brian Salomon TBR=bsalomon@google.com,csmartdalton@google.com Change-Id: I3718ae210ebc403959d187160214cf0df895e4f5 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:10419 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345718 Reviewed-by: Brian Salomon Commit-Queue: Brian Salomon --- gm/addarc.cpp | 4 ++-- include/private/GrTypesPriv.h | 2 +- src/gpu/GrCaps.cpp | 4 ---- src/gpu/GrCaps.h | 4 ---- src/gpu/GrPathRendererChain.cpp | 14 +++++------ src/gpu/gl/GrGLCaps.cpp | 8 ------- src/gpu/ops/GrStencilAndCoverPathRenderer.cpp | 3 +-- src/gpu/tessellate/GrFillPathShader.cpp | 23 ++++++++----------- .../tessellate/GrStrokeTessellateShader.cpp | 2 +- .../tessellate/GrTessellationPathRenderer.cpp | 6 ++--- tools/flags/CommonFlagsGpu.cpp | 8 +++---- 11 files changed, 28 insertions(+), 50 deletions(-) diff --git a/gm/addarc.cpp b/gm/addarc.cpp index 1d44be9a65..f19822218a 100644 --- a/gm/addarc.cpp +++ b/gm/addarc.cpp @@ -55,7 +55,7 @@ protected: SkPathBuilder path; path.addArc(r, startAngle, sweepAngle); - canvas->drawPath(path.detach().setIsVolatile(true), paint); + canvas->drawPath(path.detach(), paint); r.inset(inset, inset); sign = -sign; @@ -260,7 +260,7 @@ DEF_SIMPLE_GM(manyarcs, canvas, 620, 330) { html_canvas_arc(&path, 18, 15, 10, startAngle, startAngle + (sweepAngles[j] * sign), anticlockwise, true); path.lineTo(0, 28); - canvas->drawPath(path.detach().setIsVolatile(true), paint); + canvas->drawPath(path.detach(), paint); canvas->translate(30, 0); } canvas->restore(); diff --git a/include/private/GrTypesPriv.h b/include/private/GrTypesPriv.h index 119dbaa196..ef57711126 100644 --- a/include/private/GrTypesPriv.h +++ b/include/private/GrTypesPriv.h @@ -780,7 +780,7 @@ enum class GpuPathRenderers { kAALinearizing = 1 << 6, kSmall = 1 << 7, kTriangulating = 1 << 8, - kDefault = ((1 << 9) - 1) // All path renderers. + kDefault = ((1 << 9) - 1) & ~kTessellation // All but kTessellation. }; /** diff --git a/src/gpu/GrCaps.cpp b/src/gpu/GrCaps.cpp index 9cffcc0a3b..2181858fcc 100644 --- a/src/gpu/GrCaps.cpp +++ b/src/gpu/GrCaps.cpp @@ -58,7 +58,6 @@ GrCaps::GrCaps(const GrContextOptions& options) { fShouldCollapseSrcOverToSrcWhenAble = false; fDriverDisableCCPR = false; fDriverDisableMSAACCPR = false; - fDisableTessellationPathRenderer = false; fBlendEquationSupport = kBasic_BlendEquationSupport; fAdvBlendEqDisableFlags = 0; @@ -117,7 +116,6 @@ void GrCaps::applyOptionsOverrides(const GrContextOptions& options) { if (options.fDisableDriverCorrectnessWorkarounds) { SkASSERT(!fDriverDisableCCPR); SkASSERT(!fDriverDisableMSAACCPR); - SkASSERT(!fDisableTessellationPathRenderer); SkASSERT(!fAvoidStencilBuffers); SkASSERT(!fAvoidWritePixelsFastPath); SkASSERT(!fRequiresManualFBBarrierAfterTessellatedStencilDraw); @@ -248,8 +246,6 @@ void GrCaps::dumpJSON(SkJSONWriter* writer) const { writer->appendBool("Disable CCPR on current driver [workaround]", fDriverDisableCCPR); writer->appendBool("Disable MSAA version of CCPR on current driver [workaround]", fDriverDisableMSAACCPR); - writer->appendBool("Disable GrTessellationPathRenderer current driver [workaround]", - fDisableTessellationPathRenderer); writer->appendBool("Clamp-to-border", fClampToBorderSupport); writer->appendBool("Prefer VRAM Use over flushes [workaround]", fPreferVRAMUseOverFlushes); diff --git a/src/gpu/GrCaps.h b/src/gpu/GrCaps.h index 4a51fcdc61..70be646606 100644 --- a/src/gpu/GrCaps.h +++ b/src/gpu/GrCaps.h @@ -381,9 +381,6 @@ public: bool driverDisableCCPR() const { return fDriverDisableCCPR; } bool driverDisableMSAACCPR() const { return fDriverDisableMSAACCPR; } - // Should we disable GrTessellationPathRenderer due to a faulty driver? - bool disableTessellationPathRenderer() const { return fDisableTessellationPathRenderer; } - // Returns how to sample the dst values for the passed in GrRenderTargetProxy. GrDstSampleType getDstSampleTypeForProxy(const GrRenderTargetProxy*) const; @@ -513,7 +510,6 @@ protected: // Driver workaround bool fDriverDisableCCPR : 1; bool fDriverDisableMSAACCPR : 1; - bool fDisableTessellationPathRenderer : 1; bool fAvoidStencilBuffers : 1; bool fAvoidWritePixelsFastPath : 1; bool fRequiresManualFBBarrierAfterTessellatedStencilDraw : 1; diff --git a/src/gpu/GrPathRendererChain.cpp b/src/gpu/GrPathRendererChain.cpp index 62b6e8aabd..4f5807cf6c 100644 --- a/src/gpu/GrPathRendererChain.cpp +++ b/src/gpu/GrPathRendererChain.cpp @@ -32,6 +32,13 @@ GrPathRendererChain::GrPathRendererChain(GrRecordingContext* context, const Opti if (options.fGpuPathRenderers & GpuPathRenderers::kDashLine) { fChain.push_back(sk_make_sp()); } + if (options.fGpuPathRenderers & GpuPathRenderers::kTessellation) { + if (GrTessellationPathRenderer::IsSupported(caps)) { + auto tess = sk_make_sp(context); + context->priv().addOnFlushCallbackObject(tess.get()); + fChain.push_back(std::move(tess)); + } + } if (options.fGpuPathRenderers & GpuPathRenderers::kAAConvex) { fChain.push_back(sk_make_sp()); } @@ -69,13 +76,6 @@ GrPathRendererChain::GrPathRendererChain(GrRecordingContext* context, const Opti if (options.fGpuPathRenderers & GpuPathRenderers::kTriangulating) { fChain.push_back(sk_make_sp()); } - if (options.fGpuPathRenderers & GpuPathRenderers::kTessellation) { - if (GrTessellationPathRenderer::IsSupported(caps)) { - auto tess = sk_make_sp(context); - context->priv().addOnFlushCallbackObject(tess.get()); - fChain.push_back(std::move(tess)); - } - } // We always include the default path renderer (as well as SW), so we can draw any path fChain.push_back(sk_make_sp()); diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp index 7770d77d49..4efaec2ee3 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/src/gpu/gl/GrGLCaps.cpp @@ -3982,14 +3982,6 @@ void GrGLCaps::applyDriverCorrectnessWorkarounds(const GrGLContextInfo& ctxInfo, fDriverDisableMSAACCPR = true; } - if (kIntel_GrGLVendor == ctxInfo.vendor() || // IntelIris640 drops draws completely. - ctxInfo.renderer() == kMaliT_GrGLRenderer || // Some curves appear flat on GalaxyS6. - ctxInfo.renderer() == kAdreno3xx_GrGLRenderer || - ctxInfo.renderer() == kAdreno430_GrGLRenderer || - ctxInfo.renderer() == kAdreno4xx_other_GrGLRenderer) { // We get garbage on Adreno405. - fDisableTessellationPathRenderer = true; - } - // http://skbug.com/9739 bool isNVIDIAPascal = kNVIDIA_GrGLDriver == ctxInfo.driver() && diff --git a/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp b/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp index cbe0c14cdd..6a5a356028 100644 --- a/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp +++ b/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp @@ -49,8 +49,7 @@ GrStencilAndCoverPathRenderer::onCanDrawPath(const CanDrawPathArgs& args) const // path. if (args.fShape->style().strokeRec().isHairlineStyle() || args.fShape->style().hasNonDashPathEffect() || - args.fHasUserStencilSettings || - !args.fShape->hasUnstyledKey()) { + args.fHasUserStencilSettings) { return CanDrawPath::kNo; } if (GrAAType::kCoverage == args.fAAType && !args.fProxy->canUseMixedSamples(*args.fCaps)) { diff --git a/src/gpu/tessellate/GrFillPathShader.cpp b/src/gpu/tessellate/GrFillPathShader.cpp index 11c5c1399e..3cb0fcbc10 100644 --- a/src/gpu/tessellate/GrFillPathShader.cpp +++ b/src/gpu/tessellate/GrFillPathShader.cpp @@ -98,24 +98,21 @@ void GrFillCubicHullShader::emitVertexCode(Impl*, GrGLSLVertexBuilder* v, const } } + // Find the "turn direction" of each corner and net turn direction. + float4 dir; + float netdir = 0.0; + for (int i = 0; i < 4; ++i) { + float2 prev = P[i] - P[(i + 3) & 3], next = P[(i + 1) & 3] - P[i]; + dir[i] = sign(determinant(float2x2(prev, next))); + netdir += dir[i]; + } + // sk_VertexID comes in fan order. Convert to strip order. int vertexidx = sk_VertexID; vertexidx ^= vertexidx >> 1; - // Find the "turn direction" of each corner and net turn direction. - float vertexdir = 0; - float netdir = 0; - for (int i = 0; i < 4; ++i) { - float2 prev = P[i] - P[(i + 3) & 3], next = P[(i + 1) & 3] - P[i]; - float dir = sign(determinant(float2x2(prev, next))); - if (i == vertexidx) { - vertexdir = dir; - } - netdir += dir; - } - // Remove the non-convex vertex, if any. - if (vertexdir != sign(netdir)) { + if (dir[vertexidx] != sign(netdir)) { vertexidx = (vertexidx + 1) & 3; } diff --git a/src/gpu/tessellate/GrStrokeTessellateShader.cpp b/src/gpu/tessellate/GrStrokeTessellateShader.cpp index e10292aeaf..66f2289b52 100644 --- a/src/gpu/tessellate/GrStrokeTessellateShader.cpp +++ b/src/gpu/tessellate/GrStrokeTessellateShader.cpp @@ -1022,7 +1022,7 @@ class GrStrokeTessellateShader::IndirectImpl : public GrGLSLGeometryProcessor { // for seaming with the previous stroke. (The double sided edge at the end will // actually come from the section of our strip that belongs to the stroke.) if (combinedEdgeID >= 0) { - outset = (turn < 0) ? min(outset, 0) : max(outset, 0); + outset = clamp(outset, (turn < 0) ? -1 : 0, (turn >= 0) ? 1 : 0); } } combinedEdgeID = max(combinedEdgeID, 0); diff --git a/src/gpu/tessellate/GrTessellationPathRenderer.cpp b/src/gpu/tessellate/GrTessellationPathRenderer.cpp index 110128ae20..5d20a30b1c 100644 --- a/src/gpu/tessellate/GrTessellationPathRenderer.cpp +++ b/src/gpu/tessellate/GrTessellationPathRenderer.cpp @@ -36,9 +36,7 @@ constexpr static auto kAtlasAlgorithm = GrDynamicAtlas::RectanizerAlgorithm::kPo constexpr static int kMaxAtlasPathHeight = 128; bool GrTessellationPathRenderer::IsSupported(const GrCaps& caps) { - return caps.drawInstancedSupport() && - caps.shaderCaps()->vertexIDSupport() && - !caps.disableTessellationPathRenderer(); + return caps.drawInstancedSupport() && caps.shaderCaps()->vertexIDSupport(); } GrTessellationPathRenderer::GrTessellationPathRenderer(GrRecordingContext* rContext) @@ -315,7 +313,7 @@ bool GrTessellationPathRenderer::tryAddPathToAtlas( // Check if the path is too large for an atlas. Since we use "minDimension" for height in the // atlas, limiting to kMaxAtlasPathHeight^2 pixels guarantees height <= kMaxAtlasPathHeight. - if ((uint64_t)maxDimenstion * minDimension > kMaxAtlasPathHeight * kMaxAtlasPathHeight || + if (maxDimenstion * minDimension > kMaxAtlasPathHeight * kMaxAtlasPathHeight || maxDimenstion > fMaxAtlasPathWidth) { return false; } diff --git a/tools/flags/CommonFlagsGpu.cpp b/tools/flags/CommonFlagsGpu.cpp index 04b7540a2f..b83ced1dc1 100644 --- a/tools/flags/CommonFlagsGpu.cpp +++ b/tools/flags/CommonFlagsGpu.cpp @@ -29,8 +29,8 @@ static DEFINE_bool(cc, false, "Allow coverage counting shortcuts to render paths static DEFINE_string(pr, "", "Set of enabled gpu path renderers. Defined as a list of: " - "[~]none [~]dashline [~]nvpr [~]ccpr [~]aahairline [~]aaconvex [~]aalinearizing " - "[~]small [~]tri [~]tess [~]all"); + "[~]none [~]dashline [~]tess [~]nvpr [~]ccpr [~]aahairline [~]aaconvex " + "[~]aalinearizing [~]small [~]tri] [~]all"); static DEFINE_int(internalSamples, 4, "Number of samples for internal draws that use MSAA or mixed samples."); @@ -46,6 +46,8 @@ static GpuPathRenderers get_named_pathrenderers_flags(const char* name) { return GpuPathRenderers::kNone; } else if (!strcmp(name, "dashline")) { return GpuPathRenderers::kDashLine; + } else if (!strcmp(name, "tess")) { + return GpuPathRenderers::kTessellation; } else if (!strcmp(name, "nvpr")) { return GpuPathRenderers::kStencilAndCover; } else if (!strcmp(name, "ccpr")) { @@ -60,8 +62,6 @@ static GpuPathRenderers get_named_pathrenderers_flags(const char* name) { return GpuPathRenderers::kSmall; } else if (!strcmp(name, "tri")) { return GpuPathRenderers::kTriangulating; - } else if (!strcmp(name, "tess")) { - return GpuPathRenderers::kTessellation; } else if (!strcmp(name, "default")) { return GpuPathRenderers::kDefault; }