From 01a9128a6f8e8a18cce11de2891f4b2052035f53 Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Thu, 26 Jul 2018 08:03:04 -0400 Subject: Maybe fix numStencilBits TSAN crash Bug: skia:7390 Change-Id: Ied9c92147324ddb1245df796cd201df35657bbe0 Reviewed-on: https://skia-review.googlesource.com/143304 Reviewed-by: Greg Daniel Commit-Queue: Robert Phillips --- src/gpu/GrDrawingManager.cpp | 21 +++++++++++++-------- src/gpu/GrOpList.cpp | 22 ++++++++++++++++++++++ src/gpu/GrResourceAllocator.cpp | 13 +++++++++---- src/gpu/GrResourceProvider.cpp | 7 ++++--- src/gpu/GrSurfaceProxy.cpp | 4 +++- src/gpu/GrSurfaceProxyPriv.h | 3 ++- 6 files changed, 53 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp index 8b0fb22f6b..645b3d17a0 100644 --- a/src/gpu/GrDrawingManager.cpp +++ b/src/gpu/GrDrawingManager.cpp @@ -103,6 +103,13 @@ void GrDrawingManager::freeGpuResources() { fSoftwarePathRenderer = nullptr; } +static void end_oplist_flush_if_not_unique(const sk_sp& opList) { + if (!opList->unique()) { + // TODO: Eventually this should be guaranteed unique: http://skbug.com/7111 + opList->endFlush(); + } +} + // MDB TODO: make use of the 'proxy' parameter. GrSemaphoresSubmitted GrDrawingManager::internalFlush(GrSurfaceProxy*, GrResourceCache::FlushType type, @@ -210,6 +217,11 @@ GrSemaphoresSubmitted GrDrawingManager::internalFlush(GrSurfaceProxy*, &error)) { if (GrResourceAllocator::AssignError::kFailedProxyInstantiation == error) { for (int i = startIndex; i < stopIndex; ++i) { + if (fOpLists[i] && !fOpLists[i]->isFullyInstantiated()) { + // If the backing surface wasn't allocated drop the entire opList. + end_oplist_flush_if_not_unique(fOpLists[i]); // http://skbug.com/7111 + fOpLists[i] = nullptr; + } if (fOpLists[i]) { fOpLists[i]->purgeOpsWithUninstantiatedProxies(); } @@ -259,13 +271,6 @@ GrSemaphoresSubmitted GrDrawingManager::internalFlush(GrSurfaceProxy*, return result; } -static void end_oplist_flush_if_not_unique(const sk_sp& opList) { - if (!opList->unique()) { - // TODO: Eventually this should be guaranteed unique: http://skbug.com/7111 - opList->endFlush(); - } -} - bool GrDrawingManager::executeOpLists(int startIndex, int stopIndex, GrOpFlushState* flushState) { SkASSERT(startIndex <= stopIndex && stopIndex <= fOpLists.count()); @@ -286,7 +291,7 @@ bool GrDrawingManager::executeOpLists(int startIndex, int stopIndex, GrOpFlushSt } if (resourceProvider->explicitlyAllocateGPUResources()) { - if (!fOpLists[i]->isInstantiated()) { + if (!fOpLists[i]->isFullyInstantiated()) { // If the backing surface wasn't allocated drop the draw of the entire opList. end_oplist_flush_if_not_unique(fOpLists[i]); // http://skbug.com/7111 fOpLists[i] = nullptr; diff --git a/src/gpu/GrOpList.cpp b/src/gpu/GrOpList.cpp index c724ea9ba4..c9b61bedab 100644 --- a/src/gpu/GrOpList.cpp +++ b/src/gpu/GrOpList.cpp @@ -10,6 +10,7 @@ #include "GrContext.h" #include "GrDeferredProxyUploader.h" #include "GrMemoryPool.h" +#include "GrRenderTargetPriv.h" #include "GrSurfaceProxy.h" #include "GrTextureProxyPriv.h" @@ -138,6 +139,27 @@ bool GrOpList::isInstantiated() const { return fTarget.get()->priv().isInstantiated(); } +bool GrOpList::isFullyInstantiated() const { + if (!this->isInstantiated()) { + return false; + } + + GrSurfaceProxy* proxy = fTarget.get(); + bool needsStencil = proxy->asRenderTargetProxy() + ? proxy->asRenderTargetProxy()->needsStencil() + : false; + + if (needsStencil) { + GrRenderTarget* rt = proxy->priv().peekRenderTarget(); + + if (!rt->renderTargetPriv().getStencilAttachment()) { + return false; + } + } + + return true; +} + #ifdef SK_DEBUG static const char* op_to_name(GrLoadOp op) { return GrLoadOp::kLoad == op ? "load" : GrLoadOp::kClear == op ? "clear" : "discard"; diff --git a/src/gpu/GrResourceAllocator.cpp b/src/gpu/GrResourceAllocator.cpp index d253ca2c1a..199acefe7a 100644 --- a/src/gpu/GrResourceAllocator.cpp +++ b/src/gpu/GrResourceAllocator.cpp @@ -180,7 +180,10 @@ sk_sp GrResourceAllocator::findSurfaceFor(const GrSurfaceProxy* proxy surface->resourcePriv().makeBudgeted(); } - GrSurfaceProxyPriv::AttachStencilIfNeeded(fResourceProvider, surface.get(), needsStencil); + if (!GrSurfaceProxyPriv::AttachStencilIfNeeded(fResourceProvider, surface.get(), + needsStencil)) { + return nullptr; + } return surface; } @@ -248,9 +251,11 @@ bool GrResourceAllocator::assign(int* startIndex, int* stopIndex, : false; if (cur->proxy()->priv().isInstantiated()) { - GrSurfaceProxyPriv::AttachStencilIfNeeded(fResourceProvider, - cur->proxy()->priv().peekSurface(), - needsStencil); + if (!GrSurfaceProxyPriv::AttachStencilIfNeeded(fResourceProvider, + cur->proxy()->priv().peekSurface(), + needsStencil)) { + *outError = AssignError::kFailedProxyInstantiation; + } fActiveIntvls.insertByIncreasingEnd(cur); diff --git a/src/gpu/GrResourceProvider.cpp b/src/gpu/GrResourceProvider.cpp index 69a54dd862..3450b008f2 100644 --- a/src/gpu/GrResourceProvider.cpp +++ b/src/gpu/GrResourceProvider.cpp @@ -402,10 +402,11 @@ bool GrResourceProvider::attachStencilAttachment(GrRenderTarget* rt) { if (!stencil) { // Need to try and create a new stencil stencil.reset(this->gpu()->createStencilAttachmentForRenderTarget(rt, width, height)); - if (stencil) { - this->assignUniqueKeyToResource(sbKey, stencil.get()); - SkDEBUGCODE(newStencil = true;) + if (!stencil) { + return false; } + this->assignUniqueKeyToResource(sbKey, stencil.get()); + SkDEBUGCODE(newStencil = true;) } if (rt->renderTargetPriv().attachStencilAttachment(std::move(stencil))) { #ifdef SK_DEBUG diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp index 9e79ead801..14f535e0e9 100644 --- a/src/gpu/GrSurfaceProxy.cpp +++ b/src/gpu/GrSurfaceProxy.cpp @@ -416,7 +416,9 @@ bool GrSurfaceProxyPriv::doLazyInstantiation(GrResourceProvider* resourceProvide ? fProxy->asRenderTargetProxy()->needsStencil() : false; - GrSurfaceProxyPriv::AttachStencilIfNeeded(resourceProvider, surface.get(), needsStencil); + if (!GrSurfaceProxyPriv::AttachStencilIfNeeded(resourceProvider, surface.get(), needsStencil)) { + return false; + } this->assign(std::move(surface)); return true; diff --git a/src/gpu/GrSurfaceProxyPriv.h b/src/gpu/GrSurfaceProxyPriv.h index a5113f22d5..47e99b74c0 100644 --- a/src/gpu/GrSurfaceProxyPriv.h +++ b/src/gpu/GrSurfaceProxyPriv.h @@ -86,7 +86,8 @@ public: GrSurfaceProxy::LazyInstantiationType::kUninstantiate == lazyInstantiationType(); } - static bool AttachStencilIfNeeded(GrResourceProvider*, GrSurface*, bool needsStencil); + static bool SK_WARN_UNUSED_RESULT AttachStencilIfNeeded(GrResourceProvider*, GrSurface*, + bool needsStencil); private: explicit GrSurfaceProxyPriv(GrSurfaceProxy* proxy) : fProxy(proxy) {} -- cgit v1.2.3