No longer forward refs and unrefs from the GrSurfaceProxy to the backing GrSurface
This is setting up for GrIORefProxy to just become SkRefCnt and GrProxyRef to just become sk_sp.
Change-Id: Ica66565a353de980a7070e0788f1f2b17565baee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/220297
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrDeinstantiateProxyTracker.cpp b/src/gpu/GrDeinstantiateProxyTracker.cpp
index 25c1d15..f250056 100644
--- a/src/gpu/GrDeinstantiateProxyTracker.cpp
+++ b/src/gpu/GrDeinstantiateProxyTracker.cpp
@@ -18,8 +18,7 @@
SkASSERT(proxy != fProxies[i].get());
}
#endif
- proxy->firstRefAccess().ref(fCache);
- fProxies.push_back(sk_sp<GrSurfaceProxy>(proxy));
+ fProxies.push_back(sk_ref_sp<GrSurfaceProxy>(proxy));
}
void GrDeinstantiateProxyTracker::deinstantiateAllProxies() {
diff --git a/src/gpu/GrDeinstantiateProxyTracker.h b/src/gpu/GrDeinstantiateProxyTracker.h
index 06f0b1d..c6e16d2 100644
--- a/src/gpu/GrDeinstantiateProxyTracker.h
+++ b/src/gpu/GrDeinstantiateProxyTracker.h
@@ -15,7 +15,7 @@
class GrDeinstantiateProxyTracker {
public:
- GrDeinstantiateProxyTracker(GrResourceCache* cache) : fCache(cache) {}
+ GrDeinstantiateProxyTracker() {}
// Adds a proxy which will be deinstantiated at the end of flush. The same proxy may not be
// added multiple times.
@@ -25,7 +25,6 @@
void deinstantiateAllProxies();
private:
- GrResourceCache* fCache;
SkTArray<sk_sp<GrSurfaceProxy>> fProxies;
};
diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp
index 5cedb39..54a21ea 100644
--- a/src/gpu/GrDrawingManager.cpp
+++ b/src/gpu/GrDrawingManager.cpp
@@ -259,8 +259,7 @@
fCpuBufferCache = GrBufferAllocPool::CpuBufferCache::Make(maxCachedBuffers);
}
- GrOpFlushState flushState(gpu, resourceProvider, resourceCache, &fTokenTracker,
- fCpuBufferCache);
+ GrOpFlushState flushState(gpu, resourceProvider, &fTokenTracker, fCpuBufferCache);
GrOnFlushResourceProvider onFlushProvider(this);
// TODO: AFAICT the only reason fFlushState is on GrDrawingManager rather than on the
diff --git a/src/gpu/GrOpFlushState.cpp b/src/gpu/GrOpFlushState.cpp
index 6bb096d..996ea15 100644
--- a/src/gpu/GrOpFlushState.cpp
+++ b/src/gpu/GrOpFlushState.cpp
@@ -16,14 +16,14 @@
//////////////////////////////////////////////////////////////////////////////
GrOpFlushState::GrOpFlushState(GrGpu* gpu, GrResourceProvider* resourceProvider,
- GrResourceCache* cache, GrTokenTracker* tokenTracker,
+ GrTokenTracker* tokenTracker,
sk_sp<GrBufferAllocPool::CpuBufferCache> cpuBufferCache)
: fVertexPool(gpu, cpuBufferCache)
, fIndexPool(gpu, std::move(cpuBufferCache))
, fGpu(gpu)
, fResourceProvider(resourceProvider)
, fTokenTracker(tokenTracker)
- , fDeinstantiateProxyTracker(cache) {}
+ , fDeinstantiateProxyTracker() {}
const GrCaps& GrOpFlushState::caps() const {
return *fGpu->caps();
diff --git a/src/gpu/GrOpFlushState.h b/src/gpu/GrOpFlushState.h
index 3eeb5fc..5e81e4a 100644
--- a/src/gpu/GrOpFlushState.h
+++ b/src/gpu/GrOpFlushState.h
@@ -29,7 +29,7 @@
// vertexSpace and indexSpace may either be null or an alloation of size
// GrBufferAllocPool::kDefaultBufferSize. If the latter, then CPU memory is only allocated for
// vertices/indices when a buffer larger than kDefaultBufferSize is required.
- GrOpFlushState(GrGpu*, GrResourceProvider*, GrResourceCache*, GrTokenTracker*,
+ GrOpFlushState(GrGpu*, GrResourceProvider*, GrTokenTracker*,
sk_sp<GrBufferAllocPool::CpuBufferCache> = nullptr);
~GrOpFlushState() final { this->reset(); }
diff --git a/src/gpu/GrProxyProvider.cpp b/src/gpu/GrProxyProvider.cpp
index ee8ab36..7a3c966 100644
--- a/src/gpu/GrProxyProvider.cpp
+++ b/src/gpu/GrProxyProvider.cpp
@@ -103,17 +103,12 @@
}
GrTextureProxy* proxy = fUniquelyKeyedProxies.find(key);
- sk_sp<GrTextureProxy> result;
if (proxy) {
- GrResourceCache* cache = nullptr;
- if (auto directContext = fImageContext->priv().asDirectContext()) {
- cache = directContext->priv().getResourceCache();
- }
- proxy->firstRefAccess().ref(cache);
- result.reset(proxy);
- SkASSERT(result->origin() == origin);
+ SkASSERT(proxy->getProxyRefCnt() >= 1);
+ SkASSERT(proxy->origin() == origin);
+ return sk_ref_sp(proxy);
}
- return result;
+ return nullptr;
}
///////////////////////////////////////////////////////////////////////////////
diff --git a/src/gpu/GrProxyRef.h b/src/gpu/GrProxyRef.h
index 5ea5c33..1048a49 100644
--- a/src/gpu/GrProxyRef.h
+++ b/src/gpu/GrProxyRef.h
@@ -26,32 +26,17 @@
~GrProxyRef() { this->reset(); }
void setProxy(sk_sp<T> proxy) {
- SkASSERT(SkToBool(fProxy) == fOwnRef);
- SkSafeUnref(fProxy);
- if (!proxy) {
- fProxy = nullptr;
- fOwnRef = false;
- } else {
- fProxy = proxy.release(); // due to the semantics of this class we unpack from sk_sp
- fOwnRef = true;
- }
+ fProxy = std::move(proxy);
}
- T* get() const { return fProxy; }
+ T* get() const { return fProxy.get(); }
- // Shortcut for calling setProxy() with NULL.
void reset() {
- if (fOwnRef) {
- SkASSERT(fProxy);
- fProxy->unref();
- fOwnRef = false;
- }
fProxy = nullptr;
}
private:
- T* fProxy = nullptr;
- mutable bool fOwnRef = false;
+ sk_sp<T> fProxy;
};
using GrSurfaceProxyRef = GrProxyRef<GrSurfaceProxy>;
diff --git a/src/gpu/GrResourceAllocator.cpp b/src/gpu/GrResourceAllocator.cpp
index d37d81c..82c8b82 100644
--- a/src/gpu/GrResourceAllocator.cpp
+++ b/src/gpu/GrResourceAllocator.cpp
@@ -44,10 +44,10 @@
continue;
}
- if (cur->uses() >= cur->proxy()->priv().getTotalRefs()) {
+ if (cur->uses() >= cur->proxy()->priv().getProxyRefCnt()) {
// All the refs on the proxy are known to the resource allocator thus no one
// should be holding onto it outside of Ganesh.
- SkASSERT(cur->uses() == cur->proxy()->priv().getTotalRefs());
+ SkASSERT(cur->uses() == cur->proxy()->priv().getProxyRefCnt());
cur->markAsRecyclable();
}
}
diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp
index 04b629e..466f799 100644
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -51,6 +51,16 @@
}
#endif
+#if GR_TEST_UTILS
+int32_t GrIORefProxy::getBackingRefCnt_TestOnly() const {
+ if (fTarget) {
+ return fTarget->fRefCnt;
+ }
+
+ return -1; // no backing GrSurface
+}
+#endif
+
// Lazy-callback version
GrSurfaceProxy::GrSurfaceProxy(LazyInstantiateCallback&& callback, LazyInstantiationType lazyType,
const GrBackendFormat& format, const GrSurfaceDesc& desc,
@@ -183,7 +193,8 @@
return nullptr;
}
- if (!GrSurfaceProxyPriv::AttachStencilIfNeeded(resourceProvider, surface.get(), needsStencil)) {
+ if (!GrSurfaceProxyPriv::AttachStencilIfNeeded(resourceProvider, surface.get(),
+ needsStencil)) {
return nullptr;
}
@@ -210,9 +221,7 @@
SkDEBUGCODE(this->validateSurface(surface.get());)
- fTarget = surface.release();
-
- this->INHERITED::transferRefs();
+ fTarget = std::move(surface);
#ifdef SK_DEBUG
if (this->asRenderTargetProxy()) {
@@ -236,7 +245,8 @@
if (uniqueKey && uniqueKey->isValid()) {
SkASSERT(fTarget->getUniqueKey().isValid() && fTarget->getUniqueKey() == *uniqueKey);
}
- return GrSurfaceProxyPriv::AttachStencilIfNeeded(resourceProvider, fTarget, needsStencil);
+ return GrSurfaceProxyPriv::AttachStencilIfNeeded(resourceProvider, fTarget.get(),
+ needsStencil);
}
sk_sp<GrSurface> surface = this->createSurfaceImpl(resourceProvider, sampleCnt, needsStencil,
@@ -258,8 +268,7 @@
void GrSurfaceProxy::deinstantiate() {
SkASSERT(this->isInstantiated());
-
- this->release();
+ fTarget = nullptr;
}
void GrSurfaceProxy::computeScratchKey(GrScratchKey* key) const {
@@ -331,8 +340,6 @@
if (fTarget) {
SkASSERT(fTarget->getContext() == context);
}
-
- INHERITED::validate();
}
#endif
diff --git a/src/gpu/GrSurfaceProxy.h b/src/gpu/GrSurfaceProxy.h
index 30de3bb..cc19d93 100644
--- a/src/gpu/GrSurfaceProxy.h
+++ b/src/gpu/GrSurfaceProxy.h
@@ -34,117 +34,40 @@
class GrIORefProxy : public SkNoncopyable {
public:
void ref() const {
- this->validate();
-
+ SkASSERT(fRefCnt >= 1);
++fRefCnt;
- if (fTarget) {
- fTarget->ref();
- }
}
void unref() const {
- this->validate();
-
- if (fTarget) {
- fTarget->unref();
- }
-
- --fRefCnt;
- this->didRemoveRef();
- }
-
-#ifdef SK_DEBUG
- bool isUnique_debugOnly() const { // For asserts.
- SkASSERT(fRefCnt >= 0);
- return 1 == fRefCnt;
- }
-#endif
-
- void release() {
- // In the current hybrid world, the proxy and backing surface are ref/unreffed in
- // synchrony. Each ref we've added or removed to the proxy was mirrored to the backing
- // surface. Though, that backing surface could be owned by other proxies as well. Remove
- // a ref from the backing surface for each ref the proxy has since we are about to remove
- // our pointer to the surface. If this proxy is reinstantiated then all the proxy's refs
- // get transferred to the (possibly new) backing surface.
- for (int refs = fRefCnt; refs; --refs) {
- fTarget->unref();
- }
- fTarget = nullptr;
- }
-
- void validate() const {
-#ifdef SK_DEBUG
SkASSERT(fRefCnt >= 1);
-
- if (fTarget) {
- // The backing GrSurface can have more refs than the proxy if the proxy
- // started off wrapping an external resource (that came in with refs).
- // The GrSurface should never have fewer refs than the proxy however.
- SkASSERT(fTarget->fRefCnt >= fRefCnt);
- }
-#endif
- }
-
- int32_t getBackingRefCnt_TestOnly() const;
-
-protected:
- GrIORefProxy() : fTarget(nullptr), fRefCnt(1) {}
- GrIORefProxy(sk_sp<GrSurface> surface) : fRefCnt(1) {
- // Since we're manually forwarding on refs & unrefs we don't want sk_sp doing
- // anything extra.
- fTarget = surface.release();
- }
- virtual ~GrIORefProxy() {
- // We don't unref 'fTarget' here since the 'unref' method will already
- // have forwarded on the unref call that got us here.
- }
-
- // Privileged method that allows going from ref count = 0 to ref count = 1.
- void addInitialRef(GrResourceCache* cache) const {
- this->validate();
- ++fRefCnt;
- if (fTarget) {
- fTarget->proxyAccess().ref(cache);
- }
- }
-
- // This GrIORefProxy was deferred before but has just been instantiated. To
- // make all the reffing & unreffing work out we now need to transfer any deferred
- // refs & unrefs to the new GrSurface
- void transferRefs() {
- SkASSERT(fTarget);
- // Make sure we're going to take some ownership of our target.
- SkASSERT(fRefCnt > 0);
-
- SkASSERT(fTarget->fRefCnt > 0);
- SkASSERT(fRefCnt >= 0);
- // Don't xfer the proxy's creation ref. If we're going to subtract a ref do it via unref()
- // so that proper cache notifications occur.
- if (!fRefCnt) {
- fTarget->unref();
- } else {
- fTarget->fRefCnt += (fRefCnt - 1);
- }
- }
-
- int32_t internalGetProxyRefCnt() const { return fRefCnt; }
- int32_t internalGetTotalRefs() const { return fRefCnt; }
-
- // For deferred proxies this will be null. For wrapped proxies it will point to the
- // wrapped resource.
- GrSurface* fTarget;
-
-private:
- // This class is used to manage conversion of refs to pending reads/writes.
- template <typename> friend class GrProxyRef;
-
- void didRemoveRef() const {
+ --fRefCnt;
if (0 == fRefCnt) {
delete this;
}
}
+ bool unique() const {
+ SkASSERT(fRefCnt >= 1);
+ return 1 == fRefCnt;
+ }
+
+#if GR_TEST_UTILS
+ int32_t getBackingRefCnt_TestOnly() const;
+#endif
+
+protected:
+ GrIORefProxy() : fRefCnt(1) {}
+ GrIORefProxy(sk_sp<GrSurface> surface) : fTarget(std::move(surface)), fRefCnt(1) {}
+
+ virtual ~GrIORefProxy() {}
+
+ int32_t internalGetProxyRefCnt() const { return fRefCnt; }
+
+ // For deferred proxies this will be null until the proxy is instantiated.
+ // For wrapped proxies it will point to the wrapped resource.
+ sk_sp<GrSurface> fTarget;
+
+private:
mutable int32_t fRefCnt;
};
@@ -203,7 +126,7 @@
};
LazyState lazyInstantiationState() const {
- if (fTarget || !SkToBool(fLazyInstantiateCallback)) {
+ if (this->isInstantiated() || !SkToBool(fLazyInstantiateCallback)) {
return LazyState::kNot;
} else {
if (fWidth <= 0) {
@@ -333,7 +256,7 @@
bool isInstantiated() const { return SkToBool(fTarget); }
// If the proxy is already instantiated, return its backing GrTexture; if not, return null.
- GrSurface* peekSurface() const { return fTarget; }
+ GrSurface* peekSurface() const { return fTarget.get(); }
// If this is a texture proxy and the proxy is already instantiated, return its backing
// GrTexture; if not, return null.
@@ -443,7 +366,6 @@
void setIgnoredByResourceAllocator() { fIgnoredByResourceAllocator = true; }
int32_t getProxyRefCnt() const { return this->internalGetProxyRefCnt(); }
- int32_t getTotalRefs() const { return this->internalGetTotalRefs(); }
void computeScratchKey(GrScratchKey*) const;
@@ -530,25 +452,4 @@
typedef GrIORefProxy INHERITED;
};
-class GrSurfaceProxy::FirstRefAccess {
-private:
- void ref(GrResourceCache* cache) { fProxy->addInitialRef(cache); }
-
- FirstRefAccess(GrSurfaceProxy* proxy) : fProxy(proxy) {}
-
- // No taking addresses of this type.
- const FirstRefAccess* operator&() const = delete;
- FirstRefAccess* operator&() = delete;
-
- GrSurfaceProxy* fProxy;
-
- friend class GrSurfaceProxy;
- friend class GrProxyProvider;
- friend class GrDeinstantiateProxyTracker;
-};
-
-inline GrSurfaceProxy::FirstRefAccess GrSurfaceProxy::firstRefAccess() {
- return FirstRefAccess(this);
-}
-
#endif
diff --git a/src/gpu/GrSurfaceProxyPriv.h b/src/gpu/GrSurfaceProxyPriv.h
index b585dae..0dfb591 100644
--- a/src/gpu/GrSurfaceProxyPriv.h
+++ b/src/gpu/GrSurfaceProxyPriv.h
@@ -17,14 +17,8 @@
data members or virtual methods. */
class GrSurfaceProxyPriv {
public:
- // Beware! Woe betide anyone whosoever calls this method.
- // The refs on proxies and their backing GrSurfaces shift around based on whether the proxy
- // is instantiated or not. Additionally, the lifetime of a proxy (and a GrSurface) also
- // depends on the read and write refs (So this method can validly return 0).
int32_t getProxyRefCnt() const { return fProxy->getProxyRefCnt(); }
- int32_t getTotalRefs() const { return fProxy->getTotalRefs(); }
-
void computeScratchKey(GrScratchKey* key) const { return fProxy->computeScratchKey(key); }
// Create a GrSurface-derived class that meets the requirements (i.e, desc, renderability)
diff --git a/src/gpu/GrTextureProxy.cpp b/src/gpu/GrTextureProxy.cpp
index cbfbc25..6a9fd26 100644
--- a/src/gpu/GrTextureProxy.cpp
+++ b/src/gpu/GrTextureProxy.cpp
@@ -46,7 +46,7 @@
, fDeferredUploader(nullptr) {
if (fTarget->getUniqueKey().isValid()) {
fProxyProvider = fTarget->asTexture()->getContext()->priv().proxyProvider();
- fProxyProvider->adoptUniqueKeyFromSurface(this, fTarget);
+ fProxyProvider->adoptUniqueKeyFromSurface(this, fTarget.get());
}
}
diff --git a/src/gpu/ccpr/GrCCClipPath.h b/src/gpu/ccpr/GrCCClipPath.h
index 17e628c..94792d0 100644
--- a/src/gpu/ccpr/GrCCClipPath.h
+++ b/src/gpu/ccpr/GrCCClipPath.h
@@ -33,7 +33,7 @@
//
// This assert also guarantees there won't be a lazy proxy callback with a dangling pointer
// back into this class, since no proxy will exist after we destruct, if the assert passes.
- SkASSERT(!fAtlasLazyProxy || fAtlasLazyProxy->isUnique_debugOnly());
+ SkASSERT(!fAtlasLazyProxy || fAtlasLazyProxy->unique());
}
bool isInitialized() const { return fAtlasLazyProxy != nullptr; }