Tessellation bug fixes
* Fix visitProxies when using DDL
* Handle when the GrClip is nullptr
* Use the same cropping/hw disable logic for onStencilClip as we do
for onDrawClip.
Bug: skia:10419
Change-Id: I2ebd2b12ac9cc77cece6d3fbf6c16e246bafe544
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344479
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/tessellate/GrPathTessellateOp.cpp b/src/gpu/tessellate/GrPathTessellateOp.cpp
index 0e38c17..62e307a 100644
--- a/src/gpu/tessellate/GrPathTessellateOp.cpp
+++ b/src/gpu/tessellate/GrPathTessellateOp.cpp
@@ -29,6 +29,14 @@
using OpFlags = GrTessellationPathRenderer::OpFlags;
+void GrPathTessellateOp::visitProxies(const VisitProxyFunc& fn) const {
+ if (fPipelineForFills) {
+ fPipelineForFills->visitProxies(fn);
+ } else {
+ fProcessors.visitProxies(fn);
+ }
+}
+
GrPathTessellateOp::FixedFunctionFlags GrPathTessellateOp::fixedFunctionFlags() const {
auto flags = FixedFunctionFlags::kUsesStencil;
if (GrAAType::kNone != fAAType) {
@@ -384,8 +392,9 @@
}
fPipelineForFills = GrSimpleMeshDrawOpHelper::CreatePipeline(
- args.fCaps, args.fArena, args.fWriteView.swizzle(), std::move(*args.fClip),
- *args.fDstProxyView, std::move(fProcessors), pipelineFlags);
+ args.fCaps, args.fArena, args.fWriteView.swizzle(),
+ (args.fClip) ? std::move(*args.fClip) : GrAppliedClip::Disabled(), *args.fDstProxyView,
+ std::move(fProcessors), pipelineFlags);
}
void GrPathTessellateOp::onPrepare(GrOpFlushState* flushState) {
diff --git a/src/gpu/tessellate/GrPathTessellateOp.h b/src/gpu/tessellate/GrPathTessellateOp.h
index 03b1a4e..622f9c8 100644
--- a/src/gpu/tessellate/GrPathTessellateOp.h
+++ b/src/gpu/tessellate/GrPathTessellateOp.h
@@ -38,7 +38,7 @@
}
const char* name() const override { return "GrPathTessellateOp"; }
- void visitProxies(const VisitProxyFunc& fn) const override { fProcessors.visitProxies(fn); }
+ void visitProxies(const VisitProxyFunc& fn) const override;
GrProcessorSet::Analysis finalize(const GrCaps& caps, const GrAppliedClip* clip,
bool hasMixedSampledCoverage,
GrClampType clampType) override {
diff --git a/src/gpu/tessellate/GrStrokeIndirectOp.cpp b/src/gpu/tessellate/GrStrokeIndirectOp.cpp
index 09e54ef..8aab558 100644
--- a/src/gpu/tessellate/GrStrokeIndirectOp.cpp
+++ b/src/gpu/tessellate/GrStrokeIndirectOp.cpp
@@ -28,8 +28,8 @@
return;
}
this->prePreparePrograms(GrStrokeTessellateShader::Mode::kIndirect, arena, writeView,
- std::move(*clip), dstProxyView, renderPassXferBarriers, colorLoadOp,
- *context->priv().caps());
+ (clip) ? std::move(*clip) : GrAppliedClip::Disabled(), dstProxyView,
+ renderPassXferBarriers, colorLoadOp, *context->priv().caps());
if (fFillProgram) {
context->priv().recordProgramInfo(fFillProgram);
}
diff --git a/src/gpu/tessellate/GrStrokeOp.cpp b/src/gpu/tessellate/GrStrokeOp.cpp
index a96d812..29f1128 100644
--- a/src/gpu/tessellate/GrStrokeOp.cpp
+++ b/src/gpu/tessellate/GrStrokeOp.cpp
@@ -29,6 +29,16 @@
this->setBounds(devBounds, HasAABloat(GrAAType::kCoverage == fAAType), IsHairline::kNo);
}
+void GrStrokeOp::visitProxies(const VisitProxyFunc& fn) const {
+ if (fFillProgram) {
+ fFillProgram->visitFPProxies(fn);
+ } else if (fStencilProgram) {
+ fStencilProgram->visitFPProxies(fn);
+ } else {
+ fProcessors.visitProxies(fn);
+ }
+}
+
GrDrawOp::FixedFunctionFlags GrStrokeOp::fixedFunctionFlags() const {
// We might not actually end up needing stencil, but won't know for sure until finalize().
// Request it just in case we do end up needing it.
diff --git a/src/gpu/tessellate/GrStrokeOp.h b/src/gpu/tessellate/GrStrokeOp.h
index 7cfa216..c5adc91 100644
--- a/src/gpu/tessellate/GrStrokeOp.h
+++ b/src/gpu/tessellate/GrStrokeOp.h
@@ -30,7 +30,7 @@
GrPaint&&);
const char* name() const override { return "GrStrokeTessellateOp"; }
- void visitProxies(const VisitProxyFunc& fn) const override { fProcessors.visitProxies(fn); }
+ void visitProxies(const VisitProxyFunc& fn) const override;
FixedFunctionFlags fixedFunctionFlags() const override;
GrProcessorSet::Analysis finalize(const GrCaps&, const GrAppliedClip*,
bool hasMixedSampledCoverage, GrClampType) override;
diff --git a/src/gpu/tessellate/GrStrokeTessellateOp.cpp b/src/gpu/tessellate/GrStrokeTessellateOp.cpp
index 5ee4226..54811e4 100644
--- a/src/gpu/tessellate/GrStrokeTessellateOp.cpp
+++ b/src/gpu/tessellate/GrStrokeTessellateOp.cpp
@@ -20,9 +20,9 @@
GrXferBarrierFlags renderPassXferBarriers,
GrLoadOp colorLoadOp) {
this->prePreparePrograms(GrStrokeTessellateShader::Mode::kTessellation,
- context->priv().recordTimeAllocator(), writeView, std::move(*clip),
- dstProxyView, renderPassXferBarriers, colorLoadOp,
- *context->priv().caps());
+ context->priv().recordTimeAllocator(), writeView,
+ (clip) ? std::move(*clip) : GrAppliedClip::Disabled(), dstProxyView,
+ renderPassXferBarriers, colorLoadOp, *context->priv().caps());
if (fStencilProgram) {
context->priv().recordProgramInfo(fStencilProgram);
}
diff --git a/src/gpu/tessellate/GrTessellationPathRenderer.cpp b/src/gpu/tessellate/GrTessellationPathRenderer.cpp
index ef4d835..5d20a30 100644
--- a/src/gpu/tessellate/GrTessellationPathRenderer.cpp
+++ b/src/gpu/tessellate/GrTessellationPathRenderer.cpp
@@ -141,33 +141,100 @@
return CanDrawPath::kYes;
}
-static GrOp::Owner make_stroke_op(GrRecordingContext* context, GrAAType aaType,
- const SkMatrix& viewMatrix, const SkStrokeRec& stroke,
- const SkPath& path, GrPaint&& paint,
- const GrShaderCaps& shaderCaps) {
- // Only use hardware tessellation if the path has a somewhat large number of verbs. Otherwise we
- // seem to be better off using indirect draws. Our back door for HW tessellation shaders isn't
- // currently capable of passing varyings to the fragment shader either, so if the paint uses
- // varyings we need to use indirect draws.
- if (shaderCaps.tessellationSupport() && path.countVerbs() > 50 && !paint.usesVaryingCoords() &&
- !SkPathPriv::ConicWeightCnt(path)) {
- return GrOp::Make<GrStrokeTessellateOp>(context, aaType, viewMatrix, stroke, path,
- std::move(paint));
+static GrOp::Owner make_op(GrRecordingContext* rContext, const GrSurfaceContext* surfaceContext,
+ GrTessellationPathRenderer::OpFlags opFlags, GrAAType aaType,
+ const SkRect& shapeDevBounds, const SkMatrix& viewMatrix,
+ const GrStyledShape& shape, GrPaint&& paint) {
+ constexpr static auto kLinearizationIntolerance =
+ GrTessellationPathRenderer::kLinearizationIntolerance;
+ constexpr static auto kMaxResolveLevel = GrTessellationPathRenderer::kMaxResolveLevel;
+ using OpFlags = GrTessellationPathRenderer::OpFlags;
+
+ const GrShaderCaps& shaderCaps = *rContext->priv().caps()->shaderCaps();
+
+ SkPath path;
+ shape.asPath(&path);
+
+ // Find the worst-case log2 number of line segments that a curve in this path might need to be
+ // divided into.
+ int worstCaseResolveLevel = GrWangsFormula::worst_case_cubic_log2(kLinearizationIntolerance,
+ shapeDevBounds.width(),
+ shapeDevBounds.height());
+ if (worstCaseResolveLevel > kMaxResolveLevel) {
+ // The path is too large for our internal indirect draw shaders. Crop it to the viewport.
+ auto viewport = SkRect::MakeIWH(surfaceContext->width(), surfaceContext->height());
+ float inflationRadius = 1;
+ const SkStrokeRec& stroke = shape.style().strokeRec();
+ if (stroke.getStyle() == SkStrokeRec::kHairline_Style) {
+ inflationRadius += SkStrokeRec::GetInflationRadius(stroke.getJoin(), stroke.getMiter(),
+ stroke.getCap(), 1);
+ } else if (stroke.getStyle() != SkStrokeRec::kFill_Style) {
+ inflationRadius += stroke.getInflationRadius() * viewMatrix.getMaxScale();
+ }
+ viewport.outset(inflationRadius, inflationRadius);
+
+ SkPath viewportPath;
+ viewportPath.addRect(viewport);
+ // Perform the crop in device space so it's a simple rect-path intersection.
+ path.transform(viewMatrix);
+ if (!Op(viewportPath, path, kIntersect_SkPathOp, &path)) {
+ // The crop can fail if the PathOps encounter NaN or infinities. Return true
+ // because drawing nothing is acceptable behavior for FP overflow.
+ return nullptr;
+ }
+
+ // Transform the path back to its own local space.
+ SkMatrix inverse;
+ if (!viewMatrix.invert(&inverse)) {
+ return nullptr; // Singular view matrix. Nothing would have drawn anyway. Return null.
+ }
+ path.transform(inverse);
+ path.setIsVolatile(true);
+
+ SkRect newDevBounds;
+ viewMatrix.mapRect(&newDevBounds, path.getBounds());
+ worstCaseResolveLevel = GrWangsFormula::worst_case_cubic_log2(kLinearizationIntolerance,
+ newDevBounds.width(),
+ newDevBounds.height());
+ // kMaxResolveLevel should be large enough to tessellate paths the size of any screen we
+ // might encounter.
+ SkASSERT(worstCaseResolveLevel <= kMaxResolveLevel);
+ }
+
+ if (!shape.style().isSimpleFill()) {
+ const SkStrokeRec& stroke = shape.style().strokeRec();
+ SkASSERT(stroke.getStyle() != SkStrokeRec::kStrokeAndFill_Style);
+ // Only use hardware tessellation if the path has a somewhat large number of verbs.
+ // Otherwise we seem to be better off using indirect draws. Our back door for HW
+ // tessellation shaders isn't currently capable of passing varyings to the fragment shader
+ // either, so if the paint uses varyings we need to use indirect draws.
+ if (shaderCaps.tessellationSupport() &&
+ path.countVerbs() > 50 &&
+ !paint.usesVaryingCoords() &&
+ !SkPathPriv::ConicWeightCnt(path)) {
+ return GrOp::Make<GrStrokeTessellateOp>(rContext, aaType, viewMatrix, stroke, path,
+ std::move(paint));
+ } else {
+ return GrOp::Make<GrStrokeIndirectOp>(rContext, aaType, viewMatrix, path, stroke,
+ std::move(paint));
+ }
} else {
- return GrOp::Make<GrStrokeIndirectOp>(context, aaType, viewMatrix, path, stroke,
- std::move(paint));
+ if ((1 << worstCaseResolveLevel) > shaderCaps.maxTessellationSegments()) {
+ // The path is too large for hardware tessellation; a curve in this bounding box could
+ // potentially require more segments than are supported by the hardware. Fall back on
+ // indirect draws.
+ opFlags |= OpFlags::kDisableHWTessellation;
+ }
+ return GrOp::Make<GrPathTessellateOp>(rContext, viewMatrix, path, std::move(paint), aaType,
+ opFlags);
}
}
bool GrTessellationPathRenderer::onDrawPath(const DrawPathArgs& args) {
GrSurfaceDrawContext* surfaceDrawContext = args.fRenderTargetContext;
- const GrShaderCaps& shaderCaps = *args.fContext->priv().caps()->shaderCaps();
-
- SkPath path;
- args.fShape->asPath(&path);
SkRect devBounds;
- args.fViewMatrix->mapRect(&devBounds, path.getBounds());
+ args.fViewMatrix->mapRect(&devBounds, args.fShape->bounds());
// See if the path is small and simple enough to atlas instead of drawing directly.
//
@@ -177,9 +244,9 @@
SkIRect devIBounds;
SkIPoint16 locationInAtlas;
bool transposedInAtlas;
- if (args.fShape->style().isSimpleFill() &&
- this->tryAddPathToAtlas(*args.fContext->priv().caps(), *args.fViewMatrix, path, devBounds,
- args.fAAType, &devIBounds, &locationInAtlas, &transposedInAtlas)) {
+ if (this->tryAddPathToAtlas(*args.fContext->priv().caps(), *args.fViewMatrix, *args.fShape,
+ devBounds, args.fAAType, &devIBounds, &locationInAtlas,
+ &transposedInAtlas)) {
// The atlas is not compatible with DDL. We should only be using it on direct contexts.
SkASSERT(args.fContext->asDirectContext());
#ifdef SK_DEBUG
@@ -190,6 +257,7 @@
int worstCaseNumSegments = GrWangsFormula::worst_case_cubic(kLinearizationIntolerance,
devIBounds.width(),
devIBounds.height());
+ const GrShaderCaps& shaderCaps = *args.fContext->priv().caps()->shaderCaps();
SkASSERT(worstCaseNumSegments <= shaderCaps.maxTessellationSegments());
}
#endif
@@ -201,76 +269,21 @@
return true;
}
- // Find the worst-case log2 number of line segments that a curve in this path might need to be
- // divided into.
- int worstCaseResolveLevel = GrWangsFormula::worst_case_cubic_log2(kLinearizationIntolerance,
- devBounds.width(),
- devBounds.height());
- if (worstCaseResolveLevel > kMaxResolveLevel) {
- // The path is too large for our internal indirect draw shaders. Crop it to the viewport.
- auto viewport = SkRect::MakeIWH(surfaceDrawContext->width(),
- surfaceDrawContext->height());
- float inflationRadius = 1;
- const SkStrokeRec& stroke = args.fShape->style().strokeRec();
- if (stroke.getStyle() == SkStrokeRec::kHairline_Style) {
- inflationRadius += SkStrokeRec::GetInflationRadius(stroke.getJoin(), stroke.getMiter(),
- stroke.getCap(), 1);
- } else if (stroke.getStyle() != SkStrokeRec::kFill_Style) {
- inflationRadius += stroke.getInflationRadius() * args.fViewMatrix->getMaxScale();
- }
- viewport.outset(inflationRadius, inflationRadius);
-
- SkPath viewportPath;
- viewportPath.addRect(viewport);
- // Perform the crop in device space so it's a simple rect-path intersection.
- path.transform(*args.fViewMatrix);
- if (!Op(viewportPath, path, kIntersect_SkPathOp, &path)) {
- // The crop can fail if the PathOps encounter NaN or infinities. Return true
- // because drawing nothing is acceptable behavior for FP overflow.
- return true;
- }
-
- // Transform the path back to its own local space.
- SkMatrix inverse;
- if (!args.fViewMatrix->invert(&inverse)) {
- return true; // Singular view matrix. Nothing would have drawn anyway. Return true.
- }
- path.transform(inverse);
- path.setIsVolatile(true);
- args.fViewMatrix->mapRect(&devBounds, path.getBounds());
- worstCaseResolveLevel = GrWangsFormula::worst_case_cubic_log2(kLinearizationIntolerance,
- devBounds.width(),
- devBounds.height());
- // kMaxResolveLevel should be large enough to tessellate paths the size of any screen we
- // might encounter.
- SkASSERT(worstCaseResolveLevel <= kMaxResolveLevel);
+ if (auto op = make_op(args.fContext, surfaceDrawContext, OpFlags::kNone, args.fAAType,
+ devBounds, *args.fViewMatrix, *args.fShape, std::move(args.fPaint))) {
+ surfaceDrawContext->addDrawOp(args.fClip, std::move(op));
}
-
- GrOp::Owner op;
- if (!args.fShape->style().isSimpleFill()) {
- const SkStrokeRec& stroke = args.fShape->style().strokeRec();
- SkASSERT(stroke.getStyle() != SkStrokeRec::kStrokeAndFill_Style);
- op = make_stroke_op(args.fContext, args.fAAType, *args.fViewMatrix, stroke, path,
- std::move(args.fPaint), shaderCaps);
- } else {
- auto drawPathFlags = OpFlags::kNone;
- if ((1 << worstCaseResolveLevel) > shaderCaps.maxTessellationSegments()) {
- // The path is too large for hardware tessellation; a curve in this bounding box could
- // potentially require more segments than are supported by the hardware. Fall back on
- // indirect draws.
- drawPathFlags |= OpFlags::kDisableHWTessellation;
- }
- op = GrOp::Make<GrPathTessellateOp>(args.fContext, *args.fViewMatrix, path,
- std::move(args.fPaint), args.fAAType, drawPathFlags);
- }
- surfaceDrawContext->addDrawOp(args.fClip, std::move(op));
return true;
}
bool GrTessellationPathRenderer::tryAddPathToAtlas(
- const GrCaps& caps, const SkMatrix& viewMatrix, const SkPath& path, const SkRect& devBounds,
- GrAAType aaType, SkIRect* devIBounds, SkIPoint16* locationInAtlas,
+ const GrCaps& caps, const SkMatrix& viewMatrix, const GrStyledShape& shape,
+ const SkRect& devBounds, GrAAType aaType, SkIRect* devIBounds, SkIPoint16* locationInAtlas,
bool* transposedInAtlas) {
+ if (!shape.style().isSimpleFill()) {
+ return false;
+ }
+
if (!fMaxAtlasPathWidth) {
return false;
}
@@ -281,6 +294,8 @@
// Atlas paths require their points to be transformed on the CPU and copied into an "uber path".
// Check if this path has too many points to justify this extra work.
+ SkPath path;
+ shape.asPath(&path);
if (path.countPoints() > 200) {
return false;
}
@@ -327,14 +342,14 @@
}
void GrTessellationPathRenderer::onStencilPath(const StencilPathArgs& args) {
- SkPath path;
- args.fShape->asPath(&path);
-
+ GrSurfaceDrawContext* surfaceDrawContext = args.fRenderTargetContext;
GrAAType aaType = (GrAA::kYes == args.fDoStencilMSAA) ? GrAAType::kMSAA : GrAAType::kNone;
-
- auto op = GrOp::Make<GrPathTessellateOp>(
- args.fContext, *args.fViewMatrix, path, GrPaint(), aaType, OpFlags::kStencilOnly);
- args.fRenderTargetContext->addDrawOp(args.fClip, std::move(op));
+ SkRect devBounds;
+ args.fViewMatrix->mapRect(&devBounds, args.fShape->bounds());
+ if (auto op = make_op(args.fContext, surfaceDrawContext, OpFlags::kStencilOnly, aaType,
+ devBounds, *args.fViewMatrix, *args.fShape, GrPaint())) {
+ surfaceDrawContext->addDrawOp(args.fClip, std::move(op));
+ }
}
void GrTessellationPathRenderer::preFlush(GrOnFlushResourceProvider* onFlushRP,
diff --git a/src/gpu/tessellate/GrTessellationPathRenderer.h b/src/gpu/tessellate/GrTessellationPathRenderer.h
index 88840fc..020ae0d 100644
--- a/src/gpu/tessellate/GrTessellationPathRenderer.h
+++ b/src/gpu/tessellate/GrTessellationPathRenderer.h
@@ -64,9 +64,9 @@
return &fAtlasUberPaths[idx];
}
// Allocates space in fAtlas if the path is small and simple enough, and if there is room.
- bool tryAddPathToAtlas(const GrCaps&, const SkMatrix&, const SkPath&, const SkRect& devBounds,
- GrAAType, SkIRect* devIBounds, SkIPoint16* locationInAtlas,
- bool* transposedInAtlas);
+ bool tryAddPathToAtlas(const GrCaps&, const SkMatrix&, const GrStyledShape&,
+ const SkRect& devBounds, GrAAType, SkIRect* devIBounds,
+ SkIPoint16* locationInAtlas, bool* transposedInAtlas);
void renderAtlas(GrOnFlushResourceProvider*);
GrDynamicAtlas fAtlas;