Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e20ea783 authored May 07, 2009 by Jack Palevich

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.

parent 538bcd70

core/jni/android_opengl_GLES10.cpp

+28 −16

Original line number	Diff line number	Diff line
		@@ -291,7 +291,13 @@ android_glColorPointerBounds__IIILjava_nio_Buffer_2I
		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 @@ android_glColorPointerBounds__IIILjava_nio_Buffer_2I
		(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 @@ android_glNormalPointerBounds__IILjava_nio_Buffer_2I
		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 @@ android_glTexCoordPointerBounds__IIILjava_nio_Buffer_2I
		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 @@ android_glTexCoordPointerBounds__IIILjava_nio_Buffer_2I
		(GLvoid *)pointer,
		(GLsizei)remaining
		);
		if (_array) {
		releasePointer(_env, _array, pointer, JNI_FALSE);
		}
		}

		/* void glTexEnvf ( GLenum target, GLenum pname, GLfloat param ) */
		@@ -3369,7 +3378,13 @@ android_glVertexPointerBounds__IIILjava_nio_Buffer_2I
		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 @@ android_glVertexPointerBounds__IIILjava_nio_Buffer_2I
		(GLvoid *)pointer,
		(GLsizei)remaining
		);
		if (_array) {
		releasePointer(_env, _array, pointer, JNI_FALSE);
		}
		}

		/* void glViewport ( GLint x, GLint y, GLsizei width, GLsizei height ) */

core/jni/com_google_android_gles_jni_GLImpl.cpp

+28 −16

Original line number	Diff line number	Diff line
		@@ -291,7 +291,13 @@ android_glColorPointerBounds__IIILjava_nio_Buffer_2I
		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 @@ android_glColorPointerBounds__IIILjava_nio_Buffer_2I
		(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 @@ android_glNormalPointerBounds__IILjava_nio_Buffer_2I
		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 @@ android_glTexCoordPointerBounds__IIILjava_nio_Buffer_2I
		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 @@ android_glTexCoordPointerBounds__IIILjava_nio_Buffer_2I
		(GLvoid *)pointer,
		(GLsizei)remaining
		);
		if (_array) {
		releasePointer(_env, _array, pointer, JNI_FALSE);
		}
		}

		/* void glTexEnvf ( GLenum target, GLenum pname, GLfloat param ) */
		@@ -3369,7 +3378,13 @@ android_glVertexPointerBounds__IIILjava_nio_Buffer_2I
		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 @@ android_glVertexPointerBounds__IIILjava_nio_Buffer_2I
		(GLvoid *)pointer,
		(GLsizei)remaining
		);
		if (_array) {
		releasePointer(_env, _array, pointer, JNI_FALSE);
		}
		}

		/* void glViewport ( GLint x, GLint y, GLsizei width, GLsizei height ) */

opengl/java/android/opengl/GLES10.java

+27 −27

Original line number	Diff line number	Diff line
		@@ -395,13 +395,6 @@ public class GLES10 {
		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 @@ public class GLES10 {
		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 @@ public class GLES10 {
		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 @@ public class GLES10 {
		(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 @@ public class GLES10 {
		int stride,
		java.nio.Buffer pointer
		) {
		glTexCoordPointerBounds(
		size,
		type,
		stride,
		pointer,
		pointer.remaining()
		);
		if (((size == 2) \|\|
		(size == 3) \|\|
		(size == 4)) &&
		@@ -1159,13 +1166,6 @@ public class GLES10 {
		(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 @@ public class GLES10 {
		int stride,
		java.nio.Buffer pointer
		) {
		glVertexPointerBounds(
		size,
		type,
		stride,
		pointer,
		pointer.remaining()
		);
		if (((size == 2) \|\|
		(size == 3) \|\|
		(size == 4)) &&
		@@ -1304,13 +1311,6 @@ public class GLES10 {
		(stride >= 0)) {
		_vertexPointer = pointer;
		}
		glVertexPointerBounds(
		size,
		type,
		stride,
		pointer,
		pointer.remaining()
		);
		}

		// C function void glViewport ( GLint x, GLint y, GLsizei width, GLsizei height )

opengl/java/com/google/android/gles_jni/GLImpl.java

+27 −27

Original line number	Diff line number	Diff line
		@@ -172,13 +172,6 @@ public class GLImpl implements GL10, GL10Ext, GL11, GL11Ext, GL11ExtensionPack {
		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 @@ public class GLImpl implements GL10, GL10Ext, GL11, GL11Ext, GL11ExtensionPack {
		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 @@ public class GLImpl implements GL10, GL10Ext, GL11, GL11Ext, GL11ExtensionPack {
		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 @@ public class GLImpl implements GL10, GL10Ext, GL11, GL11Ext, GL11ExtensionPack {
		(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 @@ public class GLImpl implements GL10, GL10Ext, GL11, GL11Ext, GL11ExtensionPack {
		int stride,
		java.nio.Buffer pointer
		) {
		glTexCoordPointerBounds(
		size,
		type,
		stride,
		pointer,
		pointer.remaining()
		);
		if (((size == 2) \|\|
		(size == 3) \|\|
		(size == 4)) &&
		@@ -947,13 +954,6 @@ public class GLImpl implements GL10, GL10Ext, GL11, GL11Ext, GL11ExtensionPack {
		(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 @@ public class GLImpl implements GL10, GL10Ext, GL11, GL11Ext, GL11ExtensionPack {
		int stride,
		java.nio.Buffer pointer
		) {
		glVertexPointerBounds(
		size,
		type,
		stride,
		pointer,
		pointer.remaining()
		);
		if (((size == 2) \|\|
		(size == 3) \|\|
		(size == 4)) &&
		@@ -1092,13 +1099,6 @@ public class GLImpl implements GL10, GL10Ext, GL11, GL11Ext, GL11ExtensionPack {
		(stride >= 0)) {
		_vertexPointer = pointer;
		}
		glVertexPointerBounds(
		size,
		type,
		stride,
		pointer,
		pointer.remaining()
		);
		}

		// C function void glViewport ( GLint x, GLint y, GLsizei width, GLsizei height )

opengl/tools/glgen/src/JniCodeEmitter.java

+48 −19

Original line number	Diff line number	Diff line
		@@ -454,6 +454,15 @@ public class JniCodeEmitter {

		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 @@ public class JniCodeEmitter {
		}
		}

		// emitBoundsChecks(jfunc, out, iii);
		emitFunctionCall(jfunc, out, iii, false);

		boolean isVoid = jfunc.getType().isVoid();

		if (!isVoid) {
		@@ -873,19 +879,39 @@ public class JniCodeEmitter {
		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);
		}

		if (isPointerFunc) {
		out.println(indent +
		cname +
		" = (" +
		cfunc.getArgType(cIndex).getDeclaration() +
		") _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 + ");");
		"_buf, &" + array + ", &" + remaining +
		");");
		}

		if (nullAllowed) {
		out.println(indent + "}");
		@@ -987,6 +1013,7 @@ public class JniCodeEmitter {
		");");
		out.println(indent + "}");
		} else if (jfunc.getArgType(idx).isBuffer()) {
		if (! isPointerFunc) {
		String array = numBufferArgs <= 1 ? "_array" :
		"_" + bufferArgNames.get(bufArgIdx++) + "Array";
		out.println(indent + "if (" + array + ") {");
		@@ -995,12 +1022,14 @@ public class JniCodeEmitter {
		cfunc.getArgName(cIndex) +
		", " +
		(cfunc.getArgType(cIndex).isConst() ?
		"JNI_FALSE" : "_exception ? JNI_FALSE : JNI_TRUE") +
		"JNI_FALSE" : "_exception ? JNI_FALSE :" +
		" JNI_TRUE") +
		");");
		out.println(indent + "}");
		}
		}
		}
		}

		if (!isVoid) {
		out.println(indent + "return _returnValue;");