Bug 975 - GLJPanel's OffscreenDrawable shall not double swap - Fix auto-swap mechanism ; Refined API doc getDefaultReadBuffer() ; Add GLDrawableUtil.swapBuffersBeforeRead(..)

Commit 82f679b064784213591b460fc5eaa1f5f196fbd1 which introduces the default swap-buffers
mechanism is erroneous:

The OffscreenBack backend requires the following operation order:

Order-1:
  [1] - GL display
  [2] - GL swapBuffers (always due to single-buffer non-MSAA or MSAA offscreen drawable)
  [3] - readPixels

+++

Commit 82f679b064784213591b460fc5eaa1f5f196fbd1 however introduced:

Order-2:
  [a] - GL display
  [b] - readPixels
  [c] - GL swapBuffers (always due to single-buffer non-MSAA or MSAA offscreen drawable)

since [a] and [b] happened in Updater's display method, and [c] followed the same
triggered by GLAutoDrawableHelper.

+++

The proof, commit d46d9ad8f998a7128d9f023294d5f489673d6d8a, is faulty,
since it always included the 'snapshot' GL event listener
which turned-off auto-swap and swapped before read-pixels.
TL;DR it enforced proper Order-1.

+++

This fix allows the Backend to intercept disable GLDrawableHelper's setAutoSwapBufferMode(..)
and perform the auto-swap mode itself in the proper Order-1.

The unit test has been refined to optionally disable the snapshot
to validate auto-swap mode.

+++

Refined GLBase and GLContext's API doc for 'getDefaultReadBuffer()'

+++

Add GLDrawableUtil.swapBuffersBeforeRead(..)
and reuse it for TileRendererBase (original impl.).

3 files changed, 86 insertions, 10 deletions

diff --git a/src/jogl/classes/javax/media/opengl/GLBase.java b/src/jogl/classes/javax/media/opengl/GLBase.java
index 2d6aed139..3f0c77949 100644
--- a/src/jogl/classes/javax/media/opengl/GLBase.java
+++ b/src/jogl/classes/javax/media/opengl/GLBase.java
@@ -627,14 +627,24 @@ public interface GLBase {
     * Returns the default color buffer within the current bound
     * {@link #getDefaultReadFramebuffer()}, i.e. GL_READ_FRAMEBUFFER,
     * which will be used as the source for pixel reading commands,
-    * like {@link GL#glReadPixels(int, int, int, int, int, int, java.nio.Buffer)} etc.
+    * like {@link GL#glReadPixels(int, int, int, int, int, int, java.nio.Buffer) glReadPixels} etc.
     * <p>
     * For offscreen framebuffer objects this is {@link GL#GL_COLOR_ATTACHMENT0},
     * otherwise this is {@link GL#GL_FRONT} for single buffer configurations
     * and {@link GL#GL_BACK} for double buffer configurations.
     * </p>
+    * <p>
+    * Note-1: Neither ES1 nor ES2 supports selecting the read buffer via glReadBuffer
+    * and {@link GL#GL_BACK} is the default.
+    * </p>
+    * <p>
+    * Note-2: ES3 only supports {@link GL#GL_BACK}, {@link GL#GL_NONE} or {@link GL#GL_COLOR_ATTACHMENT0}+i
+    * </p>
+    * <p>
+    * Note-3: See {@link com.jogamp.opengl.util.GLDrawableUtil#swapBuffersBeforeRead(GLCapabilitiesImmutable) swapBuffersBeforeRead}
+    * for read-pixels and swap-buffers implications.
+    * </p>
     */
    public int getDefaultReadBuffer();
-
 }
 
diff --git a/src/jogl/classes/javax/media/opengl/GLContext.java b/src/jogl/classes/javax/media/opengl/GLContext.java
index 9245d10c2..9a4311772 100644
--- a/src/jogl/classes/javax/media/opengl/GLContext.java
+++ b/src/jogl/classes/javax/media/opengl/GLContext.java
@@ -1277,12 +1277,23 @@ public abstract class GLContext {
    * Returns the default color buffer within the current bound
    * {@link #getDefaultReadFramebuffer()}, i.e. GL_READ_FRAMEBUFFER​,
    * which will be used as the source for pixel reading commands,
-   * like {@link GL#glReadPixels(int, int, int, int, int, int, java.nio.Buffer)} etc.
+   * like {@link GL#glReadPixels(int, int, int, int, int, int, java.nio.Buffer) glReadPixels} etc.
    * <p>
    * For offscreen framebuffer objects this is {@link GL#GL_COLOR_ATTACHMENT0},
    * otherwise this is {@link GL#GL_FRONT} for single buffer configurations
    * and {@link GL#GL_BACK} for double buffer configurations.
    * </p>
+   * <p>
+   * Note-1: Neither ES1 nor ES2 supports selecting the read buffer via glReadBuffer
+   * and {@link GL#GL_BACK} is the default.
+   * </p>
+   * <p>
+   * Note-2: ES3 only supports {@link GL#GL_BACK}, {@link GL#GL_NONE} or {@link GL#GL_COLOR_ATTACHMENT0}+i
+   * </p>
+   * <p>
+   * Note-3: See {@link com.jogamp.opengl.util.GLDrawableUtil#swapBuffersBeforeRead(GLCapabilitiesImmutable) swapBuffersBeforeRead}
+   * for read-pixels and swap-buffers implications.
+   * </p>
    */
   public abstract int getDefaultReadBuffer();
 
diff --git a/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java b/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
index c80b405b5..adc0a0d23 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;
@@ -223,6 +224,8 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
   }
 
   private final GLDrawableHelper helper;
+  private boolean autoSwapBufferMode;
+
   private volatile boolean isInitialized;
 
   //
@@ -357,6 +360,7 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     if( null != shareWith ) {
         helper.setSharedContext(null, shareWith);
     }
+    autoSwapBufferMode = helper.getAutoSwapBufferMode();
 
     this.setFocusable(true); // allow keyboard input!
     this.addHierarchyListener(hierarchyListener);
@@ -944,12 +948,22 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
 
   @Override
   public void setAutoSwapBufferMode(boolean enable) {
-    helper.setAutoSwapBufferMode(enable);
+    this.autoSwapBufferMode = enable;
+    boolean backendHandlesSwapBuffer = false;
+    if( isInitialized ) {
+        final Backend b = backend;
+        if ( null != b ) {
+            backendHandlesSwapBuffer= b.handlesSwapBuffer();
+        }
+    }
+    if( !backendHandlesSwapBuffer ) {
+        helper.setAutoSwapBufferMode(enable);
+    }
   }
 
   @Override
   public boolean getAutoSwapBufferMode() {
-    return helper.getAutoSwapBufferMode();
+    return autoSwapBufferMode;
   }
 
   @Override
@@ -1355,6 +1369,14 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     public boolean preGL(Graphics g);
 
     /**
+     * Return true if backend handles 'swap buffer' itself
+     * and hence the helper's setAutoSwapBuffer(enable) shall not be called.
+     * In this case {@link GLJPanel#autoSwapBufferMode} is being used
+     * in the backend to determine whether to swap buffers or not.
+     */
+    public boolean handlesSwapBuffer();
+
+    /**
      * Shall invoke underlying drawable's swapBuffer.
      */
     public void swapBuffers();
@@ -1394,6 +1416,7 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
 
     private volatile GLContextImpl offscreenContext; // volatile: avoid locking for read-only access
     private boolean flipVertical;
+    private int frameCount = 0;
 
     // For saving/restoring of OpenGL state during ReadPixels
     private final GLPixelStorageModes psm =  new GLPixelStorageModes();
@@ -1417,7 +1440,7 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     @Override
     public final void initialize() {
       if(DEBUG) {
-          System.err.println(getThreadName()+": OffscreenBackend: initialize()");
+          System.err.println(getThreadName()+": OffscreenBackend: initialize() - frameCount "+frameCount);
       }
       try {
           final GLContext[] shareWith = { null };
@@ -1430,10 +1453,20 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
                                                     chooser,
                                                     panelWidth, panelHeight);
           offscreenDrawable.setRealized(true);
+          if( DEBUG ) {
+              offscreenDrawable.getNativeSurface().addSurfaceUpdatedListener(new SurfaceUpdatedListener() {
+                  @Override
+                  public final void surfaceUpdated(Object updater, NativeSurface ns, long when) {
+                      System.err.println(getThreadName()+": OffscreenBackend.swapBuffers - frameCount "+frameCount);
+                  } } );
+          }
+
           offscreenContext = (GLContextImpl) offscreenDrawable.createContext(shareWith[0]);
           offscreenContext.setContextCreationFlags(additionalCtxCreationFlags);
           if( GLContext.CONTEXT_NOT_CURRENT < offscreenContext.makeCurrent() ) {
               isInitialized = true;
+              helper.setAutoSwapBufferMode(false); // we handle swap-buffers, see handlesSwapBuffer()
+
               final GL gl = offscreenContext.getGL();
               flipVertical = !GLJPanel.this.skipGLOrientationVerticalFlip && offscreenDrawable.isGLOriented();
               final GLCapabilitiesImmutable chosenCaps = offscreenDrawable.getChosenGLCapabilities();
@@ -1493,7 +1526,7 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     @Override
     public final void destroy() {
       if(DEBUG) {
-          System.err.println(getThreadName()+": OffscreenBackend: destroy() - offscreenContext: "+(null!=offscreenContext)+" - offscreenDrawable: "+(null!=offscreenDrawable));
+          System.err.println(getThreadName()+": OffscreenBackend: destroy() - offscreenContext: "+(null!=offscreenContext)+" - offscreenDrawable: "+(null!=offscreenDrawable)+" - frameCount "+frameCount);
       }
       if ( null != offscreenContext && offscreenContext.isCreated() ) {
         if( GLContext.CONTEXT_NOT_CURRENT < offscreenContext.makeCurrent() ) {
@@ -1554,6 +1587,11 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     }
 
     @Override
+    public final boolean handlesSwapBuffer() {
+        return true;
+    }
+
+    @Override
     public final void swapBuffers() {
         final GLDrawable d = offscreenDrawable;
         if( null != d ) {
@@ -1564,9 +1602,14 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     @Override
     public final void postGL(Graphics g, boolean isDisplay) {
       if (isDisplay) {
-        // offscreenDrawable is already swapped,
-        // either by GLDrawableHelper.invoke or user's GLEL according to auto-swap-buffer-mode.
-
+        if(DEBUG) {
+            System.err.println(getThreadName()+": GLJPanel.OffscreenBackend.postGL.0: - frameCount "+frameCount);
+        }
+        if( autoSwapBufferMode ) {
+            // Since we only use a single-buffer non-MSAA or double-buffered MSAA offscreenDrawable,
+            // we can always swap!
+            offscreenDrawable.swapBuffers();
+        }
         final GL gl = offscreenContext.getGL();
 
         final int componentCount;
@@ -1645,6 +1688,9 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
         }
 
         // Must now copy pixels from offscreen context into surface
+        if(DEBUG) {
+            System.err.println(getThreadName()+": GLJPanel.OffscreenBackend.postGL.readPixels: - frameCount "+frameCount);
+        }
 
         // Save current modes
         psm.setAlignment(gl, alignment, alignment);
@@ -1731,9 +1777,13 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
       helper.invokeGL(offscreenDrawable, offscreenContext, updaterDisplayAction, updaterInitAction);
 
       if ( null != alignedImage ) {
+        if( DEBUG ) {
+            System.err.println(getThreadName()+": GLJPanel.OffscreenBackend.doPaintComponent.drawImage: - frameCount "+frameCount);
+        }
         // Draw resulting image in one shot
         g.drawImage(alignedImage, 0, 0, alignedImage.getWidth(), alignedImage.getHeight(), null); // Null ImageObserver since image data is ready.
       }
+      frameCount++;
     }
 
     @Override
@@ -2078,6 +2128,11 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     }
 
     @Override
+    public final boolean handlesSwapBuffer() {
+        return false;
+    }
+
+    @Override
     public final void swapBuffers() {
         final GLDrawable d = joglDrawable;
         if( null != d ) {