diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md index 39c956329..b5f65b787 100644 --- a/platform/android/CHANGELOG.md +++ b/platform/android/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog MapLibre Native for Android +## 11.10.3 + +### 🐞 Bug fixes + +- Revert fix [#3536](https://github.com/maplibre/maplibre-native/pull/3536) due to `getSource` crashes. + ## 11.10.2 ### 🐞 Bug fixes diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/custom_geometry_source.cpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/custom_geometry_source.cpp index 25cdd9798..79b3116ef 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/custom_geometry_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/custom_geometry_source.cpp @@ -141,18 +141,18 @@ void CustomGeometrySource::setTileData( // Update the core source if not cancelled if (!isCancelled(z, x, y)) { - getSource().CustomGeometrySource::setTileData(CanonicalTileID(z, x, y), - GeoJSON(geometry)); + source.as()->CustomGeometrySource::setTileData(CanonicalTileID(z, x, y), + GeoJSON(geometry)); } } void CustomGeometrySource::invalidateTile(jni::JNIEnv&, jni::jint z, jni::jint x, jni::jint y) { - getSource().CustomGeometrySource::invalidateTile(CanonicalTileID(z, x, y)); + source.as()->CustomGeometrySource::invalidateTile(CanonicalTileID(z, x, y)); } void CustomGeometrySource::invalidateBounds(jni::JNIEnv& env, const jni::Object& jBounds) { auto bounds = LatLngBounds::getLatLngBounds(env, jBounds); - getSource().CustomGeometrySource::invalidateRegion(bounds); + source.as()->CustomGeometrySource::invalidateRegion(bounds); } jni::Local>> CustomGeometrySource::querySourceFeatures( @@ -162,7 +162,7 @@ jni::Local>> CustomGeometrySource::quer std::vector features; if (rendererFrontend) { - features = rendererFrontend->querySourceFeatures(getSource().getID(), {{}, toFilter(env, jfilter)}); + features = rendererFrontend->querySourceFeatures(source.getID(), {{}, toFilter(env, jfilter)}); } return Feature::convert(env, features); } diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.cpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.cpp index cb51a1326..bc37a5f5c 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.cpp @@ -50,12 +50,12 @@ GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, const jni::String& sourceId, cons std::make_unique(jni::Make(env, sourceId), convertGeoJSONOptions(env, options))), converter(std::make_unique>(Scheduler::GetBackground(), - getSource().impl().getOptions())) {} + source.as()->impl().getOptions())) {} GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, AndroidRendererFrontend* frontend) : Source(env, coreSource, createJavaPeer(env), frontend), converter(std::make_unique>(Scheduler::GetBackground(), - getSource().impl().getOptions())) {} + source.as()->impl().getOptions())) {} GeoJSONSource::~GeoJSONSource() = default; @@ -83,11 +83,11 @@ void GeoJSONSource::setGeometry(jni::JNIEnv& env, const jni::Object().setURL(jni::Make(env, url)); + source.as()->setURL(jni::Make(env, url)); } jni::Local GeoJSONSource::getURL(jni::JNIEnv& env) { - std::optional url = getSource().getURL(); + std::optional url = source.as()->getURL(); return url ? jni::Make(env, *url) : jni::Local(); } @@ -98,7 +98,7 @@ jni::Local>> GeoJSONSource::querySource std::vector features; if (rendererFrontend) { - features = rendererFrontend->querySourceFeatures(getSource().getID(), {{}, toFilter(env, jfilter)}); + features = rendererFrontend->querySourceFeatures(source.getID(), {{}, toFilter(env, jfilter)}); } return Feature::convert(env, features); } @@ -112,7 +112,7 @@ jni::Local>> GeoJSONSource::getClusterC mbgl::Feature _feature = Feature::convert(env, feature); _feature.properties["cluster_id"] = static_cast(_feature.properties["cluster_id"].get()); const auto featureExtension = rendererFrontend->queryFeatureExtensions( - getSource().getID(), _feature, "supercluster", "children", {}); + source.getID(), _feature, "supercluster", "children", {}); if (featureExtension.is()) { return Feature::convert(env, featureExtension.get()); } @@ -131,7 +131,7 @@ jni::Local>> GeoJSONSource::getClusterL const std::map options = {{"limit", static_cast(limit)}, {"offset", static_cast(offset)}}; auto featureExtension = rendererFrontend->queryFeatureExtensions( - getSource().getID(), _feature, "supercluster", "leaves", options); + source.getID(), _feature, "supercluster", "leaves", options); if (featureExtension.is()) { return Feature::convert(env, featureExtension.get()); } @@ -148,7 +148,7 @@ jint GeoJSONSource::getClusterExpansionZoom(jni::JNIEnv& env, const jni::Object< mbgl::Feature _feature = Feature::convert(env, feature); _feature.properties["cluster_id"] = static_cast(_feature.properties["cluster_id"].get()); auto featureExtension = rendererFrontend->queryFeatureExtensions( - getSource().getID(), _feature, "supercluster", "expansion-zoom", {}); + source.getID(), _feature, "supercluster", "expansion-zoom", {}); if (featureExtension.is()) { auto value = featureExtension.get(); if (value.is()) { @@ -181,20 +181,12 @@ void GeoJSONSource::setAsync(Update::Converter converterFn) { awaitingUpdate = std::make_unique( std::move(converterFn), std::make_unique>( - *Scheduler::GetCurrent(), - [awaitingUpdateWeak = std::weak_ptr(awaitingUpdate), - sourceWeak = getWeakSource(), - updateWeak = std::weak_ptr(update)](std::shared_ptr geoJSONData) { - auto awaitingUpdate = awaitingUpdateWeak.lock(); - auto source = sourceWeak.lock(); - auto update = updateWeak.lock(); - if (!awaitingUpdate || !source || !update) return; - + *Scheduler::GetCurrent(), [this](std::shared_ptr geoJSONData) { // conversion from Java features to core ones finished android::UniqueEnv _env = android::AttachEnv(); // Update the core source - source->get().as()->setGeoJSONData(std::move(geoJSONData)); + source.as()->setGeoJSONData(std::move(geoJSONData)); // if there is an awaiting update, execute it, otherwise, release resources if (awaitingUpdate) { diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.hpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.hpp index ac4999bb5..6a360484b 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.hpp @@ -70,8 +70,8 @@ private: jni::Local getURL(jni::JNIEnv&); jni::Local> createJavaPeer(jni::JNIEnv&); - std::shared_ptr awaitingUpdate; - std::shared_ptr update; + std::unique_ptr awaitingUpdate; + std::unique_ptr update; std::shared_ptr threadPool; std::unique_ptr> converter; diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/image_source.cpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/image_source.cpp index f1f6559e4..8d278b318 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/image_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/image_source.cpp @@ -29,20 +29,20 @@ ImageSource::~ImageSource() = default; void ImageSource::setURL(jni::JNIEnv& env, const jni::String& url) { // Update the core source - getSource().ImageSource::setURL(jni::Make(env, url)); + source.as()->ImageSource::setURL(jni::Make(env, url)); } jni::Local ImageSource::getURL(jni::JNIEnv& env) { - std::optional url = getSource().ImageSource::getURL(); + std::optional url = source.as()->ImageSource::getURL(); return url ? jni::Make(env, *url) : jni::Local(); } void ImageSource::setImage(jni::JNIEnv& env, const jni::Object& bitmap) { - getSource().setImage(Bitmap::GetImage(env, bitmap)); + source.as()->setImage(Bitmap::GetImage(env, bitmap)); } void ImageSource::setCoordinates(jni::JNIEnv& env, const jni::Object& coordinatesObject) { - getSource().setCoordinates(LatLngQuad::getLatLngArray(env, coordinatesObject)); + source.as()->setCoordinates(LatLngQuad::getLatLngArray(env, coordinatesObject)); } jni::Local> ImageSource::createJavaPeer(jni::JNIEnv& env) { diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/raster_dem_source.cpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/raster_dem_source.cpp index 56bad65aa..107c96a06 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/raster_dem_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/raster_dem_source.cpp @@ -26,7 +26,7 @@ RasterDEMSource::RasterDEMSource(jni::JNIEnv& env, mbgl::style::Source& coreSour RasterDEMSource::~RasterDEMSource() = default; jni::Local RasterDEMSource::getURL(jni::JNIEnv& env) { - auto url = getSource().RasterDEMSource::getURL(); + std::optional url = source.as()->RasterDEMSource::getURL(); return url ? jni::Make(env, *url) : jni::Local(); } diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/raster_source.cpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/raster_source.cpp index cba024237..d023fca6d 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/raster_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/raster_source.cpp @@ -25,7 +25,7 @@ RasterSource::RasterSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, An RasterSource::~RasterSource() = default; jni::Local RasterSource::getURL(jni::JNIEnv& env) { - auto url = getSource().RasterSource::getURL(); + std::optional url = source.as()->RasterSource::getURL(); return url ? jni::Make(env, *url) : jni::Local(); } diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/source.cpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/source.cpp index af11e2a59..ec7026d44 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/source.cpp @@ -70,13 +70,13 @@ Source::Source(jni::JNIEnv& env, mbgl::style::Source& coreSource, const jni::Object& obj, AndroidRendererFrontend* frontend) - : source(std::make_shared>(coreSource)), + : source(coreSource), javaPeer(jni::NewGlobal(env, obj)), rendererFrontend(frontend) {} Source::Source(jni::JNIEnv&, std::unique_ptr coreSource) : ownedSource(std::move(coreSource)), - source(std::make_shared>(*ownedSource)) {} + source(*ownedSource) {} Source::~Source() { if (ownedSource) { @@ -102,24 +102,24 @@ Source::~Source() { } jni::Local Source::getId(jni::JNIEnv& env) { - return jni::Make(env, source->get().getID()); + return jni::Make(env, source.getID()); } jni::Local Source::getAttribution(jni::JNIEnv& env) { - auto attribution = source->get().getAttribution(); + auto attribution = source.getAttribution(); return attribution ? jni::Make(env, attribution.value()) : jni::Make(env, ""); } void Source::setPrefetchZoomDelta(jni::JNIEnv& env, jni::Integer& delta) { if (!delta) { - source->get().setPrefetchZoomDelta(std::nullopt); + source.setPrefetchZoomDelta(std::nullopt); } else { - source->get().setPrefetchZoomDelta(jni::Unbox(env, delta)); + source.setPrefetchZoomDelta(jni::Unbox(env, delta)); } } jni::Local Source::getPrefetchZoomDelta(jni::JNIEnv& env) { - auto delta = source->get().getPrefetchZoomDelta(); + auto delta = source.getPrefetchZoomDelta(); if (delta.has_value()) { return jni::Box(env, jni::jint(delta.value())); } @@ -128,14 +128,14 @@ jni::Local Source::getPrefetchZoomDelta(jni::JNIEnv& env) { void Source::setMaxOverscaleFactorForParentTiles(jni::JNIEnv& env, jni::Integer& maxOverscaleFactor) { if (!maxOverscaleFactor) { - source->get().setMaxOverscaleFactorForParentTiles(std::nullopt); + source.setMaxOverscaleFactorForParentTiles(std::nullopt); } else { - source->get().setMaxOverscaleFactorForParentTiles(jni::Unbox(env, maxOverscaleFactor)); + source.setMaxOverscaleFactorForParentTiles(jni::Unbox(env, maxOverscaleFactor)); } } jni::Local Source::getMaxOverscaleFactorForParentTiles(jni::JNIEnv& env) { - auto maxOverscaleFactor = source->get().getMaxOverscaleFactorForParentTiles(); + auto maxOverscaleFactor = source.getMaxOverscaleFactorForParentTiles(); if (maxOverscaleFactor) { return jni::Box(env, jni::jint(*maxOverscaleFactor)); } @@ -151,7 +151,7 @@ void Source::addToStyle(JNIEnv& env, const jni::Object& obj, mbgl::style style.addSource(std::move(ownedSource)); // Add peer to core source - source->get().peer = std::unique_ptr(this); + source.peer = std::unique_ptr(this); // Add strong reference to java source javaPeer = jni::NewGlobal(env, obj); @@ -167,7 +167,7 @@ void Source::addToMap(JNIEnv& env, const jni::Object& obj, mbgl::Map& ma map.getStyle().addSource(std::move(ownedSource)); // Add peer to core source - source->get().peer = std::unique_ptr(this); + source.peer = std::unique_ptr(this); // Add strong reference to java source javaPeer = jni::NewGlobal(env, obj); @@ -182,26 +182,26 @@ bool Source::removeFromMap(JNIEnv&, const jni::Object&, mbgl::Map& map) } // Remove the source from the map and take ownership - ownedSource = map.getStyle().removeSource(source->get().getID()); + ownedSource = map.getStyle().removeSource(source.getID()); // The source may not be removed if any layers still reference it return ownedSource != nullptr; } jni::Local Source::isVolatile(jni::JNIEnv& env) { - return jni::Box(env, jni::jboolean(source->get().isVolatile())); + return jni::Box(env, jni::jboolean(source.isVolatile())); } void Source::setVolatile(JNIEnv& env, jni::Boolean& value) { - source->get().setVolatile(jni::Unbox(env, value)); + source.setVolatile(jni::Unbox(env, value)); } void Source::setMinimumTileUpdateInterval(JNIEnv& env, jni::Long& interval) { - source->get().setMinimumTileUpdateInterval(Milliseconds(jni::Unbox(env, interval))); + source.setMinimumTileUpdateInterval(Milliseconds(jni::Unbox(env, interval))); } jni::Local Source::getMinimumTileUpdateInterval(JNIEnv& env) { - return jni::Box(env, jni::jlong(source->get().getMinimumTileUpdateInterval().count() / 1000000)); + return jni::Box(env, jni::jlong(source.getMinimumTileUpdateInterval().count() / 1000000)); } void Source::releaseJavaPeer() { diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/source.hpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/source.hpp index 8c9243f62..b7782c348 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/source.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/source.hpp @@ -8,7 +8,6 @@ #include "../../android_renderer_frontend.hpp" #include -#include namespace mbgl { namespace android { @@ -64,21 +63,12 @@ public: jni::Local getMinimumTileUpdateInterval(JNIEnv&); protected: - template - T& getSource() { - return *ownedSource->as(); - } - - std::weak_ptr> getWeakSource() const { return source; } - -private: // Set on newly created sources until added to the map. std::unique_ptr ownedSource; - // Reference that is valid at all times. - std::shared_ptr> source; + // Raw pointer that is valid at all times. + mbgl::style::Source& source; -protected: // Set when the source is added to a map. jni::Global> javaPeer; diff --git a/platform/android/MapLibreAndroid/src/cpp/style/sources/vector_source.cpp b/platform/android/MapLibreAndroid/src/cpp/style/sources/vector_source.cpp index e3e77d57c..01e092f83 100644 --- a/platform/android/MapLibreAndroid/src/cpp/style/sources/vector_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/style/sources/vector_source.cpp @@ -31,7 +31,7 @@ VectorSource::VectorSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, An VectorSource::~VectorSource() = default; jni::Local VectorSource::getURL(jni::JNIEnv& env) { - std::optional url = getSource().VectorSource::getURL(); + std::optional url = source.as()->VectorSource::getURL(); return url ? jni::Make(env, *url) : jni::Local(); } @@ -42,7 +42,7 @@ jni::Local>> VectorSource::querySourceF std::vector features; if (rendererFrontend) { - features = rendererFrontend->querySourceFeatures(getSource().getID(), + features = rendererFrontend->querySourceFeatures(source.getID(), {toVector(env, jSourceLayerIds), toFilter(env, jfilter)}); } return Feature::convert(env, features); diff --git a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/GeoJsonSourceTests.java b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/GeoJsonSourceTests.java index 04fbfc2a9..9f75618c0 100644 --- a/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/GeoJsonSourceTests.java +++ b/platform/android/MapLibreAndroidTestApp/src/androidTest/java/org/maplibre/android/testapp/style/GeoJsonSourceTests.java @@ -1,5 +1,6 @@ package org.maplibre.android.testapp.style; +import org.junit.Ignore; import org.maplibre.android.style.sources.CannotAddSourceException; import org.maplibre.geojson.Feature; import org.maplibre.geojson.FeatureCollection; @@ -157,6 +158,7 @@ public class GeoJsonSourceTests extends EspressoTest { } @Test + @Ignore("https://github.com/maplibre/maplibre-native/issues/3493") public void testDuplicateSourceDuringAsyncSetGeoJson() { // regression test for segfault when setting GeoJSON contents // for a source that has been deleted diff --git a/platform/android/VERSION b/platform/android/VERSION index bd0f144b3..d5b7da553 100644 --- a/platform/android/VERSION +++ b/platform/android/VERSION @@ -1 +1 @@ -11.10.2 +11.10.3