Have GrAHardwareBufferImageGenerator use lazy proxies.
Bug: skia:
Change-Id: I0c98157ec36f056c3c1b217f3b6561e371b4f057
Reviewed-on: https://skia-review.googlesource.com/148987
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Stan Iliev <stani@google.com>
Commit-Queue: Stan Iliev <stani@google.com>
diff --git a/src/gpu/GrAHardwareBufferImageGenerator.cpp b/src/gpu/GrAHardwareBufferImageGenerator.cpp
index e69467c..9d7c27b 100644
--- a/src/gpu/GrAHardwareBufferImageGenerator.cpp
+++ b/src/gpu/GrAHardwareBufferImageGenerator.cpp
@@ -23,6 +23,8 @@
#include "GrTexture.h"
#include "GrTextureProxy.h"
#include "SkMessageBus.h"
+#include "gl/GrGLDefines.h"
+#include "gl/GrGLTypes.h"
#include <EGL/egl.h>
#include <EGL/eglext.h>
@@ -54,33 +56,39 @@
}
GrAHardwareBufferImageGenerator::GrAHardwareBufferImageGenerator(const SkImageInfo& info,
- AHardwareBuffer* graphicBuffer, SkAlphaType alphaType)
+ AHardwareBuffer* hardwareBuffer, SkAlphaType alphaType)
: INHERITED(info)
- , fGraphicBuffer(graphicBuffer) {
- AHardwareBuffer_acquire(fGraphicBuffer);
+ , fHardwareBuffer(hardwareBuffer) {
+ 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() {
- AHardwareBuffer_release(fGraphicBuffer);
- this->clear();
-}
-
-void GrAHardwareBufferImageGenerator::clear() {
- if (fOriginalTexture) {
- // Notify the original cache that it can free the last ref, so it happens on the correct
- // thread.
- GrGpuResourceFreedMessage msg { fOriginalTexture, fOwningContextID };
- SkMessageBus<GrGpuResourceFreedMessage>::Post(msg);
- fOriginalTexture = nullptr;
- }
+ this->releaseTextureRef();
+ AHardwareBuffer_release(fHardwareBuffer);
}
///////////////////////////////////////////////////////////////////////////////////////////////////
sk_sp<GrTextureProxy> GrAHardwareBufferImageGenerator::onGenerateTexture(
GrContext* context, const SkImageInfo& info, const SkIPoint& origin, bool willNeedMipMaps) {
- auto proxy = this->makeProxy(context);
- if (!proxy) {
+ this->makeProxy(context);
+ if (!fCachedProxy) {
return nullptr;
}
@@ -88,9 +96,9 @@
if (0 == origin.fX && 0 == origin.fY &&
info.width() == this->getInfo().width() && info.height() == this->getInfo().height()) {
makingASubset = false;
- if (!willNeedMipMaps || GrMipMapped::kYes == proxy->mipMapped()) {
+ if (!willNeedMipMaps || GrMipMapped::kYes == fCachedProxy->mipMapped()) {
// If the caller wants the full texture and we have the correct mip support, we're done
- return proxy;
+ return fCachedProxy;
}
}
// Otherwise, make a copy for the requested subset or for mip maps.
@@ -98,7 +106,7 @@
GrMipMapped mipMapped = willNeedMipMaps ? GrMipMapped::kYes : GrMipMapped::kNo;
- sk_sp<GrTextureProxy> texProxy = GrSurfaceProxy::Copy(context, proxy.get(), mipMapped,
+ 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
@@ -112,14 +120,11 @@
// should match.
SkASSERT(context->uniqueID() == fOwningContextID);
- // Clear out the old cached texture.
- this->clear();
+ // 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();
- // We need to get the actual GrTexture so force instantiation of the GrTextureProxy
- texProxy->instantiate(context->contextPriv().resourceProvider());
- GrTexture* texture = texProxy->peekTexture();
- SkASSERT(texture);
- fOriginalTexture = texture;
+ fCachedProxy = texProxy;
}
return texProxy;
}
@@ -130,6 +135,7 @@
: fImage(image)
, fDisplay(display) { }
~BufferCleanupHelper() {
+ // eglDestroyImageKHR will remove a ref from the AHardwareBuffer
eglDestroyImageKHR(fDisplay, fImage);
}
private:
@@ -145,7 +151,7 @@
static GrBackendTexture make_gl_backend_texture(
GrContext* context, AHardwareBuffer* hardwareBuffer,
- int width, int height,
+ int width, int height, GrPixelConfig config,
GrAHardwareBufferImageGenerator::DeleteImageProc* deleteProc,
GrAHardwareBufferImageGenerator::DeleteImageCtx* deleteCtx) {
while (GL_NO_ERROR != glGetError()) {} //clear GL errors
@@ -154,6 +160,7 @@
EGLint attribs[] = { EGL_IMAGE_PRESERVED_KHR, EGL_TRUE,
EGL_NONE };
EGLDisplay display = eglGetCurrentDisplay();
+ // eglCreateImageKHR will add a ref to the AHardwareBuffer
EGLImageKHR image = eglCreateImageKHR(display, EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID,
clientBuffer, attribs);
if (EGL_NO_IMAGE_KHR == image) {
@@ -187,6 +194,19 @@
GrGLTextureInfo textureInfo;
textureInfo.fTarget = GL_TEXTURE_EXTERNAL_OES;
textureInfo.fID = texID;
+ switch (config) {
+ case kRGBA_8888_GrPixelConfig:
+ textureInfo.fFormat = GR_GL_RGBA8;
+ break;
+ case kRGBA_half_GrPixelConfig:
+ textureInfo.fFormat = GR_GL_RGBA16F;
+ break;
+ case kRGB_565_GrPixelConfig:
+ textureInfo.fFormat = GR_GL_RGB565;
+ break;
+ default:
+ SkASSERT(false);
+ }
*deleteProc = GrAHardwareBufferImageGenerator::DeleteEGLImage;
*deleteCtx = new BufferCleanupHelper(image, display);
@@ -196,14 +216,15 @@
static GrBackendTexture make_backend_texture(
GrContext* context, AHardwareBuffer* hardwareBuffer,
- int width, int height,
+ int width, int height, GrPixelConfig config,
GrAHardwareBufferImageGenerator::DeleteImageProc* deleteProc,
GrAHardwareBufferImageGenerator::DeleteImageCtx* deleteCtx) {
if (context->abandoned() || kOpenGL_GrBackend != context->contextPriv().getBackend()) {
// Check if GrContext is not abandoned and the backend is GL.
return GrBackendTexture();
}
- return make_gl_backend_texture(context, hardwareBuffer, width, height, deleteProc, deleteCtx);
+ return make_gl_backend_texture(context, hardwareBuffer, width, height, config, deleteProc,
+ deleteCtx);
}
static void free_backend_texture(GrBackendTexture* backendTexture) {
@@ -222,19 +243,23 @@
}
}
-sk_sp<GrTextureProxy> GrAHardwareBufferImageGenerator::makeProxy(GrContext* context) {
+void GrAHardwareBufferImageGenerator::makeProxy(GrContext* context) {
if (context->abandoned() || kOpenGL_GrBackend != context->contextPriv().getBackend()) {
// Check if GrContext is not abandoned and the backend is GL.
- return nullptr;
+ return;
}
- auto proxyProvider = context->contextPriv().proxyProvider();
-
- // return a cached GrTexture if invoked with the same context
- if (fOriginalTexture && fOwningContextID == context->uniqueID()) {
- return proxyProvider->createWrapped(sk_ref_sp(fOriginalTexture),
- kTopLeft_GrSurfaceOrigin);
+ if (SK_InvalidGenID != fOwningContextID) {
+ SkASSERT(fCachedProxy);
+ if (context->uniqueID() != fOwningContextID) {
+ this->releaseTextureRef();
+ } else {
+ return;
+ }
}
+ SkASSERT(!fCachedProxy);
+
+ fOwningContextID = context->uniqueID();
GrPixelConfig pixelConfig;
switch (this->getInfo().colorType()) {
@@ -248,48 +273,82 @@
pixelConfig = kRGB_565_GrPixelConfig;
break;
default:
- return nullptr;
+ return;
}
- DeleteImageProc deleteImageProc = nullptr;
- DeleteImageCtx deleteImageCtx = nullptr;
+ int width = this->getInfo().width();
+ int height = this->getInfo().height();
- GrBackendTexture backendTex = make_backend_texture(context, fGraphicBuffer,
- this->getInfo().width(),
- this->getInfo().height(),
- &deleteImageProc,
- &deleteImageCtx);
+ GrSurfaceDesc desc;
+ desc.fWidth = width;
+ desc.fHeight = height;
+ desc.fConfig = pixelConfig;
- if (!backendTex.isValid()) {
- return nullptr;
- }
- SkASSERT(deleteImageProc && deleteImageCtx);
- backendTex.fConfig = pixelConfig;
- sk_sp<GrTexture> tex = context->contextPriv().resourceProvider()->wrapBackendTexture(
- backendTex, kAdopt_GrWrapOwnership);
- if (!tex) {
- free_backend_texture(&backendTex);
- deleteImageProc(deleteImageCtx);
- return nullptr;
+ GrTextureType textureType = GrTextureType::k2D;
+ if (context->contextPriv().getBackend() == kOpenGL_GrBackend) {
+ textureType = GrTextureType::kExternal;
}
- sk_sp<GrReleaseProcHelper> releaseProcHelper(
+ auto proxyProvider = context->contextPriv().proxyProvider();
+
+ AHardwareBuffer* hardwareBuffer = fHardwareBuffer;
+ AHardwareBuffer_acquire(hardwareBuffer);
+
+ GrTexture** ownedTexturePtr = &fOwnedTexture;
+
+ fCachedProxy = proxyProvider->createLazyProxy(
+ [context, hardwareBuffer, width, height, pixelConfig, ownedTexturePtr]
+ (GrResourceProvider* resourceProvider) {
+ if (!resourceProvider) {
+ AHardwareBuffer_release(hardwareBuffer);
+ return sk_sp<GrTexture>();
+ }
+
+ SkASSERT(!*ownedTexturePtr);
+
+ DeleteImageProc deleteImageProc = nullptr;
+ DeleteImageCtx deleteImageCtx = nullptr;
+
+ GrBackendTexture backendTex = make_backend_texture(context, hardwareBuffer,
+ width, height, pixelConfig,
+ &deleteImageProc,
+ &deleteImageCtx);
+ if (!backendTex.isValid()) {
+ return sk_sp<GrTexture>();
+ }
+ SkASSERT(deleteImageProc && deleteImageCtx);
+
+ backendTex.fConfig = pixelConfig;
+ sk_sp<GrTexture> tex = resourceProvider->wrapBackendTexture(
+ backendTex, kAdopt_GrWrapOwnership);
+ if (!tex) {
+ free_backend_texture(&backendTex);
+ deleteImageProc(deleteImageCtx);
+ return sk_sp<GrTexture>();
+ }
+
+ sk_sp<GrReleaseProcHelper> releaseProcHelper(
new GrReleaseProcHelper(deleteImageProc, deleteImageCtx));
- tex->setRelease(std::move(releaseProcHelper));
+ tex->setRelease(releaseProcHelper);
- // We fail this assert, if the context has changed. This will be fully handled after
- // skbug.com/6812 is ready.
- SkASSERT(!fOriginalTexture);
+ *ownedTexturePtr = tex.get();
- this->clear();
- fOriginalTexture = tex.get();
- fOwningContextID = context->uniqueID();
- // 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(fOriginalTexture);
- return proxyProvider->createWrapped(std::move(tex), kTopLeft_GrSurfaceOrigin);
+ // 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) {
+ AHardwareBuffer_release(hardwareBuffer);
+ return;
+ }
}
bool GrAHardwareBufferImageGenerator::onIsValid(GrContext* context) const {
diff --git a/src/gpu/GrAHardwareBufferImageGenerator.h b/src/gpu/GrAHardwareBufferImageGenerator.h
index a89e09d..58b5abf 100644
--- a/src/gpu/GrAHardwareBufferImageGenerator.h
+++ b/src/gpu/GrAHardwareBufferImageGenerator.h
@@ -9,6 +9,8 @@
#include "SkImageGenerator.h"
+#include "GrTypesPriv.h"
+
extern "C" {
typedef struct AHardwareBuffer AHardwareBuffer;
}
@@ -46,12 +48,21 @@
private:
GrAHardwareBufferImageGenerator(const SkImageInfo&, AHardwareBuffer*, SkAlphaType);
- sk_sp<GrTextureProxy> makeProxy(GrContext* context);
- void clear();
+ void makeProxy(GrContext* context);
- AHardwareBuffer* fGraphicBuffer;
- GrTexture* fOriginalTexture = nullptr;
- uint32_t fOwningContextID;
+ 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.
+ GrTexture* fOwnedTexture = nullptr;
+ uint32_t fOwningContextID = SK_InvalidGenID;
+
+ sk_sp<GrTextureProxy> fCachedProxy;
typedef SkImageGenerator INHERITED;
};
diff --git a/src/gpu/GrBackendTextureImageGenerator.cpp b/src/gpu/GrBackendTextureImageGenerator.cpp
index 74722c6..212aded 100644
--- a/src/gpu/GrBackendTextureImageGenerator.cpp
+++ b/src/gpu/GrBackendTextureImageGenerator.cpp
@@ -139,8 +139,8 @@
textureType = GrGLTexture::TextureTypeFromTarget(glInfo.fTarget);
}
sk_sp<GrTextureProxy> proxy = proxyProvider->createLazyProxy(
- [refHelper, releaseProcHelper, semaphore,
- backendTexture](GrResourceProvider* resourceProvider) {
+ [refHelper, releaseProcHelper, semaphore, backendTexture]
+ (GrResourceProvider* resourceProvider) {
if (!resourceProvider) {
return sk_sp<GrTexture>();
}