From 20bf031db719f7baa4c6e74734fc999061e08fe2 Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Thu, 19 Jul 2012 21:15:10 +0200 Subject: Bug 599 - FBObject / Offscreen Support - Part 1 - New FBObject implementation handling FBO and it's attachments *** API CHANGE: Util -> Core *** while it's size and sample-count can be reconfigured on the fly. - com.jogamp.opengl.util.FBObject -> com.jogamp.opengl.FBObject - agnostic to texture unit - separate attachments using OO hierarchy reflecting FBO - handling MSAA and blitting - no FBO destruction for reconfig (attach/detach) - New GLFBODrawableImpl impl. an FBObject based GLDrawable - Instantiated by a dummy native surface (onscreen and invisible) hooked up to a dummy GLDrawable, which is the delegation for context creation. - Utilizies ProxySurface.UpstreamSurfaceHook for dummy surface avoiding specialization for native platforms. - TODO: Allow to utilize common surface interface as a dummy-surface to supporting API seperation of windowing/GL. The latter allows impl. of createGLDrawable(NativeSurface) with FBO. - New OffscreenAutoDrawable (extends GLAutoDrawableDelegate) for all offscreen drawables. Shall replace GLPbuffer. - New GLCapabilities*.isFBO() / setFBO(boolean) to request FBO offscreen, similar to isPBuffer(). Rule: if both are requested, FBO shall be favored. - GLContext adds raw FBO availability query (min. FBO avail), FBObject contains fine grained queries (TODO: Move parts to GLContext for efficiency). - Add framebuffer tracking, allowing fast querying: - GLBase/GLContext: public int getBoundFramebuffer(int target); public int getDefaultDrawFramebuffer(); public int getDefaultReadFramebuffer(); - GLContextImpl public final void setBoundFramebuffer(int target, int framebufferName) .. called by GL impl bind framebuffer - GL: getDefaultDrawFramebuffer(), getDefaultReadFramebuffer() Adding default framebuffer queries being issued by GL.glBindFramebuffer(target, 0) w/ a default framebuffer, o.e. zero. This allows a transparent use of a custom FBO even in case the applications attempts to reset FBO to zero. Value flow: GL <- GLContext <- GLDrawable, - GLCapabilities handle fbo/pbuffer seperate, don't disable the other - GLContext/GL track read/write framebuffer to be queried by FBObject to determine whether to bind/unbind a framebuffer - Test cases for multiple FBO w/ and w/o MSAA Other Features: - New interface ProxySurface.UpstreamSurfaceHook, allowing to hook an upstream surface of unknown type providing lifecycle and information (size, ..) callbacks. Used for all new dummy NativeSurface impl and SWT GLCanvas. - GLContext -> GLDrawable propagation context/drawable lifecycle via ProxySurface.UpstreamSurfaceHook allowing dynamic resources to react (create, init, ..) - contextRealized() - contextMadeCurrent() - SurfaceChangeable -> MutableSurface currently only contains setting the surface handle. TODO: May need to move ProxySurface.UpstreamSurfaceHook -> MutableSurface.UpstreamSurfaceHook, allowing other impl. classes (NEWT OffscreenWindow) to utilize the new upstream hookup mechanism - will allow FBO/Dummy window to work. - SWT GLCanvas using ProxySurface.UpstreamSurfaceHook for proper size propagation. - New GLAutoDrawable::getUpstreamWidget(), allowing GLEventListener to fetch the owning Java side UI element (NEWT, SWT, AWT, ..). - GLDrawableFactory: Removed createOffscreenSurface() - unused and not GL related - EGLDrawableFactory handles device/profile avail. mapping while actually creating context/drawable. This allows us to learn whether the ES context is software/hardware as well as FBO avail. - EGLDrawable: Removed secret buckets of EGL configs :) Employ native surface (X11, WGL, ..) to EGL 'mapping' in EGLDrawableFactory utilizing new EGLUpstreamSurfaceHook (implements ProxySurface.UpstreamSurfaceHook). Other Bugs: - Add CTX_OPTION_DEBUG to ctx/extension cache key since only a debug ctx may expose the ARB debug capability. This bug caused lack of ARB/AMD debug functionality. - Fix GLProfile deadlock (debug mode, w/ EGL/ES, no X11), dump availability information _after_ lock. - ImmModeSink draw(): Use GL's glDrawElements(..), don't cast for GL2ES1. Fixes use for GL2ES2. - Fix KeyEvent.getKeyChar() comment (-> only stable for keyTyped(..)) Misc: - Refined alot of API doc - New GLExtensions holds commonly used GL extension strings, allows better referencing and usage lookup. - Move GL (interface) decl. to GLBase - GLBuffers: Cleanup API doc (format, types) - TextureIO: Add PAM and PPM static suffix identifier - GLCapabilities getNumSamples() returns 0 if sampleBuffers is disabled, this seems to be more natural. - finalized a lot --- src/jogl/classes/jogamp/opengl/GLPbufferImpl.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src/jogl/classes/jogamp/opengl/GLPbufferImpl.java') diff --git a/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java b/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java index a8277fd71..bbc28e283 100644 --- a/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java +++ b/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java @@ -54,8 +54,7 @@ import javax.media.opengl.GLPbuffer; public class GLPbufferImpl extends GLAutoDrawableBase implements GLPbuffer { private int floatMode; - public GLPbufferImpl(GLDrawableImpl pbufferDrawable, - GLContext sharedContext) { + public GLPbufferImpl(GLDrawableImpl pbufferDrawable, GLContext sharedContext) { super(pbufferDrawable, null); // drawable := pbufferDrawable GLCapabilitiesImmutable caps = (GLCapabilitiesImmutable) @@ -111,6 +110,11 @@ public class GLPbufferImpl extends GLAutoDrawableBase implements GLPbuffer { // GLAutoDrawable completion // + @Override + public final Object getUpstreamWidget() { + return null; + } + @Override public void destroy() { defaultDestroyOp(); -- cgit v1.2.3 From 4b5a0f6557d7152ec770bc13ad3c494449de0529 Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Sun, 22 Jul 2012 04:16:55 +0200 Subject: Fix Bug 606 - New AWT threading implementation breaks .. ; Fix GLAutoDrawable multi-threading w/ proper pattern (hope so) Considering code changes and remarks: 3ed491213f8f7f05d7b9866b50d764370d8ff5f6 1a91ec5c8b6fd9d9db7bc115569c369fe7b38e9b 3334a924309a9361a448d69bc707d4cce416b430 4f27bcecf7484dc041551f52a5c49e2884cb3867 It seems necessary to have - recursive locking employed for all semantic actions which changes drawable & context (and the Window resource) - to avoid deadlock, we have to ensure the locked code segment will not spawn off to another thread, or a thread holds the lock, spawns of an action requiring the lock. .. sure - other read-only methods (flags, ..) shall at least utilize a safe local copy of a volatile field if further use to produce the result is necessary. - flags like sendReshape require to be volatile to guarantee it's being processed Patch impacts: AWT/SWT GLCanvas, GLAutoDrawableBase [and it's specializations] and hopefully closes any loopholes of missing a cache hit, etc. If you review this and find optimizations, i.e. removing a lock due to semantics etc, don't hold back and discuss it, please. --- .../classes/com/jogamp/opengl/swt/GLCanvas.java | 172 ++++++----- .../javax/media/opengl/GLAutoDrawableDelegate.java | 44 ++- .../classes/javax/media/opengl/awt/GLCanvas.java | 317 +++++++++++---------- .../classes/jogamp/opengl/GLAutoDrawableBase.java | 249 +++++++++++----- .../classes/jogamp/opengl/GLDrawableHelper.java | 17 +- src/jogl/classes/jogamp/opengl/GLPbufferImpl.java | 29 +- .../classes/com/jogamp/newt/opengl/GLWindow.java | 36 +-- src/newt/classes/jogamp/newt/WindowImpl.java | 99 +++---- .../jogl/acore/TestGLAutoDrawableDelegateNEWT.java | 10 +- .../acore/TestGLContextDrawableSwitchNEWT.java | 10 +- .../jogl/acore/TestGLContextSurfaceLockNEWT.java | 92 ++++-- 11 files changed, 649 insertions(+), 426 deletions(-) (limited to 'src/jogl/classes/jogamp/opengl/GLPbufferImpl.java') diff --git a/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java b/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java index 0d9d3ddf5..64ee1c1ad 100644 --- a/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java +++ b/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java @@ -63,25 +63,13 @@ import org.eclipse.swt.widgets.Shell; import com.jogamp.common.GlueGenVersion; import com.jogamp.common.os.Platform; import com.jogamp.common.util.VersionUtil; +import com.jogamp.common.util.locks.LockFactory; +import com.jogamp.common.util.locks.RecursiveLock; import com.jogamp.nativewindow.swt.SWTAccessor; import com.jogamp.opengl.JoglVersion; /** * Native SWT Canvas implementing GLAutoDrawable - *

- * FIXME: If this instance runs in multithreading mode, see {@link Threading#isSingleThreaded()} (impossible), - * proper recursive locking is required for drawable/context @ destroy and display. - * Recreation etc could pull those instances while animating! - * Simply locking before using drawable/context offthread - * would allow a deadlock situation! - *

- *

- * NOTE: [MT-0] Methods utilizing [volatile] drawable/context are not synchronized. - In case any of the methods are called outside of a locked state - extra care should be added. Maybe we shall expose locking facilities to the user. - However, since the user shall stick to the GLEventListener model while utilizing - GLAutoDrawable implementations, she is safe due to the implicit locked state. - *

*/ public class GLCanvas extends Canvas implements GLAutoDrawable { @@ -97,8 +85,9 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { //private static final boolean useSWTThread = ThreadingImpl.getMode() != ThreadingImpl.WORKER; /* GL Stuff */ + private final RecursiveLock lock = LockFactory.createRecursiveLock(); private final GLDrawableHelper helper = new GLDrawableHelper(); - private volatile GLDrawable drawable; // volatile avoids locking all accessors. FIXME still need to sync destroy/display + private volatile GLDrawable drawable; // volatile: avoid locking for read-only access private GLContext context; /* Native window surface */ @@ -112,7 +101,7 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { private final GLCapabilitiesImmutable glCapsRequested; /* Flag indicating whether an unprocessed reshape is pending. */ - private volatile boolean sendReshape; + private volatile boolean sendReshape; // volatile: maybe written by WindowManager thread w/o locking /* * Invokes init(...) on all GLEventListeners. Assumes context is current when run. @@ -141,10 +130,16 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { }; /* Action to make specified context current prior to running displayAction */ - private final Runnable makeCurrentAndDisplayAction = new Runnable() { + private final Runnable makeCurrentAndDisplayOnEDTAction = new Runnable() { @Override public void run() { - helper.invokeGL(drawable, context, displayAction, initAction); + final RecursiveLock _lock = lock; + _lock.lock(); + try { + helper.invokeGL(drawable, context, displayAction, initAction); + } finally { + _lock.unlock(); + } } }; @@ -157,10 +152,16 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { }; /* Swaps buffers, making the GLContext current first */ - private final Runnable makeCurrentAndSwapBuffersAction = new Runnable() { + private final Runnable makeCurrentAndSwapBuffersOnEDTAction = new Runnable() { @Override public void run() { - helper.invokeGL(drawable, context, swapBuffersAction, initAction); + final RecursiveLock _lock = lock; + _lock.lock(); + try { + helper.invokeGL(drawable, context, swapBuffersAction, initAction); + } finally { + _lock.unlock(); + } } }; @@ -181,16 +182,33 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { private final Runnable disposeOnEDTGLAction = new Runnable() { @Override public void run() { - helper.disposeGL(GLCanvas.this, drawable, context, postDisposeGLAction); - } - }; - - private final Runnable disposeGraphicsDeviceAction = new Runnable() { - @Override - public void run() { - if (null != device) { - device.close(); - device = null; + final RecursiveLock _lock = lock; + _lock.lock(); + try { + if (null != drawable && null != context) { + boolean animatorPaused = false; + final GLAnimatorControl animator = getAnimator(); + if (null != animator) { + animatorPaused = animator.pause(); + } + + if(context.isCreated()) { + helper.disposeGL(GLCanvas.this, drawable, context, postDisposeGLAction); + } + + if (animatorPaused) { + animator.resume(); + } + } + // SWT is owner of the device handle, not us. + // Hence close() operation is a NOP. + if (null != device) { + device.close(); + device = null; + } + SWTAccessor.setRealized(GLCanvas.this, false); // unrealize .. + } finally { + _lock.unlock(); } } }; @@ -229,7 +247,8 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { clientArea = GLCanvas.this.getClientArea(); - /* Get the nativewindow-Graphics Device associated with this control (which is determined by the parent Composite) */ + /* Get the nativewindow-Graphics Device associated with this control (which is determined by the parent Composite). + * Note: SWT is owner of the native handle, hence no closing operation will be a NOP. */ device = SWTAccessor.getDevice(this); /* Since we have no means of querying the screen index yet, assume 0. Good choice due to Xinerama alike settings anyways. */ final int screenIdx = 0; @@ -345,7 +364,7 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { @Override public void display() { - runInGLThread(makeCurrentAndDisplayAction); + runInGLThread(makeCurrentAndDisplayOnEDTAction); } @Override @@ -370,7 +389,8 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { @Override public GL getGL() { - return (null == context) ? null : context.getGL(); + final GLContext _context = context; + return (null == _context) ? null : _context.getGL(); } @Override @@ -400,27 +420,35 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { @Override public GLContext setContext(GLContext newCtx) { - final GLContext oldCtx = context; - final boolean newCtxCurrent = helper.switchContext(drawable, oldCtx, newCtx, additionalCtxCreationFlags); - context=(GLContextImpl)newCtx; - if(newCtxCurrent) { - context.makeCurrent(); + final RecursiveLock _lock = lock; + _lock.lock(); + try { + final GLContext oldCtx = context; + final boolean newCtxCurrent = helper.switchContext(drawable, oldCtx, newCtx, additionalCtxCreationFlags); + context=(GLContextImpl)newCtx; + if(newCtxCurrent) { + context.makeCurrent(); + } + return oldCtx; + } finally { + _lock.unlock(); } - return oldCtx; } @Override public void setContextCreationFlags(final int arg0) { additionalCtxCreationFlags = arg0; - if(null != context) { - context.setContextCreationFlags(additionalCtxCreationFlags); + final GLContext _context = context; + if(null != _context) { + _context.setContextCreationFlags(additionalCtxCreationFlags); } } @Override public GL setGL(final GL arg0) { - if (null != context) { - context.setGL(arg0); + final GLContext _context = context; + if (null != _context) { + _context.setGL(arg0); return arg0; } return null; @@ -428,12 +456,18 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { @Override public GLContext createContext(final GLContext shareWith) { - if(drawable != null) { - final GLContext _ctx = drawable.createContext(shareWith); - _ctx.setContextCreationFlags(additionalCtxCreationFlags); - return _ctx; + final RecursiveLock _lock = lock; + _lock.lock(); + try { + if(drawable != null) { + final GLContext _ctx = drawable.createContext(shareWith); + _ctx.setContextCreationFlags(additionalCtxCreationFlags); + return _ctx; + } + return null; + } finally { + _lock.unlock(); } - return null; } @Override @@ -452,7 +486,8 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { @Override public GLDrawableFactory getFactory() { - return (drawable != null) ? drawable.getFactory() : null; + final GLDrawable _drawable = drawable; + return (_drawable != null) ? _drawable.getFactory() : null; } @Override @@ -462,17 +497,20 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { @Override public long getHandle() { - return (drawable != null) ? drawable.getHandle() : 0; + final GLDrawable _drawable = drawable; + return (_drawable != null) ? _drawable.getHandle() : 0; } @Override public NativeSurface getNativeSurface() { - return (drawable != null) ? drawable.getNativeSurface() : null; + final GLDrawable _drawable = drawable; + return (_drawable != null) ? _drawable.getNativeSurface() : null; } @Override public boolean isRealized() { - return (drawable != null) ? drawable.isRealized() : false; + final GLDrawable _drawable = drawable; + return (_drawable != null) ? _drawable.isRealized() : false; } @Override @@ -482,41 +520,19 @@ public class GLCanvas extends Canvas implements GLAutoDrawable { @Override public void swapBuffers() throws GLException { - runInGLThread(makeCurrentAndSwapBuffersAction); + runInGLThread(makeCurrentAndSwapBuffersOnEDTAction); } // FIXME: API of update() method ? @Override public void update() { - // FIXME: display(); + // FIXME: display(); ? } @Override public void dispose() { - if (null != drawable && null != context) { // drawable is volatile! - boolean animatorPaused = false; - final GLAnimatorControl animator = getAnimator(); - if (null != animator) { - // can't remove us from animator for recreational addNotify() - animatorPaused = animator.pause(); - } - - if(context.isCreated()) { - runInGLThread(disposeOnEDTGLAction); - } - - if (animatorPaused) { - animator.resume(); - } - } - final Display display = getDisplay(); - - if (display.getThread() == Thread.currentThread()) { - disposeGraphicsDeviceAction.run(); - } else { - display.syncExec(disposeGraphicsDeviceAction); - } - super.dispose(); + runInGLThread(disposeOnEDTGLAction); + super.dispose(); } /** diff --git a/src/jogl/classes/javax/media/opengl/GLAutoDrawableDelegate.java b/src/jogl/classes/javax/media/opengl/GLAutoDrawableDelegate.java index 1f6166719..76959f3f4 100644 --- a/src/jogl/classes/javax/media/opengl/GLAutoDrawableDelegate.java +++ b/src/jogl/classes/javax/media/opengl/GLAutoDrawableDelegate.java @@ -67,18 +67,18 @@ public class GLAutoDrawableDelegate extends GLAutoDrawableBase { } // - // make protected methods accessible + // expose default methods // - public void defaultWindowRepaintOp() { + public final void windowRepaintOp() { super.defaultWindowRepaintOp(); } - public void defaultWindowResizedOp() { + public final void windowResizedOp() { super.defaultWindowResizedOp(); } - public void defaultWindowDestroyNotifyOp() { + public final void windowDestroyNotifyOp() { super.defaultWindowDestroyNotifyOp(); } @@ -89,6 +89,9 @@ public class GLAutoDrawableDelegate extends GLAutoDrawableBase { private final RecursiveLock lock = LockFactory.createRecursiveLock(); // instance wide lock private final Object upstreamWidget; + @Override + protected final RecursiveLock getLock() { return lock; } + @Override public final Object getUpstreamWidget() { return upstreamWidget; @@ -97,39 +100,21 @@ public class GLAutoDrawableDelegate extends GLAutoDrawableBase { /** * {@inheritDoc} *

- * This implementation calls {@link #defaultDestroyOp()}. + * This implementation calls {@link #defaultDestroy()}. *

*

* User still needs to destroy the upstream window, which details are hidden from this aspect. + * This can be performed by overriding {@link #destroyImplInLock()}. *

*/ @Override - public void destroy() { - lock.lock(); - try { - defaultDestroyOp(); - } finally { - lock.unlock(); - } + public final void destroy() { + defaultDestroy(); } @Override public void display() { - if( sendDestroy ) { - sendDestroy=false; - destroy(); - return; - } - - lock.lock(); // sync: context/drawable could been recreated/destroyed while animating - try { - if( null != drawable && drawable.isRealized() && null != context ) { - // surface is locked/unlocked implicit by context's makeCurrent/release - helper.invokeGL(drawable, context, defaultDisplayAction, defaultInitAction); - } - } finally { - lock.unlock(); - } + defaultDisplay(); } // @@ -145,4 +130,9 @@ public class GLAutoDrawableDelegate extends GLAutoDrawableBase { public final void setRealized(boolean realized) { } + @Override + public final void swapBuffers() throws GLException { + defaultSwapBuffers(); + } + } diff --git a/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java b/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java index c2e36ef9b..694a081b8 100644 --- a/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java +++ b/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java @@ -84,6 +84,8 @@ import javax.media.opengl.Threading; import com.jogamp.common.GlueGenVersion; import com.jogamp.common.util.VersionUtil; +import com.jogamp.common.util.locks.LockFactory; +import com.jogamp.common.util.locks.RecursiveLock; import com.jogamp.nativewindow.awt.AWTGraphicsConfiguration; import com.jogamp.nativewindow.awt.AWTGraphicsDevice; import com.jogamp.nativewindow.awt.AWTGraphicsScreen; @@ -132,20 +134,6 @@ import jogamp.opengl.GLDrawableHelper; * - *

- * FIXME: If this instance runs in multithreading mode, see {@link Threading#isSingleThreaded()} (default: single-thread), - * proper recursive locking is required for drawable/context @ destroy and display. - * Recreation etc could pull those instances while animating! - * Simply locking before using drawable/context offthread - * would allow a deadlock situation! - *

- *

- * NOTE: [MT-0] Methods utilizing [volatile] drawable/context are not synchronized. - In case any of the methods are called outside of a locked state - extra care should be added. Maybe we shall expose locking facilities to the user. - However, since the user shall stick to the GLEventListener model while utilizing - GLAutoDrawable implementations, she is safe due to the implicit locked state. - *

*/ @SuppressWarnings("serial") @@ -153,11 +141,12 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing private static final boolean DEBUG = Debug.debug("GLCanvas"); + private final RecursiveLock lock = LockFactory.createRecursiveLock(); private final GLDrawableHelper helper = new GLDrawableHelper(); private AWTGraphicsConfiguration awtConfig; - private volatile GLDrawable drawable; // volatile avoids locking all accessors. FIXME still need to sync destroy/display + private volatile GLDrawable drawable; // volatile: avoid locking for read-only access private GLContextImpl context; - private boolean sendReshape = false; + private volatile boolean sendReshape = false; // volatile: maybe written by EDT w/o locking // copy of the cstr args, mainly for recreation private GLCapabilitiesImmutable capsReqUser; @@ -278,8 +267,9 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @Override public final boolean isOffscreenLayerSurfaceEnabled() { - if(null != drawable) { - return ((JAWTWindow)drawable.getNativeSurface()).isOffscreenLayerSurfaceEnabled(); + final GLDrawable _drawable = drawable; + if(null != _drawable) { + return ((JAWTWindow)_drawable.getNativeSurface()).isOffscreenLayerSurfaceEnabled(); } return false; } @@ -398,12 +388,18 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @Override public GLContext createContext(final GLContext shareWith) { - if(drawable != null) { - final GLContext _ctx = drawable.createContext(shareWith); - _ctx.setContextCreationFlags(additionalCtxCreationFlags); - return _ctx; + final RecursiveLock _lock = lock; + _lock.lock(); + try { + if(drawable != null) { + final GLContext _ctx = drawable.createContext(shareWith); + _ctx.setContextCreationFlags(additionalCtxCreationFlags); + return _ctx; + } + return null; + } finally { + _lock.unlock(); } - return null; } @Override @@ -412,7 +408,8 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @Override public boolean isRealized() { - return (null != drawable) ? drawable.isRealized() : false; + final GLDrawable _drawable = drawable; + return ( null != _drawable ) ? _drawable.isRealized() : false; } @Override @@ -427,50 +424,13 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @Override public void display() { - if( !validateGLDrawable() ) { - if(DEBUG) { - System.err.println(getThreadName()+": Info: GLCanvas display - skipped GL render, drawable not valid yet"); - } - return; // not yet available .. - } Threading.invoke(true, displayOnEDTAction, getTreeLock()); - awtWindowClosingProtocol.addClosingListenerOneShot(); } private void dispose(boolean regenerate) { - final GLAnimatorControl animator = getAnimator(); - if(DEBUG) { - System.err.println(getThreadName()+": Info: dispose("+regenerate+") - START, hasContext " + - (null!=context) + ", hasDrawable " + (null!=drawable)+", "+animator); - Thread.dumpStack(); - } - - if(null!=drawable && null!=context) { // drawable is volatile! - boolean animatorPaused = false; - if(null!=animator) { - // can't remove us from animator for recreational addNotify() - animatorPaused = animator.pause(); - } - - disposeRegenerate=regenerate; - - if(context.isCreated()) { - Threading.invoke(true, disposeOnEDTAction, getTreeLock()); - } - - if(animatorPaused) { - animator.resume(); - } - } - - if(!regenerate) { - disposeAbstractGraphicsDevice(); - } - - if(DEBUG) { - System.err.println(getThreadName()+": dispose("+regenerate+") - END, "+animator); - } + disposeRegenerate=regenerate; + Threading.invoke(true, disposeOnEDTAction, getTreeLock()); } /** @@ -530,43 +490,49 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @SuppressWarnings("deprecation") @Override public void addNotify() { - if(DEBUG) { - System.err.println(getThreadName()+": Info: addNotify - start, bounds: "+this.getBounds()); - Thread.dumpStack(); - } - - /** - * 'super.addNotify()' determines the GraphicsConfiguration, - * while calling this class's overriden 'getGraphicsConfiguration()' method - * after which it creates the native peer. - * Hence we have to set the 'awtConfig' before since it's GraphicsConfiguration - * is being used in getGraphicsConfiguration(). - * This code order also allows recreation, ie re-adding the GLCanvas. - */ - awtConfig = chooseGraphicsConfiguration(capsReqUser, capsReqUser, chooser, device); - if(null==awtConfig) { - throw new GLException("Error: NULL AWTGraphicsConfiguration"); - } - - // before native peer is valid: X11 - disableBackgroundErase(); - - // issues getGraphicsConfiguration() and creates the native peer - super.addNotify(); - - // after native peer is valid: Windows - disableBackgroundErase(); - - if (!Beans.isDesignTime()) { - createDrawableAndContext(); - } - - // init drawable by paint/display makes the init sequence more equal - // for all launch flavors (applet/javaws/..) - // validateGLDrawable(); - - if(DEBUG) { - System.err.println(getThreadName()+": Info: addNotify - end: peer: "+getPeer()); + final RecursiveLock _lock = lock; + _lock.lock(); + try { + if(DEBUG) { + System.err.println(getThreadName()+": Info: addNotify - start, bounds: "+this.getBounds()); + Thread.dumpStack(); + } + + /** + * 'super.addNotify()' determines the GraphicsConfiguration, + * while calling this class's overriden 'getGraphicsConfiguration()' method + * after which it creates the native peer. + * Hence we have to set the 'awtConfig' before since it's GraphicsConfiguration + * is being used in getGraphicsConfiguration(). + * This code order also allows recreation, ie re-adding the GLCanvas. + */ + awtConfig = chooseGraphicsConfiguration(capsReqUser, capsReqUser, chooser, device); + if(null==awtConfig) { + throw new GLException("Error: NULL AWTGraphicsConfiguration"); + } + + // before native peer is valid: X11 + disableBackgroundErase(); + + // issues getGraphicsConfiguration() and creates the native peer + super.addNotify(); + + // after native peer is valid: Windows + disableBackgroundErase(); + + if (!Beans.isDesignTime()) { + createDrawableAndContext(); + } + + // init drawable by paint/display makes the init sequence more equal + // for all launch flavors (applet/javaws/..) + // validateGLDrawable(); + + if(DEBUG) { + System.err.println(getThreadName()+": Info: addNotify - end: peer: "+getPeer()); + } + } finally { + _lock.unlock(); } } @@ -585,23 +551,24 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing } private boolean validateGLDrawable() { - boolean realized = false; - if (!Beans.isDesignTime()) { - if ( null != drawable ) { // OK: drawable is volatile - realized = drawable.isRealized(); - if ( !realized && 0 < drawable.getWidth() * drawable.getHeight() ) { - // make sure drawable realization happens on AWT EDT, due to AWTTree lock - AWTEDTExecutor.singleton.invoke(true, setRealizedOnEDTAction); - realized = true; - sendReshape=true; // ensure a reshape is being send .. - if(DEBUG) { - System.err.println(getThreadName()+": Realized Drawable: "+drawable.toString()); - Thread.dumpStack(); - } + final GLDrawable _drawable = drawable; + if ( null != _drawable ) { + if( _drawable.isRealized() ) { + return true; + } + if (!Beans.isDesignTime() && + 0 < _drawable.getWidth() * _drawable.getHeight() ) { + // make sure drawable realization happens on AWT EDT, due to AWTTree lock + AWTEDTExecutor.singleton.invoke(true, setRealizedOnEDTAction); + sendReshape=true; // ensure a reshape is being send .. + if(DEBUG) { + System.err.println(getThreadName()+": Realized Drawable: "+_drawable.toString()); + Thread.dumpStack(); } + return true; } } - return realized; + return false; } private Runnable setRealizedOnEDTAction = new Runnable() { @Override @@ -633,9 +600,6 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing try { dispose(false); } finally { - context=null; - drawable=null; - awtConfig=null; super.removeNotify(); } } @@ -655,7 +619,8 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @Override public void reshape(int x, int y, int width, int height) { super.reshape(x, y, width, height); - if(null != drawable && drawable.isRealized() && !drawable.getChosenGLCapabilities().isOnscreen()) { + final GLDrawable _drawable = drawable; + if(null != _drawable && _drawable.isRealized() && !_drawable.getChosenGLCapabilities().isOnscreen()) { dispose(true); } else { sendReshape = true; @@ -710,13 +675,19 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @Override public GLContext setContext(GLContext newCtx) { - final GLContext oldCtx = context; - final boolean newCtxCurrent = helper.switchContext(drawable, oldCtx, newCtx, additionalCtxCreationFlags); - context=(GLContextImpl)newCtx; - if(newCtxCurrent) { - context.makeCurrent(); + final RecursiveLock _lock = lock; + _lock.lock(); + try { + final GLContext oldCtx = context; + final boolean newCtxCurrent = helper.switchContext(drawable, oldCtx, newCtx, additionalCtxCreationFlags); + context=(GLContextImpl)newCtx; + if(newCtxCurrent) { + context.makeCurrent(); + } + return oldCtx; + } finally { + _lock.unlock(); } - return oldCtx; } @Override @@ -729,15 +700,15 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing if (Beans.isDesignTime()) { return null; } - GLContext ctx = getContext(); - return (ctx == null) ? null : ctx.getGL(); + final GLContext _context = context; + return (_context == null) ? null : _context.getGL(); } @Override public GL setGL(GL gl) { - GLContext ctx = getContext(); - if (ctx != null) { - ctx.setGL(gl); + final GLContext _context = context; + if (_context != null) { + _context.setGL(gl); return gl; } return null; @@ -762,8 +733,9 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @Override public void setContextCreationFlags(int flags) { additionalCtxCreationFlags = flags; - if(null != context) { - context.setContextCreationFlags(additionalCtxCreationFlags); + final GLContext _context = context; + if(null != _context) { + _context.setContextCreationFlags(additionalCtxCreationFlags); } } @@ -796,26 +768,30 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing @Override public NativeSurface getNativeSurface() { - return (null != drawable) ? drawable.getNativeSurface() : null; + final GLDrawable _drawable = drawable; + return (null != _drawable) ? _drawable.getNativeSurface() : null; } @Override public long getHandle() { - return (null != drawable) ? drawable.getHandle() : 0; + final GLDrawable _drawable = drawable; + return (null != _drawable) ? _drawable.getHandle() : 0; } @Override public GLDrawableFactory getFactory() { - return (null != drawable) ? drawable.getFactory() : null; + final GLDrawable _drawable = drawable; + return (null != _drawable) ? _drawable.getFactory() : null; } @Override public String toString() { - final int dw = (null!=drawable) ? drawable.getWidth() : -1; - final int dh = (null!=drawable) ? drawable.getHeight() : -1; + final GLDrawable _drawable = drawable; + final int dw = (null!=_drawable) ? _drawable.getWidth() : -1; + final int dh = (null!=_drawable) ? _drawable.getHeight() : -1; return "AWT-GLCanvas[Realized "+isRealized()+ - ",\n\t"+((null!=drawable)?drawable.getClass().getName():"null-drawable")+ + ",\n\t"+((null!=_drawable)?_drawable.getClass().getName():"null-drawable")+ ",\n\tFactory "+getFactory()+ ",\n\thandle 0x"+Long.toHexString(getHandle())+ ",\n\tDrawable size "+dw+"x"+dh+ @@ -829,7 +805,7 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing // private boolean disposeRegenerate; - private final Runnable postDisposeAction = new Runnable() { + private final Runnable postDisposeOnEDTAction = new Runnable() { @Override public void run() { context=null; @@ -859,7 +835,47 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing private final Runnable disposeOnEDTAction = new Runnable() { @Override public void run() { - helper.disposeGL(GLCanvas.this, drawable, context, postDisposeAction); + final RecursiveLock _lock = lock; + _lock.lock(); + try { + final GLAnimatorControl animator = getAnimator(); + + if(DEBUG) { + System.err.println(getThreadName()+": Info: dispose("+disposeRegenerate+") - START, hasContext " + + (null!=context) + ", hasDrawable " + (null!=drawable)+", "+animator); + Thread.dumpStack(); + } + + if(null!=drawable && null!=context) { + boolean animatorPaused = false; + if(null!=animator) { + // can't remove us from animator for recreational addNotify() + animatorPaused = animator.pause(); + } + + if(context.isCreated()) { + helper.disposeGL(GLCanvas.this, drawable, context, postDisposeOnEDTAction); + } + + if(animatorPaused) { + animator.resume(); + } + } + + if(!disposeRegenerate) { + if(null != awtConfig) { + disposeAbstractGraphicsDevice(); + } + awtConfig=null; + } + + if(DEBUG) { + System.err.println(getThreadName()+": dispose("+disposeRegenerate+") - END, "+animator); + } + + } finally { + _lock.unlock(); + } } }; @@ -879,7 +895,6 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing if(DEBUG) { System.err.println(getThreadName()+": GLCanvas.dispose(false): closed GraphicsDevice: "+adeviceMsg+", result: "+closed); } - awtConfig=null; } } }; @@ -890,7 +905,7 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing * * @see #chooseGraphicsConfiguration(javax.media.opengl.GLCapabilitiesImmutable, javax.media.opengl.GLCapabilitiesImmutable, javax.media.opengl.GLCapabilitiesChooser, java.awt.GraphicsDevice) */ - void disposeAbstractGraphicsDevice() { + private void disposeAbstractGraphicsDevice() { if( EventQueue.isDispatchThread() || Thread.holdsLock(getTreeLock()) ) { disposeAbstractGraphicsDeviceAction.run(); } else { @@ -941,14 +956,30 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing private final Runnable displayOnEDTAction = new Runnable() { @Override public void run() { - helper.invokeGL(drawable, context, displayAction, initAction); + final RecursiveLock _lock = lock; + _lock.lock(); + try { + if( validateGLDrawable() ) { + helper.invokeGL(drawable, context, displayAction, initAction); + } else if(DEBUG) { + System.err.println(getThreadName()+": Info: GLCanvas display - skipped GL render, drawable not valid yet"); + } + } finally { + _lock.unlock(); + } } }; private final Runnable swapBuffersOnEDTAction = new Runnable() { @Override public void run() { - helper.invokeGL(drawable, context, swapBuffersAction, initAction); + final RecursiveLock _lock = lock; + _lock.lock(); + try { + helper.invokeGL(drawable, context, swapBuffersAction, initAction); + } finally { + _lock.unlock(); + } } }; diff --git a/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java b/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java index 5c6d7446a..fe6d0fd76 100644 --- a/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java +++ b/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java @@ -46,6 +46,7 @@ import javax.media.opengl.GLException; import javax.media.opengl.GLProfile; import javax.media.opengl.GLRunnable; +import com.jogamp.common.util.locks.RecursiveLock; import com.jogamp.opengl.util.Animator; @@ -63,11 +64,11 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { protected final GLDrawableHelper helper = new GLDrawableHelper(); protected final FPSCounterImpl fpsCounter = new FPSCounterImpl(); - protected GLDrawableImpl drawable; + protected volatile GLDrawableImpl drawable; // volatile: avoid locking for read-only access protected GLContextImpl context; protected int additionalCtxCreationFlags = 0; - protected boolean sendReshape = false; - protected boolean sendDestroy = false; + protected volatile boolean sendReshape = false; // volatile: maybe written by WindowManager thread w/o locking + protected volatile boolean sendDestroy = false; // volatile: maybe written by WindowManager thread w/o locking public GLAutoDrawableBase(GLDrawableImpl drawable, GLContextImpl context) { this.drawable = drawable; @@ -75,31 +76,47 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { resetFPSCounter(); } + protected abstract RecursiveLock getLock(); + /** Returns the delegated GLDrawable */ public final GLDrawable getDelegatedDrawable() { return drawable; } /** Default implementation to handle repaint events from the windowing system */ - protected void defaultWindowRepaintOp() { - if( null != drawable && drawable.isRealized() ) { - if( !drawable.getNativeSurface().isSurfaceLockedByOtherThread() && !helper.isAnimatorAnimating() ) { + protected final void defaultWindowRepaintOp() { + final GLDrawable _drawable = drawable; + if( null != _drawable && _drawable.isRealized() ) { + if( !_drawable.getNativeSurface().isSurfaceLockedByOtherThread() && !helper.isAnimatorAnimating() ) { display(); } - } + } } /** Default implementation to handle resize events from the windowing system */ - protected void defaultWindowResizedOp() { - if( null!=drawable ) { + protected final void defaultWindowResizedOp() { + final GLDrawable _drawable = drawable; + if( null!=_drawable ) { if(DEBUG) { System.err.println("GLAutoDrawableBase.sizeChanged: ("+Thread.currentThread().getName()+"): "+getWidth()+"x"+getHeight()+" - surfaceHandle 0x"+Long.toHexString(getNativeSurface().getSurfaceHandle())); } - sendReshape = true; - defaultWindowRepaintOp(); + sendReshape = true; // async if display() doesn't get called below, but avoiding deadlock + if( _drawable.isRealized() ) { + if( !_drawable.getNativeSurface().isSurfaceLockedByOtherThread() && !helper.isAnimatorAnimating() ) { + display(); + } + } } } - /** Default implementation to handle destroy notifications from the windowing system */ - protected void defaultWindowDestroyNotifyOp() { + /** + * Default implementation to handle destroy notifications from the windowing system. + * + *

+ * If the {@link NativeSurface} does not implement {@link WindowClosingProtocol} + * or {@link WindowClosingMode#DISPOSE_ON_CLOSE} is enabled (default), + * {@link #defaultDestroy()} is being called. + *

+ */ + protected final void defaultWindowDestroyNotifyOp() { final NativeSurface ns = getNativeSurface(); final boolean shallClose; if(ns instanceof WindowClosingProtocol) { @@ -108,27 +125,66 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { shallClose = true; } if( shallClose ) { - // Is an animator thread perform rendering? - if (helper.isExternalAnimatorRunning()) { - // Pause animations before initiating safe destroy. - final GLAnimatorControl ctrl = helper.getAnimator(); - final boolean isPaused = ctrl.pause(); - destroy(); - if(isPaused) { - ctrl.resume(); - } - } else if (null != ns && ns.isSurfaceLockedByOtherThread()) { - // surface is locked by another thread - // Flag that destroy should be performed on the next - // attempt to display. - sendDestroy = true; - } else { - // Without an external thread animating or locking the - // surface, we are safe. - destroy (); - } + destroyAvoidAwareOfLocking(); } } + + /** + * Calls {@link #destroy()} + * directly if the following requirements are met: + * + *

+ * Otherwise destroy is being flagged to be called within the next + * call of display(). + *

+ *

+ * This method is being used to avoid deadlock if + * destruction is desired by other threads, e.g. the window manager. + *

+ * @see #defaultWindowDestroyNotifyOp() + * @see #defaultDisplay() + */ + protected final void destroyAvoidAwareOfLocking() { + final NativeSurface ns = getNativeSurface(); + + final GLAnimatorControl ctrl = helper.getAnimator(); + + // Is an animator thread perform rendering? + if ( helper.isAnimatorRunningOnOtherThread() ) { + // Pause animations before initiating safe destroy. + final boolean isPaused = ctrl.pause(); + destroy(); + if(isPaused) { + ctrl.resume(); + } + } else if (null != ns && ns.isSurfaceLockedByOtherThread()) { + // surface is locked by another thread + // Flag that destroy should be performed on the next + // attempt to display. + sendDestroy = true; // async, but avoiding deadlock + } else { + // Without an external thread animating or locking the + // surface, we are safe. + destroy(); + } + } + + /** + * Calls {@link #destroyImplInLock()} while claiming the lock. + */ + protected final void defaultDestroy() { + final RecursiveLock lock = getLock(); + lock.lock(); + try { + destroyImplInLock(); + } finally { + lock.unlock(); + } + } /** * Default implementation to destroys the drawable and context of this GLAutoDrawable: @@ -137,24 +193,42 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { *
  • destroys the GLContext, if valid
    • *
  • destroys the GLDrawable, if valid
    • * + *

    Method assumes the lock is being hold.

    + *

    Override it to extend it to destroy your resources, i.e. the actual window. + * In such case call super.destroyImplInLock first.

    */ - protected void defaultDestroyOp() { - if( null != drawable && drawable.isRealized() ) { - if( null != context && context.isCreated() ) { + protected void destroyImplInLock() { + final GLContext _context = context; + final GLDrawable _drawable = drawable; + if( null != _drawable && _drawable.isRealized() ) { + if( null != _context && _context.isCreated() ) { // Catch dispose GLExceptions by GLEventListener, just 'print' them // so we can continue with the destruction. try { - helper.disposeGL(this, drawable, context, null); + helper.disposeGL(this, _drawable, _context, null); } catch (GLException gle) { gle.printStackTrace(); } } - drawable.setRealized(false); + _drawable.setRealized(false); } context = null; drawable = null; } + public final void defaultSwapBuffers() throws GLException { + final RecursiveLock _lock = getLock(); + _lock.lock(); + try { + if(drawable!=null && context != null) { + drawable.swapBuffers(); + helper.invokeGL(drawable, context, defaultSwapAction, defaultInitAction); + } + } finally { + _lock.unlock(); + } + } + // // GLAutoDrawable // @@ -179,6 +253,30 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { fpsCounter.tickFPS(); } }; + protected final void defaultDisplay() { + if( sendDestroy ) { + sendDestroy=false; + destroy(); + return; + } + final RecursiveLock _lock = getLock(); + _lock.lock(); + try { + if( null != context ) { + // surface is locked/unlocked implicit by context's makeCurrent/release + helper.invokeGL(drawable, context, defaultDisplayAction, defaultInitAction); + } + } finally { + _lock.unlock(); + } + } + + protected final Runnable defaultSwapAction = new Runnable() { + @Override + public final void run() { + drawable.swapBuffers(); + } } ; + @Override public final GLContext getContext() { return context; @@ -186,27 +284,35 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { @Override public final GLContext setContext(GLContext newCtx) { - final GLContext oldCtx = context; - final boolean newCtxCurrent = helper.switchContext(drawable, oldCtx, newCtx, additionalCtxCreationFlags); - context=(GLContextImpl)newCtx; - if(newCtxCurrent) { - context.makeCurrent(); + final RecursiveLock lock = getLock(); + lock.lock(); + try { + final GLContext oldCtx = context; + final boolean newCtxCurrent = helper.switchContext(drawable, oldCtx, newCtx, additionalCtxCreationFlags); + context=(GLContextImpl)newCtx; + if(newCtxCurrent) { + context.makeCurrent(); + } + return oldCtx; + } finally { + lock.unlock(); } - return oldCtx; } @Override public final GL getGL() { - if (context == null) { + final GLContext _context = context; + if (_context == null) { return null; } - return context.getGL(); + return _context.getGL(); } @Override public final GL setGL(GL gl) { - if (context != null) { - context.setGL(gl); + final GLContext _context = context; + if (_context != null) { + _context.setGL(gl); return gl; } return null; @@ -261,8 +367,9 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { @Override public final void setContextCreationFlags(int flags) { additionalCtxCreationFlags = flags; - if(null != context) { - context.setContextCreationFlags(additionalCtxCreationFlags); + final GLContext _context = context; + if(null != _context) { + _context.setContextCreationFlags(additionalCtxCreationFlags); } } @@ -331,27 +438,36 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { @Override public final GLContext createContext(final GLContext shareWith) { - if(drawable != null) { - final GLContext _ctx = drawable.createContext(shareWith); - _ctx.setContextCreationFlags(additionalCtxCreationFlags); - return _ctx; + final RecursiveLock lock = getLock(); + lock.lock(); + try { + if(drawable != null) { + final GLContext _ctx = drawable.createContext(shareWith); + _ctx.setContextCreationFlags(additionalCtxCreationFlags); + return _ctx; + } + return null; + } finally { + lock.unlock(); } - return null; } @Override public final boolean isRealized() { - return null != drawable ? drawable.isRealized() : false; + final GLDrawable _drawable = drawable; + return null != _drawable ? _drawable.isRealized() : false; } @Override public int getWidth() { - return null != drawable ? drawable.getWidth() : 0; + final GLDrawable _drawable = drawable; + return null != _drawable ? _drawable.getWidth() : 0; } @Override public int getHeight() { - return null != drawable ? drawable.getHeight() : 0; + final GLDrawable _drawable = drawable; + return null != _drawable ? _drawable.getHeight() : 0; } /** @@ -373,30 +489,27 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { return helper.getSkipContextReleaseThread(); } - @Override - public final void swapBuffers() throws GLException { - if(drawable!=null && context != null) { - drawable.swapBuffers(); - } - } - @Override public final GLCapabilitiesImmutable getChosenGLCapabilities() { - return null != drawable ? drawable.getChosenGLCapabilities() : null; + final GLDrawable _drawable = drawable; + return null != _drawable ? _drawable.getChosenGLCapabilities() : null; } @Override public final GLProfile getGLProfile() { - return null != drawable ? drawable.getGLProfile() : null; + final GLDrawable _drawable = drawable; + return null != _drawable ? _drawable.getGLProfile() : null; } @Override public final NativeSurface getNativeSurface() { - return null != drawable ? drawable.getNativeSurface() : null; + final GLDrawable _drawable = drawable; + return null != _drawable ? _drawable.getNativeSurface() : null; } @Override public final long getHandle() { - return null != drawable ? drawable.getHandle() : 0; + final GLDrawable _drawable = drawable; + return null != _drawable ? _drawable.getHandle() : 0; } } diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java index 0c01aa676..090c5fe69 100644 --- a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java +++ b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java @@ -189,8 +189,9 @@ public class GLDrawableHelper { */ public final void dispose(GLAutoDrawable drawable) { synchronized(listenersLock) { - for (int i=0; i < listeners.size(); i++) { - listeners.get(i).dispose(drawable); + final ArrayList _listeners = listeners; + for (int i=0; i < _listeners.size(); i++) { + _listeners.get(i).dispose(drawable); } } } @@ -209,8 +210,9 @@ public class GLDrawableHelper { /** The default init action to be called once after ctx is being created @ 1st makeCurrent(). */ public final void init(GLAutoDrawable drawable) { synchronized(listenersLock) { - for (int i=0; i < listeners.size(); i++) { - final GLEventListener listener = listeners.get(i) ; + final ArrayList _listeners = listeners; + for (int i=0; i < _listeners.size(); i++) { + final GLEventListener listener = _listeners.get(i) ; // If make current ctx, invoked by invokGL(..), results in a new ctx, init gets called. // This may happen not just for initial setup, but for ctx recreation due to resource change (drawable/window), @@ -232,8 +234,9 @@ public class GLDrawableHelper { } private final void displayImpl(GLAutoDrawable drawable) { synchronized(listenersLock) { - for (int i=0; i < listeners.size(); i++) { - final GLEventListener listener = listeners.get(i) ; + final ArrayList _listeners = listeners; + for (int i=0; i < _listeners.size(); i++) { + final GLEventListener listener = _listeners.get(i) ; // GLEventListener may need to be init, // in case this one is added after the realization of the GLAutoDrawable init( listener, drawable, true /* sendReshape */) ; @@ -324,7 +327,7 @@ public class GLDrawableHelper { } } - public final boolean isExternalAnimatorRunning() { + public final boolean isAnimatorRunningOnOtherThread() { return ( null != animatorCtrl ) ? animatorCtrl.isStarted() && animatorCtrl.getThread() != Thread.currentThread() : false ; } diff --git a/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java b/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java index bbc28e283..6b64120fe 100644 --- a/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java +++ b/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java @@ -42,10 +42,14 @@ package jogamp.opengl; import javax.media.opengl.GLCapabilitiesImmutable; import javax.media.opengl.GLContext; +import javax.media.opengl.GLDrawable; import javax.media.opengl.GLDrawableFactory; import javax.media.opengl.GLException; import javax.media.opengl.GLPbuffer; +import com.jogamp.common.util.locks.LockFactory; +import com.jogamp.common.util.locks.RecursiveLock; + /** Platform-independent class exposing pbuffer functionality to applications. This class is not exposed in the public API as it would probably add no value; however it implements the GLDrawable @@ -101,7 +105,7 @@ public class GLPbufferImpl extends GLAutoDrawableBase implements GLPbuffer { // // GLDrawable delegation // - + @Override public final void setRealized(boolean realized) { } @@ -109,6 +113,10 @@ public class GLPbufferImpl extends GLAutoDrawableBase implements GLPbuffer { // // GLAutoDrawable completion // + private final RecursiveLock lock = LockFactory.createRecursiveLock(); // instance wide lock + + @Override + protected final RecursiveLock getLock() { return lock; } @Override public final Object getUpstreamWidget() { @@ -117,7 +125,7 @@ public class GLPbufferImpl extends GLAutoDrawableBase implements GLPbuffer { @Override public void destroy() { - defaultDestroyOp(); + defaultDestroy(); } @Override @@ -126,12 +134,23 @@ public class GLPbufferImpl extends GLAutoDrawableBase implements GLPbuffer { } @Override - public void display() { - if( null != drawable && drawable.isRealized() && null != context ) { - helper.invokeGL(drawable, context, defaultDisplayAction, initAction); + public final void display() { + final RecursiveLock _lock = lock; + _lock.lock(); // sync: context/drawable could been recreated/destroyed while animating + try { + if( null != context ) { + helper.invokeGL(drawable, context, defaultDisplayAction, initAction); + } + } finally { + _lock.unlock(); } } + @Override + public final void swapBuffers() throws GLException { + defaultSwapBuffers(); + } + //---------------------------------------------------------------------- // Internals only below this point // diff --git a/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java b/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java index 32d44502f..2205aec8e 100644 --- a/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java +++ b/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java @@ -53,6 +53,7 @@ import javax.media.opengl.GLCapabilitiesImmutable; import javax.media.opengl.GLContext; import javax.media.opengl.GLDrawableFactory; import javax.media.opengl.GLEventListener; +import javax.media.opengl.GLException; import javax.media.opengl.GLProfile; import jogamp.newt.WindowImpl; @@ -62,6 +63,7 @@ import jogamp.opengl.GLDrawableImpl; import com.jogamp.common.GlueGenVersion; import com.jogamp.common.util.VersionUtil; +import com.jogamp.common.util.locks.RecursiveLock; import com.jogamp.newt.NewtFactory; import com.jogamp.newt.Screen; import com.jogamp.newt.Window; @@ -87,13 +89,6 @@ import com.jogamp.opengl.JoglVersion; * you can inject {@link javax.media.opengl.GLRunnable} objects * via {@link #invoke(boolean, javax.media.opengl.GLRunnable)} to the OpenGL command stream.
    *

    - *

    - * NOTE: [MT-0] Methods utilizing [volatile] drawable/context are not synchronized. - In case any of the methods are called outside of a locked state - extra care should be added. Maybe we shall expose locking facilities to the user. - However, since the user shall stick to the GLEventListener model while utilizing - GLAutoDrawable implementations, she is safe due to the implicit locked state. - *

    */ public class GLWindow extends GLAutoDrawableBase implements GLAutoDrawable, Window, NEWTEventConsumer, FPSCounter { private final WindowImpl window; @@ -437,7 +432,7 @@ public class GLWindow extends GLAutoDrawableBase implements GLAutoDrawable, Wind //e1.printStackTrace(); } - defaultDestroyOp(); + destroyImplInLock(); if(Window.DEBUG_IMPLEMENTATION) { System.err.println("GLWindow.destroy() "+WindowImpl.getThreadName()+", fin"); @@ -515,6 +510,11 @@ public class GLWindow extends GLAutoDrawableBase implements GLAutoDrawable, Wind private GLContext sharedContext = null; + @Override + protected final RecursiveLock getLock() { + return window.getLock(); + } + /** * Specifies an {@link javax.media.opengl.GLContext OpenGL context} to share with.
    * At native creation, {@link #setVisible(boolean) setVisible(true)}, @@ -537,19 +537,18 @@ public class GLWindow extends GLAutoDrawableBase implements GLAutoDrawable, Wind return; } - window.lockWindow(); // sync: context/drawable could have been recreated/destroyed while animating + final RecursiveLock lock = window.getLock(); + lock.lock(); // sync: context/drawable could have been recreated/destroyed while animating try { - if( null == context && 0 display + setVisible(true); } } finally { - window.unlockWindow(); + lock.unlock(); } } @@ -559,7 +558,7 @@ public class GLWindow extends GLAutoDrawableBase implements GLAutoDrawable, Wind private GLDrawableFactory factory; @Override - public GLDrawableFactory getFactory() { + public final GLDrawableFactory getFactory() { return factory; } @@ -567,6 +566,11 @@ public class GLWindow extends GLAutoDrawableBase implements GLAutoDrawable, Wind public final void setRealized(boolean realized) { } + @Override + public final void swapBuffers() throws GLException { + defaultSwapBuffers(); + } + //---------------------------------------------------------------------- // NEWTEventConsumer // diff --git a/src/newt/classes/jogamp/newt/WindowImpl.java b/src/newt/classes/jogamp/newt/WindowImpl.java index 002144b2f..b12e42680 100644 --- a/src/newt/classes/jogamp/newt/WindowImpl.java +++ b/src/newt/classes/jogamp/newt/WindowImpl.java @@ -564,9 +564,11 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer @Override public final int lockSurface() throws NativeWindowException, RuntimeException { - windowLock.lock(); - surfaceLock.lock(); - int res = surfaceLock.getHoldCount() == 1 ? LOCK_SURFACE_NOT_READY : LOCK_SUCCESS; // new lock ? + final RecursiveLock _wlock = windowLock; + final RecursiveLock _slock = surfaceLock; + _wlock.lock(); + _slock.lock(); + int res = _slock.getHoldCount() == 1 ? LOCK_SURFACE_NOT_READY : LOCK_SUCCESS; // new lock ? if ( LOCK_SURFACE_NOT_READY == res ) { try { @@ -583,8 +585,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } } finally { if (LOCK_SURFACE_NOT_READY >= res) { - surfaceLock.unlock(); - windowLock.unlock(); + _slock.unlock(); + _wlock.unlock(); } } } @@ -593,10 +595,12 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer @Override public final void unlockSurface() { - surfaceLock.validateLocked(); - windowLock.validateLocked(); + final RecursiveLock _slock = surfaceLock; + final RecursiveLock _wlock = windowLock; + _slock.validateLocked(); + _wlock.validateLocked(); - if (surfaceLock.getHoldCount() == 1) { + if (_slock.getHoldCount() == 1) { final AbstractGraphicsDevice adevice = getGraphicsConfiguration().getScreen().getDevice(); try { unlockSurfaceImpl(); @@ -604,8 +608,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer adevice.unlock(); } } - surfaceLock.unlock(); - windowLock.unlock(); + _slock.unlock(); + _wlock.unlock(); } @Override @@ -618,21 +622,10 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer return surfaceLock.getOwner(); } - public final void lockWindow() { - windowLock.lock(); - } - public final void unlockWindow() { - windowLock.unlock(); + public final RecursiveLock getLock() { + return windowLock; } - public final boolean isWindowLockedByOtherThread() { - return windowLock.isLockedByOtherThread(); - } - - public final Thread getWindowLockOwner() { - return windowLock.getOwner(); - } - public long getSurfaceHandle() { return windowHandle; // default: return window handle } @@ -670,11 +663,12 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer public Point getLocationOnScreen(Point storage) { if(isNativeValid()) { Point d; - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { d = getLocationOnScreenImpl(0, 0); } finally { - windowLock.unlock(); + _lock.unlock(); } if(null!=d) { if(null!=storage) { @@ -717,7 +711,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer boolean nativeWindowCreated = false; boolean madeVisible = false; - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { if(null!=lifecycleHook) { lifecycleHook.resetCounter(); @@ -739,7 +734,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer madeVisible = nativeWindowCreated; } // always flag visible, allowing a retry .. - WindowImpl.this.visible = true; + WindowImpl.this.visible = true; } else if(WindowImpl.this.visible != visible) { if(isNativeValid()) { setVisibleImpl(visible, getX(), getY(), getWidth(), getHeight()); @@ -766,7 +761,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer System.err.println("Window setVisible: END ("+getThreadName()+") "+getX()+"/"+getY()+" "+getWidth()+"x"+getHeight()+", fs "+fullscreen+", windowHandle "+toHexString(windowHandle)+", visible: "+WindowImpl.this.visible+", nativeWindowCreated: "+nativeWindowCreated+", madeVisible: "+madeVisible); } } finally { - windowLock.unlock(); + _lock.unlock(); } if( nativeWindowCreated || madeVisible ) { sendWindowEvent(WindowEvent.EVENT_WINDOW_RESIZED); // trigger a resize/relayout and repaint to listener @@ -801,7 +796,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer public final void run() { boolean recreate = false; - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { if ( !isFullscreen() && ( getWidth() != width || getHeight() != height ) ) { recreate = isNativeValid() && !getGraphicsConfiguration().getChosenCapabilities().isOnscreen(); @@ -842,7 +838,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer if(recreate) { screen.removeReference(); // bring back ref-count } - windowLock.unlock(); + _lock.unlock(); } } } @@ -863,7 +859,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer if(null!=lifecycleHook) { lifecycleHook.destroyActionPreLock(); } - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { if(DEBUG_IMPLEMENTATION) { System.err.println("Window DestroyAction() "+getThreadName()); @@ -917,7 +914,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer hasFocus = false; parentWindowHandle = 0; - windowLock.unlock(); + _lock.unlock(); } if(animatorPaused) { lifecycleHook.resumeRenderingAction(); @@ -1002,8 +999,9 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer int width = getWidth(); int height = getHeight(); boolean wasVisible; - - windowLock.lock(); + + final RecursiveLock _lock = windowLock; + _lock.lock(); try { if(isNativeValid()) { // force recreation if offscreen, since it may become onscreen @@ -1220,7 +1218,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer System.err.println("Window.reparentWindow: END-1 ("+getThreadName()+") windowHandle "+toHexString(windowHandle)+", visible: "+visible+", parentWindowHandle "+toHexString(parentWindowHandle)+", parentWindow "+ Display.hashCodeNullSafe(parentWindow)+" "+x+"/"+y+" "+width+"x"+height); } } finally { - windowLock.unlock(); + _lock.unlock(); } if(wasVisible) { switch (operation) { @@ -1245,7 +1243,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer private class ReparentActionRecreate implements Runnable { public final void run() { - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { visible = true; if(DEBUG_IMPLEMENTATION) { @@ -1253,7 +1252,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } setVisible(true); // native creation } finally { - windowLock.unlock(); + _lock.unlock(); } } } @@ -1291,7 +1290,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } public final void run() { - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { if(WindowImpl.this.undecorated != undecorated) { // set current state @@ -1311,7 +1311,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } } } finally { - windowLock.unlock(); + _lock.unlock(); } sendWindowEvent(WindowEvent.EVENT_WINDOW_RESIZED); // trigger a resize/relayout and repaint to listener } @@ -1333,7 +1333,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } public final void run() { - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { if(WindowImpl.this.alwaysOnTop != alwaysOnTop) { // set current state @@ -1353,7 +1354,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } } } finally { - windowLock.unlock(); + _lock.unlock(); } sendWindowEvent(WindowEvent.EVENT_WINDOW_RESIZED); // trigger a resize/relayout and repaint to listener } @@ -1545,7 +1546,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer "\n, ParentWindow "+parentWindow+ "\n, ParentWindowHandle "+toHexString(parentWindowHandle)+" ("+(0!=getParentWindowHandle())+")"+ "\n, WindowHandle "+toHexString(getWindowHandle())+ - "\n, SurfaceHandle "+toHexString(getSurfaceHandle())+ " (lockedExt window "+isWindowLockedByOtherThread()+", surface "+isSurfaceLockedByOtherThread()+")"+ + "\n, SurfaceHandle "+toHexString(getSurfaceHandle())+ " (lockedExt window "+windowLock.isLockedByOtherThread()+", surface "+isSurfaceLockedByOtherThread()+")"+ "\n, Pos "+getX()+"/"+getY()+" (auto "+autoPosition()+"), size "+getWidth()+"x"+getHeight()+ "\n, Visible "+isVisible()+", focus "+hasFocus()+ "\n, Undecorated "+undecorated+" ("+isUndecorated()+")"+ @@ -1675,7 +1676,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } public final void run() { - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { if(DEBUG_IMPLEMENTATION) { System.err.println("Window setPosition: "+getX()+"/"+getY()+" -> "+x+"/"+y+", fs "+fullscreen+", windowHandle "+toHexString(windowHandle)); @@ -1689,7 +1691,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } } } finally { - windowLock.unlock(); + _lock.unlock(); } } } @@ -1718,7 +1720,8 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer public boolean fsOn() { return fullscreen; } public final void run() { - windowLock.lock(); + final RecursiveLock _lock = windowLock; + _lock.lock(); try { // set current state WindowImpl.this.fullscreen = fullscreen; @@ -1795,7 +1798,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer } } } finally { - windowLock.unlock(); + _lock.unlock(); } sendWindowEvent(WindowEvent.EVENT_WINDOW_RESIZED); // trigger a resize/relayout and repaint to listener } @@ -1917,7 +1920,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer // special repaint treatment case WindowEvent.EVENT_WINDOW_REPAINT: // queue repaint event in case window is locked, ie in operation - if( null != getWindowLockOwner() ) { + if( null != windowLock.getOwner() ) { // make sure only one repaint event is queued if(!repaintQueued) { repaintQueued=true; @@ -1936,7 +1939,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer // common treatment case WindowEvent.EVENT_WINDOW_RESIZED: // queue event in case window is locked, ie in operation - if( null != getWindowLockOwner() ) { + if( null != windowLock.getOwner() ) { final boolean discardTO = QUEUED_EVENT_TO <= System.currentTimeMillis()-e.getWhen(); if(DEBUG_IMPLEMENTATION) { System.err.println("Window.consumeEvent: "+Thread.currentThread().getName()+" - queued "+e+", discard-to "+discardTO); diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLAutoDrawableDelegateNEWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLAutoDrawableDelegateNEWT.java index eb716677d..426b7734f 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLAutoDrawableDelegateNEWT.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLAutoDrawableDelegateNEWT.java @@ -87,8 +87,8 @@ public class TestGLAutoDrawableDelegateNEWT extends UITestCase { final GLAutoDrawableDelegate glad = new GLAutoDrawableDelegate(drawable, context, window) { @Override - public void destroy() { - super.destroy(); // destroys drawable/context + protected void destroyImplInLock() { + super.destroyImplInLock(); // destroys drawable/context window.destroy(); // destroys the actual window } }; @@ -97,15 +97,15 @@ public class TestGLAutoDrawableDelegateNEWT extends UITestCase { window.addWindowListener(new WindowAdapter() { @Override public void windowRepaint(WindowUpdateEvent e) { - glad.defaultWindowRepaintOp(); + glad.windowRepaintOp(); } @Override public void windowResized(WindowEvent e) { - glad.defaultWindowResizedOp(); + glad.windowResizedOp(); } @Override public void windowDestroyNotify(WindowEvent e) { - glad.defaultWindowDestroyNotifyOp(); + glad.windowDestroyNotifyOp(); } }); window.addWindowListener(wl); diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextDrawableSwitchNEWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextDrawableSwitchNEWT.java index 06aa29b4f..92b4c5238 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextDrawableSwitchNEWT.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextDrawableSwitchNEWT.java @@ -90,8 +90,8 @@ public class TestGLContextDrawableSwitchNEWT extends UITestCase { final GLAutoDrawableDelegate glad = new GLAutoDrawableDelegate(drawable, null, window) { @Override - public void destroy() { - super.destroy(); // destroys drawable/context + protected void destroyImplInLock() { + super.destroyImplInLock(); window.destroy(); // destroys the actual window } }; @@ -100,15 +100,15 @@ public class TestGLContextDrawableSwitchNEWT extends UITestCase { window.addWindowListener(new WindowAdapter() { @Override public void windowRepaint(WindowUpdateEvent e) { - glad.defaultWindowRepaintOp(); + glad.windowRepaintOp(); } @Override public void windowResized(WindowEvent e) { - glad.defaultWindowResizedOp(); + glad.windowResizedOp(); } @Override public void windowDestroyNotify(WindowEvent e) { - glad.defaultWindowDestroyNotifyOp(); + glad.windowDestroyNotifyOp(); } }); window.addWindowListener(wl); diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextSurfaceLockNEWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextSurfaceLockNEWT.java index 78988c0e5..b3516d6b4 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextSurfaceLockNEWT.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextSurfaceLockNEWT.java @@ -33,9 +33,9 @@ import java.io.IOException; import javax.media.nativewindow.NativeSurface; import javax.media.opengl.GLAutoDrawable; import javax.media.opengl.GLCapabilities; +import javax.media.opengl.GLEventListener; import javax.media.opengl.GLProfile; -import org.junit.Assert; import org.junit.Test; import com.jogamp.newt.opengl.GLWindow; @@ -69,7 +69,7 @@ public class TestGLContextSurfaceLockNEWT extends UITestCase { } public void run() { - System.err.println("Animatr "+id+": PRE: "+Thread.currentThread().getName()); + System.err.println("Animatr "+id+", count "+frameCount+": PRE: "+Thread.currentThread().getName()); for(int c=0; c Date: Fri, 3 Aug 2012 01:38:37 +0300 Subject: Fix X11 Display Connection leak w/ new GLAutoDrawableBase code when used w/ offscreen drawables, reported by Mark Raynsford New common GLAutoDrawableBase missed to close the AbstractGraphicsDevice in case it has been created and dedicated for the passed GLDrawable. This detailed knowledge is only known to the creator, hence it is passed in the constructor and is being passed through all specializations. Further more the new X11/GLX impl. of GLDrawableFactory's 'createMutableSurfaceImpl' always creates it's own private X11 display connection to avoid locking / threading issues. Since the old implementation reused the shared display connection which is prone to threading issues, this bug was not visible before. Also fixed the unit test TestNEWTCloseX11DisplayBug565, now correctly validating that no display connection is left over after a new cycle of create/destroy of onscreen and offscreen drawables. --- make/scripts/tests.sh | 6 ++-- .../com/jogamp/opengl/OffscreenAutoDrawable.java | 13 ++++++-- .../javax/media/opengl/GLAutoDrawableDelegate.java | 14 +++++--- .../classes/jogamp/opengl/GLAutoDrawableBase.java | 39 ++++++++++++++++------ .../jogamp/opengl/GLDrawableFactoryImpl.java | 2 +- src/jogl/classes/jogamp/opengl/GLPbufferImpl.java | 4 +-- .../classes/jogamp/nativewindow/x11/X11Util.java | 2 +- .../classes/com/jogamp/newt/opengl/GLWindow.java | 2 +- .../test/junit/jogl/acore/TestFBODrawableNEWT.java | 2 +- .../jogl/acore/TestGLAutoDrawableDelegateNEWT.java | 2 +- .../acore/TestGLContextDrawableSwitchNEWT.java | 2 +- .../jogl/acore/TestNEWTCloseX11DisplayBug565.java | 8 ++--- 12 files changed, 64 insertions(+), 32 deletions(-) (limited to 'src/jogl/classes/jogamp/opengl/GLPbufferImpl.java') diff --git a/make/scripts/tests.sh b/make/scripts/tests.sh index af6218968..43d04c6bd 100755 --- a/make/scripts/tests.sh +++ b/make/scripts/tests.sh @@ -61,7 +61,7 @@ function jrun() { #D_ARGS="-Djogl.debug=all -Dnativewindow.debug=all -Dnewt.debug=all" #D_ARGS="-Djogl.debug=all -Dnativewindow.debug=all -Dnewt.debug=all -Djogamp.debug.Lock" #D_ARGS="-Djogl.debug=all -Dnativewindow.debug=all" - D_ARGS="-Dnewt.debug.Window" + #D_ARGS="-Dnewt.debug.Window" #D_ARGS="-Djogl.debug.GLDrawable" #D_ARGS="-Djogl.debug.EGLDrawableFactory.DontQuery -Djogl.debug.GLDrawable" #D_ARGS="-Djogl.debug.EGLDrawableFactory.QueryNativeTK -Djogl.debug.GLDrawable" @@ -217,7 +217,7 @@ function testawtswt() { #testnoawt com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestGearsES2NEWT $* #testnoawt com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestElektronenMultipliziererNEWT $* #testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestFloatUtil01MatrixMatrixMultNOUI $* -#testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestNEWTCloseX11DisplayBug565 $* +testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestNEWTCloseX11DisplayBug565 $* #testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestMainVersionGLWindowNEWT $* #testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestGLProfile01NEWT $* #testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestShutdownCompleteNEWT $* @@ -328,7 +328,7 @@ function testawtswt() { #testawt com.jogamp.opengl.test.junit.newt.parenting.TestParenting03AWT $* #testawt com.jogamp.opengl.test.junit.newt.parenting.TestParenting04AWT $* #testawtswt com.jogamp.opengl.test.junit.newt.parenting.TestParenting01aSWT $* -testawtswt com.jogamp.opengl.test.junit.newt.parenting.TestParenting04SWT $* +#testawtswt com.jogamp.opengl.test.junit.newt.parenting.TestParenting04SWT $* #testawt com.jogamp.opengl.test.junit.newt.parenting.TestParentingFocusTraversal01AWT $* #testawt com.jogamp.opengl.test.junit.newt.parenting.TestParentingOffscreenLayer01GLCanvasAWT $* #testawt com.jogamp.opengl.test.junit.newt.parenting.TestParentingOffscreenLayer02NewtCanvasAWT $* diff --git a/src/jogl/classes/com/jogamp/opengl/OffscreenAutoDrawable.java b/src/jogl/classes/com/jogamp/opengl/OffscreenAutoDrawable.java index 1ea8595c6..8450ffdb0 100644 --- a/src/jogl/classes/com/jogamp/opengl/OffscreenAutoDrawable.java +++ b/src/jogl/classes/com/jogamp/opengl/OffscreenAutoDrawable.java @@ -28,6 +28,7 @@ package com.jogamp.opengl; +import javax.media.nativewindow.AbstractGraphicsDevice; import javax.media.opengl.GLAutoDrawableDelegate; import javax.media.opengl.GLContext; import javax.media.opengl.GLDrawable; @@ -45,8 +46,16 @@ import jogamp.opengl.GLFBODrawableImpl; */ public class OffscreenAutoDrawable extends GLAutoDrawableDelegate { - public OffscreenAutoDrawable(GLDrawable drawable, GLContext context, Object upstreamWidget) { - super(drawable, context, upstreamWidget); + /** + * @param drawable a valid {@link GLDrawable}, may not be realized yet. + * @param context a valid {@link GLContext}, may not be made current (created) yet. + * @param ownDevice pass true if {@link AbstractGraphicsDevice#close()} shall be issued, + * otherwise pass false. Closing the device is required in case + * the drawable is created w/ it's own new instance, e.g. offscreen drawables, + * and no further lifecycle handling is applied. + */ + public OffscreenAutoDrawable(GLDrawable drawable, GLContext context, boolean ownDevice) { + super(drawable, context, null, ownDevice); } /** diff --git a/src/jogl/classes/javax/media/opengl/GLAutoDrawableDelegate.java b/src/jogl/classes/javax/media/opengl/GLAutoDrawableDelegate.java index 76959f3f4..67e81270d 100644 --- a/src/jogl/classes/javax/media/opengl/GLAutoDrawableDelegate.java +++ b/src/jogl/classes/javax/media/opengl/GLAutoDrawableDelegate.java @@ -28,6 +28,8 @@ package javax.media.opengl; +import javax.media.nativewindow.AbstractGraphicsDevice; + import com.jogamp.common.util.locks.LockFactory; import com.jogamp.common.util.locks.RecursiveLock; @@ -57,12 +59,16 @@ public class GLAutoDrawableDelegate extends GLAutoDrawableBase { public static final boolean DEBUG = Debug.debug("GLAutoDrawableDelegate"); /** - * @param drawable - * @param context + * @param drawable a valid {@link GLDrawable}, may not be realized yet. + * @param context a valid {@link GLContext}, may not be made current (created) yet. * @param upstreamWidget optional UI element holding this instance, see {@link #getUpstreamWidget()}. + * @param ownDevice pass true if {@link AbstractGraphicsDevice#close()} shall be issued, + * otherwise pass false. Closing the device is required in case + * the drawable is created w/ it's own new instance, e.g. offscreen drawables, + * and no further lifecycle handling is applied. */ - public GLAutoDrawableDelegate(GLDrawable drawable, GLContext context, Object upstreamWidget) { - super((GLDrawableImpl)drawable, (GLContextImpl)context); + public GLAutoDrawableDelegate(GLDrawable drawable, GLContext context, Object upstreamWidget, boolean ownDevice) { + super((GLDrawableImpl)drawable, (GLContextImpl)context, ownDevice); this.upstreamWidget = null; } diff --git a/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java b/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java index fe6d0fd76..cc4e1b434 100644 --- a/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java +++ b/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java @@ -30,6 +30,8 @@ package jogamp.opengl; import java.io.PrintStream; +import javax.media.nativewindow.AbstractGraphicsConfiguration; +import javax.media.nativewindow.AbstractGraphicsDevice; import javax.media.nativewindow.NativeSurface; import javax.media.nativewindow.WindowClosingProtocol; import javax.media.nativewindow.WindowClosingProtocol.WindowClosingMode; @@ -66,13 +68,23 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { protected volatile GLDrawableImpl drawable; // volatile: avoid locking for read-only access protected GLContextImpl context; + protected final boolean ownDevice; protected int additionalCtxCreationFlags = 0; protected volatile boolean sendReshape = false; // volatile: maybe written by WindowManager thread w/o locking protected volatile boolean sendDestroy = false; // volatile: maybe written by WindowManager thread w/o locking - public GLAutoDrawableBase(GLDrawableImpl drawable, GLContextImpl context) { + /** + * @param drawable a valid {@link GLDrawableImpl}, may not be realized yet. + * @param context a valid {@link GLContextImpl}, may not be made current (created) yet. + * @param ownDevice pass true if {@link AbstractGraphicsDevice#close()} shall be issued, + * otherwise pass false. Closing the device is required in case + * the drawable is created w/ it's own new instance, e.g. offscreen drawables, + * and no further lifecycle handling is applied. + */ + public GLAutoDrawableBase(GLDrawableImpl drawable, GLContextImpl context, boolean ownDevice) { this.drawable = drawable; this.context = context; + this.ownDevice = ownDevice; resetFPSCounter(); } @@ -174,7 +186,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { } /** - * Calls {@link #destroyImplInLock()} while claiming the lock. + * Calls {@link #destroyImplInLock()} while claiming the lock. */ protected final void defaultDestroy() { final RecursiveLock lock = getLock(); @@ -200,17 +212,22 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, FPSCounter { protected void destroyImplInLock() { final GLContext _context = context; final GLDrawable _drawable = drawable; - if( null != _drawable && _drawable.isRealized() ) { - if( null != _context && _context.isCreated() ) { - // Catch dispose GLExceptions by GLEventListener, just 'print' them - // so we can continue with the destruction. - try { - helper.disposeGL(this, _drawable, _context, null); - } catch (GLException gle) { - gle.printStackTrace(); + if( null != _drawable ) { + if( _drawable.isRealized() ) { + if( null != _context && _context.isCreated() ) { + // Catch dispose GLExceptions by GLEventListener, just 'print' them + // so we can continue with the destruction. + try { + helper.disposeGL(this, _drawable, _context, null); + } catch (GLException gle) { + gle.printStackTrace(); + } } + _drawable.setRealized(false); + } + if( ownDevice ) { + _drawable.getNativeSurface().getGraphicsConfiguration().getScreen().getDevice().close(); } - _drawable.setRealized(false); } context = null; drawable = null; diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableFactoryImpl.java b/src/jogl/classes/jogamp/opengl/GLDrawableFactoryImpl.java index f092288fb..f7808294b 100644 --- a/src/jogl/classes/jogamp/opengl/GLDrawableFactoryImpl.java +++ b/src/jogl/classes/jogamp/opengl/GLDrawableFactoryImpl.java @@ -245,7 +245,7 @@ public abstract class GLDrawableFactoryImpl extends GLDrawableFactory { if(null==drawable) { throw new GLException("Could not create Pbuffer drawable for: "+device+", "+capsChosen+", "+width+"x"+height); } - return new GLPbufferImpl( drawable, shareWith); + return new GLPbufferImpl( drawable, shareWith, true); } //--------------------------------------------------------------------------- diff --git a/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java b/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java index 6b64120fe..32f4cb696 100644 --- a/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java +++ b/src/jogl/classes/jogamp/opengl/GLPbufferImpl.java @@ -58,8 +58,8 @@ import com.jogamp.common.util.locks.RecursiveLock; public class GLPbufferImpl extends GLAutoDrawableBase implements GLPbuffer { private int floatMode; - public GLPbufferImpl(GLDrawableImpl pbufferDrawable, GLContext sharedContext) { - super(pbufferDrawable, null); // drawable := pbufferDrawable + public GLPbufferImpl(GLDrawableImpl pbufferDrawable, GLContext sharedContext, boolean ownDevice) { + super(pbufferDrawable, null, ownDevice); // drawable := pbufferDrawable GLCapabilitiesImmutable caps = (GLCapabilitiesImmutable) drawable.getNativeSurface().getGraphicsConfiguration().getChosenCapabilities(); diff --git a/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java b/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java index fcc374751..7b46a1df0 100644 --- a/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java +++ b/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java @@ -255,7 +255,7 @@ public class X11Util { */ public static int shutdown(boolean realXCloseOpenAndPendingDisplays, boolean verbose) { int num=0; - if(DEBUG||verbose||pendingDisplayList.size() > 0) { + if(DEBUG || verbose || openDisplayMap.size() > 0 || pendingDisplayList.size() > 0) { System.err.println("X11Util.Display: Shutdown (close open / pending Displays: "+realXCloseOpenAndPendingDisplays+ ", open (no close attempt): "+openDisplayMap.size()+"/"+openDisplayList.size()+ ", pending (not closed, marked uncloseable): "+pendingDisplayList.size()+")"); diff --git a/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java b/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java index d662a743a..0fc1b4e89 100644 --- a/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java +++ b/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java @@ -97,7 +97,7 @@ public class GLWindow extends GLAutoDrawableBase implements GLAutoDrawable, Wind * Constructor. Do not call this directly -- use {@link #create()} instead. */ protected GLWindow(Window window) { - super(null, null); + super(null, null, false); this.window = (WindowImpl) window; this.window.setHandleDestroyNotify(false); window.addWindowListener(new WindowAdapter() { diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestFBODrawableNEWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestFBODrawableNEWT.java index 1a33845b3..7977347a7 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestFBODrawableNEWT.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestFBODrawableNEWT.java @@ -167,7 +167,7 @@ public class TestFBODrawableNEWT extends UITestCase { final FBObject.RenderAttachment depthA = fbo.getDepthAttachment(); Assert.assertNotNull(depthA); - final OffscreenAutoDrawable glad = new OffscreenAutoDrawable(fboDrawable, context, null); + final OffscreenAutoDrawable glad = new OffscreenAutoDrawable(fboDrawable, context, true); glad.addGLEventListener(demo); glad.addGLEventListener(new GLEventListener() { diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLAutoDrawableDelegateNEWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLAutoDrawableDelegateNEWT.java index 426b7734f..96d9b2e28 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLAutoDrawableDelegateNEWT.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLAutoDrawableDelegateNEWT.java @@ -85,7 +85,7 @@ public class TestGLAutoDrawableDelegateNEWT extends UITestCase { Assert.assertTrue(GLContext.CONTEXT_CURRENT_NEW==res || GLContext.CONTEXT_CURRENT==res); context.release(); - final GLAutoDrawableDelegate glad = new GLAutoDrawableDelegate(drawable, context, window) { + final GLAutoDrawableDelegate glad = new GLAutoDrawableDelegate(drawable, context, window, false) { @Override protected void destroyImplInLock() { super.destroyImplInLock(); // destroys drawable/context diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextDrawableSwitchNEWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextDrawableSwitchNEWT.java index 92b4c5238..cece4c6d5 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextDrawableSwitchNEWT.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestGLContextDrawableSwitchNEWT.java @@ -88,7 +88,7 @@ public class TestGLContextDrawableSwitchNEWT extends UITestCase { drawable.setRealized(true); Assert.assertTrue(drawable.isRealized()); - final GLAutoDrawableDelegate glad = new GLAutoDrawableDelegate(drawable, null, window) { + final GLAutoDrawableDelegate glad = new GLAutoDrawableDelegate(drawable, null, window, false) { @Override protected void destroyImplInLock() { super.destroyImplInLock(); diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestNEWTCloseX11DisplayBug565.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestNEWTCloseX11DisplayBug565.java index e14d5b800..33a9b7799 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestNEWTCloseX11DisplayBug565.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestNEWTCloseX11DisplayBug565.java @@ -43,10 +43,10 @@ public class TestNEWTCloseX11DisplayBug565 { if(NativeWindowFactory.TYPE_X11 == NativeWindowFactory.getNativeWindowType(false)) { final int openD = X11Util.getOpenDisplayConnectionNumber() - open0; - if(openD>1) { + if( openD > 0) { X11Util.dumpOpenDisplayConnections(); X11Util.dumpPendingDisplayConnections(); - Assert.assertTrue("More than 1 new open display connections", false); + Assert.assertEquals("New display connection didn't close", 0, openD); } } } @@ -86,10 +86,10 @@ public class TestNEWTCloseX11DisplayBug565 { if(NativeWindowFactory.TYPE_X11 == NativeWindowFactory.getNativeWindowType(false)) { final int openD = X11Util.getOpenDisplayConnectionNumber() - open0; - if(openD>1) { + if(openD > 0) { X11Util.dumpOpenDisplayConnections(); X11Util.dumpPendingDisplayConnections(); - Assert.assertTrue("More than 1 new open display connections", false); + Assert.assertEquals("New display connection didn't close", 0, openD); } } } -- cgit v1.2.3