From 6a0176bf033c780bb92396220db8140f30948345 Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Tue, 30 Jan 2018 09:28:44 -0500 Subject: [PATCH] Add ref counted wrapped around GrTexture ReleaseProc Bug: skia: Change-Id: I0cd11a539fd6b16d4b3f9512694f84e0a429518c Reviewed-on: https://skia-review.googlesource.com/101341 Commit-Queue: Greg Daniel Reviewed-by: Robert Phillips Reviewed-by: Brian Salomon --- include/gpu/GrTexture.h | 13 ++++++--- include/private/GrTypesPriv.h | 16 +++++++++++ src/gpu/GrAHardwareBufferImageGenerator.cpp | 5 +++- src/gpu/GrBackendTextureImageGenerator.cpp | 8 +++++- src/gpu/GrProxyProvider.cpp | 21 ++++++++------ src/gpu/gl/GrGLTexture.h | 17 ++++++----- src/gpu/mock/GrMockTexture.h | 21 +++++--------- src/gpu/mtl/GrMtlTexture.h | 8 ++---- src/gpu/vk/GrVkImage.cpp | 6 ++-- src/gpu/vk/GrVkImage.h | 32 +++++++++------------ src/gpu/vk/GrVkTexture.h | 4 +-- 11 files changed, 85 insertions(+), 66 deletions(-) diff --git a/include/gpu/GrTexture.h b/include/gpu/GrTexture.h index 28f0d4b548..899b3ed58c 100644 --- a/include/gpu/GrTexture.h +++ b/include/gpu/GrTexture.h @@ -41,7 +41,7 @@ public: /** * This function steals the backend texture from a uniquely owned GrTexture with no pending * IO, passing it out to the caller. The GrTexture is deleted in the process. - * + * * Note that if the GrTexture is not uniquely owned (no other refs), or has pending IO, this * function will fail. */ @@ -55,11 +55,16 @@ public: } #endif - // These match the definitions in SkImage, for whence they came + virtual void setRelease(sk_sp releaseHelper) = 0; + + // These match the definitions in SkImage, from whence they came. + // TODO: Either move Chrome over to new api or remove their need to call this on GrTexture typedef void* ReleaseCtx; typedef void (*ReleaseProc)(ReleaseCtx); - - virtual void setRelease(ReleaseProc proc, ReleaseCtx ctx) = 0; + void setRelease(ReleaseProc proc, ReleaseCtx ctx) { + sk_sp helper(new GrReleaseProcHelper(proc, ctx)); + this->setRelease(std::move(helper)); + } /** Access methods that are only to be used within Skia code. */ inline GrTexturePriv texturePriv(); diff --git a/include/private/GrTypesPriv.h b/include/private/GrTypesPriv.h index cfcbff9af9..89861e3d5b 100644 --- a/include/private/GrTypesPriv.h +++ b/include/private/GrTypesPriv.h @@ -1048,4 +1048,20 @@ static inline GrPixelConfigIsClamped GrGetPixelConfigIsClamped(GrPixelConfig con : GrPixelConfigIsClamped::kYes; } +class GrReleaseProcHelper : public SkRefCnt { +public: + // These match the definitions in SkImage, from whence they came + typedef void* ReleaseCtx; + typedef void (*ReleaseProc)(ReleaseCtx); + + GrReleaseProcHelper(ReleaseProc proc, ReleaseCtx ctx) : fReleaseProc(proc), fReleaseCtx(ctx) {} + ~GrReleaseProcHelper() override { + fReleaseProc(fReleaseCtx); + } + +private: + ReleaseProc fReleaseProc; + ReleaseCtx fReleaseCtx; +}; + #endif diff --git a/src/gpu/GrAHardwareBufferImageGenerator.cpp b/src/gpu/GrAHardwareBufferImageGenerator.cpp index 74073adeef..085d27ac10 100644 --- a/src/gpu/GrAHardwareBufferImageGenerator.cpp +++ b/src/gpu/GrAHardwareBufferImageGenerator.cpp @@ -229,7 +229,10 @@ sk_sp GrAHardwareBufferImageGenerator::makeProxy(GrContext* cont eglDestroyImageKHR(display, image); return nullptr; } - tex->setRelease(deleteImageTexture, new BufferCleanupHelper(image, display)); + sk_sp releaseHelper( + new GrReleaseProcHelper(deleteImageTexture, new BufferCleanupHelper(image, display))); + + tex->setRelease(std::move(releaseHelper)); // We fail this assert, if the context has changed. This will be fully handled after // skbug.com/6812 is ready. diff --git a/src/gpu/GrBackendTextureImageGenerator.cpp b/src/gpu/GrBackendTextureImageGenerator.cpp index 064727e248..262250ce4d 100644 --- a/src/gpu/GrBackendTextureImageGenerator.cpp +++ b/src/gpu/GrBackendTextureImageGenerator.cpp @@ -162,9 +162,15 @@ sk_sp GrBackendTextureImageGenerator::onGenerateTexture( } refHelper->fBorrowedTexture = tex.get(); + sk_sp releaseHelper( + new GrReleaseProcHelper(ReleaseRefHelper_TextureReleaseProc, refHelper)); + // By setting this release proc on the texture we are passing our ref on the // refHelper to the texture. - tex->setRelease(ReleaseRefHelper_TextureReleaseProc, refHelper); + // DDL TODO: Once we are start reusing Lazy Proxies, we need to add a ref to the + // refHelper here, we'll still move the releaseHelper though since the texture + // will be the only one ever calling that release proc. + tex->setRelease(std::move(releaseHelper)); } return tex; diff --git a/src/gpu/GrProxyProvider.cpp b/src/gpu/GrProxyProvider.cpp index c492062196..5bb58af36f 100644 --- a/src/gpu/GrProxyProvider.cpp +++ b/src/gpu/GrProxyProvider.cpp @@ -383,15 +383,18 @@ sk_sp GrProxyProvider::createWrappedTextureProxy( desc.fConfig = backendTex.config(); GrMipMapped mipMapped = backendTex.hasMipMaps() ? GrMipMapped::kYes : GrMipMapped::kNo; + sk_sp releaseHelper; + if (releaseProc) { + releaseHelper.reset(new GrReleaseProcHelper(releaseProc, releaseCtx)); + } + sk_sp proxy = this->createLazyProxy( - [backendTex, ownership, releaseProc, releaseCtx] + [backendTex, ownership, releaseHelper] (GrResourceProvider* resourceProvider, GrSurfaceOrigin* /*outOrigin*/) { if (!resourceProvider) { - // This lazy proxy was never initialized. If it has a releaseProc we must call - // it now so that the client knows they can free the underlying backend object. - if (releaseProc) { - releaseProc(releaseCtx); - } + // This lazy proxy was never initialized. If this had a releaseHelper it will + // get unrefed when we delete this lambda and will call the release proc so that + // the client knows they can free the underlying backend object. return sk_sp(); } @@ -400,8 +403,10 @@ sk_sp GrProxyProvider::createWrappedTextureProxy( if (!tex) { return sk_sp(); } - if (releaseProc) { - tex->setRelease(releaseProc, releaseCtx); + if (releaseHelper) { + // DDL TODO: once we are reusing lazy proxies, remove this move and hold onto to + // the ref till the lambda goes away. + tex->setRelease(std::move(releaseHelper)); } SkASSERT(!tex->asRenderTarget()); // Strictly a GrTexture // Make sure we match how we created the proxy with SkBudgeted::kNo diff --git a/src/gpu/gl/GrGLTexture.h b/src/gpu/gl/GrGLTexture.h index b7c8716cc7..2b10be8689 100644 --- a/src/gpu/gl/GrGLTexture.h +++ b/src/gpu/gl/GrGLTexture.h @@ -36,7 +36,7 @@ public: ~GrGLTexture() override { // check that invokeReleaseProc has been called (if needed) - SkASSERT(!fReleaseProc); + SkASSERT(!fReleaseHelper); } GrBackendObject getTextureHandle() const override; @@ -44,9 +44,8 @@ public: void textureParamsModified() override { fTexParams.invalidate(); } - void setRelease(ReleaseProc proc, ReleaseCtx ctx) override { - fReleaseProc = proc; - fReleaseCtx = ctx; + void setRelease(sk_sp releaseHelper) override { + fReleaseHelper = std::move(releaseHelper); } // These functions are used to track the texture parameters associated with the texture. @@ -90,9 +89,10 @@ protected: private: void invokeReleaseProc() { - if (fReleaseProc) { - fReleaseProc(fReleaseCtx); - fReleaseProc = nullptr; + if (fReleaseHelper) { + // Depending on the ref count of fReleaseHelper this may or may not actually trigger the + // ReleaseProc to be called. + fReleaseHelper.reset(); } } @@ -104,8 +104,7 @@ private: GrBackendObjectOwnership fTextureIDOwnership; bool fBaseLevelHasBeenBoundToFBO = false; - ReleaseProc fReleaseProc = nullptr; - ReleaseCtx fReleaseCtx = nullptr; + sk_sp fReleaseHelper; typedef GrTexture INHERITED; }; diff --git a/src/gpu/mock/GrMockTexture.h b/src/gpu/mock/GrMockTexture.h index a25ff9ba67..0be1da1f4c 100644 --- a/src/gpu/mock/GrMockTexture.h +++ b/src/gpu/mock/GrMockTexture.h @@ -20,11 +20,8 @@ public: : GrMockTexture(gpu, desc, mipMapsStatus, info) { this->registerWithCache(budgeted); } - ~GrMockTexture() override { - if (fReleaseProc) { - fReleaseProc(fReleaseCtx); - } - } + ~GrMockTexture() override {} + GrBackendObject getTextureHandle() const override { return reinterpret_cast(&fInfo); } @@ -34,9 +31,8 @@ public: } void textureParamsModified() override {} - void setRelease(ReleaseProc proc, ReleaseCtx ctx) override { - fReleaseProc = proc; - fReleaseCtx = ctx; + void setRelease(sk_sp releaseHelper) override { + fReleaseHelper = std::move(releaseHelper); } protected: @@ -46,18 +42,15 @@ protected: : GrSurface(gpu, desc) , INHERITED(gpu, desc, kITexture2DSampler_GrSLType, GrSamplerState::Filter::kMipMap, mipMapsStatus) - , fInfo(info) - , fReleaseProc(nullptr) - , fReleaseCtx(nullptr) {} + , fInfo(info) {} bool onStealBackendTexture(GrBackendTexture*, SkImage::BackendTextureReleaseProc*) override { return false; } private: - GrMockTextureInfo fInfo; - ReleaseProc fReleaseProc; - ReleaseCtx fReleaseCtx; + GrMockTextureInfo fInfo; + sk_sp fReleaseHelper; typedef GrTexture INHERITED; }; diff --git a/src/gpu/mtl/GrMtlTexture.h b/src/gpu/mtl/GrMtlTexture.h index 2d7c3011df..3433f38db0 100644 --- a/src/gpu/mtl/GrMtlTexture.h +++ b/src/gpu/mtl/GrMtlTexture.h @@ -33,12 +33,11 @@ public: bool reallocForMipmap(GrMtlGpu* gpu, uint32_t mipLevels); - void setRelease(GrTexture::ReleaseProc proc, GrTexture::ReleaseCtx ctx) override { + void setRelease(sk_sp releaseHelper) override { // 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. - fReleaseProc = proc; - fReleaseCtx = ctx; + fReleaseHelper = std::move(releaseHelper); } protected: @@ -64,8 +63,7 @@ private: id fTexture; - ReleaseProc fReleaseProc = nullptr; - ReleaseCtx fReleaseCtx = nullptr; + sk_sp fReleaseHelper; typedef GrTexture INHERITED; }; diff --git a/src/gpu/vk/GrVkImage.cpp b/src/gpu/vk/GrVkImage.cpp index d23ebfe360..5e0ffe240f 100644 --- a/src/gpu/vk/GrVkImage.cpp +++ b/src/gpu/vk/GrVkImage.cpp @@ -152,13 +152,13 @@ void GrVkImage::abandonImage() { } } -void GrVkImage::setResourceRelease(ReleaseProc proc, ReleaseCtx ctx) { +void GrVkImage::setResourceRelease(sk_sp releaseHelper) { // Forward the release proc on to GrVkImage::Resource - fResource->setRelease(proc, ctx); + fResource->setRelease(std::move(releaseHelper)); } void GrVkImage::Resource::freeGPUData(const GrVkGpu* gpu) const { - SkASSERT(!fReleaseProc); + SkASSERT(!fReleaseHelper); VK_CALL(gpu, DestroyImage(gpu->device(), fImage, nullptr)); bool isLinear = (VK_IMAGE_TILING_LINEAR == fImageTiling); GrVkMemory::FreeImageMemory(gpu, isLinear, fAlloc); diff --git a/src/gpu/vk/GrVkImage.h b/src/gpu/vk/GrVkImage.h index b71ed4940c..587c3a8d25 100644 --- a/src/gpu/vk/GrVkImage.h +++ b/src/gpu/vk/GrVkImage.h @@ -83,7 +83,7 @@ public: typedef void* ReleaseCtx; typedef void (*ReleaseProc)(ReleaseCtx); - void setResourceRelease(ReleaseProc proc, ReleaseCtx ctx); + void setResourceRelease(sk_sp releaseHelper); protected: void releaseImage(const GrVkGpu* gpu); @@ -98,23 +98,18 @@ private: class Resource : public GrVkResource { public: Resource() - : INHERITED() - , fReleaseProc(nullptr) - , fReleaseCtx(nullptr) - , fImage(VK_NULL_HANDLE) { + : fImage(VK_NULL_HANDLE) { fAlloc.fMemory = VK_NULL_HANDLE; fAlloc.fOffset = 0; } Resource(VkImage image, const GrVkAlloc& alloc, VkImageTiling tiling) - : fReleaseProc(nullptr) - , fReleaseCtx(nullptr) - , fImage(image) + : fImage(image) , fAlloc(alloc) , fImageTiling(tiling) {} ~Resource() override { - SkASSERT(!fReleaseProc); + SkASSERT(!fReleaseHelper); } #ifdef SK_TRACE_VK_RESOURCES @@ -122,18 +117,16 @@ private: SkDebugf("GrVkImage: %d (%d refs)\n", fImage, this->getRefCnt()); } #endif - void setRelease(ReleaseProc proc, ReleaseCtx ctx) const { - fReleaseProc = proc; - fReleaseCtx = ctx; + void setRelease(sk_sp releaseHelper) { + fReleaseHelper = std::move(releaseHelper); } protected: - mutable ReleaseProc fReleaseProc; - mutable ReleaseCtx fReleaseCtx; + mutable sk_sp fReleaseHelper; private: void freeGPUData(const GrVkGpu* gpu) const override; void abandonGPUData() const override { - SkASSERT(!fReleaseProc); + SkASSERT(!fReleaseHelper); } VkImage fImage; @@ -151,9 +144,10 @@ private: } private: void invokeReleaseProc() const { - if (fReleaseProc) { - fReleaseProc(fReleaseCtx); - fReleaseProc = nullptr; + if (fReleaseHelper) { + // Depending on the ref count of fReleaseHelper this may or may not actually trigger + // the ReleaseProc to be called. + fReleaseHelper.reset(); } } @@ -161,7 +155,7 @@ private: void abandonGPUData() const override; }; - const Resource* fResource; + Resource* fResource; friend class GrVkRenderTarget; }; diff --git a/src/gpu/vk/GrVkTexture.h b/src/gpu/vk/GrVkTexture.h index a2a357a182..f2fabc887b 100644 --- a/src/gpu/vk/GrVkTexture.h +++ b/src/gpu/vk/GrVkTexture.h @@ -39,9 +39,9 @@ public: // In Vulkan we call the release proc after we are finished with the underlying // GrVkImage::Resource object (which occurs after the GPU has finsihed all work on it). - void setRelease(GrTexture::ReleaseProc proc, GrTexture::ReleaseCtx ctx) override { + void setRelease(sk_sp releaseHelper) override { // Forward the release proc on to GrVkImage - this->setResourceRelease(proc, ctx); + this->setResourceRelease(std::move(releaseHelper)); } protected: