From b2c5dae65df952999f0a12b9df80bc1433ffa19a Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Mon, 4 Mar 2019 10:25:17 -0500 Subject: [PATCH] Simplify promise image lazy instantiation callbacks. Now that we never re-fulfill a promise image we no longer need to deinstantiate promise image proxies. They now can use kSingleUse callback semantics. This was the only usage of the kDeinstantiate lazy callback type so it is removed. The DeinstantiateProxyTracker is also no longer required and is removed. The GrTexture idle callback mechanism now uses GrReleaseProcHelper, which has been extended to support chaining multiple callbacks together and an abandon() method that aborts calling the callback in the destructor. It has been renamed GrRefCntedCallback to reflect its more general usage. Bug: skia:8800 Change-Id: I857c9eec57fdf706631a266ec8bea682d6657a7c Reviewed-on: https://skia-review.googlesource.com/c/skia/+/196500 Commit-Queue: Brian Salomon Reviewed-by: Robert Phillips --- gn/gpu.gni | 2 - include/gpu/GrSurface.h | 8 +- include/gpu/GrTexture.h | 15 +- include/private/GrSurfaceProxy.h | 2 - include/private/GrTypesPriv.h | 40 ++++- src/gpu/GrAHardwareBufferImageGenerator.cpp | 4 +- src/gpu/GrBackendTextureImageGenerator.cpp | 8 +- src/gpu/GrBackendTextureImageGenerator.h | 2 +- src/gpu/GrDeinstantiateProxyTracker.cpp | 32 ---- src/gpu/GrDeinstantiateProxyTracker.h | 29 ---- src/gpu/GrDrawingManager.cpp | 4 +- src/gpu/GrOpFlushState.h | 6 - src/gpu/GrProxyProvider.cpp | 14 +- src/gpu/GrResourceAllocator.cpp | 13 +- src/gpu/GrResourceAllocator.h | 6 +- src/gpu/GrSurfaceProxyPriv.h | 6 - src/gpu/gl/GrGLRenderTarget.h | 2 +- src/gpu/gl/GrGLTexture.h | 18 +-- src/gpu/gl/GrGLTextureRenderTarget.h | 2 +- src/gpu/mock/GrMockTexture.h | 32 +--- src/gpu/mtl/GrMtlRenderTarget.h | 2 +- src/gpu/mtl/GrMtlTexture.h | 19 +-- src/gpu/mtl/GrMtlTextureRenderTarget.h | 2 +- src/gpu/vk/GrVkImage.cpp | 27 ++-- src/gpu/vk/GrVkImage.h | 11 +- src/gpu/vk/GrVkRenderTarget.h | 2 +- src/gpu/vk/GrVkTexture.cpp | 27 ++-- src/gpu/vk/GrVkTexture.h | 12 +- src/gpu/vk/GrVkTextureRenderTarget.h | 2 +- src/image/SkImage_GpuBase.cpp | 162 ++++++-------------- tests/GrSurfaceTest.cpp | 35 +---- tests/LazyProxyTest.cpp | 28 +--- tests/ResourceAllocatorTest.cpp | 28 +--- 33 files changed, 174 insertions(+), 428 deletions(-) delete mode 100644 src/gpu/GrDeinstantiateProxyTracker.cpp delete mode 100644 src/gpu/GrDeinstantiateProxyTracker.h diff --git a/gn/gpu.gni b/gn/gpu.gni index 4904f420ca..bf6be625de 100644 --- a/gn/gpu.gni +++ b/gn/gpu.gni @@ -88,8 +88,6 @@ skia_gpu_sources = [ "$_src/gpu/GrDefaultGeoProcFactory.h", "$_src/gpu/GrDeferredProxyUploader.h", "$_src/gpu/GrDeferredUpload.h", - "$_src/gpu/GrDeinstantiateProxyTracker.cpp", - "$_src/gpu/GrDeinstantiateProxyTracker.h", "$_src/gpu/GrDistanceFieldGenFromVector.cpp", "$_src/gpu/GrDistanceFieldGenFromVector.h", "$_src/gpu/GrDrawingManager.cpp", diff --git a/include/gpu/GrSurface.h b/include/gpu/GrSurface.h index c4ff818348..40ce8a1669 100644 --- a/include/gpu/GrSurface.h +++ b/include/gpu/GrSurface.h @@ -45,7 +45,7 @@ public: virtual GrBackendFormat backendFormat() const = 0; - void setRelease(sk_sp releaseHelper) { + void setRelease(sk_sp releaseHelper) { this->onSetRelease(releaseHelper); fReleaseHelper = std::move(releaseHelper); } @@ -55,7 +55,7 @@ public: typedef void* ReleaseCtx; typedef void (*ReleaseProc)(ReleaseCtx); void setRelease(ReleaseProc proc, ReleaseCtx ctx) { - sk_sp helper(new GrReleaseProcHelper(proc, ctx)); + sk_sp helper(new GrRefCntedCallback(proc, ctx)); this->setRelease(std::move(helper)); } @@ -132,7 +132,7 @@ protected: private: const char* getResourceType() const override { return "Surface"; } - virtual void onSetRelease(sk_sp releaseHelper) = 0; + virtual void onSetRelease(sk_sp releaseHelper) = 0; void invokeReleaseProc() { // Depending on the ref count of fReleaseHelper this may or may not actually trigger the // ReleaseProc to be called. @@ -143,7 +143,7 @@ private: int fWidth; int fHeight; GrInternalSurfaceFlags fSurfaceFlags; - sk_sp fReleaseHelper; + sk_sp fReleaseHelper; typedef GrGpuResource INHERITED; }; diff --git a/include/gpu/GrTexture.h b/include/gpu/GrTexture.h index 542e785b3d..4a7935f680 100644 --- a/include/gpu/GrTexture.h +++ b/include/gpu/GrTexture.h @@ -57,9 +57,10 @@ public: * will always be called before the texture is destroyed, even in unusual shutdown scenarios * (e.g. GrContext::abandonContext()). */ - using IdleProc = void(void*); - virtual void setIdleProc(IdleProc, void* context) = 0; - virtual void* idleContext() const = 0; + virtual void addIdleProc(sk_sp callback) { + callback->addChild(std::move(fIdleCallback)); + fIdleCallback = std::move(callback); + } /** Access methods that are only to be used within Skia code. */ inline GrTexturePriv texturePriv(); @@ -70,7 +71,15 @@ protected: virtual bool onStealBackendTexture(GrBackendTexture*, SkImage::BackendTextureReleaseProc*) = 0; + sk_sp fIdleCallback; + + void willRemoveLastRefOrPendingIO() override { + // We're about to be idle in the resource cache. Do our part to trigger the idle callback. + fIdleCallback.reset(); + } + private: + void computeScratchKey(GrScratchKey*) const override; size_t onGpuMemorySize() const override; void markMipMapsDirty(); diff --git a/include/private/GrSurfaceProxy.h b/include/private/GrSurfaceProxy.h index c9eec6c607..7c572bb1db 100644 --- a/include/private/GrSurfaceProxy.h +++ b/include/private/GrSurfaceProxy.h @@ -208,8 +208,6 @@ public: enum class LazyInstantiationType { kSingleUse, // Instantiation callback is allowed to be called only once. kMultipleUse, // Instantiation callback can be called multiple times. - kDeinstantiate, // Instantiation callback can be called multiple times, - // but we will deinstantiate the proxy after every flush. }; enum class LazyState { diff --git a/include/private/GrTypesPriv.h b/include/private/GrTypesPriv.h index 63ceb4d3ae..d6ea362762 100644 --- a/include/private/GrTypesPriv.h +++ b/include/private/GrTypesPriv.h @@ -1554,20 +1554,44 @@ static inline GrPixelConfig GrColorTypeToPixelConfig(GrColorType config, return kUnknown_GrPixelConfig; } -class GrReleaseProcHelper : public SkRefCnt { +/** + * Ref-counted object that calls a callback from its destructor. These can be chained together. Any + * owner can cancel calling the callback via abandon(). + */ +class GrRefCntedCallback : public SkRefCnt { public: - // These match the definitions in SkImage, from whence they came - typedef void* ReleaseCtx; - typedef void (*ReleaseProc)(ReleaseCtx); + using Context = void*; + using Callback = void (*)(Context); - GrReleaseProcHelper(ReleaseProc proc, ReleaseCtx ctx) : fReleaseProc(proc), fReleaseCtx(ctx) { + GrRefCntedCallback(Callback proc, Context ctx) : fReleaseProc(proc), fReleaseCtx(ctx) { SkASSERT(proc); } - ~GrReleaseProcHelper() override { fReleaseProc(fReleaseCtx); } + ~GrRefCntedCallback() override { fReleaseProc ? fReleaseProc(fReleaseCtx) : void(); } + + /** + * After abandon is called the release proc will no longer be called in the destructor. This + * does not recurse on child release procs or unref them. + */ + void abandon() { + fReleaseProc = nullptr; + fReleaseCtx = nullptr; + } + + /** Adds another GrRefCntedCallback that this will unref in its destructor. */ + void addChild(sk_sp next) { + if (!fNext) { + fNext = std::move(next); + return; + } + fNext->addChild(std::move(next)); + } + + Context context() const { return fReleaseCtx; } private: - ReleaseProc fReleaseProc; - ReleaseCtx fReleaseCtx; + sk_sp fNext; + Callback fReleaseProc; + Context fReleaseCtx; }; #endif diff --git a/src/gpu/GrAHardwareBufferImageGenerator.cpp b/src/gpu/GrAHardwareBufferImageGenerator.cpp index 9220a1921e..205595526f 100644 --- a/src/gpu/GrAHardwareBufferImageGenerator.cpp +++ b/src/gpu/GrAHardwareBufferImageGenerator.cpp @@ -179,9 +179,7 @@ sk_sp GrAHardwareBufferImageGenerator::makeProxy(GrRecordingCont } if (deleteImageProc) { - sk_sp releaseProcHelper( - new GrReleaseProcHelper(deleteImageProc, deleteImageCtx)); - tex->setRelease(releaseProcHelper); + tex->setRelease(deleteImageProc, deleteImageCtx); } return tex; diff --git a/src/gpu/GrBackendTextureImageGenerator.cpp b/src/gpu/GrBackendTextureImageGenerator.cpp index 0c41cc1567..e184e8e1d3 100644 --- a/src/gpu/GrBackendTextureImageGenerator.cpp +++ b/src/gpu/GrBackendTextureImageGenerator.cpp @@ -106,7 +106,7 @@ sk_sp GrBackendTextureImageGenerator::onGenerateTexture( auto proxyProvider = context->priv().proxyProvider(); fBorrowingMutex.acquire(); - sk_sp releaseProcHelper; + sk_sp releaseProcHelper; if (SK_InvalidGenID != fRefHelper->fBorrowingContextID) { if (fRefHelper->fBorrowingContextID != context->priv().contextID()) { fBorrowingMutex.release(); @@ -119,10 +119,10 @@ sk_sp GrBackendTextureImageGenerator::onGenerateTexture( } else { SkASSERT(!fRefHelper->fBorrowingContextReleaseProc); // The ref we add to fRefHelper here will be passed into and owned by the - // GrReleaseProcHelper. + // GrRefCntedCallback. fRefHelper->ref(); - releaseProcHelper.reset(new GrReleaseProcHelper(ReleaseRefHelper_TextureReleaseProc, - fRefHelper)); + releaseProcHelper.reset( + new GrRefCntedCallback(ReleaseRefHelper_TextureReleaseProc, fRefHelper)); fRefHelper->fBorrowingContextReleaseProc = releaseProcHelper.get(); } fRefHelper->fBorrowingContextID = context->priv().contextID(); diff --git a/src/gpu/GrBackendTextureImageGenerator.h b/src/gpu/GrBackendTextureImageGenerator.h index 74dd3eac7c..d461176d6a 100644 --- a/src/gpu/GrBackendTextureImageGenerator.h +++ b/src/gpu/GrBackendTextureImageGenerator.h @@ -73,7 +73,7 @@ private: // texture are finished on the borrowing context before we open this back up to other // contexts. In general a ref to this release proc is owned by all proxies and gpu uses of // the backend texture. - GrReleaseProcHelper* fBorrowingContextReleaseProc; + GrRefCntedCallback* fBorrowingContextReleaseProc; uint32_t fBorrowingContextID; }; diff --git a/src/gpu/GrDeinstantiateProxyTracker.cpp b/src/gpu/GrDeinstantiateProxyTracker.cpp deleted file mode 100644 index 9870617f24..0000000000 --- a/src/gpu/GrDeinstantiateProxyTracker.cpp +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright 2018 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#include "GrDeinstantiateProxyTracker.h" - -#include "GrSurfaceProxy.h" -#include "GrSurfaceProxyPriv.h" - -void GrDeinstantiateProxyTracker::addProxy(GrSurfaceProxy* proxy) { -#ifdef SK_DEBUG - using LazyType = GrSurfaceProxy::LazyInstantiationType; - SkASSERT(LazyType::kDeinstantiate == proxy->priv().lazyInstantiationType()); - for (int i = 0; i < fProxies.count(); ++i) { - SkASSERT(proxy != fProxies[i].get()); - } -#endif - fProxies.push_back(sk_ref_sp(proxy)); -} - -void GrDeinstantiateProxyTracker::deinstantiateAllProxies() { - for (int i = 0; i < fProxies.count(); ++i) { - GrSurfaceProxy* proxy = fProxies[i].get(); - SkASSERT(proxy->priv().isSafeToDeinstantiate()); - proxy->deinstantiate(); - } - - fProxies.reset(); -} diff --git a/src/gpu/GrDeinstantiateProxyTracker.h b/src/gpu/GrDeinstantiateProxyTracker.h deleted file mode 100644 index 2555ab18c0..0000000000 --- a/src/gpu/GrDeinstantiateProxyTracker.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright 2018 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#ifndef GrDeinstantiateProxyTracker_DEFINED -#define GrDeinstantiateProxyTracker_DEFINED - -#include "GrSurfaceProxy.h" -#include "SkTArray.h" - -class GrDeinstantiateProxyTracker { -public: - GrDeinstantiateProxyTracker() {} - - // Adds a proxy which will be deinstantiated at the end of flush. The same proxy may not be - // added multiple times. - void addProxy(GrSurfaceProxy* proxy); - - // Loops through all tracked proxies and deinstantiates them. - void deinstantiateAllProxies(); - -private: - SkTArray> fProxies; -}; - -#endif diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp index aa22cef4ad..ae49d0a215 100644 --- a/src/gpu/GrDrawingManager.cpp +++ b/src/gpu/GrDrawingManager.cpp @@ -292,7 +292,7 @@ GrSemaphoresSubmitted GrDrawingManager::flush(GrSurfaceProxy* proxy, bool flushed = false; { - GrResourceAllocator alloc(resourceProvider, flushState.deinstantiateProxyTracker()); + GrResourceAllocator alloc(resourceProvider); for (int i = 0; i < fDAG.numOpLists(); ++i) { if (fDAG.opList(i)) { fDAG.opList(i)->gatherProxyIntervals(&alloc); @@ -343,8 +343,6 @@ GrSemaphoresSubmitted GrDrawingManager::flush(GrSurfaceProxy* proxy, GrSemaphoresSubmitted result = gpu->finishFlush(proxy, access, flags, numSemaphores, backendSemaphores); - flushState.deinstantiateProxyTracker()->deinstantiateAllProxies(); - // Give the cache a chance to purge resources that become purgeable due to flushing. if (flushed) { resourceCache->purgeAsNeeded(); diff --git a/src/gpu/GrOpFlushState.h b/src/gpu/GrOpFlushState.h index 07983d421d..bc109200a0 100644 --- a/src/gpu/GrOpFlushState.h +++ b/src/gpu/GrOpFlushState.h @@ -12,7 +12,6 @@ #include "GrAppliedClip.h" #include "GrBufferAllocPool.h" #include "GrDeferredUpload.h" -#include "GrDeinstantiateProxyTracker.h" #include "GrRenderTargetProxy.h" #include "SkArenaAlloc.h" #include "SkArenaAllocList.h" @@ -109,8 +108,6 @@ public: // permissible). GrAtlasManager* atlasManager() const final; - GrDeinstantiateProxyTracker* deinstantiateProxyTracker() { return &fDeinstantiateProxyTracker; } - private: /** GrMeshDrawOp::Target override. */ SkArenaAlloc* allocator() override { return &fArena; } @@ -164,9 +161,6 @@ private: // Variables that are used to track where we are in lists as ops are executed SkArenaAllocList::Iter fCurrDraw; SkArenaAllocList::Iter fCurrUpload; - - // Used to track the proxies that need to be deinstantiated after we finish a flush - GrDeinstantiateProxyTracker fDeinstantiateProxyTracker; }; #endif diff --git a/src/gpu/GrProxyProvider.cpp b/src/gpu/GrProxyProvider.cpp index 64b3e9556b..2540cbb1aa 100644 --- a/src/gpu/GrProxyProvider.cpp +++ b/src/gpu/GrProxyProvider.cpp @@ -488,11 +488,8 @@ sk_sp GrProxyProvider::wrapBackendTexture(const GrBackendTexture return nullptr; } - sk_sp releaseHelper; if (releaseProc) { - releaseHelper.reset(new GrReleaseProcHelper(releaseProc, releaseCtx)); - // This gives the texture a ref on the releaseHelper - tex->setRelease(std::move(releaseHelper)); + tex->setRelease(releaseProc, releaseCtx); } SkASSERT(!tex->asRenderTarget()); // Strictly a GrTexture @@ -529,11 +526,8 @@ sk_sp GrProxyProvider::wrapRenderableBackendTexture( return nullptr; } - sk_sp releaseHelper; if (releaseProc) { - releaseHelper.reset(new GrReleaseProcHelper(releaseProc, releaseCtx)); - // This gives the texture a ref on the releaseHelper - tex->setRelease(std::move(releaseHelper)); + tex->setRelease(releaseProc, releaseCtx); } SkASSERT(tex->asRenderTarget()); // A GrTextureRenderTarget @@ -563,9 +557,9 @@ sk_sp GrProxyProvider::wrapBackendRenderTarget( return nullptr; } - sk_sp releaseHelper; + sk_sp releaseHelper; if (releaseProc) { - releaseHelper.reset(new GrReleaseProcHelper(releaseProc, releaseCtx)); + releaseHelper.reset(new GrRefCntedCallback(releaseProc, releaseCtx)); // This gives the render target a ref on the releaseHelper rt->setRelease(std::move(releaseHelper)); } diff --git a/src/gpu/GrResourceAllocator.cpp b/src/gpu/GrResourceAllocator.cpp index 0d3c34e0ea..81c27096bd 100644 --- a/src/gpu/GrResourceAllocator.cpp +++ b/src/gpu/GrResourceAllocator.cpp @@ -7,7 +7,6 @@ #include "GrResourceAllocator.h" -#include "GrDeinstantiateProxyTracker.h" #include "GrGpuResourcePriv.h" #include "GrOpList.h" #include "GrRenderTargetProxy.h" @@ -107,12 +106,7 @@ void GrResourceAllocator::addInterval(GrSurfaceProxy* proxy, unsigned int start, if (proxy->readOnly() || !fResourceProvider->explicitlyAllocateGPUResources()) { // FIXME: remove this once we can do the lazy instantiation from assign instead. if (GrSurfaceProxy::LazyState::kNot != proxy->lazyInstantiationState()) { - if (proxy->priv().doLazyInstantiation(fResourceProvider)) { - if (proxy->priv().lazyInstantiationType() == - GrSurfaceProxy::LazyInstantiationType::kDeinstantiate) { - fDeinstantiateTracker->addProxy(proxy); - } - } + proxy->priv().doLazyInstantiation(fResourceProvider); } } } @@ -375,11 +369,6 @@ bool GrResourceAllocator::assign(int* startIndex, int* stopIndex, AssignError* o if (GrSurfaceProxy::LazyState::kNot != cur->proxy()->lazyInstantiationState()) { if (!cur->proxy()->priv().doLazyInstantiation(fResourceProvider)) { *outError = AssignError::kFailedProxyInstantiation; - } else { - if (GrSurfaceProxy::LazyInstantiationType::kDeinstantiate == - cur->proxy()->priv().lazyInstantiationType()) { - fDeinstantiateTracker->addProxy(cur->proxy()); - } } } else if (sk_sp surface = this->findSurfaceFor(cur->proxy(), needsStencil)) { // TODO: make getUniqueKey virtual on GrSurfaceProxy diff --git a/src/gpu/GrResourceAllocator.h b/src/gpu/GrResourceAllocator.h index ea1250f537..cb366e6777 100644 --- a/src/gpu/GrResourceAllocator.h +++ b/src/gpu/GrResourceAllocator.h @@ -16,7 +16,6 @@ #include "SkTDynamicHash.h" #include "SkTMultiMap.h" -class GrDeinstantiateProxyTracker; class GrResourceProvider; // Print out explicit allocation information @@ -42,8 +41,8 @@ class GrResourceProvider; */ class GrResourceAllocator { public: - GrResourceAllocator(GrResourceProvider* resourceProvider, GrDeinstantiateProxyTracker* tracker) - : fResourceProvider(resourceProvider), fDeinstantiateTracker(tracker) {} + GrResourceAllocator(GrResourceProvider* resourceProvider) + : fResourceProvider(resourceProvider) {} ~GrResourceAllocator(); @@ -212,7 +211,6 @@ private: static const int kInitialArenaSize = 128 * sizeof(Interval); GrResourceProvider* fResourceProvider; - GrDeinstantiateProxyTracker* fDeinstantiateTracker; FreePoolMultiMap fFreePool; // Recently created/used GrSurfaces IntvlHash fIntvlHash; // All the intervals, hashed by proxyID diff --git a/src/gpu/GrSurfaceProxyPriv.h b/src/gpu/GrSurfaceProxyPriv.h index a9076b89de..8fe7f8329f 100644 --- a/src/gpu/GrSurfaceProxyPriv.h +++ b/src/gpu/GrSurfaceProxyPriv.h @@ -60,12 +60,6 @@ public: return fProxy->fLazyInstantiationType; } - bool isSafeToDeinstantiate() const { - return SkToBool(fProxy->fTarget) && - SkToBool(fProxy->fLazyInstantiateCallback) && - GrSurfaceProxy::LazyInstantiationType::kDeinstantiate == lazyInstantiationType(); - } - static bool SK_WARN_UNUSED_RESULT AttachStencilIfNeeded(GrResourceProvider*, GrSurface*, bool needsStencil); diff --git a/src/gpu/gl/GrGLRenderTarget.h b/src/gpu/gl/GrGLRenderTarget.h index caedf081dd..1e7c4778af 100644 --- a/src/gpu/gl/GrGLRenderTarget.h +++ b/src/gpu/gl/GrGLRenderTarget.h @@ -95,7 +95,7 @@ private: size_t onGpuMemorySize() const override; - void onSetRelease(sk_sp releaseHelper) override {} + void onSetRelease(sk_sp releaseHelper) override {} int msaaSamples() const; // The number total number of samples, including both MSAA and resolve texture samples. diff --git a/src/gpu/gl/GrGLTexture.h b/src/gpu/gl/GrGLTexture.h index 6c22b4d050..423a763e2e 100644 --- a/src/gpu/gl/GrGLTexture.h +++ b/src/gpu/gl/GrGLTexture.h @@ -75,12 +75,6 @@ public: fNonSamplerParams.invalidate(); } - void setIdleProc(IdleProc proc, void* context) override { - fIdleProc = proc; - fIdleProcContext = context; - } - void* idleContext() const override { return fIdleProcContext; } - // These functions are used to track the texture parameters associated with the texture. GrGpu::ResetTimestamp getCachedParamsTimestamp() const { return fParamsTimestamp; } const SamplerParams& getCachedSamplerParams() const { return fSamplerParams; } @@ -124,21 +118,11 @@ protected: bool onStealBackendTexture(GrBackendTexture*, SkImage::BackendTextureReleaseProc*) override; private: - void onSetRelease(sk_sp releaseHelper) override {} - - void willRemoveLastRefOrPendingIO() override { - if (fIdleProc) { - fIdleProc(fIdleProcContext); - fIdleProc = nullptr; - fIdleProcContext = nullptr; - } - } + void onSetRelease(sk_sp releaseHelper) override {} SamplerParams fSamplerParams; NonSamplerParams fNonSamplerParams; GrGpu::ResetTimestamp fParamsTimestamp; - IdleProc* fIdleProc = nullptr; - void* fIdleProcContext = nullptr; GrGLuint fID; GrGLenum fFormat; GrBackendObjectOwnership fTextureIDOwnership; diff --git a/src/gpu/gl/GrGLTextureRenderTarget.h b/src/gpu/gl/GrGLTextureRenderTarget.h index 12a8ea08d2..a3b18419a7 100644 --- a/src/gpu/gl/GrGLTextureRenderTarget.h +++ b/src/gpu/gl/GrGLTextureRenderTarget.h @@ -67,7 +67,7 @@ private: size_t onGpuMemorySize() const override; - void onSetRelease(sk_sp releaseHelper) override {} + void onSetRelease(sk_sp releaseHelper) override {} }; #ifdef SK_BUILD_FOR_WIN diff --git a/src/gpu/mock/GrMockTexture.h b/src/gpu/mock/GrMockTexture.h index f1d977c9d4..7beb66c17c 100644 --- a/src/gpu/mock/GrMockTexture.h +++ b/src/gpu/mock/GrMockTexture.h @@ -44,12 +44,6 @@ public: void textureParamsModified() override {} - void setIdleProc(IdleProc proc, void* context) override { - fIdleProc = proc; - fIdleProcContext = context; - } - void* idleContext() const override { return fIdleProcContext; } - protected: // constructor for subclasses GrMockTexture(GrMockGpu* gpu, const GrSurfaceDesc& desc, GrMipMapsStatus mipMapsStatus, @@ -70,23 +64,10 @@ protected: return false; } - // protected so that GrMockTextureRenderTarget can call this to avoid "inheritance via - // dominance" warning. - void willRemoveLastRefOrPendingIO() override { - if (fIdleProc) { - fIdleProc(fIdleProcContext); - fIdleProc = nullptr; - fIdleProcContext = nullptr; - } - } - private: - void onSetRelease(sk_sp releaseHelper) override {} + void onSetRelease(sk_sp releaseHelper) override {} GrMockTextureInfo fInfo; - sk_sp fReleaseHelper; - IdleProc* fIdleProc = nullptr; - void* fIdleProcContext = nullptr; typedef GrTexture INHERITED; }; @@ -139,7 +120,7 @@ protected: : GrSurface(gpu, desc), INHERITED(gpu, desc), fInfo(info) {} private: - void onSetRelease(sk_sp releaseHelper) override {} + void onSetRelease(sk_sp releaseHelper) override {} GrMockRenderTargetInfo fInfo; @@ -177,8 +158,12 @@ public: return GrMockTexture::backendFormat(); } +protected: + // This avoids an inherits via dominance warning on MSVC. + void willRemoveLastRefOrPendingIO() override { GrTexture::willRemoveLastRefOrPendingIO(); } + private: - void onSetRelease(sk_sp releaseHelper) override {} + void onSetRelease(sk_sp releaseHelper) override {} void onAbandon() override { GrRenderTarget::onAbandon(); @@ -190,9 +175,6 @@ private: GrMockTexture::onRelease(); } - // We implement this to avoid the inheritance via dominance warning. - void willRemoveLastRefOrPendingIO() override { GrMockTexture::willRemoveLastRefOrPendingIO(); } - size_t onGpuMemorySize() const override { int numColorSamples = this->numColorSamples(); if (numColorSamples > 1) { diff --git a/src/gpu/mtl/GrMtlRenderTarget.h b/src/gpu/mtl/GrMtlRenderTarget.h index c6a13aac9d..282a443eb8 100644 --- a/src/gpu/mtl/GrMtlRenderTarget.h +++ b/src/gpu/mtl/GrMtlRenderTarget.h @@ -81,7 +81,7 @@ private: bool completeStencilAttachment() override; - void onSetRelease(sk_sp releaseHelper) override {} + void onSetRelease(sk_sp releaseHelper) override {} typedef GrRenderTarget INHERITED; }; diff --git a/src/gpu/mtl/GrMtlTexture.h b/src/gpu/mtl/GrMtlTexture.h index ae283bda1c..9134e7d860 100644 --- a/src/gpu/mtl/GrMtlTexture.h +++ b/src/gpu/mtl/GrMtlTexture.h @@ -36,12 +36,6 @@ public: bool reallocForMipmap(GrMtlGpu* gpu, uint32_t mipLevels); - void setIdleProc(IdleProc proc, void* context) override { - fIdleProc = proc; - fIdleProcContext = context; - } - void* idleContext() const override { return fIdleProcContext; } - protected: GrMtlTexture(GrMtlGpu*, const GrSurfaceDesc&, id, GrMipMapsStatus); @@ -66,15 +60,7 @@ private: // Since all MTLResources are inherently ref counted, we can call the Release proc when we // delete the GrMtlTexture without worry of the MTLTexture getting deleted before it is done on // the GPU. Thus we do nothing special here with the releaseHelper. - void onSetRelease(sk_sp releaseHelper) override {} - - void willRemoveLastRefOrPendingIO() override { - if (fIdleProc) { - fIdleProc(fIdleProcContext); - fIdleProc = nullptr; - fIdleProcContext = nullptr; - } - } + void onSetRelease(sk_sp releaseHelper) override {} GrMtlTexture(GrMtlGpu*, SkBudgeted, const GrSurfaceDesc&, id, GrMipMapsStatus); @@ -83,9 +69,6 @@ private: GrWrapCacheable, GrIOType); id fTexture; - sk_sp fReleaseHelper; - IdleProc* fIdleProc = nullptr; - void* fIdleProcContext = nullptr; typedef GrTexture INHERITED; }; diff --git a/src/gpu/mtl/GrMtlTextureRenderTarget.h b/src/gpu/mtl/GrMtlTextureRenderTarget.h index b15d2896c0..f62233002d 100644 --- a/src/gpu/mtl/GrMtlTextureRenderTarget.h +++ b/src/gpu/mtl/GrMtlTextureRenderTarget.h @@ -76,7 +76,7 @@ private: numColorSamples, GrMipMapped::kNo, false); } - void onSetRelease(sk_sp releaseHelper) override {} + void onSetRelease(sk_sp releaseHelper) override {} }; #endif diff --git a/src/gpu/vk/GrVkImage.cpp b/src/gpu/vk/GrVkImage.cpp index 99140f67d7..524c33e567 100644 --- a/src/gpu/vk/GrVkImage.cpp +++ b/src/gpu/vk/GrVkImage.cpp @@ -257,7 +257,7 @@ void GrVkImage::abandonImage() { } } -void GrVkImage::setResourceRelease(sk_sp releaseHelper) { +void GrVkImage::setResourceRelease(sk_sp releaseHelper) { SkASSERT(fResource); // Forward the release proc on to GrVkImage::Resource fResource->setRelease(std::move(releaseHelper)); @@ -270,11 +270,10 @@ void GrVkImage::Resource::freeGPUData(GrVkGpu* gpu) const { GrVkMemory::FreeImageMemory(gpu, isLinear, fAlloc); } -void GrVkImage::Resource::setIdleProc(GrVkTexture* owner, GrTexture::IdleProc proc, - void* context) const { +void GrVkImage::Resource::replaceIdleProc( + GrVkTexture* owner, sk_sp idleCallback) const { fOwningTexture = owner; - fIdleProc = proc; - fIdleProcContext = context; + fIdleCallback = std::move(idleCallback); } void GrVkImage::Resource::removeOwningTexture() const { fOwningTexture = nullptr; } @@ -283,22 +282,16 @@ void GrVkImage::Resource::notifyAddedToCommandBuffer() const { ++fNumCommandBuff void GrVkImage::Resource::notifyRemovedFromCommandBuffer() const { SkASSERT(fNumCommandBufferOwners); - if (--fNumCommandBufferOwners || !fIdleProc) { + if (--fNumCommandBufferOwners || !fIdleCallback) { return; } - if (fOwningTexture && fOwningTexture->resourcePriv().hasRefOrPendingIO()) { - return; - } - fIdleProc(fIdleProcContext); if (fOwningTexture) { - fOwningTexture->setIdleProc(nullptr, nullptr); - // Changing the texture's proc should change ours. - SkASSERT(!fIdleProc); - SkASSERT(!fIdleProc); - } else { - fIdleProc = nullptr; - fIdleProcContext = nullptr; + if (fOwningTexture->resourcePriv().hasRefOrPendingIO()) { + return; + } + fOwningTexture->removeIdleProc(); } + fIdleCallback.reset(); } void GrVkImage::BorrowedResource::freeGPUData(GrVkGpu* gpu) const { diff --git a/src/gpu/vk/GrVkImage.h b/src/gpu/vk/GrVkImage.h index 994924d406..2da325d1c2 100644 --- a/src/gpu/vk/GrVkImage.h +++ b/src/gpu/vk/GrVkImage.h @@ -139,7 +139,7 @@ public: typedef void* ReleaseCtx; typedef void (*ReleaseProc)(ReleaseCtx); - void setResourceRelease(sk_sp releaseHelper); + void setResourceRelease(sk_sp releaseHelper); // Helpers to use for setting the layout of the VkImage static VkPipelineStageFlags LayoutToPipelineSrcStageFlags(const VkImageLayout layout); @@ -182,7 +182,7 @@ private: SkDebugf("GrVkImage: %d (%d refs)\n", fImage, this->getRefCnt()); } #endif - void setRelease(sk_sp releaseHelper) { + void setRelease(sk_sp releaseHelper) { fReleaseHelper = std::move(releaseHelper); } @@ -192,7 +192,7 @@ private: * referring to the Resource then it calls the proc. Otherwise, the Resource calls it * when the last command buffer reference goes away and the GrVkTexture is purgeable. */ - void setIdleProc(GrVkTexture* owner, GrTexture::IdleProc, void* context) const; + void replaceIdleProc(GrVkTexture* owner, sk_sp) const; void removeOwningTexture() const; /** @@ -204,7 +204,7 @@ private: bool isOwnedByCommandBuffer() const { return fNumCommandBufferOwners > 0; } protected: - mutable sk_sp fReleaseHelper; + mutable sk_sp fReleaseHelper; void invokeReleaseProc() const { if (fReleaseHelper) { @@ -225,8 +225,7 @@ private: GrVkAlloc fAlloc; VkImageTiling fImageTiling; mutable int fNumCommandBufferOwners = 0; - mutable GrTexture::IdleProc* fIdleProc = nullptr; - mutable void* fIdleProcContext = nullptr; + mutable sk_sp fIdleCallback; mutable GrVkTexture* fOwningTexture = nullptr; typedef GrVkResource INHERITED; diff --git a/src/gpu/vk/GrVkRenderTarget.h b/src/gpu/vk/GrVkRenderTarget.h index 393d54dad7..bcfa823ea0 100644 --- a/src/gpu/vk/GrVkRenderTarget.h +++ b/src/gpu/vk/GrVkRenderTarget.h @@ -162,7 +162,7 @@ private: // In Vulkan we call the release proc after we are finished with the underlying // GrVkImage::Resource object (which occurs after the GPU has finished all work on it). - void onSetRelease(sk_sp releaseHelper) override { + void onSetRelease(sk_sp releaseHelper) override { // Forward the release proc on to GrVkImage this->setResourceRelease(std::move(releaseHelper)); } diff --git a/src/gpu/vk/GrVkTexture.cpp b/src/gpu/vk/GrVkTexture.cpp index cdb9351a81..74f9b15688 100644 --- a/src/gpu/vk/GrVkTexture.cpp +++ b/src/gpu/vk/GrVkTexture.cpp @@ -124,8 +124,7 @@ void GrVkTexture::onRelease() { // who will handle it. If the resource is still tied to a command buffer we let it handle it. // Otherwise, we handle it. if (this->hasResource() && this->resource()->isOwnedByCommandBuffer()) { - fIdleProc = nullptr; - fIdleProcContext = nullptr; + fIdleCallback.reset(); } // we create this and don't hand it off, so we should always destroy it @@ -144,8 +143,7 @@ void GrVkTexture::onAbandon() { // who will handle it. If the resource is still tied to a command buffer we let it handle it. // Otherwise, we handle it. if (this->hasResource() && this->resource()->isOwnedByCommandBuffer()) { - fIdleProc = nullptr; - fIdleProcContext = nullptr; + fIdleCallback.reset(); } // we create this and don't hand it off, so we should always destroy it @@ -171,28 +169,25 @@ const GrVkImageView* GrVkTexture::textureView() { return fTextureView; } -void GrVkTexture::setIdleProc(IdleProc proc, void* context) { - fIdleProc = proc; - fIdleProcContext = context; +void GrVkTexture::addIdleProc(sk_sp callback) { + INHERITED::addIdleProc(callback); if (auto* resource = this->resource()) { - resource->setIdleProc(proc ? this : nullptr, proc, context); + resource->replaceIdleProc(this, fIdleCallback); } } void GrVkTexture::willRemoveLastRefOrPendingIO() { - if (!fIdleProc) { + if (!fIdleCallback) { return; } // This is called when the GrTexture is purgeable. However, we need to check whether the // Resource is still owned by any command buffers. If it is then it will call the proc. auto* resource = this->hasResource() ? this->resource() : nullptr; - if (resource && resource->isOwnedByCommandBuffer()) { - return; - } - fIdleProc(fIdleProcContext); - fIdleProc = nullptr; - fIdleProcContext = nullptr; if (resource) { - resource->setIdleProc(nullptr, nullptr, nullptr); + if (resource->isOwnedByCommandBuffer()) { + return; + } + resource->replaceIdleProc(this, nullptr); } + fIdleCallback.reset(); } diff --git a/src/gpu/vk/GrVkTexture.h b/src/gpu/vk/GrVkTexture.h index 0cfce86219..ffd4a3fd08 100644 --- a/src/gpu/vk/GrVkTexture.h +++ b/src/gpu/vk/GrVkTexture.h @@ -38,8 +38,8 @@ public: const GrVkImageView* textureView(); - void setIdleProc(IdleProc, void* context) override; - void* idleContext() const override { return fIdleProcContext; } + void addIdleProc(sk_sp) override; + void removeIdleProc() { fIdleCallback.reset(); } protected: GrVkTexture(GrVkGpu*, const GrSurfaceDesc&, const GrVkImageInfo&, sk_sp, @@ -54,6 +54,8 @@ protected: return false; } + void willRemoveLastRefOrPendingIO() override; + private: GrVkTexture(GrVkGpu*, SkBudgeted, const GrSurfaceDesc&, const GrVkImageInfo&, sk_sp layout, const GrVkImageView* imageView, @@ -64,16 +66,12 @@ private: // In Vulkan we call the release proc after we are finished with the underlying // GrVkImage::Resource object (which occurs after the GPU has finished all work on it). - void onSetRelease(sk_sp releaseHelper) override { + void onSetRelease(sk_sp releaseHelper) override { // Forward the release proc on to GrVkImage this->setResourceRelease(std::move(releaseHelper)); } - void willRemoveLastRefOrPendingIO() override; - const GrVkImageView* fTextureView; - GrTexture::IdleProc* fIdleProc = nullptr; - void* fIdleProcContext = nullptr; typedef GrTexture INHERITED; }; diff --git a/src/gpu/vk/GrVkTextureRenderTarget.h b/src/gpu/vk/GrVkTextureRenderTarget.h index 5e90aa04b6..8093bac07a 100644 --- a/src/gpu/vk/GrVkTextureRenderTarget.h +++ b/src/gpu/vk/GrVkTextureRenderTarget.h @@ -107,7 +107,7 @@ private: // In Vulkan we call the release proc after we are finished with the underlying // GrVkImage::Resource object (which occurs after the GPU has finished all work on it). - void onSetRelease(sk_sp releaseHelper) override { + void onSetRelease(sk_sp releaseHelper) override { // Forward the release proc on to GrVkImage this->setResourceRelease(std::move(releaseHelper)); } diff --git a/src/image/SkImage_GpuBase.cpp b/src/image/SkImage_GpuBase.cpp index 594fd05bd9..de32e8e516 100644 --- a/src/image/SkImage_GpuBase.cpp +++ b/src/image/SkImage_GpuBase.cpp @@ -403,19 +403,16 @@ sk_sp SkImage_GpuBase::MakePromiseImageLazyProxy( /** * This class is the lazy instantiation callback for promise images. It manages calling the * client's Fulfill, Release, and Done procs. It attempts to reuse a GrTexture instance in - * cases where the client provides the same SkPromiseImageTexture for successive Fulfill calls. - * The created GrTexture is given a key based on a unique ID associated with the - * SkPromiseImageTexture. When the texture enters "idle" state (meaning it is not being used by - * the GPU and is at rest in the resource cache) the client's Release proc is called - * using GrTexture's idle proc mechanism. If the same SkPromiseImageTexture is provided for - * another fulfill we find the cached GrTexture. If the proxy, and therefore this object, - * is destroyed, we invalidate the GrTexture's key. Also if the client overwrites or - * destroys their SkPromiseImageTexture we invalidate the key. + * cases where the client provides the same SkPromiseImageTexture as Fulfill results for + * multiple SkImages. The created GrTexture is given a key based on a unique ID associated with + * the SkPromiseImageTexture. * - * Currently a GrTexture is only reused for a given SkPromiseImageTexture if the - * SkPromiseImageTexture is reused in Fulfill for the same promise SkImage. However, we'd - * like to relax that so that a SkPromiseImageTexture can be reused with different promise - * SkImages that will reuse a single GrTexture. + * The GrTexutre idle proc mechanism is used to call the Release and Done procs. We use this + * instead of the GrSurface release proc because the GrTexture is cached and therefore may + * outlive the proxy into which this callback is installed. + * + * A key invalidation message is installed on the SkPromiseImageTexture so that the GrTexture + * is deleted once it can no longer be used to instantiate a proxy. */ class PromiseLazyInstantiateCallback { public: @@ -425,34 +422,46 @@ sk_sp SkImage_GpuBase::MakePromiseImageLazyProxy( PromiseImageTextureContext context, GrPixelConfig config) : fFulfillProc(fulfillProc), fConfig(config) { - auto doneHelper = sk_make_sp(doneProc, context); - fReleaseContext = sk_make_sp( - releaseProc, context, std::move(doneHelper)); + auto doneHelper = sk_make_sp(doneProc, context); + fIdleCallback = sk_make_sp(releaseProc, context); + fIdleCallback->addChild(std::move(doneHelper)); + } + PromiseLazyInstantiateCallback(PromiseLazyInstantiateCallback&&) = default; + PromiseLazyInstantiateCallback(const PromiseLazyInstantiateCallback&) { + // Because we get wrapped in std::function we must be copyable. But we should never + // be copied. + SkASSERT(false); + } + PromiseLazyInstantiateCallback& operator=(PromiseLazyInstantiateCallback&&) = default; + PromiseLazyInstantiateCallback& operator=(const PromiseLazyInstantiateCallback&) { + SkASSERT(false); + return *this; } - ~PromiseLazyInstantiateCallback() = default; + ~PromiseLazyInstantiateCallback() { + if (fIdleCallback) { + // We were never fulfilled. Pass false so done proc is still called. + fIdleCallback->abandon(); + } + } sk_sp operator()(GrResourceProvider* resourceProvider) { - if (fTexture) { - return fTexture; - } - - sk_sp cachedTexture; - GrBackendTexture backendTexture; - sk_sp promiseTexture = - fFulfillProc(fReleaseContext->textureContext()); - fReleaseContext->notifyWasFulfilled(); + SkASSERT(fIdleCallback); + PromiseImageTextureContext textureContext = fIdleCallback->context(); + sk_sp promiseTexture = fFulfillProc(textureContext); + // From here on out our contract is that the release proc must be called, even if + // the return from fulfill was invalid or we fail for some other reason. if (!promiseTexture) { - fReleaseContext->release(); + // Make sure we explicitly reset this because our destructor assumes a non-null + // fIdleCallback means fulfill was never called. + fIdleCallback.reset(); return sk_sp(); } - backendTexture = promiseTexture->backendTexture(); + auto backendTexture = promiseTexture->backendTexture(); backendTexture.fConfig = fConfig; if (!backendTexture.isValid()) { - // Even though the GrBackendTexture is not valid, we must call the release - // proc to keep our contract of always calling Fulfill and Release in pairs. - fReleaseContext->release(); + fIdleCallback.reset(); return sk_sp(); } @@ -475,102 +484,19 @@ sk_sp SkImage_GpuBase::MakePromiseImageLazyProxy( kRead_GrIOType))) { tex->resourcePriv().setUniqueKey(key); } else { - // Even though we failed to wrap the backend texture, we must call the release - // proc to keep our contract of always calling Fulfill and Release in pairs. - fReleaseContext->release(); + fIdleCallback.reset(); return sk_sp(); } } - this->addToIdleContext(tex.get()); - fTexture = tex; - SkASSERT(fContextID == SK_InvalidUniqueID || - fContextID == tex->getContext()->priv().contextID()); - fContextID = tex->getContext()->priv().contextID(); - promiseTexture->addKeyToInvalidate(fContextID, key); + tex->addIdleProc(std::move(fIdleCallback)); + promiseTexture->addKeyToInvalidate(tex->getContext()->priv().contextID(), key); return std::move(tex); } private: - // The GrTexture's idle callback mechanism is used to call the client's Release proc via - // this class. This also owns a ref counted helper that calls the client's ReleaseProc when - // the ref count reaches zero. The callback and any Fulfilled but un-Released texture share - // ownership of the IdleContext. Thus, the IdleContext is destroyed and calls the Done proc - // after the last fulfilled texture goes idle and calls the Release proc or the proxy's - // destructor destroys the lazy callback, whichever comes last. - class IdleContext { - public: - class PromiseImageReleaseContext; - - IdleContext() = default; - - ~IdleContext() = default; - - void addImageReleaseContext(sk_sp context) { - fReleaseContexts.addToHead(std::move(context)); - } - - static void IdleProc(void* context) { - IdleContext* idleContext = static_cast(context); - for (ReleaseContextList::Iter iter = idleContext->fReleaseContexts.headIter(); - iter.get(); - iter.next()) { - (*iter.get())->release(); - } - idleContext->fReleaseContexts.reset(); - delete idleContext; - } - - class PromiseImageReleaseContext : public SkNVRefCnt { - public: - PromiseImageReleaseContext(PromiseImageTextureReleaseProc releaseProc, - PromiseImageTextureContext textureContext, - sk_sp doneHelper) - : fReleaseProc(releaseProc) - , fTextureContext(textureContext) - , fDoneHelper(std::move(doneHelper)) {} - - ~PromiseImageReleaseContext() { SkASSERT(fIsReleased); } - - void release() { - SkASSERT(!fIsReleased); - fReleaseProc(fTextureContext); - fIsReleased = true; - } - - void notifyWasFulfilled() { fIsReleased = false; } - bool isReleased() const { return fIsReleased; } - - PromiseImageTextureContext textureContext() const { return fTextureContext; } - - private: - PromiseImageTextureReleaseProc fReleaseProc; - PromiseImageTextureContext fTextureContext; - sk_sp fDoneHelper; - bool fIsReleased = true; - }; - - private: - using ReleaseContextList = SkTLList, 4>; - ReleaseContextList fReleaseContexts; - }; - - void addToIdleContext(GrTexture* texture) { - SkASSERT(!fReleaseContext->isReleased()); - IdleContext* idleContext = static_cast(texture->idleContext()); - if (!idleContext) { - idleContext = new IdleContext(); - texture->setIdleProc(IdleContext::IdleProc, idleContext); - } - idleContext->addImageReleaseContext(fReleaseContext); - } - - sk_sp fReleaseContext; - sk_sp fTexture; + sk_sp fIdleCallback; PromiseImageTextureFulfillProc fFulfillProc; GrPixelConfig fConfig; - - // ID of the GrContext that we are interacting with. - uint32_t fContextID = SK_InvalidUniqueID; } callback(fulfillProc, releaseProc, doneProc, textureContext, config); GrProxyProvider* proxyProvider = context->priv().proxyProvider(); @@ -584,5 +510,5 @@ sk_sp SkImage_GpuBase::MakePromiseImageLazyProxy( return proxyProvider->createLazyProxy(std::move(callback), backendFormat, desc, origin, mipMapped, GrInternalSurfaceFlags::kReadOnly, SkBackingFit::kExact, SkBudgeted::kNo, - GrSurfaceProxy::LazyInstantiationType::kDeinstantiate); + GrSurfaceProxy::LazyInstantiationType::kSingleUse); } diff --git a/tests/GrSurfaceTest.cpp b/tests/GrSurfaceTest.cpp index dae3941054..503652c355 100644 --- a/tests/GrSurfaceTest.cpp +++ b/tests/GrSurfaceTest.cpp @@ -362,7 +362,7 @@ static sk_sp make_wrapped_texture(GrContext* context, bool renderable delete releaseContext; }; texture->setRelease( - sk_make_sp(release, new ReleaseContext{context, backendTexture})); + sk_make_sp(release, new ReleaseContext{context, backendTexture})); return texture; } @@ -424,7 +424,8 @@ DEF_GPUTEST(TextureIdleProcTest, reporter, options) { // Makes a texture, possibly adds a key, and sets the callback. auto make = [&m, &keyAdder, &proc, &idleIDs](GrContext* context, int num) { sk_sp texture = m(context); - texture->setIdleProc(proc, new Context{&idleIDs, num}); + texture->addIdleProc( + sk_make_sp(proc, new Context{&idleIDs, num})); keyAdder(texture.get()); return texture; }; @@ -487,28 +488,6 @@ DEF_GPUTEST(TextureIdleProcTest, reporter, options) { // Now that the draw is fully consumed by the GPU, the texture should be idle. REPORTER_ASSERT(reporter, idleIDs.find(2) != idleIDs.end()); - // Make a proxy that should deinstantiate even if we keep a ref on it. - auto deinstantiateLazyCB = [&make, &context](GrResourceProvider* rp) { - return make(context, 3); - }; - proxy = context->priv().proxyProvider()->createLazyProxy( - deinstantiateLazyCB, backendFormat, desc, - GrSurfaceOrigin::kTopLeft_GrSurfaceOrigin, GrMipMapped::kNo, - GrInternalSurfaceFlags ::kNone, SkBackingFit::kExact, budgeted, - GrSurfaceProxy::LazyInstantiationType::kDeinstantiate); - rtc->drawTexture(GrNoClip(), std::move(proxy), GrSamplerState::Filter::kNearest, - SkBlendMode::kSrcOver, SkPMColor4f(), SkRect::MakeWH(w, h), - SkRect::MakeWH(w, h), GrAA::kNo, GrQuadAAFlags::kNone, - SkCanvas::kFast_SrcRectConstraint, SkMatrix::I(), nullptr); - // At this point the proxy shouldn't even be instantiated, there is no texture with - // id 3. - REPORTER_ASSERT(reporter, idleIDs.find(3) == idleIDs.end()); - context->flush(); - context->priv().getGpu()->testingOnly_flushGpuAndSync(); - // Now that the draw is fully consumed, we should have deinstantiated the proxy and - // the texture it made should be idle. - REPORTER_ASSERT(reporter, idleIDs.find(3) != idleIDs.end()); - // Make sure we make the call during various shutdown scenarios where the texture // might persist after context is destroyed, abandoned, etc. We test three // variations of each scenario. One where the texture is just created. Another, @@ -522,7 +501,7 @@ DEF_GPUTEST(TextureIdleProcTest, reporter, options) { if (api == GrBackendApi::kVulkan) { continue; } - int id = 4; + int id = 3; enum class DrawType { kNoDraw, kDraw, @@ -610,7 +589,7 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(TextureIdleProcCacheManipulationTest, reporter, con auto idleTexture = idleMaker(context, false); auto otherTexture = otherMaker(context, false); otherTexture->ref(); - idleTexture->setIdleProc(idleProc, otherTexture.get()); + idleTexture->addIdleProc(sk_make_sp(idleProc, otherTexture.get())); otherTexture.reset(); idleTexture.reset(); } @@ -627,7 +606,7 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(TextureIdleProcFlushTest, reporter, contextInfo) { for (const auto& idleMaker : {make_wrapped_texture, make_normal_texture}) { auto idleTexture = idleMaker(context, false); - idleTexture->setIdleProc(idleProc, context); + idleTexture->addIdleProc(sk_make_sp(idleProc, context)); auto info = SkImageInfo::Make(10, 10, kRGBA_8888_SkColorType, kPremul_SkAlphaType); auto surf = SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info, 1, nullptr); // We'll draw two images to the canvas. One is a normal texture-backed image. The other is @@ -660,7 +639,7 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(TextureIdleProcRerefTest, reporter, contextInfo) { // called). idleTexture->resourcePriv().removeScratchKey(); context->flush(); - idleTexture->setIdleProc(idleProc, idleTexture.get()); + idleTexture->addIdleProc(sk_make_sp(idleProc, idleTexture.get())); idleTexture->setRelease(releaseProc, &isReleased); auto* raw = idleTexture.get(); idleTexture.reset(); diff --git a/tests/LazyProxyTest.cpp b/tests/LazyProxyTest.cpp index e016e4f673..94ca6cb6cf 100644 --- a/tests/LazyProxyTest.cpp +++ b/tests/LazyProxyTest.cpp @@ -248,9 +248,8 @@ DEF_GPUTEST(LazyProxyReleaseTest, reporter, /* options */) { using LazyInstantiationType = GrSurfaceProxy::LazyInstantiationType; for (bool doInstantiate : {true, false}) { - for (auto lazyType : {LazyInstantiationType::kSingleUse, - LazyInstantiationType::kMultipleUse, - LazyInstantiationType::kDeinstantiate}) { + for (auto lazyType : + {LazyInstantiationType::kSingleUse, LazyInstantiationType::kMultipleUse}) { int testCount = 0; // Sets an integer to 1 when the callback is called and -1 when it is deleted. class TestCallback { @@ -454,7 +453,7 @@ DEF_GPUTEST(LazyProxyDeinstantiateTest, reporter, /* options */) { ctx->priv().caps()->getBackendFormatFromColorType(kRGBA_8888_SkColorType); using LazyType = GrSurfaceProxy::LazyInstantiationType; - for (auto lazyType : {LazyType::kSingleUse, LazyType::kMultipleUse, LazyType::kDeinstantiate}) { + for (auto lazyType : {LazyType::kSingleUse, LazyType::kMultipleUse}) { sk_sp rtc = ctx->priv().makeDeferredRenderTargetContext( format, SkBackingFit::kExact, 100, 100, kRGBA_8888_GrPixelConfig, nullptr); @@ -497,11 +496,7 @@ DEF_GPUTEST(LazyProxyDeinstantiateTest, reporter, /* options */) { ctx->flush(); REPORTER_ASSERT(reporter, 1 == instantiateTestValue); - if (LazyType::kDeinstantiate == lazyType) { - REPORTER_ASSERT(reporter, 1 == releaseTestValue); - } else { - REPORTER_ASSERT(reporter, 0 == releaseTestValue); - } + REPORTER_ASSERT(reporter, 0 == releaseTestValue); // This should cause the uninstantiate proxies to be instantiated again but have no effect // on the others @@ -510,20 +505,11 @@ DEF_GPUTEST(LazyProxyDeinstantiateTest, reporter, /* options */) { rtc->priv().testingOnly_addDrawOp(LazyDeinstantiateTestOp::Make(ctx.get(), lazyProxy)); ctx->flush(); - if (LazyType::kDeinstantiate == lazyType) { - REPORTER_ASSERT(reporter, 2 == instantiateTestValue); - REPORTER_ASSERT(reporter, 2 == releaseTestValue); - } else { - REPORTER_ASSERT(reporter, 1 == instantiateTestValue); - REPORTER_ASSERT(reporter, 0 == releaseTestValue); - } + REPORTER_ASSERT(reporter, 1 == instantiateTestValue); + REPORTER_ASSERT(reporter, 0 == releaseTestValue); lazyProxy.reset(); - if (LazyType::kDeinstantiate == lazyType) { - REPORTER_ASSERT(reporter, 2 == releaseTestValue); - } else { - REPORTER_ASSERT(reporter, 1 == releaseTestValue); - } + REPORTER_ASSERT(reporter, 1 == releaseTestValue); gpu->deleteTestingOnlyBackendTexture(backendTex); } diff --git a/tests/ResourceAllocatorTest.cpp b/tests/ResourceAllocatorTest.cpp index a71221d24f..69118fc22f 100644 --- a/tests/ResourceAllocatorTest.cpp +++ b/tests/ResourceAllocatorTest.cpp @@ -10,7 +10,6 @@ #include "Test.h" #include "GrContextPriv.h" -#include "GrDeinstantiateProxyTracker.h" #include "GrGpu.h" #include "GrProxyProvider.h" #include "GrResourceAllocator.h" @@ -90,8 +89,7 @@ static void cleanup_backend(GrContext* context, const GrBackendTexture& backendT // assigned different GrSurfaces. static void overlap_test(skiatest::Reporter* reporter, GrResourceProvider* resourceProvider, GrSurfaceProxy* p1, GrSurfaceProxy* p2, bool expectedResult) { - GrDeinstantiateProxyTracker deinstantiateTracker; - GrResourceAllocator alloc(resourceProvider, &deinstantiateTracker); + GrResourceAllocator alloc(resourceProvider); alloc.addInterval(p1, 0, 4); alloc.addInterval(p2, 1, 2); @@ -113,8 +111,7 @@ static void overlap_test(skiatest::Reporter* reporter, GrResourceProvider* resou static void non_overlap_test(skiatest::Reporter* reporter, GrResourceProvider* resourceProvider, GrSurfaceProxy* p1, GrSurfaceProxy* p2, bool expectedResult) { - GrDeinstantiateProxyTracker deinstantiateTracker; - GrResourceAllocator alloc(resourceProvider, &deinstantiateTracker); + GrResourceAllocator alloc(resourceProvider); alloc.addInterval(p1, 0, 2); alloc.addInterval(p2, 3, 5); @@ -288,7 +285,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ResourceAllocatorStressTest, reporter, ctxInf } sk_sp make_lazy(GrProxyProvider* proxyProvider, const GrCaps* caps, - const ProxyParams& p, bool deinstantiate) { + const ProxyParams& p) { GrColorType grCT = SkColorTypeToGrColorType(p.fColorType); GrPixelConfig config = GrColorTypeToPixelConfig(grCT, GrSRGBEncoded::kNo); @@ -311,8 +308,7 @@ sk_sp make_lazy(GrProxyProvider* proxyProvider, const GrCaps* ca } }; const GrBackendFormat format = caps->getBackendFormatFromColorType(p.fColorType); - auto lazyType = deinstantiate ? GrSurfaceProxy::LazyInstantiationType ::kDeinstantiate - : GrSurfaceProxy::LazyInstantiationType ::kSingleUse; + auto lazyType = GrSurfaceProxy::LazyInstantiationType ::kSingleUse; GrInternalSurfaceFlags flags = GrInternalSurfaceFlags::kNone; return proxyProvider->createLazyProxy(callback, format, desc, p.fOrigin, GrMipMapped::kNo, flags, p.fFit, SkBudgeted::kNo, lazyType); @@ -334,28 +330,20 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(LazyDeinstantiation, reporter, ctxInfo) { rtParams.fIsRT = true; auto proxyProvider = context->priv().proxyProvider(); auto caps = context->priv().caps(); - auto p0 = make_lazy(proxyProvider, caps, texParams, true); - auto p1 = make_lazy(proxyProvider, caps, texParams, false); + auto p0 = make_lazy(proxyProvider, caps, texParams); texParams.fFit = rtParams.fFit = SkBackingFit::kApprox; - auto p2 = make_lazy(proxyProvider, caps, rtParams, true); - auto p3 = make_lazy(proxyProvider, caps, rtParams, false); + auto p1 = make_lazy(proxyProvider, caps, rtParams); - GrDeinstantiateProxyTracker deinstantiateTracker; { - GrResourceAllocator alloc(resourceProvider, &deinstantiateTracker); + GrResourceAllocator alloc(resourceProvider); alloc.addInterval(p0.get(), 0, 1); alloc.addInterval(p1.get(), 0, 1); - alloc.addInterval(p2.get(), 0, 1); - alloc.addInterval(p3.get(), 0, 1); alloc.markEndOfOpList(0); int startIndex, stopIndex; GrResourceAllocator::AssignError error; alloc.assign(&startIndex, &stopIndex, &error); } - deinstantiateTracker.deinstantiateAllProxies(); - REPORTER_ASSERT(reporter, !p0->isInstantiated()); + REPORTER_ASSERT(reporter, p0->isInstantiated()); REPORTER_ASSERT(reporter, p1->isInstantiated()); - REPORTER_ASSERT(reporter, !p2->isInstantiated()); - REPORTER_ASSERT(reporter, p3->isInstantiated()); } }