From 691daeb45d268c312bcaec409d5238435a7a7e4c Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Mon, 11 Feb 2019 13:03:58 -0800 Subject: [PATCH] JNI enhancement: allow mutation of non-direct buffers. The bug was reproduced and the fix was verified by copying the following code snippet into one of our Android samples: val norm = floatArrayOf(1.0f, 0.0f, 0.0f) val tang = floatArrayOf(0.0f, 1.0f, 0.0f, 1.0f) val expected = floatArrayOf(0.5f, 0.5f, 0.5f, 0.5f) val resultBuffer = FloatBuffer.allocate(4) val qtc = VertexBuffer.QuatTangentContext() qtc.quatCount = 1 qtc.quatType = VertexBuffer.QuatType.FLOAT4 qtc.outBuffer = resultBuffer qtc.normals = FloatBuffer.wrap(norm) qtc.tangents = FloatBuffer.wrap(tang) VertexBuffer.populateTangentQuaternions(qtc) Log.e("Filament", "${expected[0]} == ${resultBuffer[0]}") Fixes #695. --- .../filament-android/src/main/cpp/NioUtils.cpp | 18 ++++++++++-------- .../filament-android/src/main/cpp/NioUtils.h | 5 ++++- .../src/main/cpp/VertexBuffer.cpp | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/android/filament-android/src/main/cpp/NioUtils.cpp b/android/filament-android/src/main/cpp/NioUtils.cpp index 37d77d3bee..2a69aaf812 100644 --- a/android/filament-android/src/main/cpp/NioUtils.cpp +++ b/android/filament-android/src/main/cpp/NioUtils.cpp @@ -26,7 +26,8 @@ struct { jmethodID getBufferType; } gNioUtils; -AutoBuffer::AutoBuffer(JNIEnv *env, jobject buffer, jint size) noexcept : mEnv(env) { +AutoBuffer::AutoBuffer(JNIEnv *env, jobject buffer, jint size, bool commit) noexcept : mEnv(env), + mDoCommit(commit) { mBuffer = env->NewGlobalRef(buffer); mType = (BufferType) env->CallStaticIntMethod( @@ -108,27 +109,28 @@ AutoBuffer::AutoBuffer(AutoBuffer &&rhs) noexcept { AutoBuffer::~AutoBuffer() noexcept { JNIEnv *env = mEnv; if (mBaseArray) { + jint mode = mDoCommit ? 0 : JNI_ABORT; switch (mType) { case BufferType::BYTE: - env->ReleaseByteArrayElements((jbyteArray)mBaseArray, (jbyte *) mData, JNI_ABORT); + env->ReleaseByteArrayElements((jbyteArray)mBaseArray, (jbyte *) mData, mode); break; case BufferType::CHAR: - env->ReleaseCharArrayElements((jcharArray)mBaseArray, (jchar *) mData, JNI_ABORT); + env->ReleaseCharArrayElements((jcharArray)mBaseArray, (jchar *) mData, mode); break; case BufferType::SHORT: - env->ReleaseShortArrayElements((jshortArray)mBaseArray, (jshort *) mData, JNI_ABORT); + env->ReleaseShortArrayElements((jshortArray)mBaseArray, (jshort *) mData, mode); break; case BufferType::INT: - env->ReleaseIntArrayElements((jintArray)mBaseArray, (jint *) mData, JNI_ABORT); + env->ReleaseIntArrayElements((jintArray)mBaseArray, (jint *) mData, mode); break; case BufferType::LONG: - env->ReleaseLongArrayElements((jlongArray)mBaseArray, (jlong *) mData, JNI_ABORT); + env->ReleaseLongArrayElements((jlongArray)mBaseArray, (jlong *) mData, mode); break; case BufferType::FLOAT: - env->ReleaseFloatArrayElements((jfloatArray)mBaseArray, (jfloat *) mData, JNI_ABORT); + env->ReleaseFloatArrayElements((jfloatArray)mBaseArray, (jfloat *) mData, mode); break; case BufferType::DOUBLE: - env->ReleaseDoubleArrayElements((jdoubleArray)mBaseArray, (jdouble *) mData, JNI_ABORT); + env->ReleaseDoubleArrayElements((jdoubleArray)mBaseArray, (jdouble *) mData, mode); break; } env->DeleteGlobalRef(mBaseArray); diff --git a/android/filament-android/src/main/cpp/NioUtils.h b/android/filament-android/src/main/cpp/NioUtils.h index b444ccb058..1ae7af4617 100644 --- a/android/filament-android/src/main/cpp/NioUtils.h +++ b/android/filament-android/src/main/cpp/NioUtils.h @@ -32,7 +32,9 @@ public: DOUBLE }; - AutoBuffer(JNIEnv* env, jobject buffer, jint size) noexcept; + // Clients should pass "true" for the commit argument if they intend to mutate the buffer + // contents from native code. + AutoBuffer(JNIEnv* env, jobject buffer, jint size, bool commit = false) noexcept; AutoBuffer(AutoBuffer&& rhs) noexcept; ~AutoBuffer() noexcept; @@ -62,4 +64,5 @@ private: void* mData = nullptr; jobject mBuffer = nullptr; jarray mBaseArray = nullptr; + bool mDoCommit = false; }; diff --git a/android/filament-android/src/main/cpp/VertexBuffer.cpp b/android/filament-android/src/main/cpp/VertexBuffer.cpp index 5b99567540..93310e41f4 100644 --- a/android/filament-android/src/main/cpp/VertexBuffer.cpp +++ b/android/filament-android/src/main/cpp/VertexBuffer.cpp @@ -121,7 +121,7 @@ Java_com_google_android_filament_VertexBuffer_nPopulateTangentQuaternions(JNIEnv jint outStride, jobject normals, jint normalsRemaining, jint normalsStride, jobject tangents, jint tangentsRemaining, jint tangentsStride) { - AutoBuffer outNioBuffer(env, outBuffer, outRemaining); + AutoBuffer outNioBuffer(env, outBuffer, outRemaining, true); void* outData = outNioBuffer.getData(); AutoBuffer normalsNioBuffer(env, normals, normalsRemaining);