Handle failure to submit semaphores in vulkan backend.
Bug: skia:9603
Change-Id: I04bacda40383667b0655c14fba0181fe7d0a51da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/255147
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index 4aa9659..43411bf 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -641,25 +641,59 @@
this->stats()->incNumFinishFlushes();
GrResourceProvider* resourceProvider = fContext->priv().resourceProvider();
- if (this->caps()->semaphoreSupport()) {
- for (int i = 0; i < info.fNumSemaphores; ++i) {
- std::unique_ptr<GrSemaphore> semaphore;
+ struct SemaphoreInfo {
+ std::unique_ptr<GrSemaphore> fSemaphore;
+ bool fDidCreate = false;
+ };
+
+ bool failedSemaphoreCreation = false;
+ std::unique_ptr<SemaphoreInfo[]> semaphoreInfos(new SemaphoreInfo[info.fNumSemaphores]);
+ if (this->caps()->semaphoreSupport() && info.fNumSemaphores) {
+ for (int i = 0; i < info.fNumSemaphores && !failedSemaphoreCreation; ++i) {
if (info.fSignalSemaphores[i].isInitialized()) {
- semaphore = resourceProvider->wrapBackendSemaphore(
+ semaphoreInfos[i].fSemaphore = resourceProvider->wrapBackendSemaphore(
info.fSignalSemaphores[i],
GrResourceProvider::SemaphoreWrapType::kWillSignal,
kBorrow_GrWrapOwnership);
} else {
- semaphore = resourceProvider->makeSemaphore(false);
+ semaphoreInfos[i].fSemaphore = resourceProvider->makeSemaphore(false);
+ semaphoreInfos[i].fDidCreate = true;
}
- this->insertSemaphore(semaphore.get());
-
- if (!info.fSignalSemaphores[i].isInitialized()) {
- info.fSignalSemaphores[i] = semaphore->backendSemaphore();
+ if (!semaphoreInfos[i].fSemaphore) {
+ semaphoreInfos[i].fDidCreate = false;
+ failedSemaphoreCreation = true;
+ }
+ }
+ if (!failedSemaphoreCreation) {
+ for (int i = 0; i < info.fNumSemaphores && !failedSemaphoreCreation; ++i) {
+ this->insertSemaphore(semaphoreInfos[i].fSemaphore.get());
}
}
}
- this->onFinishFlush(proxies, n, access, info, externalRequests);
+
+ // We always want to try flushing, so do that before checking if we failed semaphore creation.
+ if (!this->onFinishFlush(proxies, n, access, info, externalRequests) ||
+ failedSemaphoreCreation) {
+ // If we didn't do the flush or failed semaphore creations then none of the semaphores were
+ // submitted. Therefore the client can't wait on any of the semaphores. Additionally any
+ // semaphores we created here the client is not responsible for deleting so we must make
+ // sure they get deleted. We do this by changing the ownership from borrowed to owned.
+ for (int i = 0; i < info.fNumSemaphores; ++i) {
+ if (semaphoreInfos[i].fDidCreate) {
+ SkASSERT(semaphoreInfos[i].fSemaphore);
+ semaphoreInfos[i].fSemaphore->setIsOwned();
+ }
+ }
+ return GrSemaphoresSubmitted::kNo;
+ }
+
+ for (int i = 0; i < info.fNumSemaphores; ++i) {
+ if (!info.fSignalSemaphores[i].isInitialized()) {
+ SkASSERT(semaphoreInfos[i].fSemaphore);
+ info.fSignalSemaphores[i] = semaphoreInfos[i].fSemaphore->backendSemaphore();
+ }
+ }
+
return this->caps()->semaphoreSupport() ? GrSemaphoresSubmitted::kYes
: GrSemaphoresSubmitted::kNo;
}
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index c05c818..9bf6e7d 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -669,7 +669,7 @@
virtual bool onCopySurface(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
const SkIPoint& dstPoint) = 0;
- virtual void onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
+ virtual bool onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo&, const GrPrepareForExternalIORequests&) = 0;
#ifdef SK_ENABLE_DUMP_GPU
diff --git a/src/gpu/GrSemaphore.h b/src/gpu/GrSemaphore.h
index fdff863..77e5b79 100644
--- a/src/gpu/GrSemaphore.h
+++ b/src/gpu/GrSemaphore.h
@@ -26,6 +26,13 @@
// semaphores so we can set the client's GrBackendSemaphore object after we've created the
// internal semaphore.
virtual GrBackendSemaphore backendSemaphore() const = 0;
+
+private:
+ friend class GrGpu; // for setIsOwned
+ // This is only used in GrGpu to handle the case where we created a semaphore that was meant to
+ // be borrowed, but we failed to submit it. So we must go back and switch the semaphore to owned
+ // so that it gets deleted.
+ virtual void setIsOwned() = 0;
};
#endif
diff --git a/src/gpu/dawn/GrDawnGpu.cpp b/src/gpu/dawn/GrDawnGpu.cpp
index 950442a..fbb3404 100644
--- a/src/gpu/dawn/GrDawnGpu.cpp
+++ b/src/gpu/dawn/GrDawnGpu.cpp
@@ -464,9 +464,10 @@
fDevice.Tick();
}
-void GrDawnGpu::onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
+bool GrDawnGpu::onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo& info, const GrPrepareForExternalIORequests&) {
this->flush();
+ return true;
}
static wgpu::Texture get_dawn_texture_from_surface(GrSurface* src) {
diff --git a/src/gpu/dawn/GrDawnGpu.h b/src/gpu/dawn/GrDawnGpu.h
index 837ccbb..fb47209 100644
--- a/src/gpu/dawn/GrDawnGpu.h
+++ b/src/gpu/dawn/GrDawnGpu.h
@@ -155,7 +155,7 @@
bool onCopySurface(GrSurface* dst, GrSurface* src,
const SkIRect& srcRect, const SkIPoint& dstPoint) override;
- void onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
+ bool onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo& info, const GrPrepareForExternalIORequests&) override;
wgpu::Device fDevice;
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index d459a29..4315e17 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -3850,7 +3850,7 @@
return attribState;
}
-void GrGLGpu::onFinishFlush(GrSurfaceProxy*[], int, SkSurface::BackendSurfaceAccess access,
+bool GrGLGpu::onFinishFlush(GrSurfaceProxy*[], int, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo& info, const GrPrepareForExternalIORequests&) {
// If we inserted semaphores during the flush, we need to call GLFlush.
bool insertedSemaphore = info.fNumSemaphores > 0 && this->caps()->semaphoreSupport();
@@ -3884,6 +3884,7 @@
// See if any previously inserted finish procs are good to go.
this->checkFinishProcs();
}
+ return true;
}
void GrGLGpu::submit(GrOpsRenderPass* renderPass) {
@@ -3963,6 +3964,7 @@
std::unique_ptr<GrSemaphore> GrGLGpu::prepareTextureForCrossContextUsage(GrTexture* texture) {
// Set up a semaphore to be signaled once the data is ready, and flush GL
std::unique_ptr<GrSemaphore> semaphore = this->makeSemaphore(true);
+ SkASSERT(semaphore);
this->insertSemaphore(semaphore.get());
// We must call flush here to make sure the GrGLSync object gets created and sent to the gpu.
GL_CALL(Flush());
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index d3b8682..fb597f5 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -291,7 +291,7 @@
void flushBlendAndColorWrite(const GrXferProcessor::BlendInfo& blendInfo, const GrSwizzle&);
- void onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
+ bool onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo&, const GrPrepareForExternalIORequests&) override;
bool waitSync(GrGLsync, uint64_t timeout, bool flush);
diff --git a/src/gpu/gl/GrGLSemaphore.h b/src/gpu/gl/GrGLSemaphore.h
index 4c2b1b1..75880aa 100644
--- a/src/gpu/gl/GrGLSemaphore.h
+++ b/src/gpu/gl/GrGLSemaphore.h
@@ -43,6 +43,10 @@
private:
GrGLSemaphore(GrGLGpu* gpu, bool isOwned);
+ void setIsOwned() override {
+ fIsOwned = true;
+ }
+
GrGLGpu* fGpu;
GrGLsync fSync;
bool fIsOwned;
diff --git a/src/gpu/mock/GrMockGpu.h b/src/gpu/mock/GrMockGpu.h
index feda977..a04e6c0 100644
--- a/src/gpu/mock/GrMockGpu.h
+++ b/src/gpu/mock/GrMockGpu.h
@@ -126,11 +126,12 @@
void onResolveRenderTarget(GrRenderTarget* target, const SkIRect&, GrSurfaceOrigin,
ForExternalIO) override {}
- void onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
+ bool onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo& info, const GrPrepareForExternalIORequests&) override {
if (info.fFinishedProc) {
info.fFinishedProc(info.fFinishedContext);
}
+ return true;
}
GrStencilAttachment* createStencilAttachmentForRenderTarget(
diff --git a/src/gpu/mtl/GrMtlGpu.h b/src/gpu/mtl/GrMtlGpu.h
index e6a6acf..4ec4858 100644
--- a/src/gpu/mtl/GrMtlGpu.h
+++ b/src/gpu/mtl/GrMtlGpu.h
@@ -185,7 +185,7 @@
void resolveTexture(id<MTLTexture> colorTexture, id<MTLTexture> resolveTexture);
- void onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
+ bool onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo& info, const GrPrepareForExternalIORequests&) override;
// Function that uploads data onto textures with private storage mode (GPU access only).
diff --git a/src/gpu/mtl/GrMtlGpu.mm b/src/gpu/mtl/GrMtlGpu.mm
index c17da53..32e168f 100644
--- a/src/gpu/mtl/GrMtlGpu.mm
+++ b/src/gpu/mtl/GrMtlGpu.mm
@@ -185,7 +185,7 @@
}
}
-void GrMtlGpu::onFinishFlush(GrSurfaceProxy*[], int, SkSurface::BackendSurfaceAccess,
+bool GrMtlGpu::onFinishFlush(GrSurfaceProxy*[], int, SkSurface::BackendSurfaceAccess,
const GrFlushInfo& info, const GrPrepareForExternalIORequests&) {
bool forceSync = SkToBool(info.fFlags & kSyncCpu_GrFlushFlag) ||
(info.fFinishedProc && !this->mtlCaps().fenceSyncSupport());
@@ -211,6 +211,7 @@
}
this->submitCommandBuffer(kSkip_SyncQueue);
}
+ return true;
}
void GrMtlGpu::checkFinishProcs() {
diff --git a/src/gpu/mtl/GrMtlSemaphore.h b/src/gpu/mtl/GrMtlSemaphore.h
index d157b18..1991c9a 100644
--- a/src/gpu/mtl/GrMtlSemaphore.h
+++ b/src/gpu/mtl/GrMtlSemaphore.h
@@ -33,6 +33,8 @@
private:
GrMtlSemaphore(id<MTLEvent> event, uint64_t value) API_AVAILABLE(macos(10.14), ios(12.0));
+ void setIsOwned() override {}
+
id<MTLEvent> fEvent API_AVAILABLE(macos(10.14), ios(12.0));
uint64_t fValue;
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 427e172..9fbab33 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -1986,7 +1986,7 @@
barrier);
}
-void GrVkGpu::onFinishFlush(GrSurfaceProxy* proxies[], int n,
+bool GrVkGpu::onFinishFlush(GrSurfaceProxy* proxies[], int n,
SkSurface::BackendSurfaceAccess access, const GrFlushInfo& info,
const GrPrepareForExternalIORequests& externalRequests) {
SkASSERT(n >= 0);
@@ -2063,6 +2063,9 @@
} else {
this->submitCommandBuffer(kSkip_SyncQueue, info.fFinishedProc, info.fFinishedContext);
}
+ // TODO: We may fail to wait on fences or submit command buffers. We should return false here if
+ // we fail any part of the submission.
+ return true;
}
static int get_surface_sample_cnt(GrSurface* surf) {
diff --git a/src/gpu/vk/GrVkGpu.h b/src/gpu/vk/GrVkGpu.h
index c709bf2..b1c1d20 100644
--- a/src/gpu/vk/GrVkGpu.h
+++ b/src/gpu/vk/GrVkGpu.h
@@ -229,7 +229,7 @@
bool onCopySurface(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
const SkIPoint& dstPoint) override;
- void onFinishFlush(GrSurfaceProxy*[], int, SkSurface::BackendSurfaceAccess access,
+ bool onFinishFlush(GrSurfaceProxy*[], int, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo&, const GrPrepareForExternalIORequests&) override;
// Ends and submits the current command buffer to the queue and then creates a new command
diff --git a/src/gpu/vk/GrVkSemaphore.cpp b/src/gpu/vk/GrVkSemaphore.cpp
index e2d3e5f..191b6a3 100644
--- a/src/gpu/vk/GrVkSemaphore.cpp
+++ b/src/gpu/vk/GrVkSemaphore.cpp
@@ -23,7 +23,12 @@
createInfo.pNext = nullptr;
createInfo.flags = 0;
VkSemaphore semaphore = VK_NULL_HANDLE;
- GR_VK_CALL_ERRCHECK(gpu, CreateSemaphore(gpu->device(), &createInfo, nullptr, &semaphore));
+ VkResult result;
+ GR_VK_CALL_RESULT(gpu, result, CreateSemaphore(gpu->device(), &createInfo, nullptr,
+ &semaphore));
+ if (result != VK_SUCCESS) {
+ return nullptr;
+ }
return std::unique_ptr<GrVkSemaphore>(new GrVkSemaphore(gpu, semaphore, false, false, isOwned));
}
diff --git a/src/gpu/vk/GrVkSemaphore.h b/src/gpu/vk/GrVkSemaphore.h
index 8c083d3..b18db6d 100644
--- a/src/gpu/vk/GrVkSemaphore.h
+++ b/src/gpu/vk/GrVkSemaphore.h
@@ -59,6 +59,10 @@
fHasBeenSubmittedToQueueForWait = true;
}
+ void setIsOwned() {
+ fIsOwned = true;
+ }
+
#ifdef SK_TRACE_VK_RESOURCES
void dumpInfo() const override {
SkDebugf("GrVkSemaphore: %d (%d refs)\n", fSemaphore, this->getRefCnt());
@@ -81,6 +85,10 @@
GrVkSemaphore(GrVkGpu* gpu, VkSemaphore semaphore, bool prohibitSignal, bool prohibitWait,
bool isOwned);
+ void setIsOwned() override {
+ fResource->setIsOwned();
+ }
+
Resource* fResource;
GrVkGpu* fGpu;