From a94ff9252df66c303f48489c3e8926104941465c Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Sat, 16 Feb 2013 03:55:22 +0100 Subject: Fix Bug 691 (part-3): NSOpenGLLayer::openGLContextForPixelFormat(..) on main-thread deadlock'ed due to locked shared context NSOpenGLLayer::openGLContextForPixelFormat(..) is performed on main-thread at 1st NSOpenGLLayer display method. This happened irregulary, i.e. sometimes (T0) right after NSOpenGLLayer creation and attachSurfaceLayer()/AddCASublayer(..), sometimes later (T1). NSOpenGLLayer::openGLContextForPixelFormat(..) uses the passed shared user context. The shared user context is locked at NSOpenGLLayer's creation (T0) and if performed at this early time the call deadlocks due to pthread_mutex wait for the shared user context. This fix performs NSOpenGLLayer creation and layer attachment while the shared user context is kept unlocked and enforces NSOpenGLLayer display and hence NSOpenGLLayer::openGLContextForPixelFormat(..). Added CGL.setNSOpenGLLayerEnabled(..) to enable/disable NSOpenGLLayer - currently not used. - Passed AddRemove tests for GLCanvas/Swing and GLWindow/NewtCanvasAWT w/ 100 loops on Java6 and Java7 on OSX. - Passed Instruments Leaks test w/ 10 loops on Java6 and Java7 --- make/config/jogl/cgl-macosx-CustomJavaCode.java | 22 +++++++++++++++ make/config/jogl/cgl-macosx.cfg | 2 ++ make/scripts/tests.sh | 4 +-- make/stub_includes/opengl/macosx-window-system.h | 1 + .../jogamp/opengl/macosx/cgl/MacOSXCGLContext.java | 31 +++++++++++++++++----- .../macosx/MacOSXWindowSystemInterface-calayer.m | 28 ++++++++++++++----- .../media/nativewindow/OffscreenLayerSurface.java | 8 ++++++ .../jogamp/nativewindow/macosx/OSXUtil.java | 28 +++++++++++++++++++ src/nativewindow/native/macosx/OSXmisc.m | 16 +++++++---- .../acore/TestAddRemove01GLCanvasSwingAWT.java | 2 +- .../TestAddRemove02GLWindowNewtCanvasAWT.java | 2 +- 11 files changed, 123 insertions(+), 21 deletions(-) diff --git a/make/config/jogl/cgl-macosx-CustomJavaCode.java b/make/config/jogl/cgl-macosx-CustomJavaCode.java index b9a37d0c6..d32e0ae8f 100644 --- a/make/config/jogl/cgl-macosx-CustomJavaCode.java +++ b/make/config/jogl/cgl-macosx-CustomJavaCode.java @@ -1,6 +1,14 @@ /** * Creates the NSOpenGLLayer for FBO/PBuffer w/ optional GL3 shader program on Main-Thread + *

+ * It is mandatory that the shared context handle ctx + * is not locked while calling this method. + *

+ *

+ * The NSOpenGLLayer starts in enabled mode, + * you may enable/disable it via {@link #setNSOpenGLLayerEnabled(long, boolean)}. + *

*/ public static long createNSOpenGLLayer(final long ctx, final int gl3ShaderProgramName, final long fmt, final long p, final int texID, final boolean opaque, final int texWidth, final int texHeight) { @@ -10,6 +18,20 @@ public static long createNSOpenGLLayer(final long ctx, final int gl3ShaderProgra } } ).longValue(); } +/** + * Enable or disable NSOpenGLLayer. + * + *

+ * If disabled, the NSOpenGLLayer will not be displayed, i.e. rendered. + *

+ */ +public static void setNSOpenGLLayerEnabled(final long nsOpenGLLayer, final boolean enable) { + OSXUtil.RunOnMainThread(true, new Runnable() { + public void run() { + setNSOpenGLLayerEnabledImpl(nsOpenGLLayer, enable); + } } ); +} + /** * Releases the NSOpenGLLayer on Main-Thread */ diff --git a/make/config/jogl/cgl-macosx.cfg b/make/config/jogl/cgl-macosx.cfg index 98123f605..edb5bfbbf 100644 --- a/make/config/jogl/cgl-macosx.cfg +++ b/make/config/jogl/cgl-macosx.cfg @@ -35,8 +35,10 @@ CustomCCode #include CustomCCode #include "macosx-window-system.h" AccessControl createNSOpenGLLayerImpl PRIVATE +AccessControl setNSOpenGLLayerEnabledImpl PRIVATE AccessControl releaseNSOpenGLLayerImpl PRIVATE RenameJavaMethod createNSOpenGLLayer createNSOpenGLLayerImpl +RenameJavaMethod setNSOpenGLLayerEnabled setNSOpenGLLayerEnabledImpl RenameJavaMethod releaseNSOpenGLLayer releaseNSOpenGLLayerImpl IncludeAs CustomJavaCode CGL cgl-macosx-CustomJavaCode.java diff --git a/make/scripts/tests.sh b/make/scripts/tests.sh index 559c2aa6a..a20ec507c 100755 --- a/make/scripts/tests.sh +++ b/make/scripts/tests.sh @@ -301,8 +301,8 @@ function testawtswt() { #testnoawt com.jogamp.opengl.test.junit.jogl.acore.ect.TestExclusiveContext12FPSAnimNEWT $* #testawt com.jogamp.opengl.test.junit.jogl.acore.TestOffscreenLayer01GLCanvasAWT $* #testawt com.jogamp.opengl.test.junit.jogl.acore.TestOffscreenLayer02NewtCanvasAWT $* -#testawt com.jogamp.opengl.test.junit.jogl.acore.TestAddRemove01GLCanvasSwingAWT $* -testawt com.jogamp.opengl.test.junit.jogl.acore.TestAddRemove02GLWindowNewtCanvasAWT $* +testawt com.jogamp.opengl.test.junit.jogl.acore.TestAddRemove01GLCanvasSwingAWT $* +#testawt com.jogamp.opengl.test.junit.jogl.acore.TestAddRemove02GLWindowNewtCanvasAWT $* #testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestGLAutoDrawableDelegateOnOffscrnCapsNEWT $* #testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestGLAutoDrawableFactoryOffscrnCapsNEWT $* diff --git a/make/stub_includes/opengl/macosx-window-system.h b/make/stub_includes/opengl/macosx-window-system.h index a2da66878..402a16efc 100644 --- a/make/stub_includes/opengl/macosx-window-system.h +++ b/make/stub_includes/opengl/macosx-window-system.h @@ -56,6 +56,7 @@ void setContextTextureImageToPBuffer(NSOpenGLContext* ctx, NSOpenGLPixelBuffer* Bool isNSOpenGLPixelBuffer(uint64_t object); NSOpenGLLayer* createNSOpenGLLayer(NSOpenGLContext* ctx, int gl3ShaderProgramName, NSOpenGLPixelFormat* fmt, NSOpenGLPixelBuffer* p, uint32_t texID, Bool opaque, int texWidth, int texHeight); +void setNSOpenGLLayerEnabled(NSOpenGLLayer* layer, Bool enable); void setNSOpenGLLayerSwapInterval(NSOpenGLLayer* layer, int interval); void waitUntilNSOpenGLLayerIsReady(NSOpenGLLayer* layer, long to_micros); void setNSOpenGLLayerNeedsDisplayFBO(NSOpenGLLayer* layer, uint32_t texID); diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java index 838a0387d..37aca0cd7 100644 --- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java +++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java @@ -687,13 +687,32 @@ public abstract class MacOSXCGLContext extends GLContextImpl } else { gl3ShaderProgramName = 0; } - nsOpenGLLayer = CGL.createNSOpenGLLayer(ctx, gl3ShaderProgramName, nsOpenGLLayerPFmt, pbufferHandle, texID, chosenCaps.isBackgroundOpaque(), lastWidth, lastHeight); - nsOpenGLLayerPFmt = 0; // NSOpenGLLayer will release pfmt - if (DEBUG) { - System.err.println("NS create nsOpenGLLayer "+toHexString(nsOpenGLLayer)+" w/ pbuffer "+toHexString(pbufferHandle)+", texID "+texID+", texSize "+lastWidth+"x"+lastHeight+", "+drawable); + + /** + * Perform NSOpenGLLayer creation and attaching on main-thread, + * hence release the lock on our context - which will be used to + * create a shared context within NSOpenGLLayer. + */ + final long cglCtx = CGL.getCGLContext(ctx); + if(0 == cglCtx) { + throw new InternalError("Null CGLContext for: "+this); + } + final boolean ctxUnlocked = CGL.kCGLNoError == CGL.CGLUnlockContext(cglCtx); + try { + nsOpenGLLayer = CGL.createNSOpenGLLayer(ctx, gl3ShaderProgramName, nsOpenGLLayerPFmt, pbufferHandle, texID, chosenCaps.isBackgroundOpaque(), lastWidth, lastHeight); + nsOpenGLLayerPFmt = 0; // NSOpenGLLayer will release pfmt + if (DEBUG) { + System.err.println("NS create nsOpenGLLayer "+toHexString(nsOpenGLLayer)+" w/ pbuffer "+toHexString(pbufferHandle)+", texID "+texID+", texSize "+lastWidth+"x"+lastHeight+", "+drawable); + } + backingLayerHost.attachSurfaceLayer(nsOpenGLLayer); + setSwapInterval(1); // enabled per default in layered surface + } finally { + if( ctxUnlocked ) { + if( CGL.kCGLNoError != CGL.CGLLockContext(cglCtx) ) { + throw new InternalError("Could not re-lock CGLContext for: "+this); + } + } } - backingLayerHost.attachSurfaceLayer(nsOpenGLLayer); - setSwapInterval(1); // enabled per default in layered surface } else { lastWidth = drawable.getWidth(); lastHeight = drawable.getHeight(); diff --git a/src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m b/src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m index 8d1286169..55c4ad053 100644 --- a/src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m +++ b/src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m @@ -130,6 +130,7 @@ extern GLboolean glIsVertexArray (GLuint array); @private GLfloat gl_texCoords[8]; NSOpenGLContext* myCtx; + Bool isGLEnabled; @protected NSOpenGLContext* parentCtx; @@ -171,6 +172,7 @@ extern GLboolean glIsVertexArray (GLuint array); texWidth: (int) texWidth texHeight: (int) texHeight; +- (void) setGLEnabled: (Bool) enable; - (Bool) validateTexSizeWithNewSize; - (Bool) validateTexSize: (int) _texWidth texHeight: (int) _texHeight; - (void) setTextureID: (int) _texID; @@ -266,6 +268,7 @@ static const GLfloat gl_verts[] = { swapIntervalCounter = 0; timespec_now(&lastWaitTime); shallDraw = NO; + isGLEnabled = YES; newTexWidth = _texWidth; newTexHeight = _texHeight; [self validateTexSizeWithNewSize]; @@ -335,6 +338,12 @@ static const GLfloat gl_verts[] = { return self; } +- (void) setGLEnabled: (Bool) enable +{ + DBG_PRINT("MyNSOpenGLLayer::setGLEnabled: %p, %d -> %d\n", self, (int)isGLEnabled, (int)enable); + isGLEnabled = enable; +} + - (Bool) validateTexSizeWithNewSize { return [self validateTexSize: newTexWidth texHeight: newTexHeight]; @@ -444,6 +453,7 @@ static const GLfloat gl_verts[] = { { DBG_PRINT("MyNSOpenGLLayer::openGLPixelFormatForDisplayMask: %p (refcnt %d) - parent-pfmt %p -> new-pfmt %p\n", self, (int)[self retainCount], parentPixelFmt, parentPixelFmt); + // We simply take over ownership of parent PixelFormat .. return parentPixelFmt; } @@ -531,7 +541,6 @@ static const GLfloat gl_verts[] = { [self disableAnimation]; pthread_mutex_lock(&renderLock); [self deallocPBuffer]; - // [[self openGLContext] dealloc]; pthread_mutex_unlock(&renderLock); pthread_cond_destroy(&renderSignal); pthread_mutex_destroy(&renderLock); @@ -548,8 +557,8 @@ static const GLfloat gl_verts[] = { { CGRect lRectS = [[self superlayer] bounds]; - DBG_PRINT("MyNSOpenGLLayer::resizeWithOldSuperlayerSize: %p, texSize %dx%d, bounds: %lfx%lf -> %lfx%lf (refcnt %d)\n", - self, texWidth, texHeight, size.width, size.height, lRectS.size.width, lRectS.size.height, (int)[self retainCount]); + DBG_PRINT("MyNSOpenGLLayer::resizeWithOldSuperlayerSize: %p, texSize %dx%d, bounds: %lfx%lf -> %lf/%lf %lfx%lf (refcnt %d)\n", + self, texWidth, texHeight, size.width, size.height, lRectS.origin.x, lRectS.origin.y, lRectS.size.width, lRectS.size.height, (int)[self retainCount]); newTexWidth = lRectS.size.width; newTexHeight = lRectS.size.height; @@ -562,8 +571,8 @@ static const GLfloat gl_verts[] = { - (BOOL)canDrawInOpenGLContext:(NSOpenGLContext *)context pixelFormat:(NSOpenGLPixelFormat *)pixelFormat forLayerTime:(CFTimeInterval)timeInterval displayTime:(const CVTimeStamp *)timeStamp { - SYNC_PRINT("", (int)shallDraw); - return shallDraw; + SYNC_PRINT("", (int)shallDraw, (int)isGLEnabled); + return shallDraw && isGLEnabled; } - (void)drawInOpenGLContext:(NSOpenGLContext *)context pixelFormat:(NSOpenGLPixelFormat *)pixelFormat @@ -573,7 +582,7 @@ static const GLfloat gl_verts[] = { SYNC_PRINT("<* "); // NSLog(@"MyNSOpenGLLayer::DRAW: %@",[NSThread callStackSymbols]); - if( shallDraw && ( NULL != pbuffer || NULL != newPBuffer || 0 != textureID ) ) { + if( isGLEnabled && shallDraw && ( NULL != pbuffer || NULL != newPBuffer || 0 != textureID ) ) { [context makeCurrentContext]; if( NULL != newPBuffer ) { // volatile OK @@ -825,6 +834,13 @@ NSOpenGLLayer* createNSOpenGLLayer(NSOpenGLContext* ctx, int gl3ShaderProgramNam return [[[MyNSOpenGLLayer alloc] init] setupWithContext:ctx gl3ShaderProgramName: (GLuint)gl3ShaderProgramName pixelFormat: fmt pbuffer: p texIDArg: (GLuint)texID opaque: opaque texWidth: texWidth texHeight: texHeight]; } + +void setNSOpenGLLayerEnabled(NSOpenGLLayer* layer, Bool enable) { + NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init]; + MyNSOpenGLLayer* l = (MyNSOpenGLLayer*) layer; + [l setGLEnabled: enable]; + [pool release]; +} void setNSOpenGLLayerSwapInterval(NSOpenGLLayer* layer, int interval) { NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init]; diff --git a/src/nativewindow/classes/javax/media/nativewindow/OffscreenLayerSurface.java b/src/nativewindow/classes/javax/media/nativewindow/OffscreenLayerSurface.java index f6bc5822b..ba60a7f38 100644 --- a/src/nativewindow/classes/javax/media/nativewindow/OffscreenLayerSurface.java +++ b/src/nativewindow/classes/javax/media/nativewindow/OffscreenLayerSurface.java @@ -33,6 +33,14 @@ package javax.media.nativewindow; public interface OffscreenLayerSurface { /** * Attach the offscreen layer to this offscreen layer surface. + *

+ * Implementation may realize all required resources at this point. + *

+ *

+ * It is mandatory that any related resources, e.g. a shared context, + * are not locked while calling this method. + *

+ * * @see #isOffscreenLayerSurfaceEnabled() * @throws NativeWindowException if {@link #isOffscreenLayerSurfaceEnabled()} == false */ diff --git a/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java b/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java index b765a68c3..d85d1a84b 100644 --- a/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java +++ b/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java @@ -136,12 +136,31 @@ public class OSXUtil implements ToolkitProperties { return GetNSWindow0(nsView); } + /** + * Create a CALayer suitable to act as a root CALayer on the main-thread. + * @see #DestroyCALayer(long) + * @see #AddCASublayer(long, long) + */ public static long CreateCALayer(final int x, final int y, final int width, final int height) { return OSXUtil.RunOnMainThread(true, new Function() { public Long eval(Object... args) { return Long.valueOf( CreateCALayer0(x, y, width, height) ); } } ).longValue(); } + + /** + * Attach a sub CALayer to the root CALayer on the main-thread. + *

+ * Method will trigger a display + * call to the CALayer hierarchy to enforce resource creation if required, e.g. an NSOpenGLContext. + *

+ *

+ * It is mandatory that any related resources, e.g. a shared NSOpenGLContext, + * are not locked while calling this method. + *

+ * @see #CreateCALayer(int, int, int, int) + * @see #RemoveCASublayer(long, long) + */ public static void AddCASublayer(final long rootCALayer, final long subCALayer) { if(0==rootCALayer || 0==subCALayer) { throw new IllegalArgumentException("rootCALayer 0x"+Long.toHexString(rootCALayer)+", subCALayer 0x"+Long.toHexString(subCALayer)); @@ -152,6 +171,10 @@ public class OSXUtil implements ToolkitProperties { } }); } + + /** + * Detach a sub CALayer from the root CALayer on the main-thread. + */ public static void RemoveCASublayer(final long rootCALayer, final long subCALayer) { if(0==rootCALayer || 0==subCALayer) { throw new IllegalArgumentException("rootCALayer 0x"+Long.toHexString(rootCALayer)+", subCALayer 0x"+Long.toHexString(subCALayer)); @@ -161,6 +184,11 @@ public class OSXUtil implements ToolkitProperties { RemoveCASublayer0(rootCALayer, subCALayer); } } ); } + + /** + * Destroy a CALayer on the main-thread. + * @see #CreateCALayer(int, int, int, int) + */ public static void DestroyCALayer(final long caLayer) { if(0==caLayer) { throw new IllegalArgumentException("caLayer 0x"+Long.toHexString(caLayer)); diff --git a/src/nativewindow/native/macosx/OSXmisc.m b/src/nativewindow/native/macosx/OSXmisc.m index 83f3c821f..8d876c175 100644 --- a/src/nativewindow/native/macosx/OSXmisc.m +++ b/src/nativewindow/native/macosx/OSXmisc.m @@ -430,6 +430,9 @@ JNIEXPORT void JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_AddCASublayer0 MyCALayer* rootLayer = (MyCALayer*) ((intptr_t) rootCALayer); CALayer* subLayer = (CALayer*) ((intptr_t) subCALayer); + [CATransaction begin]; + [CATransaction setValue:(id)kCFBooleanTrue forKey:kCATransactionDisableActions]; + CGRect lRectRoot = [rootLayer frame]; DBG_PRINT("CALayer::AddCASublayer0.0: Origin %p frame0: %lf/%lf %lfx%lf\n", rootLayer, lRectRoot.origin.x, lRectRoot.origin.y, lRectRoot.size.width, lRectRoot.size.height); @@ -444,9 +447,6 @@ JNIEXPORT void JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_AddCASublayer0 rootLayer, (int)[rootLayer retainCount], subLayer, lRectRoot.origin.x, lRectRoot.origin.y, lRectRoot.size.width, lRectRoot.size.height, (int)[subLayer retainCount]); - [CATransaction begin]; - [CATransaction setValue:(id)kCFBooleanTrue forKey:kCATransactionDisableActions]; - // simple 1:1 layout ! [subLayer setFrame:lRectRoot]; [rootLayer addSublayer:subLayer]; @@ -454,14 +454,20 @@ JNIEXPORT void JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_AddCASublayer0 // no animations for add/remove/swap sublayers etc // doesn't work: [layer removeAnimationForKey: kCAOnOrderIn, kCAOnOrderOut, kCATransition] [rootLayer removeAllAnimations]; - // [rootLayer addAnimation:nil forKey:kCATransition]; + // [rootLayer addAnimation:nil forKey:kCATransition]; // JAU [rootLayer setAutoresizingMask: (kCALayerWidthSizable|kCALayerHeightSizable)]; [rootLayer setNeedsDisplayOnBoundsChange: YES]; [subLayer removeAllAnimations]; - // [sublayer addAnimation:nil forKey:kCATransition]; + // [subLayer addAnimation:nil forKey:kCATransition]; // JAU [subLayer setAutoresizingMask: (kCALayerWidthSizable|kCALayerHeightSizable)]; [subLayer setNeedsDisplayOnBoundsChange: YES]; + // Trigger display and hence ctx creation. + // The latter is essential since since the parent-context lock is cleared + // only for this window of time (method call). + [rootLayer setNeedsDisplay]; + [rootLayer displayIfNeeded]; + [CATransaction commit]; [pool release]; diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove01GLCanvasSwingAWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove01GLCanvasSwingAWT.java index 2038124b5..61652371b 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove01GLCanvasSwingAWT.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove01GLCanvasSwingAWT.java @@ -140,8 +140,8 @@ public class TestAddRemove01GLCanvasSwingAWT extends UITestCase { SwingUtilities.invokeAndWait(new Runnable() { public void run() { if( visible ) { - jFrame.validate(); jFrame.pack(); + jFrame.validate(); } jFrame.setVisible(visible); } } ) ; diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove02GLWindowNewtCanvasAWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove02GLWindowNewtCanvasAWT.java index 0f12f967a..b4272a9c0 100644 --- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove02GLWindowNewtCanvasAWT.java +++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove02GLWindowNewtCanvasAWT.java @@ -142,8 +142,8 @@ public class TestAddRemove02GLWindowNewtCanvasAWT extends UITestCase { SwingUtilities.invokeAndWait(new Runnable() { public void run() { if( visible ) { - jFrame.validate(); jFrame.pack(); + jFrame.validate(); } jFrame.setVisible(visible); } } ) ; -- cgit v1.2.3