From 30bd30d6563041b71f40e4c336e636768ae26f9d Mon Sep 17 00:00:00 2001
From: Sven Gothel <sgothel@jausoft.com>
Date: Tue, 14 Jan 2014 20:25:07 +0100
Subject: Bug 942: Bug 942 - Review GLBuffer[State|Size]Tracker and NIO mapped
buffers
Commit f8a74c9831c65725a699320c27e62161a0378241 reverted
commit 7c5483d5b20aed9c87c5ce3f6bc840b6546edcd1
due to the fact that the buffer binding itself is _not_
shared across shared GLContext!
Apply uncritical changes of 7c5483d5b20aed9c87c5ce3f6bc840b6546edcd1:
+++
Simplify GLBufferSizeTracker creation @ GLContextImpl ctor,
make it final.
+++
Clear the GLBufferSizeTracker (@destruction) only if no more
created shares are left!
+++
Refine API doc.
+++
---
.../classes/jogamp/opengl/GLBufferSizeTracker.java | 73 ++++++++++++----------
.../jogamp/opengl/GLBufferStateTracker.java | 43 ++++++-------
src/jogl/classes/jogamp/opengl/GLContextImpl.java | 25 +++-----
.../classes/jogamp/opengl/GLContextShareSet.java | 24 -------
4 files changed, 73 insertions(+), 92 deletions(-)
diff --git a/src/jogl/classes/jogamp/opengl/GLBufferSizeTracker.java b/src/jogl/classes/jogamp/opengl/GLBufferSizeTracker.java
index fa05902d5..b6d9b5682 100644
--- a/src/jogl/classes/jogamp/opengl/GLBufferSizeTracker.java
+++ b/src/jogl/classes/jogamp/opengl/GLBufferSizeTracker.java
@@ -45,47 +45,56 @@ import com.jogamp.common.util.IntLongHashMap;
/**
* 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, 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
+ * objects.
+ * <p>
+ * <code>glMapBuffer</code> or <code>glMapBufferRange</code> etc
+ * returns a <code>java.nio.ByteBuffer</code>
+ * instance reflecting the returned native address of respective calls
+ * and the actual buffer size.
+ * </p>
+ * <p>
+ * In case the buffer size is unknown, we need to compute this size by using
+ * <code>glGetBufferParameteriv</code> with a pname of <code>GL_BUFFER_SIZE</code>.
+ * The latter appears to be problematic due to the returned <code>int</code> value,
+ * where size should be of type <code>long</code>.
+ * Further more, this query appears to be costly for each glMapBuffer call
+ * at for Apple's new multithreaded OpenGL implementation.
+ * </p>
+ * <p>
+ * The buffer size state is shared across all shared OpenGL context,
+ * hence we share the GLBufferSizeTracker instance across all shared GLContexts.
+ * Hence utilizing this instance must be synchronized to be thread safe due to multithreading usage.
+ * </p>
+ * <p>
+ * We track the sizes of allocated buffer objects.
+ * We track calls to <code>glBindBuffer</code> etc to see which buffer is bound to
+ * which target and to <code>glBufferData</code> to see how large the buffer's
+ * allocated size is. When <code>glMapBuffer</code> 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
+ * call.
+ * </p>
+ * <p>
+ * 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;
+ * see, for example, <code>glVertexPointer</code> and the calls down to
+ * <code>GLBufferStateTracker.getBoundBufferObject()</code>. Note that we currently
+ * ignore new binding targets such as <code>GL_TRANSFORM_FEEDBACK_BUFFER_NV</code>;
* the fact that new binding targets may be added in the future makes
- * it impossible to cache state for these new targets. <P>
- *
+ * it impossible to cache state for these new targets.
+ * </p>
+ * <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
+ * where <code>glBindBuffer</code> may not have been called before trying to call
+ * <code>glBufferData</code>. 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.)
+ * </p>
*/
-
public class GLBufferSizeTracker {
protected static final boolean DEBUG;
@@ -102,11 +111,11 @@ public class GLBufferSizeTracker {
// pattern of buffer objects indicates that the fact that this map
// never shrinks is probably not that bad.
private final IntLongHashMap bufferSizeMap;
- private final long keyNotFount = 0xFFFFFFFFFFFFFFFFL;
+ private final long sizeNotFount = 0xFFFFFFFFFFFFFFFFL;
public GLBufferSizeTracker() {
bufferSizeMap = new IntLongHashMap();
- bufferSizeMap.setKeyNotFoundValue(keyNotFount);
+ bufferSizeMap.setKeyNotFoundValue(sizeNotFount);
}
public final void setBufferSize(GLBufferStateTracker bufferStateTracker,
@@ -155,7 +164,7 @@ public class GLBufferSizeTracker {
// point we almost certainly should if the application is
// written correctly
long sz = bufferSizeMap.get(buffer);
- if (keyNotFount == sz) {
+ if (sizeNotFount == sz) {
// For robustness, try to query this value from the GL as we used to
// FIXME: both functions return 'int' types, which is not suitable,
// since buffer lenght is 64bit ?
diff --git a/src/jogl/classes/jogamp/opengl/GLBufferStateTracker.java b/src/jogl/classes/jogamp/opengl/GLBufferStateTracker.java
index 16b7edca8..8f79990a8 100644
--- a/src/jogl/classes/jogamp/opengl/GLBufferStateTracker.java
+++ b/src/jogl/classes/jogamp/opengl/GLBufferStateTracker.java
@@ -50,8 +50,13 @@ import com.jogamp.common.util.IntIntHashMap;
* GLBufferStateTracker objects are allocated on a per-OpenGL-context basis.
* 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>
- *
+ * taking a long as argument is called.
+ * <p>
+ * The buffer binding state is local to it's OpenGL context,
+ * i.e. not shared across multiple OpenGL context.
+ * Hence this code is thread safe due to no multithreading usage.
+ * </p>
+ * <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.
@@ -61,19 +66,15 @@ import com.jogamp.common.util.IntIntHashMap;
* 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
+ * re-associate the queried size with the buffer object ID.
+ * </p>
+ * <p>
+ * For <i>unknown</i> 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.
- *
+ * </p>
*/
public class GLBufferStateTracker {
@@ -90,13 +91,13 @@ public class GLBufferStateTracker {
// OpenGL specifications.
// http://www.opengl.org/sdk/docs/man/xhtml/glBindBuffer.xml
private final IntIntHashMap bindingMap;
- private final int keyNotFound = 0xFFFFFFFF;
+ private final int bindingNotFound = 0xFFFFFFFF;
private final int[] bufTmp = new int[1];
public GLBufferStateTracker() {
bindingMap = new IntIntHashMap();
- bindingMap.setKeyNotFoundValue(keyNotFound);
+ bindingMap.setKeyNotFoundValue(bindingNotFound);
// Start with known unbound targets for known keys
// setBoundBufferObject(GL2ES3.GL_VERTEX_ARRAY_BINDING, 0); // not using default VAO (removed in GL3 core) - only explicit
@@ -139,20 +140,20 @@ public class GLBufferStateTracker {
return value is valid. */
public final int getBoundBufferObject(int target, GL caller) {
int value = bindingMap.get(target);
- if (keyNotFound == value) {
+ if (bindingNotFound == value) {
// 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;
+ final int queryTarget;
switch (target) {
case GL2ES3.GL_VERTEX_ARRAY_BINDING: queryTarget = GL2ES3.GL_VERTEX_ARRAY_BINDING; break;
- 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 GL2.GL_PIXEL_PACK_BUFFER: queryTarget = GL2.GL_PIXEL_PACK_BUFFER_BINDING; break;
- case GL2.GL_PIXEL_UNPACK_BUFFER: queryTarget = GL2.GL_PIXEL_UNPACK_BUFFER_BINDING; break;
- case GL4.GL_DRAW_INDIRECT_BUFFER: queryTarget = GL4.GL_DRAW_INDIRECT_BUFFER_BINDING; break;
- default: gotQueryTarget = false; break;
+ 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 GL2.GL_PIXEL_PACK_BUFFER: queryTarget = GL2.GL_PIXEL_PACK_BUFFER_BINDING; break;
+ case GL2.GL_PIXEL_UNPACK_BUFFER: queryTarget = GL2.GL_PIXEL_UNPACK_BUFFER_BINDING; break;
+ case GL4.GL_DRAW_INDIRECT_BUFFER: queryTarget = GL4.GL_DRAW_INDIRECT_BUFFER_BINDING; break;
+ default: queryTarget = 0; gotQueryTarget = false; break;
}
if (gotQueryTarget) {
final int glerrPre = caller.glGetError(); // clear
diff --git a/src/jogl/classes/jogamp/opengl/GLContextImpl.java b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
index 8f1ba4585..66eed9d96 100644
--- a/src/jogl/classes/jogamp/opengl/GLContextImpl.java
+++ b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
@@ -102,8 +102,8 @@ public abstract class GLContextImpl extends GLContext {
// Tracks creation and initialization of buffer objects to avoid
// repeated glGet calls upon glMapBuffer operations
- private GLBufferSizeTracker bufferSizeTracker; // Singleton - Set by GLContextShareSet
- private final GLBufferStateTracker bufferStateTracker = new GLBufferStateTracker();
+ private final GLBufferSizeTracker bufferSizeTracker;
+ private final GLBufferStateTracker bufferStateTracker;
private final GLStateTracker glStateTracker = new GLStateTracker();
private GLDebugMessageHandler glDebugHandler = null;
private final int[] boundFBOTarget = new int[] { 0, 0 }; // { draw, read }
@@ -138,10 +138,14 @@ public abstract class GLContextImpl extends GLContext {
public GLContextImpl(GLDrawableImpl drawable, GLContext shareWith) {
super();
- if (shareWith != null) {
+ bufferStateTracker = new GLBufferStateTracker();
+ if ( null != shareWith ) {
GLContextShareSet.registerSharing(this, shareWith);
+ bufferSizeTracker = ((GLContextImpl)shareWith).getBufferSizeTracker();
+ assert (bufferSizeTracker != null) : "shared context hash null bufferSizeTracker: "+shareWith;
+ } else {
+ bufferSizeTracker = new GLBufferSizeTracker();
}
- GLContextShareSet.synchronizeBufferObjectSharing(shareWith, this);
this.drawable = drawable;
this.drawableRead = drawable;
@@ -150,13 +154,8 @@ public abstract class GLContextImpl extends GLContext {
}
private final void clearStates() {
- // 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
- if (bufferSizeTracker != null) {
- bufferSizeTracker.clearCachedBufferSizes();
+ if( !GLContextShareSet.hasCreatedSharedLeft(this) ) {
+ bufferSizeTracker.clearCachedBufferSizes();
}
bufferStateTracker.clearBufferObjectState();
glStateTracker.setEnabled(false);
@@ -2123,10 +2122,6 @@ public abstract class GLContextImpl extends GLContext {
//----------------------------------------------------------------------
// Helpers for buffer object optimizations
- public final void setBufferSizeTracker(GLBufferSizeTracker bufferSizeTracker) {
- this.bufferSizeTracker = bufferSizeTracker;
- }
-
public final GLBufferSizeTracker getBufferSizeTracker() {
return bufferSizeTracker;
}
diff --git a/src/jogl/classes/jogamp/opengl/GLContextShareSet.java b/src/jogl/classes/jogamp/opengl/GLContextShareSet.java
index 483767b44..c057c904c 100644
--- a/src/jogl/classes/jogamp/opengl/GLContextShareSet.java
+++ b/src/jogl/classes/jogamp/opengl/GLContextShareSet.java
@@ -262,30 +262,6 @@ public class GLContextShareSet {
return false;
}
- /** 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 synchronizeBufferObjectSharing(final GLContext olderContextOrNull, final GLContext newContext) {
- final GLContextImpl older = (GLContextImpl) olderContextOrNull;
- final 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
--
cgit v1.2.3