Bug 894 - GLDrawableFactory* [dummy|offscreen] Surface creation w/ own device does _not_ require locking on global shared device.

5 files changed, 41 insertions, 36 deletions

diff --git a/src/jogl/classes/javax/media/opengl/GLDrawableFactory.java b/src/jogl/classes/javax/media/opengl/GLDrawableFactory.java
index 2af4ba306..2b1d228ad 100644
--- a/src/jogl/classes/javax/media/opengl/GLDrawableFactory.java
+++ b/src/jogl/classes/javax/media/opengl/GLDrawableFactory.java
@@ -490,6 +490,9 @@ public abstract class GLDrawableFactory {
    * If neither FBO nor Pbuffer is available,
    * a simple pixmap/bitmap auto drawable is created, which is unlikely to be hardware accelerated.
    * </p>
+   * <p>
+   * The resulting {@link GLOffscreenAutoDrawable} has it's own independent device instance using <code>device</code> details.
+   * </p>
    *
    * @param device which {@link AbstractGraphicsDevice#getConnection() connection} denotes the shared device to be used, may be <code>null</code> for the platform's default device.
    * @param caps the requested GLCapabilties
@@ -540,6 +543,9 @@ public abstract class GLDrawableFactory {
    * If neither FBO nor Pbuffer is available,
    * a simple pixmap/bitmap auto drawable is created, which is unlikely to be hardware accelerated.
    * </p>
+   * <p>
+   * The resulting {@link GLOffscreenAutoDrawable} has it's own independent device instance using <code>device</code> details.
+   * </p>
    *
    * @param device which {@link AbstractGraphicsDevice#getConnection() connection} denotes the shared device to be used, may be <code>null</code> for the platform's default device.
    * @param caps the requested GLCapabilties
@@ -604,6 +610,9 @@ public abstract class GLDrawableFactory {
    * If neither FBO nor Pbuffer is available,
    * a simple pixmap/bitmap drawable is created, which is unlikely to be hardware accelerated.
    * </p>
+   * <p>
+   * The resulting {@link GLDrawable} has it's own independent device instance using <code>device</code> details.
+   * </p>
    *
    * @param device which {@link AbstractGraphicsDevice#getConnection() connection} denotes the shared device to be used, may be <code>null</code> for the platform's default device.
    * @param caps the requested GLCapabilties
@@ -657,6 +666,10 @@ public abstract class GLDrawableFactory {
    * you will be able to implement a new native windowing system  binding almost on-the-fly,
    * see {@link com.jogamp.opengl.swt.GLCanvas}.
    * </p>
+   * <p>
+   * The resulting {@link GLOffscreenAutoDrawable} has it's own independent device instance using <code>device</code> details
+   * which may be blocking depending on platform and windowing-toolkit requirements.
+   * </p>
    *
    * @param device which {@link AbstractGraphicsDevice#getConnection() connection} denotes the shared the target device, may be <code>null</code> for the platform's default device.
    *        Caller has to ensure it is compatible w/ the given <code>windowHandle</code>
diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableFactoryImpl.java b/src/jogl/classes/jogamp/opengl/GLDrawableFactoryImpl.java
index cadf40f02..c914b5e10 100644
--- a/src/jogl/classes/jogamp/opengl/GLDrawableFactoryImpl.java
+++ b/src/jogl/classes/jogamp/opengl/GLDrawableFactoryImpl.java
@@ -270,16 +270,10 @@ public abstract class GLDrawableFactoryImpl extends GLDrawableFactory {
     }
 
     final GLCapabilitiesImmutable capsChosen = GLGraphicsConfigurationUtil.fixGLPBufferGLCapabilities(capsRequested);
-    GLDrawableImpl drawable = null;
-    device.lock();
-    try {
-        drawable = createOffscreenDrawableImpl( createMutableSurfaceImpl(device, true, capsChosen, capsRequested, chooser,
-                                                                     new UpstreamSurfaceHookMutableSize(width, height) ) );
-        if(null != drawable) {
-            drawable.setRealized(true);
-        }
-    } finally {
-        device.unlock();
+    final GLDrawableImpl drawable = createOffscreenDrawableImpl( createMutableSurfaceImpl(device, true, capsChosen, capsRequested, chooser,
+                                                                 new UpstreamSurfaceHookMutableSize(width, height) ) );
+    if(null != drawable) {
+        drawable.setRealized(true);
     }
 
     return new GLPbufferImpl( drawable, (GLContextImpl) drawable.createContext(shareWith) );
@@ -350,19 +344,14 @@ public abstract class GLDrawableFactoryImpl extends GLDrawableFactory {
 
     final GLCapabilitiesImmutable capsChosen = GLGraphicsConfigurationUtil.fixOffscreenGLCapabilities(capsRequested, this, device);
 
-    device.lock();
-    try {
-        if( capsChosen.isFBO() ) {
-            // Use minimum GLCapabilities for the dummy surface w/ same profile
-            final ProxySurface dummySurface = createDummySurfaceImpl(device, true, new GLCapabilities(capsChosen.getGLProfile()), capsRequested, null, width, height);
-            final GLDrawableImpl dummyDrawable = createOnscreenDrawableImpl(dummySurface);
-            return new GLFBODrawableImpl.ResizeableImpl(this, dummyDrawable, dummySurface, capsChosen, 0);
-        }
-        return createOffscreenDrawableImpl( createMutableSurfaceImpl(device, true, capsChosen, capsRequested, chooser,
-                                                                     new UpstreamSurfaceHookMutableSize(width, height) ) );
-    } finally {
-        device.unlock();
+    if( capsChosen.isFBO() ) {
+        // Use minimum GLCapabilities for the dummy surface w/ same profile
+        final ProxySurface dummySurface = createDummySurfaceImpl(device, true, new GLCapabilities(capsChosen.getGLProfile()), capsRequested, null, width, height);
+        final GLDrawableImpl dummyDrawable = createOnscreenDrawableImpl(dummySurface);
+        return new GLFBODrawableImpl.ResizeableImpl(this, dummyDrawable, dummySurface, capsChosen, 0);
     }
+    return createOffscreenDrawableImpl( createMutableSurfaceImpl(device, true, capsChosen, capsRequested, chooser,
+                                                                 new UpstreamSurfaceHookMutableSize(width, height) ) );
   }
 
   @Override
@@ -371,12 +360,16 @@ public abstract class GLDrawableFactoryImpl extends GLDrawableFactory {
     if(null == device) {
         throw new GLException("No shared device for requested: "+deviceReq+", createNewDevice "+createNewDevice);
     }
-    device.lock();
+    if( !createNewDevice ) {
+        device.lock();
+    }
     try {
         final ProxySurface dummySurface = createDummySurfaceImpl(device, createNewDevice, capsRequested, capsRequested, chooser, 64, 64);
         return createOnscreenDrawableImpl(dummySurface);
     } finally {
-        device.unlock();
+        if( !createNewDevice ) {
+            device.unlock();
+        }
     }
   }
 
@@ -398,7 +391,7 @@ public abstract class GLDrawableFactoryImpl extends GLDrawableFactory {
    * Lifecycle (destruction) of the TBD surface handle shall be handled by the caller.
    * </p>
    * @param device a valid platform dependent target device.
-   * @param createNewDevice if <code>true</code> a new device instance is created using <code>device</code> details,
+   * @param createNewDevice if <code>true</code> a new independent device instance is created using <code>device</code> details,
    *                        otherwise <code>device</code> instance is used as-is.
    * @param capsChosen
    * @param capsRequested
@@ -418,6 +411,9 @@ public abstract class GLDrawableFactoryImpl extends GLDrawableFactory {
    * It is used to allow the creation of a {@link GLDrawable} and {@link GLContext} to query information.
    * It also allows creation of framebuffer objects which are used for rendering or using a shared GLContext w/o actually rendering to a usable framebuffer.
    * </p>
+   * <p>
+   * Creates a new independent device instance using <code>deviceReq</code> details.
+   * </p>
    * @param deviceReq which {@link javax.media.nativewindow.AbstractGraphicsDevice#getConnection() connection} denotes the shared device to be used, may be <code>null</code> for the platform's default device.
    * @param requestedCaps
    * @param chooser the custom chooser, may be null for default
@@ -434,12 +430,7 @@ public abstract class GLDrawableFactoryImpl extends GLDrawableFactory {
     if(null == device) {
         throw new GLException("No shared device for requested: "+deviceReq);
     }
-    device.lock();
-    try {
-        return createDummySurfaceImpl(device, true, requestedCaps, requestedCaps, chooser, width, height);
-    } finally {
-        device.unlock();
-    }
+    return createDummySurfaceImpl(device, true, requestedCaps, requestedCaps, chooser, width, height);
   }
 
   /**
diff --git a/src/jogl/classes/jogamp/opengl/egl/EGLDisplayUtil.java b/src/jogl/classes/jogamp/opengl/egl/EGLDisplayUtil.java
index 3d864ad76..0577124eb 100644
--- a/src/jogl/classes/jogamp/opengl/egl/EGLDisplayUtil.java
+++ b/src/jogl/classes/jogamp/opengl/egl/EGLDisplayUtil.java
@@ -34,6 +34,7 @@ import java.util.Iterator;
 import javax.media.nativewindow.AbstractGraphicsDevice;
 import javax.media.nativewindow.NativeSurface;
 import javax.media.nativewindow.NativeWindowFactory;
+import javax.media.nativewindow.ToolkitLock;
 import javax.media.opengl.GLException;
 
 import jogamp.opengl.Debug;
@@ -260,6 +261,7 @@ public class EGLDisplayUtil {
     };
 
     /**
+     * Using the default {@link ToolkitLock}, via {@link NativeWindowFactory#getDefaultToolkitLock(String, long)}.
      * @param nativeDisplayID
      * @param connection
      * @param unitID
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawableFactory.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawableFactory.java
index 994eee8d7..e41c97827 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawableFactory.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawableFactory.java
@@ -151,7 +151,7 @@ public class MacOSXCGLDrawableFactory extends GLDrawableFactoryImpl {
   static class SharedResource implements SharedResourceRunner.Resource {
       // private MacOSXCGLDrawable drawable;
       // private MacOSXCGLContext context;
-      private GLRendererQuirks glRendererQuirks;
+      private final GLRendererQuirks glRendererQuirks;
       MacOSXGraphicsDevice device;
       boolean valid;
       boolean hasNPOTTextures;
@@ -211,7 +211,7 @@ public class MacOSXCGLDrawableFactory extends GLDrawableFactoryImpl {
       return false;
   }
 
-  private HashSet<String> devicesTried = new HashSet<String>();
+  private final HashSet<String> devicesTried = new HashSet<String>();
 
   private boolean getDeviceTried(String connection) {
       synchronized (devicesTried) {
diff --git a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLDrawableFactory.java b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLDrawableFactory.java
index 95485b033..64ed197ea 100644
--- a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLDrawableFactory.java
+++ b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLDrawableFactory.java
@@ -41,7 +41,6 @@
 package jogamp.opengl.windows.wgl;
 
 import java.nio.Buffer;
-
 import java.nio.ShortBuffer;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -171,8 +170,8 @@ public class WindowsWGLDrawableFactory extends GLDrawableFactoryImpl {
   private HashMap<String /*connection*/, SharedResourceRunner.Resource> sharedMap;
 
   private long processAffinityChanges = 0;
-  private PointerBuffer procMask = PointerBuffer.allocateDirect(1);
-  private PointerBuffer sysMask = PointerBuffer.allocateDirect(1);
+  private final PointerBuffer procMask = PointerBuffer.allocateDirect(1);
+  private final PointerBuffer sysMask = PointerBuffer.allocateDirect(1);
 
   @Override
   protected void enterThreadCriticalZone() {