Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[android] jni clean up - missing a couple DeleteLocalRef #11253

Merged
merged 1 commit into from
Feb 21, 2018

Conversation

osana
Copy link
Contributor

@osana osana commented Feb 20, 2018

No description provided.

@osana osana requested a review from tobrun February 20, 2018 03:58
@osana
Copy link
Contributor Author

osana commented Feb 20, 2018

@tobrun please look at the changes. I am not 100% sure about them - it was easier to do then discuss.

return *conversion::convert<mbgl::Color, int>(env, polygon.Get(env, field));
auto jColor = polygon.Get(env, field);
mbgl::Color color = *conversion::convert<mbgl::Color, int>(env, jColor);
jni::DeleteLocalRef(env, jColor);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jColor is of the type jint and these are just aliases for normal native value types, so there are no special lifetime or cleanup considerations.

@@ -34,6 +34,7 @@ mbgl::CameraOptions CameraPosition::getCameraOptions(jni::JNIEnv& env, jni::Obje
static auto zoom = CameraPosition::javaClass.GetField<jni::jdouble>(env, "zoom");

auto center = LatLng::getLatLng(env, position.Get(env, target));
jni::DeleteLocalRef(env, target);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great find!

auto name = jni::Make<std::string>(env, image.Get(env, nameField));
auto jName = image.Get(env, nameField);
auto name = jni::Make<std::string>(env, jName);
jni::DeleteLocalRef(env, jName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great find as well, do we need to delete the pixels local ref?

Copy link
Contributor Author

@osana osana Feb 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobrun Turns out that
jni::GetArrayRegion(env, *pixels, 0, size, reinterpret_cast<jbyte*>(premultipliedImage.data.get())); does the release as per https://developer.android.com/training/articles/perf-jni.html

@osana osana force-pushed the osana-DelLocalRefs-more branch 3 times, most recently from 529b126 to 2f3fa19 Compare February 21, 2018 13:56
@osana osana merged commit c2d9d6a into master Feb 21, 2018
@osana osana added the Android Mapbox Maps SDK for Android label Feb 27, 2018
@osana osana added this to the android-v6.0.0 milestone Feb 27, 2018
@osana osana deleted the osana-DelLocalRefs-more branch February 27, 2018 17:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants