AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Be more consistent about calling release procs in SkImage factories (take 2)

Browse Source 
I will follow this up with a Chrome-side CL to fix the call sites of the following factories:
   MakeFromCompressedTexture
   MakeFromTexture
   MakeFromYUVATexturesCopyWithExternalBackend
   MakeFromYUVTexturesCopyWithExternalBackend
   MakeFromNV12TexturesCopyWithExternalBackend

Here is the Chrome-side CL: https://chromium-review.googlesource.com/c/chromium/src/+/2264598/ (Skia's SkImage::Make* factories now guarantee cleanup)

Here is the Chrome-side CL that adds the guard flag:
https://chromium-review.googlesource.com/c/chromium/src/+/2273067/ (Add flag in order to roll a Skia CL into Chrome)

TBR=bsalomon@google.com
Bug: 1097484
Change-Id: Ic2fcdc116f0f866b33d752b6d5abc784c7f65be6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/299663
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This commit is contained in:
Robert Phillips 2020-06-29 13:05:29 -04:00 committed by Skia Commit-Bot
parent ad43fd834f
commit a1121331a4
11 changed files with 73 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/gpu/GrProxyProvider.cpp
View File

@ -523,11 +523,11 @@ sk_sp<GrTextureProxy> GrProxyProvider::wrapBackendTexture(const GrBackendTexture
this->isDDLProvider()));
}
sk_sp<GrTextureProxy> GrProxyProvider::wrapCompressedBackendTexture(const GrBackendTexture& beTex,
GrWrapOwnership ownership,
GrWrapCacheable cacheable,
ReleaseProc releaseProc,
ReleaseContext releaseCtx) {
sk_sp<GrTextureProxy> GrProxyProvider::wrapCompressedBackendTexture(
const GrBackendTexture& beTex,
GrWrapOwnership ownership,
GrWrapCacheable cacheable,
sk_sp<GrRefCntedCallback> releaseHelper) {
if (this->isAbandoned()) {
return nullptr;
}
@ -546,8 +546,8 @@ sk_sp<GrTextureProxy> GrProxyProvider::wrapCompressedBackendTexture(const GrBack
return nullptr;
}
if (releaseProc) {
tex->setRelease(releaseProc, releaseCtx);
if (releaseHelper) {
tex->setRelease(std::move(releaseHelper));
}
SkASSERT(!tex->asRenderTarget()); // Strictly a GrTexture
@ -563,8 +563,7 @@ sk_sp<GrTextureProxy> GrProxyProvider::wrapRenderableBackendTexture(
int sampleCnt,
GrWrapOwnership ownership,
GrWrapCacheable cacheable,
ReleaseProc releaseProc,
ReleaseContext releaseCtx) {
sk_sp<GrRefCntedCallback> releaseHelper) {
if (this->isAbandoned()) {
return nullptr;
}
@ -588,8 +587,8 @@ sk_sp<GrTextureProxy> GrProxyProvider::wrapRenderableBackendTexture(
return nullptr;
}
if (releaseProc) {
tex->setRelease(releaseProc, releaseCtx);
if (releaseHelper) {
tex->setRelease(std::move(releaseHelper));
}
SkASSERT(tex->asRenderTarget()); // A GrTextureRenderTarget
@ -602,8 +601,7 @@ sk_sp<GrTextureProxy> GrProxyProvider::wrapRenderableBackendTexture(
sk_sp<GrSurfaceProxy> GrProxyProvider::wrapBackendRenderTarget(
const GrBackendRenderTarget& backendRT,
ReleaseProc releaseProc,
ReleaseContext releaseCtx) {
sk_sp<GrRefCntedCallback> releaseHelper) {
if (this->isAbandoned()) {
return nullptr;
}
@ -621,8 +619,8 @@ sk_sp<GrSurfaceProxy> GrProxyProvider::wrapBackendRenderTarget(
return nullptr;
}
if (releaseProc) {
rt->setRelease(releaseProc, releaseCtx);
if (releaseHelper) {
rt->setRelease(std::move(releaseHelper));
}
SkASSERT(!rt->asTexture()); // A GrRenderTarget that's not textureable

13
src/gpu/GrProxyProvider.h
View File

@ -117,9 +117,10 @@ public:
GrIOType,
sk_sp<GrRefCntedCallback> = nullptr);
sk_sp<GrTextureProxy> wrapCompressedBackendTexture(const GrBackendTexture&, GrWrapOwnership,
GrWrapCacheable, ReleaseProc = nullptr,
ReleaseContext = nullptr);
sk_sp<GrTextureProxy> wrapCompressedBackendTexture(const GrBackendTexture&,
GrWrapOwnership,
GrWrapCacheable,
sk_sp<GrRefCntedCallback> releaseHelper);
/*
* Create a texture proxy that wraps a backend texture and is both texture-able and renderable
@ -128,15 +129,13 @@ public:
int sampleCnt,
GrWrapOwnership,
GrWrapCacheable,
ReleaseProc = nullptr,
ReleaseContext = nullptr);
sk_sp<GrRefCntedCallback> releaseHelper);
/*
* Create a render target proxy that wraps a backend render target
*/
sk_sp<GrSurfaceProxy> wrapBackendRenderTarget(const GrBackendRenderTarget&,
ReleaseProc = nullptr,
ReleaseContext = nullptr);
sk_sp<GrRefCntedCallback> releaseHelper);
/*
* Create a render target proxy that wraps a backend texture

14
src/gpu/GrRenderTargetContext.cpp
View File

@ -289,12 +289,11 @@ std::unique_ptr<GrRenderTargetContext> GrRenderTargetContext::MakeFromBackendTex
int sampleCnt,
GrSurfaceOrigin origin,
const SkSurfaceProps* surfaceProps,
ReleaseProc releaseProc,
ReleaseContext releaseCtx) {
sk_sp<GrRefCntedCallback> releaseHelper) {
SkASSERT(sampleCnt > 0);
sk_sp<GrTextureProxy> proxy(context->priv().proxyProvider()->wrapRenderableBackendTexture(
tex, sampleCnt, kBorrow_GrWrapOwnership, GrWrapCacheable::kNo, releaseProc,
releaseCtx));
tex, sampleCnt, kBorrow_GrWrapOwnership, GrWrapCacheable::kNo,
std::move(releaseHelper)));
if (!proxy) {
return nullptr;
}
@ -331,8 +330,13 @@ std::unique_ptr<GrRenderTargetContext> GrRenderTargetContext::MakeFromBackendRen
const SkSurfaceProps* surfaceProps,
ReleaseProc releaseProc,
ReleaseContext releaseCtx) {
sk_sp<GrRefCntedCallback> releaseHelper;
if (releaseProc) {
releaseHelper.reset(new GrRefCntedCallback(releaseProc, releaseCtx));
}
sk_sp<GrSurfaceProxy> proxy(
context->priv().proxyProvider()->wrapBackendRenderTarget(rt, releaseProc, releaseCtx));
context->priv().proxyProvider()->wrapBackendRenderTarget(rt, std::move(releaseHelper)));
if (!proxy) {
return nullptr;
}

4
src/gpu/GrRenderTargetContext.h
View File

@ -118,8 +118,8 @@ public:
// Creates a GrRenderTargetContext that wraps the passed in GrBackendTexture.
static std::unique_ptr<GrRenderTargetContext> MakeFromBackendTexture(
GrRecordingContext*, GrColorType, sk_sp<SkColorSpace>, const GrBackendTexture&,
int sampleCnt, GrSurfaceOrigin, const SkSurfaceProps*, ReleaseProc releaseProc,
ReleaseContext releaseCtx);
int sampleCnt, GrSurfaceOrigin, const SkSurfaceProps*,
sk_sp<GrRefCntedCallback> releaseHelper);
static std::unique_ptr<GrRenderTargetContext> MakeFromBackendTextureAsRenderTarget(
GrRecordingContext*, GrColorType, sk_sp<SkColorSpace>, const GrBackendTexture&,

23
src/image/SkImage_Gpu.cpp
View File

@ -197,6 +197,11 @@ sk_sp<SkImage> SkImage::MakeFromCompressedTexture(GrContext* ctx,
sk_sp<SkColorSpace> cs,
TextureReleaseProc releaseP,
ReleaseContext releaseC) {
sk_sp<GrRefCntedCallback> releaseHelper;
if (releaseP) {
releaseHelper.reset(new GrRefCntedCallback(releaseP, releaseC));
}
if (!ctx) {
return nullptr;
}
@ -209,7 +214,7 @@ sk_sp<SkImage> SkImage::MakeFromCompressedTexture(GrContext* ctx,
GrProxyProvider* proxyProvider = ctx->priv().proxyProvider();
sk_sp<GrTextureProxy> proxy = proxyProvider->wrapCompressedBackendTexture(
tex, kBorrow_GrWrapOwnership, GrWrapCacheable::kNo, releaseP, releaseC);
tex, kBorrow_GrWrapOwnership, GrWrapCacheable::kNo, std::move(releaseHelper));
if (!proxy) {
return nullptr;
}
@ -226,6 +231,13 @@ sk_sp<SkImage> SkImage::MakeFromTexture(GrContext* ctx,
const GrBackendTexture& tex, GrSurfaceOrigin origin,
SkColorType ct, SkAlphaType at, sk_sp<SkColorSpace> cs,
TextureReleaseProc releaseP, ReleaseContext releaseC) {
#ifndef SK_LEGACY_MAKEFROMTEXTURE_BEHAVIOR
sk_sp<GrRefCntedCallback> releaseHelper;
if (releaseP) {
releaseHelper.reset(new GrRefCntedCallback(releaseP, releaseC));
}
#endif
if (!ctx) {
return nullptr;
}
@ -241,10 +253,12 @@ sk_sp<SkImage> SkImage::MakeFromTexture(GrContext* ctx,
return nullptr;
}
#ifdef SK_LEGACY_MAKEFROMTEXTURE_BEHAVIOR
sk_sp<GrRefCntedCallback> releaseHelper;
if (releaseP) {
releaseHelper.reset(new GrRefCntedCallback(releaseP, releaseC));
}
#endif
return new_wrapped_texture_common(ctx, tex, grColorType, origin, at, std::move(cs),
kBorrow_GrWrapOwnership, std::move(releaseHelper));
@ -368,6 +382,11 @@ sk_sp<SkImage> SkImage::MakeFromYUVATexturesCopyWithExternalBackend(
ReleaseContext releaseContext) {
const GrCaps* caps = ctx->priv().caps();
sk_sp<GrRefCntedCallback> releaseHelper;
if (textureReleaseProc) {
releaseHelper.reset(new GrRefCntedCallback(textureReleaseProc, releaseContext));
}
GrColorType grColorType = SkColorTypeAndFormatToGrColorType(caps, kRGBA_8888_SkColorType,
backendTexture.getBackendFormat());
if (GrColorType::kUnknown == grColorType) {
@ -384,7 +403,7 @@ sk_sp<SkImage> SkImage::MakeFromYUVATexturesCopyWithExternalBackend(
// in order to draw to it for the yuv->rgb conversion.
auto renderTargetContext = GrRenderTargetContext::MakeFromBackendTexture(
ctx, grColorType, std::move(imageColorSpace), backendTexture, 1, imageOrigin,
nullptr, textureReleaseProc, releaseContext);
nullptr, std::move(releaseHelper));
if (!renderTargetContext) {
return nullptr;
}

36
src/image/SkSurface_Gpu.cpp
View File

@ -422,11 +422,10 @@ sk_sp<SkSurface> SkSurface::MakeFromBackendTexture(GrContext* context,
const GrBackendTexture& backendTexture,
TextureReleaseProc textureReleaseProc,
ReleaseContext releaseContext) {
SkScopeExit callProc([&] {
if (textureReleaseProc) {
textureReleaseProc(releaseContext);
}
});
sk_sp<GrRefCntedCallback> releaseHelper;
if (textureReleaseProc) {
releaseHelper.reset(new GrRefCntedCallback(textureReleaseProc, releaseContext));
}
if (!context || !c.isValid()) {
return nullptr;
@ -454,11 +453,10 @@ sk_sp<SkSurface> SkSurface::MakeFromBackendTexture(GrContext* context,
auto rtc = GrRenderTargetContext::MakeFromBackendTexture(
context, grCT, c.refColorSpace(), backendTexture, c.sampleCount(), c.origin(),
&c.surfaceProps(), textureReleaseProc, releaseContext);
&c.surfaceProps(), std::move(releaseHelper));
if (!rtc) {
return nullptr;
}
callProc.clear();
auto device = SkGpuDevice::Make(context, std::move(rtc), SkGpuDevice::kUninit_InitContents);
if (!device) {
@ -519,11 +517,10 @@ sk_sp<SkSurface> SkSurface::MakeFromBackendTexture(GrContext* context, const GrB
const SkSurfaceProps* props,
SkSurface::TextureReleaseProc textureReleaseProc,
SkSurface::ReleaseContext releaseContext) {
SkScopeExit callProc([&] {
if (textureReleaseProc) {
textureReleaseProc(releaseContext);
}
});
sk_sp<GrRefCntedCallback> releaseHelper;
if (textureReleaseProc) {
releaseHelper.reset(new GrRefCntedCallback(textureReleaseProc, releaseContext));
}
if (!context) {
return nullptr;
@ -542,11 +539,10 @@ sk_sp<SkSurface> SkSurface::MakeFromBackendTexture(GrContext* context, const GrB
auto rtc = GrRenderTargetContext::MakeFromBackendTexture(
context, grColorType, std::move(colorSpace), tex, sampleCnt, origin, props,
textureReleaseProc, releaseContext);
std::move(releaseHelper));
if (!rtc) {
return nullptr;
}
callProc.clear();
auto device = SkGpuDevice::Make(context, std::move(rtc), SkGpuDevice::kUninit_InitContents);
if (!device) {
@ -560,11 +556,10 @@ bool SkSurface_Gpu::onReplaceBackendTexture(const GrBackendTexture& backendTextu
ContentChangeMode mode,
TextureReleaseProc releaseProc,
ReleaseContext releaseContext) {
SkScopeExit callProc([&] {
if (releaseProc) {
releaseProc(releaseContext);
}
});
sk_sp<GrRefCntedCallback> releaseHelper;
if (releaseProc) {
releaseHelper.reset(new GrRefCntedCallback(releaseProc, releaseContext));
}
auto context = this->fDevice->context();
if (context->abandoned()) {
@ -604,11 +599,10 @@ bool SkSurface_Gpu::onReplaceBackendTexture(const GrBackendTexture& backendTextu
}
auto rtc = GrRenderTargetContext::MakeFromBackendTexture(
context, oldRTC->colorInfo().colorType(), std::move(colorSpace), backendTexture,
sampleCnt, origin, &this->props(), releaseProc, releaseContext);
sampleCnt, origin, &this->props(), std::move(releaseHelper));
if (!rtc) {
return false;
}
callProc.clear();
fDevice->replaceRenderTargetContext(std::move(rtc), mode);
return true;
}

2
tests/EGLImageTest.cpp
View File

@ -193,7 +193,7 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(EGLImageTest, reporter, ctxInfo) {
// Should not be able to wrap as a RT
{
auto temp = GrRenderTargetContext::MakeFromBackendTexture(
context0, colorType, nullptr, backendTex, 1, origin, nullptr, nullptr, nullptr);
context0, colorType, nullptr, backendTex, 1, origin, nullptr, nullptr);
if (temp) {
ERRORF(reporter, "Should not be able to wrap an EXTERNAL texture as a RT.");
}

2
tests/GrTestingBackendTextureUploadTest.cpp
View File

@ -65,7 +65,7 @@ void testing_only_texture_test(skiatest::Reporter* reporter, GrContext* context,
sk_sp<GrTextureProxy> wrappedProxy;
if (GrRenderable::kYes == renderable) {
wrappedProxy = context->priv().proxyProvider()->wrapRenderableBackendTexture(
backendTex, 1, kAdopt_GrWrapOwnership, GrWrapCacheable::kNo);
backendTex, 1, kAdopt_GrWrapOwnership, GrWrapCacheable::kNo, nullptr);
} else {
wrappedProxy = context->priv().proxyProvider()->wrapBackendTexture(
backendTex, kAdopt_GrWrapOwnership, GrWrapCacheable::kNo, GrIOType::kRW_GrIOType);

2
tests/MtlCopySurfaceTest.mm
View File

@ -42,7 +42,7 @@ DEF_GPUTEST_FOR_METAL_CONTEXT(MtlCopySurfaceTest, reporter, ctxInfo) {
GrBackendRenderTarget backendRT(kWidth, kHeight, 1, fbInfo);
GrProxyProvider* proxyProvider = context->priv().proxyProvider();
sk_sp<GrSurfaceProxy> srcProxy = proxyProvider->wrapBackendRenderTarget(backendRT);
sk_sp<GrSurfaceProxy> srcProxy = proxyProvider->wrapBackendRenderTarget(backendRT, nullptr);
auto dstProxy = GrSurfaceProxy::Copy(context,
srcProxy.get(),

2
tests/ProxyConversionTest.cpp
View File

@ -26,7 +26,7 @@ static sk_sp<GrSurfaceProxy> make_wrapped_rt(GrProxyProvider* provider,
GrColorType colorType) {
auto backendRT =
gpu->createTestingOnlyBackendRenderTarget(size.width(), size.height(), colorType);
return provider->wrapBackendRenderTarget(backendRT, nullptr, nullptr);
return provider->wrapBackendRenderTarget(backendRT, nullptr);
}
void clean_up_wrapped_rt(GrGpu* gpu, sk_sp<GrSurfaceProxy> proxy) {

6
tests/ProxyTest.cpp
View File

@ -225,7 +225,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(WrappedProxyTest, reporter, ctxInfo) {
GrBackendRenderTarget backendRT = gpu->createTestingOnlyBackendRenderTarget(
kWidthHeight, kWidthHeight, grColorType);
sk_sp<GrSurfaceProxy> sProxy(
proxyProvider->wrapBackendRenderTarget(backendRT, nullptr, nullptr));
proxyProvider->wrapBackendRenderTarget(backendRT, nullptr));
check_surface(reporter, sProxy.get(), kWidthHeight, kWidthHeight, SkBudgeted::kNo);
static constexpr int kExpectedNumSamples = 1;
check_rendertarget(reporter, caps, resourceProvider, sProxy->asRenderTargetProxy(),
@ -253,7 +253,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(WrappedProxyTest, reporter, ctxInfo) {
GrBackendRenderTarget backendRT(kWidthHeight, kWidthHeight, numSamples,
kStencilBits, fboInfo);
sk_sp<GrSurfaceProxy> sProxy(
proxyProvider->wrapBackendRenderTarget(backendRT, nullptr, nullptr));
proxyProvider->wrapBackendRenderTarget(backendRT, nullptr));
check_surface(reporter, sProxy.get(), kWidthHeight, kWidthHeight, SkBudgeted::kNo);
check_rendertarget(reporter, caps, resourceProvider, sProxy->asRenderTargetProxy(),
supportedNumSamples, SkBackingFit::kExact, 0);
@ -290,7 +290,7 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(WrappedProxyTest, reporter, ctxInfo) {
sk_sp<GrSurfaceProxy> sProxy = proxyProvider->wrapRenderableBackendTexture(
backendTex, supportedNumSamples, kBorrow_GrWrapOwnership,
GrWrapCacheable::kNo, nullptr, nullptr);
GrWrapCacheable::kNo, nullptr);
if (!sProxy) {
context->deleteBackendTexture(backendTex);
continue; // This can fail on Mesa