Enforce non-overlap constraint from GrRenderTargetOpList
Enforces this constraint from one central location, rather than
relying on each Op to remember to check if overlap is allowed from its
onCombineIfPossible method.
Fixes an issue where we need to check the total bounds of both chains
for overlap (not the bounds of individual Ops).
Bug: skia:8671
Change-Id: I163651c868847884459acfc00d13ffdfca3a27c3
Reviewed-on: https://skia-review.googlesource.com/c/185815
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrProcessorSet.cpp b/src/gpu/GrProcessorSet.cpp
index 156e33f..478d250 100644
--- a/src/gpu/GrProcessorSet.cpp
+++ b/src/gpu/GrProcessorSet.cpp
@@ -217,8 +217,8 @@
SkToBool(props & GrXPFactory::AnalysisProperties::kRequiresDstTexture);
analysis.fCompatibleWithCoverageAsAlpha &=
SkToBool(props & GrXPFactory::AnalysisProperties::kCompatibleWithAlphaAsCoverage);
- analysis.fRequiresBarrierBetweenOverlappingDraws = SkToBool(
- props & GrXPFactory::AnalysisProperties::kRequiresBarrierBetweenOverlappingDraws);
+ analysis.fRequiresNonOverlappingDraws = SkToBool(
+ props & GrXPFactory::AnalysisProperties::kRequiresNonOverlappingDraws);
if (props & GrXPFactory::AnalysisProperties::kIgnoresInputColor) {
colorFPsToEliminate = this->numColorFragmentProcessors();
analysis.fInputColorType =
@@ -242,5 +242,12 @@
fFlags |= kFinalized_Flag;
analysis.fIsInitialized = true;
+#ifdef SK_DEBUG
+ bool hasXferBarrier =
+ fXP.fProcessor &&
+ GrXferBarrierType::kNone_GrXferBarrierType != fXP.fProcessor->xferBarrierType(caps);
+ bool needsNonOverlappingDraws = analysis.fRequiresDstTexture || hasXferBarrier;
+ SkASSERT(analysis.fRequiresNonOverlappingDraws == needsNonOverlappingDraws);
+#endif
return analysis;
}
diff --git a/src/gpu/GrProcessorSet.h b/src/gpu/GrProcessorSet.h
index 241096b..f60392c 100644
--- a/src/gpu/GrProcessorSet.h
+++ b/src/gpu/GrProcessorSet.h
@@ -81,9 +81,7 @@
bool isInitialized() const { return fIsInitialized; }
bool usesLocalCoords() const { return fUsesLocalCoords; }
bool requiresDstTexture() const { return fRequiresDstTexture; }
- bool requiresBarrierBetweenOverlappingDraws() const {
- return fRequiresBarrierBetweenOverlappingDraws;
- }
+ bool requiresNonOverlappingDraws() const { return fRequiresNonOverlappingDraws; }
bool isCompatibleWithCoverageAsAlpha() const { return fCompatibleWithCoverageAsAlpha; }
bool inputColorIsIgnored() const { return fInputColorType == kIgnored_InputColorType; }
@@ -96,7 +94,7 @@
: fUsesLocalCoords(false)
, fCompatibleWithCoverageAsAlpha(true)
, fRequiresDstTexture(false)
- , fRequiresBarrierBetweenOverlappingDraws(false)
+ , fRequiresNonOverlappingDraws(false)
, fIsInitialized(true)
, fInputColorType(kOriginal_InputColorType) {}
enum InputColorType : uint32_t {
@@ -112,7 +110,7 @@
PackedBool fUsesLocalCoords : 1;
PackedBool fCompatibleWithCoverageAsAlpha : 1;
PackedBool fRequiresDstTexture : 1;
- PackedBool fRequiresBarrierBetweenOverlappingDraws : 1;
+ PackedBool fRequiresNonOverlappingDraws : 1;
PackedBool fIsInitialized : 1;
PackedInputColorType fInputColorType : 2;
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index 31b8349..a0728fe 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -1973,8 +1973,8 @@
}
GrXferProcessor::DstProxy dstProxy;
- GrProcessorSet::Analysis processorAnalysis = op->finalize(*this->caps(), &appliedClip);
- if (processorAnalysis.requiresDstTexture()) {
+ GrProcessorSet::Analysis analysis = op->finalize(*this->caps(), &appliedClip);
+ if (analysis.requiresDstTexture()) {
if (!this->setupDstProxy(this->asRenderTargetProxy(), clip, *op, &dstProxy)) {
fContext->contextPriv().opMemoryPool()->release(std::move(op));
return;
@@ -1986,7 +1986,7 @@
if (willAddFn) {
willAddFn(op.get(), opList->uniqueID());
}
- opList->addOp(std::move(op), *this->caps(), std::move(appliedClip), dstProxy);
+ opList->addDrawOp(std::move(op), analysis, std::move(appliedClip), dstProxy, *this->caps());
}
bool GrRenderTargetContext::setupDstProxy(GrRenderTargetProxy* rtProxy, const GrClip& clip,
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index 7823cdb..a511146 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -115,10 +115,14 @@
////////////////////////////////////////////////////////////////////////////////
-GrRenderTargetOpList::OpChain::OpChain(std::unique_ptr<GrOp> op, GrAppliedClip* appliedClip,
- const DstProxy* dstProxy)
- : fList{std::move(op)}, fAppliedClip(appliedClip) {
- if (dstProxy) {
+GrRenderTargetOpList::OpChain::OpChain(std::unique_ptr<GrOp> op,
+ GrProcessorSet::Analysis processorAnalysis,
+ GrAppliedClip* appliedClip, const DstProxy* dstProxy)
+ : fList{std::move(op)}
+ , fProcessorAnalysis(processorAnalysis)
+ , fAppliedClip(appliedClip) {
+ if (fProcessorAnalysis.requiresDstTexture()) {
+ SkASSERT(dstProxy && dstProxy->proxy());
fDstProxy = *dstProxy;
}
fBounds = fList.head()->bounds();
@@ -222,20 +226,29 @@
return chainA;
}
-// Attempts to concatenate two chains and merge ops across the chains. Upon failure the original
-// chain heads and tails are returned. Upon success the new chain's head and tail are returned
-// (and null for the second head/tail).
+// Attempts to concatenate the given chain onto our own and merge ops across the chains. Returns
+// whether the operation succeeded. On success, the provided list will be returned empty.
bool GrRenderTargetOpList::OpChain::tryConcat(
- List* list, const DstProxy& dstProxy, const GrAppliedClip* appliedClip,
- const SkRect& bounds, const GrCaps& caps, GrOpMemoryPool* pool, GrAuditTrail* auditTrail) {
+ List* list, GrProcessorSet::Analysis processorAnalysis, const DstProxy& dstProxy,
+ const GrAppliedClip* appliedClip, const SkRect& bounds, const GrCaps& caps,
+ GrOpMemoryPool* pool, GrAuditTrail* auditTrail) {
SkASSERT(!fList.empty());
SkASSERT(!list->empty());
+ SkASSERT(fProcessorAnalysis.requiresDstTexture() == SkToBool(fDstProxy.proxy()));
+ SkASSERT(processorAnalysis.requiresDstTexture() == SkToBool(dstProxy.proxy()));
// All returns use explicit tuple constructor rather than {a, b} to work around old GCC bug.
if (fList.head()->classID() != list->head()->classID() ||
SkToBool(fAppliedClip) != SkToBool(appliedClip) ||
(fAppliedClip && *fAppliedClip != *appliedClip) ||
- SkToBool(fDstProxy.proxy()) != SkToBool(dstProxy.proxy()) ||
- (fDstProxy.proxy() && fDstProxy != dstProxy)) {
+ (fProcessorAnalysis.requiresNonOverlappingDraws() !=
+ processorAnalysis.requiresNonOverlappingDraws()) ||
+ (fProcessorAnalysis.requiresNonOverlappingDraws() &&
+ // Non-overlaping draws are only required when Ganesh will either insert a barrier,
+ // or read back a new dst texture between draws. In either case, we can neither
+ // chain nor combine overlapping Ops.
+ GrRectsTouchOrOverlap(fBounds, bounds)) ||
+ (fProcessorAnalysis.requiresDstTexture() != processorAnalysis.requiresDstTexture()) ||
+ (fProcessorAnalysis.requiresDstTexture() && fDstProxy != dstProxy)) {
return false;
}
@@ -275,7 +288,8 @@
bool GrRenderTargetOpList::OpChain::prependChain(OpChain* that, const GrCaps& caps,
GrOpMemoryPool* pool, GrAuditTrail* auditTrail) {
- if (!that->tryConcat(&fList, fDstProxy, fAppliedClip, fBounds, caps, pool, auditTrail)) {
+ if (!that->tryConcat(
+ &fList, fProcessorAnalysis, fDstProxy, fAppliedClip, fBounds, caps, pool, auditTrail)) {
this->validate();
// append failed
return false;
@@ -296,12 +310,10 @@
return true;
}
-std::unique_ptr<GrOp> GrRenderTargetOpList::OpChain::appendOp(std::unique_ptr<GrOp> op,
- const DstProxy* dstProxy,
- const GrAppliedClip* appliedClip,
- const GrCaps& caps,
- GrOpMemoryPool* pool,
- GrAuditTrail* auditTrail) {
+std::unique_ptr<GrOp> GrRenderTargetOpList::OpChain::appendOp(
+ std::unique_ptr<GrOp> op, GrProcessorSet::Analysis processorAnalysis,
+ const DstProxy* dstProxy, const GrAppliedClip* appliedClip, const GrCaps& caps,
+ GrOpMemoryPool* pool, GrAuditTrail* auditTrail) {
const GrXferProcessor::DstProxy noDstProxy;
if (!dstProxy) {
dstProxy = &noDstProxy;
@@ -309,7 +321,8 @@
SkASSERT(op->isChainHead() && op->isChainTail());
SkRect opBounds = op->bounds();
List chain(std::move(op));
- if (!this->tryConcat(&chain, *dstProxy, appliedClip, opBounds, caps, pool, auditTrail)) {
+ if (!this->tryConcat(
+ &chain, processorAnalysis, *dstProxy, appliedClip, opBounds, caps, pool, auditTrail)) {
// append failed, give the op back to the caller.
this->validate();
return chain.popHead();
@@ -630,11 +643,11 @@
}
}
-void GrRenderTargetOpList::recordOp(std::unique_ptr<GrOp> op,
- const GrCaps& caps,
- GrAppliedClip* clip,
- const DstProxy* dstProxy) {
+void GrRenderTargetOpList::recordOp(
+ std::unique_ptr<GrOp> op, GrProcessorSet::Analysis processorAnalysis, GrAppliedClip* clip,
+ const DstProxy* dstProxy, const GrCaps& caps) {
SkDEBUGCODE(op->validate();)
+ SkASSERT(processorAnalysis.requiresDstTexture() == (dstProxy && dstProxy->proxy()));
SkASSERT(fTarget.get());
// A closed GrOpList should never receive new/more ops
@@ -663,8 +676,8 @@
int i = 0;
while (true) {
OpChain& candidate = fOpChains.fromBack(i);
- op = candidate.appendOp(std::move(op), dstProxy, clip, caps, fOpMemoryPool.get(),
- fAuditTrail);
+ op = candidate.appendOp(std::move(op), processorAnalysis, dstProxy, clip, caps,
+ fOpMemoryPool.get(), fAuditTrail);
if (!op) {
return;
}
@@ -686,7 +699,7 @@
clip = fClipAllocator.make<GrAppliedClip>(std::move(*clip));
SkDEBUGCODE(fNumClips++;)
}
- fOpChains.emplace_back(std::move(op), clip, dstProxy);
+ fOpChains.emplace_back(std::move(op), processorAnalysis, clip, dstProxy);
}
void GrRenderTargetOpList::forwardCombine(const GrCaps& caps) {
diff --git a/src/gpu/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h
index 2335440..7e44467 100644
--- a/src/gpu/GrRenderTargetOpList.h
+++ b/src/gpu/GrRenderTargetOpList.h
@@ -13,6 +13,7 @@
#include "GrPathRendering.h"
#include "GrPrimitiveProcessor.h"
#include "ops/GrOp.h"
+#include "ops/GrDrawOp.h"
#include "SkArenaAlloc.h"
#include "SkClipStack.h"
#include "SkMatrix.h"
@@ -68,11 +69,11 @@
op->visitProxies(addDependency);
- this->recordOp(std::move(op), caps);
+ this->recordOp(std::move(op), GrProcessorSet::EmptySetAnalysis(), nullptr, nullptr, caps);
}
- void addOp(std::unique_ptr<GrOp> op, const GrCaps& caps, GrAppliedClip&& clip,
- const DstProxy& dstProxy) {
+ void addDrawOp(std::unique_ptr<GrDrawOp> op, const GrProcessorSet::Analysis& processorAnalysis,
+ GrAppliedClip&& clip, const DstProxy& dstProxy, const GrCaps& caps) {
auto addDependency = [ &caps, this ] (GrSurfaceProxy* p) {
this->addDependency(p, caps);
};
@@ -83,7 +84,8 @@
addDependency(dstProxy.proxy());
}
- this->recordOp(std::move(op), caps, clip.doesClip() ? &clip : nullptr, &dstProxy);
+ this->recordOp(std::move(op), processorAnalysis, clip.doesClip() ? &clip : nullptr,
+ &dstProxy, caps);
}
void discard();
@@ -140,7 +142,7 @@
public:
OpChain(const OpChain&) = delete;
OpChain& operator=(const OpChain&) = delete;
- OpChain(std::unique_ptr<GrOp>, GrAppliedClip*, const DstProxy*);
+ OpChain(std::unique_ptr<GrOp>, GrProcessorSet::Analysis, GrAppliedClip*, const DstProxy*);
~OpChain() {
// The ops are stored in a GrMemoryPool and must be explicitly deleted via the pool.
@@ -166,9 +168,9 @@
// Attempts to add 'op' to this chain either by merging or adding to the tail. Returns
// 'op' to the caller upon failure, otherwise null. Fails when the op and chain aren't of
// the same op type, have different clips or dst proxies.
- std::unique_ptr<GrOp> appendOp(std::unique_ptr<GrOp> op, const DstProxy*,
- const GrAppliedClip*, const GrCaps&, GrOpMemoryPool*,
- GrAuditTrail*);
+ std::unique_ptr<GrOp> appendOp(std::unique_ptr<GrOp> op, GrProcessorSet::Analysis,
+ const DstProxy*, const GrAppliedClip*, const GrCaps&,
+ GrOpMemoryPool*, GrAuditTrail*);
private:
class List {
@@ -196,11 +198,12 @@
void validate() const;
- bool tryConcat(List*, const DstProxy&, const GrAppliedClip*, const SkRect& bounds,
- const GrCaps&, GrOpMemoryPool*, GrAuditTrail*);
+ bool tryConcat(List*, GrProcessorSet::Analysis, const DstProxy&, const GrAppliedClip*,
+ const SkRect& bounds, const GrCaps&, GrOpMemoryPool*, GrAuditTrail*);
static List DoConcat(List, List, const GrCaps&, GrOpMemoryPool*, GrAuditTrail*);
List fList;
+ GrProcessorSet::Analysis fProcessorAnalysis;
DstProxy fDstProxy;
GrAppliedClip* fAppliedClip;
SkRect fBounds;
@@ -210,8 +213,8 @@
void gatherProxyIntervals(GrResourceAllocator*) const override;
- void recordOp(std::unique_ptr<GrOp>, const GrCaps& caps, GrAppliedClip* = nullptr,
- const DstProxy* = nullptr);
+ void recordOp(std::unique_ptr<GrOp>, GrProcessorSet::Analysis, GrAppliedClip*, const DstProxy*,
+ const GrCaps& caps);
void forwardCombine(const GrCaps&);
diff --git a/src/gpu/GrXferProcessor.cpp b/src/gpu/GrXferProcessor.cpp
index e962165..fb2eeea 100644
--- a/src/gpu/GrXferProcessor.cpp
+++ b/src/gpu/GrXferProcessor.cpp
@@ -176,10 +176,8 @@
SkASSERT(!(result & AnalysisProperties::kRequiresDstTexture));
if ((result & AnalysisProperties::kReadsDstInShader) &&
!caps.shaderCaps()->dstReadInShaderSupport()) {
- result |= AnalysisProperties::kRequiresDstTexture;
- if (caps.textureBarrierSupport()) {
- result |= AnalysisProperties::kRequiresBarrierBetweenOverlappingDraws;
- }
+ result |= AnalysisProperties::kRequiresDstTexture |
+ AnalysisProperties::kRequiresNonOverlappingDraws;
}
return result;
}
diff --git a/src/gpu/GrXferProcessor.h b/src/gpu/GrXferProcessor.h
index bdd56f3..295bf00 100644
--- a/src/gpu/GrXferProcessor.h
+++ b/src/gpu/GrXferProcessor.h
@@ -275,10 +275,10 @@
*/
kRequiresDstTexture = 0x10,
/**
- * If set overlapping draws may not be combined because a barrier must be inserted between
- * them.
+ * If set, each pixel can only be touched once during a draw (e.g., because we have a dst
+ * texture or because we need an xfer barrier).
*/
- kRequiresBarrierBetweenOverlappingDraws = 0x20,
+ kRequiresNonOverlappingDraws = 0x20,
};
GR_DECL_BITFIELD_CLASS_OPS_FRIENDS(AnalysisProperties);
diff --git a/src/gpu/effects/GrCustomXfermode.cpp b/src/gpu/effects/GrCustomXfermode.cpp
index 1f0104a..86e7037 100644
--- a/src/gpu/effects/GrCustomXfermode.cpp
+++ b/src/gpu/effects/GrCustomXfermode.cpp
@@ -355,7 +355,7 @@
return AnalysisProperties::kCompatibleWithAlphaAsCoverage;
} else {
return AnalysisProperties::kCompatibleWithAlphaAsCoverage |
- AnalysisProperties::kRequiresBarrierBetweenOverlappingDraws;
+ AnalysisProperties::kRequiresNonOverlappingDraws;
}
}
return AnalysisProperties::kCompatibleWithAlphaAsCoverage |
diff --git a/src/gpu/ops/GrAtlasTextOp.cpp b/src/gpu/ops/GrAtlasTextOp.cpp
index 08afa4b..c55abce 100644
--- a/src/gpu/ops/GrAtlasTextOp.cpp
+++ b/src/gpu/ops/GrAtlasTextOp.cpp
@@ -165,9 +165,6 @@
}
auto analysis = fProcessors.finalize(color, coverage, clip, false, caps, &fGeoData[0].fColor);
fUsesLocalCoords = analysis.usesLocalCoords();
- fCanCombineOnTouchOrOverlap =
- !analysis.requiresDstTexture() &&
- !(fProcessors.xferProcessor() && fProcessors.xferProcessor()->xferBarrierType(caps));
return analysis;
}
@@ -446,10 +443,6 @@
return CombineResult::kCannotCombine;
}
- if (!fCanCombineOnTouchOrOverlap && GrRectsTouchOrOverlap(this->bounds(), that->bounds())) {
- return CombineResult::kCannotCombine;
- }
-
if (fMaskType != that->fMaskType) {
return CombineResult::kCannotCombine;
}
diff --git a/src/gpu/ops/GrAtlasTextOp.h b/src/gpu/ops/GrAtlasTextOp.h
index 41bd9dc..5faf5fa 100644
--- a/src/gpu/ops/GrAtlasTextOp.h
+++ b/src/gpu/ops/GrAtlasTextOp.h
@@ -161,7 +161,6 @@
GrProcessorSet fProcessors;
struct {
uint32_t fUsesLocalCoords : 1;
- uint32_t fCanCombineOnTouchOrOverlap : 1;
uint32_t fUseGammaCorrectDistanceTable : 1;
uint32_t fNeedsGlyphTransform : 1;
};
diff --git a/src/gpu/ops/GrDashOp.cpp b/src/gpu/ops/GrDashOp.cpp
index 89214dc..beed368 100644
--- a/src/gpu/ops/GrDashOp.cpp
+++ b/src/gpu/ops/GrDashOp.cpp
@@ -265,9 +265,6 @@
coverage = GrProcessorAnalysisCoverage::kSingleChannel;
}
auto analysis = fProcessorSet.finalize(fColor, coverage, clip, false, caps, &fColor);
- fDisallowCombineOnTouchOrOverlap = analysis.requiresDstTexture() ||
- (fProcessorSet.xferProcessor() &&
- fProcessorSet.xferProcessor()->xferBarrierType(caps));
fUsesLocalCoords = analysis.usesLocalCoords();
return analysis;
}
@@ -634,10 +631,6 @@
if (fProcessorSet != that->fProcessorSet) {
return CombineResult::kCannotCombine;
}
- if (fDisallowCombineOnTouchOrOverlap &&
- GrRectsTouchOrOverlap(this->bounds(), that->bounds())) {
- return CombineResult::kCannotCombine;
- }
if (this->aaMode() != that->aaMode()) {
return CombineResult::kCannotCombine;
@@ -675,7 +668,6 @@
SkSTArray<1, LineData, true> fLines;
SkPMColor4f fColor;
- bool fDisallowCombineOnTouchOrOverlap : 1;
bool fUsesLocalCoords : 1;
bool fFullDash : 1;
// We use 3 bits for this 3-value enum because MSVS makes the underlying types signed.
diff --git a/src/gpu/ops/GrSimpleMeshDrawOpHelper.cpp b/src/gpu/ops/GrSimpleMeshDrawOpHelper.cpp
index 446d313..8d9da02 100644
--- a/src/gpu/ops/GrSimpleMeshDrawOpHelper.cpp
+++ b/src/gpu/ops/GrSimpleMeshDrawOpHelper.cpp
@@ -16,7 +16,6 @@
: fProcessors(args.fProcessorSet)
, fPipelineFlags(0)
, fAAType((int)aaType)
- , fRequiresDstTexture(false)
, fUsesLocalCoords(false)
, fCompatibleWithAlphaAsCoveage(false) {
SkDEBUGCODE(fDidAnalysis = false);
@@ -55,12 +54,6 @@
if (*fProcessors != *that.fProcessors) {
return false;
}
- if (fRequiresDstTexture ||
- (fProcessors->xferProcessor() && fProcessors->xferProcessor()->xferBarrierType(caps))) {
- if (GrRectsTouchOrOverlap(thisBounds, thatBounds)) {
- return false;
- }
- }
}
bool result = fPipelineFlags == that.fPipelineFlags && (fAAType == that.fAAType ||
(noneAsCoverageAA && none_as_coverage_aa_compatible(this->aaType(), that.aaType())));
@@ -91,7 +84,6 @@
} else {
analysis = GrProcessorSet::EmptySetAnalysis();
}
- fRequiresDstTexture = analysis.requiresDstTexture();
fUsesLocalCoords = analysis.usesLocalCoords();
fCompatibleWithAlphaAsCoveage = analysis.isCompatibleWithCoverageAsAlpha();
return analysis;
diff --git a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
index 88f56e6..f9c6f41 100644
--- a/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
+++ b/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
@@ -137,7 +137,6 @@
GrProcessorSet* fProcessors;
unsigned fPipelineFlags : 8;
unsigned fAAType : 2;
- unsigned fRequiresDstTexture : 1;
unsigned fUsesLocalCoords : 1;
unsigned fCompatibleWithAlphaAsCoveage : 1;
SkDEBUGCODE(unsigned fMadePipeline : 1;)