Remove caching of textures in GrAHardwareBufferImageGenerator.
We will rely on the UniqueKey system controlled up the stack by
SkImage_Lazy to avoid making redudent GrTextureProxies.
Bug: skia:
Change-Id: I64ee38ecd8651cc7d5e062b6d47a1871b752772a
Reviewed-on: https://skia-review.googlesource.com/152384
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrAHardwareBufferImageGenerator.cpp b/src/gpu/GrAHardwareBufferImageGenerator.cpp
index 4920f1c..359aa38 100644
--- a/src/gpu/GrAHardwareBufferImageGenerator.cpp
+++ b/src/gpu/GrAHardwareBufferImageGenerator.cpp
@@ -105,90 +105,32 @@
AHardwareBuffer_acquire(fHardwareBuffer);
}
-void GrAHardwareBufferImageGenerator::releaseTextureRef() {
- // We must release our ref on the proxy before we send the free message to the actual texture so
- // that we make sure the last ref (if it is owned by this class) is released on the owning
- // context.
- fCachedProxy.reset();
- if (fOwnedTexture) {
- SkASSERT(fOwningContextID != SK_InvalidGenID);
- // Notify the original cache that it can free the last ref, so it happens on the correct
- // thread.
- GrGpuResourceFreedMessage msg { fOwnedTexture, fOwningContextID };
- SkMessageBus<GrGpuResourceFreedMessage>::Post(msg);
- fOwnedTexture = nullptr;
- fOwningContextID = SK_InvalidGenID;
- }
-}
-
GrAHardwareBufferImageGenerator::~GrAHardwareBufferImageGenerator() {
- this->releaseTextureRef();
AHardwareBuffer_release(fHardwareBuffer);
}
///////////////////////////////////////////////////////////////////////////////////////////////////
-sk_sp<GrTextureProxy> GrAHardwareBufferImageGenerator::onGenerateTexture(
- GrContext* context, const SkImageInfo& info, const SkIPoint& origin, bool willNeedMipMaps) {
- this->makeProxy(context);
- if (!fCachedProxy) {
- return nullptr;
- }
-
- bool makingASubset = true;
- if (0 == origin.fX && 0 == origin.fY &&
- info.width() == this->getInfo().width() && info.height() == this->getInfo().height()) {
- makingASubset = false;
- if (!willNeedMipMaps || GrMipMapped::kYes == fCachedProxy->mipMapped()) {
- // If the caller wants the full texture and we have the correct mip support, we're done
- return fCachedProxy;
- }
- }
- // Otherwise, make a copy for the requested subset or for mip maps.
- SkIRect subset = SkIRect::MakeXYWH(origin.fX, origin.fY, info.width(), info.height());
-
- GrMipMapped mipMapped = willNeedMipMaps ? GrMipMapped::kYes : GrMipMapped::kNo;
-
- sk_sp<GrTextureProxy> texProxy = GrSurfaceProxy::Copy(context, fCachedProxy.get(), mipMapped,
- subset, SkBudgeted::kYes);
- if (!makingASubset && texProxy) {
- // We are in this case if we wanted the full texture, but we will be mip mapping the
- // texture. Therefore we want to update the cached texture so that we point to the
- // mipped version instead of the old one.
- SkASSERT(willNeedMipMaps);
- SkASSERT(GrMipMapped::kYes == texProxy->mipMapped());
-
- // The only way we should get into here is if we just made a new texture in makeProxy or
- // we found a cached texture in the same context. Thus the current and cached contexts
- // should match.
- SkASSERT(context->uniqueID() == fOwningContextID);
-
- // Since we no longer will be caching the and reusing the texture that actually wraps the
- // hardware buffer, we can release our refs on it.
- this->releaseTextureRef();
-
- fCachedProxy = texProxy;
- }
- return texProxy;
-}
-
-class BufferCleanupHelper {
+class GLCleanupHelper {
public:
- BufferCleanupHelper(EGLImageKHR image, EGLDisplay display)
- : fImage(image)
+ GLCleanupHelper(GrGLuint texID, EGLImageKHR image, EGLDisplay display)
+ : fTexID(texID)
+ , fImage(image)
, fDisplay(display) { }
- ~BufferCleanupHelper() {
+ ~GLCleanupHelper() {
+ glDeleteTextures(1, &fTexID);
// eglDestroyImageKHR will remove a ref from the AHardwareBuffer
eglDestroyImageKHR(fDisplay, fImage);
}
private:
+ GrGLuint fTexID;
EGLImageKHR fImage;
- EGLDisplay fDisplay;
+ EGLDisplay fDisplay;
};
-void GrAHardwareBufferImageGenerator::DeleteEGLImage(void* context) {
- BufferCleanupHelper* cleanupHelper = static_cast<BufferCleanupHelper*>(context);
+void GrAHardwareBufferImageGenerator::DeleteGLTexture(void* context) {
+ GLCleanupHelper* cleanupHelper = static_cast<GLCleanupHelper*>(context);
delete cleanupHelper;
}
@@ -244,8 +186,8 @@
textureInfo.fTarget = *backendFormat.getGLTarget();
textureInfo.fFormat = *backendFormat.getGLFormat();
- *deleteProc = GrAHardwareBufferImageGenerator::DeleteEGLImage;
- *deleteCtx = new BufferCleanupHelper(image, display);
+ *deleteProc = GrAHardwareBufferImageGenerator::DeleteGLTexture;
+ *deleteCtx = new GLCleanupHelper(texID, image, display);
return GrBackendTexture(width, height, GrMipMapped::kNo, textureInfo);
}
@@ -266,22 +208,6 @@
deleteCtx, createProtectedImage, backendFormat);
}
-static void free_backend_texture(GrBackendTexture* backendTexture) {
- SkASSERT(backendTexture && backendTexture->isValid());
-
- switch (backendTexture->backend()) {
- case kOpenGL_GrBackend: {
- GrGLTextureInfo texInfo;
- SkAssertResult(backendTexture->getGLTextureInfo(&texInfo));
- glDeleteTextures(1, &texInfo.fID);
- return;
- }
- case kVulkan_GrBackend: // fall through
- default:
- return;
- }
-}
-
GrBackendFormat get_backend_format(GrBackend backend, uint32_t bufferFormat) {
if (backend == kOpenGL_GrBackend) {
switch (bufferFormat) {
@@ -304,30 +230,18 @@
return GrBackendFormat();
}
-void GrAHardwareBufferImageGenerator::makeProxy(GrContext* context) {
+sk_sp<GrTextureProxy> GrAHardwareBufferImageGenerator::makeProxy(GrContext* context) {
if (context->abandoned() || kOpenGL_GrBackend != context->contextPriv().getBackend()) {
// Check if GrContext is not abandoned and the backend is GL.
- return;
+ return nullptr;
}
- if (SK_InvalidGenID != fOwningContextID) {
- SkASSERT(fCachedProxy);
- if (context->uniqueID() != fOwningContextID) {
- this->releaseTextureRef();
- } else {
- return;
- }
- }
- SkASSERT(!fCachedProxy);
-
- fOwningContextID = context->uniqueID();
-
GrPixelConfig pixelConfig;
GrBackendFormat backendFormat = get_backend_format(context->contextPriv().getBackend(),
fBufferFormat);
if (!context->contextPriv().caps()->getConfigFromBackendFormat(
backendFormat, this->getInfo().colorType(), &pixelConfig)) {
- return;
+ return nullptr;
}
int width = this->getInfo().width();
@@ -348,20 +262,16 @@
AHardwareBuffer* hardwareBuffer = fHardwareBuffer;
AHardwareBuffer_acquire(hardwareBuffer);
- GrGpuResource** ownedTexturePtr = &fOwnedTexture;
const bool isProtectedContent = fIsProtectedContent;
- fCachedProxy = proxyProvider->createLazyProxy(
- [context, hardwareBuffer, width, height, pixelConfig, ownedTexturePtr,
- isProtectedContent, backendFormat]
+ sk_sp<GrTextureProxy> texProxy = proxyProvider->createLazyProxy(
+ [context, hardwareBuffer, width, height, pixelConfig, isProtectedContent, backendFormat]
(GrResourceProvider* resourceProvider) {
if (!resourceProvider) {
AHardwareBuffer_release(hardwareBuffer);
return sk_sp<GrTexture>();
}
- SkASSERT(!*ownedTexturePtr);
-
DeleteImageProc deleteImageProc = nullptr;
DeleteImageCtx deleteImageCtx = nullptr;
@@ -377,10 +287,8 @@
SkASSERT(deleteImageProc && deleteImageCtx);
backendTex.fConfig = pixelConfig;
- sk_sp<GrTexture> tex = resourceProvider->wrapBackendTexture(
- backendTex, kAdopt_GrWrapOwnership);
+ sk_sp<GrTexture> tex = resourceProvider->wrapBackendTexture(backendTex);
if (!tex) {
- free_backend_texture(&backendTex);
deleteImageProc(deleteImageCtx);
return sk_sp<GrTexture>();
}
@@ -389,24 +297,36 @@
new GrReleaseProcHelper(deleteImageProc, deleteImageCtx));
tex->setRelease(releaseProcHelper);
- *ownedTexturePtr = tex.get();
-
- // Attach our texture to this context's resource cache. This ensures that deletion
- // will happen in the correct thread/context. This adds the only ref to the texture
- // that will persist from this point. To trigger GrTexture deletion a message is
- // sent by generator dtor or by makeProxy when it is invoked with a different
- // context.
- context->contextPriv().getResourceCache()->insertCrossContextGpuResource(tex.get());
return tex;
},
desc, kTopLeft_GrSurfaceOrigin, GrMipMapped::kNo, textureType, SkBackingFit::kExact,
SkBudgeted::kNo);
-
- if (!fCachedProxy) {
+ if (!texProxy) {
AHardwareBuffer_release(hardwareBuffer);
- return;
}
+ return texProxy;
+}
+
+sk_sp<GrTextureProxy> GrAHardwareBufferImageGenerator::onGenerateTexture(
+ GrContext* context, const SkImageInfo& info, const SkIPoint& origin, bool willNeedMipMaps) {
+ sk_sp<GrTextureProxy> texProxy = this->makeProxy(context);
+ if (!texProxy) {
+ return nullptr;
+ }
+
+ if (0 == origin.fX && 0 == origin.fY &&
+ info.width() == this->getInfo().width() && info.height() == this->getInfo().height()) {
+ // If the caller wants the full texture we're done. The caller will handle making a copy for
+ // mip maps if that is required.
+ return texProxy;
+ }
+ // Otherwise, make a copy for the requested subset.
+ SkIRect subset = SkIRect::MakeXYWH(origin.fX, origin.fY, info.width(), info.height());
+
+ GrMipMapped mipMapped = willNeedMipMaps ? GrMipMapped::kYes : GrMipMapped::kNo;
+
+ return GrSurfaceProxy::Copy(context, texProxy.get(), mipMapped, subset, SkBudgeted::kYes);
}
bool GrAHardwareBufferImageGenerator::onIsValid(GrContext* context) const {
diff --git a/src/gpu/GrAHardwareBufferImageGenerator.h b/src/gpu/GrAHardwareBufferImageGenerator.h
index ecf5d64..f62741d 100644
--- a/src/gpu/GrAHardwareBufferImageGenerator.h
+++ b/src/gpu/GrAHardwareBufferImageGenerator.h
@@ -38,7 +38,7 @@
typedef void* DeleteImageCtx;
typedef void (*DeleteImageProc)(DeleteImageCtx);
- static void DeleteEGLImage(void* ctx);
+ static void DeleteGLTexture(void* ctx);
protected:
@@ -51,32 +51,15 @@
private:
GrAHardwareBufferImageGenerator(const SkImageInfo&, AHardwareBuffer*, SkAlphaType,
bool isProtectedContent, uint32_t bufferFormat);
- void makeProxy(GrContext* context);
+ sk_sp<GrTextureProxy> makeProxy(GrContext* context);
void releaseTextureRef();
static void ReleaseRefHelper_TextureReleaseProc(void* ctx);
AHardwareBuffer* fHardwareBuffer;
-
- // There is never a ref associated with this pointer. We rely on our atomic bookkeeping
- // with the context ID to know when this pointer is valid and safe to use. This lets us
- // avoid releasing a ref from another thread, or get into races during context shutdown.
- //
- // We store this object as a GrGpuResource* and not a GrTexture* even though we are always
- // using a GrTexutre. The reason for this is that it is possible for the underlying GrTexture
- // object to get freed before this class sends its unref message (i.e. if the owning GrContext
- // is destroyed). In this case, when we try to create the unfef message to be posted, we end up
- // casting the GrTexture* to a GrGpuResource*. Since GrTexture has virtual inheritance, this
- // cast causes us to dereference the vptr to get the offset to the base pointer. In other words
- // casting with virtual inheritance counts as a use and we hit a use after free issue. Thus if
- // we store a GrGpuResource* here instead then we don't run into the issue of needing a cast.
- GrGpuResource* fOwnedTexture = nullptr;
- uint32_t fOwningContextID = SK_InvalidGenID;
- uint32_t fBufferFormat;
-
- sk_sp<GrTextureProxy> fCachedProxy;
- const bool fIsProtectedContent;
+ uint32_t fBufferFormat;
+ const bool fIsProtectedContent;
typedef SkImageGenerator INHERITED;
};