From 41190c3830157abdf9649cbf7767e57108f55075 Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Tue, 18 Feb 2014 21:47:06 +0100 Subject: 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.). --- .../com/jogamp/opengl/util/GLDrawableUtil.java | 21 +++++++ .../com/jogamp/opengl/util/TileRendererBase.java | 15 +---- src/jogl/classes/javax/media/opengl/GLBase.java | 14 ++++- src/jogl/classes/javax/media/opengl/GLContext.java | 13 +++- .../classes/javax/media/opengl/awt/GLJPanel.java | 69 +++++++++++++++++++--- 5 files changed, 109 insertions(+), 23 deletions(-) (limited to 'src/jogl/classes') diff --git a/src/jogl/classes/com/jogamp/opengl/util/GLDrawableUtil.java b/src/jogl/classes/com/jogamp/opengl/util/GLDrawableUtil.java index ec6d54d66..cf88e7bf6 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/GLDrawableUtil.java +++ b/src/jogl/classes/com/jogamp/opengl/util/GLDrawableUtil.java @@ -30,6 +30,8 @@ package com.jogamp.opengl.util; import javax.media.nativewindow.AbstractGraphicsDevice; import javax.media.opengl.GLAnimatorControl; import javax.media.opengl.GLAutoDrawable; +import javax.media.opengl.GLBase; +import javax.media.opengl.GLCapabilitiesImmutable; import javax.media.opengl.GLContext; import javax.media.opengl.GLDrawable; import javax.media.opengl.GLEventListener; @@ -168,4 +170,23 @@ public class GLDrawableUtil { if(bIsPaused) { bAnim.resume(); } } + /** + * Determines whether the chosen {@link GLCapabilitiesImmutable} + * requires a {@link GLDrawable#swapBuffers() swap-buffers} + * before reading pixels. + *

+ * Usually one uses the {@link GLBase#getDefaultReadBuffer() default-read-buffer} + * in which case {@link GLDrawable#swapBuffers() swap-buffers} shall happen after calling reading pixels, the default. + *

+ *

+ * However, multisampling offscreen {@link javax.media.opengl.GLFBODrawable}s + * utilize {@link GLDrawable#swapBuffers() swap-buffers} to downsample + * the multisamples into the readable sampling sink. + * In this case, we require {@link GLDrawable#swapBuffers() swap-buffers} before reading pixels. + *

+ * @return chosenCaps.isFBO() && chosenCaps.getSampleBuffers() + */ + public static final boolean swapBuffersBeforeRead(final GLCapabilitiesImmutable chosenCaps) { + return chosenCaps.isFBO() && chosenCaps.getSampleBuffers(); + } } diff --git a/src/jogl/classes/com/jogamp/opengl/util/TileRendererBase.java b/src/jogl/classes/com/jogamp/opengl/util/TileRendererBase.java index 22f5c017c..dabde83dd 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/TileRendererBase.java +++ b/src/jogl/classes/com/jogamp/opengl/util/TileRendererBase.java @@ -412,22 +412,11 @@ public abstract class TileRendererBase { * requires a pre-{@link GLDrawable#swapBuffers() swap-buffers} * before accessing the results, i.e. before {@link #endTile(GL)}. *

- * Usually one uses the {@link GL#getDefaultReadBuffer() default-read-buffer}, i.e. - * {@link GL#GL_FRONT} for single-buffer and {@link GL#GL_BACK} for double-buffer {@link GLDrawable}s - * and {@link GL#GL_COLOR_ATTACHMENT0} for offscreen framebuffer objects.
- * Here {@link GLDrawable#swapBuffers() swap-buffers} shall happen after calling {@link #endTile(GL)}, the default. + * See {@link GLDrawableUtil#swapBuffersBeforeRead(GLCapabilitiesImmutable)}. *

- *

- * However, multisampling offscreen {@link GLFBODrawable}s - * utilize {@link GLDrawable#swapBuffers() swap-buffers} to downsample - * the multisamples into the readable sampling sink. - * In this case, we require a {@link GLDrawable#swapBuffers() swap-buffers} before calling {@link #endTile(GL)}. - *

- * @param chosenCaps the chosen {@link GLCapabilitiesImmutable} - * @return chosenCaps.isFBO() && chosenCaps.getSampleBuffers() */ public final boolean reqPreSwapBuffers(GLCapabilitiesImmutable chosenCaps) { - return chosenCaps.isFBO() && chosenCaps.getSampleBuffers(); + return GLDrawableUtil.swapBuffersBeforeRead(chosenCaps); } /** 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. *

* 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. *

+ *

+ * Note-1: Neither ES1 nor ES2 supports selecting the read buffer via glReadBuffer + * and {@link GL#GL_BACK} is the default. + *

+ *

+ * Note-2: ES3 only supports {@link GL#GL_BACK}, {@link GL#GL_NONE} or {@link GL#GL_COLOR_ATTACHMENT0}+i + *

+ *

+ * Note-3: See {@link com.jogamp.opengl.util.GLDrawableUtil#swapBuffersBeforeRead(GLCapabilitiesImmutable) swapBuffersBeforeRead} + * for read-pixels and swap-buffers implications. + *

*/ 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. *

* 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. *

+ *

+ * Note-1: Neither ES1 nor ES2 supports selecting the read buffer via glReadBuffer + * and {@link GL#GL_BACK} is the default. + *

+ *

+ * Note-2: ES3 only supports {@link GL#GL_BACK}, {@link GL#GL_NONE} or {@link GL#GL_COLOR_ATTACHMENT0}+i + *

+ *

+ * Note-3: See {@link com.jogamp.opengl.util.GLDrawableUtil#swapBuffersBeforeRead(GLCapabilitiesImmutable) swapBuffersBeforeRead} + * for read-pixels and swap-buffers implications. + *

*/ 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 @@ -1354,6 +1368,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. */ @@ -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() ) { @@ -1553,6 +1586,11 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing return true; } + @Override + public final boolean handlesSwapBuffer() { + return true; + } + @Override public final void swapBuffers() { final GLDrawable d = offscreenDrawable; @@ -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 @@ -2077,6 +2127,11 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing return true; } + @Override + public final boolean handlesSwapBuffer() { + return false; + } + @Override public final void swapBuffers() { final GLDrawable d = joglDrawable; -- cgit v1.2.3