Fixed Issue 151: starting up the Animator before the GLJPanel has been shown result in an error

The root cause of this error was the fact that
WindowsPbufferGLContext.destroyImpl() uses WGL extensions to clean up
resources associated with the pbuffer. Because these extensions are in
the public WGL interface, they are wrapped by the DebugGL. However, an
OpenGL context is not current at the time these routines are called,
and it is illegal to call glGetError() at those points. The DebugGL
pipeline was implicitly calling glGetError() after each of those
calls, leading to the failure.

This bug unmasked a couple of others. The code in the DebugGL needed a
recursion count to make sure that glGetError() didn't get called in an
infinite loop. Also, as a side effect of the fix for Issue 160,
calling getGL() on the GLJPanel outside of GLEventListener.init() was
causing a NullPointerException to be thrown. The GLJPanel has been
fixed to return null in this case, and the specification of
GLDrawable.getGL() has been improved. In order to make the behavior
between the GLCanvas and GLJPanel similar, the GL object is now reset
in the GLDrawable each time the underlying OpenGL context is
recreated. This allows end users to set up e.g. the DebugGL
unconditionally in their GLEventListener.init() method. The JOGL demos
have been changed to reflect this.

The test case in the bug report will be updated with code similar to
the originally submitted test case (i.e., the Animator is started
early) but which now works.


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

5 files changed, 47 insertions, 6 deletions

diff --git a/src/net/java/games/gluegen/opengl/BuildComposablePipeline.java b/src/net/java/games/gluegen/opengl/BuildComposablePipeline.java
index 230add51a..895de5d2d 100644
--- a/src/net/java/games/gluegen/opengl/BuildComposablePipeline.java
+++ b/src/net/java/games/gluegen/opengl/BuildComposablePipeline.java
@@ -302,6 +302,9 @@ public class BuildComposablePipeline
       output.print(  "  public " + getPipelineName() + "(" + baseName + " ");
       output.println(getDownstreamObjectName() + ")");
       output.println("  {");
+      output.println("    if (" + getDownstreamObjectName() + " == null) {");
+      output.println("      throw new IllegalArgumentException(\"null " + getDownstreamObjectName() + "\");");
+      output.println("    }");
       output.print(  "    this." + getDownstreamObjectName());
       output.println(" = " + getDownstreamObjectName() + ";");
       output.println("  }");
@@ -374,6 +377,7 @@ public class BuildComposablePipeline
       output.println();
       output.println("    // Loop repeatedly to allow for distributed GL implementations,");
       output.println("    // as detailed in the glGetError() specification");
+      output.println("    int recursionDepth = 10;");
       output.println("    do {");
       output.println("      switch (err) {");
       output.println("        case GL_INVALID_ENUM: buf.append(\"GL_INVALID_ENUM \"); break;");
@@ -385,7 +389,7 @@ public class BuildComposablePipeline
       output.println("        case GL_NO_ERROR: throw new InternalError(\"Should not be treating GL_NO_ERROR as error\");");
       output.println("        default: throw new InternalError(\"Unknown glGetError() return value: \" + err);");
       output.println("      }");
-      output.println(  "    } while ((err = " +
+      output.println("    } while ((--recursionDepth >= 0) && (err = " +
 		     getDownstreamObjectName() +
 		     ".glGetError()) != GL_NO_ERROR);");
       output.println("    throw new GLException(buf.toString());");
@@ -459,6 +463,9 @@ public class BuildComposablePipeline
       output.print(  "  public " + getPipelineName() + "(" + getBaseInterfaceName() + " ");
       output.println(getDownstreamObjectName() + ", PrintStream " + getOutputStreamName() + ")");
       output.println("  {");
+      output.println("    if (" + getDownstreamObjectName() + " == null) {");
+      output.println("      throw new IllegalArgumentException(\"null " + getDownstreamObjectName() + "\");");
+      output.println("    }");
       output.print(  "    this." + getDownstreamObjectName());
       output.println(" = " + getDownstreamObjectName() + ";");
       output.print(  "    this." + getOutputStreamName());
diff --git a/src/net/java/games/jogl/GLDrawable.java b/src/net/java/games/jogl/GLDrawable.java
index 7c671c0b2..7c120f7af 100644
--- a/src/net/java/games/jogl/GLDrawable.java
+++ b/src/net/java/games/jogl/GLDrawable.java
@@ -94,10 +94,22 @@ public interface GLDrawable extends ComponentEvents {
       Dimension is null a new one will be allocated and returned. */
   public Dimension getSize(Dimension d);
 
-  /** Returns the {@link GL} pipeline object this GLDrawable uses. */
+  /** Returns the {@link GL} pipeline object this GLDrawable uses.  If
+      this method is called outside of the {@link GLEventListener}'s
+      callback methods (init, display, etc.) it may return null. Users
+      should not rely on the identity of the returned GL object; for
+      example, users should not maintain a hash table with the GL
+      object as the key. Additionally, the GL object should not be
+      cached in client code, but should be re-fetched from the
+      GLDrawable at the beginning of each call to init, display,
+      etc. */
   public GL getGL();
 
-  /** Sets the {@link GL} pipeline object this GLDrawable uses. */
+  /** Sets the {@link GL} pipeline object this GLDrawable uses. This
+      should only be called from within the GLEventListener's callback
+      methods, and usually only from within the init() method, in
+      order to install a composable pipeline. See the JOGL demos for
+      examples. */
   public void setGL(GL gl);
 
   /** Returns the {@link GLU} pipeline object this GLDrawable uses. */
diff --git a/src/net/java/games/jogl/GLJPanel.java b/src/net/java/games/jogl/GLJPanel.java
index d21081dfe..bf102710b 100644
--- a/src/net/java/games/jogl/GLJPanel.java
+++ b/src/net/java/games/jogl/GLJPanel.java
@@ -291,17 +291,27 @@ public final class GLJPanel extends JPanel implements GLDrawable {
 
   public GL getGL() {
     if (!hardwareAccelerationDisabled) {
+      if (pbuffer == null) {
+        return null;
+      }
       return pbuffer.getGL();
     } else {
+      if (offscreenContext == null) {
+        return null;
+      }
       return offscreenContext.getGL();
     }
   }
 
   public void setGL(GL gl) {
     if (!hardwareAccelerationDisabled) {
-      pbuffer.setGL(gl);
+      if (pbuffer != null) {
+        pbuffer.setGL(gl);
+      }
     } else {
-      offscreenContext.setGL(gl);
+      if (offscreenContext != null) {
+        offscreenContext.setGL(gl);
+      }
     }
   }
 
@@ -473,6 +483,9 @@ public final class GLJPanel extends JPanel implements GLDrawable {
 
     public void init(GLDrawable drawable) {
       if (!hardwareAccelerationDisabled) {
+        if (DEBUG) {
+          System.err.println("GLJPanel$Updater.init(): pbufferInitializationCompleted = true");
+        }
         pbufferInitializationCompleted = true;
         EventQueue.invokeLater(new Runnable() {
             public void run() {
diff --git a/src/net/java/games/jogl/impl/GLContext.java b/src/net/java/games/jogl/impl/GLContext.java
index 318ff24a2..e2f591278 100644
--- a/src/net/java/games/jogl/impl/GLContext.java
+++ b/src/net/java/games/jogl/impl/GLContext.java
@@ -484,6 +484,12 @@ public abstract class GLContext {
    * the definition of "available".
    */
   protected void resetGLFunctionAvailability() {
+    // In order to be able to allow the user to uniformly install the
+    // debug and trace pipelines in their GLEventListener.init()
+    // method (for both GLCanvas and GLJPanel), we need to reset the
+    // actual GL object in the GLDrawable as well
+    setGL(createGL());
+
     functionAvailability.flush();
     if (!haveResetGLUProcAddressTable) {
       if (DEBUG) {
diff --git a/src/net/java/games/jogl/impl/windows/WindowsPbufferGLContext.java b/src/net/java/games/jogl/impl/windows/WindowsPbufferGLContext.java
index f0ce74d9d..b0a164f26 100644
--- a/src/net/java/games/jogl/impl/windows/WindowsPbufferGLContext.java
+++ b/src/net/java/games/jogl/impl/windows/WindowsPbufferGLContext.java
@@ -432,7 +432,10 @@ public class WindowsPbufferGLContext extends WindowsGLContext {
     if (hglrc != 0) {
       super.destroyImpl();
       // Must release DC and pbuffer
-      GL gl = getGL();
+      // NOTE that since the context is not current, glGetError() can
+      // not be called here, so we skip the use of any composable
+      // pipelines
+      GL gl = createGL();
       if (gl.wglReleasePbufferDCARB(buffer, hdc) == 0) {
         throw new GLException("Error releasing pbuffer device context: error code " + WGL.GetLastError());
       }