Bug 712716 - Fix AndroidGraphicBuffer resource management. r=snorp,jmuizelaar

gfx/gl/GLContext.h

@@ -137,6 +137,7 @@ enum ShaderProgramType {
     ComponentAlphaPass2ProgramType,
     Copy2DProgramType,
     Copy2DRectProgramType,
+    Copy2DExternalProgramType,
     NumProgramTypes
 };
 

gfx/layers/opengl/LayerManagerOGL.cpp

@@ -246,6 +246,11 @@ LayerManagerOGL::Initialize(nsRefPtr<GLContext> aContext, bool force)
   SHADER_PROGRAM(Copy2DRectProgramType, CopyProgram,
                  sCopyVS, sCopy2DRectFS);
 
+#ifdef ANDROID
+  SHADER_PROGRAM(Copy2DExternalProgramType, CopyProgram,
+                 sCopyVS, sCopy2DExternalFS);
+#endif
+
 #undef SHADER_PROGRAM
 
   NS_ASSERTION(programIndex == NumProgramTypes,

gfx/layers/opengl/LayerManagerOGL.h

@@ -234,6 +234,11 @@ public:
   CopyProgram *GetCopy2DRectProgram() {
     return static_cast<CopyProgram*>(mPrograms[gl::Copy2DRectProgramType]);
   }
+#ifdef ANDROID
+  CopyProgram *GetCopy2DExternalProgram() {
+    return static_cast<CopyProgram*>(mPrograms[gl::Copy2DExternalProgramType]);
+  }
+#endif
 
   ColorTextureLayerProgram *GetFBOLayerProgram() {
     return static_cast<ColorTextureLayerProgram*>(mPrograms[GetFBOLayerProgramType()]);

gfx/layers/opengl/LayerManagerOGLShaders.txt

@@ -269,6 +269,19 @@ void main()
 }
 @end
 
+@shader sCopy2DExternalFS
+$FRAGMENT_SHADER_HEADER$
+#extension GL_OES_EGL_image_external : require
+
+varying vec2 vTexCoord;
+
+uniform samplerExternalOES uTexture;
+void main()
+{
+  gl_FragColor = texture2D(uTexture, vTexCoord);
+}
+@end
+
 @shader sCopy2DRectFS
 #extension GL_ARB_texture_rectangle : enable
 

widget/android/AndroidDirectTexture.cpp

@@ -142,7 +142,7 @@ AndroidDirectTexture::Reallocate(PRUint32 aWidth, PRUint32 aHeight, gfxASurface:
 }
 
 bool
-AndroidDirectTexture::Bind()
+AndroidDirectTexture::Bind(GLenum target)
 {
   MutexAutoLock lock(mLock);
 
@@ -153,7 +153,7 @@ AndroidDirectTexture::Bind()
     mNeedFlip = false;
   }
 
-  return mFrontBuffer->Bind();
+  return mFrontBuffer->Bind(target);
 }
 
 } /* mozilla */

widget/android/AndroidDirectTexture.h

@@ -69,7 +69,7 @@ public:
   PRUint32 Width() { return mWidth; }
   PRUint32 Height() { return mHeight; }
 
-  bool Bind();
+  bool Bind(GLenum target);
 
 private:
   mozilla::Mutex mLock;

widget/android/AndroidGraphicBuffer.cpp

@@ -46,6 +46,7 @@
 
 #define EGL_NATIVE_BUFFER_ANDROID 0x3140
 #define EGL_IMAGE_PRESERVED_KHR   0x30D2
+#define GL_TEXTURE_EXTERNAL_OES   0x8D65
 
 typedef void *EGLContext;
 typedef void *EGLDisplay;
@@ -64,10 +65,23 @@ typedef gfxASurface::gfxImageFormat gfxImageFormat;
 #define ANDROID_LIBUI_PATH "libui.so"
 #define ANDROID_GLES_PATH "libGLESv2.so"
 #define ANDROID_EGL_PATH "libEGL.so"
+#define ANDROID_LIBC_PATH "libc.so"
 
 // Really I have no idea, but this should be big enough
 #define GRAPHIC_BUFFER_SIZE 1024
 
+// This layout is taken from the android source code
+// We use this to get at the incRef/decRef functions
+// to manage AndroidGraphicBuffer.
+struct android_native_base_t
+{
+  int magic;
+  int version;
+  void* reserved[4];
+  void (*incRef)(android_native_base_t* base);
+  void (*decRef)(android_native_base_t* base);
+};
+
 enum {
     /* buffer is never read in software */
     GRALLOC_USAGE_SW_READ_NEVER   = 0x00000000,
@@ -165,6 +179,9 @@ public:
   typedef int (*pfnGraphicBufferReallocate)(void*, PRUint32 w, PRUint32 h, PRUint32 format);
   pfnGraphicBufferReallocate fGraphicBufferReallocate;
 
+  typedef void* (*pfnMalloc)(size_t size);
+  pfnMalloc fMalloc;
+
   bool EnsureInitialized()
   {
     if (mInitialized) {
@@ -202,6 +219,19 @@ public:
       return false;
     }
 
+    handle = dlopen(ANDROID_LIBC_PATH, RTLD_LAZY);
+    if (!handle) {
+      LOG("Couldn't load libc.so");
+      return false;
+    }
+
+    fMalloc = (pfnMalloc)dlsym(handle, "malloc");
+
+    if (!fMalloc) {
+      LOG("Failed to lookup malloc");
+      return false;
+    }
+
     handle = dlopen(ANDROID_LIBUI_PATH, RTLD_LAZY);
     if (!handle) {
       LOG("Couldn't load libui.so");
@@ -270,43 +300,41 @@ AndroidGraphicBuffer::~AndroidGraphicBuffer()
 void
 AndroidGraphicBuffer::DestroyBuffer()
 {
-  /**
-   * XXX: eglDestroyImageKHR crashes sometimes due to refcount badness (I think)
-   *
-   * If you look at egl.cpp (https://github.com/android/platform_frameworks_base/blob/master/opengl/libagl/egl.cpp#L2002)
-   * you can see that eglCreateImageKHR just refs the native buffer, and eglDestroyImageKHR
-   * just unrefs it. Somehow the ref count gets messed up and things are already destroyed
-   * by the time eglDestroyImageKHR gets called. For now, at least, just not calling
-   * eglDestroyImageKHR should be fine since we do free the GraphicBuffer below.
-   *
-   * Bug 712716
-   */
-#if 0
   if (mEGLImage) {
     if (sGLFunctions.EnsureInitialized()) {
-      sGLFunctions.fDestroyImageKHR(sGLFunctions.fGetDisplay(EGL_DEFAULT_DISPLAY), mEGLImage);
+      EGLDisplay display = sGLFunctions.fGetDisplay(EGL_DEFAULT_DISPLAY);
+      sGLFunctions.fDestroyImageKHR(display, mEGLImage);
       mEGLImage = NULL;
     }
   }
-#endif
   mEGLImage = NULL;
 
-  if (mHandle) {
-    if (sGLFunctions.EnsureInitialized()) {
-      sGLFunctions.fGraphicBufferDtor(mHandle);
-    }
-    free(mHandle);
-    mHandle = NULL;
-  }
-
+  // Refcount will destroy the object for us at the correct time, even after
+  // deleting the EGLImage the driver still sometimes holds a reference
+  // at this point.
+  void* nativeBuffer = sGLFunctions.fGraphicBufferGetNativeBuffer(mHandle);
+  android_native_base_t* nativeBufferBase = (android_native_base_t*)nativeBuffer;
+  nativeBufferBase->decRef(nativeBufferBase);
+  mHandle = NULL;
 }
 
 bool
 AndroidGraphicBuffer::EnsureBufferCreated()
 {
   if (!mHandle) {
-    mHandle = malloc(GRAPHIC_BUFFER_SIZE);
+    // Using libc malloc is important here:
+    // libxul is linked with jemalloc, so using malloc here would give us a jemalloc managed block.
+    // However AndroidGraphicBuffer are native refcounted objects that will be released with libc
+    // when the ref count goes to zero. If this isn't allocated with libc, the refcount dlfree
+    // will crash when releasing.
+    mHandle = sGLFunctions.fMalloc(GRAPHIC_BUFFER_SIZE);
     sGLFunctions.fGraphicBufferCtor(mHandle, mWidth, mHeight, GetAndroidFormat(mFormat), GetAndroidUsage(mUsage));
+
+    void* nativeBuffer = sGLFunctions.fGraphicBufferGetNativeBuffer(mHandle);
+
+    android_native_base_t* nativeBufferBase = (android_native_base_t*)nativeBuffer;
+    nativeBufferBase->incRef(nativeBufferBase);
+
   }
 
   return true;
@@ -429,15 +457,23 @@ AndroidGraphicBuffer::EnsureEGLImage()
   if (!EnsureInitialized())
     return false;
 
-  EGLint eglImgAttrs[] = { EGL_IMAGE_PRESERVED_KHR, EGL_TRUE, EGL_NONE, EGL_NONE };
-  void* nativeBuffer = sGLFunctions.fGraphicBufferGetNativeBuffer(mHandle);
+  EGLint eglImgAttrs[] = { EGL_IMAGE_PRESERVED_KHR,
+                           EGL_TRUE, EGL_NONE, EGL_NONE };
 
-  mEGLImage = sGLFunctions.fCreateImageKHR(sGLFunctions.fGetDisplay(EGL_DEFAULT_DISPLAY), EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID, (EGLClientBuffer)nativeBuffer, eglImgAttrs);
+  EGLDisplay display = sGLFunctions.fGetDisplay(EGL_DEFAULT_DISPLAY);
+  if (!display) {
+    return false;
+  }
+  void* nativeBuffer = sGLFunctions.fGraphicBufferGetNativeBuffer(mHandle);
+  mEGLImage = sGLFunctions.fCreateImageKHR(display, EGL_NO_CONTEXT,
+                                           EGL_NATIVE_BUFFER_ANDROID,
+                                           (EGLClientBuffer)nativeBuffer,
+                                           eglImgAttrs);
   return mEGLImage != NULL;
 }
 
 bool
-AndroidGraphicBuffer::Bind()
+AndroidGraphicBuffer::Bind(GLenum target)
 {
   if (!EnsureInitialized())
     return false;
@@ -448,7 +484,7 @@ AndroidGraphicBuffer::Bind()
   }
 
   clearGLError();
-  sGLFunctions.fImageTargetTexture2DOES(GL_TEXTURE_2D, mEGLImage);
+  sGLFunctions.fImageTargetTexture2DOES(target, mEGLImage);
   return ensureNoGLError("glEGLImageTargetTexture2DOES");
 }
 

widget/android/AndroidGraphicBuffer.h

@@ -39,6 +39,7 @@
 #define AndroidGraphicBuffer_h_
 
 #include "gfxASurface.h"
+#include <GLES2/gl2.h>
 #include "nsRect.h"
 
 typedef void* EGLImageKHR;
@@ -77,7 +78,7 @@ public:
   PRUint32 Width() { return mWidth; }
   PRUint32 Height() { return mHeight; }
 
-  bool Bind();
+  bool Bind(GLenum target);
 
   static bool IsBlacklisted();
 

widget/android/Makefile.in

@@ -92,7 +92,7 @@ XPIDLSRCS	= \
 
 SHARED_LIBRARY_LIBS = ../xpwidgets/libxpwidgets_s.a
 
-EXPORTS = AndroidBridge.h AndroidJavaWrappers.h AndroidFlexViewWrapper.h
+EXPORTS = AndroidBridge.h AndroidJavaWrappers.h AndroidFlexViewWrapper.h AndroidDirectTexture.h AndroidGraphicBuffer.h
 
 include $(topsrcdir)/config/rules.mk