From 99d205d0c5f047ef9a6a6e21f351abe415ed3b15 Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Thu, 28 Oct 2010 01:24:58 +0200 Subject: 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. --- .../com/jogamp/opengl/impl/GLDrawableHelper.java | 2 - .../com/jogamp/opengl/util/AWTAnimatorImpl.java | 2 +- .../classes/com/jogamp/opengl/util/Animator.java | 105 +++++++++++++-------- .../com/jogamp/opengl/util/AnimatorBase.java | 12 ++- .../com/jogamp/opengl/util/AnimatorImpl.java | 73 -------------- .../jogamp/opengl/util/DefaultAnimatorImpl.java | 73 ++++++++++++++ ...TestSwingAWTRobotUsageBeforeJOGLInitBug411.java | 25 +++-- .../test/junit/newt/TestGLWindows01NEWT.java | 3 +- .../junit/newt/TestGLWindows02NEWTAnimated.java | 4 + .../classes/com/jogamp/newt/opengl/GLWindow.java | 12 ++- 10 files changed, 176 insertions(+), 135 deletions(-) delete mode 100644 src/jogl/classes/com/jogamp/opengl/util/AnimatorImpl.java create mode 100644 src/jogl/classes/com/jogamp/opengl/util/DefaultAnimatorImpl.java 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/**/ 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/AnimatorImpl.java deleted file mode 100644 index 8f2715e0a..000000000 --- a/src/jogl/classes/com/jogamp/opengl/util/AnimatorImpl.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Copyright (c) 2008 Sun Microsystems, Inc. All Rights Reserved. - * Copyright (c) 2010 JogAmp Community. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * - Redistribution of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * - * - Redistribution in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * Neither the name of Sun Microsystems, Inc. or the names of - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * This software is provided "AS IS," without a warranty of any kind. ALL - * EXPRESS OR IMPLIED CONDITIONS, REPRESENTATIONS AND WARRANTIES, - * INCLUDING ANY IMPLIED WARRANTY OF MERCHANTABILITY, FITNESS FOR A - * PARTICULAR PURPOSE OR NON-INFRINGEMENT, ARE HEREBY EXCLUDED. SUN - * MICROSYSTEMS, INC. ("SUN") AND ITS LICENSORS SHALL NOT BE LIABLE FOR - * ANY DAMAGES SUFFERED BY LICENSEE AS A RESULT OF USING, MODIFYING OR - * DISTRIBUTING THIS SOFTWARE OR ITS DERIVATIVES. IN NO EVENT WILL SUN OR - * ITS LICENSORS BE LIABLE FOR ANY LOST REVENUE, PROFIT OR DATA, OR FOR - * DIRECT, INDIRECT, SPECIAL, CONSEQUENTIAL, INCIDENTAL OR PUNITIVE - * DAMAGES, HOWEVER CAUSED AND REGARDLESS OF THE THEORY OF LIABILITY, - * ARISING OUT OF THE USE OF OR INABILITY TO USE THIS SOFTWARE, EVEN IF - * SUN HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGES. - */ - -package com.jogamp.opengl.util; - -import java.util.*; -import javax.media.opengl.*; - -/** 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 AnimatorImpl { - public void display(AnimatorBase animator, - boolean ignoreExceptions, - boolean printExceptions) { - List drawables = animator.acquireDrawables(); - try { - for (int i=0; - animator.isAnimating() && !animator.getShouldStop() && !animator.getShouldPause() && i