Bug 712716 - Fix AndroidGraphicBuffer resource management. r=snorp,jmuizelaar
☠☠ backed out by c405ba24c018 ☠ ☠
authorBenoit Girard <b56girard@gmail.com>
Mon, 12 Mar 2012 16:28:02 -0400
changeset 89340 b52bae0250f719571ae6cadff9f941668ac1fd83
parent 89339 2dd6ac8eb1a47f3bcbed4bcf024d68b3a825ae95
child 89341 306e3275b7744f182d98240ed65226b462ba1eb0
push id22242
push userkgupta@mozilla.com
push dateWed, 14 Mar 2012 15:19:09 +0000
treeherdermozilla-central@936ef50fa498 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp, jmuizelaar
bugs712716
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 712716 - Fix AndroidGraphicBuffer resource management. r=snorp,jmuizelaar
gfx/gl/GLContext.h
gfx/layers/opengl/LayerManagerOGL.cpp
gfx/layers/opengl/LayerManagerOGL.h
gfx/layers/opengl/LayerManagerOGLShaders.txt
widget/android/AndroidDirectTexture.cpp
widget/android/AndroidDirectTexture.h
widget/android/AndroidGraphicBuffer.cpp
widget/android/AndroidGraphicBuffer.h
widget/android/Makefile.in
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -131,16 +131,17 @@ enum ShaderProgramType {
     BGRXLayerProgramType,
     RGBARectLayerProgramType,
     ColorLayerProgramType,
     YCbCrLayerProgramType,
     ComponentAlphaPass1ProgramType,
     ComponentAlphaPass2ProgramType,
     Copy2DProgramType,
     Copy2DRectProgramType,
+    Copy2DExternalProgramType,
     NumProgramTypes
 };
 
 
 /**
  * A TextureImage encapsulates a surface that can be drawn to by a
  * Thebes gfxContext and (hopefully efficiently!) synchronized to a
  * texture in the server.  TextureImages are associated with one and
--- a/gfx/layers/opengl/LayerManagerOGL.cpp
+++ b/gfx/layers/opengl/LayerManagerOGL.cpp
@@ -241,16 +241,21 @@ LayerManagerOGL::Initialize(nsRefPtr<GLC
   SHADER_PROGRAM(ComponentAlphaPass2ProgramType, ComponentAlphaTextureLayerProgram,
                  sLayerVS, sComponentPass2FS);
   /* Copy programs (used for final framebuffer blit) */
   SHADER_PROGRAM(Copy2DProgramType, CopyProgram,
                  sCopyVS, sCopy2DFS);
   SHADER_PROGRAM(Copy2DRectProgramType, CopyProgram,
                  sCopyVS, sCopy2DRectFS);
 
+#ifdef ANDROID
+  SHADER_PROGRAM(Copy2DExternalProgramType, CopyProgram,
+                 sCopyVS, sCopy2DExternalFS);
+#endif
+
 #undef SHADER_PROGRAM
 
   NS_ASSERTION(programIndex == NumProgramTypes,
                "not all programs were initialized!");
 
   /**
    * We'll test the ability here to bind NPOT textures to a framebuffer, if
    * this fails we'll try ARB_texture_rectangle.
--- a/gfx/layers/opengl/LayerManagerOGL.h
+++ b/gfx/layers/opengl/LayerManagerOGL.h
@@ -229,16 +229,21 @@ public:
              (mPrograms[gl::ComponentAlphaPass2ProgramType]);
   }
   CopyProgram *GetCopy2DProgram() {
     return static_cast<CopyProgram*>(mPrograms[gl::Copy2DProgramType]);
   }
   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()]);
   }
 
   gl::ShaderProgramType GetFBOLayerProgramType() {
     if (mFBOTextureTarget == LOCAL_GL_TEXTURE_RECTANGLE_ARB)
       return gl::RGBARectLayerProgramType;
--- a/gfx/layers/opengl/LayerManagerOGLShaders.txt
+++ b/gfx/layers/opengl/LayerManagerOGLShaders.txt
@@ -264,16 +264,29 @@ varying vec2 vTexCoord;
 
 uniform sampler2D uTexture;
 void main()
 {
   gl_FragColor = texture2D(uTexture, vTexCoord);
 }
 @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
 
 $FRAGMENT_SHADER_HEADER$
 
 varying vec2 vTexCoord;
 uniform vec2 uTexCoordMultiplier;
 
--- a/widget/android/AndroidDirectTexture.cpp
+++ b/widget/android/AndroidDirectTexture.cpp
@@ -137,23 +137,23 @@ AndroidDirectTexture::Reallocate(PRUint3
     mWidth = aWidth;
     mHeight = aHeight;
   }
 
   return result;
 }
 
 bool
-AndroidDirectTexture::Bind()
+AndroidDirectTexture::Bind(GLenum target)
 {
   MutexAutoLock lock(mLock);
 
   if (mNeedFlip) {
     AndroidGraphicBuffer* tmp = mBackBuffer;
     mBackBuffer = mFrontBuffer;
     mFrontBuffer = tmp;
     mNeedFlip = false;
   }
 
-  return mFrontBuffer->Bind();
+  return mFrontBuffer->Bind(target);
 }
 
 } /* mozilla */
--- a/widget/android/AndroidDirectTexture.h
+++ b/widget/android/AndroidDirectTexture.h
@@ -64,17 +64,17 @@ public:
   bool Unlock(bool aFlip = true);
 
   bool Reallocate(PRUint32 aWidth, PRUint32 aHeight);
   bool Reallocate(PRUint32 aWidth, PRUint32 aHeight, gfxASurface::gfxImageFormat aFormat);
 
   PRUint32 Width() { return mWidth; }
   PRUint32 Height() { return mHeight; }
 
-  bool Bind();
+  bool Bind(GLenum target);
 
 private:
   mozilla::Mutex mLock;
   bool mNeedFlip;
 
   PRUint32 mWidth;
   PRUint32 mHeight;
   gfxASurface::gfxImageFormat mFormat;
--- a/widget/android/AndroidGraphicBuffer.cpp
+++ b/widget/android/AndroidGraphicBuffer.cpp
@@ -41,36 +41,50 @@
 #include "AndroidGraphicBuffer.h"
 #include "AndroidBridge.h"
 #include "mozilla/Preferences.h"
 
 #define LOG(args...)  __android_log_print(ANDROID_LOG_INFO, "AndroidGraphicBuffer" , ## args)
 
 #define EGL_NATIVE_BUFFER_ANDROID 0x3140
 #define EGL_IMAGE_PRESERVED_KHR   0x30D2
+#define GL_TEXTURE_EXTERNAL_OES   0x8D65
 
 typedef PRUint32 EGLenum;
 typedef PRInt32 EGLint;
 typedef PRUint32 EGLBoolean;
 
 typedef gfxASurface::gfxImageFormat gfxImageFormat;
 
 #define EGL_TRUE 1
 #define EGL_FALSE 0
 #define EGL_NONE 0x3038
 #define EGL_NO_CONTEXT (EGLContext)0
 #define EGL_DEFAULT_DISPLAY  (void*)0
 
 #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,
     /* buffer is rarely read in software */
     GRALLOC_USAGE_SW_READ_RARELY  = 0x00000002,
     /* buffer is often read in software */
     GRALLOC_USAGE_SW_READ_OFTEN   = 0x00000003,
     /* mask for the software read values */
@@ -158,16 +172,19 @@ public:
   pfnGraphicBufferUnlock fGraphicBufferUnlock;
 
   typedef void* (*pfnGraphicBufferGetNativeBuffer)(void*);
   pfnGraphicBufferGetNativeBuffer fGraphicBufferGetNativeBuffer;
 
   typedef int (*pfnGraphicBufferReallocate)(void*, PRUint32 w, PRUint32 h, PRUint32 format);
   pfnGraphicBufferReallocate fGraphicBufferReallocate;
 
+  typedef void* (*pfnMalloc)(size_t size);
+  pfnMalloc fMalloc;
+
   bool EnsureInitialized()
   {
     if (mInitialized) {
       return true;
     }
 
     void *handle = dlopen(ANDROID_EGL_PATH, RTLD_LAZY);
     if (!handle) {
@@ -195,16 +212,29 @@ public:
     fBindTexture = (pfnBindTexture)dlsym(handle, "glBindTexture");
     fGLGetError = (pfnGLGetError)dlsym(handle, "glGetError");
 
     if (!fImageTargetTexture2DOES || !fBindTexture || !fGLGetError) {
       LOG("Failed to find some GL functions");
       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");
       return false;
     }
 
     fGraphicBufferCtor = (pfnGraphicBufferCtor)dlsym(handle, "_ZN7android13GraphicBufferC1Ejjij");
     fGraphicBufferDtor = (pfnGraphicBufferDtor)dlsym(handle, "_ZN7android13GraphicBufferD1Ev");
@@ -263,53 +293,51 @@ AndroidGraphicBuffer::AndroidGraphicBuff
 AndroidGraphicBuffer::~AndroidGraphicBuffer()
 {
   DestroyBuffer();
 }
 
 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;
 }
 
 bool
 AndroidGraphicBuffer::EnsureInitialized()
 {
@@ -422,36 +450,44 @@ AndroidGraphicBuffer::EnsureEGLImage()
 {
   if (mEGLImage)
     return true;
 
 
   if (!EnsureInitialized())
     return false;
 
-  EGLint eglImgAttrs[] = { EGL_IMAGE_PRESERVED_KHR, EGL_TRUE, EGL_NONE, EGL_NONE };
+  EGLint eglImgAttrs[] = { EGL_IMAGE_PRESERVED_KHR,
+                           EGL_TRUE, EGL_NONE, EGL_NONE };
+
+  EGLDisplay display = sGLFunctions.fGetDisplay(EGL_DEFAULT_DISPLAY);
+  if (!display) {
+    return false;
+  }
   void* nativeBuffer = sGLFunctions.fGraphicBufferGetNativeBuffer(mHandle);
-
-  mEGLImage = sGLFunctions.fCreateImageKHR(sGLFunctions.fGetDisplay(EGL_DEFAULT_DISPLAY), EGL_NO_CONTEXT, EGL_NATIVE_BUFFER_ANDROID, (EGLClientBuffer)nativeBuffer, eglImgAttrs);
+  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;
 
   if (!EnsureEGLImage()) {
     LOG("No valid EGLImage!");
     return false;
   }
 
   clearGLError();
-  sGLFunctions.fImageTargetTexture2DOES(GL_TEXTURE_2D, mEGLImage);
+  sGLFunctions.fImageTargetTexture2DOES(target, mEGLImage);
   return ensureNoGLError("glEGLImageTargetTexture2DOES");
 }
 
 static const char* const sAllowedBoards[] = {
   "venus2",     // Motorola Droid Pro
   "tuna",       // Galaxy Nexus
   "omap4sdp",   // Amazon Kindle Fire
   "droid2",     // Motorola Droid 2
--- a/widget/android/AndroidGraphicBuffer.h
+++ b/widget/android/AndroidGraphicBuffer.h
@@ -34,16 +34,17 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef AndroidGraphicBuffer_h_
 #define AndroidGraphicBuffer_h_
 
 #include "gfxASurface.h"
+#include <GLES2/gl2.h>
 #include "nsRect.h"
 
 typedef void* EGLImageKHR;
 typedef void* EGLClientBuffer;
 
 namespace mozilla {
 
 /**
@@ -72,17 +73,17 @@ public:
   int Lock(PRUint32 usage, unsigned char **bits);
   int Lock(PRUint32 usage, const nsIntRect& rect, unsigned char **bits);
   int Unlock();
   bool Reallocate(PRUint32 aWidth, PRUint32 aHeight, gfxASurface::gfxImageFormat aFormat);
 
   PRUint32 Width() { return mWidth; }
   PRUint32 Height() { return mHeight; }
 
-  bool Bind();
+  bool Bind(GLenum target);
 
   static bool IsBlacklisted();
 
 private:
   PRUint32 mWidth;
   PRUint32 mHeight;
   PRUint32 mUsage;
   gfxASurface::gfxImageFormat mFormat;
--- a/widget/android/Makefile.in
+++ b/widget/android/Makefile.in
@@ -87,17 +87,17 @@ NOT_THERE_YET_CPPSRCS = \
 	$(NULL)
 
 XPIDLSRCS	= \
 	nsIAndroidBridge.idl \
 	$(NULL)
 
 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
 
 DEFINES += -D_IMPL_NS_WIDGET
 #DEFINES += -DDEBUG_WIDGETS
 
 LOCAL_INCLUDES += \
 	-I$(topsrcdir)/widget/xpwidgets \