GLContextImpl*: createImpl() / makeCurrentImpl() refinement / robostness

createImpl(): If successful must leave context current.

makeCurrentImpl(): Is only called if context is not just created,
hence the boolean parameter 'boolean newCreatedContext' is removed.

This clearifies and actually cleans up the native makeContextCurrent/releaseContext call pairs.

MacOSXCGLContext: CGL and NS impl. of native makeContextCurrent/releaseContext uses CGL locking
to provide a thread safety. This is recommended in OS X OpenGL documentation on
[shared context] multithreaded use cases.

Post creation code, as seen in some pbuffer cases is moved to overriden createImpl() methods.

14 files changed, 105 insertions, 49 deletions

diff --git a/src/jogl/classes/jogamp/opengl/GLContextImpl.java b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
index 9611a1651..d43c2d99d 100644
--- a/src/jogl/classes/jogamp/opengl/GLContextImpl.java
+++ b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
@@ -360,7 +360,7 @@ public abstract class GLContextImpl extends GLContext {
   public int makeCurrent() throws GLException {
     // One context can only be current by one thread,
     // and one thread can only have one context current!
-    GLContext current = getCurrent();
+    final GLContext current = getCurrent();
     if (current != null) {
       if (current == this) {
         // Assume we don't need to make this context current again
@@ -454,7 +454,6 @@ public abstract class GLContextImpl extends GLContext {
           if (0 == drawable.getHandle()) {
               throw new GLException("drawable has invalid handle: "+drawable);
           }
-          boolean newCreated = false;
           if (!isCreated()) {
             GLProfile.initProfiles(
                     getGLDrawable().getNativeSurface().getGraphicsConfiguration().getScreen().getDevice());
@@ -462,28 +461,30 @@ public abstract class GLContextImpl extends GLContext {
             if (null != shareWith) {
                 shareWith.getDrawableImpl().lockSurface();
             }
+            boolean created;
             try {
-                newCreated = createImpl(shareWith); // may throws exception if fails!
+                created = createImpl(shareWith); // may throws exception if fails!
             } finally {
                 if (null != shareWith) {
                     shareWith.getDrawableImpl().unlockSurface();
                 }                
             }
             if (DEBUG) {
-                if(newCreated) {
+                if(created) {
                     System.err.println(getThreadName() + ": !!! Create GL context OK: " + toHexString(contextHandle) + " for " + getClass().getName());
                 } else {
                     System.err.println(getThreadName() + ": !!! Create GL context FAILED for " + getClass().getName());
                 }
             }
-            if(!newCreated) {
+            if(!created) {
                 shallUnlockSurface = true;
                 return CONTEXT_NOT_CURRENT;
             }
             GLContextShareSet.contextCreated(this);
+            return CONTEXT_CURRENT_NEW;
           }
-          makeCurrentImpl(newCreated);
-          return newCreated ? CONTEXT_CURRENT_NEW : CONTEXT_CURRENT ;
+          makeCurrentImpl();
+          return CONTEXT_CURRENT;
       } catch (RuntimeException e) {
         shallUnlockSurface = true;
         throw e;
@@ -494,26 +495,41 @@ public abstract class GLContextImpl extends GLContext {
       }
     }
   }
-  protected abstract void makeCurrentImpl(boolean newCreatedContext) throws GLException;
+  protected abstract void makeCurrentImpl() throws GLException;
+  
+  /** 
+   * Platform dependent entry point for context creation.<br>
+   *
+   * This method is called from {@link #makeCurrentLocking()} .. {@link #makeCurrent()} .<br>
+   *
+   * The implementation shall verify this context with a 
+   * <code>MakeContextCurrent</code> call.<br>
+   *
+   * The implementation <b>must</b> leave the context current.<br>
+   * 
+   * @param share the shared context or null
+   * @return the valid and current context if successful, or null
+   * @throws GLException
+   */
   protected abstract boolean createImpl(GLContextImpl sharedWith) throws GLException ;
 
   /** 
    * Platform dependent but harmonized implementation of the <code>ARB_create_context</code>
    * mechanism to create a context.<br>
    *
-   * This method is called from {@link #createContextARB}.<br>
+   * This method is called from {@link #createContextARB}, {@link #createImpl(GLContextImpl)} .. {@link #makeCurrent()} .<br>
    *
    * The implementation shall verify this context with a 
    * <code>MakeContextCurrent</code> call.<br>
    *
-   * The implementation shall leave the context current.<br>
+   * The implementation <b>must</b> leave the context current.<br>
    *
    * @param share the shared context or null
    * @param direct flag if direct is requested
    * @param ctxOptionFlags <code>ARB_create_context</code> related, see references below
    * @param major major number
    * @param minor minor number
-   * @return the valid context if successfull, or null
+   * @return the valid and current context if successful, or null
    *
    * @see #makeCurrent
    * @see #CTX_PROFILE_COMPAT
diff --git a/src/jogl/classes/jogamp/opengl/egl/EGLContext.java b/src/jogl/classes/jogamp/opengl/egl/EGLContext.java
index 6af35e3aa..6f1ab4b07 100644
--- a/src/jogl/classes/jogamp/opengl/egl/EGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/egl/EGLContext.java
@@ -98,7 +98,7 @@ public abstract class EGLContext extends GLContextImpl {
         return true;
     }
 
-    protected void makeCurrentImpl(boolean newCreated) throws GLException {
+    protected void makeCurrentImpl() throws GLException {
         if(EGL.EGL_NO_DISPLAY==((EGLDrawable)drawable).getDisplay() ) {
             throw new GLException("drawable not properly initialized, NO DISPLAY: "+drawable);
         }
diff --git a/src/jogl/classes/jogamp/opengl/egl/EGLExternalContext.java b/src/jogl/classes/jogamp/opengl/egl/EGLExternalContext.java
index 389daa7ca..4c45241d3 100644
--- a/src/jogl/classes/jogamp/opengl/egl/EGLExternalContext.java
+++ b/src/jogl/classes/jogamp/opengl/egl/EGLExternalContext.java
@@ -66,7 +66,7 @@ public class EGLExternalContext extends EGLContext {
         lastContext = null;
     }
 
-    protected void makeCurrentImpl(boolean newCreated) throws GLException {
+    protected void makeCurrentImpl() throws GLException {
     }
 
     protected void releaseImpl() throws GLException {
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
index d063c3a7b..87ce199ec 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
@@ -236,11 +236,10 @@ public abstract class MacOSXCGLContext extends GLContextImpl
     return 0 != contextHandle;
   }
   
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
+  protected void makeCurrentImpl() throws GLException {
     if (getOpenGLMode() != ((MacOSXCGLDrawable)drawable).getOpenGLMode()) {
       setOpenGLMode(((MacOSXCGLDrawable)drawable).getOpenGLMode());
     }
-
     if (!impl.makeCurrent(contextHandle)) {
       throw new GLException("Error making Context current: "+this);
     }      
@@ -511,11 +510,30 @@ public abstract class MacOSXCGLContext extends GLContextImpl
     }
     
     public boolean makeCurrent(long ctx) {
-      return CGL.makeCurrentContext(ctx);
+      final long cglCtx = CGL.getCGLContext(ctx);
+      if(0 == cglCtx) {
+          throw new InternalError("Null CGLContext for: "+this);
+      }
+      int err = CGL.CGLLockContext(cglCtx);
+      if(CGL.kCGLNoError == err) {
+          return CGL.makeCurrentContext(ctx);
+      } else if(DEBUG) {
+          System.err.println("NSGL: Could not lock context: err 0x"+Integer.toHexString(err)+": "+this);
+      }
+      return false;
     }
 
     public boolean release(long ctx) {
-      return CGL.clearCurrentContext(ctx);
+      final boolean res = CGL.clearCurrentContext(ctx);
+      final long cglCtx = CGL.getCGLContext(ctx);
+      if(0 == cglCtx) {
+          throw new InternalError("Null CGLContext for: "+this);
+      }
+      final int err = CGL.CGLUnlockContext(cglCtx);
+      if(DEBUG && CGL.kCGLNoError != err) {
+          System.err.println("CGL: Could not unlock context: err 0x"+Integer.toHexString(err)+": "+this);
+      }      
+      return res && CGL.kCGLNoError == err;
     }
 
     public boolean setSwapInterval(int interval) {
@@ -594,11 +612,30 @@ public abstract class MacOSXCGLContext extends GLContextImpl
     }
     
     public boolean makeCurrent(long ctx) {
-      return CGL.CGLSetCurrentContext(ctx) == CGL.kCGLNoError;
+      int err = CGL.CGLLockContext(ctx);
+      if(CGL.kCGLNoError == err) {
+          err = CGL.CGLSetCurrentContext(ctx);
+          if(CGL.kCGLNoError == err) {
+              return true;
+          } else if(DEBUG) {
+              System.err.println("CGL: Could not make context current: err 0x"+Integer.toHexString(err)+": "+this);
+          }
+      } else if(DEBUG) {
+          System.err.println("CGL: Could not lock context: err 0x"+Integer.toHexString(err)+": "+this);
+      }
+      return false;
     }
 
     public boolean release(long ctx) {
-      return (CGL.CGLSetCurrentContext(0) == CGL.kCGLNoError);
+      int err = CGL.CGLSetCurrentContext(0);
+      if(DEBUG && CGL.kCGLNoError != err) {
+          System.err.println("CGL: Could not release current context: err 0x"+Integer.toHexString(err)+": "+this);
+      }
+      int err2 = CGL.CGLUnlockContext(ctx);
+      if(DEBUG && CGL.kCGLNoError != err2) {
+          System.err.println("CGL: Could not unlock context: err 0x"+Integer.toHexString(err2)+": "+this);
+      }
+      return CGL.kCGLNoError == err && CGL.kCGLNoError == err2;
     }
     
     public boolean setSwapInterval(int interval) {
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawableFactory.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawableFactory.java
index 3f350f59f..cea4feea8 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawableFactory.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawableFactory.java
@@ -49,7 +49,6 @@ import javax.media.nativewindow.AbstractGraphicsConfiguration;
 import javax.media.nativewindow.AbstractGraphicsDevice;
 import javax.media.nativewindow.AbstractGraphicsScreen;
 import javax.media.nativewindow.DefaultGraphicsScreen;
-import javax.media.nativewindow.GraphicsConfigurationFactory;
 import javax.media.nativewindow.NativeSurface;
 import javax.media.nativewindow.NativeWindowFactory;
 import javax.media.nativewindow.ProxySurface;
@@ -212,6 +211,7 @@ public class MacOSXCGLDrawableFactory extends GLDrawableFactoryImpl {
                             gle.printStackTrace();
                         }
                     } finally {
+                        context.release();
                         context.destroy();
                     }
                 }
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXExternalCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXExternalCGLContext.java
index 2b24ed378..1a0319cc6 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXExternalCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXExternalCGLContext.java
@@ -139,7 +139,7 @@ public class MacOSXExternalCGLContext extends MacOSXCGLContext {
     lastContext = null;
   }
 
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
+  protected void makeCurrentImpl() throws GLException {
   }
 
   protected void releaseImpl() throws GLException {
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXOnscreenCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXOnscreenCGLContext.java
index 6de94085f..9e051311c 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXOnscreenCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXOnscreenCGLContext.java
@@ -52,8 +52,8 @@ public class MacOSXOnscreenCGLContext extends MacOSXCGLContext {
   }
 
   @Override
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
-      super.makeCurrentImpl(newCreated);
+  protected void makeCurrentImpl() throws GLException {
+      super.makeCurrentImpl();
       drawableUpdatedNotify();  
   }
     
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXPbufferCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXPbufferCGLContext.java
index f4b71d37d..7ba7d2d5a 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXPbufferCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXPbufferCGLContext.java
@@ -38,6 +38,8 @@ import javax.media.opengl.GLContext;
 import javax.media.opengl.GLException;
 import javax.media.opengl.GLPbuffer;
 
+import jogamp.opengl.GLContextImpl;
+
 public class MacOSXPbufferCGLContext extends MacOSXCGLContext {
 
   // State for render-to-texture and render-to-texture-rectangle support
@@ -60,10 +62,9 @@ public class MacOSXPbufferCGLContext extends MacOSXCGLContext {
   public void releasePbufferFromTexture() {
   }
 
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
-    super.makeCurrentImpl(newCreated);
-    
-    if (newCreated) {
+  protected boolean createImpl(GLContextImpl shareWith) {
+    boolean res = super.createImpl(shareWith);
+    if(res) {  
       // Initialize render-to-texture support if requested
       final GL gl = getGL();
       final MacOSXPbufferCGLDrawable osxPDrawable = (MacOSXPbufferCGLDrawable)drawable;
@@ -81,6 +82,7 @@ public class MacOSXPbufferCGLContext extends MacOSXCGLContext {
                       0, GL.GL_RGB, GL.GL_UNSIGNED_BYTE, null);
       gl.glCopyTexSubImage2D(textureTarget, 0, 0, 0, 0, 0, drawable.getWidth(), drawable.getHeight());
     }
+    return res;
   }
 
   public int getFloatingPointMode() {
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/awt/MacOSXJava2DCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/awt/MacOSXJava2DCGLContext.java
index a9376fb34..cfec6d6a4 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/awt/MacOSXJava2DCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/awt/MacOSXJava2DCGLContext.java
@@ -74,7 +74,7 @@ public class MacOSXJava2DCGLContext extends MacOSXCGLContext implements Java2DGL
     this.graphics = g;
   }
 
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
+  protected void makeCurrentImpl() throws GLException {
     if (!Java2D.makeOGLContextCurrentOnSurface(graphics, contextHandle)) {
       throw new GLException("Error making context current");
     }            
@@ -85,6 +85,16 @@ public class MacOSXJava2DCGLContext extends MacOSXCGLContext implements Java2DGL
     
     long ctx = Java2D.createOGLContextOnSurface(graphics, share);
     if (ctx == 0) {
+      if(DEBUG) { 
+          System.err.println("Error creating current: "+this);
+      }
+      return false;
+    }
+    if (!Java2D.makeOGLContextCurrentOnSurface(graphics, contextHandle)) {
+      Java2D.destroyOGLContext(ctx);
+      if(DEBUG) { 
+          System.err.println("Error making created context current: "+this);
+      }
       return false;
     }
     setGLFunctionAvailability(true, true, 0, 0, CTX_PROFILE_COMPAT|CTX_OPTION_ANY); // use GL_VERSION
diff --git a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsExternalWGLContext.java b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsExternalWGLContext.java
index c3588fd48..7882982eb 100644
--- a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsExternalWGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsExternalWGLContext.java
@@ -122,7 +122,7 @@ public class WindowsExternalWGLContext extends WindowsWGLContext {
     lastContext = null;
   }
 
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
+  protected void makeCurrentImpl() throws GLException {
   }
 
   protected void releaseImpl() throws GLException {
diff --git a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsPbufferWGLContext.java b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsPbufferWGLContext.java
index 97c63ea52..0f610495d 100644
--- a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsPbufferWGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsPbufferWGLContext.java
@@ -42,6 +42,8 @@ package jogamp.opengl.windows.wgl;
 
 import javax.media.opengl.*;
 
+import jogamp.opengl.GLContextImpl;
+
 public class WindowsPbufferWGLContext extends WindowsWGLContext {
   // State for render-to-texture and render-to-texture-rectangle support
   private boolean rtt;       // render-to-texture?
@@ -86,9 +88,9 @@ public class WindowsPbufferWGLContext extends WindowsWGLContext {
     }
   }
 
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
-    super.makeCurrentImpl(newCreated);
-    if (newCreated) {
+  protected boolean createImpl(GLContextImpl shareWith) {
+    boolean res = super.createImpl(shareWith);
+    if(res) {
       GLCapabilitiesImmutable capabilities = drawable.getChosenGLCapabilities();
 
       // Initialize render-to-texture support if requested
@@ -135,6 +137,7 @@ public class WindowsPbufferWGLContext extends WindowsWGLContext {
         }
       }
     }
+    return res;
   }
 
   public int getFloatingPointMode() {
diff --git a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLContext.java b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLContext.java
index 300b5c5c7..10963b70f 100644
--- a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLContext.java
@@ -332,7 +332,7 @@ public class WindowsWGLContext extends GLContextImpl {
         if(glCaps.getGLProfile().isGL3()) {
           WGL.wglMakeCurrent(0, 0);
           WGL.wglDeleteContext(temp_ctx);
-          throw new GLException("WindowsWGLContext.createContext failed, but context > GL2 requested "+getGLVersion()+", ");
+          throw new GLException("WindowsWGLContext.createContext ctx !ARB, context > GL2 requested "+getGLVersion());
         }
         if(DEBUG) {
           System.err.println("WindowsWGLContext.createContext failed, fall back to !ARB context "+getGLVersion());
@@ -361,15 +361,10 @@ public class WindowsWGLContext extends GLContextImpl {
     return true;
   }
   
-  protected void  makeCurrentImpl(boolean newCreated) throws GLException {
+  protected void  makeCurrentImpl() throws GLException {
     if (WGL.wglGetCurrentContext() != contextHandle) {
       if (!wglMakeContextCurrent(drawable.getHandle(), drawableRead.getHandle(), contextHandle)) {
         throw new GLException("Error making context current: 0x" + toHexString(contextHandle) + ", werr: " + GDI.GetLastError() + ", " + this);
-      } else {
-        if (DEBUG && newCreated) {
-          System.err.println(getThreadName() + ": wglMakeCurrent(hdc " + toHexString(drawable.getHandle()) +
-                             ", contextHandle " + toHexString(contextHandle) + ") succeeded");
-        }
       }
     }
   }
diff --git a/src/jogl/classes/jogamp/opengl/x11/glx/X11ExternalGLXContext.java b/src/jogl/classes/jogamp/opengl/x11/glx/X11ExternalGLXContext.java
index 70881a040..e2297d33a 100644
--- a/src/jogl/classes/jogamp/opengl/x11/glx/X11ExternalGLXContext.java
+++ b/src/jogl/classes/jogamp/opengl/x11/glx/X11ExternalGLXContext.java
@@ -116,7 +116,7 @@ public class X11ExternalGLXContext extends X11GLXContext {
     lastContext = null;
   }
 
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
+  protected void makeCurrentImpl() throws GLException {
   }
 
   protected void releaseImpl() throws GLException {
diff --git a/src/jogl/classes/jogamp/opengl/x11/glx/X11GLXContext.java b/src/jogl/classes/jogamp/opengl/x11/glx/X11GLXContext.java
index d3507cbaa..af164989f 100644
--- a/src/jogl/classes/jogamp/opengl/x11/glx/X11GLXContext.java
+++ b/src/jogl/classes/jogamp/opengl/x11/glx/X11GLXContext.java
@@ -370,7 +370,7 @@ public abstract class X11GLXContext extends GLContextImpl {
         if(glp.isGL3()) {
           glXMakeContextCurrent(display, 0, 0, 0);
           GLX.glXDestroyContext(display, temp_ctx);
-          throw new GLException("X11GLXContext.createContextImpl failed, but context > GL2 requested - requested: "+glp+", current: "+getGLVersion()+", ");
+          throw new GLException("X11GLXContext.createContextImpl ctx !ARB, context > GL2 requested - requested: "+glp+", current: "+getGLVersion()+", ");
         }
         if(DEBUG) {
           System.err.println("X11GLXContext.createContextImpl failed, fall back to !ARB context "+getGLVersion());
@@ -394,7 +394,7 @@ public abstract class X11GLXContext extends GLContextImpl {
     return true;
   }
 
-  protected void makeCurrentImpl(boolean newCreated) throws GLException {
+  protected void makeCurrentImpl() throws GLException {
     long dpy = drawable.getNativeSurface().getDisplayHandle();
 
     if (GLX.glXGetCurrentContext() != contextHandle) {
@@ -406,13 +406,6 @@ public abstract class X11GLXContext extends GLContextImpl {
         } finally {
             X11Util.setX11ErrorHandler(false, false);
         }
-        if (DEBUG && newCreated) {
-            System.err.println(getThreadName() + ": glXMakeCurrent(display " + 
-                               toHexString(dpy)+
-                               ", drawable " + toHexString(drawable.getHandle()) +
-                               ", drawableRead " + toHexString(drawableRead.getHandle()) +
-                               ", context " + toHexString(contextHandle) + ") succeeded");
-        }
     }
   }