More GLSurfaceView cleanup.

+ The mDone flag is now a pair of flags: mShouldExit and mExited. The
problem with mDone was that it meant "had been asked to exit", but was
being used by some observers as "had exited". Using two variables means
that observers can observe either "had been asked to exit" or "had exited",
as they prefer.

+ Simplyify where we check for mShouldExit. We now check for it at the
  top of our innermost guardedRun while loop.

+ requestExitAndWait now waits for mExited to be set to true to know
that a thread has exited, rather than using join(). This means we can use
wait() for the check, which releases the sGLThreadManager
monitor, avoiding a potential deadlock.

+ move the event queue into the sGLThreadManager monitor. This avoids
having to acquire two locks in order to enque/deque events, which also
avoids the potential for lock ordering deadlocks.

+ Simplify the event dequeueing code. We now deque one event each time
through the main GLSurfaceView loop. Events still have priority over
rendering, so there isn't any semantic change, it just cleans up the code.

+ Avoid trying to acquire an egl Surface if we're paused.

+ To simplify reasoning about the code, call sGLThreadManager.notifyAll()
in every case where we modify one of the variables that's protected by
the sGLThreadManager monitor. It would be slightly more efficient to only
notify when we change variables that could cause a thread to wait(), but
then we would have to redo our analysis every time we change any code.

+ Clean up the logic for creating the EGL surface and then calling the
renderer's onSurfaceCreated / onSurfaceChanged methods.

+ Implement work-around for bug 2263168	"Need to draw twice after
  screen rotation..."

1 files changed, 125 insertions, 134 deletions

diff --git a/opengl/java/android/opengl/GLSurfaceView.java b/opengl/java/android/opengl/GLSurfaceView.java
index 9ca57ba..e60859b 100644
--- a/opengl/java/android/opengl/GLSurfaceView.java
+++ b/opengl/java/android/opengl/GLSurfaceView.java
@@ -145,6 +145,10 @@ import android.view.SurfaceView;
  */
 public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback {
     private final static boolean LOG_THREADS = false;
+    private final static boolean LOG_SURFACE = false;
+    private final static boolean LOG_RENDERER = false;
+    // Work-around for bug 2263168
+    private final static boolean DRAW_TWICE_AFTER_SIZE_CHANGED = true;
     /**
      * The renderer only renders
      * when the surface is created, or when {@link #requestRender} is called.
@@ -879,7 +883,7 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
                 gl = mGLWrapper.wrap(gl);
             }
 
-            if ((mDebugFlags & (DEBUG_CHECK_GL_ERROR | DEBUG_LOG_GL_CALLS))!= 0) {
+            if ((mDebugFlags & (DEBUG_CHECK_GL_ERROR | DEBUG_LOG_GL_CALLS)) != 0) {
                 int configFlags = 0;
                 Writer log = null;
                 if ((mDebugFlags & DEBUG_CHECK_GL_ERROR) != 0) {
@@ -949,7 +953,6 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
     class GLThread extends Thread {
         GLThread(Renderer renderer) {
             super();
-            mDone = false;
             mWidth = 0;
             mHeight = 0;
             mRequestRender = true;
@@ -982,7 +985,7 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
                 mHaveEgl = false;
                 mEglHelper.destroySurface();
                 mEglHelper.finish();
-                sGLThreadManager.releaseEglSurface(this);
+                sGLThreadManager.releaseEglSurfaceLocked(this);
             }
         }
 
@@ -990,81 +993,89 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
             mEglHelper = new EglHelper();
             try {
                 GL10 gl = null;
-                boolean tellRendererSurfaceCreated = true;
-                boolean tellRendererSurfaceChanged = true;
-
-                /*
-                 * This is our main activity thread's loop, we go until
-                 * asked to quit.
-                 */
-                while (!isDone()) {
-                    /*
-                     *  Update the asynchronous state (window size)
-                     */
-                    int w = 0;
-                    int h = 0;
-                    boolean changed = false;
-                    boolean needStart = false;
-                    boolean eventsWaiting = false;
+                boolean createEglSurface = false;
+                boolean sizeChanged = false;
+                int w = 0;
+                int h = 0;
+                Runnable event = null;
 
+                while (true) {
                     synchronized (sGLThreadManager) {
                         while (true) {
-                            // Manage acquiring and releasing the SurfaceView
-                            // surface and the EGL surface.
-                            if (mPaused) {
+                            if (mShouldExit) {
+                                return;
+                            }
+
+                            if (! mEventQueue.isEmpty()) {
+                                event = mEventQueue.remove(0);
+                                break;
+                            }
+
+                            // Do we need to release the EGL surface?
+                            if (mHaveEgl && mPaused) {
+                                if (LOG_SURFACE) {
+                                    Log.i("GLThread", "releasing EGL surface because paused tid=" + getId());
+                                }
                                 stopEglLocked();
                             }
-                            if (!mHasSurface) {
-                                if (!mWaitingForSurface) {
-                                    stopEglLocked();
-                                    mWaitingForSurface = true;
-                                    sGLThreadManager.notifyAll();
+
+                            // Have we lost the surface view surface?
+                            if ((! mHasSurface) && (! mWaitingForSurface)) {
+                                if (LOG_SURFACE) {
+                                    Log.i("GLThread", "noticed surfaceView surface lost tid=" + getId());
                                 }
-                            } else {
-                                if (!mHaveEgl) {
-                                    if (sGLThreadManager.tryAcquireEglSurface(this)) {
-                                        mHaveEgl = true;
-                                        mEglHelper.start();
-                                        mRequestRender = true;
-                                        needStart = true;
-                                    }
+                                if (mHaveEgl) {
+                                    stopEglLocked();
                                 }
+                                mWaitingForSurface = true;
+                                sGLThreadManager.notifyAll();
                             }
 
-                            // Check if we need to wait. If not, update any state
-                            // that needs to be updated, copy any state that
-                            // needs to be copied, and use "break" to exit the
-                            // wait loop.
-
-                            if (mDone) {
-                                return;
+                            // Have we acquired the surface view surface?
+                            if (mHasSurface && mWaitingForSurface) {
+                                if (LOG_SURFACE) {
+                                    Log.i("GLThread", "noticed surfaceView surface acquired tid=" + getId());
+                                }
+                                mWaitingForSurface = false;
+                                sGLThreadManager.notifyAll();
                             }
 
-                            if (mEventsWaiting) {
-                                eventsWaiting = true;
-                                mEventsWaiting = false;
-                                break;
-                            }
+                            // Ready to draw?
+                            if ((!mPaused) && mHasSurface
+                                && (mWidth > 0) && (mHeight > 0)
+                                && (mRequestRender || (mRenderMode == RENDERMODE_CONTINUOUSLY))) {
+
+                                // If we don't have an egl surface, try to acquire one.
+                                if ((! mHaveEgl) && sGLThreadManager.tryAcquireEglSurfaceLocked(this)) {
+                                    mHaveEgl = true;
+                                    mEglHelper.start();
+                                    createEglSurface = true;
+                                    sizeChanged = true;
+                                    sGLThreadManager.notifyAll();
+                                }
 
-                            if ( (! mPaused) && mHasSurface && mHaveEgl
-                                    && (mWidth > 0) && (mHeight > 0)
-                                    && (mRequestRender || (mRenderMode == RENDERMODE_CONTINUOUSLY))
-                                    ) {
-                                changed = mSizeChanged;
-                                w = mWidth;
-                                h = mHeight;
-                                mSizeChanged = false;
-                                mRequestRender = false;
-                                if (mHasSurface && mWaitingForSurface) {
-                                    changed = true;
-                                    mWaitingForSurface = false;
+                                if (mHaveEgl) {
+                                    if (mSizeChanged) {
+                                        sizeChanged = true;
+                                        w = mWidth;
+                                        h = mHeight;
+                                        if (DRAW_TWICE_AFTER_SIZE_CHANGED) {
+                                            // We keep mRequestRender true so that we draw twice after the size changes.
+                                            // (Once because of mSizeChanged, the second time because of mRequestRender.)
+                                            // This forces the updated graphics onto the screen.
+                                        } else {
+                                            mRequestRender = false;
+                                        }
+                                        mSizeChanged = false;
+                                    } else {
+                                        mRequestRender = false;
+                                    }
                                     sGLThreadManager.notifyAll();
+                                    break;
                                 }
-                                break;
                             }
 
-                            // By design, this is the only place where we wait().
-
+                            // By design, this is the only place in a GLThread thread where we wait().
                             if (LOG_THREADS) {
                                 Log.i("GLThread", "waiting tid=" + getId());
                             }
@@ -1072,48 +1083,39 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
                         }
                     } // end of synchronized(sGLThreadManager)
 
-                    /*
-                     * Handle queued events
-                     */
-                    if (eventsWaiting) {
-                        Runnable r;
-                        while ((r = getEvent()) != null) {
-                            r.run();
-                            if (isDone()) {
-                                return;
-                            }
-                        }
-                        // Go back and see if we need to wait to render.
+                    if (event != null) {
+                        event.run();
+                        event = null;
                         continue;
                     }
 
-                    if (needStart) {
-                        tellRendererSurfaceCreated = true;
-                        changed = true;
-                    }
-                    if (changed) {
+                    if (createEglSurface) {
                         gl = (GL10) mEglHelper.createSurface(getHolder());
-                        tellRendererSurfaceChanged = true;
-                    }
-                    if (tellRendererSurfaceCreated) {
+                        if (LOG_RENDERER) {
+                            Log.w("GLThread", "onSurfaceCreated");
+                        }
                         mRenderer.onSurfaceCreated(gl, mEglHelper.mEglConfig);
-                        tellRendererSurfaceCreated = false;
+                        createEglSurface = false;
                     }
-                    if (tellRendererSurfaceChanged) {
+
+                    if (sizeChanged) {
+                        if (LOG_RENDERER) {
+                            Log.w("GLThread", "onSurfaceChanged(" + w + ", " + h + ")");
+                        }
                         mRenderer.onSurfaceChanged(gl, w, h);
-                        tellRendererSurfaceChanged = false;
+                        sizeChanged = false;
                     }
-                    if ((w > 0) && (h > 0)) {
-                        /* draw a frame here */
-                        mRenderer.onDrawFrame(gl);
-
-                        /*
-                         * Once we're done with GL, we need to call swapBuffers()
-                         * to instruct the system to display the rendered frame
-                         */
-                        mEglHelper.swap();
+
+                    if (LOG_RENDERER) {
+                        Log.w("GLThread", "onDrawFrame");
+                    }
+                    mRenderer.onDrawFrame(gl);
+                    if(!mEglHelper.swap()) {
+                        if (LOG_SURFACE) {
+                            Log.i("GLThread", "egl surface lost tid=" + getId());
+                        }
                     }
-                 }
+                }
             } finally {
                 /*
                  * clean-up everything...
@@ -1124,21 +1126,13 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
             }
         }
 
-        private boolean isDone() {
-            synchronized (sGLThreadManager) {
-                return mDone;
-            }
-        }
-
         public void setRenderMode(int renderMode) {
             if ( !((RENDERMODE_WHEN_DIRTY <= renderMode) && (renderMode <= RENDERMODE_CONTINUOUSLY)) ) {
                 throw new IllegalArgumentException("renderMode");
             }
             synchronized(sGLThreadManager) {
                 mRenderMode = renderMode;
-                if (renderMode == RENDERMODE_CONTINUOUSLY) {
-                    sGLThreadManager.notifyAll();
-                }
+                sGLThreadManager.notifyAll();
             }
         }
 
@@ -1172,7 +1166,7 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
                 }
                 mHasSurface = false;
                 sGLThreadManager.notifyAll();
-                while(!mWaitingForSurface && isAlive() && ! mDone) {
+                while((!mWaitingForSurface) && (!mExited)) {
                     try {
                         sGLThreadManager.wait();
                     } catch (InterruptedException e) {
@@ -1202,6 +1196,7 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
                 mWidth = w;
                 mHeight = h;
                 mSizeChanged = true;
+                mRequestRender = true;
                 sGLThreadManager.notifyAll();
             }
         }
@@ -1210,13 +1205,15 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
             // don't call this from GLThread thread or it is a guaranteed
             // deadlock!
             synchronized(sGLThreadManager) {
-                mDone = true;
+                mShouldExit = true;
                 sGLThreadManager.notifyAll();
-            }
-            try {
-                join();
-            } catch (InterruptedException ex) {
-                Thread.currentThread().interrupt();
+                while (! mExited) {
+                    try {
+                        sGLThreadManager.wait();
+                    } catch (InterruptedException ex) {
+                        Thread.currentThread().interrupt();
+                    }
+                }
             }
         }
 
@@ -1225,28 +1222,19 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
          * @param r the runnable to be run on the GL rendering thread.
          */
         public void queueEvent(Runnable r) {
-            synchronized(this) {
-                mEventQueue.add(r);
-                synchronized(sGLThreadManager) {
-                    mEventsWaiting = true;
-                    sGLThreadManager.notifyAll();
-                }
+            if (r == null) {
+                throw new IllegalArgumentException("r must not be null");
             }
-        }
-
-        private Runnable getEvent() {
-            synchronized(this) {
-                if (mEventQueue.size() > 0) {
-                    return mEventQueue.remove(0);
-                }
-
+            synchronized(sGLThreadManager) {
+                mEventQueue.add(r);
+                sGLThreadManager.notifyAll();
             }
-            return null;
         }
 
         // Once the thread is started, all accesses to the following member
         // variables are protected by the sGLThreadManager monitor
-        private boolean mDone;
+        private boolean mShouldExit;
+        private boolean mExited;
         private boolean mPaused;
         private boolean mHasSurface;
         private boolean mWaitingForSurface;
@@ -1255,11 +1243,10 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
         private int mHeight;
         private int mRenderMode;
         private boolean mRequestRender;
-        private boolean mEventsWaiting;
+        private ArrayList<Runnable> mEventQueue = new ArrayList<Runnable>();
         // End of member variables protected by the sGLThreadManager monitor.
 
         private Renderer mRenderer;
-        private ArrayList<Runnable> mEventQueue = new ArrayList<Runnable>();
         private EglHelper mEglHelper;
     }
 
@@ -1309,7 +1296,7 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
             if (LOG_THREADS) {
                 Log.i("GLThread", "exiting tid=" +  thread.getId());
             }
-            thread.mDone = true;
+            thread.mExited = true;
             if (mEglOwner == thread) {
                 mEglOwner = null;
             }
@@ -1318,10 +1305,11 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
 
         /*
          * Tries once to acquire the right to use an EGL
-         * surface. Does not block.
+         * surface. Does not block. Requires that we are already
+         * in the sGLThreadManager monitor when this is called.
          * @return true if the right to use an EGL surface was acquired.
          */
-        public synchronized boolean tryAcquireEglSurface(GLThread thread) {
+        public boolean tryAcquireEglSurfaceLocked(GLThread thread) {
             if (mEglOwner == thread || mEglOwner == null) {
                 mEglOwner = thread;
                 notifyAll();
@@ -1329,8 +1317,11 @@ public class GLSurfaceView extends SurfaceView implements SurfaceHolder.Callback
             }
             return false;
         }
-
-        public synchronized void releaseEglSurface(GLThread thread) {
+        /*
+         * Releases the EGL surface. Requires that we are already in the
+         * sGLThreadManager monitor when this is called.
+         */
+        public void releaseEglSurfaceLocked(GLThread thread) {
             if (mEglOwner == thread) {
                 mEglOwner = null;
             }