Require native-order direct buffers for glXXXPointer APIs.
This was always a documented restriction, but was not enforced by the runtime until now.
Until now, if you passed in some other kind of buffer, it would sometimes work, and
sometimes fail. The failures happened when the Java VM moved the buffer data while
OpenGL was still holding a pointer to it.
Now we throw an exception rather than leaving the system in a potentially bad state.
diff --git a/core/jni/android_opengl_GLES10.cpp b/core/jni/android_opengl_GLES10.cpp
index 6756580..7e388ef 100644
--- a/core/jni/android_opengl_GLES10.cpp
+++ b/core/jni/android_opengl_GLES10.cpp
@@ -291,7 +291,13 @@
jint _remaining;
GLvoid *pointer = (GLvoid *) 0;
- pointer = (GLvoid *)getPointer(_env, pointer_buf, &_array, &_remaining);
+ if (pointer_buf) {
+ pointer = (GLvoid *) _env->GetDirectBufferAddress(pointer_buf);
+ if ( ! pointer ) {
+ _env->ThrowNew(IAEClass, "Must use a native order direct Buffer");
+ return;
+ }
+ }
glColorPointerBounds(
(GLint)size,
(GLenum)type,
@@ -299,9 +305,6 @@
(GLvoid *)pointer,
(GLsizei)remaining
);
- if (_array) {
- releasePointer(_env, _array, pointer, JNI_FALSE);
- }
}
/* void glCompressedTexImage2D ( GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLsizei imageSize, const GLvoid *data ) */
@@ -2762,16 +2765,19 @@
jint _remaining;
GLvoid *pointer = (GLvoid *) 0;
- pointer = (GLvoid *)getPointer(_env, pointer_buf, &_array, &_remaining);
+ if (pointer_buf) {
+ pointer = (GLvoid *) _env->GetDirectBufferAddress(pointer_buf);
+ if ( ! pointer ) {
+ _env->ThrowNew(IAEClass, "Must use a native order direct Buffer");
+ return;
+ }
+ }
glNormalPointerBounds(
(GLenum)type,
(GLsizei)stride,
(GLvoid *)pointer,
(GLsizei)remaining
);
- if (_array) {
- releasePointer(_env, _array, pointer, JNI_FALSE);
- }
}
/* void glOrthof ( GLfloat left, GLfloat right, GLfloat bottom, GLfloat top, GLfloat zNear, GLfloat zFar ) */
@@ -3014,7 +3020,13 @@
jint _remaining;
GLvoid *pointer = (GLvoid *) 0;
- pointer = (GLvoid *)getPointer(_env, pointer_buf, &_array, &_remaining);
+ if (pointer_buf) {
+ pointer = (GLvoid *) _env->GetDirectBufferAddress(pointer_buf);
+ if ( ! pointer ) {
+ _env->ThrowNew(IAEClass, "Must use a native order direct Buffer");
+ return;
+ }
+ }
glTexCoordPointerBounds(
(GLint)size,
(GLenum)type,
@@ -3022,9 +3034,6 @@
(GLvoid *)pointer,
(GLsizei)remaining
);
- if (_array) {
- releasePointer(_env, _array, pointer, JNI_FALSE);
- }
}
/* void glTexEnvf ( GLenum target, GLenum pname, GLfloat param ) */
@@ -3369,7 +3378,13 @@
jint _remaining;
GLvoid *pointer = (GLvoid *) 0;
- pointer = (GLvoid *)getPointer(_env, pointer_buf, &_array, &_remaining);
+ if (pointer_buf) {
+ pointer = (GLvoid *) _env->GetDirectBufferAddress(pointer_buf);
+ if ( ! pointer ) {
+ _env->ThrowNew(IAEClass, "Must use a native order direct Buffer");
+ return;
+ }
+ }
glVertexPointerBounds(
(GLint)size,
(GLenum)type,
@@ -3377,9 +3392,6 @@
(GLvoid *)pointer,
(GLsizei)remaining
);
- if (_array) {
- releasePointer(_env, _array, pointer, JNI_FALSE);
- }
}
/* void glViewport ( GLint x, GLint y, GLsizei width, GLsizei height ) */
diff --git a/core/jni/com_google_android_gles_jni_GLImpl.cpp b/core/jni/com_google_android_gles_jni_GLImpl.cpp
index 7ad0b9e..6f2a438 100644
--- a/core/jni/com_google_android_gles_jni_GLImpl.cpp
+++ b/core/jni/com_google_android_gles_jni_GLImpl.cpp
@@ -291,7 +291,13 @@
jint _remaining;
GLvoid *pointer = (GLvoid *) 0;
- pointer = (GLvoid *)getPointer(_env, pointer_buf, &_array, &_remaining);
+ if (pointer_buf) {
+ pointer = (GLvoid *) _env->GetDirectBufferAddress(pointer_buf);
+ if ( ! pointer ) {
+ _env->ThrowNew(IAEClass, "Must use a native order direct Buffer");
+ return;
+ }
+ }
glColorPointerBounds(
(GLint)size,
(GLenum)type,
@@ -299,9 +305,6 @@
(GLvoid *)pointer,
(GLsizei)remaining
);
- if (_array) {
- releasePointer(_env, _array, pointer, JNI_FALSE);
- }
}
/* void glCompressedTexImage2D ( GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLsizei imageSize, const GLvoid *data ) */
@@ -2762,16 +2765,19 @@
jint _remaining;
GLvoid *pointer = (GLvoid *) 0;
- pointer = (GLvoid *)getPointer(_env, pointer_buf, &_array, &_remaining);
+ if (pointer_buf) {
+ pointer = (GLvoid *) _env->GetDirectBufferAddress(pointer_buf);
+ if ( ! pointer ) {
+ _env->ThrowNew(IAEClass, "Must use a native order direct Buffer");
+ return;
+ }
+ }
glNormalPointerBounds(
(GLenum)type,
(GLsizei)stride,
(GLvoid *)pointer,
(GLsizei)remaining
);
- if (_array) {
- releasePointer(_env, _array, pointer, JNI_FALSE);
- }
}
/* void glOrthof ( GLfloat left, GLfloat right, GLfloat bottom, GLfloat top, GLfloat zNear, GLfloat zFar ) */
@@ -3014,7 +3020,13 @@
jint _remaining;
GLvoid *pointer = (GLvoid *) 0;
- pointer = (GLvoid *)getPointer(_env, pointer_buf, &_array, &_remaining);
+ if (pointer_buf) {
+ pointer = (GLvoid *) _env->GetDirectBufferAddress(pointer_buf);
+ if ( ! pointer ) {
+ _env->ThrowNew(IAEClass, "Must use a native order direct Buffer");
+ return;
+ }
+ }
glTexCoordPointerBounds(
(GLint)size,
(GLenum)type,
@@ -3022,9 +3034,6 @@
(GLvoid *)pointer,
(GLsizei)remaining
);
- if (_array) {
- releasePointer(_env, _array, pointer, JNI_FALSE);
- }
}
/* void glTexEnvf ( GLenum target, GLenum pname, GLfloat param ) */
@@ -3369,7 +3378,13 @@
jint _remaining;
GLvoid *pointer = (GLvoid *) 0;
- pointer = (GLvoid *)getPointer(_env, pointer_buf, &_array, &_remaining);
+ if (pointer_buf) {
+ pointer = (GLvoid *) _env->GetDirectBufferAddress(pointer_buf);
+ if ( ! pointer ) {
+ _env->ThrowNew(IAEClass, "Must use a native order direct Buffer");
+ return;
+ }
+ }
glVertexPointerBounds(
(GLint)size,
(GLenum)type,
@@ -3377,9 +3392,6 @@
(GLvoid *)pointer,
(GLsizei)remaining
);
- if (_array) {
- releasePointer(_env, _array, pointer, JNI_FALSE);
- }
}
/* void glViewport ( GLint x, GLint y, GLsizei width, GLsizei height ) */
diff --git a/opengl/java/android/opengl/GLES10.java b/opengl/java/android/opengl/GLES10.java
index 147b60f..db52b82 100644
--- a/opengl/java/android/opengl/GLES10.java
+++ b/opengl/java/android/opengl/GLES10.java
@@ -395,13 +395,6 @@
int stride,
java.nio.Buffer pointer
) {
- if ((size == 4) &&
- ((type == GL_FLOAT) ||
- (type == GL_UNSIGNED_BYTE) ||
- (type == GL_FIXED)) &&
- (stride >= 0)) {
- _colorPointer = pointer;
- }
glColorPointerBounds(
size,
type,
@@ -409,6 +402,13 @@
pointer,
pointer.remaining()
);
+ if ((size == 4) &&
+ ((type == GL_FLOAT) ||
+ (type == GL_UNSIGNED_BYTE) ||
+ (type == GL_FIXED)) &&
+ (stride >= 0)) {
+ _colorPointer = pointer;
+ }
}
// C function void glCompressedTexImage2D ( GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLsizei imageSize, const GLvoid *data )
@@ -956,6 +956,12 @@
int stride,
java.nio.Buffer pointer
) {
+ glNormalPointerBounds(
+ type,
+ stride,
+ pointer,
+ pointer.remaining()
+ );
if (((type == GL_FLOAT) ||
(type == GL_BYTE) ||
(type == GL_SHORT) ||
@@ -963,12 +969,6 @@
(stride >= 0)) {
_normalPointer = pointer;
}
- glNormalPointerBounds(
- type,
- stride,
- pointer,
- pointer.remaining()
- );
}
// C function void glOrthof ( GLfloat left, GLfloat right, GLfloat bottom, GLfloat top, GLfloat zNear, GLfloat zFar )
@@ -1149,6 +1149,13 @@
int stride,
java.nio.Buffer pointer
) {
+ glTexCoordPointerBounds(
+ size,
+ type,
+ stride,
+ pointer,
+ pointer.remaining()
+ );
if (((size == 2) ||
(size == 3) ||
(size == 4)) &&
@@ -1159,13 +1166,6 @@
(stride >= 0)) {
_texCoordPointer = pointer;
}
- glTexCoordPointerBounds(
- size,
- type,
- stride,
- pointer,
- pointer.remaining()
- );
}
// C function void glTexEnvf ( GLenum target, GLenum pname, GLfloat param )
@@ -1294,6 +1294,13 @@
int stride,
java.nio.Buffer pointer
) {
+ glVertexPointerBounds(
+ size,
+ type,
+ stride,
+ pointer,
+ pointer.remaining()
+ );
if (((size == 2) ||
(size == 3) ||
(size == 4)) &&
@@ -1304,13 +1311,6 @@
(stride >= 0)) {
_vertexPointer = pointer;
}
- glVertexPointerBounds(
- size,
- type,
- stride,
- pointer,
- pointer.remaining()
- );
}
// C function void glViewport ( GLint x, GLint y, GLsizei width, GLsizei height )
diff --git a/opengl/java/com/google/android/gles_jni/GLImpl.java b/opengl/java/com/google/android/gles_jni/GLImpl.java
index 47f07d0..4e365ef 100644
--- a/opengl/java/com/google/android/gles_jni/GLImpl.java
+++ b/opengl/java/com/google/android/gles_jni/GLImpl.java
@@ -172,13 +172,6 @@
int stride,
java.nio.Buffer pointer
) {
- if ((size == 4) &&
- ((type == GL_FLOAT) ||
- (type == GL_UNSIGNED_BYTE) ||
- (type == GL_FIXED)) &&
- (stride >= 0)) {
- _colorPointer = pointer;
- }
glColorPointerBounds(
size,
type,
@@ -186,6 +179,13 @@
pointer,
pointer.remaining()
);
+ if ((size == 4) &&
+ ((type == GL_FLOAT) ||
+ (type == GL_UNSIGNED_BYTE) ||
+ (type == GL_FIXED)) &&
+ (stride >= 0)) {
+ _colorPointer = pointer;
+ }
}
// C function void glCompressedTexImage2D ( GLenum target, GLint level, GLenum internalformat, GLsizei width, GLsizei height, GLint border, GLsizei imageSize, const GLvoid *data )
@@ -744,6 +744,12 @@
int stride,
java.nio.Buffer pointer
) {
+ glNormalPointerBounds(
+ type,
+ stride,
+ pointer,
+ pointer.remaining()
+ );
if (((type == GL_FLOAT) ||
(type == GL_BYTE) ||
(type == GL_SHORT) ||
@@ -751,12 +757,6 @@
(stride >= 0)) {
_normalPointer = pointer;
}
- glNormalPointerBounds(
- type,
- stride,
- pointer,
- pointer.remaining()
- );
}
// C function void glOrthof ( GLfloat left, GLfloat right, GLfloat bottom, GLfloat top, GLfloat zNear, GLfloat zFar )
@@ -937,6 +937,13 @@
int stride,
java.nio.Buffer pointer
) {
+ glTexCoordPointerBounds(
+ size,
+ type,
+ stride,
+ pointer,
+ pointer.remaining()
+ );
if (((size == 2) ||
(size == 3) ||
(size == 4)) &&
@@ -947,13 +954,6 @@
(stride >= 0)) {
_texCoordPointer = pointer;
}
- glTexCoordPointerBounds(
- size,
- type,
- stride,
- pointer,
- pointer.remaining()
- );
}
// C function void glTexEnvf ( GLenum target, GLenum pname, GLfloat param )
@@ -1082,6 +1082,13 @@
int stride,
java.nio.Buffer pointer
) {
+ glVertexPointerBounds(
+ size,
+ type,
+ stride,
+ pointer,
+ pointer.remaining()
+ );
if (((size == 2) ||
(size == 3) ||
(size == 4)) &&
@@ -1092,13 +1099,6 @@
(stride >= 0)) {
_vertexPointer = pointer;
}
- glVertexPointerBounds(
- size,
- type,
- stride,
- pointer,
- pointer.remaining()
- );
}
// C function void glViewport ( GLint x, GLint y, GLsizei width, GLsizei height )
diff --git a/opengl/tools/glgen/src/JniCodeEmitter.java b/opengl/tools/glgen/src/JniCodeEmitter.java
index ef0dbd0..b0997b1 100644
--- a/opengl/tools/glgen/src/JniCodeEmitter.java
+++ b/opengl/tools/glgen/src/JniCodeEmitter.java
@@ -454,6 +454,15 @@
String iii = indent + indent;
+ // emitBoundsChecks(jfunc, out, iii);
+ emitFunctionCall(jfunc, out, iii, false);
+
+ // Set the pointer after we call the native code, so that if
+ // the native code throws an exception we don't modify the
+ // pointer. We assume that the native code is written so that
+ // if an exception is thrown, then the underlying glXXXPointer
+ // function will not have been called.
+
String fname = jfunc.getName();
if (isPointerFunc) {
// TODO - deal with VBO variants
@@ -498,9 +507,6 @@
}
}
- // emitBoundsChecks(jfunc, out, iii);
- emitFunctionCall(jfunc, out, iii, false);
-
boolean isVoid = jfunc.getType().isVoid();
if (!isVoid) {
@@ -873,19 +879,39 @@
String array = numBufferArgs <= 1 ? "_array" :
"_" + bufferArgNames.get(bufArgIdx++) + "Array";
- boolean nullAllowed = isNullAllowed(cfunc);
+ boolean nullAllowed = isNullAllowed(cfunc) || isPointerFunc;
if (nullAllowed) {
out.println(indent + "if (" + cname + "_buf) {");
out.print(indent);
}
- out.println(indent +
+ if (isPointerFunc) {
+ out.println(indent +
cname +
" = (" +
cfunc.getArgType(cIndex).getDeclaration() +
- ")getPointer(_env, " +
- cname +
- "_buf, &" + array + ", &" + remaining + ");");
+ ") _env->GetDirectBufferAddress(" +
+ (mUseCPlusPlus ? "" : "_env, ") +
+ cname + "_buf);");
+ String iii = " ";
+ out.println(iii + indent + "if ( ! " + cname + " ) {");
+ out.println(iii + iii + indent +
+ (mUseCPlusPlus ? "_env" : "(*_env)") +
+ "->ThrowNew(" +
+ (mUseCPlusPlus ? "" : "_env, ") +
+ "IAEClass, \"Must use a native order direct Buffer\");");
+ out.println(iii + iii + indent + "return;");
+ out.println(iii + indent + "}");
+ } else {
+ out.println(indent +
+ cname +
+ " = (" +
+ cfunc.getArgType(cIndex).getDeclaration() +
+ ")getPointer(_env, " +
+ cname +
+ "_buf, &" + array + ", &" + remaining +
+ ");");
+ }
if (nullAllowed) {
out.println(indent + "}");
@@ -987,17 +1013,20 @@
");");
out.println(indent + "}");
} else if (jfunc.getArgType(idx).isBuffer()) {
- String array = numBufferArgs <= 1 ? "_array" :
- "_" + bufferArgNames.get(bufArgIdx++) + "Array";
- out.println(indent + "if (" + array + ") {");
- out.println(indent + indent +
- "releasePointer(_env, " + array + ", " +
- cfunc.getArgName(cIndex) +
- ", " +
- (cfunc.getArgType(cIndex).isConst() ?
- "JNI_FALSE" : "_exception ? JNI_FALSE : JNI_TRUE") +
- ");");
- out.println(indent + "}");
+ if (! isPointerFunc) {
+ String array = numBufferArgs <= 1 ? "_array" :
+ "_" + bufferArgNames.get(bufArgIdx++) + "Array";
+ out.println(indent + "if (" + array + ") {");
+ out.println(indent + indent +
+ "releasePointer(_env, " + array + ", " +
+ cfunc.getArgName(cIndex) +
+ ", " +
+ (cfunc.getArgType(cIndex).isConst() ?
+ "JNI_FALSE" : "_exception ? JNI_FALSE :" +
+ " JNI_TRUE") +
+ ");");
+ out.println(indent + "}");
+ }
}
}
}