diff options
author | John Firebaugh <john.firebaugh@gmail.com> | 2017-11-21 11:07:43 -0800 |
---|---|---|
committer | John Firebaugh <john.firebaugh@gmail.com> | 2017-11-21 11:14:22 -0800 |
commit | 13ec42c58d974492ae6a1b7db290ba7dd6d0b314 (patch) | |
tree | 7be2d832217f1213a624309258ca03f97b709e50 | |
parent | 9e70ea05b78eb1997d6da72aeb3a4b4b3a9e9c8f (diff) | |
download | qtlocation-mapboxgl-upstream/jfirebaugh-source-peer-wip.tar.gz |
Clean up Java peer / C++ peer / core source managementupstream/jfirebaugh-source-peer-wip
9 files changed, 137 insertions, 233 deletions
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java index 6c1827a679..1307495a0e 100755 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java @@ -741,7 +741,11 @@ final class NativeMapView { if (isDestroyedOn("removeSource")) { return null; } - return nativeRemoveSourceById(sourceId); + Source source = getSource(sourceId); + if (source) { + removeSource(source); + } + return source; } public Source removeSource(@NonNull Source source) { diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp index 381b41fab5..ebbaa9f5f5 100755 --- a/platform/android/src/native_map_view.cpp +++ b/platform/android/src/native_map_view.cpp @@ -856,8 +856,7 @@ jni::Array<jni::Object<Source>> NativeMapView::getSources(JNIEnv& env) { jni::Array<jni::Object<Source>> jSources = jni::Array<jni::Object<Source>>::New(env, sources.size(), Source::javaClass); int index = 0; for (auto source : sources) { - auto jSource = jni::Object<Source>(createJavaSourcePeer(env, *rendererFrontend, *source)); - jSources.Set(env, index, jSource); + jSources.Set(env, index, Source::peerForCoreSource(env, *rendererFrontend, *source)); index++; } @@ -872,8 +871,7 @@ jni::Object<Source> NativeMapView::getSource(JNIEnv& env, jni::String sourceId) return jni::Object<Source>(); } - // Create and return the source's native peer - return jni::Object<Source>(createJavaSourcePeer(env, *rendererFrontend, *coreSource)); + return Source::peerForCoreSource(env, *rendererFrontend, *coreSource); } void NativeMapView::addSource(JNIEnv& env, jni::Object<Source> obj, jlong sourcePtr) { @@ -881,22 +879,12 @@ void NativeMapView::addSource(JNIEnv& env, jni::Object<Source> obj, jlong source Source *source = reinterpret_cast<Source *>(sourcePtr); try { - source->addToMap(env, obj, *map); - source->setRendererFrontend(*rendererFrontend); + source->addToMap(env, obj, *map, *rendererFrontend); } catch (const std::runtime_error& error) { jni::ThrowNew(env, jni::FindClass(env, "com/mapbox/mapboxsdk/style/sources/CannotAddSourceException"), error.what()); } } -jni::Object<Source> NativeMapView::removeSourceById(JNIEnv& env, jni::String id) { - std::unique_ptr<mbgl::style::Source> coreSource = map->getStyle().removeSource(jni::Make<std::string>(env, id)); - if (coreSource) { - return jni::Object<Source>(removeSourceFromMap(std::move(coreSource))); - } else { - return jni::Object<Source>(); - } -} - void NativeMapView::removeSource(JNIEnv& env, jni::Object<Source> obj, jlong sourcePtr) { assert(sourcePtr != 0); @@ -1044,7 +1032,6 @@ void NativeMapView::registerNative(jni::JNIEnv& env) { METHOD(&NativeMapView::getSources, "nativeGetSources"), METHOD(&NativeMapView::getSource, "nativeGetSource"), METHOD(&NativeMapView::addSource, "nativeAddSource"), - METHOD(&NativeMapView::removeSourceById, "nativeRemoveSourceById"), METHOD(&NativeMapView::removeSource, "nativeRemoveSource"), METHOD(&NativeMapView::addImage, "nativeAddImage"), METHOD(&NativeMapView::addImages, "nativeAddImages"), diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp index 99d047ff29..5a63db96c5 100755 --- a/platform/android/src/native_map_view.hpp +++ b/platform/android/src/native_map_view.hpp @@ -232,8 +232,6 @@ public: void addSource(JNIEnv&, jni::Object<Source>, jlong nativePtr); - jni::Object<Source> removeSourceById(JNIEnv&, jni::String); - void removeSource(JNIEnv&, jni::Object<Source>, jlong nativePtr); void addImage(JNIEnv&, jni::String, jni::jint, jni::jint, jni::jfloat, jni::Array<jbyte>); diff --git a/platform/android/src/style/sources/source.cpp b/platform/android/src/style/sources/source.cpp index 8a872f753d..93f04ced20 100644 --- a/platform/android/src/style/sources/source.cpp +++ b/platform/android/src/style/sources/source.cpp @@ -13,130 +13,148 @@ // C++ -> Java conversion #include "../conversion/property_value.hpp" +#include <mbgl/style/sources/geojson_source.hpp> +#include <mbgl/style/sources/image_source.hpp> +#include <mbgl/style/sources/raster_source.hpp> +#include <mbgl/style/sources/vector_source.hpp> + +#include "geojson_source.hpp" +#include "image_source.hpp" +#include "raster_source.hpp" +#include "unknown_source.hpp" +#include "vector_source.hpp" +#include "custom_geometry_source.hpp" + #include <string> namespace mbgl { namespace android { - /** - * Invoked when the construction is initiated from the jvm through a subclass - */ - Source::Source(jni::JNIEnv&, std::unique_ptr<mbgl::style::Source> coreSource) - : ownedSource(std::move(coreSource)) - , source(*ownedSource) { +static unique_ptr<Source> createSourcePeer(jni::JNIEnv& env, mbgl::style::Source& coreSource, AndroidRendererFrontend& frontend) { + if (coreSource.is<mbgl::style::VectorSource>()) { + return std::make_unique<VectorSource>(env, *coreSource.as<mbgl::style::VectorSource>(), frontend); + } else if (coreSource.is<mbgl::style::RasterSource>()) { + return std::make_unique<RasterSource>(env, *coreSource.as<mbgl::style::RasterSource>(), frontend); + } else if (coreSource.is<mbgl::style::GeoJSONSource>()) { + return std::make_unique<GeoJSONSource>(env, *coreSource.as<mbgl::style::GeoJSONSource>(), frontend); + } else if (coreSource.is<mbgl::style::ImageSource>()) { + return std::make_unique<ImageSource>(env, *coreSource.as<mbgl::style::ImageSource>(), frontend); + } else { + return std::make_unique<UnknownSource>(env, coreSource, frontend); } +} - Source::Source(mbgl::style::Source& coreSource) - : source(coreSource) { +jni::Object<Source> Source::peerForCoreSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, AndroidRendererFrontend& frontend) { + if (coreSource.peer.has_value()) { + return mbgl::util::any_cast<std::unique_ptr<Source>&>(coreSource.peer)->javaPeer; + } else { + return (coreSource.peer = createSourcePeer(coreSource))->javaPeer; } - - Source::~Source() { - // Before being added to a map, the Java peer owns this C++ peer and cleans - // up after itself correctly through the jni native peer bindings. - // After being added to the map, the ownership is flipped and the C++ peer has a strong reference - // to it's Java peer, preventing the Java peer from being GC'ed. - // In this case, the core source initiates the destruction, which requires releasing the Java peer, - // while also resetting it's nativePtr to 0 to prevent the subsequent GC of the Java peer from - // re-entering this dtor. - if (ownedSource.get() == nullptr && javaPeer.get() != nullptr) { - //Manually clear the java peer - android::UniqueEnv env = android::AttachEnv(); - static auto nativePtrField = javaClass.GetField<jlong>(*env, "nativePtr"); - javaPeer->Set(*env, nativePtrField, (jlong) 0); - javaPeer.reset(); - } +} + +Source::Source(jni::JNIEnv&, std::unique_ptr<mbgl::style::Source> coreSource) + : ownedSource(std::move(coreSource)) + , source(*ownedSource) + , frontend(nullptr) { +} + +Source::Source(jni::JNIEnv&, mbgl::style::Source& coreSource, jni::Object<Source> javaPeer_, AndroidRendererFrontend& frontend_) + : source(coreSource) + , javaPeer(javaPeer_.NewGlobalRef()) + , frontend(&frontend_) { +} + +Source::~Source() { + // Before being added to a map, the Java peer owns this C++ peer and cleans + // up after itself correctly through the jni native peer bindings. + // After being added to the map, the ownership is flipped and the C++ peer has a strong reference + // to it's Java peer, preventing the Java peer from being GC'ed. + // In this case, the core source initiates the destruction, which requires releasing the Java peer, + // while also resetting it's nativePtr to 0 to prevent the subsequent GC of the Java peer from + // re-entering this dtor. + if (!ownedSource) { + assert(javaPeer); + android::UniqueEnv env = android::AttachEnv(); + static auto nativePtrField = javaClass.GetField<jlong>(*env, "nativePtr"); + javaPeer->Set<jlong>(*env, nativePtrField, 0); + javaPeer.reset(); } +} - style::Source& Source::get() { - return source; - } +style::Source& Source::getCoreSource() { + return source; +} - jni::String Source::getId(jni::JNIEnv& env) { - return jni::Make<jni::String>(env, source.getID()); - } +jni::String Source::getId(jni::JNIEnv& env) { + return jni::Make<jni::String>(env, source.getID()); +} - jni::String Source::getAttribution(jni::JNIEnv& env) { - auto attribution = source.getAttribution(); - return attribution ? jni::Make<jni::String>(env, attribution.value()) : jni::Make<jni::String>(env,""); - } - - void Source::addToMap(JNIEnv& env, jni::Object<Source> obj, mbgl::Map& map) { - // Check to see if we own the source first - if (!ownedSource) { - throw std::runtime_error("Cannot add source twice"); - } +jni::String Source::getAttribution(jni::JNIEnv& env) { + auto attribution = source.getAttribution(); + return attribution ? jni::Make<jni::String>(env, attribution.value()) : jni::Make<jni::String>(env,""); +} - // Add source to map and release ownership - map.getStyle().addSource(releaseCoreSource()); - attachJavaSource(env, obj); +void Source::addToMap(JNIEnv& env, jni::Object<Source> obj, mbgl::Map& map, AndroidRendererFrontend& frontend_) { + // Check to see if we own the source first + if (!ownedSource) { + throw std::runtime_error("Cannot add source twice"); } - void Source::removeFromMap(JNIEnv&, jni::Object<Source>, mbgl::Map& map) { - //Cannot remove if not attached yet - if (ownedSource) { - throw std::runtime_error("Cannot remove source detached source"); - } + // Add source to map and release ownership. + map.getStyle().addSource(std::move(ownedSource)); - // Remove the source from the map - std::unique_ptr<mbgl::style::Source> coreSource = map.getStyle().removeSource(source.getID()); - attachCoreSource(std::move(coreSource)); - } + // Transfer C++ peer ownership from Java peer to core source. + source.peer = std::unique_ptr<Source>(this); - jni::jobject* Source::attachCoreSource(std::unique_ptr<style::Source> coreSource) { - // Take over the source - ownedSource = std::move(coreSource); + // Add strong reference to Java peer. + javaPeer = obj.NewGlobalRef(env); - // Release the peer relationships. These will be re-established when the source is added to a map - if (ownedSource->peer.has_value()) { - util::any_cast<std::unique_ptr<Source>>(&(ownedSource->peer))->release(); - ownedSource->peer = nullptr; - } + rendererFrontend = &frontend_; +} - return (jni::jobject *) javaPeer.release()->Get(); +void Source::removeFromMap(JNIEnv&, jni::Object<Source>, mbgl::Map& map) { + // Cannot remove if not attached yet + if (ownedSource) { + throw std::runtime_error("Cannot remove source detached source"); } - void Source::attachJavaSource(JNIEnv& env, jni::Object<Source> obj) { - // Add peer to core source - source.peer = std::unique_ptr<Source>(this); + // Remove the source from the map and take ownership. + ownedSource = map.getStyle().removeSource(source.getID()); - // Add strong reference to java source - javaPeer = obj.NewGlobalRef(env); - } - - void Source::setRendererFrontend(AndroidRendererFrontend& frontend_) { - rendererFrontend = &frontend_; - } + // Transfer C++ peer ownership from core source to Java peer. + assert(ownedSource->peer.has_value()); + util::any_cast<std::unique_ptr<Source>&>(ownedSource->peer).release(); + ownedSource->peer.reset(); - std::unique_ptr<mbgl::style::Source> Source::releaseCoreSource() { - assert(ownedSource != nullptr); - return std::move(ownedSource); - } + // Release strong reference to Java peer. + assert(javaPeer); + javaPeer.release(); - jni::jobject* Source::getJavaPeer(jni::JNIEnv& env) { - if(!javaPeer) { - // Create Java peer for sources already in the map - auto jp = jni::Object<Source>(createJavaPeer(env)); - attachJavaSource(env, jp); - jni::DeleteLocalRef(env, jp); - } - return (jni::jobject *) javaPeer->Get(); - } + rendererFrontend = nullptr; +} - jni::Class<Source> Source::javaClass; +jni::Class<Source> Source::javaClass; - void Source::registerNative(jni::JNIEnv& env) { - // Lookup the class - Source::javaClass = *jni::Class<Source>::Find(env).NewGlobalRef(env).release(); +void Source::registerNative(jni::JNIEnv& env) { + // Lookup the class + Source::javaClass = *jni::Class<Source>::Find(env).NewGlobalRef(env).release(); - #define METHOD(MethodPtr, name) jni::MakeNativePeerMethod<decltype(MethodPtr), (MethodPtr)>(name) + #define METHOD(MethodPtr, name) jni::MakeNativePeerMethod<decltype(MethodPtr), (MethodPtr)>(name) - // Register the peer - jni::RegisterNativePeer<Source>(env, Source::javaClass, "nativePtr", - METHOD(&Source::getId, "nativeGetId"), - METHOD(&Source::getAttribution, "nativeGetAttribution") - ); + // Register the peer + jni::RegisterNativePeer<Source>(env, Source::javaClass, "nativePtr", + METHOD(&Source::getId, "nativeGetId"), + METHOD(&Source::getAttribution, "nativeGetAttribution") + ); - } + // Register subclasses + GeoJSONSource::registerNative(env); + ImageSource::registerNative(env); + RasterSource::registerNative(env); + UnknownSource::registerNative(env); + VectorSource::registerNative(env); + CustomGeometrySource::registerNative(env); +} } // namespace android } // namespace mbgl diff --git a/platform/android/src/style/sources/source.hpp b/platform/android/src/style/sources/source.hpp index d6dde17983..adc71df9dd 100644 --- a/platform/android/src/style/sources/source.hpp +++ b/platform/android/src/style/sources/source.hpp @@ -14,59 +14,44 @@ namespace android { class Source : private mbgl::util::noncopyable { public: - static constexpr auto Name() { return "com/mapbox/mapboxsdk/style/sources/Source"; }; static jni::Class<Source> javaClass; - static void registerNative(jni::JNIEnv&); + static jni::Object<Source> peerForCoreSource(jni::JNIEnv&, AndroidRendererFrontend&, mbgl::style::Source&); + /* - * Called when a Java object is created on the c++ side + * Called when a Java object is created for a core source that belongs to a map. */ - Source(mbgl::style::Source&); + Source(jni::JNIEnv&, mbgl::style::Source&, jni::Object<Source> javaPeer, AndroidRendererFrontend&); /* - * Called when a Java object was created from the jvm side + * Called when a Java object is created for a new core source that does not belong to a map. */ Source(jni::JNIEnv&, std::unique_ptr<mbgl::style::Source>); virtual ~Source(); - style::Source& get(); + style::Source& getCoreSource(); void addToMap(JNIEnv&, jni::Object<Source>, mbgl::Map&); - void removeFromMap(JNIEnv&, jni::Object<Source>, mbgl::Map&); - void attachJavaSource(JNIEnv& env, jni::Object<Source>); - jni::jobject* attachCoreSource(std::unique_ptr<style::Source>); - - void setRendererFrontend(AndroidRendererFrontend&); - - jni::jobject* getJavaPeer(jni::JNIEnv&); - - virtual jni::jobject* createJavaPeer(jni::JNIEnv&) = 0; - jni::String getId(jni::JNIEnv&); - jni::String getAttribution(jni::JNIEnv&); protected: - // Release the owned view and return it - std::unique_ptr<mbgl::style::Source> releaseCoreSource(); - - // Set on newly created sources until added to the map + // Set on newly created sources until added to the map. std::unique_ptr<mbgl::style::Source> ownedSource; - // Raw pointer that is valid until the source is removed from the map + // Raw pointer that is valid at all times. mbgl::style::Source& source; - // Set when the source is added to a map + // Set when the source is added to a map. jni::UniqueObject<Source> javaPeer; - // RendererFrontend pointer is valid only when - // added to the map + // RendererFrontend pointer is valid only when added to the map. AndroidRendererFrontend* rendererFrontend; }; diff --git a/platform/android/src/style/sources/sources.cpp b/platform/android/src/style/sources/sources.cpp deleted file mode 100644 index 173f5c705c..0000000000 --- a/platform/android/src/style/sources/sources.cpp +++ /dev/null @@ -1,69 +0,0 @@ -#include "sources.hpp" - -#include <mbgl/style/source.hpp> -#include <mbgl/style/sources/geojson_source.hpp> -#include <mbgl/style/sources/image_source.hpp> -#include <mbgl/style/sources/raster_source.hpp> -#include <mbgl/style/sources/vector_source.hpp> - -#include "source.hpp" -#include "geojson_source.hpp" -#include "image_source.hpp" -#include "raster_source.hpp" -#include "unknown_source.hpp" -#include "vector_source.hpp" -#include "custom_geometry_source.hpp" - -namespace { - - using namespace mbgl::android; - - Source* initializeSourcePeer(mbgl::style::Source& coreSource) { - Source* source; - if (coreSource.peer.has_value()) { - source = mbgl::util::any_cast<std::unique_ptr<Source>>(&coreSource.peer)->get(); - } else if (coreSource.is<mbgl::style::VectorSource>()) { - source = new VectorSource(*coreSource.as<mbgl::style::VectorSource>()); - } else if (coreSource.is<mbgl::style::RasterSource>()) { - source = new RasterSource(*coreSource.as<mbgl::style::RasterSource>()); - } else if (coreSource.is<mbgl::style::GeoJSONSource>()) { - source = new GeoJSONSource(*coreSource.as<mbgl::style::GeoJSONSource>()); - } else if (coreSource.is<mbgl::style::ImageSource>()) { - source = new ImageSource(*coreSource.as<mbgl::style::ImageSource>()); - } else { - source = new UnknownSource(coreSource); - } - - return source; - } -} // namespace - -namespace mbgl { -namespace android { - - -jni::jobject* createJavaSourcePeer(jni::JNIEnv& env, AndroidRendererFrontend& frontend, mbgl::style::Source& coreSource) { - std::unique_ptr<Source> peerSource = std::unique_ptr<Source>(initializeSourcePeer(coreSource)); - peerSource->setRendererFrontend(frontend); - jni::jobject* result = peerSource->getJavaPeer(env); - peerSource.release(); - return result; -} - -jni::jobject* removeSourceFromMap(std::unique_ptr<mbgl::style::Source>&& coreSource) { - std::unique_ptr<Source> peerSource = std::unique_ptr<Source>(initializeSourcePeer(*coreSource)); - return peerSource.release()->attachCoreSource(std::move(coreSource)); -} - -void registerNativeSources(jni::JNIEnv& env) { - Source::registerNative(env); - GeoJSONSource::registerNative(env); - ImageSource::registerNative(env); - RasterSource::registerNative(env); - UnknownSource::registerNative(env); - VectorSource::registerNative(env); - CustomGeometrySource::registerNative(env); -} - -} -} diff --git a/platform/android/src/style/sources/sources.hpp b/platform/android/src/style/sources/sources.hpp deleted file mode 100644 index b2ab3d78ee..0000000000 --- a/platform/android/src/style/sources/sources.hpp +++ /dev/null @@ -1,20 +0,0 @@ -#pragma once - -#include <mbgl/style/source.hpp> - -#include "source.hpp" -#include "../../android_renderer_frontend.hpp" - -#include <jni/jni.hpp> - -namespace mbgl { -namespace android { - - jni::jobject* createJavaSourcePeer(jni::JNIEnv&, AndroidRendererFrontend&, mbgl::style::Source&); - - jni::jobject* removeSourceFromMap(std::unique_ptr<mbgl::style::Source>&&); - - void registerNativeSources(jni::JNIEnv&); - -} // namespace android -} // namespace mbgl diff --git a/platform/android/src/style/sources/vector_source.cpp b/platform/android/src/style/sources/vector_source.cpp index 7fe45441bd..adc5dc107a 100644 --- a/platform/android/src/style/sources/vector_source.cpp +++ b/platform/android/src/style/sources/vector_source.cpp @@ -30,8 +30,8 @@ namespace android { ) { } - VectorSource::VectorSource(mbgl::style::VectorSource& coreSource) - : Source(coreSource) { + VectorSource::VectorSource(jni::JNIEnv& env, mbgl::style::VectorSource& coreSource, AndroidRendererFrontend& frontend) + : Source(env, coreSource, createJavaPeer(env), frontend) { } VectorSource::~VectorSource() = default; @@ -56,7 +56,7 @@ namespace android { jni::Class<VectorSource> VectorSource::javaClass; - jni::jobject* VectorSource::createJavaPeer(jni::JNIEnv& env) { + jni::Object<Source> VectorSource::createJavaPeer(jni::JNIEnv& env) { static auto constructor = VectorSource::javaClass.template GetConstructor<jni::jlong>(env); return VectorSource::javaClass.New(env, constructor, reinterpret_cast<jni::jlong>(this)); } diff --git a/platform/android/src/style/sources/vector_source.hpp b/platform/android/src/style/sources/vector_source.hpp index 509fe068d1..914bcb5519 100644 --- a/platform/android/src/style/sources/vector_source.hpp +++ b/platform/android/src/style/sources/vector_source.hpp @@ -19,7 +19,7 @@ public: VectorSource(jni::JNIEnv&, jni::String, jni::Object<>); - VectorSource(mbgl::style::VectorSource&); + VectorSource(jni::JNIEnv& env, mbgl::style::VectorSource&, AndroidRendererFrontend&); ~VectorSource(); @@ -28,7 +28,8 @@ public: jni::String getURL(jni::JNIEnv&); - jni::jobject* createJavaPeer(jni::JNIEnv&); +private: + jni::Object<Source> createJavaPeer(jni::JNIEnv&); }; // class VectorSource |