From 4afb4438ebf0c8e7d4b4ae5fcfd10d363be6c3c7 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Wed, 30 Oct 2019 12:04:01 +0200 Subject: [android] Convert GeoJSON features to tiles in background Composing tiles from the GeoJSON features is an expensive operation that might block UI thread on updating the `GeoJsonSource` with the new data. This change moves tile composing to the background thread and thus unblocks the UI thread. --- include/mbgl/style/sources/geojson_source.hpp | 1 + .../android/src/style/sources/geojson_source.cpp | 82 ++++++++++------------ .../android/src/style/sources/geojson_source.hpp | 20 ++++-- src/mbgl/style/sources/geojson_source.cpp | 4 ++ 4 files changed, 56 insertions(+), 51 deletions(-) diff --git a/include/mbgl/style/sources/geojson_source.hpp b/include/mbgl/style/sources/geojson_source.hpp index c8dc65b912..549393e9b2 100644 --- a/include/mbgl/style/sources/geojson_source.hpp +++ b/include/mbgl/style/sources/geojson_source.hpp @@ -60,6 +60,7 @@ public: void setGeoJSONData(std::shared_ptr); optional getURL() const; + const GeoJSONOptions& getOptions() const; class Impl; const Impl& impl() const; diff --git a/platform/android/src/style/sources/geojson_source.cpp b/platform/android/src/style/sources/geojson_source.cpp index 5ff4864275..0eece4b1ad 100644 --- a/platform/android/src/style/sources/geojson_source.cpp +++ b/platform/android/src/style/sources/geojson_source.cpp @@ -44,18 +44,16 @@ namespace android { } GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, const jni::String& sourceId, const jni::Object<>& options) - : Source(env, std::make_unique( - jni::Make(env, sourceId), - convertGeoJSONOptions(env, options))) - , converter(std::make_unique>(Scheduler::GetBackground())) { - } + : Source(env, + std::make_unique(jni::Make(env, sourceId), + convertGeoJSONOptions(env, options))), + converter(std::make_unique>(Scheduler::GetBackground(), + source.as()->getOptions())) {} - GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, - mbgl::style::Source& coreSource, - AndroidRendererFrontend& frontend) - : Source(env, coreSource, createJavaPeer(env), frontend) - , converter(std::make_unique>(Scheduler::GetBackground())) { - } + GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, AndroidRendererFrontend& frontend) + : Source(env, coreSource, createJavaPeer(env), frontend), + converter(std::make_unique>(Scheduler::GetBackground(), + source.as()->getOptions())) {} GeoJSONSource::~GeoJSONSource() = default; @@ -63,7 +61,7 @@ namespace android { std::shared_ptr json = std::make_shared(jni::Make(env, jString)); - Update::Converter converterFn = [this, json](ActorRef _callback) { + Update::Converter converterFn = [this, json](ActorRef _callback) { converter->self().invoke(&FeatureConverter::convertJson, json, _callback); }; @@ -84,11 +82,11 @@ namespace android { void GeoJSONSource::setURL(jni::JNIEnv& env, const jni::String& url) { // Update the core source - source.as()->GeoJSONSource::setURL(jni::Make(env, url)); + source.as()->setURL(jni::Make(env, url)); } jni::Local GeoJSONSource::getURL(jni::JNIEnv& env) { - optional url = source.as()->GeoJSONSource::getURL(); + optional url = source.as()->getURL(); return url ? jni::Make(env, *url) : jni::Local(); } @@ -166,7 +164,7 @@ namespace android { auto global = jni::NewGlobal(env, jObject); auto object = std::make_shared(std::move(global)); - Update::Converter converterFn = [this, object](ActorRef _callback) { + Update::Converter converterFn = [this, object](ActorRef _callback) { converter->self().invoke(&FeatureConverter::convertObject, object, _callback); }; @@ -175,25 +173,23 @@ namespace android { void GeoJSONSource::setAsync(Update::Converter converterFn) { awaitingUpdate = std::make_unique( - std::move(converterFn), - std::make_unique>( - *Scheduler::GetCurrent(), - [this](GeoJSON geoJSON) { - // conversion from Java features to core ones finished - android::UniqueEnv _env = android::AttachEnv(); - - // Update the core source - source.as()->GeoJSONSource::setGeoJSON(geoJSON); - - // if there is an awaiting update, execute it, otherwise, release resources - if (awaitingUpdate) { - update = std::move(awaitingUpdate); - update->converterFn(update->callback->self()); - } else { - update.reset(); - } - }) - ); + std::move(converterFn), + std::make_unique>( + *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.as()->setGeoJSONData(std::move(geoJSONData)); + + // if there is an awaiting update, execute it, otherwise, release resources + if (awaitingUpdate) { + update = std::move(awaitingUpdate); + update->converterFn(update->callback->self()); + } else { + update.reset(); + } + })); // If another update is running, wait if (update) { @@ -230,12 +226,10 @@ namespace android { ); } - void FeatureConverter::convertJson(std::shared_ptr json, - ActorRef callback) { + void FeatureConverter::convertJson(std::shared_ptr json, ActorRef callback) { using namespace mbgl::style::conversion; android::UniqueEnv _env = android::AttachEnv(); - // Convert the jni object Error error; optional converted = parseGeoJSON(*json, error); @@ -243,23 +237,23 @@ namespace android { mbgl::Log::Error(mbgl::Event::JNI, "Error setting geo json: " + error.message); return; } - - callback.invoke(&Callback::operator(), *converted); + callback.invoke(&GeoJSONDataCallback::operator(), style::GeoJSONData::create(*converted, options)); } template - void FeatureConverter::convertObject(std::shared_ptr, jni::EnvAttachingDeleter>> jObject, ActorRef callback) { + void FeatureConverter::convertObject( + std::shared_ptr, jni::EnvAttachingDeleter>> jObject, + ActorRef callback) { using namespace mbgl::android::geojson; android::UniqueEnv _env = android::AttachEnv(); // Convert the jni object auto geometry = JNIType::convert(*_env, *jObject); - callback.invoke(&Callback::operator(), GeoJSON(geometry)); + callback.invoke(&GeoJSONDataCallback::operator(), style::GeoJSONData::create(geometry, options)); } - Update::Update(Converter _converterFn, std::unique_ptr> _callback) - : converterFn(std::move(_converterFn)) - , callback(std::move(_callback)) {} + Update::Update(Converter _converterFn, std::unique_ptr> _callback) + : converterFn(std::move(_converterFn)), callback(std::move(_callback)) {} } // namespace android } // namespace mbgl diff --git a/platform/android/src/style/sources/geojson_source.hpp b/platform/android/src/style/sources/geojson_source.hpp index e737e41924..e506191ceb 100644 --- a/platform/android/src/style/sources/geojson_source.hpp +++ b/platform/android/src/style/sources/geojson_source.hpp @@ -11,22 +11,28 @@ namespace mbgl { namespace android { -using Callback = std::function; +using GeoJSONDataCallback = std::function)>; -struct FeatureConverter { - void convertJson(std::shared_ptr, ActorRef); +class FeatureConverter { +public: + explicit FeatureConverter(style::GeoJSONOptions options_) : options(std::move(options_)) {} + void convertJson(std::shared_ptr, ActorRef); template - void convertObject(std::shared_ptr, jni::EnvAttachingDeleter>>, ActorRef); + void convertObject(std::shared_ptr, jni::EnvAttachingDeleter>>, + ActorRef); + +private: + style::GeoJSONOptions options; }; struct Update { - using Converter = std::function)>; + using Converter = std::function)>; Converter converterFn; - std::unique_ptr> callback; + std::unique_ptr> callback; - Update(Converter, std::unique_ptr>); + Update(Converter, std::unique_ptr>); }; class GeoJSONSource : public Source { diff --git a/src/mbgl/style/sources/geojson_source.cpp b/src/mbgl/style/sources/geojson_source.cpp index 79e7c23459..3832977cd4 100644 --- a/src/mbgl/style/sources/geojson_source.cpp +++ b/src/mbgl/style/sources/geojson_source.cpp @@ -45,6 +45,10 @@ optional GeoJSONSource::getURL() const { return url; } +const GeoJSONOptions& GeoJSONSource::getOptions() const { + return impl().getOptions(); +} + void GeoJSONSource::loadDescription(FileSource& fileSource) { if (!url) { loaded = true; -- cgit v1.2.1