Revert "Distinguish between "flushed" and "finished" idle state callb…

…acks on GrTexture."

This reverts commit 9ac0407.

Reason for revert: Breaking DDL Win10 skpbench bot

Original change's description:
> Distinguish between "flushed" and "finished" idle state callbacks on GrTexture.
> 
> This is necessary to convert the promise image API to call Release when all
> work is flushed and Done when all work is complete (future work).
> 
> Change-Id: I9745952bb0978ca2aaa79aeed460730b2fea856e
> Bug: skia:8800
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/197163
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>

TBR=egdaniel@google.com,bsalomon@google.com,robertphillips@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: skia:8800
Change-Id: I5e6c4ea072beb4fb67a53d2ea2b007a7d201799d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/198603
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

include/gpu/GrTexture.h

@@ -49,31 +49,17 @@ class SK_API GrTexture : virtual public GrSurface {
     }
 #endif
 
-    /** See addIdleProc. */
-    enum class IdleState {
-        kFlushed,
-        kFinished
-    };
     /**
-     * Installs a proc on this texture. It will be called when the texture becomes "idle". There
-     * are two types of idle states as indicated by IdleState. For managed backends (e.g. GL where
-     * a driver typically handles CPU/GPU synchronization of resource access) there is no difference
-     * between the two. They both mean "all work related to the resource has been flushed to the
-     * backend API and the texture is not owned outside the resource cache".
-     *
-     * If the API is unmanaged (e.g. Vulkan) then kFinished has the additional constraint that the
-     * work flushed to the GPU is finished.
+     * Installs a proc on this texture. It will be called when the texture becomes "idle". Idle is
+     * defined to mean that the texture has no refs or pending IOs and that GPU I/O operations on
+     * the texture are completed if the backend API disallows deletion of a texture before such
+     * operations occur (e.g. Vulkan). After the idle proc is called it is removed. The idle proc
+     * will always be called before the texture is destroyed, even in unusual shutdown scenarios
+     * (e.g. GrContext::abandonContext()).
      */
-    virtual void addIdleProc(sk_sp<GrRefCntedCallback> idleProc, IdleState) {
-        // This is the default implementation for the managed case where the IdleState can be
-        // ignored. Unmanaged backends, e.g. Vulkan, must override this to consider IdleState.
-        fIdleProcs.push_back(std::move(idleProc));
-    }
-    /** Helper version of addIdleProc that creates the ref-counted wrapper. */
-    void addIdleProc(GrRefCntedCallback::Callback callback,
-                     GrRefCntedCallback::Context context,
-                     IdleState state) {
-        this->addIdleProc(sk_make_sp<GrRefCntedCallback>(callback, context), state);
+    virtual void addIdleProc(sk_sp<GrRefCntedCallback> callback) {
+        callback->addChild(std::move(fIdleCallback));
+        fIdleCallback = std::move(callback);
     }
 
     /** Access methods that are only to be used within Skia code. */
@@ -85,11 +71,11 @@ class SK_API GrTexture : virtual public GrSurface {
 
     virtual bool onStealBackendTexture(GrBackendTexture*, SkImage::BackendTextureReleaseProc*) = 0;
 
-    SkTArray<sk_sp<GrRefCntedCallback>> fIdleProcs;
+    sk_sp<GrRefCntedCallback> fIdleCallback;
 
     void willRemoveLastRefOrPendingIO() override {
-        // We're about to be idle in the resource cache. Do our part to trigger the idle callbacks.
-        fIdleProcs.reset();
+        // We're about to be idle in the resource cache. Do our part to trigger the idle callback.
+        fIdleCallback.reset();
     }
 
 private:

include/private/GrTypesPriv.h

@@ -1555,7 +1555,8 @@ static inline GrPixelConfig GrColorTypeToPixelConfig(GrColorType config,
 }
 
 /**
- * Ref-counted object that calls a callback from its destructor.
+ * Ref-counted object that calls a callback from its destructor. These can be chained together. Any
+ * owner can cancel calling the callback via abandon().
  */
 class GrRefCntedCallback : public SkRefCnt {
 public:
@@ -1567,9 +1568,28 @@ class GrRefCntedCallback : public SkRefCnt {
     }
     ~GrRefCntedCallback() override { fReleaseProc ? fReleaseProc(fReleaseCtx) : void(); }
 
+    /**
+     * After abandon is called the release proc will no longer be called in the destructor. This
+     * does not recurse on child release procs or unref them.
+     */
+    void abandon() {
+        fReleaseProc = nullptr;
+        fReleaseCtx = nullptr;
+    }
+
+    /** Adds another GrRefCntedCallback that this will unref in its destructor. */
+    void addChild(sk_sp<GrRefCntedCallback> next) {
+        if (!fNext) {
+            fNext = std::move(next);
+            return;
+        }
+        fNext->addChild(std::move(next));
+    }
+
     Context context() const { return fReleaseCtx; }
 
 private:
+    sk_sp<GrRefCntedCallback> fNext;
     Callback fReleaseProc;
     Context fReleaseCtx;
 };

src/gpu/vk/GrVkImage.cpp

@@ -270,37 +270,28 @@ void GrVkImage::Resource::freeGPUData(GrVkGpu* gpu) const {
     GrVkMemory::FreeImageMemory(gpu, isLinear, fAlloc);
 }
 
-void GrVkImage::Resource::addIdleProc(GrVkTexture* owningTexture,
-                                      sk_sp<GrRefCntedCallback> idleProc) const {
-    SkASSERT(!fOwningTexture || fOwningTexture == owningTexture);
-    fOwningTexture = owningTexture;
-    fIdleProcs.push_back(std::move(idleProc));
+void GrVkImage::Resource::replaceIdleProc(
+        GrVkTexture* owner, sk_sp<GrRefCntedCallback> idleCallback) const {
+    fOwningTexture = owner;
+    fIdleCallback = std::move(idleCallback);
 }
 
-int GrVkImage::Resource::idleProcCnt() const { return fIdleProcs.count(); }
-
-sk_sp<GrRefCntedCallback> GrVkImage::Resource::idleProc(int i) const { return fIdleProcs[i]; }
-
-void GrVkImage::Resource::resetIdleProcs() const { fIdleProcs.reset(); }
-
 void GrVkImage::Resource::removeOwningTexture() const { fOwningTexture = nullptr; }
 
 void GrVkImage::Resource::notifyAddedToCommandBuffer() const { ++fNumCommandBufferOwners; }
 
 void GrVkImage::Resource::notifyRemovedFromCommandBuffer() const {
     SkASSERT(fNumCommandBufferOwners);
-    if (--fNumCommandBufferOwners || !fIdleProcs.count()) {
+    if (--fNumCommandBufferOwners || !fIdleCallback) {
         return;
     }
     if (fOwningTexture) {
         if (fOwningTexture->resourcePriv().hasRefOrPendingIO()) {
-            // Wait for the texture to become idle in the cache to call the procs.
             return;
         }
-        fOwningTexture->callIdleProcsOnBehalfOfResource();
-    } else {
-        fIdleProcs.reset();
+        fOwningTexture->removeIdleProc();
     }
+    fIdleCallback.reset();
 }
 
 void GrVkImage::BorrowedResource::freeGPUData(GrVkGpu* gpu) const {

src/gpu/vk/GrVkImage.h

@@ -187,15 +187,12 @@ class GrVkImage : SkNoncopyable {
         }
 
         /**
-         * These are used to coordinate calling the "finished" idle procs between the GrVkTexture
-         * and the Resource. If the GrVkTexture becomes purgeable and if there are no command
-         * buffers referring to the Resource then it calls the procs. Otherwise, the Resource calls
-         * them when the last command buffer reference goes away and the GrVkTexture is purgeable.
+         * These are used to coordinate calling the idle proc between the GrVkTexture and the
+         * Resource. If the GrVkTexture becomes purgeable and if there are no command buffers
+         * referring to the Resource then it calls the proc. Otherwise, the Resource calls it
+         * when the last command buffer reference goes away and the GrVkTexture is purgeable.
          */
-        void addIdleProc(GrVkTexture*, sk_sp<GrRefCntedCallback>) const;
-        int idleProcCnt() const;
-        sk_sp<GrRefCntedCallback> idleProc(int) const;
-        void resetIdleProcs() const;
+        void replaceIdleProc(GrVkTexture* owner, sk_sp<GrRefCntedCallback>) const;
         void removeOwningTexture() const;
 
         /**
@@ -228,7 +225,7 @@ class GrVkImage : SkNoncopyable {
         GrVkAlloc      fAlloc;
         VkImageTiling  fImageTiling;
         mutable int fNumCommandBufferOwners = 0;
-        mutable SkTArray<sk_sp<GrRefCntedCallback>> fIdleProcs;
+        mutable sk_sp<GrRefCntedCallback> fIdleCallback;
         mutable GrVkTexture* fOwningTexture = nullptr;
 
         typedef GrVkResource INHERITED;

src/gpu/vk/GrVkTexture.cpp

@@ -120,11 +120,11 @@ GrVkTexture::~GrVkTexture() {
 }
 
 void GrVkTexture::onRelease() {
-    // We're about to be severed from our GrVkResource. If there are "finish" idle procs we have to
-    // decide who will handle them. If the resource is still tied to a command buffer we let it
-    // handle them. Otherwise, we handle them.
+    // We're about to be severed from our GrVkResource. If there is an idle proc we have to decide
+    // who will handle it. If the resource is still tied to a command buffer we let it handle it.
+    // Otherwise, we handle it.
     if (this->hasResource() && this->resource()->isOwnedByCommandBuffer()) {
-        this->removeFinishIdleProcs();
+        fIdleCallback.reset();
     }
 
     // we create this and don't hand it off, so we should always destroy it
@@ -139,11 +139,11 @@ void GrVkTexture::onRelease() {
 }
 
 void GrVkTexture::onAbandon() {
-    // We're about to be severed from our GrVkResource. If there are "finish" idle procs we have to
-    // decide who will handle them. If the resource is still tied to a command buffer we let it
-    // handle them. Otherwise, we handle them.
+    // We're about to be severed from our GrVkResource. If there is an idle proc we have to decide
+    // who will handle it. If the resource is still tied to a command buffer we let it handle it.
+    // Otherwise, we handle it.
     if (this->hasResource() && this->resource()->isOwnedByCommandBuffer()) {
-        this->removeFinishIdleProcs();
+        fIdleCallback.reset();
     }
 
     // we create this and don't hand it off, so we should always destroy it
@@ -169,70 +169,25 @@ const GrVkImageView* GrVkTexture::textureView() {
     return fTextureView;
 }
 
-void GrVkTexture::addIdleProc(sk_sp<GrRefCntedCallback> idleProc, IdleState type) {
-    INHERITED::addIdleProc(idleProc, type);
-    if (type == IdleState::kFinished) {
-        if (auto* resource = this->resource()) {
-            resource->addIdleProc(this, std::move(idleProc));
-        }
-    }
-}
-
-void GrVkTexture::callIdleProcsOnBehalfOfResource() {
-    // If we got here then the resource is being removed from its last command buffer and the
-    // texture is idle in the cache. Any kFlush idle procs should already have been called. So
-    // the texture and resource should have the same set of procs.
-    SkASSERT(this->resource());
-    SkASSERT(this->resource()->idleProcCnt() == fIdleProcs.count());
-#ifdef SK_DEBUG
-    for (int i = 0; i < fIdleProcs.count(); ++i) {
-        SkASSERT(fIdleProcs[i] == this->resource()->idleProc(i));
+void GrVkTexture::addIdleProc(sk_sp<GrRefCntedCallback> callback) {
+    INHERITED::addIdleProc(callback);
+    if (auto* resource = this->resource()) {
+        resource->replaceIdleProc(this, fIdleCallback);
     }
-#endif
-    fIdleProcs.reset();
-    this->resource()->resetIdleProcs();
 }
 
 void GrVkTexture::willRemoveLastRefOrPendingIO() {
-    if (!fIdleProcs.count()) {
+    if (!fIdleCallback) {
         return;
     }
     // This is called when the GrTexture is purgeable. However, we need to check whether the
     // Resource is still owned by any command buffers. If it is then it will call the proc.
     auto* resource = this->hasResource() ? this->resource() : nullptr;
-    bool callFinishProcs = !resource || !resource->isOwnedByCommandBuffer();
-    if (callFinishProcs) {
-        // Everything must go!
-        fIdleProcs.reset();
-        resource->resetIdleProcs();
-    } else {
-        // The procs that should be called on flush but not finish are those that are owned
-        // by the GrVkTexture and not the Resource. We do this by copying the resource's array
-        // and thereby dropping refs to procs we own but the resource does not.
-        SkASSERT(resource);
-        fIdleProcs.reset(resource->idleProcCnt());
-        for (int i = 0; i < fIdleProcs.count(); ++i) {
-            fIdleProcs[i] = resource->idleProc(i);
-        }
-    }
-}
-
-void GrVkTexture::removeFinishIdleProcs() {
-    // This should only be called by onRelease/onAbandon when we have already checked for a
-    // resource.
-    const auto* resource = this->resource();
-    SkASSERT(resource);
-    SkSTArray<4, sk_sp<GrRefCntedCallback>> procsToKeep;
-    int resourceIdx = 0;
-    // The idle procs that are common between the GrVkTexture and its Resource should be found in
-    // the same order.
-    for (int i = 0; i < fIdleProcs.count(); ++i) {
-        if (fIdleProcs[i] == resource->idleProc(resourceIdx)) {
-            ++resourceIdx;
-        } else {
-            procsToKeep.push_back(fIdleProcs[i]);
+    if (resource) {
+        if (resource->isOwnedByCommandBuffer()) {
+            return;
         }
+        resource->replaceIdleProc(this, nullptr);
     }
-    SkASSERT(resourceIdx == resource->idleProcCnt());
-    fIdleProcs = procsToKeep;
+    fIdleCallback.reset();
 }

src/gpu/vk/GrVkTexture.h

@@ -38,8 +38,8 @@ class GrVkTexture : public GrTexture, public virtual GrVkImage {
 
     const GrVkImageView* textureView();
 
-    void addIdleProc(sk_sp<GrRefCntedCallback>, IdleState) override;
-    void callIdleProcsOnBehalfOfResource();
+    void addIdleProc(sk_sp<GrRefCntedCallback>) override;
+    void removeIdleProc() { fIdleCallback.reset(); }
 
 protected:
     GrVkTexture(GrVkGpu*, const GrSurfaceDesc&, const GrVkImageInfo&, sk_sp<GrVkImageLayout>,
@@ -71,8 +71,6 @@ class GrVkTexture : public GrTexture, public virtual GrVkImage {
         this->setResourceRelease(std::move(releaseHelper));
     }
 
-    void removeFinishIdleProcs();
-
     const GrVkImageView* fTextureView;
 
     typedef GrTexture INHERITED;

src/image/SkImage_GpuBase.cpp

@@ -426,8 +426,10 @@ sk_sp<GrTextureProxy> SkImage_GpuBase::MakePromiseImageLazyProxy(
                                        PromiseImageTextureDoneProc doneProc,
                                        PromiseImageTextureContext context,
                                        GrPixelConfig config)
-                : fFulfillProc(fulfillProc), fReleaseProc(releaseProc), fConfig(config) {
-            fDoneCallback = sk_make_sp<GrRefCntedCallback>(doneProc, context);
+                : fFulfillProc(fulfillProc), fConfig(config) {
+            auto doneHelper = sk_make_sp<GrRefCntedCallback>(doneProc, context);
+            fIdleCallback = sk_make_sp<GrRefCntedCallback>(releaseProc, context);
+            fIdleCallback->addChild(std::move(doneHelper));
         }
         PromiseLazyInstantiateCallback(PromiseLazyInstantiateCallback&&) = default;
         PromiseLazyInstantiateCallback(const PromiseLazyInstantiateCallback&) {
@@ -441,20 +443,30 @@ sk_sp<GrTextureProxy> SkImage_GpuBase::MakePromiseImageLazyProxy(
             return *this;
         }
 
+        ~PromiseLazyInstantiateCallback() {
+            if (fIdleCallback) {
+                // We were never fulfilled. Pass false so done proc is still called.
+                fIdleCallback->abandon();
+            }
+        }
+
         sk_sp<GrSurface> operator()(GrResourceProvider* resourceProvider) {
-            SkASSERT(fDoneCallback);
-            PromiseImageTextureContext textureContext = fDoneCallback->context();
+            SkASSERT(fIdleCallback);
+            PromiseImageTextureContext textureContext = fIdleCallback->context();
             sk_sp<SkPromiseImageTexture> promiseTexture = fFulfillProc(textureContext);
             // From here on out our contract is that the release proc must be called, even if
             // the return from fulfill was invalid or we fail for some other reason.
-            auto releaseCallback = sk_make_sp<GrRefCntedCallback>(fReleaseProc, textureContext);
             if (!promiseTexture) {
+                // Make sure we explicitly reset this because our destructor assumes a non-null
+                // fIdleCallback means fulfill was never called.
+                fIdleCallback.reset();
                 return sk_sp<GrTexture>();
             }
 
             auto backendTexture = promiseTexture->backendTexture();
             backendTexture.fConfig = fConfig;
             if (!backendTexture.isValid()) {
+                fIdleCallback.reset();
                 return sk_sp<GrTexture>();
             }
 
@@ -477,19 +489,18 @@ sk_sp<GrTextureProxy> SkImage_GpuBase::MakePromiseImageLazyProxy(
                              kRead_GrIOType))) {
                     tex->resourcePriv().setUniqueKey(key);
                 } else {
+                    fIdleCallback.reset();
                     return sk_sp<GrTexture>();
                 }
             }
-            tex->addIdleProc(std::move(releaseCallback), GrTexture::IdleState::kFinished);
-            tex->addIdleProc(std::move(fDoneCallback), GrTexture::IdleState::kFinished);
+            tex->addIdleProc(std::move(fIdleCallback));
             promiseTexture->addKeyToInvalidate(tex->getContext()->priv().contextID(), key);
             return std::move(tex);
         }
 
     private:
+        sk_sp<GrRefCntedCallback> fIdleCallback;
         PromiseImageTextureFulfillProc fFulfillProc;
-        PromiseImageTextureReleaseProc fReleaseProc;
-        sk_sp<GrRefCntedCallback> fDoneCallback;
         GrPixelConfig fConfig;
     } callback(fulfillProc, releaseProc, doneProc, textureContext, config);