From 9b1a0e2f58f3e2380c5c1992b48b629f20b72610 Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Thu, 16 Apr 2020 09:00:54 -0700 Subject: [PATCH] Remove of static JNI fields (#2400) Static JNI lookups were causing issues with multiple libraries. We now do the lookups when we need them as they are effectively just hashmap lookups and we do them only in places where the work we need to perform will be much larger than a simple hash map lookup anyway. This chane also manually registers filament-utils JNI bindings to get rid of unnecessary symboles. We should do the same for other Filament libraries (the symbols are pretty long and we now have many of them). --- android/common/CallbackUtils.cpp | 64 ++++++++++--------- android/common/CallbackUtils.h | 11 ++++ android/common/NioUtils.cpp | 53 +++++++-------- android/common/NioUtils.h | 8 +++ .../src/main/cpp/Filament.cpp | 10 +-- .../src/main/cpp/Material.cpp | 32 ++++------ .../src/main/cpp/RenderableManager.cpp | 1 - .../src/main/cpp/Utils.cpp | 51 +++++++-------- android/gltfio-android/CMakeLists.txt | 1 - .../gltfio-android/src/main/cpp/Gltfio.cpp | 42 ------------ 10 files changed, 114 insertions(+), 159 deletions(-) delete mode 100644 android/gltfio-android/src/main/cpp/Gltfio.cpp diff --git a/android/common/CallbackUtils.cpp b/android/common/CallbackUtils.cpp index 905f66e2f7..03ea6aaecd 100644 --- a/android/common/CallbackUtils.cpp +++ b/android/common/CallbackUtils.cpp @@ -18,14 +18,19 @@ #include "CallbackUtils.h" -struct { +static void initCallbackJni(JNIEnv* env, CallbackJni& callbackUtils) { #ifdef ANDROID - jclass handlerClass; - jmethodID post; + callbackUtils.handlerClass = env->FindClass("android/os/Handler"); + callbackUtils.handlerClass = (jclass) env->NewGlobalRef(callbackUtils.handlerClass); + callbackUtils.post = env->GetMethodID(callbackUtils.handlerClass, + "post", "(Ljava/lang/Runnable;)Z"); #endif - jclass executorClass; - jmethodID execute; -} gCallbackUtils; + + callbackUtils.executorClass = env->FindClass("java/util/concurrent/Executor"); + callbackUtils.executorClass = (jclass) env->NewGlobalRef(callbackUtils.executorClass); + callbackUtils.execute = env->GetMethodID(callbackUtils.executorClass, + "execute", "(Ljava/lang/Runnable;)V"); +} JniBufferCallback* JniBufferCallback::make(filament::Engine* engine, JNIEnv* env, jobject handler, jobject callback, AutoBuffer&& buffer) { @@ -38,22 +43,27 @@ JniBufferCallback::JniBufferCallback(JNIEnv* env, jobject handler, jobject callb : mEnv(env) , mHandler(env->NewGlobalRef(handler)) , mCallback(env->NewGlobalRef(callback)) - , mBuffer(std::move(buffer)){ + , mBuffer(std::move(buffer)) { + initCallbackJni(env, mCallbackUtils); } JniBufferCallback::~JniBufferCallback() { if (mHandler && mCallback) { #ifdef ANDROID - if (mEnv->IsInstanceOf(mHandler, gCallbackUtils.handlerClass)) { - mEnv->CallBooleanMethod(mHandler, gCallbackUtils.post, mCallback); + if (mEnv->IsInstanceOf(mHandler, mCallbackUtils.handlerClass)) { + mEnv->CallBooleanMethod(mHandler, mCallbackUtils.post, mCallback); } #endif - if (mEnv->IsInstanceOf(mHandler, gCallbackUtils.executorClass)) { - mEnv->CallVoidMethod(mHandler, gCallbackUtils.execute, mCallback); + if (mEnv->IsInstanceOf(mHandler, mCallbackUtils.executorClass)) { + mEnv->CallVoidMethod(mHandler, mCallbackUtils.execute, mCallback); } } mEnv->DeleteGlobalRef(mHandler); mEnv->DeleteGlobalRef(mCallback); +#ifdef ANDROID + mEnv->DeleteGlobalRef(mCallbackUtils.handlerClass); +#endif + mEnv->DeleteGlobalRef(mCallbackUtils.executorClass); } void JniBufferCallback::invoke(void*, size_t, void* user) { @@ -72,37 +82,29 @@ JniImageCallback::JniImageCallback(JNIEnv* env, jobject handler, jobject callbac : mEnv(env) , mHandler(env->NewGlobalRef(handler)) , mCallback(env->NewGlobalRef(callback)) - , mImage(image) { } + , mImage(image) { + initCallbackJni(env, mCallbackUtils); +} JniImageCallback::~JniImageCallback() { if (mHandler && mCallback) { #ifdef ANDROID - if (mEnv->IsInstanceOf(mHandler, gCallbackUtils.handlerClass)) { - mEnv->CallBooleanMethod(mHandler, gCallbackUtils.post, mCallback); + if (mEnv->IsInstanceOf(mHandler, mCallbackUtils.handlerClass)) { + mEnv->CallBooleanMethod(mHandler, mCallbackUtils.post, mCallback); } #endif - if (mEnv->IsInstanceOf(mHandler, gCallbackUtils.executorClass)) { - mEnv->CallVoidMethod(mHandler, gCallbackUtils.execute, mCallback); + if (mEnv->IsInstanceOf(mHandler, mCallbackUtils.executorClass)) { + mEnv->CallVoidMethod(mHandler, mCallbackUtils.execute, mCallback); } } mEnv->DeleteGlobalRef(mHandler); mEnv->DeleteGlobalRef(mCallback); +#ifdef ANDROID + mEnv->DeleteGlobalRef(mCallbackUtils.handlerClass); +#endif + mEnv->DeleteGlobalRef(mCallbackUtils.executorClass); } -void JniImageCallback::invoke(void* image, void* user) { +void JniImageCallback::invoke(void*, void* user) { reinterpret_cast(user)->~JniImageCallback(); } - -void registerCallbackUtils(JNIEnv *env) { -#ifdef ANDROID - gCallbackUtils.handlerClass = env->FindClass("android/os/Handler"); - gCallbackUtils.handlerClass = (jclass) env->NewGlobalRef(gCallbackUtils.handlerClass); - gCallbackUtils.post = env->GetMethodID(gCallbackUtils.handlerClass, - "post", "(Ljava/lang/Runnable;)Z"); -#endif - - gCallbackUtils.executorClass = env->FindClass("java/util/concurrent/Executor"); - gCallbackUtils.executorClass = (jclass) env->NewGlobalRef(gCallbackUtils.executorClass); - gCallbackUtils.execute = env->GetMethodID(gCallbackUtils.executorClass, - "execute", "(Ljava/lang/Runnable;)V"); -} diff --git a/android/common/CallbackUtils.h b/android/common/CallbackUtils.h index f9cd1ec625..4ef710c682 100644 --- a/android/common/CallbackUtils.h +++ b/android/common/CallbackUtils.h @@ -23,6 +23,15 @@ #include +struct CallbackJni { +#ifdef ANDROID + jclass handlerClass; + jmethodID post; +#endif + jclass executorClass; + jmethodID execute; +}; + struct JniBufferCallback { static JniBufferCallback* make(filament::Engine* engine, JNIEnv* env, jobject handler, jobject callback, AutoBuffer&& buffer); @@ -37,6 +46,7 @@ private: jobject mHandler; jobject mCallback; AutoBuffer mBuffer; + CallbackJni mCallbackUtils; }; struct JniImageCallback { @@ -53,4 +63,5 @@ private: jobject mHandler; jobject mCallback; long mImage; + CallbackJni mCallbackUtils; }; diff --git a/android/common/NioUtils.cpp b/android/common/NioUtils.cpp index be4f637723..d060463a2a 100644 --- a/android/common/NioUtils.cpp +++ b/android/common/NioUtils.cpp @@ -18,20 +18,28 @@ #include -struct { - jclass jniClass; - jmethodID getBasePointer; - jmethodID getBaseArray; - jmethodID getBaseArrayOffset; - jmethodID getBufferType; -} gNioUtils; +#include -AutoBuffer::AutoBuffer(JNIEnv *env, jobject buffer, jint size, bool commit) noexcept : mEnv(env), +AutoBuffer::AutoBuffer(JNIEnv *env, jobject buffer, jint size, bool commit) noexcept : + mEnv(env), mDoCommit(commit) { + + mNioUtils.jniClass = env->FindClass("com/google/android/filament/NioUtils"); + mNioUtils.jniClass = (jclass) env->NewGlobalRef(mNioUtils.jniClass); + + mNioUtils.getBasePointer = env->GetStaticMethodID(mNioUtils.jniClass, + "getBasePointer", "(Ljava/nio/Buffer;JI)J"); + mNioUtils.getBaseArray = env->GetStaticMethodID(mNioUtils.jniClass, + "getBaseArray", "(Ljava/nio/Buffer;)Ljava/lang/Object;"); + mNioUtils.getBaseArrayOffset = env->GetStaticMethodID(mNioUtils.jniClass, + "getBaseArrayOffset", "(Ljava/nio/Buffer;I)I"); + mNioUtils.getBufferType = env->GetStaticMethodID(mNioUtils.jniClass, + "getBufferType", "(Ljava/nio/Buffer;)I"); + mBuffer = env->NewGlobalRef(buffer); mType = (BufferType) env->CallStaticIntMethod( - gNioUtils.jniClass, gNioUtils.getBufferType, mBuffer); + mNioUtils.jniClass, mNioUtils.getBufferType, mBuffer); switch (mType) { case BufferType::BYTE: @@ -56,16 +64,16 @@ AutoBuffer::AutoBuffer(JNIEnv *env, jobject buffer, jint size, bool commit) noex jlong address = (jlong) env->GetDirectBufferAddress(mBuffer); if (address) { // Direct buffer case - mData = reinterpret_cast(env->CallStaticLongMethod(gNioUtils.jniClass, - gNioUtils.getBasePointer, mBuffer, address, mShift)); + mData = reinterpret_cast(env->CallStaticLongMethod(mNioUtils.jniClass, + mNioUtils.getBasePointer, mBuffer, address, mShift)); mUserData = mData; } else { // wrapped array case - jarray array = (jarray) env->CallStaticObjectMethod(gNioUtils.jniClass, - gNioUtils.getBaseArray, mBuffer); + jarray array = (jarray) env->CallStaticObjectMethod(mNioUtils.jniClass, + mNioUtils.getBaseArray, mBuffer); - jint offset = env->CallStaticIntMethod(gNioUtils.jniClass, - gNioUtils.getBaseArrayOffset, mBuffer, mShift); + jint offset = env->CallStaticIntMethod(mNioUtils.jniClass, + mNioUtils.getBaseArrayOffset, mBuffer, mShift); mBaseArray = (jarray) env->NewGlobalRef(array); switch (mType) { @@ -138,18 +146,5 @@ AutoBuffer::~AutoBuffer() noexcept { if (mBuffer) { env->DeleteGlobalRef(mBuffer); } -} - -void registerNioUtils(JNIEnv *env) { - gNioUtils.jniClass = env->FindClass("com/google/android/filament/NioUtils"); - gNioUtils.jniClass = (jclass) env->NewGlobalRef(gNioUtils.jniClass); - - gNioUtils.getBasePointer = env->GetStaticMethodID(gNioUtils.jniClass, "getBasePointer", - "(Ljava/nio/Buffer;JI)J"); - gNioUtils.getBaseArray = env->GetStaticMethodID(gNioUtils.jniClass, "getBaseArray", - "(Ljava/nio/Buffer;)Ljava/lang/Object;"); - gNioUtils.getBaseArrayOffset = env->GetStaticMethodID(gNioUtils.jniClass, "getBaseArrayOffset", - "(Ljava/nio/Buffer;I)I"); - gNioUtils.getBufferType = env->GetStaticMethodID(gNioUtils.jniClass, "getBufferType", - "(Ljava/nio/Buffer;)I"); + mEnv->DeleteGlobalRef(mNioUtils.jniClass); } diff --git a/android/common/NioUtils.h b/android/common/NioUtils.h index 1ae7af4617..2f407cc8fe 100644 --- a/android/common/NioUtils.h +++ b/android/common/NioUtils.h @@ -65,4 +65,12 @@ private: jobject mBuffer = nullptr; jarray mBaseArray = nullptr; bool mDoCommit = false; + + struct { + jclass jniClass; + jmethodID getBasePointer; + jmethodID getBaseArray; + jmethodID getBaseArrayOffset; + jmethodID getBufferType; + } mNioUtils; }; diff --git a/android/filament-android/src/main/cpp/Filament.cpp b/android/filament-android/src/main/cpp/Filament.cpp index b72135f251..03ac36b840 100644 --- a/android/filament-android/src/main/cpp/Filament.cpp +++ b/android/filament-android/src/main/cpp/Filament.cpp @@ -16,24 +16,16 @@ #include -extern void registerCallbackUtils(JNIEnv*); -extern void registerMaterial(JNIEnv*); -extern void registerNioUtils(JNIEnv*); - namespace filament { extern jint JNI_OnLoad(JavaVM* vm, void* reserved); }; -jint JNI_OnLoad(JavaVM* vm, void* reserved) { +JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { JNIEnv* env; if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6) != JNI_OK) { return -1; } - registerCallbackUtils(env); - registerMaterial(env); - registerNioUtils(env); - #if ANDROID ::filament::JNI_OnLoad(vm, reserved); #endif diff --git a/android/filament-android/src/main/cpp/Material.cpp b/android/filament-android/src/main/cpp/Material.cpp index 6331520d46..6fda58f1b6 100644 --- a/android/filament-android/src/main/cpp/Material.cpp +++ b/android/filament-android/src/main/cpp/Material.cpp @@ -22,12 +22,6 @@ using namespace filament; -struct { - jclass parameterClass; - jmethodID parameterAdd; - jfieldID parameterSamplerOffset; -} gMaterial; - extern "C" JNIEXPORT jlong JNICALL Java_com_google_android_filament_Material_nBuilderBuild(JNIEnv *env, jclass, jlong nativeEngine, jobject buffer_, jint size) { @@ -191,16 +185,27 @@ Java_com_google_android_filament_Material_nGetParameters(JNIEnv* env, jclass, size_t received = material->getParameters(info, (size_t) count); assert(received == count); - jint offset = env->GetStaticIntField(gMaterial.parameterClass, gMaterial.parameterSamplerOffset); + jclass parameterClass = env->FindClass("com/google/android/filament/Material$Parameter"); + parameterClass = (jclass) env->NewLocalRef(parameterClass); + + jmethodID parameterAdd = env->GetStaticMethodID(parameterClass, "add", + "(Ljava/util/List;Ljava/lang/String;III)V"); + + jfieldID parameterSamplerOffset = env->GetStaticFieldID(parameterClass, + "SAMPLER_OFFSET", "I"); + + jint offset = env->GetStaticIntField(parameterClass, parameterSamplerOffset); for (size_t i = 0; i < received; i++) { jint type = info[i].isSampler ? (jint) info[i].samplerType + offset : (jint) info[i].type; env->CallStaticVoidMethod( - gMaterial.parameterClass, gMaterial.parameterAdd, + parameterClass, parameterAdd, parameters, env->NewStringUTF(info[i].name), type, (jint) info[i].precision, (jint) info[i].count); } + env->DeleteLocalRef(parameterClass); + delete[] info; } @@ -221,14 +226,3 @@ Java_com_google_android_filament_Material_nHasParameter(JNIEnv* env, jclass, env->ReleaseStringUTFChars(name_, name); return (jboolean) hasParameter; } - -void registerMaterial(JNIEnv *env) { - gMaterial.parameterClass = env->FindClass("com/google/android/filament/Material$Parameter"); - gMaterial.parameterClass = (jclass) env->NewGlobalRef(gMaterial.parameterClass); - - gMaterial.parameterAdd = env->GetStaticMethodID(gMaterial.parameterClass, "add", - "(Ljava/util/List;Ljava/lang/String;III)V"); - - gMaterial.parameterSamplerOffset = env->GetStaticFieldID(gMaterial.parameterClass, - "SAMPLER_OFFSET", "I"); -} diff --git a/android/filament-android/src/main/cpp/RenderableManager.cpp b/android/filament-android/src/main/cpp/RenderableManager.cpp index 642e4ec19b..d93719d701 100644 --- a/android/filament-android/src/main/cpp/RenderableManager.cpp +++ b/android/filament-android/src/main/cpp/RenderableManager.cpp @@ -14,7 +14,6 @@ * limitations under the License. */ - #include #include diff --git a/android/filament-utils-android/src/main/cpp/Utils.cpp b/android/filament-utils-android/src/main/cpp/Utils.cpp index 4af63b5e5c..1e30099568 100644 --- a/android/filament-utils-android/src/main/cpp/Utils.cpp +++ b/android/filament-utils-android/src/main/cpp/Utils.cpp @@ -28,29 +28,7 @@ using namespace filament; using namespace filament::math; using namespace image; -extern void registerCallbackUtils(JNIEnv*); -extern void registerNioUtils(JNIEnv*); - -namespace gltfio { - void JNI_OnLoad(JNIEnv* env); -} - -jint JNI_OnLoad(JavaVM* vm, void*) { - JNIEnv* env; - if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6) != JNI_OK) { - return -1; - } - - gltfio::JNI_OnLoad(env); - - registerCallbackUtils(env); - registerNioUtils(env); - - return JNI_VERSION_1_6; -} - -extern "C" JNIEXPORT jlong JNICALL -Java_com_google_android_filament_utils_KtxLoader_nCreateTexture(JNIEnv* env, jclass, +static jlong nCreateTexture(JNIEnv* env, jclass, jlong nativeEngine, jobject javaBuffer, jint remaining, jboolean srgb) { Engine* engine = (Engine*) nativeEngine; AutoBuffer buffer(env, javaBuffer, remaining); @@ -61,8 +39,7 @@ Java_com_google_android_filament_utils_KtxLoader_nCreateTexture(JNIEnv* env, jcl }, bundle); } -extern "C" JNIEXPORT jlong JNICALL -Java_com_google_android_filament_utils_KtxLoader_nCreateIndirectLight(JNIEnv* env, jclass, +static jlong nCreateIndirectLight(JNIEnv* env, jclass, jlong nativeEngine, jobject javaBuffer, jint remaining, jboolean srgb) { Engine* engine = (Engine*) nativeEngine; AutoBuffer buffer(env, javaBuffer, remaining); @@ -84,8 +61,7 @@ Java_com_google_android_filament_utils_KtxLoader_nCreateIndirectLight(JNIEnv* en return (jlong) indirectLight; } -extern "C" JNIEXPORT jlong JNICALL -Java_com_google_android_filament_utils_KtxLoader_nCreateSkybox(JNIEnv* env, jclass, +static jlong nCreateSkybox(JNIEnv* env, jclass, jlong nativeEngine, jobject javaBuffer, jint remaining, jboolean srgb) { Engine* engine = (Engine*) nativeEngine; AutoBuffer buffer(env, javaBuffer, remaining); @@ -96,3 +72,24 @@ Java_com_google_android_filament_utils_KtxLoader_nCreateSkybox(JNIEnv* env, jcla }, bundle); return (jlong) Skybox::Builder().environment(cubemap).showSun(true).build(*engine); } + + +JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void*) { + JNIEnv* env; + if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6) != JNI_OK) { + return -1; + } + + jclass c = env->FindClass("com/google/android/filament/utils/KtxLoader"); + if (c == nullptr) return JNI_ERR; + + static const JNINativeMethod methods[] = { + {"nCreateTexture", "(JLjava/nio/Buffer;IZ)J", reinterpret_cast(nCreateTexture)}, + {"nCreateIndirectLight", "(JLjava/nio/Buffer;IZ)J", reinterpret_cast(nCreateIndirectLight)}, + {"nCreateSkybox", "(JLjava/nio/Buffer;IZ)J", reinterpret_cast(nCreateSkybox)}, + }; + int rc = env->RegisterNatives(c, methods, sizeof(methods) / sizeof(JNINativeMethod)); + if (rc != JNI_OK) return rc; + + return JNI_VERSION_1_6; +} diff --git a/android/gltfio-android/CMakeLists.txt b/android/gltfio-android/CMakeLists.txt index 26cf549485..6e3d601e0c 100644 --- a/android/gltfio-android/CMakeLists.txt +++ b/android/gltfio-android/CMakeLists.txt @@ -55,7 +55,6 @@ set(GLTFIO_SRCS src/main/cpp/FilamentAsset.cpp src/main/cpp/MaterialProvider.cpp src/main/cpp/ResourceLoader.cpp - src/main/cpp/Gltfio.cpp ${FILAMENT_DIR}/include/gltfio/resources/gltfresources_lite.h ${FILAMENT_DIR}/include/gltfio/resources/gltfresources.h diff --git a/android/gltfio-android/src/main/cpp/Gltfio.cpp b/android/gltfio-android/src/main/cpp/Gltfio.cpp deleted file mode 100644 index 282489c00b..0000000000 --- a/android/gltfio-android/src/main/cpp/Gltfio.cpp +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (C) 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include - -#include - -extern void registerNioUtils(JNIEnv*); - -jint JNI_OnLoad(JavaVM* vm, void*) { - JNIEnv* env; - if (vm->GetEnv(reinterpret_cast(&env), JNI_VERSION_1_6) != JNI_OK) { - return -1; - } - - registerNioUtils(env); - - return JNI_VERSION_1_6; -} - -// This alternative init function is necessary when gltfio is loaded using an ELF dependency. -namespace gltfio { - - UTILS_PUBLIC - void JNI_OnLoad(JNIEnv* env) { - registerNioUtils(env); - } - -}