Loosen up uniqueKey asserts between GrTextureProxy and GrTexture in DDLs (alt. version)
When recording a DDL it is possible for a uniquely keyed resource to lose its uniqueKey in the direct context (and its proxyProvider) while the DDL's proxyProvider still has a proxy with a uniqueKey.
Bug: 1056730
Change-Id: I565b08a8eb280aea19fc3052c758e059392a4c12
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/289890
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrProxyProvider.cpp b/src/gpu/GrProxyProvider.cpp
index cf20dd1..5edcdc5 100644
--- a/src/gpu/GrProxyProvider.cpp
+++ b/src/gpu/GrProxyProvider.cpp
@@ -54,6 +54,9 @@
return false;
}
+ // Only the proxyProvider that created a proxy should be assigning unique keys to it.
+ SkASSERT(this->isDDLProvider() == proxy->creatingProvider());
+
#ifdef SK_DEBUG
{
GrContext* direct = fImageContext->priv().asDirectContext();
@@ -189,9 +192,11 @@
#endif
if (tex->asRenderTarget()) {
- return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(std::move(tex), useAllocator));
+ return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(std::move(tex), useAllocator,
+ this->isDDLProvider()));
} else {
- return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), useAllocator));
+ return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), useAllocator,
+ this->isDDLProvider()));
}
}
@@ -424,19 +429,18 @@
? GrMipMapsStatus::kDirty
: GrMipMapsStatus::kNotAllocated;
if (renderable == GrRenderable::kYes) {
- renderTargetSampleCnt =
- caps->getRenderTargetSampleCount(renderTargetSampleCnt, format);
+ renderTargetSampleCnt = caps->getRenderTargetSampleCount(renderTargetSampleCnt, format);
SkASSERT(renderTargetSampleCnt);
// We know anything we instantiate later from this deferred path will be
// both texturable and renderable
return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(
*caps, format, dimensions, renderTargetSampleCnt, mipMapped, mipMapsStatus, fit,
- budgeted, isProtected, surfaceFlags, useAllocator));
+ budgeted, isProtected, surfaceFlags, useAllocator, this->isDDLProvider()));
}
return sk_sp<GrTextureProxy>(new GrTextureProxy(format, dimensions, mipMapped, mipMapsStatus,
fit, budgeted, isProtected, surfaceFlags,
- useAllocator));
+ useAllocator, this->isDDLProvider()));
}
sk_sp<GrTextureProxy> GrProxyProvider::createCompressedTextureProxy(
@@ -516,7 +520,8 @@
// Make sure we match how we created the proxy with SkBudgeted::kNo
SkASSERT(GrBudgetedType::kBudgeted != tex->resourcePriv().budgetedType());
- return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), UseAllocator::kNo));
+ return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), UseAllocator::kNo,
+ this->isDDLProvider()));
}
sk_sp<GrTextureProxy> GrProxyProvider::wrapCompressedBackendTexture(const GrBackendTexture& beTex,
@@ -550,7 +555,8 @@
// Make sure we match how we created the proxy with SkBudgeted::kNo
SkASSERT(GrBudgetedType::kBudgeted != tex->resourcePriv().budgetedType());
- return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), UseAllocator::kNo));
+ return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(tex), UseAllocator::kNo,
+ this->isDDLProvider()));
}
sk_sp<GrTextureProxy> GrProxyProvider::wrapRenderableBackendTexture(
@@ -591,7 +597,8 @@
// Make sure we match how we created the proxy with SkBudgeted::kNo
SkASSERT(GrBudgetedType::kBudgeted != tex->resourcePriv().budgetedType());
- return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(std::move(tex), UseAllocator::kNo));
+ return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(std::move(tex), UseAllocator::kNo,
+ this->isDDLProvider()));
}
sk_sp<GrSurfaceProxy> GrProxyProvider::wrapBackendRenderTarget(
@@ -730,7 +737,8 @@
budgeted,
isProtected,
surfaceFlags,
- useAllocator));
+ useAllocator,
+ this->isDDLProvider()));
} else {
return sk_sp<GrTextureProxy>(new GrTextureProxy(std::move(callback),
format,
@@ -741,7 +749,8 @@
budgeted,
isProtected,
surfaceFlags,
- useAllocator));
+ useAllocator,
+ this->isDDLProvider()));
}
}
@@ -777,7 +786,7 @@
return sk_sp<GrRenderTargetProxy>(new GrTextureRenderTargetProxy(
*this->caps(), std::move(callback), format, dimensions, sampleCnt,
textureInfo->fMipMapped, mipMapsStatus, fit, budgeted, isProtected, surfaceFlags,
- useAllocator));
+ useAllocator, this->isDDLProvider()));
}
GrRenderTargetProxy::WrapsVkSecondaryCB vkSCB =
@@ -803,17 +812,20 @@
SkASSERT(renderTargetSampleCnt == 1 || renderable == GrRenderable::kYes);
GrInternalSurfaceFlags surfaceFlags = GrInternalSurfaceFlags::kNone;
+ // MakeFullyLazyProxy is only called at flush time so we know these texture proxies are
+ // not being created by a DDL provider.
static constexpr SkISize kLazyDims = {-1, -1};
if (GrRenderable::kYes == renderable) {
return sk_sp<GrTextureProxy>(new GrTextureRenderTargetProxy(
caps, std::move(callback), format, kLazyDims, renderTargetSampleCnt,
GrMipMapped::kNo, GrMipMapsStatus::kNotAllocated, SkBackingFit::kApprox,
- SkBudgeted::kYes, isProtected, surfaceFlags, useAllocator));
+ SkBudgeted::kYes, isProtected, surfaceFlags, useAllocator, GrDDLProvider::kNo));
} else {
return sk_sp<GrTextureProxy>(
new GrTextureProxy(std::move(callback), format, kLazyDims, GrMipMapped::kNo,
GrMipMapsStatus::kNotAllocated, SkBackingFit::kApprox,
- SkBudgeted::kYes, isProtected, surfaceFlags, useAllocator));
+ SkBudgeted::kYes, isProtected, surfaceFlags, useAllocator,
+ GrDDLProvider::kNo));
}
}
@@ -858,6 +870,10 @@
}
}
+GrDDLProvider GrProxyProvider::isDDLProvider() const {
+ return fImageContext->priv().asDirectContext() ? GrDDLProvider::kNo : GrDDLProvider::kYes;
+}
+
uint32_t GrProxyProvider::contextID() const {
return fImageContext->priv().contextID();
}
diff --git a/src/gpu/GrProxyProvider.h b/src/gpu/GrProxyProvider.h
index a275f29..8a8b1e1 100644
--- a/src/gpu/GrProxyProvider.h
+++ b/src/gpu/GrProxyProvider.h
@@ -223,6 +223,8 @@
*/
void processInvalidUniqueKey(const GrUniqueKey&, GrTextureProxy*, InvalidateGPUResource);
+ GrDDLProvider isDDLProvider() const;
+
// TODO: remove these entry points - it is a bit sloppy to be getting context info from here
uint32_t contextID() const;
const GrCaps* caps() const;
diff --git a/src/gpu/GrTextureProxy.cpp b/src/gpu/GrTextureProxy.cpp
index fa7e064..d955712 100644
--- a/src/gpu/GrTextureProxy.cpp
+++ b/src/gpu/GrTextureProxy.cpp
@@ -24,10 +24,13 @@
SkBudgeted budgeted,
GrProtected isProtected,
GrInternalSurfaceFlags surfaceFlags,
- UseAllocator useAllocator)
+ UseAllocator useAllocator,
+ GrDDLProvider creatingProvider)
: INHERITED(format, dimensions, fit, budgeted, isProtected, surfaceFlags, useAllocator)
, fMipMapped(mipMapped)
- , fMipMapsStatus(mipMapsStatus) SkDEBUGCODE(, fInitialMipMapsStatus(fMipMapsStatus))
+ , fMipMapsStatus(mipMapsStatus)
+ SkDEBUGCODE(, fInitialMipMapsStatus(fMipMapsStatus))
+ , fCreatingProvider(creatingProvider)
, fProxyProvider(nullptr)
, fDeferredUploader(nullptr) {
SkASSERT(!(fSurfaceFlags & GrInternalSurfaceFlags::kFramebufferOnly));
@@ -43,22 +46,28 @@
SkBudgeted budgeted,
GrProtected isProtected,
GrInternalSurfaceFlags surfaceFlags,
- UseAllocator useAllocator)
+ UseAllocator useAllocator,
+ GrDDLProvider creatingProvider)
: INHERITED(std::move(callback), format, dimensions, fit, budgeted, isProtected,
surfaceFlags, useAllocator)
, fMipMapped(mipMapped)
- , fMipMapsStatus(mipMapsStatus) SkDEBUGCODE(, fInitialMipMapsStatus(fMipMapsStatus))
+ , fMipMapsStatus(mipMapsStatus)
+ SkDEBUGCODE(, fInitialMipMapsStatus(fMipMapsStatus))
+ , fCreatingProvider(creatingProvider)
, fProxyProvider(nullptr)
, fDeferredUploader(nullptr) {
SkASSERT(!(fSurfaceFlags & GrInternalSurfaceFlags::kFramebufferOnly));
}
// Wrapped version
-GrTextureProxy::GrTextureProxy(sk_sp<GrSurface> surf, UseAllocator useAllocator)
+GrTextureProxy::GrTextureProxy(sk_sp<GrSurface> surf,
+ UseAllocator useAllocator,
+ GrDDLProvider creatingProvider)
: INHERITED(std::move(surf), SkBackingFit::kExact, useAllocator)
, fMipMapped(fTarget->asTexture()->texturePriv().mipMapped())
, fMipMapsStatus(fTarget->asTexture()->texturePriv().mipMapsStatus())
- SkDEBUGCODE(, fInitialMipMapsStatus(fMipMapsStatus))
+ SkDEBUGCODE(, fInitialMipMapsStatus(fMipMapsStatus))
+ , fCreatingProvider(creatingProvider)
, fProxyProvider(nullptr)
, fDeferredUploader(nullptr) {
if (fTarget->getUniqueKey().isValid()) {
diff --git a/src/gpu/GrTextureProxy.h b/src/gpu/GrTextureProxy.h
index 06afa9a..7781cc2 100644
--- a/src/gpu/GrTextureProxy.h
+++ b/src/gpu/GrTextureProxy.h
@@ -73,7 +73,8 @@
*/
const GrUniqueKey& getUniqueKey() const {
#ifdef SK_DEBUG
- if (this->isInstantiated() && fUniqueKey.isValid() && fSyncTargetKey) {
+ if (this->isInstantiated() && fUniqueKey.isValid() && fSyncTargetKey &&
+ fCreatingProvider == GrDDLProvider::kNo) {
GrSurface* surface = this->peekSurface();
SkASSERT(surface);
@@ -100,6 +101,8 @@
GrTextureProxyPriv texPriv();
const GrTextureProxyPriv texPriv() const;
+ SkDEBUGCODE(GrDDLProvider creatingProvider() const { return fCreatingProvider; })
+
protected:
// DDL TODO: rm the GrSurfaceProxy friending
friend class GrSurfaceProxy; // for ctors
@@ -116,7 +119,8 @@
SkBudgeted,
GrProtected,
GrInternalSurfaceFlags,
- UseAllocator);
+ UseAllocator,
+ GrDDLProvider creatingProvider);
// Lazy-callback version
// There are two main use cases for lazily-instantiated proxies:
@@ -137,15 +141,19 @@
SkBudgeted,
GrProtected,
GrInternalSurfaceFlags,
- UseAllocator);
+ UseAllocator,
+ GrDDLProvider creatingProvider);
// Wrapped version
- GrTextureProxy(sk_sp<GrSurface>, UseAllocator);
+ GrTextureProxy(sk_sp<GrSurface>, UseAllocator, GrDDLProvider creatingProvider);
~GrTextureProxy() override;
sk_sp<GrSurface> createSurface(GrResourceProvider*) const override;
+ // By default uniqueKeys are propagated from a textureProxy to its backing GrTexture.
+ // Setting syncTargetKey to false disables this behavior and only keeps the unique key
+ // on the proxy.
void setTargetKeySync(bool sync) { fSyncTargetKey = sync; }
private:
@@ -173,6 +181,12 @@
bool fSyncTargetKey = true; // Should target's unique key be sync'ed with ours.
+ // For GrTextureProxies created in a DDL recording thread it is possible for the uniqueKey
+ // to be cleared on the backing GrTexture while the uniqueKey remains on the proxy.
+ // A fCreatingProvider of DDLProvider::kYes loosens up asserts that the key of an instantiated
+ // uniquely-keyed textureProxy is also always set on the backing GrTexture.
+ GrDDLProvider fCreatingProvider = GrDDLProvider::kNo;
+
GrUniqueKey fUniqueKey;
GrProxyProvider* fProxyProvider; // only set when fUniqueKey is valid
diff --git a/src/gpu/GrTextureRenderTargetProxy.cpp b/src/gpu/GrTextureRenderTargetProxy.cpp
index e7b6e18..7dbd304 100644
--- a/src/gpu/GrTextureRenderTargetProxy.cpp
+++ b/src/gpu/GrTextureRenderTargetProxy.cpp
@@ -30,13 +30,14 @@
SkBudgeted budgeted,
GrProtected isProtected,
GrInternalSurfaceFlags surfaceFlags,
- UseAllocator useAllocator)
+ UseAllocator useAllocator,
+ GrDDLProvider creatingProvider)
: GrSurfaceProxy(format, dimensions, fit, budgeted, isProtected, surfaceFlags, useAllocator)
// for now textures w/ data are always wrapped
, GrRenderTargetProxy(caps, format, dimensions, sampleCnt, fit, budgeted, isProtected,
surfaceFlags, useAllocator)
, GrTextureProxy(format, dimensions, mipMapped, mipMapsStatus, fit, budgeted, isProtected,
- surfaceFlags, useAllocator) {
+ surfaceFlags, useAllocator, creatingProvider) {
this->initSurfaceFlags(caps);
}
@@ -52,7 +53,8 @@
SkBudgeted budgeted,
GrProtected isProtected,
GrInternalSurfaceFlags surfaceFlags,
- UseAllocator useAllocator)
+ UseAllocator useAllocator,
+ GrDDLProvider creatingProvider)
: GrSurfaceProxy(std::move(callback), format, dimensions, fit, budgeted, isProtected,
surfaceFlags, useAllocator)
// Since we have virtual inheritance, we initialize GrSurfaceProxy directly. Send null
@@ -61,7 +63,8 @@
budgeted, isProtected, surfaceFlags, useAllocator,
WrapsVkSecondaryCB::kNo)
, GrTextureProxy(LazyInstantiateCallback(), format, dimensions, mipMapped, mipMapsStatus,
- fit, budgeted, isProtected, surfaceFlags, useAllocator) {
+ fit, budgeted, isProtected, surfaceFlags, useAllocator,
+ creatingProvider) {
this->initSurfaceFlags(caps);
}
@@ -69,10 +72,11 @@
// This class is virtually derived from GrSurfaceProxy (via both GrTextureProxy and
// GrRenderTargetProxy) so its constructor must be explicitly called.
GrTextureRenderTargetProxy::GrTextureRenderTargetProxy(sk_sp<GrSurface> surf,
- UseAllocator useAllocator)
+ UseAllocator useAllocator,
+ GrDDLProvider creatingProvider)
: GrSurfaceProxy(surf, SkBackingFit::kExact, useAllocator)
, GrRenderTargetProxy(surf, useAllocator)
- , GrTextureProxy(surf, useAllocator) {
+ , GrTextureProxy(surf, useAllocator, creatingProvider) {
SkASSERT(surf->asTexture());
SkASSERT(surf->asRenderTarget());
SkASSERT(fSurfaceFlags == fTarget->surfacePriv().flags());
diff --git a/src/gpu/GrTextureRenderTargetProxy.h b/src/gpu/GrTextureRenderTargetProxy.h
index 585f60c..4eb81f2 100644
--- a/src/gpu/GrTextureRenderTargetProxy.h
+++ b/src/gpu/GrTextureRenderTargetProxy.h
@@ -38,7 +38,8 @@
SkBudgeted,
GrProtected,
GrInternalSurfaceFlags,
- UseAllocator);
+ UseAllocator,
+ GrDDLProvider creatingProvider);
// Lazy-callback version
GrTextureRenderTargetProxy(const GrCaps&,
@@ -52,11 +53,13 @@
SkBudgeted,
GrProtected,
GrInternalSurfaceFlags,
- UseAllocator);
+ UseAllocator,
+ GrDDLProvider creatingProvider);
// Wrapped version
GrTextureRenderTargetProxy(sk_sp<GrSurface>,
- UseAllocator);
+ UseAllocator,
+ GrDDLProvider creatingProvider);
void initSurfaceFlags(const GrCaps&);