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.

9 files changed, 104 insertions, 63 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) {
diff --git a/src/junit/com/jogamp/test/junit/jogl/awt/TestSwingAWTRobotUsageBeforeJOGLInitBug411.java b/src/junit/com/jogamp/test/junit/jogl/awt/TestSwingAWTRobotUsageBeforeJOGLInitBug411.java
index e77f4f8b0..fd1a15e52 100644
--- a/src/junit/com/jogamp/test/junit/jogl/awt/TestSwingAWTRobotUsageBeforeJOGLInitBug411.java
+++ b/src/junit/com/jogamp/test/junit/jogl/awt/TestSwingAWTRobotUsageBeforeJOGLInitBug411.java
@@ -146,6 +146,7 @@ public class TestSwingAWTRobotUsageBeforeJOGLInitBug411 extends UITestCase {
         });
         frame.setContentPane(panel);
         frame.setSize(512, 512);
+        frame.setLocation(0, 0);
         frame.pack();
         frame.setVisible(true);
 
@@ -216,9 +217,6 @@ public class TestSwingAWTRobotUsageBeforeJOGLInitBug411 extends UITestCase {
         colorPanel.setBackground(Color.blue);
         drawable.addGLEventListener(new SwingGLAction());
 
-        Animator animator = new Animator(drawable);
-        animator.start();
-
         Point p0 = canvas.getLocationOnScreen();
         Rectangle r0 = canvas.getBounds();
         robot.mouseMove( (int) ( p0.getX() + .5 ) ,
@@ -243,7 +241,6 @@ public class TestSwingAWTRobotUsageBeforeJOGLInitBug411 extends UITestCase {
                 frame.pack();
             }
         });
-        Assert.assertEquals(false, animator.isAnimating());
     }
 
     @Test
@@ -257,19 +254,22 @@ public class TestSwingAWTRobotUsageBeforeJOGLInitBug411 extends UITestCase {
         win0.setSize(100,100);
         win0.setVisible(true);
         Screen screen = win0.getScreen();
-        win0.setPosition(screen.getWidth()-150, screen.getHeight()-150);
+        win0.setPosition(screen.getWidth()-150, 0);
         win0.addGLEventListener(new Gears());
         Animator anim0 = new Animator(win0);
         anim0.start();
 
-        NewtCanvasAWT newtCanvasAWT = new NewtCanvasAWT(GLWindow.create(caps));
-
-        runTestGL(newtCanvasAWT, (GLAutoDrawable)newtCanvasAWT.getNEWTChild());
+        GLWindow win1 = GLWindow.create(caps);
+        NewtCanvasAWT newtCanvasAWT = new NewtCanvasAWT(win1);
+        Animator anim1 = new Animator(win1);
+        anim1.start();
+        runTestGL(newtCanvasAWT, win1);
 
         win0.destroy(true);
         Assert.assertEquals(false, anim0.isAnimating());
 
         newtCanvasAWT.destroy(true);
+        Assert.assertEquals(false, anim1.isAnimating());
 
         System.err.println("TestSwingAWTRobotUsageBeforeJOGLInitBug411.test01NewtCanvasAWT(): End");
     }
@@ -284,17 +284,22 @@ public class TestSwingAWTRobotUsageBeforeJOGLInitBug411 extends UITestCase {
         win0.setSize(100,100);
         win0.setVisible(true);
         Screen screen = win0.getScreen();
-        win0.setPosition(screen.getWidth()-150, screen.getHeight()-150);
+        win0.setPosition(screen.getWidth()-150, 0);
         win0.addGLEventListener(new Gears());
         Animator anim0 = new Animator(win0);        
         anim0.start();
 
         GLCanvas glCanvas = new GLCanvas(caps);
-
+        Animator anim1 = new Animator(glCanvas);
+        anim1.start();
         runTestGL(glCanvas, glCanvas);
 
+        Thread.sleep(100); // wait 1/10Hz to allow animation to be paused
+        Assert.assertEquals(false, anim1.isAnimating());
+
         win0.destroy(true);
         Assert.assertEquals(false, anim0.isAnimating());
+        
         System.err.println("TestSwingAWTRobotUsageBeforeJOGLInitBug411.test02GLCanvas(): End");
     }
 
diff --git a/src/junit/com/jogamp/test/junit/newt/TestGLWindows01NEWT.java b/src/junit/com/jogamp/test/junit/newt/TestGLWindows01NEWT.java
index f136b2a3c..7c06879b2 100644
--- a/src/junit/com/jogamp/test/junit/newt/TestGLWindows01NEWT.java
+++ b/src/junit/com/jogamp/test/junit/newt/TestGLWindows01NEWT.java
@@ -266,8 +266,7 @@ public class TestGLWindows01NEWT extends UITestCase {
         Assert.assertNotNull(window2);
 
         Assert.assertEquals(1,Display.getActiveDisplayNumber());
-
-        Assert.assertEquals(2,display.getReferenceCount());
+        Assert.assertEquals(1,display.getReferenceCount());
         Assert.assertEquals(true,display.isNativeValid());
         Assert.assertNotNull(display.getEDTUtil());
         Assert.assertEquals(true,display.getEDTUtil().isRunning());
diff --git a/src/junit/com/jogamp/test/junit/newt/TestGLWindows02NEWTAnimated.java b/src/junit/com/jogamp/test/junit/newt/TestGLWindows02NEWTAnimated.java
index 12cc7875a..56730d42f 100644
--- a/src/junit/com/jogamp/test/junit/newt/TestGLWindows02NEWTAnimated.java
+++ b/src/junit/com/jogamp/test/junit/newt/TestGLWindows02NEWTAnimated.java
@@ -161,9 +161,11 @@ public class TestGLWindows02NEWTAnimated extends UITestCase {
         Assert.assertNotNull(screen);
         GLWindow window1 = createWindow(screen, caps, width, height, true /* onscreen */, false /* undecorated */);
         Assert.assertNotNull(window1);
+        window1.setPosition(0, 0);
 
         GLWindow window2 = createWindow(screen, caps, width-10, height-10, true /* onscreen */, false /* undecorated */);
         Assert.assertNotNull(window2);
+        window2.setPosition(screen.getWidth()-width, 0);
 
         Animator animator1 = new Animator(window1);
         animator1.start();
@@ -194,11 +196,13 @@ public class TestGLWindows02NEWTAnimated extends UITestCase {
         Assert.assertNotNull(screen1);
         GLWindow window1 = createWindow(screen1, caps, width, height, true /* onscreen */, false /* undecorated */);
         Assert.assertNotNull(window1);
+        window1.setPosition(0, 0);
 
         Screen screen2  = NewtFactory.createScreen(display2, 0); // screen 0
         Assert.assertNotNull(screen2);
         GLWindow window2 = createWindow(screen2, caps, width-10, height-10, true /* onscreen */, false /* undecorated */);
         Assert.assertNotNull(window2);
+        window2.setPosition(screen2.getWidth()-width, 0);
 
         Animator animator1 = new Animator(window1);
         animator1.start();
diff --git a/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java b/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java
index f6901e54a..7836bec68 100644
--- a/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java
+++ b/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java
@@ -280,10 +280,14 @@ public class GLWindow implements GLAutoDrawable, Window {
         /** Window.LifecycleHook */
         public synchronized void destroyActionPreLock(boolean unrecoverable) {
             GLAnimatorControl animator = GLWindow.this.getAnimator();
-            // since we have no 'recreation model' for dispose here yet,
-            // we simply stop the animator if started.
-            if(null!=animator && animator.isStarted()) {
-                animator.stop();
+            if(null!=animator) {
+                if(unrecoverable) {
+                    if(animator.isStarted()) {
+                        animator.stop();
+                    }
+                } else if(animator.isAnimating()) {
+                    animator.pause();
+                }
             }
         }