Add syncing of gpu work before releasing resources in GrDirectContext teardowns.

As we start moving backends to tracking resources on command buffers by using
GrGpuResources, we need to make sure we don't release this resources from the
cache during teardown until they have finished on the gpu. Thus this CL adds
a way for the GrDirectContext to tell the GrGpu to finish all outstanding
work.

Bug: skia:11232
Change-Id: I953d89f514ad32f1d2c57279a670b336d7575ffe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361457
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

include/gpu/GrDirectContext.h

@@ -811,6 +811,19 @@ protected:
     GrDirectContext* asDirectContext() override { return this; }
 
 private:
+    // This call will make sure out work on the GPU is finished and will execute any outstanding
+    // asynchronous work (e.g. calling finished procs, freeing resources, etc.) related to the
+    // outstanding work on the gpu. The main use currently for this function is when tearing down or
+    // abandoning the context.
+    //
+    // When we finish up work on the GPU it could trigger callbacks to the client. In the case we
+    // are abandoning the context we don't want the client to be able to use the GrDirectContext to
+    // issue more commands during the callback. Thus before calling this function we set the
+    // GrDirectContext's state to be abandoned. However, we need to be able to get by the abaonded
+    // check in the call to know that it is safe to execute this. The shouldExecuteWhileAbandoned
+    // bool is used for this signal.
+    void syncAllOutstandingGpuWork(bool shouldExecuteWhileAbandoned);
+
     // fTaskGroup must appear before anything that uses it (e.g. fGpu), so that it is destroyed
     // after all of its users. Clients of fTaskGroup will generally want to ensure that they call
     // wait() on it as they are being destroyed, to avoid the possibility of pending tasks being

src/gpu/GrCaps.cpp

@@ -56,6 +56,7 @@ GrCaps::GrCaps(const GrContextOptions& options) {
     fWritePixelsRowBytesSupport = false;
     fReadPixelsRowBytesSupport = false;
     fShouldCollapseSrcOverToSrcWhenAble = false;
+    fMustSyncGpuDuringAbandon = true;
     fDriverDisableCCPR = false;
     fDriverDisableMSAACCPR = false;
     fDisableTessellationPathRenderer = false;

src/gpu/GrCaps.h

@@ -156,6 +156,12 @@ public:
         return fShouldCollapseSrcOverToSrcWhenAble;
     }
 
+    // When abandoning the GrDirectContext do we need to sync the GPU before we start abandoning
+    // resources.
+    bool mustSyncGpuDuringAbandon() const {
+        return fMustSyncGpuDuringAbandon;
+    }
+
     /**
      * Indicates whether GPU->CPU memory mapping for GPU resources such as vertex buffers and
      * textures allows partial mappings or full mappings.
@@ -526,6 +532,7 @@ protected:
     bool fWritePixelsRowBytesSupport                 : 1;
     bool fReadPixelsRowBytesSupport                  : 1;
     bool fShouldCollapseSrcOverToSrcWhenAble         : 1;
+    bool fMustSyncGpuDuringAbandon                   : 1;
 
     // Driver workaround
     bool fDriverDisableCCPR                          : 1;

src/gpu/GrDirectContext.cpp

@@ -63,6 +63,9 @@ GrDirectContext::~GrDirectContext() {
         this->flushAndSubmit();
     }
 
+    // We need to make sure all work is finished on the gpu before we start releasing resources.
+    this->syncAllOutstandingGpuWork(/*shouldExecuteWhileAbandoned=*/false);
+
     this->destroyDrawingManager();
     fMappedBufferManager.reset();
 
@@ -95,6 +98,9 @@ void GrDirectContext::abandonContext() {
 
     INHERITED::abandonContext();
 
+    // We need to make sure all work is finished on the gpu before we start releasing resources.
+    this->syncAllOutstandingGpuWork(this->caps()->mustSyncGpuDuringAbandon());
+
     fStrikeCache->freeAll();
 
     fMappedBufferManager->abandon();
@@ -134,6 +140,9 @@ void GrDirectContext::releaseResourcesAndAbandonContext() {
 
     INHERITED::abandonContext();
 
+    // We need to make sure all work is finished on the gpu before we start releasing resources.
+    this->syncAllOutstandingGpuWork(/*shouldExecuteWhileAbandoned=*/true);
+
     fMappedBufferManager.reset();
 
     fResourceProvider->abandon();
@@ -390,6 +399,13 @@ void GrDirectContext::checkAsyncWorkCompletion() {
     }
 }
 
+void GrDirectContext::syncAllOutstandingGpuWork(bool shouldExecuteWhileAbandoned) {
+    if (fGpu && (!this->abandoned() || shouldExecuteWhileAbandoned)) {
+        fGpu->finishOutstandingGpuWork();
+        this->checkAsyncWorkCompletion();
+    }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 void GrDirectContext::storeVkPipelineCacheData() {

src/gpu/GrGpu.h

@@ -392,6 +392,7 @@ public:
     virtual void addFinishedProc(GrGpuFinishedProc finishedProc,
                                  GrGpuFinishedContext finishedContext) = 0;
     virtual void checkFinishProcs() = 0;
+    virtual void finishOutstandingGpuWork() = 0;
 
     virtual void takeOwnershipOfBuffer(sk_sp<GrGpuBuffer>) {}
 

src/gpu/d3d/GrD3DGpu.cpp

@@ -1357,3 +1357,7 @@ GrFence SK_WARN_UNUSED_RESULT GrD3DGpu::insertFence() {
 bool GrD3DGpu::waitFence(GrFence fence) {
     return (fFence->GetCompletedValue() >= fence);
 }
+
+void GrD3DGpu::finishOutstandingGpuWork() {
+    this->waitForQueueCompletion();
+}

src/gpu/d3d/GrD3DGpu.h

@@ -118,6 +118,7 @@ public:
     void submit(GrOpsRenderPass* renderPass) override;
 
     void checkFinishProcs() override { this->checkForFinishedCommandLists(); }
+    void finishOutstandingGpuWork() override;
 
 private:
     enum class SyncQueue {

src/gpu/dawn/GrDawnGpu.cpp

@@ -133,13 +133,11 @@ GrDawnGpu::GrDawnGpu(GrDirectContext* direct, const GrContextOptions& options,
     this->initCapsAndCompiler(sk_make_sp<GrDawnCaps>(options));
 }
 
-GrDawnGpu::~GrDawnGpu() {
-    this->waitOnAllBusyStagingBuffers();
-}
+GrDawnGpu::~GrDawnGpu() { this->finishOutstandingGpuWork(); }
 
 void GrDawnGpu::disconnect(DisconnectType type) {
     if (DisconnectType::kCleanup == type) {
-        this->waitOnAllBusyStagingBuffers();
+        this->finishOutstandingGpuWork();
     }
     fStagingBufferManager.reset();
     fQueue = nullptr;
@@ -848,6 +846,10 @@ void GrDawnGpu::checkFinishProcs() {
     fFinishCallbacks.check();
 }
 
+void GrDawnGpu::finishOutstandingGpuWork() {
+    this->waitOnAllBusyStagingBuffers();
+}
+
 std::unique_ptr<GrSemaphore> GrDawnGpu::prepareTextureForCrossContextUsage(GrTexture* texture) {
     SkASSERT(!"unimplemented");
     return nullptr;

src/gpu/dawn/GrDawnGpu.h

@@ -87,6 +87,7 @@ public:
     void insertSemaphore(GrSemaphore* semaphore) override;
     void waitSemaphore(GrSemaphore* semaphore) override;
     void checkFinishProcs() override;
+    void finishOutstandingGpuWork() override;
 
     std::unique_ptr<GrSemaphore> prepareTextureForCrossContextUsage(GrTexture*) override;
 

src/gpu/gl/GrGLCaps.cpp

@@ -343,6 +343,9 @@ void GrGLCaps::init(const GrContextOptions& contextOptions,
         fSkipErrorChecks = true;
     }
 
+    // When we are abandoning the context we cannot call into GL thus we should skip any sync work.
+    fMustSyncGpuDuringAbandon = false;
+
     /**************************************************************************
     * GrShaderCaps fields
     **************************************************************************/

src/gpu/gl/GrGLGpu.cpp

@@ -3916,7 +3916,7 @@ void GrGLGpu::flush(FlushType flushType) {
 
 bool GrGLGpu::onSubmitToGpu(bool syncCpu) {
     if (syncCpu || (!fFinishCallbacks.empty() && !this->caps()->fenceSyncSupport())) {
-        GL_CALL(Finish());
+        this->finishOutstandingGpuWork();
         fFinishCallbacks.callAll(true);
     } else {
         this->flush();
@@ -4025,6 +4025,10 @@ void GrGLGpu::checkFinishProcs() {
     fFinishCallbacks.check();
 }
 
+void GrGLGpu::finishOutstandingGpuWork() {
+    GL_CALL(Finish());
+}
+
 void GrGLGpu::clearErrorsAndCheckForOOM() {
     while (this->getErrorAndCheckForOOM() != GR_GL_NO_ERROR) {}
 }

src/gpu/gl/GrGLGpu.h

@@ -177,6 +177,7 @@ public:
     void waitSemaphore(GrSemaphore* semaphore) override;
 
     void checkFinishProcs() override;
+    void finishOutstandingGpuWork() override;
 
     // Calls glGetError() until no errors are reported. Also looks for OOMs.
     void clearErrorsAndCheckForOOM();

src/gpu/mock/GrMockGpu.h

@@ -46,6 +46,7 @@ public:
     void submit(GrOpsRenderPass* renderPass) override;
 
     void checkFinishProcs() override {}
+    void finishOutstandingGpuWork() override {}
 
 private:
     GrMockGpu(GrDirectContext*, const GrMockOptions&, const GrContextOptions&);

src/gpu/mtl/GrMtlGpu.h

@@ -99,6 +99,7 @@ public:
     void insertSemaphore(GrSemaphore* semaphore) override;
     void waitSemaphore(GrSemaphore* semaphore) override;
     void checkFinishProcs() override { this->checkForFinishedCommandBuffers(); }
+    void finishOutstandingGpuWork() override;
     std::unique_ptr<GrSemaphore> prepareTextureForCrossContextUsage(GrTexture*) override;
 
     // When the Metal backend actually uses indirect command buffers, this function will actually do

src/gpu/mtl/GrMtlGpu.mm

@@ -222,12 +222,7 @@ void GrMtlGpu::submit(GrOpsRenderPass* renderPass) {
 bool GrMtlGpu::submitCommandBuffer(SyncQueue sync) {
     if (!fCurrentCmdBuffer || !fCurrentCmdBuffer->hasWork()) {
         if (sync == SyncQueue::kForce_SyncQueue) {
-            // wait for the last command buffer we've submitted to finish
-            OutstandingCommandBuffer* back =
-                    (OutstandingCommandBuffer*)fOutstandingCommandBuffers.back();
-            if (back) {
-                (*back)->waitUntilCompleted();
-            }
+            this->finishOutstandingGpuWork();
             this->checkForFinishedCommandBuffers();
         }
         // We need to manually call the finishedCallbacks since we don't add this
@@ -274,6 +269,15 @@ void GrMtlGpu::checkForFinishedCommandBuffers() {
     }
 }
 
+void GrMtlGpu::finishOutstandingGpuWork() {
+    // wait for the last command buffer we've submitted to finish
+    OutstandingCommandBuffer* back =
+            (OutstandingCommandBuffer*)fOutstandingCommandBuffers.back();
+    if (back) {
+        (*back)->waitUntilCompleted();
+    }
+}
+
 void GrMtlGpu::addFinishedProc(GrGpuFinishedProc finishedProc,
                                GrGpuFinishedContext finishedContext) {
     SkASSERT(finishedProc);

src/gpu/vk/GrVkGpu.cpp

@@ -251,25 +251,7 @@ void GrVkGpu::destroyResources() {
     }
 
     // wait for all commands to finish
-    VkResult res = VK_CALL(QueueWaitIdle(fQueue));
-
-    // On windows, sometimes calls to QueueWaitIdle return before actually signalling the fences
-    // on the command buffers even though they have completed. This causes an assert to fire when
-    // destroying the command buffers. Currently this ony seems to happen on windows, so we add a
-    // sleep to make sure the fence signals.
-#ifdef SK_DEBUG
-    if (this->vkCaps().mustSleepOnTearDown()) {
-#if defined(SK_BUILD_FOR_WIN)
-        Sleep(10); // In milliseconds
-#else
-        sleep(1);  // In seconds
-#endif
-    }
-#endif
-
-#ifdef SK_DEBUG
-    SkASSERT(VK_SUCCESS == res || VK_ERROR_DEVICE_LOST == res);
-#endif
+    this->finishOutstandingGpuWork();
 
     if (fMainCmdPool) {
         fMainCmdPool->unref();
@@ -291,7 +273,7 @@ void GrVkGpu::destroyResources() {
     fMSAALoadManager.destroyResources(this);
 
     // must call this just before we destroy the command pool and VkDevice
-    fResourceProvider.destroyResources(VK_ERROR_DEVICE_LOST == res);
+    fResourceProvider.destroyResources();
 }
 
 GrVkGpu::~GrVkGpu() {
@@ -2150,6 +2132,24 @@ bool GrVkGpu::onSubmitToGpu(bool syncCpu) {
     }
 }
 
+void GrVkGpu::finishOutstandingGpuWork() {
+    VK_CALL(QueueWaitIdle(fQueue));
+
+    // On Windows and Imagination, sometimes calls to QueueWaitIdle return before actually
+    // signalling the fences on the command buffers even though they have completed. This causes an
+    // assert to fire when destroying the command buffers. Therefore we add asleep to make sure the
+    // fence signals.
+    #ifdef SK_DEBUG
+        if (this->vkCaps().mustSleepOnTearDown()) {
+    #if defined(SK_BUILD_FOR_WIN)
+            Sleep(10);  // In milliseconds
+    #else
+            sleep(1);  // In seconds
+    #endif
+        }
+    #endif
+}
+
 void GrVkGpu::onReportSubmitHistograms() {
 #if SK_HISTOGRAMS_ENABLED
     uint64_t allocatedMemory = fMemoryAllocator->totalAllocatedMemory();

src/gpu/vk/GrVkGpu.h

@@ -162,6 +162,7 @@ public:
     void addDrawable(std::unique_ptr<SkDrawable::GpuDrawHandler> drawable);
 
     void checkFinishProcs() override { fResourceProvider.checkCommandBuffers(); }
+    void finishOutstandingGpuWork() override;
 
     std::unique_ptr<GrSemaphore> prepareTextureForCrossContextUsage(GrTexture*) override;
 

src/gpu/vk/GrVkResourceProvider.cpp

@@ -476,7 +476,7 @@ void GrVkResourceProvider::recycleStandardUniformBufferResource(const GrManagedR
     fAvailableUniformBufferResources.push_back(resource);
 }
 
-void GrVkResourceProvider::destroyResources(bool deviceLost) {
+void GrVkResourceProvider::destroyResources() {
     SkTaskGroup* taskGroup = fGpu->getContext()->priv().getTaskGroup();
     if (taskGroup) {
         taskGroup->wait();

src/gpu/vk/GrVkResourceProvider.h

@@ -204,9 +204,7 @@ public:
     // The assumption is that all queues are idle and all command buffers are finished.
     // For resource tracing to work properly, this should be called after unrefing all other
     // resource usages.
-    // If deviceLost is true, then resources will not be checked to see if they've finished
-    // before deleting (see section 4.2.4 of the Vulkan spec).
-    void destroyResources(bool deviceLost);
+    void destroyResources();
 
     void backgroundReset(GrVkCommandPool* pool);