Fixed Issue 97: Flickering when using ATI_WORKAROUND

The root cause was that JOGL's single-threaded workaround had a bug
where if automatic swap buffer support was disabled, the user's calls
to swapBuffers() were not being retargeted to run on the AWT event
queue thread, as calls to display() were.

The effect of this was that the OpenGL rendering was not necessarily
guaranteed to complete before swapBuffers() was called, unless a
glFlush() / glFinish() pair was inserted. Interestingly, testing
showed that inserting the glFlush() / glFinish() just before the call
to swapBuffers(), which was executing on the user's thread rather than
the AWT event dispatch thread, did not cause the rendering output to
complete properly. This implies either a bug in the implementation of
glFlush() / glFinish() on the testing platform or a misunderstanding
on my part of how these APIs behave. Putting the flush/finish after
all of the OpenGL work done by the user in their display() routine
worked correctly.

Regardless, the intent of the single-threaded workaround was to cause
all OpenGL-related work to be done on the event dispatch thread, which
it now does. This fixes the reported problem.


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

2 files changed, 45 insertions, 14 deletions

diff --git a/src/net/java/games/jogl/GLCanvas.java b/src/net/java/games/jogl/GLCanvas.java
index 1deac7f32..827184dac 100644
--- a/src/net/java/games/jogl/GLCanvas.java
+++ b/src/net/java/games/jogl/GLCanvas.java
@@ -72,7 +72,9 @@ public final class GLCanvas extends Canvas implements GLDrawable {
   }
   
   public void display() {
-    displayImpl();
+    withSingleThreadedWorkaroundDo(displayOnEventDispatchThreadAction,
+                                   displayAction,
+                                   false);
   }
 
   /** Overridden from Canvas; calls {@link #display}. Should not be
@@ -109,12 +111,18 @@ public final class GLCanvas extends Canvas implements GLDrawable {
     final int fy      = 0;
     final int fwidth  = width;
     final int fheight = height;
-    context.invokeGL(new Runnable() {
+    final Runnable reshapeRunnable = new Runnable() {
         public void run() {
           getGL().glViewport(fx, fy, fwidth, fheight);
           drawableHelper.reshape(GLCanvas.this, fx, fy, fwidth, fheight);
         }
-      }, true, initAction);
+      };
+    final Runnable reshapeOnEDTRunnable = new Runnable() {
+        public void run() {
+          context.invokeGL(reshapeRunnable, true, initAction);
+        }
+      };
+    withSingleThreadedWorkaroundDo(reshapeOnEDTRunnable, reshapeRunnable, true);
   }
 
   /** Overridden from Canvas to prevent Java2D's clearing of the
@@ -176,7 +184,7 @@ public final class GLCanvas extends Canvas implements GLDrawable {
   }
 
   public void swapBuffers() {
-    context.invokeGL(swapBuffersAction, false, initAction);
+    withSingleThreadedWorkaroundDo(swapBuffersOnEventDispatchThreadAction, swapBuffersAction, false);
   }
 
   public boolean canCreateOffscreenDrawable() {
@@ -197,15 +205,27 @@ public final class GLCanvas extends Canvas implements GLDrawable {
   // Internals only below this point
   //
 
-  private void displayImpl() {
+  private void withSingleThreadedWorkaroundDo(Runnable eventDispatchThreadAction,
+                                              Runnable invokeGLAction,
+                                              boolean  isReshape) {
     if (SingleThreadedWorkaround.doWorkaround() && !EventQueue.isDispatchThread()) {
       try {
-        EventQueue.invokeAndWait(displayOnEventDispatchThreadAction);
+        // Reshape events must not block on the event queue due to the
+        // possibility of deadlocks during initial component creation.
+        // This solution is not optimal, because it changes the
+        // semantics of reshape() to have some of the processing being
+        // done asynchronously, but at least it preserves the
+        // semantics of the single-threaded workaround.
+        if (!isReshape) {
+          EventQueue.invokeAndWait(eventDispatchThreadAction);
+        } else {
+          EventQueue.invokeLater(eventDispatchThreadAction);
+        }
       } catch (Exception e) {
         throw new GLException(e);
       }
     } else {
-      context.invokeGL(displayAction, false, initAction);
+      context.invokeGL(invokeGLAction, isReshape, initAction);
     }
   }
 
@@ -240,4 +260,11 @@ public final class GLCanvas extends Canvas implements GLDrawable {
   }
   private DisplayOnEventDispatchThreadAction displayOnEventDispatchThreadAction =
     new DisplayOnEventDispatchThreadAction();
+  class SwapBuffersOnEventDispatchThreadAction implements Runnable {
+    public void run() {
+      context.invokeGL(swapBuffersAction, false, initAction);
+    }
+  }
+  private SwapBuffersOnEventDispatchThreadAction swapBuffersOnEventDispatchThreadAction =
+    new SwapBuffersOnEventDispatchThreadAction();
 }
diff --git a/src/net/java/games/jogl/impl/SingleThreadedWorkaround.java b/src/net/java/games/jogl/impl/SingleThreadedWorkaround.java
index 9dbed9e43..2ffd09e6a 100755
--- a/src/net/java/games/jogl/impl/SingleThreadedWorkaround.java
+++ b/src/net/java/games/jogl/impl/SingleThreadedWorkaround.java
@@ -48,7 +48,7 @@ import java.security.PrivilegedAction;
     that is simultaneously being resized by the event queue thread */
 
 public class SingleThreadedWorkaround {
-  private static boolean ATI_WORKAROUND = false;
+  private static boolean singleThreadedWorkaround = false;
   // If the user specified the workaround's system property (either
   // true or false), don't let the automatic detection have any effect
   private static boolean systemPropertySpecified = false;
@@ -57,10 +57,14 @@ public class SingleThreadedWorkaround {
   static {
     AccessController.doPrivileged(new PrivilegedAction() {
         public Object run() {
-          String workaround = System.getProperty("ATI_WORKAROUND");
+          String workaround = System.getProperty("JOGL_SINGLE_THREADED_WORKAROUND");
+          if (workaround == null) {
+            // Old system property (for compatibility)
+            workaround = System.getProperty("ATI_WORKAROUND");
+          }
           if (workaround != null) {
             systemPropertySpecified = true;
-            ATI_WORKAROUND = Boolean.valueOf(workaround).booleanValue();
+            singleThreadedWorkaround = Boolean.valueOf(workaround).booleanValue();
           }
           verbose = (System.getProperty("jogl.verbose") != null);
           printWorkaroundNotice();
@@ -71,18 +75,18 @@ public class SingleThreadedWorkaround {
 
   public static void shouldDoWorkaround() {
     if (!systemPropertySpecified) {
-      ATI_WORKAROUND = true;
+      singleThreadedWorkaround = true;
       printWorkaroundNotice();
     }
   }
 
   public static boolean doWorkaround() {
-    return ATI_WORKAROUND;
+    return singleThreadedWorkaround;
   }
 
   private static void printWorkaroundNotice() {
-    if (ATI_WORKAROUND && verbose) {
-      System.err.println("Using ATI workaround of dispatching display() on event thread");
+    if (singleThreadedWorkaround && verbose) {
+      System.err.println("Using single-threaded workaround of dispatching display() on event thread");
     }
   }
 }