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..."
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 @@
*/
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 @@
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 @@
class GLThread extends Thread {
GLThread(Renderer renderer) {
super();
- mDone = false;
mWidth = 0;
mHeight = 0;
mRequestRender = true;
@@ -982,7 +985,7 @@
mHaveEgl = false;
mEglHelper.destroySurface();
mEglHelper.finish();
- sGLThreadManager.releaseEglSurface(this);
+ sGLThreadManager.releaseEglSurfaceLocked(this);
}
}
@@ -990,81 +993,89 @@
mEglHelper = new EglHelper();
try {
GL10 gl = null;
- boolean tellRendererSurfaceCreated = true;
- boolean tellRendererSurfaceChanged = true;
+ boolean createEglSurface = false;
+ boolean sizeChanged = false;
+ int w = 0;
+ int h = 0;
+ Runnable event = null;
- /*
- * 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;
-
+ while (true) {
synchronized (sGLThreadManager) {
while (true) {
- // Manage acquiring and releasing the SurfaceView
- // surface and the EGL surface.
- if (mPaused) {
- stopEglLocked();
- }
- if (!mHasSurface) {
- if (!mWaitingForSurface) {
- stopEglLocked();
- mWaitingForSurface = true;
- sGLThreadManager.notifyAll();
- }
- } else {
- if (!mHaveEgl) {
- if (sGLThreadManager.tryAcquireEglSurface(this)) {
- mHaveEgl = true;
- mEglHelper.start();
- mRequestRender = true;
- needStart = true;
- }
- }
- }
-
- // 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) {
+ if (mShouldExit) {
return;
}
- if (mEventsWaiting) {
- eventsWaiting = true;
- mEventsWaiting = false;
+ if (! mEventQueue.isEmpty()) {
+ event = mEventQueue.remove(0);
break;
}
- 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;
+ // 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();
+ }
+
+ // Have we lost the surface view surface?
+ if ((! mHasSurface) && (! mWaitingForSurface)) {
+ if (LOG_SURFACE) {
+ Log.i("GLThread", "noticed surfaceView surface lost tid=" + getId());
+ }
+ if (mHaveEgl) {
+ stopEglLocked();
+ }
+ mWaitingForSurface = true;
+ sGLThreadManager.notifyAll();
+ }
+
+ // 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();
+ }
+
+ // 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();
}
- break;
+
+ 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;
+ }
}
- // 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 @@
}
} // 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) {
- mRenderer.onSurfaceChanged(gl, w, h);
- tellRendererSurfaceChanged = 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 (sizeChanged) {
+ if (LOG_RENDERER) {
+ Log.w("GLThread", "onSurfaceChanged(" + w + ", " + h + ")");
+ }
+ mRenderer.onSurfaceChanged(gl, w, h);
+ sizeChanged = false;
}
- }
+
+ 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 @@
}
}
- 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 @@
}
mHasSurface = false;
sGLThreadManager.notifyAll();
- while(!mWaitingForSurface && isAlive() && ! mDone) {
+ while((!mWaitingForSurface) && (!mExited)) {
try {
sGLThreadManager.wait();
} catch (InterruptedException e) {
@@ -1202,6 +1196,7 @@
mWidth = w;
mHeight = h;
mSizeChanged = true;
+ mRequestRender = true;
sGLThreadManager.notifyAll();
}
}
@@ -1210,13 +1205,15 @@
// 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 @@
* @param r the runnable to be run on the GL rendering thread.
*/
public void queueEvent(Runnable r) {
- synchronized(this) {
+ if (r == null) {
+ throw new IllegalArgumentException("r must not be null");
+ }
+ synchronized(sGLThreadManager) {
mEventQueue.add(r);
- synchronized(sGLThreadManager) {
- mEventsWaiting = true;
- sGLThreadManager.notifyAll();
- }
+ sGLThreadManager.notifyAll();
}
}
- private Runnable getEvent() {
- synchronized(this) {
- if (mEventQueue.size() > 0) {
- return mEventQueue.remove(0);
- }
-
- }
- 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 @@
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 @@
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 @@
/*
* 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 @@
}
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;
}