Bug 1163841 - Always call eglInitialize(), but kill the preloading hack (which was crashing before). r=nchen, a=sledru
authorJames Willcox <snorp@snorp.net>
Wed, 13 May 2015 10:34:37 -0500
changeset 260532 daa1f205525a
parent 260531 58d8fb9fc5e3
child 260533 06bdddc6463d
push id810
push userryanvm@gmail.com
push date2015-05-19 21:14 +0000
treeherdermozilla-release@06bdddc6463d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnchen, sledru
bugs1163841
milestone38.0.5
Bug 1163841 - Always call eglInitialize(), but kill the preloading hack (which was crashing before). r=nchen, a=sledru
mobile/android/base/gfx/GLController.java
--- a/mobile/android/base/gfx/GLController.java
+++ b/mobile/android/base/gfx/GLController.java
@@ -16,58 +16,16 @@ import android.util.Log;
 
 import javax.microedition.khronos.egl.EGL10;
 import javax.microedition.khronos.egl.EGLConfig;
 import javax.microedition.khronos.egl.EGLContext;
 import javax.microedition.khronos.egl.EGLDisplay;
 import javax.microedition.khronos.egl.EGLSurface;
 
 /**
- * EGLPreloadingThread is purely a preloading optimization, not something
- * we rely on for anything else than performance. We will be initializing
- * EGL in GLController::initEGL() when we need it, but having EGL initialization
- * already previously done by EGLPreloadingThread::run() will make it much
- * faster for GLController to do again.
- *
- * For example, here are some timings recorded on two devices:
- *
- * Device                 | EGLPreloadingThread::run() | GLController::initEGL()
- * -----------------------+----------------------------+------------------------
- * Nexus S (Android 2.3)  | ~ 80 ms                    | < 0.1 ms
- * Nexus 10 (Android 4.3) | ~ 35 ms                    | < 0.1 ms
- */
-class EGLPreloadingThread extends Thread
-{
-    private static final String LOGTAG = "EGLPreloadingThread";
-    private EGL10 mEGL;
-    private EGLDisplay mEGLDisplay;
-
-    public EGLPreloadingThread()
-    {
-    }
-
-    @Override
-    public void run()
-    {
-        mEGL = (EGL10)EGLContext.getEGL();
-        mEGLDisplay = mEGL.eglGetDisplay(EGL10.EGL_DEFAULT_DISPLAY);
-        if (mEGLDisplay == EGL10.EGL_NO_DISPLAY) {
-            Log.w(LOGTAG, "Can't get EGL display!");
-            return;
-        }
-
-        int[] returnedVersion = new int[2];
-        if (!mEGL.eglInitialize(mEGLDisplay, returnedVersion)) {
-            Log.w(LOGTAG, "eglInitialize failed");
-            return;
-        }
-    }
-}
-
-/**
  * This class is a singleton that tracks EGL and compositor things over
  * the lifetime of Fennec running.
  * We only ever create one C++ compositor over Fennec's lifetime, but
  * most of the Java-side objects (e.g. LayerView, GeckoLayerClient,
  * LayerRenderer) can all get destroyed and re-created if the GeckoApp
  * activity is destroyed. This GLController is never destroyed, so that
  * the mCompositorCreated field and other state variables are always
  * accurate.
@@ -84,17 +42,16 @@ public class GLController {
 
     /* This is written by the compositor thread (while the UI thread
      * is blocked on it) and read by the UI thread. */
     private volatile boolean mCompositorCreated;
 
     private EGL10 mEGL;
     private EGLDisplay mEGLDisplay;
     private EGLConfig mEGLConfig;
-    private final EGLPreloadingThread mEGLPreloadingThread;
     private EGLSurface mEGLSurfaceForCompositor;
 
     private static final int LOCAL_EGL_OPENGL_ES2_BIT = 4;
 
     private static final int[] CONFIG_SPEC_16BPP = {
         EGL10.EGL_RED_SIZE, 5,
         EGL10.EGL_GREEN_SIZE, 6,
         EGL10.EGL_BLUE_SIZE, 5,
@@ -108,22 +65,16 @@ public class GLController {
         EGL10.EGL_GREEN_SIZE, 8,
         EGL10.EGL_BLUE_SIZE, 8,
         EGL10.EGL_SURFACE_TYPE, EGL10.EGL_WINDOW_BIT,
         EGL10.EGL_RENDERABLE_TYPE, LOCAL_EGL_OPENGL_ES2_BIT,
         EGL10.EGL_NONE
     };
 
     private GLController() {
-        if (AppConstants.Versions.preICS) {
-            mEGLPreloadingThread = new EGLPreloadingThread();
-            mEGLPreloadingThread.start();
-        } else {
-            mEGLPreloadingThread = null;
-        }
     }
 
     static GLController getInstance(LayerView view) {
         if (sInstance == null) {
             sInstance = new GLController();
         }
         sInstance.mView = view;
         return sInstance;
@@ -206,51 +157,36 @@ public class GLController {
         return mServerSurfaceValid;
     }
 
     private void initEGL() {
         if (mEGL != null) {
             return;
         }
 
-        // This join() should not be necessary, but makes this code a bit easier to think about.
-        // The EGLPreloadingThread should long be done by now, and even if it's not,
-        // it shouldn't be a problem to be initializing EGL from two different threads.
-        // Still, having this join() here means that we don't have to wonder about what
-        // kind of caveats might exist with EGL initialization reentrancy on various drivers.
-        if (mEGLPreloadingThread != null) {
-            try {
-                mEGLPreloadingThread.join();
-            } catch (InterruptedException e) {
-                Log.w(LOGTAG, "EGLPreloadingThread interrupted", e);
-            }
-        }
-
         mEGL = (EGL10)EGLContext.getEGL();
 
         mEGLDisplay = mEGL.eglGetDisplay(EGL10.EGL_DEFAULT_DISPLAY);
         if (mEGLDisplay == EGL10.EGL_NO_DISPLAY) {
             Log.w(LOGTAG, "Can't get EGL display!");
             return;
         }
 
-        if (AppConstants.Versions.preICS) {
-            // while calling eglInitialize here should not be necessary as it was already called
-            // by the EGLPreloadingThread, it really doesn't cost much to call it again here,
-            // and makes this code easier to think about: EGLPreloadingThread is only a
-            // preloading optimization, not something we rely on for anything else.
-            //
-            // Also note that while calling eglInitialize isn't necessary on Android 4.x
-            // (at least Android's HardwareRenderer does it for us already), it is necessary
-            // on Android 2.x.
-            int[] returnedVersion = new int[2];
-            if (!mEGL.eglInitialize(mEGLDisplay, returnedVersion)) {
-                Log.w(LOGTAG, "eglInitialize failed");
-                return;
-            }
+        // while calling eglInitialize here should not be necessary as it was already called
+        // by the EGLPreloadingThread, it really doesn't cost much to call it again here,
+        // and makes this code easier to think about: EGLPreloadingThread is only a
+        // preloading optimization, not something we rely on for anything else.
+        //
+        // Also note that while calling eglInitialize isn't necessary on Android 4.x
+        // (at least Android's HardwareRenderer does it for us already), it is necessary
+        // on Android 2.x.
+        int[] returnedVersion = new int[2];
+        if (!mEGL.eglInitialize(mEGLDisplay, returnedVersion)) {
+            Log.w(LOGTAG, "eglInitialize failed");
+            return;
         }
 
         mEGLConfig = chooseConfig();
     }
 
     private EGLConfig chooseConfig() {
         int[] desiredConfig;
         int rSize, gSize, bSize;