Add tracking of unique proxyID beside unique renderTargetID in auditTrail and clearOp
Split out of: https://skia-review.googlesource.com/c/10284/ (Omnibus: Remove GrSurface-derived classes from ops)
Change-Id: I5845a47d94decc455ec3b1f0a5876b1c82aa32e8
Reviewed-on: https://skia-review.googlesource.com/10750
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
diff --git a/include/core/SkRect.h b/include/core/SkRect.h
index bcdc4f1..b8cf846 100644
--- a/include/core/SkRect.h
+++ b/include/core/SkRect.h
@@ -327,8 +327,7 @@
otherwise return false and do not change this rectangle.
If either rectangle is empty, do nothing and return false.
*/
- bool intersect(int32_t left, int32_t top,
- int32_t right, int32_t bottom) {
+ bool intersect(int32_t left, int32_t top, int32_t right, int32_t bottom) {
if (left < right && top < bottom && !this->isEmpty() &&
fLeft < right && left < fRight && fTop < bottom && top < fBottom) {
if (fLeft < left) fLeft = left;
diff --git a/include/private/GrAuditTrail.h b/include/private/GrAuditTrail.h
index f1ae494..9fdae30 100644
--- a/include/private/GrAuditTrail.h
+++ b/include/private/GrAuditTrail.h
@@ -10,6 +10,7 @@
#include "GrConfig.h"
#include "GrGpuResource.h"
+#include "GrRenderTargetProxy.h"
#include "SkRect.h"
#include "SkString.h"
#include "SkTArray.h"
@@ -80,7 +81,9 @@
fCurrentStackTrace.push_back(SkString(framename));
}
- void addOp(const GrOp*, GrGpuResource::UniqueID renderTargetID);
+ void addOp(const GrOp*,
+ GrGpuResource::UniqueID resourceID,
+ GrRenderTargetProxy::UniqueID proxyID);
void opsCombined(const GrOp* consumer, const GrOp* consumed);
@@ -103,14 +106,23 @@
// We could just return our internal bookkeeping struct if copying the data out becomes
// a performance issue, but until then its nice to decouple
struct OpInfo {
- SkRect fBounds;
- // TODO: switch over to GrSurfaceProxy::UniqueID
- GrGpuResource::UniqueID fRenderTargetUniqueID;
+ // Will the resourceID comparison yield the same decision as the proxyID comparison?
+ bool sameDecision(GrGpuResource::UniqueID resourceUniqueID,
+ GrSurfaceProxy::UniqueID proxyUniqueID) const {
+ return (fResourceUniqueID == resourceUniqueID) ==
+ (fProxyUniqueID == proxyUniqueID);
+ }
+
struct Op {
- int fClientID;
+ int fClientID;
SkRect fBounds;
};
- SkTArray<Op> fOps;
+
+ SkRect fBounds;
+ // MDB TODO: remove fResourceUniqueID
+ GrGpuResource::UniqueID fResourceUniqueID;
+ GrSurfaceProxy::UniqueID fProxyUniqueID;
+ SkTArray<Op> fOps;
};
void getBoundsByClientID(SkTArray<OpInfo>* outInfo, int clientID);
@@ -136,11 +148,16 @@
typedef SkTArray<Op*> Ops;
struct OpNode {
- OpNode(const GrGpuResource::UniqueID& id) : fRenderTargetUniqueID(id) {}
+ OpNode(const GrGpuResource::UniqueID& resourceID, const GrSurfaceProxy::UniqueID& proxyID)
+ : fResourceUniqueID(resourceID)
+ , fProxyUniqueID(proxyID) {
+ }
SkString toJson() const;
+
SkRect fBounds;
- Ops fChildren;
- const GrGpuResource::UniqueID fRenderTargetUniqueID;
+ Ops fChildren;
+ const GrGpuResource::UniqueID fResourceUniqueID;
+ const GrSurfaceProxy::UniqueID fProxyUniqueID;
};
typedef SkTArray<std::unique_ptr<OpNode>, true> OpList;
@@ -172,8 +189,8 @@
#define GR_AUDIT_TRAIL_RESET(audit_trail) \
//GR_AUDIT_TRAIL_INVOKE_GUARD(audit_trail, fullReset);
-#define GR_AUDIT_TRAIL_ADD_OP(audit_trail, op, rt_id) \
- GR_AUDIT_TRAIL_INVOKE_GUARD(audit_trail, addOp, op, rt_id);
+#define GR_AUDIT_TRAIL_ADD_OP(audit_trail, op, resource_id, proxy_id) \
+ GR_AUDIT_TRAIL_INVOKE_GUARD(audit_trail, addOp, op, resource_id, proxy_id);
#define GR_AUDIT_TRAIL_OPS_RESULT_COMBINED(audit_trail, combineWith, op) \
GR_AUDIT_TRAIL_INVOKE_GUARD(audit_trail, opsCombined, combineWith, op);
diff --git a/src/gpu/GrAuditTrail.cpp b/src/gpu/GrAuditTrail.cpp
index 133ffea..e6b5c3a 100644
--- a/src/gpu/GrAuditTrail.cpp
+++ b/src/gpu/GrAuditTrail.cpp
@@ -10,7 +10,9 @@
const int GrAuditTrail::kGrAuditTrailInvalidID = -1;
-void GrAuditTrail::addOp(const GrOp* op, GrGpuResource::UniqueID renderTargetID) {
+void GrAuditTrail::addOp(const GrOp* op,
+ GrGpuResource::UniqueID resourceID,
+ GrRenderTargetProxy::UniqueID proxyID) {
SkASSERT(fEnabled);
Op* auditOp = new Op;
fOpPool.emplace_back(auditOp);
@@ -44,7 +46,7 @@
// We use the op pointer as a key to find the OpNode we are 'glomming' ops onto
fIDLookup.set(op->uniqueID(), auditOp->fOpListID);
- OpNode* opNode = new OpNode(renderTargetID);
+ OpNode* opNode = new OpNode(resourceID, proxyID);
opNode->fBounds = op->bounds();
opNode->fChildren.push_back(auditOp);
fOpList.emplace_back(opNode);
@@ -89,7 +91,8 @@
const OpNode* bn = fOpList[opListID].get();
SkASSERT(bn);
outOpInfo->fBounds = bn->fBounds;
- outOpInfo->fRenderTargetUniqueID = bn->fRenderTargetUniqueID;
+ outOpInfo->fResourceUniqueID = bn->fResourceUniqueID;
+ outOpInfo->fProxyUniqueID = bn->fProxyUniqueID;
for (int j = 0; j < bn->fChildren.count(); j++) {
OpInfo::Op& outOp = outOpInfo->fOps.push_back();
const Op* currentOp = bn->fChildren[j];
@@ -286,7 +289,8 @@
SkString GrAuditTrail::OpNode::toJson() const {
SkString json;
json.append("{");
- json.appendf("\"RenderTarget\": \"%u\",", fRenderTargetUniqueID.asUInt());
+ json.appendf("\"ResourceID\": \"%u\",", fResourceUniqueID.asUInt());
+ json.appendf("\"ProxyID\": \"%u\",", fProxyUniqueID.asUInt());
skrect_to_json(&json, "Bounds", fBounds);
JsonifyTArray(&json, "Ops", fChildren, true);
json.append("}");
diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp
index acc0f67..3061074 100644
--- a/src/gpu/GrRenderTargetContext.cpp
+++ b/src/gpu/GrRenderTargetContext.cpp
@@ -315,15 +315,10 @@
GrAAType::kNone);
} else {
- if (!fRenderTargetContext->accessRenderTarget()) {
- return;
- }
-
// This path doesn't handle coalescing of full screen clears b.c. it
// has to clear the entire render target - not just the content area.
// It could be done but will take more finagling.
- std::unique_ptr<GrOp> op(GrClearOp::Make(
- rtRect, color, fRenderTargetContext->accessRenderTarget(), !clearRect));
+ std::unique_ptr<GrOp> op(GrClearOp::Make(rtRect, color, fRenderTargetContext, !clearRect));
if (!op) {
return;
}
@@ -371,14 +366,9 @@
this->drawRect(clip, std::move(paint), GrAA::kNo, SkMatrix::I(), SkRect::Make(clearRect));
} else if (isFull) {
- if (this->accessRenderTarget()) {
- this->getOpList()->fullClear(this, color);
- }
+ this->getOpList()->fullClear(this, color);
} else {
- if (!this->accessRenderTarget()) {
- return;
- }
- std::unique_ptr<GrOp> op(GrClearOp::Make(clip, color, this->accessRenderTarget()));
+ std::unique_ptr<GrOp> op(GrClearOp::Make(clip, color, this));
if (!op) {
return;
}
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index d1aca0c..bbe1055 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -169,7 +169,8 @@
void GrRenderTargetOpList::reset() {
fLastFullClearOp = nullptr;
- fLastFullClearRenderTargetID.makeInvalid();
+ fLastFullClearResourceID.makeInvalid();
+ fLastFullClearProxyID.makeInvalid();
fRecordedOps.reset();
if (fInstancedRendering) {
fInstancedRendering->endFlush();
@@ -191,23 +192,36 @@
}
void GrRenderTargetOpList::fullClear(GrRenderTargetContext* renderTargetContext, GrColor color) {
+ // MDB TODO: remove this. Right now we need the renderTargetContext for the
+ // accessRenderTarget call. This method should just take the renderTargetProxy.
GrRenderTarget* renderTarget = renderTargetContext->accessRenderTarget();
+ if (!renderTarget) {
+ return;
+ }
+
// Currently this just inserts or updates the last clear op. However, once in MDB this can
// remove all the previously recorded ops and change the load op to clear with supplied
// color.
// TODO: this needs to be updated to use GrSurfaceProxy::UniqueID
- if (fLastFullClearRenderTargetID == renderTarget->uniqueID()) {
+ SkASSERT((fLastFullClearResourceID == renderTarget->uniqueID()) ==
+ (fLastFullClearProxyID == renderTargetContext->asRenderTargetProxy()->uniqueID()));
+ if (fLastFullClearResourceID == renderTarget->uniqueID()) {
// As currently implemented, fLastFullClearOp should be the last op because we would
// have cleared it when another op was recorded.
SkASSERT(fRecordedOps.back().fOp.get() == fLastFullClearOp);
fLastFullClearOp->setColor(color);
return;
}
- std::unique_ptr<GrClearOp> op(GrClearOp::Make(GrFixedClip::Disabled(), color, renderTarget));
+ std::unique_ptr<GrClearOp> op(GrClearOp::Make(GrFixedClip::Disabled(), color,
+ renderTargetContext));
+ if (!op) {
+ return;
+ }
if (GrOp* clearOp = this->recordOp(std::move(op), renderTargetContext)) {
// This is either the clear op we just created or another one that it combined with.
fLastFullClearOp = static_cast<GrClearOp*>(clearOp);
- fLastFullClearRenderTargetID = renderTarget->uniqueID();
+ fLastFullClearResourceID = renderTarget->uniqueID();
+ fLastFullClearProxyID = renderTargetContext->asRenderTargetProxy()->uniqueID();
}
}
@@ -284,7 +298,8 @@
// 1) check every op
// 2) intersect with something
// 3) find a 'blocker'
- GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), renderTarget->uniqueID());
+ GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), renderTarget->uniqueID(),
+ renderTargetContext->asRenderTargetProxy()->uniqueID());
GrOP_INFO("Recording (%s, B%u)\n"
"\tBounds LRTB (%f, %f, %f, %f)\n",
op->name(),
@@ -337,7 +352,8 @@
fRecordedOps.emplace_back(std::move(op), renderTarget, clip, dstTexture);
fRecordedOps.back().fOp->wasRecorded();
fLastFullClearOp = nullptr;
- fLastFullClearRenderTargetID.makeInvalid();
+ fLastFullClearResourceID.makeInvalid();
+ fLastFullClearProxyID.makeInvalid();
return fRecordedOps.back().fOp.get();
}
diff --git a/src/gpu/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h
index c44c341..b527b64 100644
--- a/src/gpu/GrRenderTargetOpList.h
+++ b/src/gpu/GrRenderTargetOpList.h
@@ -147,7 +147,8 @@
const DstTexture* bDstTexture);
GrClearOp* fLastFullClearOp = nullptr;
- GrGpuResource::UniqueID fLastFullClearRenderTargetID = GrGpuResource::UniqueID::InvalidID();
+ GrGpuResource::UniqueID fLastFullClearResourceID = GrGpuResource::UniqueID::InvalidID();
+ GrSurfaceProxy::UniqueID fLastFullClearProxyID = GrSurfaceProxy::UniqueID::InvalidID();
GrGpu* fGpu;
GrResourceProvider* fResourceProvider;
diff --git a/src/gpu/GrResourceProvider.cpp b/src/gpu/GrResourceProvider.cpp
index 407fffb..42f5e29 100644
--- a/src/gpu/GrResourceProvider.cpp
+++ b/src/gpu/GrResourceProvider.cpp
@@ -43,7 +43,7 @@
fQuadIndexBufferKey = gQuadIndexBufferKey;
}
-bool GrResourceProvider::IsFunctionallyExact(GrTextureProxy* proxy) {
+bool GrResourceProvider::IsFunctionallyExact(GrSurfaceProxy* proxy) {
return proxy->priv().isExact() || (SkIsPow2(proxy->width()) && SkIsPow2(proxy->height()));
}
diff --git a/src/gpu/GrResourceProvider.h b/src/gpu/GrResourceProvider.h
index 6b3633c..a4abd72 100644
--- a/src/gpu/GrResourceProvider.h
+++ b/src/gpu/GrResourceProvider.h
@@ -242,9 +242,9 @@
fGpu = nullptr;
}
- // 'Proxy' is about to be used as a texture src. This query can be used to determine if
- // it is going to need a texture domain.
- static bool IsFunctionallyExact(GrTextureProxy* proxy);
+ // 'proxy' is about to be used as a texture src or drawn to. This query can be used to
+ // determine if it is going to need a texture domain or a full clear.
+ static bool IsFunctionallyExact(GrSurfaceProxy* proxy);
const GrCaps* caps() const { return fCaps.get(); }
diff --git a/src/gpu/GrTextureOpList.cpp b/src/gpu/GrTextureOpList.cpp
index 2a021d2..668e8a3 100644
--- a/src/gpu/GrTextureOpList.cpp
+++ b/src/gpu/GrTextureOpList.cpp
@@ -89,15 +89,19 @@
#endif
// See the comment in GrRenderTargetOpList about why we pass the invalid ID here.
- this->recordOp(std::move(op), GrGpuResource::UniqueID::InvalidID());
+ this->recordOp(std::move(op),
+ GrGpuResource::UniqueID::InvalidID(),
+ GrSurfaceProxy::UniqueID::InvalidID());
return true;
}
-void GrTextureOpList::recordOp(std::unique_ptr<GrOp> op, GrGpuResource::UniqueID renderTargetID) {
+void GrTextureOpList::recordOp(std::unique_ptr<GrOp> op,
+ GrGpuResource::UniqueID resourceUniqueID,
+ GrSurfaceProxy::UniqueID proxyUniqueID) {
// A closed GrOpList should never receive new/more ops
SkASSERT(!this->isClosed());
- GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), renderTargetID);
+ GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), resourceUniqueID, proxyUniqueID);
GrOP_INFO("Re-Recording (%s, B%u)\n"
"\tBounds LRTB (%f, %f, %f, %f)\n",
op->name(),
diff --git a/src/gpu/GrTextureOpList.h b/src/gpu/GrTextureOpList.h
index 325ab5d..954289c 100644
--- a/src/gpu/GrTextureOpList.h
+++ b/src/gpu/GrTextureOpList.h
@@ -10,6 +10,7 @@
#include "GrGpuResource.h"
#include "GrOpList.h"
+#include "GrSurfaceProxy.h"
#include "SkTArray.h"
@@ -61,8 +62,11 @@
SkDEBUGCODE(void dump() const override;)
private:
- // The unique ID is only needed for the audit trail. This should be removed with MDB.
- void recordOp(std::unique_ptr<GrOp>, GrGpuResource::UniqueID renderTargetID);
+ // MDB TODO: The unique IDs are only needed for the audit trail. There should only be one
+ // on the opList itself.
+ void recordOp(std::unique_ptr<GrOp>,
+ GrGpuResource::UniqueID resourceUniqueID,
+ GrSurfaceProxy::UniqueID proxyUniqueID);
SkSTArray<2, std::unique_ptr<GrOp>, true> fRecordedOps;
GrGpu* fGpu;
diff --git a/src/gpu/ops/GrClearOp.h b/src/gpu/ops/GrClearOp.h
index 455c684..1314d47 100644
--- a/src/gpu/ops/GrClearOp.h
+++ b/src/gpu/ops/GrClearOp.h
@@ -19,18 +19,37 @@
public:
DEFINE_OP_CLASS_ID
+ // MDB TODO: replace the renderTargetContext with just the renderTargetProxy.
+ // For now, we need the renderTargetContext for its accessRenderTarget powers.
static std::unique_ptr<GrClearOp> Make(const GrFixedClip& clip, GrColor color,
- GrRenderTarget* rt) {
- std::unique_ptr<GrClearOp> op(new GrClearOp(clip, color, rt));
- if (!op->fRenderTarget) {
- return nullptr; // The clip did not contain any pixels within the render target.
+ GrRenderTargetContext* rtc) {
+ const SkIRect rtRect = SkIRect::MakeWH(rtc->width(), rtc->height());
+ if (clip.scissorEnabled() && !SkIRect::Intersects(clip.scissorRect(), rtRect)) {
+ return nullptr;
}
- return op;
+
+ // MDB TODO: remove this. In this hybrid state we need to be sure the RT is instantiable
+ // so it can carry the IO refs. In the future we will just get the proxy and
+ // it carry the IO refs.
+ if (!rtc->accessRenderTarget()) {
+ return nullptr;
+ }
+
+ return std::unique_ptr<GrClearOp>(new GrClearOp(clip, color, rtc));
}
- static std::unique_ptr<GrClearOp> Make(const SkIRect& rect, GrColor color, GrRenderTarget* rt,
+ // MDB TODO: replace the renderTargetContext with just the renderTargetProxy.
+ static std::unique_ptr<GrClearOp> Make(const SkIRect& rect, GrColor color,
+ GrRenderTargetContext* rtc,
bool fullScreen) {
- return std::unique_ptr<GrClearOp>(new GrClearOp(rect, color, rt, fullScreen));
+ SkASSERT(fullScreen || !rect.isEmpty());
+
+ // MDB TODO: remove this. See above comment.
+ if (!rtc->accessRenderTarget()) {
+ return nullptr;
+ }
+
+ return std::unique_ptr<GrClearOp>(new GrClearOp(rect, color, rtc, fullScreen));
}
const char* name() const override { return "Clear"; }
@@ -40,9 +59,10 @@
if (fClip.scissorEnabled()) {
const SkIRect& r = fClip.scissorRect();
string.appendf("L: %d, T: %d, R: %d, B: %d", r.fLeft, r.fTop, r.fRight, r.fBottom);
+ } else {
+ string.append("disabled");
}
- string.appendf("], Color: 0x%08x, RT: %d", fColor,
- fRenderTarget.get()->uniqueID().asUInt());
+ string.appendf("], Color: 0x%08x, RT: %d", fColor, fProxyUniqueID.asUInt());
string.append(INHERITED::dumpInfo());
return string;
}
@@ -50,34 +70,41 @@
void setColor(GrColor color) { fColor = color; }
private:
- GrClearOp(const GrFixedClip& clip, GrColor color, GrRenderTarget* rt)
+ GrClearOp(const GrFixedClip& clip, GrColor color, GrRenderTargetContext* rtc)
: INHERITED(ClassID())
, fClip(clip)
- , fColor(color) {
- SkIRect rtRect = SkIRect::MakeWH(rt->width(), rt->height());
+ , fColor(color)
+ , fProxyUniqueID(rtc->asSurfaceProxy()->uniqueID()) {
+
+ GrSurfaceProxy* proxy = rtc->asSurfaceProxy();
+ const SkIRect rtRect = SkIRect::MakeWH(proxy->width(), proxy->height());
if (fClip.scissorEnabled()) {
// Don't let scissors extend outside the RT. This may improve op combining.
if (!fClip.intersect(rtRect)) {
- return;
+ SkASSERT(0); // should be caught upstream
+ fClip = GrFixedClip(SkIRect::MakeEmpty());
}
- if (fClip.scissorRect() == rtRect) {
+
+ if (GrResourceProvider::IsFunctionallyExact(proxy) && fClip.scissorRect() == rtRect) {
fClip.disableScissor();
}
}
this->setBounds(SkRect::Make(fClip.scissorEnabled() ? fClip.scissorRect() : rtRect),
HasAABloat::kNo, IsZeroArea::kNo);
- fRenderTarget.reset(rt);
+ fRenderTarget.reset(rtc->accessRenderTarget());
}
- GrClearOp(const SkIRect& rect, GrColor color, GrRenderTarget* rt, bool fullScreen)
+ GrClearOp(const SkIRect& rect, GrColor color, GrRenderTargetContext* rtc, bool fullScreen)
: INHERITED(ClassID())
, fClip(GrFixedClip(rect))
, fColor(color)
- , fRenderTarget(rt) {
+ , fProxyUniqueID(rtc->asSurfaceProxy()->uniqueID()) {
+
if (fullScreen) {
fClip.disableScissor();
}
this->setBounds(SkRect::Make(rect), HasAABloat::kNo, IsZeroArea::kNo);
+ fRenderTarget.reset(rtc->accessRenderTarget());
}
bool onCombineIfPossible(GrOp* t, const GrCaps& caps) override {
@@ -86,6 +113,7 @@
// same color.
GrClearOp* cb = t->cast<GrClearOp>();
SkASSERT(cb->fRenderTarget == fRenderTarget);
+ SkASSERT(cb->fProxyUniqueID == fProxyUniqueID);
if (fClip.windowRectsState() != cb->fClip.windowRectsState()) {
return false;
}
@@ -110,11 +138,15 @@
void onPrepare(GrOpFlushState*) override {}
void onExecute(GrOpFlushState* state) override {
+ // MDB TODO: instantiate the renderTarget from the proxy in here
state->commandBuffer()->clear(fRenderTarget.get(), fClip, fColor);
}
GrFixedClip fClip;
GrColor fColor;
+
+ // MDB TODO: remove this. When the renderTargetProxy carries the refs this will be redundant.
+ GrSurfaceProxy::UniqueID fProxyUniqueID;
GrPendingIOResource<GrRenderTarget, kWrite_GrIOType> fRenderTarget;
typedef GrOp INHERITED;
diff --git a/tools/debugger/SkDebugCanvas.cpp b/tools/debugger/SkDebugCanvas.cpp
index 6349810..2d7d040 100644
--- a/tools/debugger/SkDebugCanvas.cpp
+++ b/tools/debugger/SkDebugCanvas.cpp
@@ -362,7 +362,8 @@
paint.setStyle(SkPaint::kStroke_Style);
paint.setStrokeWidth(1);
for (int i = 0; i < childrenBounds.count(); i++) {
- if (childrenBounds[i].fRenderTargetUniqueID != rtID) {
+ SkASSERT(childrenBounds[i].sameDecision(rtID, rtc->asSurfaceProxy()->uniqueID()));
+ if (childrenBounds[i].fResourceUniqueID != rtID) {
// offscreen draw, ignore for now
continue;
}