Make GrFillRectOp and GrTextureOp consistent re AA-upgrading and size checks

I believe GrFillRectOp wasn't handling AA-upgrading correctly. Both ops now also check that they aren't exceeding the index buffer limits for both AA and non-AA draws.

Change-Id: Iae586b92e1f27a908a54ae881f2b1db21ec1afc8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/251360
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
Robert Phillips 2019-10-29 12:16:35 -04:00 committed by Skia Commit-Bot
parent 793c9e8c98
commit b69001f36f
4 changed files with 54 additions and 21 deletions

View File

@ -247,10 +247,23 @@ private:
TRACE_EVENT0("skia.gpu", TRACE_FUNC);
const auto* that = t->cast<FillRectOp>();
if ((fHelper.aaType() == GrAAType::kCoverage ||
that->fHelper.aaType() == GrAAType::kCoverage) &&
fQuads.count() + that->fQuads.count() > GrResourceProvider::MaxNumAAQuads()) {
return CombineResult::kCannotCombine;
bool upgradeToCoverageAAOnMerge = false;
if (fHelper.aaType() != that->fHelper.aaType()) {
if (!GrSimpleMeshDrawOpHelper::CanUpgradeAAOnMerge(fHelper.aaType(),
that->fHelper.aaType())) {
return CombineResult::kCannotCombine;
}
upgradeToCoverageAAOnMerge = true;
}
if (fHelper.aaType() == GrAAType::kCoverage || upgradeToCoverageAAOnMerge) {
if (fQuads.count() + that->fQuads.count() > GrResourceProvider::MaxNumAAQuads()) {
return CombineResult::kCannotCombine;
}
} else {
if (fQuads.count() + that->fQuads.count() > GrResourceProvider::MaxNumNonAAQuads()) {
return CombineResult::kCannotCombine;
}
}
// Unlike most users of the draw op helper, this op can merge none-aa and coverage-aa draw
@ -270,7 +283,7 @@ private:
// The helper stores the aa type, but isCompatible(with true arg) allows the two ops' aa
// types to be none and coverage, in which case this op's aa type must be lifted to coverage
// so that quads with no aa edges can be batched with quads that have some/all edges aa'ed.
if (fHelper.aaType() == GrAAType::kNone && that->fHelper.aaType() == GrAAType::kCoverage) {
if (upgradeToCoverageAAOnMerge) {
fHelper.setAAType(GrAAType::kCoverage);
}

View File

@ -37,14 +37,9 @@ GrDrawOp::FixedFunctionFlags GrSimpleMeshDrawOpHelper::fixedFunctionFlags() cons
: GrDrawOp::FixedFunctionFlags::kNone;
}
static bool none_as_coverage_aa_compatible(GrAAType aa1, GrAAType aa2) {
return (aa1 == GrAAType::kNone && aa2 == GrAAType::kCoverage) ||
(aa1 == GrAAType::kCoverage && aa2 == GrAAType::kNone);
}
bool GrSimpleMeshDrawOpHelper::isCompatible(const GrSimpleMeshDrawOpHelper& that,
const GrCaps& caps, const SkRect& thisBounds,
const SkRect& thatBounds, bool noneAsCoverageAA) const {
const SkRect& thatBounds, bool ignoreAAType) const {
if (SkToBool(fProcessors) != SkToBool(that.fProcessors)) {
return false;
}
@ -53,8 +48,17 @@ bool GrSimpleMeshDrawOpHelper::isCompatible(const GrSimpleMeshDrawOpHelper& that
return false;
}
}
bool result = fPipelineFlags == that.fPipelineFlags && (fAAType == that.fAAType ||
(noneAsCoverageAA && none_as_coverage_aa_compatible(this->aaType(), that.aaType())));
#ifdef SK_DEBUG
if (ignoreAAType) {
// If we're ignoring AA it should be bc we already know they are the same or that
// the are different but are compatible (i.e., one is AA and the other is None)
SkASSERT(fAAType == that.fAAType || CanUpgradeAAOnMerge(this->aaType(), that.aaType()));
}
#endif
bool result = fPipelineFlags == that.fPipelineFlags &&
(ignoreAAType || fAAType == that.fAAType);
SkASSERT(!result || fCompatibleWithCoverageAsAlpha == that.fCompatibleWithCoverageAsAlpha);
SkASSERT(!result || fUsesLocalCoords == that.fUsesLocalCoords);
return result;
@ -176,8 +180,8 @@ GrProcessorSet::Analysis GrSimpleMeshDrawOpHelperWithStencil::finalizeProcessors
bool GrSimpleMeshDrawOpHelperWithStencil::isCompatible(
const GrSimpleMeshDrawOpHelperWithStencil& that, const GrCaps& caps,
const SkRect& thisBounds, const SkRect& thatBounds, bool noneAsCoverageAA) const {
return INHERITED::isCompatible(that, caps, thisBounds, thatBounds, noneAsCoverageAA) &&
const SkRect& thisBounds, const SkRect& thatBounds, bool ignoreAAType) const {
return INHERITED::isCompatible(that, caps, thisBounds, thatBounds, ignoreAAType) &&
fStencilSettings == that.fStencilSettings;
}

View File

@ -28,6 +28,11 @@ class GrSimpleMeshDrawOpHelper {
public:
struct MakeArgs;
static bool CanUpgradeAAOnMerge(GrAAType aa1, GrAAType aa2) {
return (aa1 == GrAAType::kNone && aa2 == GrAAType::kCoverage) ||
(aa1 == GrAAType::kCoverage && aa2 == GrAAType::kNone);
}
/**
* This can be used by a Op class to perform allocation and initialization such that a
* GrProcessorSet (if required) is allocated as part of the the same allocation that as
@ -54,10 +59,9 @@ public:
GrDrawOp::FixedFunctionFlags fixedFunctionFlags() const;
// noneAACompatibleWithCoverage should be set to true if the op can properly render a non-AA
// primitive merged into a coverage-based op.
// ignoreAAType should be set to true if the op already knows the AA settings are acceptible
bool isCompatible(const GrSimpleMeshDrawOpHelper& that, const GrCaps&, const SkRect& thisBounds,
const SkRect& thatBounds, bool noneAACompatibleWithCoverage = false) const;
const SkRect& thatBounds, bool ignoreAAType = false) const;
/**
* Finalizes the processor set and determines whether the destination must be provided
@ -189,7 +193,7 @@ public:
bool isCompatible(const GrSimpleMeshDrawOpHelperWithStencil& that, const GrCaps&,
const SkRect& thisBounds, const SkRect& thatBounds,
bool noneAACompatibleWithCoverage = false) const;
bool ignoreAAType = false) const;
void executeDrawsAndUploads(const GrOp*, GrOpFlushState*, const SkRect& chainBounds);

View File

@ -39,6 +39,7 @@
#include "src/gpu/ops/GrFillRectOp.h"
#include "src/gpu/ops/GrMeshDrawOp.h"
#include "src/gpu/ops/GrQuadPerEdgeAA.h"
#include "src/gpu/ops/GrSimpleMeshDrawOpHelper.h"
#include "src/gpu/ops/GrTextureOp.h"
namespace {
@ -781,14 +782,25 @@ private:
that->fTextureColorSpaceXform.get())) {
return CombineResult::kCannotCombine;
}
bool upgradeToCoverageAAOnMerge = false;
if (this->aaType() != that->aaType()) {
if (!((this->aaType() == GrAAType::kCoverage && that->aaType() == GrAAType::kNone) ||
(that->aaType() == GrAAType::kCoverage && this->aaType() == GrAAType::kNone))) {
if (!GrSimpleMeshDrawOpHelper::CanUpgradeAAOnMerge(this->aaType(), that->aaType())) {
return CombineResult::kCannotCombine;
}
upgradeToCoverageAAOnMerge = true;
}
if (this->aaType() == GrAAType::kCoverage || upgradeToCoverageAAOnMerge) {
if (fQuads.count() + that->fQuads.count() > GrResourceProvider::MaxNumAAQuads()) {
return CombineResult::kCannotCombine;
}
} else {
if (fQuads.count() + that->fQuads.count() > GrResourceProvider::MaxNumNonAAQuads()) {
return CombineResult::kCannotCombine;
}
}
if (fSaturate != that->fSaturate) {
return CombineResult::kCannotCombine;
}