Reland "Refactor GL and Metal FinishCallback into a helper object."

This is a reland of b641ddbba0

Original change's description:
> Refactor GL and Metal FinishCallback into a helper object.
> 
> Change-Id: I52ede0363016459976e453ef71cafbcc6775f22e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/283496
> Commit-Queue: Stephen White <senorblanco@chromium.org>
> Reviewed-by: Greg Daniel <egdaniel@google.com>

Change-Id: Ia77c05c0358b6ccab1e8c0f0c06212786f415cb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/284042
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Stephen White <senorblanco@chromium.org>

gn/gpu.gni

@@ -89,6 +89,8 @@ skia_gpu_sources = [
   "$_src/gpu/GrDynamicAtlas.cpp",
   "$_src/gpu/GrDynamicAtlas.h",
   "$_src/gpu/GrEagerVertexAllocator.h",
+  "$_src/gpu/GrFinishCallbacks.cpp",
+  "$_src/gpu/GrFinishCallbacks.h",
   "$_src/gpu/GrFixedClip.cpp",
   "$_src/gpu/GrFixedClip.h",
   "$_src/gpu/GrFragmentProcessor.cpp",

src/gpu/GrFinishCallbacks.cpp

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2020 Google LLC.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "src/gpu/GrFinishCallbacks.h"
+#include "src/gpu/GrGpu.h"
+
+GrFinishCallbacks::GrFinishCallbacks(GrGpu* gpu) : fGpu(gpu) {}
+
+GrFinishCallbacks::~GrFinishCallbacks() {
+    this->callAll(true);
+}
+
+void GrFinishCallbacks::add(GrGpuFinishedProc finishedProc,
+                            GrGpuFinishedContext finishedContext) {
+    SkASSERT(finishedProc);
+    FinishCallback callback;
+    callback.fCallback = finishedProc;
+    callback.fContext = finishedContext;
+    callback.fFence = fGpu->insertFence();
+    fCallbacks.push_back(callback);
+}
+
+void GrFinishCallbacks::check() {
+    // Bail after the first unfinished sync since we expect they signal in the order inserted.
+    while (!fCallbacks.empty() && fGpu->waitFence(fCallbacks.front().fFence)) {
+        fCallbacks.front().fCallback(fCallbacks.front().fContext);
+        fGpu->deleteFence(fCallbacks.front().fFence);
+        fCallbacks.pop_front();
+    }
+}
+
+void GrFinishCallbacks::callAll(bool doDelete) {
+    while (!fCallbacks.empty()) {
+        fCallbacks.front().fCallback(fCallbacks.front().fContext);
+        if (doDelete) {
+            fGpu->deleteFence(fCallbacks.front().fFence);
+        }
+        fCallbacks.pop_front();
+    }
+}

src/gpu/GrFinishCallbacks.h

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2020 Google LLC.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrFinishCallbacks_DEFINED
+#define GrFinishCallbacks_DEFINED
+
+#include "include/gpu/GrTypes.h"
+#include "include/private/GrTypesPriv.h"
+
+#include <list>
+
+class GrGpu;
+
+/**
+ * Maintains a list of callbacks to be called when work on the GPU is complete.
+ */
+
+class GrFinishCallbacks {
+public:
+    GrFinishCallbacks(GrGpu* gpu);
+   ~GrFinishCallbacks();
+
+    /**
+     * Call all the callbacks in the list. This will block until all work is done.
+     *
+     * @param doDelete        delete the contained fence object.
+     */
+    void callAll(bool doDelete);
+
+    /**
+     * Add a new callback to the list.
+     *
+     * @param finishedProc    The function to call when GPU work is complete.
+     * @param finishedContext The context object to pass back to the callback.
+     */
+    void add(GrGpuFinishedProc finishedProc, GrGpuFinishedContext finishedContext);
+
+    /**
+     * Check if any GPU work is complete, and call the associated callbacks.
+     * This call is non-blocking.
+     */
+    void check();
+
+    /**
+     * Returns true if the callback list is empty.
+     */
+    bool empty() const { return fCallbacks.empty(); }
+
+private:
+    struct FinishCallback {
+        GrGpuFinishedProc     fCallback;
+        GrGpuFinishedContext  fContext;
+        GrFence               fFence;
+    };
+
+    GrGpu*                    fGpu;
+    std::list<FinishCallback> fCallbacks;
+};
+
+#endif

src/gpu/GrGpu.h

@@ -369,7 +369,7 @@ public:
     virtual void submit(GrOpsRenderPass*) = 0;
 
     virtual GrFence SK_WARN_UNUSED_RESULT insertFence() = 0;
-    virtual bool waitFence(GrFence, uint64_t timeout = 1000) = 0;
+    virtual bool waitFence(GrFence) = 0;
     virtual void deleteFence(GrFence) const = 0;
 
     virtual std::unique_ptr<GrSemaphore> SK_WARN_UNUSED_RESULT makeSemaphore(

src/gpu/d3d/GrD3DGpu.h

@@ -62,7 +62,7 @@ public:
             const SkTArray<GrSurfaceProxy*, true>& sampledProxies) override;
 
     GrFence SK_WARN_UNUSED_RESULT insertFence() override { return 0; }
-    bool waitFence(GrFence, uint64_t) override { return true; }
+    bool waitFence(GrFence) override { return true; }
     void deleteFence(GrFence) const override {}
 
     std::unique_ptr<GrSemaphore> SK_WARN_UNUSED_RESULT makeSemaphore(bool isOwned) override {

src/gpu/dawn/GrDawnGpu.cpp

@@ -558,7 +558,7 @@ GrFence SK_WARN_UNUSED_RESULT GrDawnGpu::insertFence() {
     return GrFence();
 }
 
-bool GrDawnGpu::waitFence(GrFence fence, uint64_t timeout) {
+bool GrDawnGpu::waitFence(GrFence fence) {
     SkASSERT(!"unimplemented");
     return false;
 }

src/gpu/dawn/GrDawnGpu.h

@@ -85,7 +85,7 @@ public:
     void submit(GrOpsRenderPass*) override;
 
     GrFence SK_WARN_UNUSED_RESULT insertFence() override;
-    bool waitFence(GrFence, uint64_t timeout) override;
+    bool waitFence(GrFence) override;
     void deleteFence(GrFence) const override;
 
     std::unique_ptr<GrSemaphore> SK_WARN_UNUSED_RESULT makeSemaphore(bool isOwned = true) override;

src/gpu/gl/GrGLGpu.cpp

@@ -330,7 +330,8 @@ GrGLGpu::GrGLGpu(std::unique_ptr<GrGLContext> ctx, GrContext* context)
         , fHWProgramID(0)
         , fTempSrcFBOID(0)
         , fTempDstFBOID(0)
-        , fStencilClearFBOID(0) {
+        , fStencilClearFBOID(0)
+        , fFinishCallbacks(this) {
     SkASSERT(fGLContext);
     GrGLClearErr(this->glInterface());
     fCaps = sk_ref_sp(fGLContext->caps());
@@ -401,11 +402,7 @@ GrGLGpu::~GrGLGpu() {
 
     fSamplerObjectCache.reset();
 
-    while (!fFinishCallbacks.empty()) {
-        fFinishCallbacks.front().fCallback(fFinishCallbacks.front().fContext);
-        this->deleteSync(fFinishCallbacks.front().fSync);
-        fFinishCallbacks.pop_front();
-    }
+    fFinishCallbacks.callAll(true);
 }
 
 void GrGLGpu::disconnect(DisconnectType type) {
@@ -465,14 +462,7 @@ void GrGLGpu::disconnect(DisconnectType type) {
     if (this->glCaps().shaderCaps()->pathRenderingSupport()) {
         this->glPathRendering()->disconnect(type);
     }
-
-    while (!fFinishCallbacks.empty()) {
-        fFinishCallbacks.front().fCallback(fFinishCallbacks.front().fContext);
-        if (DisconnectType::kCleanup == type) {
-            this->deleteSync(fFinishCallbacks.front().fSync);
-        }
-        fFinishCallbacks.pop_front();
-    }
+    fFinishCallbacks.callAll(DisconnectType::kCleanup == type);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -3786,16 +3776,7 @@ GrGLAttribArrayState* GrGLGpu::HWVertexArrayState::bindInternalVertexArray(GrGLG
 
 void GrGLGpu::addFinishedProc(GrGpuFinishedProc finishedProc,
                               GrGpuFinishedContext finishedContext) {
-    SkASSERT(finishedProc);
-    FinishCallback callback;
-    callback.fCallback = finishedProc;
-    callback.fContext = finishedContext;
-    if (this->caps()->fenceSyncSupport()) {
-        callback.fSync = (GrGLsync)this->insertFence();
-    } else {
-        callback.fSync = 0;
-    }
-    fFinishCallbacks.push_back(callback);
+    fFinishCallbacks.add(finishedProc, finishedContext);
 }
 
 void GrGLGpu::flush(FlushType flushType) {
@@ -3808,20 +3789,11 @@ void GrGLGpu::flush(FlushType flushType) {
 bool GrGLGpu::onSubmitToGpu(bool syncCpu) {
     if (syncCpu || (!fFinishCallbacks.empty() && !this->caps()->fenceSyncSupport())) {
         GL_CALL(Finish());
-        // After a finish everything previously sent to GL is done.
-        for (const auto& cb : fFinishCallbacks) {
-            cb.fCallback(cb.fContext);
-            if (cb.fSync) {
-                this->deleteSync(cb.fSync);
-            } else {
-                SkASSERT(!this->caps()->fenceSyncSupport());
-            }
-        }
-        fFinishCallbacks.clear();
+        fFinishCallbacks.callAll(true);
     } else {
         this->flush();
         // See if any previously inserted finish procs are good to go.
-        this->checkFinishProcs();
+        fFinishCallbacks.check();
     }
     return true;
 }
@@ -3833,7 +3805,9 @@ void GrGLGpu::submit(GrOpsRenderPass* renderPass) {
 }
 
 GrFence SK_WARN_UNUSED_RESULT GrGLGpu::insertFence() {
-    SkASSERT(this->caps()->fenceSyncSupport());
+    if (!this->caps()->fenceSyncSupport()) {
+        return 0;
+    }
     GrGLsync sync;
     if (this->glCaps().fenceType() == GrGLCaps::FenceType::kNVFence) {
         static_assert(sizeof(GrGLsync) >= sizeof(GrGLuint));
@@ -3873,12 +3847,17 @@ bool GrGLGpu::waitSync(GrGLsync sync, uint64_t timeout, bool flush) {
     }
 }
 
-bool GrGLGpu::waitFence(GrFence fence, uint64_t timeout) {
-    return this->waitSync((GrGLsync)fence, timeout, /* flush = */ true);
+bool GrGLGpu::waitFence(GrFence fence) {
+    if (!this->caps()->fenceSyncSupport()) {
+        return true;
+    }
+    return this->waitSync(reinterpret_cast<GrGLsync>(fence), 0, false);
 }
 
 void GrGLGpu::deleteFence(GrFence fence) const {
-    this->deleteSync((GrGLsync)fence);
+    if (this->caps()->fenceSyncSupport()) {
+        this->deleteSync(reinterpret_cast<GrGLsync>(fence));
+    }
 }
 
 std::unique_ptr<GrSemaphore> SK_WARN_UNUSED_RESULT GrGLGpu::makeSemaphore(bool isOwned) {
@@ -3910,13 +3889,7 @@ void GrGLGpu::waitSemaphore(GrSemaphore* semaphore) {
 }
 
 void GrGLGpu::checkFinishProcs() {
-    // Bail after the first unfinished sync since we expect they signal in the order inserted.
-    while (!fFinishCallbacks.empty() && this->waitSync(fFinishCallbacks.front().fSync,
-                                                       /* timeout = */ 0, /* flush  = */ false)) {
-        fFinishCallbacks.front().fCallback(fFinishCallbacks.front().fContext);
-        this->deleteSync(fFinishCallbacks.front().fSync);
-        fFinishCallbacks.pop_front();
-    }
+    fFinishCallbacks.check();
 }
 
 void GrGLGpu::deleteSync(GrGLsync sync) const {

src/gpu/gl/GrGLGpu.h

@@ -8,10 +8,10 @@
 #ifndef GrGLGpu_DEFINED
 #define GrGLGpu_DEFINED
 
-#include <list>
 #include "include/core/SkTypes.h"
 #include "include/private/SkTArray.h"
 #include "src/core/SkLRUCache.h"
+#include "src/gpu/GrFinishCallbacks.h"
 #include "src/gpu/GrGpu.h"
 #include "src/gpu/GrNativeRect.h"
 #include "src/gpu/GrProgramDesc.h"
@@ -163,7 +163,7 @@ public:
     void submit(GrOpsRenderPass* renderPass) override;
 
     GrFence SK_WARN_UNUSED_RESULT insertFence() override;
-    bool waitFence(GrFence, uint64_t timeout) override;
+    bool waitFence(GrFence) override;
     void deleteFence(GrFence) const override;
 
     std::unique_ptr<GrSemaphore> SK_WARN_UNUSED_RESULT makeSemaphore(bool isOwned) override;
@@ -707,13 +707,7 @@ private:
     std::unique_ptr<SamplerObjectCache> fSamplerObjectCache;
 
     std::unique_ptr<GrGLOpsRenderPass> fCachedOpsRenderPass;
-
-    struct FinishCallback {
-        GrGpuFinishedProc fCallback;
-        GrGpuFinishedContext fContext;
-        GrGLsync fSync;
-    };
-    std::list<FinishCallback> fFinishCallbacks;
+    GrFinishCallbacks fFinishCallbacks;
 
     // If we've called a command that requires us to call glFlush than this will be set to true
     // since we defer calling flush until submit time. When we call submitToGpu if this is true then

src/gpu/mock/GrMockGpu.h

@@ -31,7 +31,7 @@ public:
             const SkTArray<GrSurfaceProxy*, true>& sampledProxies) override;
 
     GrFence SK_WARN_UNUSED_RESULT insertFence() override { return 0; }
-    bool waitFence(GrFence, uint64_t) override { return true; }
+    bool waitFence(GrFence) override { return true; }
     void deleteFence(GrFence) const override {}
 
     std::unique_ptr<GrSemaphore> SK_WARN_UNUSED_RESULT makeSemaphore(bool isOwned) override {

src/gpu/mtl/GrMtlGpu.h

@@ -8,7 +8,7 @@
 #ifndef GrMtlGpu_DEFINED
 #define GrMtlGpu_DEFINED
 
-#include <list>
+#include "src/gpu/GrFinishCallbacks.h"
 #include "src/gpu/GrGpu.h"
 #include "src/gpu/GrRenderTarget.h"
 #include "src/gpu/GrSemaphore.h"
@@ -89,7 +89,7 @@ public:
     void submit(GrOpsRenderPass* renderPass) override;
 
     GrFence SK_WARN_UNUSED_RESULT insertFence() override;
-    bool waitFence(GrFence, uint64_t) override;
+    bool waitFence(GrFence) override;
     void deleteFence(GrFence) const override;
 
     std::unique_ptr<GrSemaphore> SK_WARN_UNUSED_RESULT makeSemaphore(bool isOwned) override;
@@ -246,12 +246,7 @@ private:
 
     bool fDisconnected;
 
-    struct FinishCallback {
-        GrGpuFinishedProc fCallback;
-        GrGpuFinishedContext fContext;
-        GrFence fFence;
-    };
-    std::list<FinishCallback> fFinishCallbacks;
+    GrFinishCallbacks fFinishCallbacks;
 
     typedef GrGpu INHERITED;
 };

src/gpu/mtl/GrMtlGpu.mm

@@ -119,7 +119,8 @@ GrMtlGpu::GrMtlGpu(GrContext* context, const GrContextOptions& options,
         , fCmdBuffer(nullptr)
         , fCompiler(new SkSL::Compiler())
         , fResourceProvider(this)
-        , fDisconnected(false) {
+        , fDisconnected(false)
+        , fFinishCallbacks(this) {
     fMtlCaps.reset(new GrMtlCaps(options, fDevice, featureSet));
     fCaps = fMtlCaps;
 }
@@ -189,23 +190,13 @@ void GrMtlGpu::submitCommandBuffer(SyncQueue sync) {
 
 void GrMtlGpu::addFinishedProc(GrGpuFinishedProc finishedProc,
                                GrGpuFinishedContext finishedContext) {
-    SkASSERT(finishedProc);
-    SkASSERT(this->caps()->fenceSyncSupport());
-    FinishCallback callback;
-    callback.fCallback = finishedProc;
-    callback.fContext = finishedContext;
-    callback.fFence = this->insertFence();
-    fFinishCallbacks.push_back(callback);
+    fFinishCallbacks.add(finishedProc, finishedContext);
 }
 
 bool GrMtlGpu::onSubmitToGpu(bool syncCpu) {
     if (syncCpu) {
         this->submitCommandBuffer(kForce_SyncQueue);
-        for (const auto& cb : fFinishCallbacks) {
-            cb.fCallback(cb.fContext);
-            this->deleteFence(cb.fFence);
-        }
-        fFinishCallbacks.clear();
+        fFinishCallbacks.callAll(true);
     } else {
         this->submitCommandBuffer(kSkip_SyncQueue);
     }
@@ -213,13 +204,7 @@ bool GrMtlGpu::onSubmitToGpu(bool syncCpu) {
 }
 
 void GrMtlGpu::checkFinishProcs() {
-    // Bail after the first unfinished sync since we expect they signal in the order inserted.
-    while (!fFinishCallbacks.empty() && this->waitFence(fFinishCallbacks.front().fFence,
-                                                       /* timeout = */ 0)) {
-        fFinishCallbacks.front().fCallback(fFinishCallbacks.front().fContext);
-        this->deleteFence(fFinishCallbacks.front().fFence);
-        fFinishCallbacks.pop_front();
-    }
+    fFinishCallbacks.check();
 }
 
 std::unique_ptr<GrSemaphore> GrMtlGpu::prepareTextureForCrossContextUsage(GrTexture*) {
@@ -1324,11 +1309,11 @@ GrFence SK_WARN_UNUSED_RESULT GrMtlGpu::insertFence() {
     return (GrFence) cfFence;
 }
 
-bool GrMtlGpu::waitFence(GrFence fence, uint64_t timeout) {
+bool GrMtlGpu::waitFence(GrFence fence) {
     const void* cfFence = (const void*) fence;
     dispatch_semaphore_t semaphore = (__bridge dispatch_semaphore_t)cfFence;
 
-    long result = dispatch_semaphore_wait(semaphore, timeout);
+    long result = dispatch_semaphore_wait(semaphore, 0);
 
     return !result;
 }

src/gpu/vk/GrVkGpu.cpp

@@ -2655,11 +2655,11 @@ GrFence SK_WARN_UNUSED_RESULT GrVkGpu::insertFence() {
     return (GrFence)fence;
 }
 
-bool GrVkGpu::waitFence(GrFence fence, uint64_t timeout) {
+bool GrVkGpu::waitFence(GrFence fence) {
     SkASSERT(VK_NULL_HANDLE != (VkFence)fence);
 
     VkResult result;
-    VK_CALL_RET(result, WaitForFences(this->device(), 1, (VkFence*)&fence, VK_TRUE, timeout));
+    VK_CALL_RET(result, WaitForFences(this->device(), 1, (VkFence*)&fence, VK_TRUE, 0));
     return (VK_SUCCESS == result);
 }
 

src/gpu/vk/GrVkGpu.h

@@ -124,7 +124,7 @@ public:
     void submit(GrOpsRenderPass*) override;
 
     GrFence SK_WARN_UNUSED_RESULT insertFence() override;
-    bool waitFence(GrFence, uint64_t timeout) override;
+    bool waitFence(GrFence) override;
     void deleteFence(GrFence) const override;
 
     std::unique_ptr<GrSemaphore> SK_WARN_UNUSED_RESULT makeSemaphore(bool isOwned) override;