Use destructors to free resources owned by lazy proxy callbacks
Change-Id: I103355f12e808c636803491c86d3c887113f088c
Reviewed-on: https://skia-review.googlesource.com/c/192541
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrAHardwareBufferImageGenerator.cpp b/src/gpu/GrAHardwareBufferImageGenerator.cpp
index d42b491..9b75c3e 100644
--- a/src/gpu/GrAHardwareBufferImageGenerator.cpp
+++ b/src/gpu/GrAHardwareBufferImageGenerator.cpp
@@ -26,6 +26,7 @@
#include "GrResourceProviderPriv.h"
#include "GrTexture.h"
#include "GrTextureProxy.h"
+#include "SkExchange.h"
#include "SkMessageBus.h"
#include "gl/GrGLDefines.h"
#include "gl/GrGLTypes.h"
@@ -120,19 +121,34 @@
const bool isProtectedContent = fIsProtectedContent;
- sk_sp<GrTextureProxy> texProxy = proxyProvider->createLazyProxy(
- [context, hardwareBuffer, width, height, pixelConfig, isProtectedContent,
- backendFormat](GrResourceProvider* resourceProvider) {
- if (!resourceProvider) {
- AHardwareBuffer_release(hardwareBuffer);
- return sk_sp<GrTexture>();
- }
+ class AutoAHBRelease {
+ public:
+ AutoAHBRelease(AHardwareBuffer* ahb) : fAhb(ahb) {}
+ // std::function() must be CopyConstructible, but ours should never actually be copied.
+ AutoAHBRelease(const AutoAHBRelease&) { SkASSERT(0); }
+ AutoAHBRelease(AutoAHBRelease&& that) : fAhb(that.fAhb) { that.fAhb = nullptr; }
+ ~AutoAHBRelease() { fAhb ? AHardwareBuffer_release(fAhb) : void(); }
+ AutoAHBRelease& operator=(AutoAHBRelease&& that) {
+ fAhb = skstd::exchange(that.fAhb, nullptr);
+ return *this;
+ }
+ AutoAHBRelease& operator=(const AutoAHBRelease&) = delete;
+
+ AHardwareBuffer* get() const { return fAhb; }
+
+ private:
+ AHardwareBuffer* fAhb;
+ };
+
+ sk_sp<GrTextureProxy> texProxy = proxyProvider->createLazyProxy(
+ [context, buffer = AutoAHBRelease(hardwareBuffer), width, height, pixelConfig,
+ isProtectedContent, backendFormat](GrResourceProvider* resourceProvider) {
GrAHardwareBufferUtils::DeleteImageProc deleteImageProc = nullptr;
GrAHardwareBufferUtils::DeleteImageCtx deleteImageCtx = nullptr;
GrBackendTexture backendTex =
- GrAHardwareBufferUtils::MakeBackendTexture(context, hardwareBuffer,
+ GrAHardwareBufferUtils::MakeBackendTexture(context, buffer.get(),
width, height,
&deleteImageProc,
&deleteImageCtx,
@@ -166,9 +182,6 @@
backendFormat, desc, fSurfaceOrigin, GrMipMapped::kNo,
GrInternalSurfaceFlags::kReadOnly, SkBackingFit::kExact, SkBudgeted::kNo);
- if (!texProxy) {
- AHardwareBuffer_release(hardwareBuffer);
- }
return texProxy;
}
diff --git a/src/gpu/GrBackendTextureImageGenerator.cpp b/src/gpu/GrBackendTextureImageGenerator.cpp
index b968e141..c47efd6 100644
--- a/src/gpu/GrBackendTextureImageGenerator.cpp
+++ b/src/gpu/GrBackendTextureImageGenerator.cpp
@@ -145,10 +145,6 @@
sk_sp<GrTextureProxy> proxy = proxyProvider->createLazyProxy(
[refHelper, releaseProcHelper, semaphore,
backendTexture](GrResourceProvider* resourceProvider) {
- if (!resourceProvider) {
- return sk_sp<GrTexture>();
- }
-
if (semaphore) {
resourceProvider->priv().gpu()->waitSemaphore(semaphore);
}
diff --git a/src/gpu/GrProxyProvider.cpp b/src/gpu/GrProxyProvider.cpp
index a7b7213..64b3e95 100644
--- a/src/gpu/GrProxyProvider.cpp
+++ b/src/gpu/GrProxyProvider.cpp
@@ -247,11 +247,6 @@
sk_sp<GrTextureProxy> proxy = this->createLazyProxy(
[desc, budgeted, srcImage, fit, surfaceFlags](GrResourceProvider* resourceProvider) {
- if (!resourceProvider) {
- // Nothing to clean up here. Once the proxy (and thus lambda) is deleted the ref
- // on srcImage will be released.
- return sk_sp<GrTexture>();
- }
SkPixmap pixMap;
SkAssertResult(srcImage->peekPixels(&pixMap));
GrMipLevel mipLevel = { pixMap.addr(), pixMap.rowBytes() };
@@ -355,10 +350,6 @@
sk_sp<GrTextureProxy> proxy = this->createLazyProxy(
[desc, baseLevel, mipmaps](GrResourceProvider* resourceProvider) {
- if (!resourceProvider) {
- return sk_sp<GrTexture>();
- }
-
const int mipLevelCount = mipmaps->countLevels() + 1;
std::unique_ptr<GrMipLevel[]> texels(new GrMipLevel[mipLevelCount]);
@@ -447,10 +438,6 @@
sk_sp<GrTextureProxy> proxy = this->createLazyProxy(
[desc, data](GrResourceProvider* resourceProvider) {
- if (!resourceProvider) {
- return sk_sp<GrTexture>();
- }
-
GrMipLevel texels;
texels.fPixels = data->data();
texels.fRowBytes = GrBytesPerPixel(desc.fConfig)*desc.fWidth;
diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp
index 6051f51..f2e4485 100644
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -100,11 +100,6 @@
}
GrSurfaceProxy::~GrSurfaceProxy() {
- if (fLazyInstantiateCallback) {
- // We call the callback with a null GrResourceProvider to signal that the lambda should
- // clean itself up if it is holding onto any captured objects.
- this->fLazyInstantiateCallback(nullptr);
- }
// For this to be deleted the opList that held a ref on it (if there was one) must have been
// deleted. Which would have cleared out this back pointer.
SkASSERT(!fLastOpList);
@@ -438,7 +433,6 @@
surface = fProxy->fLazyInstantiateCallback(resourceProvider);
}
if (GrSurfaceProxy::LazyInstantiationType::kSingleUse == fProxy->fLazyInstantiationType) {
- fProxy->fLazyInstantiateCallback(nullptr);
fProxy->fLazyInstantiateCallback = nullptr;
}
if (!surface) {
diff --git a/src/image/SkImage_GpuBase.cpp b/src/image/SkImage_GpuBase.cpp
index 648f862..fcf0887 100644
--- a/src/image/SkImage_GpuBase.cpp
+++ b/src/image/SkImage_GpuBase.cpp
@@ -427,12 +427,6 @@
~PromiseLazyInstantiateCallback() = default;
sk_sp<GrSurface> operator()(GrResourceProvider* resourceProvider) {
- if (!resourceProvider) {
- if (fDelayedReleaseTexture) {
- fDelayedReleaseTexture.reset();
- }
- return nullptr;
- }
if (fDelayedReleaseTexture) {
return fDelayedReleaseTexture;
}
diff --git a/tests/GrSurfaceTest.cpp b/tests/GrSurfaceTest.cpp
index 39c6d76..bedecc2 100644
--- a/tests/GrSurfaceTest.cpp
+++ b/tests/GrSurfaceTest.cpp
@@ -453,7 +453,7 @@
auto rt = SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info, 0, nullptr);
auto rtc = rt->getCanvas()->internal_private_accessTopLayerRenderTargetContext();
auto singleUseLazyCB = [&texture](GrResourceProvider* rp) {
- return rp ? std::move(texture) : nullptr;
+ return std::move(texture);
};
GrSurfaceDesc desc;
desc.fWidth = desc.fHeight = kS;
@@ -497,7 +497,7 @@
// Make a proxy that should deinstantiate even if we keep a ref on it.
auto deinstantiateLazyCB = [&make, &context](GrResourceProvider* rp) {
- return rp ? make(context, 3) : nullptr;
+ return make(context, 3);
};
proxy = context->priv().proxyProvider()->createLazyProxy(
deinstantiateLazyCB, backendFormat, desc,
diff --git a/tests/LazyProxyTest.cpp b/tests/LazyProxyTest.cpp
index 995355f..e3fa1b8 100644
--- a/tests/LazyProxyTest.cpp
+++ b/tests/LazyProxyTest.cpp
@@ -19,6 +19,7 @@
#include "GrTexture.h"
#include "GrTextureProxy.h"
#include "GrTextureProxyPriv.h"
+#include "SkExchange.h"
#include "SkMakeUnique.h"
#include "SkRectPriv.h"
#include "mock/GrMockGpu.h"
@@ -250,18 +251,33 @@
LazyInstantiationType::kMultipleUse,
LazyInstantiationType::kDeinstantiate}) {
int testCount = 0;
- int* testCountPtr = &testCount;
+ // Sets an integer to 1 when the callback is called and -1 when it is deleted.
+ class TestCallback {
+ public:
+ TestCallback(int* value) : fValue(value) {}
+ TestCallback(const TestCallback& that) { SkASSERT(0); }
+ TestCallback(TestCallback&& that) : fValue(that.fValue) { that.fValue = nullptr; }
+
+ ~TestCallback() { fValue ? (void)(*fValue = -1) : void(); }
+
+ TestCallback& operator=(TestCallback&& that) {
+ fValue = skstd::exchange(that.fValue, nullptr);
+ return *this;
+ }
+ TestCallback& operator=(const TestCallback& that) = delete;
+
+ sk_sp<GrSurface> operator()(GrResourceProvider* resourceProvider) const {
+ *fValue = 1;
+ return {};
+ }
+
+ private:
+ int* fValue = nullptr;
+ };
sk_sp<GrTextureProxy> proxy = proxyProvider->createLazyProxy(
- [testCountPtr](GrResourceProvider* resourceProvider) {
- if (!resourceProvider) {
- *testCountPtr = -1;
- return sk_sp<GrTexture>();
- }
- *testCountPtr = 1;
- return sk_sp<GrTexture>();
- },
- format, desc, kTopLeft_GrSurfaceOrigin, GrMipMapped::kNo,
- GrInternalSurfaceFlags::kNone, SkBackingFit::kExact, SkBudgeted::kNo, lazyType);
+ TestCallback(&testCount), format, desc, kTopLeft_GrSurfaceOrigin,
+ GrMipMapped::kNo, GrInternalSurfaceFlags::kNone, SkBackingFit::kExact,
+ SkBudgeted::kNo, lazyType);
REPORTER_ASSERT(reporter, proxy.get());
REPORTER_ASSERT(reporter, 0 == testCount);
@@ -320,9 +336,6 @@
fLazyProxy = proxyProvider->createLazyProxy(
[testExecuteValue, shouldFailInstantiation, desc](GrResourceProvider* rp) {
- if (!rp) {
- return sk_sp<GrTexture>();
- }
if (shouldFailInstantiation) {
*testExecuteValue = 1;
return sk_sp<GrTexture>();
@@ -463,10 +476,6 @@
sk_sp<GrTextureProxy> lazyProxy = proxyProvider->createLazyProxy(
[instantiatePtr, releasePtr, backendTex](GrResourceProvider* rp) {
- if (!rp) {
- return sk_sp<GrTexture>();
- }
-
sk_sp<GrTexture> texture =
rp->wrapBackendTexture(backendTex, kBorrow_GrWrapOwnership,
GrWrapCacheable::kNo, kRead_GrIOType);