Resolve GrSurface/GrSurfaceProxy ref counting issue in GrResourceAllocator

The underlying issue is/was that (given that GrResourceProvider::createApproxTexture can pull a scratch texture out of the resource cache) resources the GrResourceAllocator thought is was controlling could magically be re-assigned behind its back. This CL resolves the issue by having the GrResourceAllocator maintain a strong ref on all the resources it believes it is controlling.

Change-Id: Ibcf49009dc953bd97d882177284eb57490cd5711
Reviewed-on: https://skia-review.googlesource.com/70722
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This commit is contained in:
Robert Phillips 2017-11-13 15:48:12 -05:00 committed by Skia Commit-Bot
parent 74c8436f2c
commit 5b65a84b99
2 changed files with 46 additions and 11 deletions

View File

@ -8,12 +8,27 @@
#include "GrResourceAllocator.h"
#include "GrGpuResourcePriv.h"
#include "GrOpList.h"
#include "GrResourceProvider.h"
#include "GrSurfacePriv.h"
#include "GrSurfaceProxy.h"
#include "GrSurfaceProxyPriv.h"
#include "GrTextureProxy.h"
void GrResourceAllocator::Interval::assign(sk_sp<GrSurface> s) {
SkASSERT(!fAssignedSurface);
fAssignedSurface = s;
fProxy->priv().assign(std::move(s));
}
GrResourceAllocator::~GrResourceAllocator() {
#ifndef SK_DISABLE_EXPLICIT_GPU_RESOURCE_ALLOCATION
SkASSERT(fIntvlList.empty());
SkASSERT(fActiveIntvls.empty());
SkASSERT(!fIntvlHash.count());
#endif
}
void GrResourceAllocator::addInterval(GrSurfaceProxy* proxy,
unsigned int start, unsigned int end) {
SkASSERT(start <= end);
@ -22,7 +37,7 @@ void GrResourceAllocator::addInterval(GrSurfaceProxy* proxy,
if (Interval* intvl = fIntvlHash.find(proxy->uniqueID().asUInt())) {
// Revise the interval for an existing use
// TODO: this assert is failing on the copy_on_write_retain GM!
//SkASSERT(intvl->end() <= start);
SkASSERT(intvl->end() <= start);
if (intvl->end() < end) {
intvl->extendEnd(end);
}
@ -87,7 +102,7 @@ void GrResourceAllocator::IntervalList::insertByIncreasingEnd(Interval* intvl) {
}
// 'surface' can be reused. Add it back to the free pool.
void GrResourceAllocator::freeUpSurface(GrSurface* surface) {
void GrResourceAllocator::freeUpSurface(sk_sp<GrSurface> surface) {
const GrScratchKey &key = surface->resourcePriv().getScratchKey();
if (!key.isValid()) {
@ -102,7 +117,7 @@ void GrResourceAllocator::freeUpSurface(GrSurface* surface) {
}
// TODO: fix this insertion so we get a more LRU-ish behavior
fFreePool.insert(key, SkRef(surface));
fFreePool.insert(key, surface.release());
}
// First try to reuse one of the recently allocated/used GrSurfaces in the free pool.
@ -138,7 +153,10 @@ sk_sp<GrSurface> GrResourceAllocator::findSurfaceFor(const GrSurfaceProxy* proxy
void GrResourceAllocator::expire(unsigned int curIndex) {
while (!fActiveIntvls.empty() && fActiveIntvls.peekHead()->end() < curIndex) {
Interval* temp = fActiveIntvls.popHead();
this->freeUpSurface(temp->proxy()->priv().peekSurface());
if (temp->wasAssignedSurface()) {
this->freeUpSurface(temp->detachSurface());
}
// Add temp to the free interval list so it can be reused
temp->setNext(fFreeIntervalList);
@ -168,9 +186,13 @@ void GrResourceAllocator::assign() {
SkASSERT(surface->getUniqueKey() == tex->getUniqueKey());
}
cur->proxy()->priv().assign(std::move(surface));
cur->assign(std::move(surface));
}
// TODO: handle resouce allocation failure upstack
fActiveIntvls.insertByIncreasingEnd(cur);
}
// expire all the remaining intervals to drain the active interval list
this->expire(std::numeric_limits<unsigned int>::max());
}

View File

@ -39,6 +39,8 @@ public:
: fResourceProvider(resourceProvider) {
}
~GrResourceAllocator();
unsigned int curOp() const { return fNumOps; }
void incOps() { fNumOps++; }
unsigned int numOps() const { return fNumOps; }
@ -62,7 +64,7 @@ private:
void expire(unsigned int curIndex);
// These two methods wrap the interactions with the free pool
void freeUpSurface(GrSurface* surface);
void freeUpSurface(sk_sp<GrSurface> surface);
sk_sp<GrSurface> findSurfaceFor(const GrSurfaceProxy* proxy);
struct FreePoolTraits {
@ -98,6 +100,10 @@ private:
fNext = nullptr;
}
~Interval() {
SkASSERT(!fAssignedSurface);
}
const GrSurfaceProxy* proxy() const { return fProxy; }
GrSurfaceProxy* proxy() { return fProxy; }
unsigned int start() const { return fStart; }
@ -112,13 +118,20 @@ private:
fEnd = newEnd;
}
void assign(sk_sp<GrSurface>);
bool wasAssignedSurface() const { return fAssignedSurface; }
sk_sp<GrSurface> detachSurface() { return std::move(fAssignedSurface); }
// for SkTDynamicHash
static const uint32_t& GetKey(const Interval& intvl) {
return intvl.fProxyID;
}
static uint32_t Hash(const uint32_t& key) { return key; }
private:
sk_sp<GrSurface> fAssignedSurface;
GrSurfaceProxy* fProxy;
uint32_t fProxyID; // This is here b.c. DynamicHash requires a ref to the key
unsigned int fStart;