From 7f1ce29c9bb9be8b2d8dbf9a99f14f74d5dc6d80 Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Thu, 18 May 2017 14:38:55 -0400 Subject: [PATCH] Update clearOp for split-OpList world It would reduce a lot of noise if the GrRenderTargetOpList kept a pointer to the GrCaps but, for now, I'm trying to shrink the GrRTOpList, not expand it. Change-Id: Ieed56fa2a41a3fb20234e26552ae2d301147e4f2 Reviewed-on: https://skia-review.googlesource.com/17323 Reviewed-by: Brian Salomon Commit-Queue: Robert Phillips --- src/gpu/GrRenderTargetContext.cpp | 21 ++++++++------- src/gpu/GrRenderTargetOpList.cpp | 40 +++++++-------------------- src/gpu/GrRenderTargetOpList.h | 27 +++++++++---------- src/gpu/ops/GrClearOp.h | 57 +++++++++------------------------------ 4 files changed, 46 insertions(+), 99 deletions(-) diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp index d144389..1fcb0aa 100644 --- a/src/gpu/GrRenderTargetContext.cpp +++ b/src/gpu/GrRenderTargetContext.cpp @@ -198,7 +198,7 @@ void GrRenderTargetContext::discard() { if (!op) { return; } - this->getOpList()->addOp(std::move(op), this); + this->getOpList()->addOp(std::move(op), *this->caps()); } } @@ -257,11 +257,11 @@ void GrRenderTargetContextPriv::absClear(const SkIRect* clearRect, const GrColor // 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 op(GrClearOp::Make(rtRect, color, fRenderTargetContext, !clearRect)); + std::unique_ptr op(GrClearOp::Make(rtRect, color, !clearRect)); if (!op) { return; } - fRenderTargetContext->getOpList()->addOp(std::move(op), fRenderTargetContext); + fRenderTargetContext->getOpList()->addOp(std::move(op), *fRenderTargetContext->caps()); } } @@ -305,13 +305,13 @@ void GrRenderTargetContext::internalClear(const GrFixedClip& clip, this->drawRect(clip, std::move(paint), GrAA::kNo, SkMatrix::I(), SkRect::Make(clearRect)); } else if (isFull) { - this->getOpList()->fullClear(this, color); + this->getOpList()->fullClear(*this->caps(), color); } else { - std::unique_ptr op(GrClearOp::Make(clip, color, this)); + std::unique_ptr op(GrClearOp::Make(clip, color, fRenderTargetProxy.get())); if (!op) { return; } - this->getOpList()->addOp(std::move(op), this); + this->getOpList()->addOp(std::move(op), *this->caps()); } } @@ -608,7 +608,7 @@ void GrRenderTargetContextPriv::clearStencilClip(const GrFixedClip& clip, bool i if (!op) { return; } - fRenderTargetContext->getOpList()->addOp(std::move(op), fRenderTargetContext); + fRenderTargetContext->getOpList()->addOp(std::move(op), *fRenderTargetContext->caps()); } void GrRenderTargetContextPriv::stencilPath(const GrClip& clip, @@ -666,7 +666,7 @@ void GrRenderTargetContextPriv::stencilPath(const GrClip& clip, return; } op->setClippedBounds(bounds); - fRenderTargetContext->getOpList()->addOp(std::move(op), fRenderTargetContext); + fRenderTargetContext->getOpList()->addOp(std::move(op), *fRenderTargetContext->caps()); } void GrRenderTargetContextPriv::stencilRect(const GrClip& clip, @@ -1601,7 +1601,8 @@ uint32_t GrRenderTargetContext::addDrawOp(const GrClip& clip, std::unique_ptrsetClippedBounds(bounds); - return this->getOpList()->addOp(std::move(op), this, std::move(appliedClip), dstTexture); + return this->getOpList()->addOp(std::move(op), *this->caps(), + std::move(appliedClip), dstTexture); } uint32_t GrRenderTargetContext::addLegacyMeshDrawOp(GrPipelineBuilder&& pipelineBuilder, @@ -1663,7 +1664,7 @@ uint32_t GrRenderTargetContext::addLegacyMeshDrawOp(GrPipelineBuilder&& pipeline op->addDependenciesTo(fRenderTargetProxy.get()); op->setClippedBounds(bounds); - return this->getOpList()->addOp(std::move(op), this); + return this->getOpList()->addOp(std::move(op), *this->caps()); } bool GrRenderTargetContext::setupDstTexture(GrRenderTargetProxy* rtProxy, const GrClip& clip, diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp index 537f9e6..cc7dc8f 100644 --- a/src/gpu/GrRenderTargetOpList.cpp +++ b/src/gpu/GrRenderTargetOpList.cpp @@ -158,8 +158,6 @@ bool GrRenderTargetOpList::executeOps(GrOpFlushState* flushState) { void GrRenderTargetOpList::reset() { fLastFullClearOp = nullptr; - fLastFullClearResourceID.makeInvalid(); - fLastFullClearProxyID.makeInvalid(); fLastClipStackGenID = SK_InvalidUniqueID; fRecordedOps.reset(); if (fInstancedRendering) { @@ -182,24 +180,11 @@ void GrRenderTargetOpList::freeGpuResources() { } } -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; - } - +void GrRenderTargetOpList::fullClear(const GrCaps& caps, GrColor color) { // 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 - // MDB TODO: re-enable once opLists are divided. This assertion fails when a rendering is - // aborted but the same RT is reused for the next draw. The clears really shouldn't be - // fused in that case. - //SkASSERT((fLastFullClearResourceID == renderTarget->uniqueID()) == - // (fLastFullClearProxyID == renderTargetContext->asRenderTargetProxy()->uniqueID())); - if (fLastFullClearResourceID == renderTarget->uniqueID()) { + if (fLastFullClearOp) { // 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); @@ -210,16 +195,13 @@ void GrRenderTargetOpList::fullClear(GrRenderTargetContext* renderTargetContext, fLastFullClearOp->setColor(color); return; } - std::unique_ptr op(GrClearOp::Make(GrFixedClip::Disabled(), color, - renderTargetContext)); + std::unique_ptr op(GrClearOp::Make(GrFixedClip::Disabled(), color, fTarget.get())); if (!op) { return; } - if (GrOp* clearOp = this->recordOp(std::move(op), renderTargetContext)) { + if (GrOp* clearOp = this->recordOp(std::move(op), caps)) { // This is either the clear op we just created or another one that it combined with. fLastFullClearOp = static_cast(clearOp); - fLastFullClearResourceID = renderTarget->uniqueID(); - fLastFullClearProxyID = renderTargetContext->asRenderTargetProxy()->uniqueID(); } } @@ -231,6 +213,8 @@ bool GrRenderTargetOpList::copySurface(GrResourceProvider* resourceProvider, GrSurfaceProxy* src, const SkIRect& srcRect, const SkIPoint& dstPoint) { + SkASSERT(dst->asRenderTargetProxy() == fTarget.get()); + std::unique_ptr op = GrCopySurfaceOp::Make(resourceProvider, dst->asSurfaceProxy(), src, srcRect, dstPoint); if (!op) { @@ -240,7 +224,7 @@ bool GrRenderTargetOpList::copySurface(GrResourceProvider* resourceProvider, this->addDependency(src); #endif - this->recordOp(std::move(op), dst); + this->recordOp(std::move(op), *resourceProvider->caps()); return true; } @@ -271,11 +255,10 @@ bool GrRenderTargetOpList::combineIfPossible(const RecordedOp& a, GrOp* b, } GrOp* GrRenderTargetOpList::recordOp(std::unique_ptr op, - GrRenderTargetContext* renderTargetContext, + const GrCaps& caps, GrAppliedClip* clip, const DstTexture* dstTexture) { SkASSERT(fTarget.get()); - const GrCaps* caps = renderTargetContext->caps(); // A closed GrOpList should never receive new/more ops SkASSERT(!this->isClosed()); @@ -284,8 +267,7 @@ GrOp* GrRenderTargetOpList::recordOp(std::unique_ptr op, // 1) check every op // 2) intersect with something // 3) find a 'blocker' - GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), - renderTargetContext->asRenderTargetProxy()->uniqueID()); + GR_AUDIT_TRAIL_ADD_OP(fAuditTrail, op.get(), fTarget.get()->uniqueID()); GrOP_INFO("opList: %d Recording (%s, opID: %u)\n" "\tBounds [L: %.2f, T: %.2f R: %.2f B: %.2f]\n", this->uniqueID(), @@ -302,7 +284,7 @@ GrOp* GrRenderTargetOpList::recordOp(std::unique_ptr op, while (true) { const RecordedOp& candidate = fRecordedOps.fromBack(i); - if (this->combineIfPossible(candidate, op.get(), clip, dstTexture, *caps)) { + if (this->combineIfPossible(candidate, op.get(), clip, dstTexture, caps)) { GrOP_INFO("\t\tBackward: Combining with (%s, opID: %u)\n", candidate.fOp->name(), candidate.fOp->uniqueID()); GrOP_INFO("\t\t\tBackward: Combined op info:\n"); @@ -333,8 +315,6 @@ GrOp* GrRenderTargetOpList::recordOp(std::unique_ptr op, fRecordedOps.emplace_back(std::move(op), clip, dstTexture); fRecordedOps.back().fOp->wasRecorded(this); fLastFullClearOp = nullptr; - fLastFullClearResourceID.makeInvalid(); - fLastFullClearProxyID.makeInvalid(); return fRecordedOps.back().fOp.get(); } diff --git a/src/gpu/GrRenderTargetOpList.h b/src/gpu/GrRenderTargetOpList.h index 7f40782..e985069 100644 --- a/src/gpu/GrRenderTargetOpList.h +++ b/src/gpu/GrRenderTargetOpList.h @@ -65,19 +65,18 @@ public: void prepareOps(GrOpFlushState* flushState) override; bool executeOps(GrOpFlushState* flushState) override; - uint32_t addOp(std::unique_ptr op, GrRenderTargetContext* renderTargetContext) { - this->recordOp(std::move(op), renderTargetContext, nullptr, nullptr); + uint32_t addOp(std::unique_ptr op, const GrCaps& caps) { + this->recordOp(std::move(op), caps, nullptr, nullptr); return this->uniqueID(); } - uint32_t addOp(std::unique_ptr op, GrRenderTargetContext* renderTargetContext, + uint32_t addOp(std::unique_ptr op, const GrCaps& caps, GrAppliedClip&& clip, const DstTexture& dstTexture) { - this->recordOp(std::move(op), renderTargetContext, clip.doesClip() ? &clip : nullptr, - &dstTexture); + this->recordOp(std::move(op), caps, clip.doesClip() ? &clip : nullptr, &dstTexture); return this->uniqueID(); } /** Clears the entire render target */ - void fullClear(GrRenderTargetContext*, GrColor color); + void fullClear(const GrCaps& caps, GrColor color); /** * Copies a pixel rectangle from one surface to another. This call may finalize @@ -127,8 +126,8 @@ private: // If the input op is combined with an earlier op, this returns the combined op. Otherwise, it // returns the input op. - GrOp* recordOp(std::unique_ptr, GrRenderTargetContext*, GrAppliedClip* = nullptr, - const DstTexture* = nullptr); + GrOp* recordOp(std::unique_ptr, const GrCaps& caps, + GrAppliedClip* = nullptr, const DstTexture* = nullptr); void forwardCombine(const GrCaps&); @@ -136,22 +135,20 @@ private: bool combineIfPossible(const RecordedOp& a, GrOp* b, const GrAppliedClip* bClip, const DstTexture* bDstTexture, const GrCaps&); - GrClearOp* fLastFullClearOp = nullptr; - GrGpuResource::UniqueID fLastFullClearResourceID = GrGpuResource::UniqueID::InvalidID(); - GrSurfaceProxy::UniqueID fLastFullClearProxyID = GrSurfaceProxy::UniqueID::InvalidID(); + GrClearOp* fLastFullClearOp = nullptr; std::unique_ptr fInstancedRendering; - int32_t fLastClipStackGenID; - SkIRect fLastDevClipBounds; + int32_t fLastClipStackGenID; + SkIRect fLastDevClipBounds; // For ops/opList we have mean: 5 stdDev: 28 SkSTArray<5, RecordedOp, true> fRecordedOps; // MDB TODO: 4096 for the first allocation of the clip space will be huge overkill. // Gather statistics to determine the correct size. - SkArenaAlloc fClipAllocator{4096}; - SkDEBUGCODE(int fNumClips;) + SkArenaAlloc fClipAllocator{4096}; + SkDEBUGCODE(int fNumClips;) typedef GrOpList INHERITED; }; diff --git a/src/gpu/ops/GrClearOp.h b/src/gpu/ops/GrClearOp.h index b18332f..dca7afc 100644 --- a/src/gpu/ops/GrClearOp.h +++ b/src/gpu/ops/GrClearOp.h @@ -9,49 +9,30 @@ #define GrClearOp_DEFINED #include "GrFixedClip.h" -#include "GrGpu.h" #include "GrGpuCommandBuffer.h" #include "GrOp.h" #include "GrOpFlushState.h" -#include "GrRenderTarget.h" -#include "GrRenderTargetContext.h" #include "GrResourceProvider.h" class GrClearOp final : public GrOp { 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 Make(const GrFixedClip& clip, GrColor color, - GrRenderTargetContext* rtc) { - const SkIRect rtRect = SkIRect::MakeWH(rtc->width(), rtc->height()); - if (clip.scissorEnabled() && !SkIRect::Intersects(clip.scissorRect(), rtRect)) { + GrSurfaceProxy* dstProxy) { + const SkIRect rect = SkIRect::MakeWH(dstProxy->width(), dstProxy->height()); + if (clip.scissorEnabled() && !SkIRect::Intersects(clip.scissorRect(), rect)) { return nullptr; } - // 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(new GrClearOp(clip, color, rtc)); + return std::unique_ptr(new GrClearOp(clip, color, dstProxy)); } - // MDB TODO: replace the renderTargetContext with just the renderTargetProxy. static std::unique_ptr Make(const SkIRect& rect, GrColor color, - GrRenderTargetContext* rtc, bool fullScreen) { SkASSERT(fullScreen || !rect.isEmpty()); - // MDB TODO: remove this. See above comment. - if (!rtc->accessRenderTarget()) { - return nullptr; - } - - return std::unique_ptr(new GrClearOp(rect, color, rtc, fullScreen)); + return std::unique_ptr(new GrClearOp(rect, color, fullScreen)); } const char* name() const override { return "Clear"; } @@ -59,9 +40,7 @@ public: SkString dumpInfo() const override { SkString string; string.append(INHERITED::dumpInfo()); - string.appendf("rtID: %d proxyID: %d Scissor [", - fRenderTarget.get()->uniqueID().asUInt(), - fProxyUniqueID.asUInt()); + string.appendf("Scissor [ "); 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); @@ -76,13 +55,11 @@ public: void setColor(GrColor color) { fColor = color; } private: - GrClearOp(const GrFixedClip& clip, GrColor color, GrRenderTargetContext* rtc) + GrClearOp(const GrFixedClip& clip, GrColor color, GrSurfaceProxy* proxy) : INHERITED(ClassID()) , fClip(clip) - , fColor(color) - , fProxyUniqueID(rtc->asSurfaceProxy()->uniqueID()) { + , fColor(color) { - 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. @@ -97,20 +74,17 @@ private: } this->setBounds(SkRect::Make(fClip.scissorEnabled() ? fClip.scissorRect() : rtRect), HasAABloat::kNo, IsZeroArea::kNo); - fRenderTarget.reset(rtc->accessRenderTarget()); } - GrClearOp(const SkIRect& rect, GrColor color, GrRenderTargetContext* rtc, bool fullScreen) + GrClearOp(const SkIRect& rect, GrColor color, bool fullScreen) : INHERITED(ClassID()) , fClip(GrFixedClip(rect)) - , fColor(color) - , fProxyUniqueID(rtc->asSurfaceProxy()->uniqueID()) { + , fColor(color) { 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 { @@ -118,8 +92,6 @@ private: // contains the old clear, or when the new clear is a subset of the old clear and is the // same color. GrClearOp* cb = t->cast(); - SkASSERT(cb->fRenderTarget == fRenderTarget); - SkASSERT(cb->fProxyUniqueID == fProxyUniqueID); if (fClip.windowRectsState() != cb->fClip.windowRectsState()) { return false; } @@ -144,17 +116,14 @@ private: 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); + SkASSERT(state->drawOpArgs().fRenderTarget); + + state->commandBuffer()->clear(state->drawOpArgs().fRenderTarget, fClip, fColor); } GrFixedClip fClip; GrColor fColor; - // MDB TODO: remove this. When the renderTargetProxy carries the refs this will be redundant. - GrSurfaceProxy::UniqueID fProxyUniqueID; - GrPendingIOResource fRenderTarget; - typedef GrOp INHERITED; }; -- 2.7.4