Fix DDL reattachment of uniquely keyed proxies

This fix has three parts:
   No longer clear all proxy unique keys in DDL mode
   Handle unique keys appropriately in lazy proxy instantiation
   Handle attaching to cached surfaces for non-lazy proxies

Change-Id: I86b0422a784acaf8c5f9b67cb981b440e08352de
Reviewed-on: https://skia-review.googlesource.com/154502
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

include/gpu/GrResourceKey.h

@@ -80,6 +80,22 @@ protected:
         return &fKey[kMetaDataCnt];
     }
 
+#ifdef SK_DEBUG
+    void dump() const {
+        if (!this->isValid()) {
+            SkDebugf("Invalid Key\n");
+        } else {
+            SkDebugf("hash: %d ", this->hash());
+            SkDebugf("domain: %d ", this->domain());
+            SkDebugf("size: %dB ", this->internalSize());
+            for (size_t i = 0; i < this->internalSize(); ++i) {
+                SkDebugf("%d ", fKey[i]);
+            }
+            SkDebugf("\n");
+        }
+    }
+#endif
+
     /** Used to initialize a key. */
     class Builder {
     public:
@@ -229,7 +245,7 @@ public:
     static Domain GenerateDomain();
 
     /** Creates an invalid unique key. It must be initialized using a Builder object before use. */
-    GrUniqueKey() {}
+    GrUniqueKey() : fTag(nullptr) {}
 
     GrUniqueKey(const GrUniqueKey& that) { *this = that; }
 
@@ -259,6 +275,13 @@ public:
 
     const char* tag() const { return fTag; }
 
+#ifdef SK_DEBUG
+    void dump(const char* label) const {
+        SkDebugf("%s tag: %s\n", label, fTag ? fTag : "None");
+        this->INHERITED::dump();
+    }
+#endif
+
     class Builder : public INHERITED::Builder {
     public:
         Builder(GrUniqueKey* key, Domain type, int data32Count, const char* tag = nullptr)

src/core/SkDeferredDisplayListRecorder.cpp

@@ -59,9 +59,16 @@ SkDeferredDisplayListRecorder::~SkDeferredDisplayListRecorder() {
     if (fContext) {
         auto proxyProvider = fContext->contextPriv().proxyProvider();
 
-        // DDL TODO: Remove this. DDL contexts should allow for deletion while still having live
-        // uniquely keyed proxies.
-        proxyProvider->removeAllUniqueKeys();
+        // This allows the uniquely keyed proxies to keep their keys but removes their back
+        // pointer to the about-to-be-deleted proxy provider. The proxies will use their
+        // unique key to reattach to cached versions of themselves or to appropriately tag new
+        // resources (if a cached version was not found). This system operates independent of
+        // the replaying context's proxy provider (i.e., these uniquely keyed proxies will not
+        // appear in the replaying proxy providers uniquely keyed proxy map). This should be fine
+        // since no one else should be trying to reconnect to the orphaned proxies and orphaned
+        // proxies from different DDLs that share the same key should simply reconnect to the
+        // same cached resource.
+        proxyProvider->orphanAllUniqueKeys();
     }
 }
 

src/gpu/GrProxyProvider.cpp

@@ -67,7 +67,12 @@ GrProxyProvider::GrProxyProvider(uint32_t contextUniqueID,
 }
 
 GrProxyProvider::~GrProxyProvider() {
-    SkASSERT(!fUniquelyKeyedProxies.count());
+    if (fResourceCache) {
+        // In DDL-mode a proxy provider can still have extant uniquely keyed proxies (since
+        // they need their unique keys to, potentially, find a cached resource when the
+        // DDL is played) but, in non-DDL-mode they should all have been cleaned up by this point.
+        SkASSERT(!fUniquelyKeyedProxies.count());
+    }
 }
 
 bool GrProxyProvider::assignUniqueKeyToProxy(const GrUniqueKey& key, GrTextureProxy* proxy) {
@@ -104,6 +109,7 @@ void GrProxyProvider::removeUniqueKeyFromProxy(const GrUniqueKey& key, GrTexture
     if (this->isAbandoned() || !proxy) {
         return;
     }
+
     this->processInvalidProxyUniqueKey(key, proxy, true);
 }
 
@@ -687,6 +693,15 @@ void GrProxyProvider::processInvalidProxyUniqueKey(const GrUniqueKey& key, GrTex
     }
 }
 
+void GrProxyProvider::orphanAllUniqueKeys() {
+    UniquelyKeyedProxyHash::Iter iter(&fUniquelyKeyedProxies);
+    for (UniquelyKeyedProxyHash::Iter iter(&fUniquelyKeyedProxies); !iter.done(); ++iter) {
+        GrTextureProxy& tmp = *iter;
+
+        tmp.fProxyProvider = nullptr;
+    }
+}
+
 void GrProxyProvider::removeAllUniqueKeys() {
     UniquelyKeyedProxyHash::Iter iter(&fUniquelyKeyedProxies);
     for (UniquelyKeyedProxyHash::Iter iter(&fUniquelyKeyedProxies); !iter.done(); ++iter) {

src/gpu/GrProxyProvider.h

@@ -232,6 +232,11 @@ public:
 
     int numUniqueKeyProxies_TestOnly() const;
 
+    // This is called on a DDL's proxyprovider when the DDL is finished. The uniquely keyed
+    // proxies need to keep their unique key but cannot hold on to the proxy provider unique
+    // pointer.
+    void orphanAllUniqueKeys();
+    // This is only used by GrContext::releaseResourcesAndAbandonContext()
     void removeAllUniqueKeys();
 
     /**

src/gpu/GrResourceAllocator.cpp

@@ -221,6 +221,21 @@ void GrResourceAllocator::recycleSurface(sk_sp<GrSurface> surface) {
 // If we can't find a useable one, create a new one.
 sk_sp<GrSurface> GrResourceAllocator::findSurfaceFor(const GrSurfaceProxy* proxy,
                                                      bool needsStencil) {
+
+    if (proxy->asTextureProxy() && proxy->asTextureProxy()->getUniqueKey().isValid()) {
+        // First try to reattach to a cached version if the proxy is uniquely keyed
+        sk_sp<GrSurface> surface = fResourceProvider->findByUniqueKey<GrSurface>(
+                                                        proxy->asTextureProxy()->getUniqueKey());
+        if (surface) {
+            if (!GrSurfaceProxyPriv::AttachStencilIfNeeded(fResourceProvider, surface.get(),
+                                                           needsStencil)) {
+                return nullptr;
+            }
+
+            return surface;
+        }
+    }
+
     // First look in the free pool
     GrScratchKey key;
 
@@ -242,6 +257,7 @@ sk_sp<GrSurface> GrResourceAllocator::findSurfaceFor(const GrSurfaceProxy* proxy
                                                        needsStencil)) {
             return nullptr;
         }
+        SkASSERT(!surface->getUniqueKey().isValid());
         return surface;
     }
 
@@ -350,10 +366,14 @@ bool GrResourceAllocator::assign(int* startIndex, int* stopIndex,
             }
         } else if (sk_sp<GrSurface> surface = this->findSurfaceFor(cur->proxy(), needsStencil)) {
             // TODO: make getUniqueKey virtual on GrSurfaceProxy
-            GrTextureProxy* tex = cur->proxy()->asTextureProxy();
-            if (tex && tex->getUniqueKey().isValid()) {
-                fResourceProvider->assignUniqueKeyToResource(tex->getUniqueKey(), surface.get());
-                SkASSERT(surface->getUniqueKey() == tex->getUniqueKey());
+            GrTextureProxy* texProxy = cur->proxy()->asTextureProxy();
+
+            if (texProxy && texProxy->getUniqueKey().isValid()) {
+                if (!surface->getUniqueKey().isValid()) {
+                    fResourceProvider->assignUniqueKeyToResource(texProxy->getUniqueKey(),
+                                                                 surface.get());
+                }
+                SkASSERT(surface->getUniqueKey() == texProxy->getUniqueKey());
             }
 
 #if GR_ALLOCATION_SPEW

src/gpu/GrSurfaceProxy.cpp

@@ -392,7 +392,16 @@ void GrSurfaceProxyPriv::exactify() {
 bool GrSurfaceProxyPriv::doLazyInstantiation(GrResourceProvider* resourceProvider) {
     SkASSERT(GrSurfaceProxy::LazyState::kNot != fProxy->lazyInstantiationState());
 
-    sk_sp<GrSurface> surface = fProxy->fLazyInstantiateCallback(resourceProvider);
+    sk_sp<GrSurface> surface;
+    if (fProxy->asTextureProxy() && fProxy->asTextureProxy()->getUniqueKey().isValid()) {
+        // First try to reattach to a cached version if the proxy is uniquely keyed
+        surface = resourceProvider->findByUniqueKey<GrSurface>(
+                                                        fProxy->asTextureProxy()->getUniqueKey());
+    }
+
+    if (!surface) {
+        surface = fProxy->fLazyInstantiateCallback(resourceProvider);
+    }
     if (GrSurfaceProxy::LazyInstantiationType::kSingleUse == fProxy->fLazyInstantiationType) {
         fProxy->fLazyInstantiateCallback(nullptr);
         fProxy->fLazyInstantiateCallback = nullptr;
@@ -423,7 +432,13 @@ bool GrSurfaceProxyPriv::doLazyInstantiation(GrResourceProvider* resourceProvide
     if (GrTextureProxy* texProxy = fProxy->asTextureProxy()) {
         const GrUniqueKey& key = texProxy->getUniqueKey();
         if (key.isValid()) {
-            resourceProvider->assignUniqueKeyToResource(key, surface.get());
+            if (!surface->asTexture()->getUniqueKey().isValid()) {
+                // If 'surface' is newly created, attach the unique key
+                resourceProvider->assignUniqueKeyToResource(key, surface.get());
+            } else {
+                // otherwise we had better have reattached to a cached version
+                SkASSERT(surface->asTexture()->getUniqueKey() == key);
+            }
         }
     }
 

src/gpu/GrTextureProxy.cpp

@@ -66,7 +66,11 @@ GrTextureProxy::~GrTextureProxy() {
     // Due to the order of cleanup the GrSurface this proxy may have wrapped may have gone away
     // at this point. Zero out the pointer so the cache invalidation code doesn't try to use it.
     fTarget = nullptr;
-    if (fUniqueKey.isValid()) {
+
+    // In DDL-mode, uniquely keyed proxies keep their key even after their originating
+    // proxy provider has gone away. In that case there is noone to send the invalid key
+    // message to (Note: in this case we don't want to remove its cached resource).
+    if (fUniqueKey.isValid() && fProxyProvider) {
         fProxyProvider->processInvalidProxyUniqueKey(fUniqueKey, this, false);
     } else {
         SkASSERT(!fProxyProvider);

tools/DDLPromiseImageHelper.cpp

@@ -102,9 +102,8 @@ sk_sp<SkImage> DDLPromiseImageHelper::PromiseImageCreator(const void* rawData,
     if (!curImage.fCallbackContext->backendTexture().isValid()) {
         // We weren't able to make a backend texture for this SkImage. In this case we create
         // a separate bitmap-backed image for each thread.
-        // Note: we would like to share the same bitmap between all the threads but
-        // SkBitmap is not thread-safe.
-        return SkImage::MakeRasterCopy(curImage.fBitmap.pixmap());
+        SkASSERT(curImage.fBitmap.isImmutable());
+        return SkImage::MakeFromBitmap(curImage.fBitmap);
     }
     SkASSERT(curImage.fIndex == *indexPtr);