Bug 1206 - Security: Clear exposed framebuffer after creation and before visibility

Experimenting w/ no GLEventListener attached to an GLAutoDrawable,
e.g. GLWindow (onscreen), GLJPanel (fbo offscreen),
indeed on some GL implementations the default framebuffer is uninitialized
and hence shows garbage.

GLDrawableHelper.setViewportAndClear(..)
 - Clear framebuffer after setting viewport
 - Called from:
   - public final void init(..)
   - public final void reshape(..)

 - Method is used independent of GLEventListener,
   hence this simplifies implementation: removes 'setViewport' criteria
   for init, display, reshape: it is always performed!

Note: We only attempt to help against leaking un-initialized framebuffer content
not against user-app faults, we do not clear a 2nd-buffer (double-buffering).

Note: We may still be late at resize, i.e. small noisy flickering might be visible.
This might be due to lack of proper vsync.

5 files changed, 133 insertions, 41 deletions

diff --git a/make/scripts/tests.sh b/make/scripts/tests.sh
index 03571a9d6..e9af09ea3 100644
--- a/make/scripts/tests.sh
+++ b/make/scripts/tests.sh
@@ -437,7 +437,7 @@ function testawtswt() {
 #
 # HiDPI
 #
-#testnoawt com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestGearsES2NEWT $*
+testnoawt com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestGearsES2NEWT $*
 #testnoawt com.jogamp.opengl.test.junit.jogl.demos.es2.newt.TestGearsES2SimpleNEWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.demos.es2.awt.TestGearsES2GLJPanelAWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.demos.es2.awt.TestGearsES2AWT $*
@@ -707,7 +707,7 @@ function testawtswt() {
 # <END>
 #
 
-testawt com.jogamp.opengl.test.junit.jogl.awt.TestBug1245JTabbedPanelCrashAWT $*
+#testawt com.jogamp.opengl.test.junit.jogl.awt.TestBug1245JTabbedPanelCrashAWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.awt.TestBug675BeansInDesignTimeAWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.awt.TestBug551AWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.awt.TestBug572AWT $*
diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
index 6bc93bf36..e4b5eae5c 100644
--- a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
+++ b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
@@ -76,6 +76,8 @@ public class GLDrawableHelper {
   }
 
   protected static final boolean DEBUG = GLDrawableImpl.DEBUG;
+  private static final boolean DEBUG_SETCLEAR = GLContext.DEBUG_GL || DEBUG;
+
   private final Object listenersLock = new Object();
   private final ArrayList<GLEventListener> listeners = new ArrayList<GLEventListener>();
   private final HashSet<GLEventListener> listenersToBeInit = new HashSet<GLEventListener>();
@@ -638,10 +640,10 @@ public class GLDrawableHelper {
       }
   }
 
-  private final void init(final GLEventListener l, final GLAutoDrawable drawable, final boolean sendReshape, final boolean setViewport) {
+  private final void init(final GLEventListener l, final GLAutoDrawable drawable, final boolean sendReshape) {
       l.init(drawable);
       if(sendReshape) {
-          reshape(l, drawable, 0, 0, drawable.getSurfaceWidth(), drawable.getSurfaceHeight(), setViewport, false /* checkInit */);
+          l.reshape(drawable, 0, 0, drawable.getSurfaceWidth(), drawable.getSurfaceHeight());
       }
   }
 
@@ -650,6 +652,7 @@ public class GLDrawableHelper {
    * @param sendReshape set to true if the subsequent display call won't reshape, otherwise false to avoid double reshape.
    **/
   public final void init(final GLAutoDrawable drawable, final boolean sendReshape) {
+    setViewportAndClear(drawable, 0, 0, drawable.getSurfaceWidth(), drawable.getSurfaceHeight());
     synchronized(listenersLock) {
         final ArrayList<GLEventListener> _listeners = listeners;
         final int listenerCount = _listeners.size();
@@ -661,11 +664,8 @@ public class GLDrawableHelper {
               // This may happen not just for initial setup, but for ctx recreation due to resource change (drawable/window),
               // hence it must be called unconditional, always.
               listenersToBeInit.remove(listener); // remove if exist, avoiding dbl init
-              init( listener, drawable, sendReshape, 0==i /* setViewport */);
+              init(listener, drawable, sendReshape);
             }
-        } else {
-            // Expose same GL initialization if not using any GLEventListener
-            drawable.getGL().glViewport(0, 0, drawable.getSurfaceWidth(), drawable.getSurfaceHeight());
         }
     }
   }
@@ -687,7 +687,7 @@ public class GLDrawableHelper {
             // GLEventListener may need to be init,
             // in case this one is added after the realization of the GLAutoDrawable
             if( listenersToBeInit.remove(listener) ) {
-                init( listener, drawable, true /* sendReshape */, listenersToBeInit.size() + 1 == listenerCount /* setViewport if 1st init */ );
+                init( listener, drawable, true /* sendReshape */ );
             }
             listener.display(drawable);
           }
@@ -712,41 +712,43 @@ public class GLDrawableHelper {
             // GLEventListener may need to be init,
             // in case this one is added after the realization of the GLAutoDrawable
             if( listenersToBeInit.remove(listener) ) {
-                init( listener, drawable, true /* sendReshape */, listenersToBeInit.size() + 1 == listenerCount /* setViewport if 1st init */ );
+                init( listener, drawable, true /* sendReshape */ );
             }
             action.run(drawable, listener);
           }
       }
   }
 
-  private final void reshape(final GLEventListener listener, final GLAutoDrawable drawable,
-                             final int x, final int y, final int width, final int height, final boolean setViewport, final boolean checkInit) {
-    if(checkInit) {
-        // GLEventListener may need to be init,
-        // in case this one is added after the realization of the GLAutoDrawable
-        synchronized(listenersLock) {
-            if( listenersToBeInit.remove(listener) ) {
-                listener.init(drawable);
-            }
-        }
-    }
-    if(setViewport) {
-        if( GLContext.DEBUG_GL || DEBUG ) {
-            final int glerr0 = drawable.getGL().glGetError();
-            if( GL.GL_NO_ERROR != glerr0 ) {
-                System.err.println("Info: GLDrawableHelper.reshape: pre-exisiting GL error 0x"+Integer.toHexString(glerr0));
-                ExceptionUtils.dumpStack(System.err);
-            }
-        }
-        drawable.getGL().glViewport(x, y, width, height);
-    }
-    listener.reshape(drawable, x, y, width, height);
+  /**
+   * Bug 1206: Security: Clear exposed framebuffer after creation and before visibility
+   * - Clear framebuffer after setting viewport
+   * - Since we only attempt to help against leaking un-initialized framebuffer content
+   *   not against user-app faults, we do not clear a 2nd-buffer (double-buffering).
+   */
+  private final void setViewportAndClear(final GLAutoDrawable drawable, final int x, final int y, final int width, final int height) {
+      final GL gl = drawable.getGL();
+      if( DEBUG_SETCLEAR ) {
+          final int glerr0 = gl.glGetError();
+          if( GL.GL_NO_ERROR != glerr0 ) {
+              System.err.println("Info: GLDrawableHelper.reshape: pre-exisiting GL error 0x"+Integer.toHexString(glerr0));
+              ExceptionUtils.dumpStack(System.err);
+          }
+      }
+      gl.glViewport(x, y, width, height);
+      gl.glClear(GL.GL_COLOR_BUFFER_BIT);
   }
 
   public final void reshape(final GLAutoDrawable drawable, final int x, final int y, final int width, final int height) {
+    setViewportAndClear(drawable, x, y, width, height);
     synchronized(listenersLock) {
         for (int i=0; i < listeners.size(); i++) {
-          reshape(listeners.get(i), drawable, x, y, width, height, 0==i /* setViewport */, true /* checkInit */);
+            final GLEventListener l = listeners.get(i);
+            // GLEventListener may need to be init,
+            // in case this one is added after the realization of the GLAutoDrawable
+            if( listenersToBeInit.remove(l) ) {
+                l.init(drawable);
+            }
+            l.reshape(drawable, x, y, width, height);
         }
     }
   }
diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/demos/GLClearOnInitReshape.java b/src/test/com/jogamp/opengl/test/junit/jogl/demos/GLClearOnInitReshape.java
new file mode 100644
index 000000000..46be7ef0a
--- /dev/null
+++ b/src/test/com/jogamp/opengl/test/junit/jogl/demos/GLClearOnInitReshape.java
@@ -0,0 +1,64 @@
+/**
+ * Copyright 2015 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.demos;
+
+import com.jogamp.opengl.GL;
+import com.jogamp.opengl.GLAutoDrawable;
+import com.jogamp.opengl.GLEventListener;
+
+public class GLClearOnInitReshape implements GLEventListener {
+    int lastWidth, lastHeight;
+    boolean doClear;
+
+    @Override
+    public void init(final GLAutoDrawable drawable) {
+        lastWidth = drawable.getSurfaceWidth();
+        lastHeight = drawable.getSurfaceHeight();
+        doClear = true;
+    }
+
+    @Override
+    public void dispose(final GLAutoDrawable drawable) { }
+
+    @Override
+    public void display(final GLAutoDrawable drawable) {
+        if( doClear ) {
+            drawable.getGL().glClear(GL.GL_COLOR_BUFFER_BIT);
+            doClear = false;
+        }
+    }
+
+    @Override
+    public void reshape(final GLAutoDrawable drawable, final int x, final int y, final int width, final int height) {
+        if( lastHeight != height || lastWidth != width ) {
+            doClear = true;
+            lastWidth = width;
+            lastHeight = height;
+        }
+    }
+}
diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/awt/TestGearsES2GLJPanelAWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/awt/TestGearsES2GLJPanelAWT.java
index 0312604d7..a41e4b9e7 100644
--- a/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/awt/TestGearsES2GLJPanelAWT.java
+++ b/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/awt/TestGearsES2GLJPanelAWT.java
@@ -65,6 +65,7 @@ import com.jogamp.newt.event.TraceKeyAdapter;
 import com.jogamp.newt.event.TraceWindowAdapter;
 import com.jogamp.newt.event.awt.AWTKeyAdapter;
 import com.jogamp.newt.event.awt.AWTWindowAdapter;
+import com.jogamp.opengl.test.junit.jogl.demos.GLClearOnInitReshape;
 import com.jogamp.opengl.test.junit.jogl.demos.es2.GearsES2;
 import com.jogamp.opengl.test.junit.jogl.demos.gl2.Gears;
 import com.jogamp.opengl.test.junit.util.AWTRobotUtil;
@@ -80,6 +81,7 @@ public class TestGearsES2GLJPanelAWT extends UITestCase {
     static boolean forceES3 = false;
     static boolean forceGL3 = false;
     static boolean forceGLFFP = false;
+    static int demoType = 1;
     static boolean shallUsePBuffer = false;
     static boolean shallUseBitmap = false;
     static boolean useMSAA = false;
@@ -132,6 +134,26 @@ public class TestGearsES2GLJPanelAWT extends UITestCase {
                 hasSurfacePixelScale[0]+"x"+hasSurfacePixelScale[1]+"]");
     }
 
+    protected GLEventListener createDemo(final GLCapabilities caps) {
+        final GLEventListener demo;
+        if( 1 == demoType ) {
+            if( caps.isBitmap() || caps.getGLProfile().isGL2() ) {
+                final Gears gears = new Gears(swapInterval);
+                gears.setFlipVerticalInGLOrientation(skipGLOrientationVerticalFlip);
+                demo = gears;
+            } else {
+                final GearsES2 gears = new GearsES2(swapInterval);
+                gears.setFlipVerticalInGLOrientation(skipGLOrientationVerticalFlip);
+                demo = gears;
+            }
+        } else if( 0 == demoType ) {
+            demo = new GLClearOnInitReshape();
+        } else {
+            demo = null;
+        }
+        return demo;
+    }
+
     protected GLJPanel newGLJPanel(final JFrame frame, final GLCapabilities caps, final FPSAnimator animator, final SnapshotGLEventListener snap)
             throws AWTException, InterruptedException, InvocationTargetException
     {
@@ -142,14 +164,11 @@ public class TestGearsES2GLJPanelAWT extends UITestCase {
         glJPanel.setPreferredSize(wsize);
         glJPanel.setSize(wsize);
         glJPanel.setSurfaceScale(reqSurfacePixelScale);
-        if( caps.isBitmap() || caps.getGLProfile().isGL2() ) {
-            final Gears gears = new Gears(swapInterval);
-            gears.setFlipVerticalInGLOrientation(skipGLOrientationVerticalFlip);
-            glJPanel.addGLEventListener(gears);
-        } else {
-            final GearsES2 gears = new GearsES2(swapInterval);
-            gears.setFlipVerticalInGLOrientation(skipGLOrientationVerticalFlip);
-            glJPanel.addGLEventListener(gears);
+        {
+            final GLEventListener demo = createDemo(caps);
+            if( null != demo ) {
+                glJPanel.addGLEventListener(demo);
+            }
         }
         if( null != snap ) {
             glJPanel.addGLEventListener(snap);
@@ -644,6 +663,9 @@ public class TestGearsES2GLJPanelAWT extends UITestCase {
                 shallUseBitmap = true;
             } else if(args[i].equals("-manual")) {
                 manualTest = true;
+            } else if(args[i].equals("-demo")) {
+                i++;
+                demoType = MiscUtils.atoi(args[i], 0);
             }
         }
         wsize = new Dimension(w, h);
@@ -663,6 +685,7 @@ public class TestGearsES2GLJPanelAWT extends UITestCase {
         System.err.println("shallUsePBuffer "+shallUsePBuffer);
         System.err.println("shallUseBitmap "+shallUseBitmap);
         System.err.println("manualTest "+manualTest);
+        System.err.println("demoType "+demoType);
 
         org.junit.runner.JUnitCore.main(TestGearsES2GLJPanelAWT.class.getName());
     }
diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/newt/TestGearsES2NEWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/newt/TestGearsES2NEWT.java
index 9098c3ea8..c821111a4 100644
--- a/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/newt/TestGearsES2NEWT.java
+++ b/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/newt/TestGearsES2NEWT.java
@@ -48,6 +48,7 @@ import com.jogamp.opengl.test.junit.util.MiscUtils;
 import com.jogamp.opengl.test.junit.util.UITestCase;
 import com.jogamp.opengl.util.Animator;
 import com.jogamp.opengl.util.AnimatorBase;
+import com.jogamp.opengl.test.junit.jogl.demos.GLClearOnInitReshape;
 import com.jogamp.opengl.test.junit.jogl.demos.es2.GearsES2;
 import com.jogamp.opengl.test.junit.jogl.demos.es2.LineSquareXDemoES2;
 import com.jogamp.nativewindow.NativeWindowFactory;
@@ -152,6 +153,8 @@ public class TestGearsES2NEWT extends UITestCase {
             gearsES2.setUseMappedBuffers(useMappedBuffers);
             gearsES2.setValidateBuffers(true);
             demo = gearsES2;
+        } else if( 0 == demoType ) {
+            demo = new GLClearOnInitReshape();
         } else {
             demo = null;
         }