Fix Bug 572 (2nd time): GLCanvas.validateGLDrawable() @ display() and reshape() ; GLCanvas.reshape() only if drawble valid ; GLCanvas.validateGLDrawable() also test isDisplayable() ; Fix size validation ; resizeOffscreenDrawable(..) don't validate 'safe' size 1x1

- GLCanvas.validateGLDrawable() @ display() and reshape()
  To help users using GLCanvas w/ having a realized GLCanvas/Drawable,
  validateGLDrawable() is also called at reshape().
  This shall ensure a valid drawable after even a non AWT-EDT issued first setVisible().

- GLCanvas.reshape() only if drawble valid
  Otherwise offscreen reshape attempts would happen even on unrealized drawable,
  which is not necessary.

- GLCanvas.validateGLDrawable() also test isDisplayable()
  To make sure the native peer is valid, also test isDisplayable()

- Fix size validation
  Since we have experienced odd size like 0 x -41
  test each component, i.e. 0 < width && 0 < height.
  This is done through all JOGL/NEWT components.

- resizeOffscreenDrawable(..) don't validate 'safe' size 1x1
  In case method is called w/ odd size, i.e. 0 x -41,
  the safe size 1x1 is used. However, we cannot validate this size.
  Dump WARNING if odd size is detected.

9 files changed, 188 insertions, 45 deletions

diff --git a/make/scripts/tests.sh b/make/scripts/tests.sh
index affa130fb..ed2e238fb 100755
--- a/make/scripts/tests.sh
+++ b/make/scripts/tests.sh
@@ -137,7 +137,7 @@ function jrun() {
     #D_ARGS="-Xprof"
     #D_ARGS="-Djogl.debug.Animator"
     #D_ARGS="-Dnativewindow.debug=all"
-    #D_ARGS="-Djogl.debug.GLCanvas"
+    D_ARGS="-Djogl.debug.GLCanvas"
     #D_ARGS="-Djogl.debug.GLContext -Dnativewindow.debug.X11Util.XSync"
     #D_ARGS="-Dnativewindow.debug.X11Util.XSync -Dnativewindow.debug.ToolkitLock.TraceLock"
     #D_ARGS="-Dnativewindow.debug.NativeWindow"
@@ -317,6 +317,7 @@ function testawtswt() {
 #testawt javax.media.opengl.awt.GLCanvas $*
 #testawt com.jogamp.opengl.test.junit.jogl.acore.TestMainVersionGLCanvasAWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.awt.TestBug551AWT $*
+testawt com.jogamp.opengl.test.junit.jogl.awt.TestBug572AWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.awt.TestBug611AWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.awt.TestAWT01GLn $*
 #testawt com.jogamp.opengl.test.junit.jogl.acore.TestAWTCloseX11DisplayBug565 $*
@@ -355,7 +356,8 @@ function testawtswt() {
 #testawtswt com.jogamp.opengl.test.junit.jogl.swt.TestSWTEclipseGLCanvas01GLn $*
 #testawtswt com.jogamp.opengl.test.junit.jogl.swt.TestSWTAccessor03AWTGLn $*
 #testawtswt com.jogamp.opengl.test.junit.jogl.swt.TestSWTJOGLGLCanvas01GLn $*
-testawtswt com.jogamp.opengl.test.junit.jogl.swt.TestNewtCanvasSWTGLn $*
+#testawtswt com.jogamp.opengl.test.junit.jogl.swt.TestNewtCanvasSWTGLn $*
+#testnoawt com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestGearsES2NEWT $*
 
 #
 # newt.awt (testawt)
diff --git a/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java b/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java
index 06114431a..24971ff97 100644
--- a/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java
+++ b/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java
@@ -389,7 +389,7 @@ public class GLCanvas extends Canvas implements GLAutoDrawable {
           return false;
       }
       final Rectangle nClientArea = clientArea;
-      if(0 == nClientArea.width * nClientArea.height) {
+      if(0 >= nClientArea.width || 0 >= nClientArea.height) {
           return false;
       }
                
diff --git a/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java b/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
index 335322be9..4bdae7135 100644
--- a/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
+++ b/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
@@ -440,6 +440,12 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing
 
   @Override
   public void display() {
+    if( !validateGLDrawable() ) {
+        if(DEBUG) {
+            System.err.println(getThreadName()+": Info: GLCanvas display - skipped GL render, drawable not valid yet");
+        }
+        return; // not yet available ..
+    }
     Threading.invoke(true, displayOnEDTAction, getTreeLock());
     awtWindowClosingProtocol.addClosingListenerOneShot();
   }
@@ -489,7 +495,7 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing
                    (int) ((getHeight() + bounds.getHeight()) / 2));
       return;
     }
-    if( ! this.helper.isAnimatorAnimating() ) {
+    if( ! this.helper.isExternalAnimatorAnimating() ) {
         display();
     }
   }
@@ -570,30 +576,29 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing
         if( _drawable.isRealized() ) {
             return true;
         }
-        if (!Beans.isDesignTime() &&
-             0 < _drawable.getWidth() * _drawable.getHeight() ) {
-            // make sure drawable realization happens on AWT EDT, due to AWTTree lock
-            AWTEDTExecutor.singleton.invoke(getTreeLock(), true, setRealizedOnEDTAction);
-            if( _drawable.isRealized() ) {
-                sendReshape=true; // ensure a reshape is being send ..
-                if(DEBUG) {
-                    System.err.println(getThreadName()+": Realized Drawable: "+_drawable.toString());
-                    Thread.dumpStack();
+        final RecursiveLock _lock = lock;
+        _lock.lock();
+        try {            
+            if (!Beans.isDesignTime() && isDisplayable() &&
+                 0 < _drawable.getWidth() && 0 < _drawable.getHeight() ) {
+                // make sure drawable realization happens on AWT EDT, due to AWTTree lock
+                AWTEDTExecutor.singleton.invoke(getTreeLock(), true, setRealizedOnEDTAction);
+                if( _drawable.isRealized() ) {
+                    sendReshape=true; // ensure a reshape is being send ..
+                    if(DEBUG) {
+                        System.err.println(getThreadName()+": Realized Drawable: "+_drawable.toString());
+                        Thread.dumpStack();
+                    }
+                    return true;
                 }
-                return true;
             }
+        } finally {
+            _lock.unlock();
         }
     }
     return false;
   }
-  private Runnable setRealizedOnEDTAction = new Runnable() {
-      @Override
-    public void run() {
-          final GLDrawable _drawable = drawable;
-          if ( null != _drawable && 0 < _drawable.getWidth() * _drawable.getHeight() ) {
-              _drawable.setRealized(true);
-          }
-      } };
+  private Runnable setRealizedOnEDTAction = new Runnable() { public void run() { drawable.setRealized(true); } };
 
   /** <p>Overridden to track when this component is removed from a
       container. Subclasses which override this method must call
@@ -640,11 +645,12 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing
     synchronized (getTreeLock()) { // super.reshape(..) claims tree lock, so we do extend it's lock over reshape
         super.reshape(x, y, width, height);
         
-        GLDrawableImpl _drawable = drawable;
-        if( null != _drawable ) {
-            if(DEBUG) {
-                System.err.println("GLCanvas.sizeChanged: ("+Thread.currentThread().getName()+"): "+width+"x"+height+" - surfaceHandle 0x"+Long.toHexString(getNativeSurface().getSurfaceHandle()));
-            }
+        if(DEBUG) {
+            System.err.println("GLCanvas.sizeChanged: ("+Thread.currentThread().getName()+"): "+width+"x"+height+" - surfaceHandle 0x"+Long.toHexString(getNativeSurface().getSurfaceHandle()));
+            // Thread.dumpStack();
+        }            
+        if( validateGLDrawable() ) {
+            final GLDrawableImpl _drawable = drawable;
             if( ! _drawable.getChosenGLCapabilities().isOnscreen() ) {
                 final RecursiveLock _lock = lock;
                 _lock.lock();
@@ -984,11 +990,7 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing
         final RecursiveLock _lock = lock;
         _lock.lock();
         try {            
-            if( validateGLDrawable() ) {
-                helper.invokeGL(drawable, context, displayAction, initAction);
-            } else if(DEBUG) {
-                System.err.println(getThreadName()+": Info: GLCanvas display - skipped GL render, drawable not valid yet");
-            }
+            helper.invokeGL(drawable, context, displayAction, initAction);
         } finally {
             _lock.unlock();
         }
diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
index 13c387231..d4ff9702c 100644
--- a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
+++ b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
@@ -241,9 +241,14 @@ public class GLDrawableHelper {
       if (NativeSurface.LOCK_SURFACE_NOT_READY >= lockRes) {
           throw new NativeWindowException("Could not lock surface of drawable: "+drawable);
       }
+      boolean validateSize = true;
       try {
-          if(0>=newWidth)  { newWidth = 1; }
-          if(0>=newHeight) { newHeight = 1; }
+          if(DEBUG && ( 0>=newWidth || 0>=newHeight) ) {
+              System.err.println("WARNING: Odd size detected: "+newWidth+"x"+newHeight+", using safe size 1x1. Drawable "+drawable);
+              Thread.dumpStack();
+          }
+          if(0>=newWidth)  { newWidth = 1; validateSize=false; }
+          if(0>=newHeight) { newHeight = 1; validateSize=false; }
           // propagate new size 
           if(ns instanceof ProxySurface) {
               final ProxySurface ps = (ProxySurface) ns;
@@ -266,7 +271,7 @@ public class GLDrawableHelper {
       } finally {
           ns.unlockSurface();
       }
-      if(drawable.getWidth() != newWidth || drawable.getHeight() != newHeight) {
+      if( validateSize && ( drawable.getWidth() != newWidth || drawable.getHeight() != newHeight ) ) {
           throw new InternalError("Incomplete resize operation: expected "+newWidth+"x"+newHeight+", has: "+drawable);
       }
       return drawable;
diff --git a/src/newt/classes/com/jogamp/newt/Window.java b/src/newt/classes/com/jogamp/newt/Window.java
index 78e2abc6e..cc42465f1 100644
--- a/src/newt/classes/com/jogamp/newt/Window.java
+++ b/src/newt/classes/com/jogamp/newt/Window.java
@@ -129,7 +129,7 @@ public interface Window extends NativeWindow, WindowClosingProtocol {
      * <pre>
      * if ( 0 == windowHandle && visible ) {
      *   this.visible = visible;
-     *   if( 0 &lt; width*height ) {
+     *   if( 0 &lt; width && 0 &lt; height ) {
      *     createNative();
      *   }
      * } else if ( this.visible != visible ) {
@@ -171,9 +171,9 @@ public interface Window extends NativeWindow, WindowClosingProtocol {
      * <p>
      * Zero size semantics are respected, see {@link #setVisible(boolean)}:<br>
      * <pre>
-     * if ( 0 != windowHandle && 0 &ge; width*height && visible ) {
+     * if ( visible && 0 != windowHandle && ( 0 &ge; width || 0 &ge; height ) ) {
      *   setVisible(false);
-     * } else if ( 0 == windowHandle && 0 &lt; width*height && visible ) {
+     * } else if ( visible && 0 == windowHandle && 0 &lt; width && 0 &lt; height ) {
      *   setVisible(true);
      * } else {
      *   // as expected ..
diff --git a/src/newt/classes/com/jogamp/newt/swt/NewtCanvasSWT.java b/src/newt/classes/com/jogamp/newt/swt/NewtCanvasSWT.java
index 74611706a..525225804 100644
--- a/src/newt/classes/com/jogamp/newt/swt/NewtCanvasSWT.java
+++ b/src/newt/classes/com/jogamp/newt/swt/NewtCanvasSWT.java
@@ -157,7 +157,7 @@ public class NewtCanvasSWT extends Canvas implements WindowClosingProtocol {
         }
         updateSizeCheck();
         final Rectangle nClientArea = clientArea;
-        if(0 == nClientArea.width * nClientArea.height) {
+        if(0 >= nClientArea.width || 0 >= nClientArea.height) {        
             return false;
         }
         
diff --git a/src/newt/classes/jogamp/newt/WindowImpl.java b/src/newt/classes/jogamp/newt/WindowImpl.java
index 79770189b..c94ce286b 100644
--- a/src/newt/classes/jogamp/newt/WindowImpl.java
+++ b/src/newt/classes/jogamp/newt/WindowImpl.java
@@ -797,10 +797,10 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer
                         System.err.println("Window setSize: START "+getWidth()+"x"+getHeight()+" -> "+width+"x"+height+", fs "+fullscreen+", windowHandle "+toHexString(windowHandle)+", visible "+visible);
                     }
                     int visibleAction; // 0 nop, 1 invisible, 2 visible (create)
-                    if ( isNativeValid() && 0>=width*height && visible ) {
+                    if ( visible && isNativeValid() && ( 0 >= width || 0 >= height ) ) {
                         visibleAction=1; // invisible
                         defineSize(0, 0);
-                    } else if ( !isNativeValid() && 0<width*height && visible ) {
+                    } else if ( visible && !isNativeValid() && 0 < width && 0 < height ) {
                         visibleAction = 2; // visible (create)
                         defineSize(width, height);
                     } else if ( isNativeValid() ) {
@@ -1050,7 +1050,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer
                                     setScreen( (ScreenImpl) newScreen );
                                 }
                             }
-                            if( 0<width*height ) {
+                            if( 0 < width && 0 < height ) {
                                 operation = ReparentOperation.ACTION_NATIVE_CREATION;
                             } else {
                                 operation = ReparentOperation.ACTION_NATIVE_CREATION_PENDING;
@@ -1089,7 +1089,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer
                     } else if( !isNativeValid() || forceDestroyCreate ) {
                         // Destroy this window and mark it for [pending] creation.
                         destroy();
-                        if( 0<width*height ) {
+                        if( 0 < width && 0 < height ) {
                             operation = ReparentOperation.ACTION_NATIVE_CREATION;
                         } else {
                             operation = ReparentOperation.ACTION_NATIVE_CREATION_PENDING;
diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/awt/TestBug572AWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/awt/TestBug572AWT.java
new file mode 100644
index 000000000..b3abc4137
--- /dev/null
+++ b/src/test/com/jogamp/opengl/test/junit/jogl/awt/TestBug572AWT.java
@@ -0,0 +1,127 @@
+/**
+ * Copyright 2012 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:
+ * 
+ *    1. Redistributions of source code must retain the above copyright notice, this list of
+ *       conditions and the following disclaimer.
+ * 
+ *    2. Redistributions 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.
+ * 
+ * THIS SOFTWARE IS PROVIDED BY JogAmp Community ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL JogAmp Community OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ * 
+ * The views and conclusions contained in the software and documentation are those of the
+ * authors and should not be interpreted as representing official policies, either expressed
+ * or implied, of JogAmp Community.
+ */
+ 
+package com.jogamp.opengl.test.junit.jogl.awt;
+
+import java.awt.Window;
+import java.lang.reflect.InvocationTargetException;
+
+import javax.media.opengl.GLCapabilities;
+import javax.media.opengl.GLProfile;
+import javax.media.opengl.awt.GLCanvas;
+import javax.swing.JFrame;
+import javax.swing.SwingUtilities;
+
+import junit.framework.Assert;
+
+import org.junit.Assume;
+import org.junit.Test;
+
+import com.jogamp.opengl.test.junit.jogl.demos.es2.GearsES2;
+import com.jogamp.opengl.test.junit.util.AWTRobotUtil;
+import com.jogamp.opengl.test.junit.util.UITestCase;
+
+/**
+ * Tests context creation + display on various kinds of Window implementations.
+ */
+public class TestBug572AWT extends UITestCase {
+
+    protected void runTestGL() throws InterruptedException, InvocationTargetException {
+        final Window window = new JFrame(this.getSimpleTestName(" - "));
+        final GLCapabilities caps = new GLCapabilities(GLProfile.getGL2ES2());        
+        final GLCanvas glCanvas = new GLCanvas(caps);
+        final SnapshotGLEventListener snapshooter = new SnapshotGLEventListener();
+        snapshooter.setMakeSnapshotAlways(true);
+        glCanvas.addGLEventListener(new GearsES2());
+        glCanvas.addGLEventListener(snapshooter);
+        window.add(glCanvas);
+
+        // Revalidate size/layout.
+        // Always validate if component added/removed.
+        // Ensure 1st paint of GLCanvas will have a valid size, hence drawable gets created.
+        window.setSize(512, 512);
+        window.validate();
+
+        window.setVisible(true);
+        System.err.println("XXXX-0 "+glCanvas.getDelegatedDrawable().isRealized()+", "+glCanvas);
+        
+        // Immediately displayable after issuing initial setVisible(true) .. even not within AWT-EDT ?
+        Assert.assertTrue("GLCanvas didn't become displayable", glCanvas.isDisplayable());
+        Assert.assertTrue("GLCanvas didn't become realized", glCanvas.isRealized());
+        
+        // Would be required if not immediately displayable ...   
+        // Assert.assertTrue("GLCanvas didn't become displayable and realized", AWTRobotUtil.waitForRealized(glCanvas, true));
+        // System.err.println("XXXX-1 "+glCanvas.getDelegatedDrawable().isRealized()+", "+glCanvas);
+        
+        // The AWT-EDT reshape/repaint events happen offthread later ..
+        System.err.println("XXXX-1 reshapeCount "+snapshooter.getReshapeCount());
+        System.err.println("XXXX-1 displayCount "+snapshooter.getDisplayCount());
+        
+        // Wait unitl AWT-EDT has issued reshape/repaint
+        for (int wait=0; wait<AWTRobotUtil.POLL_DIVIDER &&
+                         ( 0 == snapshooter.getReshapeCount() || 0 == snapshooter.getDisplayCount() ); 
+             wait++) {
+            Thread.sleep(AWTRobotUtil.TIME_SLICE);
+        }
+        System.err.println("XXXX-2 reshapeCount "+snapshooter.getReshapeCount());
+        System.err.println("XXXX-2 displayCount "+snapshooter.getDisplayCount());
+        
+        Assert.assertTrue("GLCanvas didn't reshape", snapshooter.getReshapeCount()>0);
+        Assert.assertTrue("GLCanvas didn't display", snapshooter.getDisplayCount()>0);
+        
+        // After initial 'setVisible(true)' all AWT manipulation needs to be done
+        // via the AWT EDT, according to the AWT spec.
+
+        Runnable cleanup = new Runnable() {
+            public void run() {
+                System.err.println("cleaning up...");
+                window.setVisible(false);
+                try {
+                    window.removeAll();
+                } catch (Throwable t) {
+                    Assume.assumeNoException(t);
+                    t.printStackTrace();
+                }
+                window.dispose();
+            }
+
+        };
+
+        // AWT / Swing on EDT..
+        SwingUtilities.invokeAndWait(cleanup);
+    }
+
+    @Test
+    public void test01() throws InterruptedException, InvocationTargetException {
+        runTestGL();
+    }
+
+    public static void main(String args[]) {
+        org.junit.runner.JUnitCore.main(TestBug572AWT.class.getName());
+    }
+}
diff --git a/src/test/com/jogamp/opengl/test/junit/util/UITestCase.java b/src/test/com/jogamp/opengl/test/junit/util/UITestCase.java
index c31555969..741014502 100644
--- a/src/test/com/jogamp/opengl/test/junit/util/UITestCase.java
+++ b/src/test/com/jogamp/opengl/test/junit/util/UITestCase.java
@@ -206,6 +206,7 @@ public abstract class UITestCase {
     public class SnapshotGLEventListener implements GLEventListener {
         private final GLReadBufferUtil screenshot;
         private volatile boolean makeShot = false;
+        private volatile boolean makeShotAlways = false;
         private volatile int displayCount=0;
         private volatile int reshapeCount=0;
         public SnapshotGLEventListener(GLReadBufferUtil screenshot) {
@@ -214,13 +215,16 @@ public abstract class UITestCase {
         public SnapshotGLEventListener() {
             this.screenshot = new GLReadBufferUtil(false, false);
         }
+        public int getDisplayCount() { return displayCount; }
+        public int getReshapeCount() { return reshapeCount; }
 
         public void init(GLAutoDrawable drawable) {}
         public void dispose(GLAutoDrawable drawable) {}
         public void display(GLAutoDrawable drawable) {
             final GL gl = drawable.getGL();
-            System.err.println(Thread.currentThread().getName()+": ** display: "+displayCount+": "+drawable.getWidth()+"x"+drawable.getHeight()+", makeShot "+makeShot);
-            if(makeShot) {
+            final boolean _makeShot = makeShot || makeShotAlways;
+            System.err.println(Thread.currentThread().getName()+": ** display: "+displayCount+": "+drawable.getWidth()+"x"+drawable.getHeight()+", makeShot "+_makeShot);
+            if(_makeShot) {
                 makeShot=false;
                 snapshot(displayCount, null, gl, screenshot, TextureIO.PNG, null);
             }
@@ -233,6 +237,9 @@ public abstract class UITestCase {
         public void setMakeSnapshot() {
             makeShot=true;
         }
+        public void setMakeSnapshotAlways(boolean v) {
+            makeShotAlways=v;
+        }
     };
     
 }