Fix Vulkan command buffer assert on device lost

BUG=skia:5939

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4483

Change-Id: Id363c4d774095b1707adbe6c8ab537c6e5ecab6c
Reviewed-on: https://skia-review.googlesource.com/4483
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>

diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index f11193f..b64d3ce 100644 (file)
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -166,12 +166,12 @@ GrVkGpu::~GrVkGpu() {
 
     // wait for all commands to finish
     fResourceProvider.checkCommandBuffers();
-    SkDEBUGCODE(VkResult res = ) VK_CALL(QueueWaitIdle(fQueue));
+    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 singals.
+    // sleep to make sure the fence signals.
 #ifdef SK_DEBUG
 #if defined(SK_BUILD_FOR_WIN)
     Sleep(10); // In milliseconds
@@ -187,8 +187,8 @@ GrVkGpu::~GrVkGpu() {
 
     fCopyManager.destroyResources(this);
 
-    // must call this just before we destroy the VkDevice
-    fResourceProvider.destroyResources();
+    // must call this just before we destroy the command pool and VkDevice
+    fResourceProvider.destroyResources(VK_ERROR_DEVICE_LOST == res);
 
     VK_CALL(DestroyCommandPool(fDevice, fCmdPool, nullptr));
 

diff --git a/src/gpu/vk/GrVkResourceProvider.cpp b/src/gpu/vk/GrVkResourceProvider.cpp
index ca4d52e..5cde9bc 100644 (file)
--- a/src/gpu/vk/GrVkResourceProvider.cpp
+++ b/src/gpu/vk/GrVkResourceProvider.cpp
@@ -307,10 +307,10 @@ void GrVkResourceProvider::recycleStandardUniformBufferResource(const GrVkResour
     fAvailableUniformBufferResources.push_back(resource);
 }
 
-void GrVkResourceProvider::destroyResources() {
+void GrVkResourceProvider::destroyResources(bool deviceLost) {
     // release our active command buffers
     for (int i = 0; i < fActiveCommandBuffers.count(); ++i) {
-        SkASSERT(fActiveCommandBuffers[i]->finished(fGpu));
+        SkASSERT(deviceLost || fActiveCommandBuffers[i]->finished(fGpu));
         SkASSERT(fActiveCommandBuffers[i]->unique());
         fActiveCommandBuffers[i]->reset(fGpu);
         fActiveCommandBuffers[i]->unref(fGpu);
@@ -318,7 +318,7 @@ void GrVkResourceProvider::destroyResources() {
     fActiveCommandBuffers.reset();
     // release our available command buffers
     for (int i = 0; i < fAvailableCommandBuffers.count(); ++i) {
-        SkASSERT(fAvailableCommandBuffers[i]->finished(fGpu));
+        SkASSERT(deviceLost || fAvailableCommandBuffers[i]->finished(fGpu));
         SkASSERT(fAvailableCommandBuffers[i]->unique());
         fAvailableCommandBuffers[i]->unref(fGpu);
     }

diff --git a/src/gpu/vk/GrVkResourceProvider.h b/src/gpu/vk/GrVkResourceProvider.h
index 0c9397e..99724a6 100644 (file)
--- a/src/gpu/vk/GrVkResourceProvider.h
+++ b/src/gpu/vk/GrVkResourceProvider.h
@@ -148,7 +148,9 @@ 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.
-    void destroyResources();
+    // 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);
 
     // Abandon any cached resources. To be used when the context/VkDevice is lost.
     // For resource tracing to work properly, this should be called after unrefing all other

diff --git a/tests/ResourceCacheTest.cpp b/tests/ResourceCacheTest.cpp
index e96653b..6c55f8a 100644 (file)
--- a/tests/ResourceCacheTest.cpp
+++ b/tests/ResourceCacheTest.cpp
@@ -1405,11 +1405,6 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(GPUMemorySize, reporter, ctxInfo) {
     GrContext* context = ctxInfo.grContext();
     GrTextureProvider* provider = context->textureProvider();
 
-    // Vulkan is unhappy with this test
-    if (kVulkan_GrBackend == ctxInfo.backend()) {
-        return;
-    }
-
     static const int kSize = 64;
 
     sk_sp<GrTexture> tex;