Animator Fix/Cleanup

    - Fix AnimatorBase: Finally using 'com.jogamp.opengl.util.AWTAnimatorImpl',
      wrong FQN lead to never use it, hence deadlock in case of AWT usage (AWT-EDT).

    - Animator
        - remove volatile for synced state isAnimated
        - new state isPaused, since shouldPause give the wrong answer for isPaused()
        - Cleanup wait condition for lifecycle tasks (start/stop/pause/resume)

    - 'AnimatorImpl' -> 'DefaultAnimatorImpl implements AnimatorBase.AnimatorImpl'

    - 'AWTAnimatorImpl implements AnimatorBase.AnimatorImpl',
      hence no derivation of a complete overwritten AnimatorImpl needed.

    - GLWindow.destroyActionPreLock()
        - Stop animator if unrecoverable, else pause only.

Tests:
    - No explicit animator stop, hence tests implicit stop/pause
      by GLDrawableHelper and/or GLWindow.

5 files changed, 76 insertions, 47 deletions

diff --git a/src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java b/src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java
index e8df40b13..5b1858c41 100644
--- a/src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java
+++ b/src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java
@@ -320,11 +320,9 @@ public class GLDrawableHelper {
 
     if(null==initAction) {
         // disposal case
-
         if(!context.isCreated()) {
             throw new GLException("Dispose case (no init action given): Native context must be created: "+context);
         }
-
         GLAnimatorControl animCtrl =  getAnimator();
         if(null!=animCtrl && animCtrl.isAnimating()) {
             animCtrl.pause();
diff --git a/src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java b/src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java
index 9dd58bb57..fbad377ad 100644
--- a/src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java
+++ b/src/jogl/classes/com/jogamp/opengl/util/AWTAnimatorImpl.java
@@ -45,7 +45,7 @@ import javax.media.opengl.*;
     implementation in a way that still allows the FPSAnimator to pick
     up this behavior if desired. */
 
-class AWTAnimatorImpl extends AnimatorImpl {
+class AWTAnimatorImpl implements AnimatorBase.AnimatorImpl {
     // For efficient rendering of Swing components, in particular when
     // they overlap one another
     private List lightweights    = new ArrayList();
diff --git a/src/jogl/classes/com/jogamp/opengl/util/Animator.java b/src/jogl/classes/com/jogamp/opengl/util/Animator.java
index ecb3878ba..4f24b22a7 100644
--- a/src/jogl/classes/com/jogamp/opengl/util/Animator.java
+++ b/src/jogl/classes/com/jogamp/opengl/util/Animator.java
@@ -60,7 +60,8 @@ public class Animator extends AnimatorBase {
     protected ThreadGroup threadGroup;
     private Runnable runnable;
     private boolean runAsFastAsPossible;
-    protected volatile boolean isAnimating;
+    protected boolean isAnimating;
+    protected boolean isPaused;
     protected volatile boolean shouldPause;
     protected volatile boolean shouldStop;
 
@@ -118,6 +119,7 @@ public class Animator extends AnimatorBase {
 
                 synchronized (Animator.this) {
                     isAnimating = true;
+                    isPaused = false;
                     Animator.this.notifyAll();
                 }
 
@@ -127,11 +129,12 @@ public class Animator extends AnimatorBase {
                         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());
                                 }
-                                isAnimating = false;
-                                wasPaused = true;
                                 Animator.this.notifyAll();
                                 try {
                                     Animator.this.wait();
@@ -142,10 +145,11 @@ public class Animator extends AnimatorBase {
                                 startTime = System.currentTimeMillis();
                                 curTime   = startTime;
                                 totalFrames = 0;
+                                isAnimating = true;
+                                isPaused = false;
                                 if(DEBUG) {
                                     System.err.println("Animator resumed: "+Thread.currentThread());
                                 }
-                                isAnimating = true;
                                 Animator.this.notifyAll();
                             }
                         }
@@ -167,6 +171,7 @@ public class Animator extends AnimatorBase {
                     shouldPause = false;
                     thread = null;
                     isAnimating = false;
+                    isPaused = false;
                     Animator.this.notifyAll();
                 }
             }
@@ -182,7 +187,35 @@ public class Animator extends AnimatorBase {
     }
 
     public final synchronized boolean isPaused() {
-        return (thread != null) && shouldPause;
+        return (thread != null) && isPaused;
+    }
+
+    interface Condition {
+        /**
+         * @return true if branching (cont waiting, action), otherwise false
+         */
+        boolean result();
+    }
+
+    private synchronized void finishLifecycleAction(Condition condition) {
+        // It's hard to tell whether the thread which changes the lifecycle has
+        // 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);
+        if (doWait) {
+            while (condition.result()) {
+                try {
+                    wait();
+                } catch (InterruptedException ie) {
+                }
+            }
+        }
+        if(DEBUG) {
+            System.err.println("finishLifecycleAction(" + condition.getClass().getName() + "): finished - waited " + doWait +
+                    ", started: " + isStarted() +", animating: " + isAnimating() +
+                    ", paused: " + isPaused());
+        }
     }
 
     public synchronized void start() {
@@ -203,15 +236,15 @@ public class Animator extends AnimatorBase {
         }
         thread.start();
 
-        if (!impl.skipWaitForCompletion(thread)) {
-            while (!isAnimating && thread != null) {
-                try {
-                    wait();
-                } catch (InterruptedException ie) {
-                }
-            }
+        finishLifecycleAction(waitForStartedCondition);
+    }
+    private class WaitForStartedCondition implements Condition {
+        public boolean result() {
+            // cont waiting until actually is animating
+            return !isAnimating || thread == null;
         }
     }
+    Condition waitForStartedCondition = new WaitForStartedCondition();
 
     public synchronized void stop() {
         boolean started = null != thread;
@@ -221,19 +254,14 @@ public class Animator extends AnimatorBase {
         shouldStop = true;
         notifyAll();
 
-        // It's hard to tell whether the thread which calls stop() has
-        // dependencies on the Animator's internal thread. Currently we
-        // use a couple of heuristics to determine whether we should do
-        // the blocking wait().
-        if (!impl.skipWaitForCompletion(thread)) {
-            while (thread != null) {
-                try {
-                    wait();
-                } catch (InterruptedException ie) {
-                }
-            }
+        finishLifecycleAction(waitForStoppedCondition);
+    }
+    private class WaitForStoppedCondition implements Condition {
+        public boolean result() {
+            return thread != null;
         }
     }
+    Condition waitForStoppedCondition = new WaitForStoppedCondition();
 
     public synchronized void pause() {
         boolean started = null != thread;
@@ -243,34 +271,33 @@ public class Animator extends AnimatorBase {
         shouldPause = true;
         notifyAll();
 
-        if (!impl.skipWaitForCompletion(thread)) {
-            while (isAnimating && thread != null) {
-                try {
-                    wait();
-                } catch (InterruptedException ie) {
-                }
-            }
+        finishLifecycleAction(waitForPausedCondition);
+    }
+    private class WaitForPausedCondition implements Condition {
+        public boolean result() {
+            // end waiting if stopped as well
+            return (!isPaused || isAnimating) && thread != null;
         }
     }
+    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;
         notifyAll();
 
-        if (!impl.skipWaitForCompletion(thread)) {
-            while (!isAnimating && thread != null) {
-                try {
-                    wait();
-                } catch (InterruptedException ie) {
-                }
-            }
+        finishLifecycleAction(waitForResumedCondition);
+    }
+    private class WaitForResumedCondition implements Condition {
+        public boolean result() {
+            // end waiting if stopped as well
+            return (isPaused || !isAnimating) && thread != null;
         }
-        resetCounter();
     }
+    Condition waitForResumedCondition = new WaitForResumedCondition();
 
     protected final boolean getShouldPause() {
         return shouldPause;
diff --git a/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java b/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java
index 24eee1875..bd5d1ad68 100644
--- a/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java
+++ b/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java
@@ -31,7 +31,6 @@ 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.Iterator;
 import java.util.List;
 import javax.media.opengl.GLAnimatorControl;
 import javax.media.opengl.GLAutoDrawable;
@@ -45,6 +44,11 @@ public abstract class AnimatorBase implements GLAnimatorControl {
 
     private static int animatorCount = 0;
 
+    public interface AnimatorImpl {
+        void display(AnimatorBase animator, boolean ignoreExceptions, boolean printExceptions);
+        boolean skipWaitForCompletion(Thread thread);
+    }
+
     protected ArrayList/*<GLAutoDrawable>*/ drawables = new ArrayList();
     protected RecursiveLock drawablesLock = new RecursiveLock();
     protected AnimatorImpl impl;
@@ -60,12 +64,12 @@ public abstract class AnimatorBase implements GLAnimatorControl {
     public AnimatorBase() {
         if(GLProfile.isAWTAvailable()) {
             try {
-                impl = (AnimatorImpl) Class.forName("com.jogamp.opengl.util.awt.AWTAnimatorImpl").newInstance();
+                impl = (AnimatorImpl) Class.forName("com.jogamp.opengl.util.AWTAnimatorImpl").newInstance();
                 baseName = "AWTAnimator";
-            } catch (Exception e) { }
+            } catch (Exception e) { e.printStackTrace(); }
         }
         if(null==impl) {
-            impl = new AnimatorImpl();
+            impl = new DefaultAnimatorImpl();
             baseName = "Animator";
         }
         synchronized (Animator.class) {
diff --git a/src/jogl/classes/com/jogamp/opengl/util/AnimatorImpl.java b/src/jogl/classes/com/jogamp/opengl/util/DefaultAnimatorImpl.java
index 8f2715e0a..650d2e8ae 100644
--- a/src/jogl/classes/com/jogamp/opengl/util/AnimatorImpl.java
+++ b/src/jogl/classes/com/jogamp/opengl/util/DefaultAnimatorImpl.java
@@ -40,7 +40,7 @@ import javax.media.opengl.*;
     implementation in a way that still allows the FPSAnimator to pick
     up this behavior if desired. */
 
-class AnimatorImpl {
+class DefaultAnimatorImpl implements AnimatorBase.AnimatorImpl {
     public void display(AnimatorBase animator,
                         boolean ignoreExceptions,
                         boolean printExceptions) {