Bug 975 - GLJPanel's OffscreenDrawable shall not double swap (custom swap by GLEventListener using [AWT]GLReadBufferUtil)

When utilizing [AWT]GLReadBufferUtil it is usually desired to read from the front-buffer
instead the back-buffer. The latter may not be defined, e.g. when using MSAA.

A GLEventListener utilizing [AWT]GLReadBufferUtil,
must perform the drawable.swapBuffers() to be able to read from the front-buffer.

Usually GLAutoDrawable.setAutoSwapBuffer(false) should be called here,
to avoid a double swap - however GLJPanel does not support toggling auto-swap
since it requires to control swap for it's own read-pixels.

Remedy for GLJPanel:
  - GLJPanel issues helper.setAutoSwapBufferMode(false) - immutable

  - Enable GLJPanel.swapBuffer() if initializes
    This was previously disabled.

  - GLJPanel's OffscreenBackend listens to  surfaceUpdated,
    to be notified whether postGL needs to swap buffer
    or the drawable.swapBuffer() was already called between preGL and postGL.

See unit tests adding/removing a snapshot GLEventListener
performing swapBuffers() and setting auto-swap accordingly.

1 files changed, 49 insertions, 25 deletions

diff --git a/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java b/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
index 522585f16..66a757d10 100644
--- a/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
+++ b/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
@@ -60,6 +60,7 @@ import java.util.List;
 
 import javax.media.nativewindow.AbstractGraphicsDevice;
 import javax.media.nativewindow.NativeSurface;
+import javax.media.nativewindow.SurfaceUpdatedListener;
 import javax.media.nativewindow.WindowClosingProtocol;
 import javax.media.opengl.GL;
 import javax.media.opengl.GL2;
@@ -222,7 +223,7 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
       return singleAWTGLPixelBufferProvider;
   }
 
-  private final GLDrawableHelper helper = new GLDrawableHelper();
+  private final GLDrawableHelper helper;
   private volatile boolean isInitialized;
 
   //
@@ -352,9 +353,13 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     this.glProfile = offscreenCaps.getGLProfile();
     this.factory = GLDrawableFactoryImpl.getFactoryImpl(glProfile);
     this.chooser = chooser;
+
+    helper = new GLDrawableHelper();
+    helper.setAutoSwapBufferMode(false); /** Always handles buffer swapping in Backend! */
     if( null != shareWith ) {
         helper.setSharedContext(null, shareWith);
     }
+
     this.setFocusable(true); // allow keyboard input!
     this.addHierarchyListener(hierarchyListener);
     this.isShowing = isShowing();
@@ -941,34 +946,25 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
 
   @Override
   public void setAutoSwapBufferMode(boolean onOrOff) {
-    // In the current implementation this is a no-op. Both the pbuffer
-    // and pixmap based rendering paths use a single-buffered surface
-    // so swapping the buffers doesn't do anything. We also don't
-    // currently have the provision to skip copying the data to the
-    // Swing portion of the GLJPanel in any of the rendering paths.
+    // In the current implementation this is a no-op.
+    // All Backend's require control of buffer swapping,
+    // i.e. as required for MSAA offscreen FBO buffer.
   }
 
   @Override
   public boolean getAutoSwapBufferMode() {
-    // In the current implementation this is a no-op. Both the pbuffer
-    // and pixmap based rendering paths use a single-buffered surface
-    // so swapping the buffers doesn't do anything. We also don't
-    // currently have the provision to skip copying the data to the
-    // Swing portion of the GLJPanel in any of the rendering paths.
+    // In the current implementation this is a no-op.
+    // All Backend's require control of buffer swapping,
+    // i.e. as required for MSAA offscreen FBO buffer.
     return true;
   }
 
   @Override
   public void swapBuffers() {
-    // In the current implementation this is a no-op. Both the pbuffer
-    // and pixmap based rendering paths use a single-buffered surface
-    // so swapping the buffers doesn't do anything. We also don't
-    // currently have the provision to skip copying the data to the
-    // Swing portion of the GLJPanel in any of the rendering paths.
-    if( printActive && isInitialized) {
+    if( isInitialized ) {
         final Backend b = backend;
         if ( null != b ) {
-            b.getDrawable().swapBuffers();
+            b.swapBuffers();
         }
     }
   }
@@ -1366,6 +1362,11 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     public boolean preGL(Graphics g);
 
     /**
+     * Shall invoke underlying drawable's swapBuffer.
+     */
+    public void swapBuffers();
+
+    /**
      * Called after the OpenGL work is done in init() and display().
      * The isDisplay argument indicates whether this was called on
      * behalf of a call to display() rather than init().
@@ -1401,6 +1402,8 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     private volatile GLContextImpl offscreenContext; // volatile: avoid locking for read-only access
     private boolean flipVertical;
 
+    private boolean needsSwapBuffers = true;
+
     // For saving/restoring of OpenGL state during ReadPixels
     private final GLPixelStorageModes psm =  new GLPixelStorageModes();
 
@@ -1436,6 +1439,12 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
                                                     chooser,
                                                     panelWidth, panelHeight);
           offscreenDrawable.setRealized(true);
+          offscreenDrawable.getNativeSurface().addSurfaceUpdatedListener(new SurfaceUpdatedListener() {
+              @Override
+              public final void surfaceUpdated(Object updater, NativeSurface ns, long when) {
+                  needsSwapBuffers = false;
+              } } );
+
           offscreenContext = (GLContextImpl) offscreenDrawable.createContext(shareWith[0]);
           offscreenContext.setContextCreationFlags(additionalCtxCreationFlags);
           if( GLContext.CONTEXT_NOT_CURRENT < offscreenContext.makeCurrent() ) {
@@ -1453,8 +1462,6 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
                           ", isGL2ES2 " + gl.isGL2ES2()+"]");
               }
               if( useGLSLFlip ) {
-                  final boolean _autoSwapBufferMode = helper.getAutoSwapBufferMode();
-                  helper.setAutoSwapBufferMode(false);
                   final GLFBODrawable fboDrawable = (GLFBODrawable) offscreenDrawable;
                   fboDrawable.setTextureUnit( GLJPanel.this.requestedTextureUnit );
                   try {
@@ -1475,7 +1482,6 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
                         fboFlipped.destroy(gl);
                         fboFlipped = null;
                       }
-                      helper.setAutoSwapBufferMode(_autoSwapBufferMode);
                   }
               } else {
                   fboFlipped = null;
@@ -1558,11 +1564,19 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
 
     @Override
     public final boolean preGL(Graphics g) {
-      // Empty in this implementation
+      needsSwapBuffers = true;
       return true;
     }
 
     @Override
+    public final void swapBuffers() {
+        final GLDrawable d = offscreenDrawable;
+        if( null != d ) {
+            d.swapBuffers();
+        }
+    }
+
+    @Override
     public final void postGL(Graphics g, boolean isDisplay) {
       if (isDisplay) {
         final GL gl = offscreenContext.getGL();
@@ -1652,7 +1666,9 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
             gl2es3.glReadBuffer(gl2es3.getDefaultReadBuffer());
         }
 
-        offscreenDrawable.swapBuffers();
+        if( needsSwapBuffers ) {
+            offscreenDrawable.swapBuffers();
+        }
 
         if(null != glslTextureRaster) { // implies flippedVertical
             final boolean viewportChange;
@@ -1924,13 +1940,13 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
 
     @Override
     public final GLCapabilitiesImmutable getChosenGLCapabilities() {
-      // FIXME: should do better than this; is it possible to using only platform-independent code?
+      // FIXME: should do better than this; is it possible to query J2D Caps ?
       return new GLCapabilities(null);
     }
 
     @Override
     public final GLProfile getGLProfile() {
-      // FIXME: should do better than this; is it possible to using only platform-independent code?
+      // FIXME: should do better than this; is it possible to query J2D's Profile ?
       return GLProfile.getDefault(GLProfile.getDefaultDevice());
     }
 
@@ -2078,6 +2094,14 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     }
 
     @Override
+    public final void swapBuffers() {
+        final GLDrawable d = joglDrawable;
+        if( null != d ) {
+            d.swapBuffers();
+        }
+    }
+
+    @Override
     public final void postGL(Graphics g, boolean isDisplay) {
       // Cause OpenGL pipeline to flush its results because
       // otherwise it's possible we will buffer up multiple frames'