Fixed another performance problem related to buffer objects pointed

out by John Burkey. glGetBufferParameterivARB call in glMapBuffer was
too expensive at least with Apple's multithreaded OpenGL
implementation. Now track both bound buffer state (refactored into
GLBufferStateTracker) as well as cache created buffers' sizes
(expressed in GLBufferSizeTracker) and query the cache instead of
OpenGL directly. Verified with VertexBufferObject demo that now no
glGet queries are made at run-time.


git-svn-id: file:///usr/local/projects/SUN/JOGL/git-svn/svn-server-sync/jogl/trunk@1003 232f8b59-042b-4e1e-8c03-345bb8c30851

4 files changed, 412 insertions, 1 deletions

diff --git a/src/classes/com/sun/opengl/impl/GLBufferSizeTracker.java b/src/classes/com/sun/opengl/impl/GLBufferSizeTracker.java
new file mode 100755
index 000000000..4538f8450
--- /dev/null
+++ b/src/classes/com/sun/opengl/impl/GLBufferSizeTracker.java
@@ -0,0 +1,188 @@
+/*
+ * Copyright (c) 2006 Sun Microsystems, Inc. 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.
+ * 
+ * You acknowledge that this software is not designed or intended for use
+ * in the design, construction, operation or maintenance of any nuclear
+ * facility.
+ * 
+ * Sun gratefully acknowledges that this software was originally authored
+ * and developed by Kenneth Bradley Russell and Christopher John Kline.
+ */
+
+package com.sun.opengl.impl;
+
+import java.util.*;
+import javax.media.opengl.*;
+
+/**
+ * Tracks as closely as possible the sizes of allocated OpenGL buffer
+ * objects. When glMapBuffer or glMapBufferARB is called, in order to
+ * turn the resulting base address into a java.nio.ByteBuffer, we need
+ * to know the size in bytes of the allocated OpenGL buffer object.
+ * Previously we would compute this size by using
+ * glGetBufferParameterivARB with a pname of GL_BUFFER_SIZE_ARB, but
+ * it appears doing so each time glMapBuffer is called is too costly
+ * on at least Apple's new multithreaded OpenGL implementation. <P>
+ *
+ * Instead we now try to track the sizes of allocated buffer objects.
+ * We watch calls to glBindBuffer to see which buffer is bound to
+ * which target and to glBufferData to see how large the buffer's
+ * allocated size is. When glMapBuffer is called, we consult our table
+ * of buffer sizes to see if we can return an answer without a glGet
+ * call. <P>
+ *
+ * We share the GLBufferSizeTracker objects among all GLContexts for
+ * which sharing is enabled, because the namespace for buffer objects
+ * is the same for these contexts. <P>
+ *
+ * Tracking the state of which buffer objects are bound is done in the
+ * GLBufferStateTracker and is not completely trivial. In the face of
+ * calls to glPushClientAttrib / glPopClientAttrib we currently punt
+ * and re-fetch the bound buffer object for the state in question;
+ * see, for example, glVertexPointer and the calls down to
+ * GLBufferStateTracker.getBoundBufferObject(). Note that we currently
+ * ignore new binding targets such as GL_TRANSFORM_FEEDBACK_BUFFER_NV;
+ * the fact that new binding targets may be added in the future makes
+ * it impossible to cache state for these new targets. <P>
+ *
+ * Ignoring new binding targets, the primary situation in which we may
+ * not be able to return a cached answer is in the case of an error,
+ * where glBindBuffer may not have been called before trying to call
+ * glBufferData. Also, if external native code modifies a buffer
+ * object, we may return an incorrect answer. (FIXME: this case
+ * requires more thought, and perhaps stochastic and
+ * exponential-fallback checking. However, note that it can only occur
+ * in the face of external native code which requires that the
+ * application be signed anyway, so there is no security risk in this
+ * area.)
+ */
+
+public class GLBufferSizeTracker {
+  // Map from buffer names to sizes.
+  // Note: should probably have some way of shrinking this map, but
+  // can't just make it a WeakHashMap because nobody holds on to the
+  // keys; would have to always track creation and deletion of buffer
+  // objects, which is probably sub-optimal. The expected usage
+  // pattern of buffer objects indicates that the fact that this map
+  // never shrinks is probably not that bad.
+  private Map/*<Integer,Integer>*/ bufferSizeMap =
+    Collections.synchronizedMap(new HashMap/*<Integer,Integer>*/());
+
+  private static final boolean DEBUG = Debug.debug("GLBufferSizeTracker");
+
+  public GLBufferSizeTracker() {
+  }
+
+  public void setBufferSize(GLBufferStateTracker bufferStateTracker,
+                            int target,
+                            GL caller,
+                            int size) {
+    // Need to do some similar queries to getBufferSize below
+    int buffer = bufferStateTracker.getBoundBufferObject(target, caller);
+    boolean valid = bufferStateTracker.isBoundBufferObjectKnown(target);
+    if (valid) {
+      if (buffer == 0) {
+        // FIXME: this really should not happen if we know what's
+        // going on. Very likely there is an OpenGL error in the
+        // application if we get here. Could silently return 0, but it
+        // seems better to get an early warning that something is
+        // wrong.
+        throw new GLException("Error: no OpenGL buffer object appears to be bound to target 0x" +
+                              Integer.toHexString(target));
+      }
+      bufferSizeMap.put(new Integer(buffer), new Integer(size));
+    }
+    // We don't know the current buffer state. Note that the buffer
+    // state tracker will have made the appropriate OpenGL query if it
+    // didn't know what was going on, so at this point we have nothing
+    // left to do except drop this piece of information on the floor.
+  }
+
+  public int getBufferSize(GLBufferStateTracker bufferStateTracker,
+                           int target,
+                           GL caller) {
+    // See whether we know what buffer is currently bound to the given
+    // state
+    int buffer = bufferStateTracker.getBoundBufferObject(target, caller);
+    boolean valid = bufferStateTracker.isBoundBufferObjectKnown(target);
+    if (valid) {
+      if (buffer == 0) {
+        // FIXME: this really should not happen if we know what's
+        // going on. Very likely there is an OpenGL error in the
+        // application if we get here. Could silently return 0, but it
+        // seems better to get an early warning that something is
+        // wrong.
+        throw new GLException("Error: no OpenGL buffer object appears to be bound to target 0x" +
+                              Integer.toHexString(target));
+      }
+      // See whether we know the size of this buffer object; at this
+      // point we almost certainly should if the application is
+      // written correctly
+      Integer key = new Integer(buffer);
+      Integer sz = (Integer) bufferSizeMap.get(key);
+      if (sz == null) {
+        // For robustness, try to query this value from the GL as we used to
+        int[] tmp = new int[1];
+        caller.glGetBufferParameterivARB(target, GL.GL_BUFFER_SIZE_ARB, tmp, 0);
+        if (tmp[0] == 0) {
+          // Assume something is wrong rather than silently going along
+          throw new GLException("Error: buffer size returned by glGetBufferParameterivARB was zero; probably application error");
+        }
+        // Assume we just don't know what's happening
+        sz = new Integer(tmp[0]);
+        bufferSizeMap.put(key, sz);
+        if (DEBUG) {
+          System.err.println("GLBufferSizeTracker.getBufferSize(): made slow query to cache size " +
+                             tmp[0] +
+                             " for buffer " +
+                             buffer);
+        }
+      }
+      return sz.intValue();
+    }
+    // We don't know what's going on in this case; query the GL for an answer
+    int[] tmp = new int[1];
+    caller.glGetBufferParameterivARB(target, GL.GL_BUFFER_SIZE_ARB, tmp, 0);
+    if (DEBUG) {
+      System.err.println("GLBufferSizeTracker.getBufferSize(): no cached buffer information");
+    }
+    return tmp[0];
+  }
+
+  // This should be called on any major event where we might start
+  // producing wrong answers, such as OpenGL context creation and
+  // destruction if we don't know whether there are other currently-
+  // created contexts that might be keeping the buffer objects alive
+  // that we're dealing with
+  public void clearCachedBufferSizes() {
+    bufferSizeMap.clear();
+  }
+}
diff --git a/src/classes/com/sun/opengl/impl/GLBufferStateTracker.java b/src/classes/com/sun/opengl/impl/GLBufferStateTracker.java
new file mode 100755
index 000000000..2c4a9963e
--- /dev/null
+++ b/src/classes/com/sun/opengl/impl/GLBufferStateTracker.java
@@ -0,0 +1,173 @@
+/*
+ * Copyright (c) 2006 Sun Microsystems, Inc. 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.
+ * 
+ * You acknowledge that this software is not designed or intended for use
+ * in the design, construction, operation or maintenance of any nuclear
+ * facility.
+ * 
+ * Sun gratefully acknowledges that this software was originally authored
+ * and developed by Kenneth Bradley Russell and Christopher John Kline.
+ */
+
+package com.sun.opengl.impl;
+
+import java.util.*;
+import javax.media.opengl.*;
+
+/**
+ * Tracks as closely as possible which OpenGL buffer object is bound
+ * to which binding target in the current OpenGL context.
+ * GLBufferStateTracker objects are allocated on a per-GLImpl basis,
+ * which is basically identical to a per-OpenGL-context basis
+ * (assuming correct usage of the GLImpl objects, which is checked by
+ * the DebugGL). This class is used to verify that e.g. the vertex
+ * buffer object extension is in use when the glVertexPointer variant
+ * taking a long as argument is called. <P>
+ * 
+ * Note that because the enumerated value used for the binding of a
+ * buffer object (e.g. GL_ARRAY_BUFFER) is different than that used to
+ * query the binding using glGetIntegerv (e.g.
+ * GL_ARRAY_BUFFER_BINDING), then in the face of new binding targets
+ * being added to the GL (e.g. GL_TRANSFORM_FEEDBACK_BUFFER_NV) it is
+ * impossible to set up a query of the buffer object currently bound
+ * to a particular state. It turns out that for some uses, such as
+ * finding the size of the currently bound buffer, this doesn't
+ * matter, though of course without knowing the buffer object we can't
+ * re-associate the queried size with the buffer object ID. <P>
+ *
+ * Because the namespace of buffer objects is the unsigned integers
+ * with 0 reserved by the GL, and because we have to be able to return
+ * both 0 and other integers as valid answers from
+ * getBoundBufferObject(), we need a second query, which is to ask
+ * whether we know the state of the binding for a given target. For
+ * "unknown" targets such as GL_TRANSFORM_FEEDBACK_BUFFER_NV we return
+ * false from this, but we also clear the valid bit and later refresh
+ * the binding state if glPushClientAttrib / glPopClientAttrib are
+ * called, since we don't want the complexity of tracking stacks of
+ * these attributes.
+ *
+ */
+
+public class GLBufferStateTracker {
+  private static final boolean DEBUG = Debug.debug("GLBufferStateTracker");
+
+  private static final Integer arrayBufferEnum        = new Integer(GL.GL_ARRAY_BUFFER);
+  private static final Integer elementArrayBufferEnum = new Integer(GL.GL_ELEMENT_ARRAY_BUFFER);
+  private static final Integer pixelPackBufferEnum    = new Integer(GL.GL_PIXEL_PACK_BUFFER);
+  private static final Integer pixelUnpackBufferEnum  = new Integer(GL.GL_PIXEL_UNPACK_BUFFER);
+  private static final Integer zero                   = new Integer(0);
+
+  // Maps binding targets to buffer objects. A null value indicates
+  // that the binding is unknown. A zero value indicates that it is
+  // known that no buffer is bound to the target.
+  private Map/*<Integer,Integer>*/ bindingMap = new HashMap/*<Integer,Integer>*/();
+
+  private int[] bufTmp = new int[1];
+
+  public GLBufferStateTracker() {
+    // Start with known unbound targets for known keys
+    bindingMap.put(arrayBufferEnum,        zero);
+    bindingMap.put(elementArrayBufferEnum, zero);
+    bindingMap.put(pixelPackBufferEnum,    zero);
+    bindingMap.put(pixelUnpackBufferEnum,  zero);
+  }
+
+  public void setBoundBufferObject(int target, int buffer) {
+    Integer key = box(target);
+    bindingMap.put(key, box(buffer));
+  }
+
+  /** Note: returns an unspecified value if the binding for the
+      specified target (e.g. GL_ARRAY_BUFFER) is currently unknown.
+      You must use isBoundBufferObjectKnown() to see whether the
+      return value is valid. */
+  public int getBoundBufferObject(int target, GL caller) {
+    Integer key = box(target);
+    Integer value = (Integer) bindingMap.get(key);
+    if (value == null) {
+      // User probably either called glPushClientAttrib /
+      // glPopClientAttrib or is querying an unknown target. See
+      // whether we know how to fetch this state.
+      boolean gotQueryTarget = true;
+      int queryTarget = 0;
+      switch (target) {
+        case GL.GL_ARRAY_BUFFER:          queryTarget = GL.GL_ARRAY_BUFFER_BINDING;         break;
+        case GL.GL_ELEMENT_ARRAY_BUFFER:  queryTarget = GL.GL_ELEMENT_ARRAY_BUFFER_BINDING; break;
+        case GL.GL_PIXEL_PACK_BUFFER:     queryTarget = GL.GL_PIXEL_PACK_BUFFER_BINDING;    break;
+        case GL.GL_PIXEL_UNPACK_BUFFER:   queryTarget = GL.GL_PIXEL_UNPACK_BUFFER_BINDING;  break;
+        default:                          gotQueryTarget = false; break;
+      }
+      if (gotQueryTarget) {
+        caller.glGetIntegerv(queryTarget, bufTmp, 0);
+        if (DEBUG) {
+          System.err.println("GLBufferStateTracker.getBoundBufferObject(): queried bound buffer " +
+                             bufTmp[0] +
+                             " for query target 0x" + Integer.toHexString(queryTarget));
+        }
+        setBoundBufferObject(target, bufTmp[0]);
+        // Try once more
+        return getBoundBufferObject(target, caller);
+      }
+      return 0;
+    }
+    return value.intValue();
+  }
+
+  /** Indicates whether the binding state for the specified target is
+      currently known. Should be called after getBoundBufferObject()
+      because that method may change the answer for a given target. */
+  public boolean isBoundBufferObjectKnown(int target) {
+    return (bindingMap.get(box(target)) != null);
+  }
+
+  /** Clears out the known/unknown state of the various buffer object
+      binding states. These will be refreshed later on an as-needed
+      basis. This is called by the implementations of
+      glPushClientAttrib / glPopClientAttrib. Might want to call this
+      from GLContext.makeCurrent() in the future to possibly increase
+      the robustness of these caches in the face of external native
+      code manipulating OpenGL state. */
+  public void clearBufferObjectState() {
+    bindingMap.clear();
+  }
+
+  // FIXME: could largely remove this and use Integer.valueOf() in JDK 5
+  private static Integer box(int key) {
+    switch (key) {
+      case 0:                          return zero;
+      case GL.GL_ARRAY_BUFFER:         return arrayBufferEnum;
+      case GL.GL_ELEMENT_ARRAY_BUFFER: return elementArrayBufferEnum;
+      case GL.GL_PIXEL_PACK_BUFFER:    return pixelPackBufferEnum;
+      case GL.GL_PIXEL_UNPACK_BUFFER:  return pixelUnpackBufferEnum;
+      default:                         return new Integer(key);
+    }
+  }
+}
diff --git a/src/classes/com/sun/opengl/impl/GLContextImpl.java b/src/classes/com/sun/opengl/impl/GLContextImpl.java
index fcd1e58be..21e6d6598 100644
--- a/src/classes/com/sun/opengl/impl/GLContextImpl.java
+++ b/src/classes/com/sun/opengl/impl/GLContextImpl.java
@@ -68,6 +68,10 @@ public abstract class GLContextImpl extends GLContext {
   // OpenGL functions.
   private GLProcAddressTable glProcAddressTable;
 
+  // Tracks creation and initialization of buffer objects to avoid
+  // repeated glGet calls upon glMapBuffer operations
+  private GLBufferSizeTracker bufferSizeTracker;
+
   // Tracks creation and deletion of server-side OpenGL objects when
   // the Java2D/OpenGL pipeline is active and using FBOs to render
   private GLObjectTracker tracker;
@@ -81,7 +85,6 @@ public abstract class GLContextImpl extends GLContext {
   }
 
   public GLContextImpl(GLContext shareWith, boolean dontShareWithJava2D) {
-    setGL(createGL());
     functionAvailability = new FunctionAvailabilityCache(this);
     GLContext shareContext = shareWith;
     if (!dontShareWithJava2D) {
@@ -95,6 +98,10 @@ public abstract class GLContextImpl extends GLContext {
       shareContext = Java2D.filterShareContext(shareWith);
     }
     GLContextShareSet.registerForObjectTracking(shareWith, this, shareContext);
+    GLContextShareSet.registerForBufferObjectSharing(shareWith, this);
+    // This must occur after the above calls into the
+    // GLContextShareSet, which set up state needed by the GL object
+    setGL(createGL());
   }
 
   public int makeCurrent() throws GLException {
@@ -184,6 +191,13 @@ public abstract class GLContextImpl extends GLContext {
       }
     }
 
+    // Because we don't know how many other contexts we might be
+    // sharing with (and it seems too complicated to implement the
+    // GLObjectTracker's ref/unref scheme for the buffer-related
+    // optimizations), simply clear the cache of known buffers' sizes
+    // when we destroy contexts
+    bufferSizeTracker.clearCachedBufferSizes();
+
     // Must hold the lock around the destroy operation to make sure we
     // don't destroy the context out from under another thread rendering to it
     lock.lock();
@@ -361,6 +375,17 @@ public abstract class GLContextImpl extends GLContext {
     return "0x" + Long.toHexString(hex);
   }
 
+  //----------------------------------------------------------------------
+  // Helpers for buffer object optimizations
+  
+  public void setBufferSizeTracker(GLBufferSizeTracker bufferSizeTracker) {
+    this.bufferSizeTracker = bufferSizeTracker;
+  }
+
+  public GLBufferSizeTracker getBufferSizeTracker() {
+    return bufferSizeTracker;
+  }
+
   //---------------------------------------------------------------------------
   // Helpers for integration with Java2D/OpenGL pipeline when FBOs are
   // being used
diff --git a/src/classes/com/sun/opengl/impl/GLContextShareSet.java b/src/classes/com/sun/opengl/impl/GLContextShareSet.java
index f2c015762..e02ef1fb5 100644
--- a/src/classes/com/sun/opengl/impl/GLContextShareSet.java
+++ b/src/classes/com/sun/opengl/impl/GLContextShareSet.java
@@ -240,6 +240,31 @@ public class GLContextShareSet {
     }
   }
 
+  /** In order to avoid glGet calls for buffer object checks related
+      to glVertexPointer, etc. calls as well as glMapBuffer calls, we
+      need to share the same GLBufferSizeTracker object between
+      contexts sharing textures and display lists. For now we keep
+      this mechanism orthogonal to the GLObjectTracker to hopefully
+      keep things easier to understand. (The GLObjectTracker is
+      currently only needed in a fairly esoteric case, when the
+      Java2D/JOGL bridge is active, but the GLBufferSizeTracker
+      mechanism is now always required.) */
+  public static void registerForBufferObjectSharing(GLContext olderContextOrNull, GLContext newContext) {
+    // FIXME: downcasts to GLContextImpl undesirable
+    GLContextImpl older = (GLContextImpl) olderContextOrNull;
+    GLContextImpl newer = (GLContextImpl) newContext;
+    GLBufferSizeTracker tracker = null;
+    if (older != null) {
+      tracker = older.getBufferSizeTracker();
+      assert (tracker != null)
+        : "registerForBufferObjectSharing was not called properly for the older context, or has a bug in it";
+    }
+    if (tracker == null) {
+      tracker = new GLBufferSizeTracker();
+    }
+    newer.setBufferSizeTracker(tracker);
+  }
+
   //----------------------------------------------------------------------
   // Internals only below this point
   //