Changed locking protocol for on-screen surfaces to only use the JAWT's

DrawingSurface locking primitives during the makeCurrent operation.
While in theory the JAWT should be used for locking for the entire
duration of the makeCurrent/release pair, in practice because OpenGL
is orthogonal to the window system this is not really necessary, at
least if higher-level locking primitives are used to make sure the
window is not torn down out from under the library while OpenGL
rendering is being performed. The OpenGL-related work done in
GLCanvas.addNotify() and removeNotify() handles this. These changes
enable simultaneous multi-head rendering on X11 systems with one
OpenGL context per thread. Changed GLContext.destroy() to acquire the
context's lock during the destroyImpl operation for correctness in the
new protocol. Changed several Mac OS X native code entry points to not
take an unnecessary NSView* argument. Tested with several demos on all
platforms to stress creation and destruction of multiple kinds of
OpenGL contexts.


git-svn-id: file:///usr/local/projects/SUN/JOGL/git-svn/svn-server-sync/jogl/trunk@624 232f8b59-042b-4e1e-8c03-345bb8c30851

8 files changed, 64 insertions, 85 deletions

diff --git a/src/classes/com/sun/opengl/impl/GLContextImpl.java b/src/classes/com/sun/opengl/impl/GLContextImpl.java
index 5f945a334..795b06e9a 100644
--- a/src/classes/com/sun/opengl/impl/GLContextImpl.java
+++ b/src/classes/com/sun/opengl/impl/GLContextImpl.java
@@ -150,7 +150,14 @@ public abstract class GLContextImpl extends GLContext {
       }
     }
 
-    destroyImpl();
+    // Must hold the lock around the destroy operation to make sure we
+    // don't destroy the context out from under another thread rendering to it
+    lock.lock();
+    try {
+      destroyImpl();
+    } finally {
+      lock.unlock();
+    }
   }
 
   protected abstract void destroyImpl() throws GLException;
diff --git a/src/classes/com/sun/opengl/impl/macosx/MacOSXGLContext.java b/src/classes/com/sun/opengl/impl/macosx/MacOSXGLContext.java
index 0a2d2f613..f4cd14c47 100644
--- a/src/classes/com/sun/opengl/impl/macosx/MacOSXGLContext.java
+++ b/src/classes/com/sun/opengl/impl/macosx/MacOSXGLContext.java
@@ -153,7 +153,7 @@ public abstract class MacOSXGLContext extends GLContextImpl
       created = true;
     }
             
-    if (!CGL.makeCurrentContext(nsContext, drawable.getView())) {
+    if (!CGL.makeCurrentContext(nsContext)) {
       throw new GLException("Error making nsContext current");
     }
             
@@ -165,14 +165,14 @@ public abstract class MacOSXGLContext extends GLContextImpl
   }
 	
   protected void releaseImpl() throws GLException {
-    if (!CGL.clearCurrentContext(nsContext, drawable.getView())) {
+    if (!CGL.clearCurrentContext(nsContext)) {
       throw new GLException("Error freeing OpenGL nsContext");
     }
   }
 	
   protected void destroyImpl() throws GLException {
     if (nsContext != 0) {
-      if (!CGL.deleteContext(nsContext, 0)) {
+      if (!CGL.deleteContext(nsContext)) {
         throw new GLException("Unable to delete OpenGL context");
       }
       if (DEBUG) {
diff --git a/src/classes/com/sun/opengl/impl/macosx/MacOSXOnscreenGLContext.java b/src/classes/com/sun/opengl/impl/macosx/MacOSXOnscreenGLContext.java
index 9e20dd6dd..dedc5a0ee 100644
--- a/src/classes/com/sun/opengl/impl/macosx/MacOSXOnscreenGLContext.java
+++ b/src/classes/com/sun/opengl/impl/macosx/MacOSXOnscreenGLContext.java
@@ -54,13 +54,14 @@ public class MacOSXOnscreenGLContext extends MacOSXGLContext {
   }
 
   protected int makeCurrentImpl() throws GLException {
+    int lockRes = 0;
     try {
-      int lockRes = drawable.lockSurface();
+      lockRes = drawable.lockSurface();
       if (lockRes == MacOSXOnscreenGLDrawable.LOCK_SURFACE_NOT_READY) {
         return CONTEXT_NOT_CURRENT;
       }
       if (lockRes == MacOSXOnscreenGLDrawable.LOCK_SURFACE_CHANGED) {
-        super.destroy();
+        destroyImpl();
       }
       int ret = super.makeCurrentImpl();
       if ((ret == CONTEXT_CURRENT) ||
@@ -72,32 +73,18 @@ public class MacOSXOnscreenGLContext extends MacOSXGLContext {
         // do this updating only upon reshape of this component or reshape or movement
         // of an ancestor, but this also wasn't sufficient and left garbage on the
         // screen in some situations.
-        CGL.updateContext(nsContext, drawable.getView());
-      } else {
-        // View might not have been ready
-        drawable.unlockSurface();
+        CGL.updateContext(nsContext);
       }
       return ret;
-    } catch (RuntimeException e) {
-      try {
+    } finally {
+      if (lockRes != MacOSXOnscreenGLDrawable.LOCK_SURFACE_NOT_READY) {
         drawable.unlockSurface();
-      } catch (Exception e2) {
-        // do nothing if unlockSurface throws
       }
-      throw(e); 
-    }
-  }
-    
-  protected void releaseImpl() throws GLException {
-    try {
-      super.releaseImpl();
-    } finally {
-      drawable.unlockSurface();
     }
   }
     
   public void swapBuffers() throws GLException {
-    if (!CGL.flushBuffer(nsContext, drawable.getView())) {
+    if (!CGL.flushBuffer(nsContext)) {
       throw new GLException("Error swapping buffers");
     }
   }
diff --git a/src/classes/com/sun/opengl/impl/windows/WindowsOnscreenGLContext.java b/src/classes/com/sun/opengl/impl/windows/WindowsOnscreenGLContext.java
index 00647e7dd..4e1dbd1c0 100644
--- a/src/classes/com/sun/opengl/impl/windows/WindowsOnscreenGLContext.java
+++ b/src/classes/com/sun/opengl/impl/windows/WindowsOnscreenGLContext.java
@@ -54,40 +54,21 @@ public class WindowsOnscreenGLContext extends WindowsGLContext {
   }
   
   protected int makeCurrentImpl() throws GLException {
+    int lockRes = 0;
     try {
-      int lockRes = drawable.lockSurface();
+      lockRes = drawable.lockSurface();
       if (lockRes == WindowsOnscreenGLDrawable.LOCK_SURFACE_NOT_READY) {
         return CONTEXT_NOT_CURRENT;
       }
       if (lockRes == WindowsOnscreenGLDrawable.LOCK_SURFACE_CHANGED) {
-        if (hglrc != 0) {
-          if (!WGL.wglDeleteContext(hglrc)) {
-            throw new GLException("Unable to delete old GL context after surface changed");
-          }
-          GLContextShareSet.contextDestroyed(this);
-          if (DEBUG) {
-            System.err.println(getThreadName() + ": !!! Destroyed OpenGL context " + toHexString(hglrc) + " due to JAWT_LOCK_SURFACE_CHANGED");
-          }
-          hglrc = 0;
-        }
+        destroyImpl();
       }
       int ret = super.makeCurrentImpl();
       return ret;
-    } catch (RuntimeException e) {
-      try {
+    } finally {
+      if (lockRes != WindowsOnscreenGLDrawable.LOCK_SURFACE_NOT_READY) {
         drawable.unlockSurface();
-      } catch (Exception e2) {
-        // do nothing if unlockSurface throws
       }
-      throw(e); 
-    }
-  }
-
-  protected void releaseImpl() throws GLException {
-    try {
-      super.releaseImpl();
-    } finally {
-      drawable.unlockSurface();
     }
   }
 }
diff --git a/src/classes/com/sun/opengl/impl/x11/X11GLContext.java b/src/classes/com/sun/opengl/impl/x11/X11GLContext.java
index 5ae52fb1d..54d1c62f3 100644
--- a/src/classes/com/sun/opengl/impl/x11/X11GLContext.java
+++ b/src/classes/com/sun/opengl/impl/x11/X11GLContext.java
@@ -128,8 +128,6 @@ public abstract class X11GLContext extends GLContextImpl {
   }
 
   protected int makeCurrentImpl() throws GLException {
-    // FIXME: in offscreen (non-pbuffer) case this is run without the
-    // AWT lock held
     boolean created = false;
     if (context == 0) {
       create();
@@ -143,7 +141,7 @@ public abstract class X11GLContext extends GLContextImpl {
       throw new GLException("Error making context current");
     } else {
       mostRecentDisplay = drawable.getDisplay();
-      if (DEBUG && VERBOSE) {
+      if (DEBUG && (VERBOSE || created)) {
         System.err.println(getThreadName() + ": glXMakeCurrent(display " + toHexString(drawable.getDisplay()) +
                            ", drawable " + toHexString(drawable.getDrawable()) +
                            ", context " + toHexString(context) + ") succeeded");
@@ -158,14 +156,25 @@ public abstract class X11GLContext extends GLContextImpl {
   }
 
   protected void releaseImpl() throws GLException {
-    if (!GLX.glXMakeCurrent(drawable.getDisplay(), 0, 0)) {
-      throw new GLException("Error freeing OpenGL context");
+    lockToolkit();
+    try {
+      if (!GLX.glXMakeCurrent(mostRecentDisplay, 0, 0)) {
+        throw new GLException("Error freeing OpenGL context");
+      }
+    } finally {
+      unlockToolkit();
     }
   }
 
   protected void destroyImpl() throws GLException {
     lockToolkit();
     if (context != 0) {
+      if (DEBUG) {
+        System.err.println("glXDestroyContext(0x" +
+                           Long.toHexString(mostRecentDisplay) +
+                           ", 0x" +
+                           Long.toHexString(context) + ")");
+      }
       GLX.glXDestroyContext(mostRecentDisplay, context);
       if (DEBUG) {
         System.err.println("!!! Destroyed OpenGL context " + context);
@@ -199,7 +208,7 @@ public abstract class X11GLContext extends GLContextImpl {
   }
 
   public synchronized String getPlatformExtensionsString() {
-    if (drawable.getDisplay() == 0) {
+    if (mostRecentDisplay == 0) {
       throw new GLException("Context not current");
     }
     if (!glXQueryExtensionsStringInitialized) {
@@ -210,7 +219,7 @@ public abstract class X11GLContext extends GLContextImpl {
     if (glXQueryExtensionsStringAvailable) {
       lockToolkit();
       try {
-        String ret = GLX.glXQueryExtensionsString(drawable.getDisplay(), GLX.DefaultScreen(drawable.getDisplay()));
+        String ret = GLX.glXQueryExtensionsString(mostRecentDisplay, GLX.DefaultScreen(mostRecentDisplay));
         if (DEBUG) {
           System.err.println("!!! GLX extensions: " + ret);
         }
@@ -249,11 +258,16 @@ public abstract class X11GLContext extends GLContextImpl {
 
 
   public void setSwapInterval(int interval) {
-    // FIXME: make the context current first? Currently assumes that
-    // will not be necessary. Make the caller do this?
-    GLXExt glXExt = getGLXExt();
-    if (glXExt.isExtensionAvailable("GLX_SGI_swap_control")) {
-      glXExt.glXSwapIntervalSGI(interval);
+    lockToolkit();
+    try {
+      // FIXME: make the context current first? Currently assumes that
+      // will not be necessary. Make the caller do this?
+      GLXExt glXExt = getGLXExt();
+      if (glXExt.isExtensionAvailable("GLX_SGI_swap_control")) {
+        glXExt.glXSwapIntervalSGI(interval);
+      }
+    } finally {
+      unlockToolkit();
     }
   }
 
diff --git a/src/classes/com/sun/opengl/impl/x11/X11OffscreenGLContext.java b/src/classes/com/sun/opengl/impl/x11/X11OffscreenGLContext.java
index e1c8eb3d1..132573f7b 100644
--- a/src/classes/com/sun/opengl/impl/x11/X11OffscreenGLContext.java
+++ b/src/classes/com/sun/opengl/impl/x11/X11OffscreenGLContext.java
@@ -68,6 +68,15 @@ public class X11OffscreenGLContext extends X11GLContext {
     return true;
   }
 
+  protected int makeCurrentImpl() throws GLException {
+    lockToolkit();
+    try {
+      return super.makeCurrentImpl();
+    } finally {
+      unlockToolkit();
+    }
+  }
+
   protected void create() {
     createContext(false);
   }
diff --git a/src/classes/com/sun/opengl/impl/x11/X11OnscreenGLContext.java b/src/classes/com/sun/opengl/impl/x11/X11OnscreenGLContext.java
index d6b79cb8f..e666fc152 100644
--- a/src/classes/com/sun/opengl/impl/x11/X11OnscreenGLContext.java
+++ b/src/classes/com/sun/opengl/impl/x11/X11OnscreenGLContext.java
@@ -54,38 +54,19 @@ public class X11OnscreenGLContext extends X11GLContext {
   }
   
   protected int makeCurrentImpl() throws GLException {
+    int lockRes = drawable.lockSurface();
     try {
-      int lockRes = drawable.lockSurface();
       if (lockRes == X11OnscreenGLDrawable.LOCK_SURFACE_NOT_READY) {
         return CONTEXT_NOT_CURRENT;
       }
       if (lockRes == X11OnscreenGLDrawable.LOCK_SURFACE_CHANGED) {
-        if (context != 0) {
-          GLX.glXDestroyContext(mostRecentDisplay, context);
-          GLContextShareSet.contextDestroyed(this);
-          if (DEBUG) {
-            System.err.println(getThreadName() + ": !!! Destroyed OpenGL context " + toHexString(context) + " due to JAWT_LOCK_SURFACE_CHANGED");
-          }
-          context = 0;
-        }
+        destroyImpl();
       }
-      int ret = super.makeCurrentImpl();
-      return ret;
-    } catch (RuntimeException e) {
-      try {
+      return super.makeCurrentImpl();
+    } finally {
+      if (lockRes != X11OnscreenGLDrawable.LOCK_SURFACE_NOT_READY) {
         drawable.unlockSurface();
-      } catch (Exception e2) {
-        // do nothing if unlockSurface throws
       }
-      throw(e); 
-    }
-  }
-
-  protected void releaseImpl() throws GLException {
-    try {
-      super.releaseImpl();
-    } finally {
-      drawable.unlockSurface();
     }
   }
 
diff --git a/src/classes/com/sun/opengl/impl/x11/X11PbufferGLContext.java b/src/classes/com/sun/opengl/impl/x11/X11PbufferGLContext.java
index 3b0d51f13..5ff3bd86c 100644
--- a/src/classes/com/sun/opengl/impl/x11/X11PbufferGLContext.java
+++ b/src/classes/com/sun/opengl/impl/x11/X11PbufferGLContext.java
@@ -90,7 +90,7 @@ public class X11PbufferGLContext extends X11GLContext {
         throw new GLException("Error making context current");
       } else {
         mostRecentDisplay = drawable.getDisplay();
-        if (DEBUG && VERBOSE) {
+        if (DEBUG && (VERBOSE || created)) {
           System.err.println(getThreadName() + ": glXMakeCurrent(display " + toHexString(drawable.getDisplay()) +
                              ", drawable " + toHexString(drawable.getDrawable()) +
                              ", context " + toHexString(context) + ") succeeded");