From 2aa296771e3e8dd6cf027f27b0455d1803244bfe Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Sun, 21 Nov 2010 03:41:22 +0100 Subject: JOGL/NEWT: Animator fixes Consider use cases with many drawables and no drawables at start, this had to be reflected all over this patch set, implementation, usage and test cases. - GLAnimatorControl - refine API doc / states - add 'void remove(GLAutoDrawable drawable);' - Animator*: - using RecursiveLock 'stateSync' for all actions out of the big synchronized (animator) block: - get status methods (thread, isPaused, ..), hence no more synchronized - display drawables change, utilizing synced ArrayList swap This removes the need for volatiles usage shouldPause/shouldStop within the display method. - added blocking wait for state change for add(GLAutoDrawable)/remove(GLAutoDrawable) method - remove flawed double checked locking in anim thread (pause/idle condition) - thread is now a daemon thread, hence it won't hinder the JVM from shutdown - - Animator use change: - Always resume after pause, except in case of final destroy -> NEWT invalidate / GLCanvas, this considers use cases with many drawables and no drawables at start. - GLDrawableHelper: Don't pause at implicit dispose() --- .../com/jogamp/opengl/util/AWTAnimatorImpl.java | 55 ++--- .../classes/com/jogamp/opengl/util/Animator.java | 226 ++++++++++++--------- .../com/jogamp/opengl/util/AnimatorBase.java | 95 ++++++--- .../jogamp/opengl/util/DefaultAnimatorImpl.java | 33 ++- .../com/jogamp/opengl/util/FPSAnimator.java | 84 +++++--- 5 files changed, 291 insertions(+), 202 deletions(-) (limited to 'src/jogl/classes/com/jogamp/opengl/util') diff --git a/src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java b/src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java index fbad377ad..2240063d9 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java +++ b/src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java @@ -36,10 +36,16 @@ package com.jogamp.opengl.util; import java.awt.Component; import java.awt.EventQueue; import java.awt.Rectangle; -import java.util.*; -import javax.swing.*; +import java.util.ArrayList; +import java.util.IdentityHashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import javax.swing.JComponent; +import javax.swing.RepaintManager; +import javax.swing.SwingUtilities; -import javax.media.opengl.*; +import javax.media.opengl.GLAutoDrawable; /** Abstraction to factor out AWT dependencies from the Animator's implementation in a way that still allows the FPSAnimator to pick @@ -52,37 +58,34 @@ class AWTAnimatorImpl implements AnimatorBase.AnimatorImpl { private Map repaintManagers = new IdentityHashMap(); private Map dirtyRegions = new IdentityHashMap(); - public void display(AnimatorBase animator, + public void display(ArrayList drawables, boolean ignoreExceptions, boolean printExceptions) { - List drawables = animator.acquireDrawables(); - try { - for (int i=0; - animator.isAnimating() && !animator.getShouldStop() && !animator.getShouldPause() && i 0) { diff --git a/src/jogl/classes/com/jogamp/opengl/util/Animator.java b/src/jogl/classes/com/jogamp/opengl/util/Animator.java index 4f24b22a7..2d4727bba 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/Animator.java +++ b/src/jogl/classes/com/jogamp/opengl/util/Animator.java @@ -44,8 +44,6 @@ import javax.media.opengl.GLAutoDrawable; import javax.media.opengl.GLException; - - /**

An Animator can be attached to one or more {@link GLAutoDrawable}s to drive their display() methods in a loop.

@@ -61,9 +59,8 @@ public class Animator extends AnimatorBase { private Runnable runnable; private boolean runAsFastAsPossible; protected boolean isAnimating; - protected boolean isPaused; - protected volatile boolean shouldPause; - protected volatile boolean shouldStop; + protected boolean pauseIssued; + protected volatile boolean stopIssued; public Animator() { super(); @@ -104,62 +101,78 @@ public class Animator extends AnimatorBase { * This method may not have an effect on subclasses. */ public final void setRunAsFastAsPossible(boolean runFast) { - runAsFastAsPossible = runFast; + stateSync.lock(); + try { + runAsFastAsPossible = runFast; + } finally { + stateSync.unlock(); + } + } + + private void setIsAnimatingSynced(boolean v) { + stateSync.lock(); + try { + isAnimating = v; + } finally { + stateSync.unlock(); + } } class MainLoop implements Runnable { public void run() { try { - if(DEBUG) { - System.err.println("Animator started: "+Thread.currentThread()); - } - startTime = System.currentTimeMillis(); - curTime = startTime; - totalFrames = 0; - synchronized (Animator.this) { - isAnimating = true; - isPaused = false; + if(DEBUG) { + System.err.println("Animator started: "+Thread.currentThread()); + } + + startTime = System.currentTimeMillis(); + curTime = startTime; + totalFrames = 0; + + animThread = Thread.currentThread(); + setIsAnimatingSynced(false); // barrier Animator.this.notifyAll(); } - while (!shouldStop) { - // Don't consume CPU unless there is work to be done and not paused - if ( !shouldStop && ( shouldPause || drawables.size() == 0 ) ) { - synchronized (Animator.this) { - boolean wasPaused = false; - while ( !shouldStop && ( shouldPause || drawables.size() == 0 ) ) { - isAnimating = false; - isPaused = true; - wasPaused = true; - if(DEBUG) { - System.err.println("Animator paused: "+Thread.currentThread()); - } - Animator.this.notifyAll(); - try { - Animator.this.wait(); - } catch (InterruptedException e) { - } + while (!stopIssued) { + synchronized (Animator.this) { + // Don't consume CPU unless there is work to be done and not paused + while (!stopIssued && (pauseIssued || drawablesEmpty)) { + boolean wasPaused = pauseIssued; + if (DEBUG) { + System.err.println("Animator paused: " + Thread.currentThread()); + } + setIsAnimatingSynced(false); // barrier + Animator.this.notifyAll(); + try { + Animator.this.wait(); + } catch (InterruptedException e) { } - if ( wasPaused ) { + + if (wasPaused) { + // resume from pause -> reset counter startTime = System.currentTimeMillis(); - curTime = startTime; + curTime = startTime; totalFrames = 0; - isAnimating = true; - isPaused = false; - if(DEBUG) { - System.err.println("Animator resumed: "+Thread.currentThread()); + if (DEBUG) { + System.err.println("Animator resume: " + Thread.currentThread()); } - Animator.this.notifyAll(); } } - } - if ( !shouldStop && !shouldPause) { - display(); - if ( !runAsFastAsPossible) { - // Avoid swamping the CPU - Thread.yield(); + if (!stopIssued && !isAnimating) { + // resume from pause or drawablesEmpty, + // implies !pauseIssued and !drawablesEmpty + setIsAnimatingSynced(true); + Animator.this.notifyAll(); } + } // sync Animator.this + if (!stopIssued) { + display(); + } + if (!stopIssued && !runAsFastAsPossible) { + // Avoid swamping the CPU + Thread.yield(); } } } finally { @@ -167,27 +180,50 @@ public class Animator extends AnimatorBase { if(DEBUG) { System.err.println("Animator stopped: "+Thread.currentThread()); } - shouldStop = false; - shouldPause = false; - thread = null; - isAnimating = false; - isPaused = false; + stopIssued = false; + pauseIssued = false; + animThread = null; + setIsAnimatingSynced(false); // barrier Animator.this.notifyAll(); } } } } - public final synchronized boolean isStarted() { - return (thread != null); + private final boolean isStartedImpl() { + return animThread != null ; + } + public final boolean isStarted() { + stateSync.lock(); + try { + return animThread != null ; + } finally { + stateSync.unlock(); + } } - public final synchronized boolean isAnimating() { - return (thread != null) && isAnimating; + private final boolean isAnimatingImpl() { + return animThread != null && isAnimating ; + } + public final boolean isAnimating() { + stateSync.lock(); + try { + return animThread != null && isAnimating ; + } finally { + stateSync.unlock(); + } } - public final synchronized boolean isPaused() { - return (thread != null) && isPaused; + private final boolean isPausedImpl() { + return animThread != null && pauseIssued ; + } + public final boolean isPaused() { + stateSync.lock(); + try { + return animThread != null && pauseIssued ; + } finally { + stateSync.unlock(); + } } interface Condition { @@ -202,109 +238,101 @@ public class Animator extends AnimatorBase { // dependencies on the Animator's internal thread. Currently we // use a couple of heuristics to determine whether we should do // the blocking wait(). - boolean doWait = !impl.skipWaitForCompletion(thread); + boolean doWait = !impl.skipWaitForCompletion(animThread); if (doWait) { while (condition.result()) { try { wait(); - } catch (InterruptedException ie) { - } + } catch (InterruptedException ie) { } } } if(DEBUG) { System.err.println("finishLifecycleAction(" + condition.getClass().getName() + "): finished - waited " + doWait + - ", started: " + isStarted() +", animating: " + isAnimating() + - ", paused: " + isPaused()); + ", started: " + isStartedImpl() +", animating: " + isAnimatingImpl() + + ", paused: " + isPausedImpl() + ", drawables " + drawables.size()); } } public synchronized void start() { - boolean started = null != thread; - if ( started || isAnimating ) { - throw new GLException("Already running (started "+started+" (false), animating "+isAnimating+" (false))"); + if ( isStartedImpl() ) { + throw new GLException("Start: Already running (started "+isStartedImpl()+" (false))"); } if (runnable == null) { runnable = new MainLoop(); } resetCounter(); - int id; String threadName = Thread.currentThread().getName()+"-"+baseName; + Thread thread; if(null==threadGroup) { thread = new Thread(runnable, threadName); } else { thread = new Thread(threadGroup, runnable, threadName); } + thread.setDaemon(true); // don't stop JVM from shutdown .. thread.start(); - finishLifecycleAction(waitForStartedCondition); } private class WaitForStartedCondition implements Condition { public boolean result() { - // cont waiting until actually is animating - return !isAnimating || thread == null; + return !isStartedImpl() || (!drawablesEmpty && !isAnimating) ; } } Condition waitForStartedCondition = new WaitForStartedCondition(); public synchronized void stop() { - boolean started = null != thread; - if ( !started ) { - throw new GLException("Not started"); + if ( !isStartedImpl() ) { + throw new GLException("Stop: Not running (started "+isStartedImpl()+" (true))"); } - shouldStop = true; + stopIssued = true; notifyAll(); - finishLifecycleAction(waitForStoppedCondition); } private class WaitForStoppedCondition implements Condition { public boolean result() { - return thread != null; + return isStartedImpl(); } } Condition waitForStoppedCondition = new WaitForStoppedCondition(); public synchronized void pause() { - boolean started = null != thread; - if ( !started || !isAnimating || shouldPause ) { - throw new GLException("Invalid state (started "+started+" (true), animating "+isAnimating+" (true), paused "+shouldPause+" (false) )"); + if ( !isStartedImpl() || pauseIssued ) { + throw new GLException("Pause: Invalid state (started "+isStartedImpl()+" (true), paused "+pauseIssued+" (false) )"); + } + stateSync.lock(); + try { + pauseIssued = true; + } finally { + stateSync.unlock(); } - shouldPause = true; notifyAll(); - finishLifecycleAction(waitForPausedCondition); } private class WaitForPausedCondition implements Condition { public boolean result() { // end waiting if stopped as well - return (!isPaused || isAnimating) && thread != null; + return isAnimating && isStartedImpl(); } } Condition waitForPausedCondition = new WaitForPausedCondition(); public synchronized void resume() { - boolean started = null != thread; - if ( !started || !shouldPause ) { - throw new GLException("Invalid state (started "+started+" (true), paused "+shouldPause+" (true) )"); - } - shouldPause = false; + if ( !isStartedImpl() || !pauseIssued ) { + throw new GLException("Resume: Invalid state (started "+isStartedImpl()+" (true), paused "+pauseIssued+" (true) )"); + } + stateSync.lock(); + try { + pauseIssued = false; + } finally { + stateSync.unlock(); + } notifyAll(); - - finishLifecycleAction(waitForResumedCondition); + finishLifecycleAction(waitForResumeCondition); } - private class WaitForResumedCondition implements Condition { + private class WaitForResumeCondition implements Condition { public boolean result() { // end waiting if stopped as well - return (isPaused || !isAnimating) && thread != null; + return !drawablesEmpty && !isAnimating && isStartedImpl(); } } - Condition waitForResumedCondition = new WaitForResumedCondition(); - - protected final boolean getShouldPause() { - return shouldPause; - } - - protected final boolean getShouldStop() { - return shouldStop; - } - + Condition waitForResumeCondition = new WaitForResumeCondition(); } diff --git a/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java b/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java index bd5d1ad68..65d6745ed 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java +++ b/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java @@ -31,13 +31,18 @@ package com.jogamp.opengl.util; import com.jogamp.common.util.locks.RecursiveLock; import com.jogamp.opengl.impl.Debug; import java.util.ArrayList; -import java.util.List; import javax.media.opengl.GLAnimatorControl; import javax.media.opengl.GLAutoDrawable; import javax.media.opengl.GLProfile; /** - * Base implementation of GLAnimatorControl + * Base implementation of GLAnimatorControl
+ *

+ * The change synchronization is done via synchronized blocks on the AnimatorBase instance.
+ * Status get / set activity is synced with a RecursiveLock, used as a memory barrier.
+ * This is suitable, since all change requests are allowed to be expensive + * as they are not expected to be called at every frame. + *

*/ public abstract class AnimatorBase implements GLAnimatorControl { protected static final boolean DEBUG = Debug.debug("Animator"); @@ -45,20 +50,21 @@ public abstract class AnimatorBase implements GLAnimatorControl { private static int animatorCount = 0; public interface AnimatorImpl { - void display(AnimatorBase animator, boolean ignoreExceptions, boolean printExceptions); + void display(ArrayList drawables, boolean ignoreExceptions, boolean printExceptions); boolean skipWaitForCompletion(Thread thread); } protected ArrayList/**/ drawables = new ArrayList(); - protected RecursiveLock drawablesLock = new RecursiveLock(); + protected boolean drawablesEmpty; protected AnimatorImpl impl; protected String baseName; - protected Thread thread; + protected Thread animThread; protected boolean ignoreExceptions; protected boolean printExceptions; protected long startTime; protected long curTime; protected int totalFrames; + protected RecursiveLock stateSync = new RecursiveLock(); /** Creates a new, empty Animator. */ public AnimatorBase() { @@ -75,6 +81,7 @@ public abstract class AnimatorBase implements GLAnimatorControl { synchronized (Animator.class) { animatorCount++; baseName = baseName.concat("-"+animatorCount); + drawablesEmpty = true; } resetCounter(); } @@ -82,25 +89,54 @@ public abstract class AnimatorBase implements GLAnimatorControl { protected abstract String getBaseName(String prefix); public synchronized void add(GLAutoDrawable drawable) { - drawablesLock.lock(); + if(DEBUG) { + System.err.println("Animator add: "+drawable.hashCode()+" - "+Thread.currentThread()); + } + // drawables list may be in use by display + stateSync.lock(); try { - drawables.add(drawable); + ArrayList newDrawables = (ArrayList) drawables.clone(); + newDrawables.add(drawable); + drawables = newDrawables; + drawablesEmpty = drawables.size() == 0; drawable.setAnimator(this); } finally { - drawablesLock.unlock(); + stateSync.unlock(); } notifyAll(); + if(!impl.skipWaitForCompletion(animThread)) { + while(isStarted() && !isPaused() && !isAnimating()) { + try { + wait(); + } catch (InterruptedException ie) { } + } + } } public synchronized void remove(GLAutoDrawable drawable) { - drawablesLock.lock(); + if(DEBUG) { + System.err.println("Animator remove: "+drawable.hashCode()+" - "+Thread.currentThread()); + } + + // drawables list may be in use by display + stateSync.lock(); try { - drawables.remove(drawable); + ArrayList newDrawables = (ArrayList) drawables.clone(); + newDrawables.remove(drawable); + drawables = newDrawables; + drawablesEmpty = drawables.size() == 0; drawable.setAnimator(null); } finally { - drawablesLock.unlock(); + stateSync.unlock(); } notifyAll(); + if(!impl.skipWaitForCompletion(animThread)) { + while(isStarted() && drawablesEmpty && isAnimating()) { + try { + wait(); + } catch (InterruptedException ie) { } + } + } } /** Called every frame to cause redrawing of all of the @@ -109,20 +145,18 @@ public abstract class AnimatorBase implements GLAnimatorControl { components this Animator manages, in particular when multiple lightweight widgets are continually being redrawn. */ protected void display() { - impl.display(this, ignoreExceptions, printExceptions); + ArrayList dl; + stateSync.lock(); + try { + dl = drawables; + } finally { + stateSync.unlock(); + } + impl.display(dl, ignoreExceptions, printExceptions); curTime = System.currentTimeMillis(); totalFrames++; } - public List acquireDrawables() { - drawablesLock.lock(); - return drawables; - } - - public void releaseDrawables() { - drawablesLock.unlock(); - } - public long getCurrentTime() { return curTime; } @@ -139,16 +173,21 @@ public abstract class AnimatorBase implements GLAnimatorControl { return totalFrames; } + public final Thread getThread() { + stateSync.lock(); + try { + return animThread; + } finally { + stateSync.unlock(); + } + } + public synchronized void resetCounter() { startTime = System.currentTimeMillis(); // overwrite startTime to real init one curTime = startTime; totalFrames = 0; } - public final synchronized Thread getThread() { - return thread; - } - /** Sets a flag causing this Animator to ignore exceptions produced while redrawing the drawables. By default this flag is set to false, causing any exception thrown to halt the Animator. */ @@ -165,10 +204,6 @@ public abstract class AnimatorBase implements GLAnimatorControl { } public String toString() { - return getClass().getName()+"[started "+isStarted()+", animating "+isAnimating()+", paused "+isPaused()+", frames "+getTotalFrames()+"]"; + return getClass().getName()+"[started "+isStarted()+", animating "+isAnimating()+", paused "+isPaused()+", frames "+getTotalFrames()+", drawable "+drawables.size()+"]"; } - - protected abstract boolean getShouldPause(); - - protected abstract boolean getShouldStop(); } diff --git a/src/jogl/classes/com/jogamp/opengl/util/DefaultAnimatorImpl.java b/src/jogl/classes/com/jogamp/opengl/util/DefaultAnimatorImpl.java index 650d2e8ae..f98c4b488 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/DefaultAnimatorImpl.java +++ b/src/jogl/classes/com/jogamp/opengl/util/DefaultAnimatorImpl.java @@ -33,37 +33,32 @@ package com.jogamp.opengl.util; -import java.util.*; -import javax.media.opengl.*; +import java.util.ArrayList; +import javax.media.opengl.GLAutoDrawable; /** Abstraction to factor out AWT dependencies from the Animator's implementation in a way that still allows the FPSAnimator to pick up this behavior if desired. */ class DefaultAnimatorImpl implements AnimatorBase.AnimatorImpl { - public void display(AnimatorBase animator, + public void display(ArrayList drawables, boolean ignoreExceptions, boolean printExceptions) { - List drawables = animator.acquireDrawables(); - try { - for (int i=0; - animator.isAnimating() && !animator.getShouldStop() && !animator.getShouldPause() && i