aboutsummaryrefslogtreecommitdiffstats
path: root/src/jogl/classes/com
diff options
context:
space:
mode:
authorSven Gothel <[email protected]>2012-07-22 04:16:55 +0200
committerSven Gothel <[email protected]>2012-07-22 04:16:55 +0200
commit4b5a0f6557d7152ec770bc13ad3c494449de0529 (patch)
tree12fc93f9e7f34b61c1d5748f0e6a1cd85fc6125b /src/jogl/classes/com
parentadc9522ccaff74eb779d4d33905d76d52acb36bb (diff)
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.
Diffstat (limited to 'src/jogl/classes/com')
-rw-r--r--src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java172
1 files changed, 94 insertions, 78 deletions
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
- * <p>
- * 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!
- * </p>
- * <p>
- * 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.
- * </p>
*/
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();
}
/**