migrate to use mapbox-java3.0 #10920

osana · 2018-01-12T16:47:42Z

migrated to use mapbox-java3.0
old 2.2.9 telemetry is still used though

osana · 2018-01-26T17:33:01Z

This should be ready for review. In the test app: Query Source Features and Query feature properties seem to work. A couple of points to consider:

writing more test to make sure JNI changes are well tested (I would suggest to do it on a new ticket)
there are now more native - java calls due to the api changes in mapbox-java. If there is a performance implication we can bring back the fromLngLat methods that take double arrays (as now we are limited to List<Point> parameters).

osana · 2018-02-02T04:51:22Z

Still have one remaining crash that I cannot figure out.
@tobrun could you have a look.
It crashes in test app . styles: FillExtrusions

It crashes while executing in
FillExtrusionActivity.onCreate()

whiel doing : GeoJsonSource source = new GeoJsonSource("extrusion-source", domTower);
It is probably something very stupid . The problem seems to be while calling
nativeSetGeometry(geometry);

It crashes pretty reliably.

I see that Navigation repo has mapboxSdk 5.3 and mapbox-java3.0 which is bad match to have at the moment so it is important to resolve this. I think Navigation people just got luck that it works.

cc @tobrun

tobrun

Looks good, just one question and a couple nits related to commented code

tobrun · 2018-02-02T13:46:33Z

platform/android/src/geojson/line_string.hpp


 static constexpr auto Type() { return "LineString"; };

 static mapbox::geojson::line_string convert(jni::JNIEnv&, jni::Object<LineString>);

- static mapbox::geojson::line_string convert(jni::JNIEnv&, jni::Object<java::util::List/*<Position>*/>);
+ static mapbox::geojson::line_string convert(jni::JNIEnv&, jni::Object<java::util::List/*<Point>*/>);


commented code

tobrun · 2018-02-02T13:46:48Z

platform/android/src/geojson/multi_line_string.cpp

 }

 return multiLineString;
 }

-mapbox::geojson::multi_line_string MultiLineString::convert(jni::JNIEnv &env, jni::Object<java::util::List/*<java::util::List<Position>>*/> jPositionListsList) {
+mapbox::geojson::multi_line_string MultiLineString::convert(jni::JNIEnv &env, jni::Object<java::util::List/*<java::util::List<Position>>*/> jPointListsList) {


commented code

tobrun · 2018-02-02T13:48:27Z

platform/android/src/geojson/point.cpp

+ return point;
+}
+
+mapbox::geojson::point Point::convert(jni::JNIEnv &env, jni::Object<java::util::List/*<Double>*/> jDoubleList) {


commented code

tobrun · 2018-02-02T13:48:35Z

platform/android/src/geojson/point.cpp

+mapbox::geojson::point Point::convert(jni::JNIEnv &env, jni::Object<java::util::List/*<Double>*/> jDoubleList) {
+ auto jDoubleArray = java::util::List::toArray<double>(env, jDoubleList);
+ // static jni::jclass* javaClass = jni::NewGlobalRef(env, &jni::FindClass(env, "java/lang/Double")).release();
+ //static auto method = jni::GetMethodID(env, javaClass, "doubleValue", "()D");


commented code

tobrun · 2018-02-07T15:54:59Z

platform/android/MapboxGLAndroidSDK/build.gradle

@@ -3,6 +3,7 @@ apply plugin: 'com.android.library'
 dependencies {
 api dependenciesList.mapboxAndroidTelemetry
 api dependenciesList.mapboxJavaGeoJSON
+ api dependenciesList.mapboxJavaTurf


what is the reason why we need this in the SDK?

tobrun · 2018-02-07T15:55:58Z

platform/android/src/geojson/conversion/geometry.hpp

+//
+// return &jarray;
+// }
+


commented code above

old 2.2.9 telementry is still used though

osana · 2018-02-07T18:12:10Z

Updated PR with requested changes
@tobrun

tobrun

looks good, can you cherry-pick this to release-boba after merging?

osana · 2018-02-16T04:30:01Z

platform/android/src/geojson/polygon.cpp

- if (jPositionListsList) {
- auto multiLine = MultiLineString::convert(env, jPositionListsList);
+ if (jPointListsList) {
+ auto multiLine = MultiLineString::convert(env, jPointListsList);


@tobrun could it be that DeleteLocalRef() for multiline was and is missing here between lines 31 & 32

yes it seems that we aren't DeleteLocalRef on jPointListsList as we are doing in the other classes eg.

mapbox-gl-native/platform/android/src/geojson/multi_point.cpp

Line 17 in 0ee6c2c

jni::DeleteLocalRef(env, jPointListsList);

. Could you add that?

@tobrun Looks like a false alarm on my side:

/src/platform/android/src/geojson/polygon.cpp:32:34: error: no viable conversion from 'mapbox::geometry::multi_line_string<double, std::vector>' to 'jni::jobject *' jni::DeleteLocalRef(env, multiLine); ^~~~~~~~~```

osana added the Android Mapbox Maps SDK for Android label Jan 12, 2018

osana requested review from Guardiola31337, tobrun and cammace January 12, 2018 16:47

osana force-pushed the osana-mbjava3-10663 branch from 65f5c47 to 31dbec6 Compare January 18, 2018 03:43

osana removed request for Guardiola31337 and cammace January 18, 2018 03:44

osana force-pushed the osana-mbjava3-10663 branch 2 times, most recently from a315df3 to 6fd77bf Compare January 19, 2018 16:26

zugaldia mentioned this pull request Jan 23, 2018

[android] Integration of the new events library #10999

Merged

osana force-pushed the osana-mbjava3-10663 branch 8 times, most recently from d9a6051 to 58a21c4 Compare January 26, 2018 17:29

osana requested a review from cammace January 26, 2018 17:38

osana force-pushed the osana-mbjava3-10663 branch 3 times, most recently from 0d249d6 to e2b19fc Compare February 2, 2018 04:39

osana self-assigned this Feb 2, 2018

osana requested a review from ivovandongen February 2, 2018 14:47

osana force-pushed the osana-mbjava3-10663 branch 4 times, most recently from 7ec560e to 5788f61 Compare February 6, 2018 15:24

osana force-pushed the osana-mbjava3-10663 branch from 5788f61 to 77e7dcf Compare February 6, 2018 19:24

osana added the ✓ ready for review label Feb 6, 2018

osana force-pushed the osana-mbjava3-10663 branch from 77e7dcf to 8ff1cb3 Compare February 6, 2018 21:00

tobrun suggested changes Feb 7, 2018

View reviewed changes

osana force-pushed the osana-mbjava3-10663 branch from 8ff1cb3 to 489454f Compare February 7, 2018 16:38

osana changed the title ~~migrated to use mapbox-java3.0~~ migrate to use mapbox-java3.0 Feb 7, 2018

[android] migrated to use mapbox-java3.0

0ee6c2c

old 2.2.9 telementry is still used though

osana force-pushed the osana-mbjava3-10663 branch from 489454f to 0ee6c2c Compare February 7, 2018 16:59

tobrun approved these changes Feb 8, 2018

View reviewed changes

osana merged commit 4498917 into master Feb 8, 2018

osana deleted the osana-mbjava3-10663 branch February 8, 2018 13:44

osana mentioned this pull request Feb 8, 2018

migrated to use mapbox-java3.0 #11149

Merged

osana removed the ✓ ready for review label Feb 8, 2018

This was referenced Feb 9, 2018

6.0.0-beta.1 crashing at startup time #11125

Closed

Update Gradle in MAS 2.x.x mapbox/mapbox-java#716

Merged

tobrun mentioned this pull request Feb 13, 2018

Update to Mapbox Java 3.0 #10663

Closed

osana mentioned this pull request Feb 16, 2018

[Android] [6.0.0-beta.2] local reference table overflow (max=512) #11212

Closed

osana commented Feb 16, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate to use mapbox-java3.0 #10920

migrate to use mapbox-java3.0 #10920

osana commented Jan 12, 2018 •

edited

Loading

osana commented Jan 26, 2018 •

edited

Loading

osana commented Feb 2, 2018

tobrun left a comment

tobrun Feb 2, 2018

tobrun Feb 2, 2018

tobrun Feb 2, 2018

tobrun Feb 2, 2018

tobrun Feb 7, 2018

tobrun Feb 7, 2018

osana commented Feb 7, 2018

tobrun left a comment

osana Feb 16, 2018

tobrun Feb 16, 2018

osana Feb 16, 2018

migrate to use mapbox-java3.0 #10920

migrate to use mapbox-java3.0 #10920

Conversation

osana commented Jan 12, 2018 • edited Loading

osana commented Jan 26, 2018 • edited Loading

osana commented Feb 2, 2018

tobrun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osana commented Feb 7, 2018

tobrun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osana commented Jan 12, 2018 •

edited

Loading

osana commented Jan 26, 2018 •

edited

Loading