Simplify GLArrayHandler and reduce VBO sideffects

VBO: Always unbind VBO ASAP after data transfer (glBufferData())
     and assignment (glVertexPointer(..), glVertexAttribPointer()).
     It's a bug to leave it bound .. due to redundancy
     and other calls which could have change the VBO binding.

Removed syncData(..), now it's only issued at enable
and hence migrated into the enable method.

10 files changed, 77 insertions, 118 deletions

diff --git a/src/jogl/classes/com/jogamp/opengl/util/GLArrayDataClient.java b/src/jogl/classes/com/jogamp/opengl/util/GLArrayDataClient.java
index 134dd9677..5a1524d05 100644
--- a/src/jogl/classes/com/jogamp/opengl/util/GLArrayDataClient.java
+++ b/src/jogl/classes/com/jogamp/opengl/util/GLArrayDataClient.java
@@ -167,13 +167,7 @@ public class GLArrayDataClient extends GLArrayDataWrapper implements GLArrayData
         } else {
             ext = null;
         }
-        if(enable) {
-            glArrayHandler.syncData(gl, true, ext);
-            glArrayHandler.enableState(gl, true, ext);
-        } else {
-            glArrayHandler.enableState(gl, false, ext);
-            glArrayHandler.syncData(gl, false, ext);
-        }
+        glArrayHandler.enableState(gl, enable, ext);
         bufferEnabled = enable;
     }
   }
diff --git a/src/jogl/classes/jogamp/opengl/util/GLArrayHandler.java b/src/jogl/classes/jogamp/opengl/util/GLArrayHandler.java
index 22690b06d..2049f9618 100644
--- a/src/jogl/classes/jogamp/opengl/util/GLArrayHandler.java
+++ b/src/jogl/classes/jogamp/opengl/util/GLArrayHandler.java
@@ -38,17 +38,12 @@ import javax.media.opengl.*;
 public interface GLArrayHandler {
 
   /**
-   * Implementation shall associate the data with the array
-   * and synchronize the data with the GPU.
-   * 
-   * @param gl current GL object
-   * @param enable true if array data shall be valid, otherwise false.
-   * @param ext extension object allowing passing of an implementation detail 
-   */
-  public void syncData(GL gl, boolean enable, Object ext);
-  
-  /**
    * Implementation shall enable or disable the array state.
+   * <p>
+   * Before enabling the array state,
+   * implementation shall associate the data with the array
+   * and synchronize the data with the GPU.
+   * </p>
    * 
    * @param gl current GL object
    * @param enable true if array shall be enabled, otherwise false.
diff --git a/src/jogl/classes/jogamp/opengl/util/GLArrayHandlerFlat.java b/src/jogl/classes/jogamp/opengl/util/GLArrayHandlerFlat.java
index dca9129ad..4a8f40608 100644
--- a/src/jogl/classes/jogamp/opengl/util/GLArrayHandlerFlat.java
+++ b/src/jogl/classes/jogamp/opengl/util/GLArrayHandlerFlat.java
@@ -41,11 +41,9 @@ public interface GLArrayHandlerFlat {
    * Implementation shall associate the data with the array
    * 
    * @param gl current GL object
-   * @param enable true if array data shall be valid, otherwise false.
-   * @param force true force data association, bypassing optimization
    * @param ext extension object allowing passing of an implementation detail 
    */
-  public void syncData(GL gl, boolean enable, boolean force, Object ext);
+  public void syncData(GL gl, Object ext);
   
   /**
    * Implementation shall enable or disable the array state.
diff --git a/src/jogl/classes/jogamp/opengl/util/GLArrayHandlerInterleaved.java b/src/jogl/classes/jogamp/opengl/util/GLArrayHandlerInterleaved.java
index d31b41582..ab916c329 100644
--- a/src/jogl/classes/jogamp/opengl/util/GLArrayHandlerInterleaved.java
+++ b/src/jogl/classes/jogamp/opengl/util/GLArrayHandlerInterleaved.java
@@ -59,17 +59,17 @@ public class GLArrayHandlerInterleaved implements GLArrayHandler {
       subArrays.add(handler);
   }
 
-  private final void syncSubData(GL gl, boolean enable, boolean force, Object ext) {
+  private final void syncSubData(GL gl, Object ext) {
       for(int i=0; i<subArrays.size(); i++) {
-          subArrays.get(i).syncData(gl, enable, force, ext);
+          subArrays.get(i).syncData(gl, ext);
       }      
   }  
   
-  public final void syncData(GL gl, boolean enable, Object ext) {
+  public final void enableState(GL gl, boolean enable, Object ext) {
     if(enable) {
         final Buffer buffer = ad.getBuffer();
-
-        if(ad.isVBO()) {
+        final boolean vboBound = ad.isVBO();
+        if(vboBound) {
             // always bind and refresh the VBO mgr, 
             // in case more than one gl*Pointer objects are in use
             gl.glBindBuffer(ad.getVBOTarget(), ad.getVBOName());
@@ -80,16 +80,12 @@ public class GLArrayHandlerInterleaved implements GLArrayHandler {
                 ad.setVBOWritten(true);
             }
         }
-        syncSubData(gl, true, true, ext);
-    } else {
-        syncSubData(gl, false, true, ext);
-        if(ad.isVBO()) {
+        syncSubData(gl, ext);
+        if(vboBound) {
             gl.glBindBuffer(ad.getVBOTarget(), 0);
         }
+        
     }
-  }
-  
-  public final void enableState(GL gl, boolean enable, Object ext) {
     for(int i=0; i<subArrays.size(); i++) {
         subArrays.get(i).enableState(gl, enable, ext);
     }      
diff --git a/src/jogl/classes/jogamp/opengl/util/GLDataArrayHandler.java b/src/jogl/classes/jogamp/opengl/util/GLDataArrayHandler.java
index 6c8e2e762..1e882e080 100644
--- a/src/jogl/classes/jogamp/opengl/util/GLDataArrayHandler.java
+++ b/src/jogl/classes/jogamp/opengl/util/GLDataArrayHandler.java
@@ -53,14 +53,14 @@ public class GLDataArrayHandler implements GLArrayHandler {
       throw new UnsupportedOperationException();
   }
   
-  public final void syncData(GL gl, boolean enable, Object ext) {
-    if(!ad.isVBO()) {
-        // makes no sense otherwise
-        throw new GLException("GLDataArrayHandler can only handle VBOs.");
-    }
+  public final void enableState(GL gl, boolean enable, Object ext) { 
     if(enable) {
+        if(!ad.isVBO()) {
+            // makes no sense otherwise
+            throw new GLException("GLDataArrayHandler can only handle VBOs.");
+        }
         Buffer buffer = ad.getBuffer();
-
+        
         // always bind and refresh the VBO mgr, 
         // in case more than one gl*Pointer objects are in use
         gl.glBindBuffer(ad.getVBOTarget(), ad.getVBOName());
@@ -70,13 +70,10 @@ public class GLDataArrayHandler implements GLArrayHandler {
             }
             ad.setVBOWritten(true);
         }
-    } else {
         gl.glBindBuffer(ad.getVBOTarget(), 0);
-    }      
-  }
-  
-  public final void enableState(GL gl, boolean enable, Object ext) { 
-      // no array association
+          
+    }
+    // no array association
   }
 }
 
diff --git a/src/jogl/classes/jogamp/opengl/util/GLFixedArrayHandler.java b/src/jogl/classes/jogamp/opengl/util/GLFixedArrayHandler.java
index d8939dc0f..3ed41893a 100644
--- a/src/jogl/classes/jogamp/opengl/util/GLFixedArrayHandler.java
+++ b/src/jogl/classes/jogamp/opengl/util/GLFixedArrayHandler.java
@@ -54,10 +54,12 @@ public class GLFixedArrayHandler implements GLArrayHandler {
       throw new UnsupportedOperationException();
   }
   
-  public final void syncData(GL gl, boolean enable, Object ext) {
+  public final void enableState(GL gl, boolean enable, Object ext) {
+    final GLPointerFunc glp = gl.getGL2ES1();
     if(enable) {
         final Buffer buffer = ad.getBuffer();
-        if(ad.isVBO()) {
+        final boolean vboBound = ad.isVBO();
+        if(vboBound) {
             // always bind and refresh the VBO mgr, 
             // in case more than one gl*Pointer objects are in use
             gl.glBindBuffer(ad.getVBOTarget(), ad.getVBOName());
@@ -68,7 +70,6 @@ public class GLFixedArrayHandler implements GLArrayHandler {
                 ad.setVBOWritten(true);
             }
         }
-        final GLPointerFunc glp = gl.getGL2ES1();
         switch(ad.getIndex()) {
             case GLPointerFunc.GL_VERTEX_ARRAY:
                 glp.glVertexPointer(ad);
@@ -85,14 +86,8 @@ public class GLFixedArrayHandler implements GLArrayHandler {
             default:
                 throw new GLException("invalid glArrayIndex: "+ad.getIndex()+":\n\t"+ad); 
         }
-    } else if(ad.isVBO()) {
         gl.glBindBuffer(ad.getVBOTarget(), 0);
-    }
-  }
-  
-  public final void enableState(GL gl, boolean enable, Object ext) {
-    final GLPointerFunc glp = gl.getGL2ES1();
-    if(enable) {
+        
         glp.glEnableClientState(ad.getIndex());        
     } else {
         glp.glDisableClientState(ad.getIndex());
diff --git a/src/jogl/classes/jogamp/opengl/util/GLFixedArrayHandlerFlat.java b/src/jogl/classes/jogamp/opengl/util/GLFixedArrayHandlerFlat.java
index 2937cc720..ff1813b95 100644
--- a/src/jogl/classes/jogamp/opengl/util/GLFixedArrayHandlerFlat.java
+++ b/src/jogl/classes/jogamp/opengl/util/GLFixedArrayHandlerFlat.java
@@ -49,25 +49,23 @@ public class GLFixedArrayHandlerFlat implements GLArrayHandlerFlat {
       return ad;
   }
   
-  public final void syncData(GL gl, boolean enable, boolean force, Object ext) {
-    if(enable) {
-        final GLPointerFunc glp = gl.getGL2ES1();
-        switch(ad.getIndex()) {
-            case GLPointerFunc.GL_VERTEX_ARRAY:
-                glp.glVertexPointer(ad);
-                break;
-            case GLPointerFunc.GL_NORMAL_ARRAY:
-                glp.glNormalPointer(ad);
-                break;
-            case GLPointerFunc.GL_COLOR_ARRAY:
-                glp.glColorPointer(ad);
-                break;
-            case GLPointerFunc.GL_TEXTURE_COORD_ARRAY:
-                glp.glTexCoordPointer(ad);
-                break;
-            default:
-                throw new GLException("invalid glArrayIndex: "+ad.getIndex()+":\n\t"+ad); 
-        }
+  public final void syncData(GL gl, Object ext) {
+    final GLPointerFunc glp = gl.getGL2ES1();
+    switch(ad.getIndex()) {
+        case GLPointerFunc.GL_VERTEX_ARRAY:
+            glp.glVertexPointer(ad);
+            break;
+        case GLPointerFunc.GL_NORMAL_ARRAY:
+            glp.glNormalPointer(ad);
+            break;
+        case GLPointerFunc.GL_COLOR_ARRAY:
+            glp.glColorPointer(ad);
+            break;
+        case GLPointerFunc.GL_TEXTURE_COORD_ARRAY:
+            glp.glTexCoordPointer(ad);
+            break;
+        default:
+            throw new GLException("invalid glArrayIndex: "+ad.getIndex()+":\n\t"+ad); 
     }
   }
 
diff --git a/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandler.java b/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandler.java
index 602d283d6..d7327a4e8 100644
--- a/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandler.java
+++ b/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandler.java
@@ -58,9 +58,9 @@ public class GLSLArrayHandler implements GLArrayHandler {
       throw new UnsupportedOperationException();
   }
   
-  public final void syncData(GL gl, boolean enable, Object ext) {
+  public final void enableState(GL gl, boolean enable, Object ext) {
     final GL2ES2 glsl = gl.getGL2ES2();
-    final ShaderState st = (ShaderState) ext;
+    final ShaderState st = (ShaderState) ext;    
     
     if(enable) {
         final Buffer buffer = ad.getBuffer();
@@ -87,6 +87,7 @@ public class GLSLArrayHandler implements GLArrayHandler {
                 }
                 ad.setVBOWritten(true);
                 st.vertexAttribPointer(glsl, ad);
+                glsl.glBindBuffer(ad.getVBOTarget(), 0);
             } else if(st.getAttribLocation(glsl, ad) >= 0) {
                 // didn't experience a performance hit on this query ..
                 // (using ShaderState's location query above to validate the location)
@@ -95,21 +96,13 @@ public class GLSLArrayHandler implements GLArrayHandler {
                 if(ad.getVBOName() != qi[0]) {
                     glsl.glBindBuffer(ad.getVBOTarget(), ad.getVBOName());
                     st.vertexAttribPointer(glsl, ad);
+                    glsl.glBindBuffer(ad.getVBOTarget(), 0);
                 }
             }
         } else if(null!=buffer) {
             st.vertexAttribPointer(glsl, ad);
         }
-    } else if(ad.isVBO()) {
-        glsl.glBindBuffer(ad.getVBOTarget(), 0);
-    }      
-  }
-  
-  public final void enableState(GL gl, boolean enable, Object ext) {
-    final GL2ES2 glsl = gl.getGL2ES2();
-    final ShaderState st = (ShaderState) ext;
-    
-    if(enable) {
+        
         st.enableVertexAttribArray(glsl, ad);
     } else {
         st.disableVertexAttribArray(glsl, ad);
diff --git a/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandlerFlat.java b/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandlerFlat.java
index c4b761b13..c5beb7dd0 100644
--- a/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandlerFlat.java
+++ b/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandlerFlat.java
@@ -51,28 +51,26 @@ public class GLSLArrayHandlerFlat implements GLArrayHandlerFlat {
       return ad;
   }
     
-  public final void syncData(GL gl, boolean enable, boolean force, Object ext) {
-    if(enable) {
-        final GL2ES2 glsl = gl.getGL2ES2();
-        final ShaderState st = (ShaderState) ext;
+  public final void syncData(GL gl, Object ext) {
+    final GL2ES2 glsl = gl.getGL2ES2();
+    final ShaderState st = (ShaderState) ext;
 
+    st.vertexAttribPointer(glsl, ad);
+    /**
+     * Due to probable application VBO switching, this might not make any sense ..
+     * 
+    if(!written) {
         st.vertexAttribPointer(glsl, ad);
-        /**
-         * Due to probable application VBO switching, this might not make any sense ..
-         * 
-        if(force) {
+    } else if(st.getAttribLocation(glsl, ad) >= 0) {
+        final int[] qi = new int[1];
+        glsl.glGetVertexAttribiv(ad.getLocation(), GL2ES2.GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING, qi, 0);
+        if(ad.getVBOName() != qi[0]) {
+            System.err.println("XXX1: "+ad.getName()+", vbo ad "+ad.getVBOName()+", gl "+qi[0]+", "+ad);
             st.vertexAttribPointer(glsl, ad);
-        } else if(st.getAttribLocation(glsl, ad) >= 0) {
-            final int[] qi = new int[1];
-            glsl.glGetVertexAttribiv(ad.getLocation(), GL2ES2.GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING, qi, 0);
-            if(ad.getVBOName() != qi[0]) {
-                System.err.println("XXX1: "+ad.getName()+", vbo ad "+ad.getVBOName()+", gl "+qi[0]+", "+ad);
-                st.vertexAttribPointer(glsl, ad);
-            } else {
-                System.err.println("XXX0: "+ad.getName()+", vbo ad "+ad.getVBOName()+", gl "+qi[0]+", "+ad);
-            }
-        }*/
-    }
+        } else {
+            System.err.println("XXX0: "+ad.getName()+", vbo ad "+ad.getVBOName()+", gl "+qi[0]+", "+ad);
+        }
+    }*/
   }
 
   public final void enableState(GL gl, boolean enable, Object ext) {
diff --git a/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandlerInterleaved.java b/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandlerInterleaved.java
index f50429623..6c80e31fa 100644
--- a/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandlerInterleaved.java
+++ b/src/jogl/classes/jogamp/opengl/util/glsl/GLSLArrayHandlerInterleaved.java
@@ -61,18 +61,18 @@ public class GLSLArrayHandlerInterleaved implements GLArrayHandler {
       subArrays.add(handler);
   }
 
-  private final void syncSubData(GL gl, boolean enable, boolean force, Object ext) {
+  private final void syncSubData(GL gl, Object ext) {
       for(int i=0; i<subArrays.size(); i++) {
-          subArrays.get(i).syncData(gl, enable, force, ext);
+          subArrays.get(i).syncData(gl, ext);
       }      
   }  
   
-  public final void syncData(GL gl, boolean enable, Object ext) {    
-    if(!ad.isVBO()) {
-        throw new InternalError("Interleaved handle is not VBO: "+ad);
-    }
-    
+  public final void enableState(GL gl, boolean enable, Object ext) {
     if(enable) {
+        if(!ad.isVBO()) {
+            throw new InternalError("Interleaved handle is not VBO: "+ad);
+        }
+        
         final Buffer buffer = ad.getBuffer();
         final boolean vboWritten = ad.isVBOWritten();
         
@@ -86,14 +86,9 @@ public class GLSLArrayHandlerInterleaved implements GLArrayHandler {
             ad.setVBOWritten(true);
         }
         // sub data will decide weather to update the vertex attrib pointer
-        syncSubData(gl, true, !vboWritten, ext);
-    } else {
-        // NOP on GLSL: syncSubData(gl, false, ext);
-        gl.glBindBuffer(ad.getVBOTarget(), 0);
+        syncSubData(gl, ext);
+        gl.glBindBuffer(ad.getVBOTarget(), 0);        
     }
-  }
-  
-  public final void enableState(GL gl, boolean enable, Object ext) {
     for(int i=0; i<subArrays.size(); i++) {
         subArrays.get(i).enableState(gl, enable, ext);
     }