Skip to content

Commit 42f0f00

Browse files
alexcristicialexcristici
committed
Fixed synchronous geojson source by using OptionalActor for converter too, instead of Actor that was blocking the thread on which it was called.
1 parent a8622a3 commit 42f0f00

3 files changed

Lines changed: 18 additions & 151 deletions

File tree

‎platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.cpp‎

Lines changed: 12 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -49,38 +49,33 @@ GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, const jni::String& sourceId, cons
4949
: Source(env,
5050
std::make_unique<mbgl::style::GeoJSONSource>(jni::Make<std::string>(env, sourceId),
5151
convertGeoJSONOptions(env, options))),
52-
converter(std::make_unique<Actor<FeatureConverter>>(Scheduler::GetBackground(),
53-
source.as<style::GeoJSONSource>()->impl().getOptions())) {}
52+
converter(std::make_unique<OptionalActor<FeatureConverter>>(
53+
source.as<style::GeoJSONSource>()->isUpdateSynchronous(),
54+
Scheduler::GetBackground(),
55+
source.as<style::GeoJSONSource>()->impl().getOptions()
56+
)) {}
5457

5558
GeoJSONSource::GeoJSONSource(jni::JNIEnv& env, mbgl::style::Source& coreSource, AndroidRendererFrontend* frontend)
5659
: Source(env, coreSource, createJavaPeer(env), frontend),
57-
converter(std::make_unique<Actor<FeatureConverter>>(Scheduler::GetBackground(),
58-
source.as<style::GeoJSONSource>()->impl().getOptions())) {}
60+
converter(std::make_unique<OptionalActor<FeatureConverter>>(
61+
source.as<style::GeoJSONSource>()->isUpdateSynchronous(),
62+
Scheduler::GetBackground(),
63+
source.as<style::GeoJSONSource>()->impl().getOptions()
64+
)) {}
5965

6066
GeoJSONSource::~GeoJSONSource() = default;
6167

6268
void GeoJSONSource::setGeoJSONString(jni::JNIEnv& env, const jni::String& jString) {
6369
std::shared_ptr<std::string> json = std::make_shared<std::string>(jni::Make<std::string>(env, jString));
6470

65-
ActorRef<FeatureConverter> converterRef = converter->self();
71+
OptionalActorRef<FeatureConverter> converterRef = converter->self();
6672
Update::Converter converterFn = [converterRef, json](ActorRef<GeoJSONDataCallback> _callback) {
6773
converterRef.invoke(&FeatureConverter::convertJson, json, _callback);
6874
};
6975

7076
setAsync(converterFn);
7177
}
7278

73-
void GeoJSONSource::setGeoJSONStringSync(jni::JNIEnv& env, const jni::String& jString) {
74-
std::shared_ptr<std::string> json = std::make_shared<std::string>(jni::Make<std::string>(env, jString));
75-
76-
ActorRef<FeatureConverter> converterRef = converter->self();
77-
Update::Converter converterFn = [converterRef, json](ActorRef<GeoJSONDataCallback> _callback) {
78-
converterRef.ask(&FeatureConverter::convertJson, json, _callback).wait();
79-
};
80-
81-
setAsync(converterFn);
82-
}
83-
8479
void GeoJSONSource::setFeatureCollection(jni::JNIEnv& env, const jni::Object<geojson::FeatureCollection>& jFeatures) {
8580
setCollectionAsync(env, jFeatures);
8681
}
@@ -93,19 +88,6 @@ void GeoJSONSource::setGeometry(jni::JNIEnv& env, const jni::Object<geojson::Geo
9388
setCollectionAsync(env, jGeometry);
9489
}
9590

96-
void GeoJSONSource::setFeatureCollectionSync(jni::JNIEnv& env,
97-
const jni::Object<geojson::FeatureCollection>& jFeatures) {
98-
setCollectionSync(env, jFeatures);
99-
}
100-
101-
void GeoJSONSource::setFeatureSync(jni::JNIEnv& env, const jni::Object<geojson::Feature>& jFeature) {
102-
setCollectionSync(env, jFeature);
103-
}
104-
105-
void GeoJSONSource::setGeometrySync(jni::JNIEnv& env, const jni::Object<geojson::Geometry>& jGeometry) {
106-
setCollectionSync(env, jGeometry);
107-
}
108-
10991
void GeoJSONSource::setURL(jni::JNIEnv& env, const jni::String& url) {
11092
// Update the core source
11193
source.as<style::GeoJSONSource>()->setURL(jni::Make<std::string>(env, url));
@@ -195,7 +177,7 @@ void GeoJSONSource::setCollectionAsync(jni::JNIEnv& env, const jni::Object<JNITy
195177
auto global = jni::NewGlobal<jni::EnvAttachingDeleter>(env, jObject);
196178
auto object = std::make_shared<decltype(global)>(std::move(global));
197179

198-
ActorRef<FeatureConverter> converterRef = converter->self();
180+
OptionalActorRef<FeatureConverter> converterRef = converter->self();
199181
Update::Converter converterFn = [converterRef, object](ActorRef<GeoJSONDataCallback> _callback) {
200182
converterRef.invoke(&FeatureConverter::convertObject<JNIType>, object, _callback);
201183
};
@@ -233,19 +215,6 @@ void GeoJSONSource::setAsync(Update::Converter converterFn) {
233215
update->converterFn(update->callback->self());
234216
}
235217

236-
template <class JNIType>
237-
void GeoJSONSource::setCollectionSync(jni::JNIEnv& env, const jni::Object<JNIType>& jObject) {
238-
auto global = jni::NewGlobal<jni::EnvAttachingDeleter>(env, jObject);
239-
auto object = std::make_shared<decltype(global)>(std::move(global));
240-
241-
ActorRef<FeatureConverter> converterRef = converter->self();
242-
Update::Converter converterFn = [converterRef, object](ActorRef<GeoJSONDataCallback> _callback) {
243-
converterRef.ask(&FeatureConverter::convertObject<JNIType>, object, _callback).wait();
244-
};
245-
246-
setAsync(converterFn);
247-
}
248-
249218
jboolean GeoJSONSource::isUpdateSynchronous(jni::JNIEnv&) {
250219
return source.as<style::GeoJSONSource>()->isUpdateSynchronous();
251220
}
@@ -268,10 +237,6 @@ void GeoJSONSource::registerNative(jni::JNIEnv& env) {
268237
METHOD(&GeoJSONSource::setFeatureCollection, "nativeSetFeatureCollection"),
269238
METHOD(&GeoJSONSource::setFeature, "nativeSetFeature"),
270239
METHOD(&GeoJSONSource::setGeometry, "nativeSetGeometry"),
271-
METHOD(&GeoJSONSource::setGeoJSONStringSync, "nativeSetGeoJsonStringSync"),
272-
METHOD(&GeoJSONSource::setFeatureCollectionSync, "nativeSetFeatureCollectionSync"),
273-
METHOD(&GeoJSONSource::setFeatureSync, "nativeSetFeatureSync"),
274-
METHOD(&GeoJSONSource::setGeometrySync, "nativeSetGeometrySync"),
275240
METHOD(&GeoJSONSource::setURL, "nativeSetUrl"),
276241
METHOD(&GeoJSONSource::getURL, "nativeGetUrl"),
277242
METHOD(&GeoJSONSource::querySourceFeatures, "querySourceFeatures"),

‎platform/android/MapLibreAndroid/src/cpp/style/sources/geojson_source.hpp‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include "source.hpp"
44
#include <mbgl/style/sources/geojson_source.hpp>
5+
#include <mbgl/actor/optional_actor.hpp>
56
#include "../../geojson/geometry.hpp"
67
#include "../../geojson/feature.hpp"
78
#include "../../geojson/feature_collection.hpp"
@@ -55,11 +56,6 @@ class GeoJSONSource : public Source {
5556
void setFeature(jni::JNIEnv&, const jni::Object<geojson::Feature>&);
5657
void setGeometry(jni::JNIEnv&, const jni::Object<geojson::Geometry>&);
5758

58-
void setGeoJSONStringSync(jni::JNIEnv&, const jni::String&);
59-
void setFeatureCollectionSync(jni::JNIEnv&, const jni::Object<geojson::FeatureCollection>&);
60-
void setFeatureSync(jni::JNIEnv&, const jni::Object<geojson::Feature>&);
61-
void setGeometrySync(jni::JNIEnv&, const jni::Object<geojson::Geometry>&);
62-
6359
void setURL(jni::JNIEnv&, const jni::String&);
6460

6561
jni::Local<jni::Array<jni::Object<geojson::Feature>>> querySourceFeatures(jni::JNIEnv&,
@@ -79,7 +75,7 @@ class GeoJSONSource : public Source {
7975
std::unique_ptr<Update> awaitingUpdate;
8076
std::unique_ptr<Update> update;
8177
std::shared_ptr<ThreadPool> threadPool;
82-
std::unique_ptr<Actor<FeatureConverter>> converter;
78+
std::unique_ptr<OptionalActor<FeatureConverter>> converter;
8379

8480
template <class JNIType>
8581
void setCollectionAsync(jni::JNIEnv&, const jni::Object<JNIType>&);

‎platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/style/sources/GeoJsonSource.kt‎

Lines changed: 4 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -240,11 +240,7 @@ class GeoJsonSource : Source {
240240
return
241241
}
242242
checkThread()
243-
if (nativeIsUpdateSynchronous()) {
244-
nativeSetFeatureSync(feature)
245-
} else {
246-
nativeSetFeature(feature)
247-
}
243+
nativeSetFeature(feature)
248244
}
249245

250246
/**
@@ -260,11 +256,7 @@ class GeoJsonSource : Source {
260256
return
261257
}
262258
checkThread()
263-
if (nativeIsUpdateSynchronous()) {
264-
nativeSetGeometrySync(geometry)
265-
} else {
266-
nativeSetGeometry(geometry)
267-
}
259+
nativeSetGeometry(geometry)
268260
}
269261

270262
/**
@@ -285,11 +277,7 @@ class GeoJsonSource : Source {
285277
val featuresCopy: List<Feature> = ArrayList(features)
286278
featureCollection = FeatureCollection.fromFeatures(featuresCopy)
287279
}
288-
if (nativeIsUpdateSynchronous()) {
289-
nativeSetFeatureCollectionSync(featureCollection)
290-
} else {
291-
nativeSetFeatureCollection(featureCollection)
292-
}
280+
nativeSetFeatureCollection(featureCollection)
293281
}
294282

295283
/**
@@ -304,77 +292,7 @@ class GeoJsonSource : Source {
304292
return
305293
}
306294
checkThread()
307-
if (nativeIsUpdateSynchronous()) {
308-
nativeSetGeoJsonStringSync(json)
309-
} else {
310-
nativeSetGeoJsonString(json)
311-
}
312-
}
313-
314-
/**
315-
* Updates the GeoJson with a single feature. The update is performed synchronously,
316-
* so the data will be immediately visible and available to query when this method returns.
317-
*
318-
* @param feature the GeoJSON [Feature] to set
319-
*/
320-
@Deprecated("use {@link GeoJsonOptions#withSynchronousUpdate(Boolean)} to enable synchronous updates on construction")
321-
fun setGeoJsonSync(feature: Feature?) {
322-
if (detached) {
323-
return
324-
}
325-
checkThread()
326-
nativeSetFeatureSync(feature)
327-
}
328-
329-
/**
330-
* Updates the GeoJson with a single geometry. The update is performed synchronously,
331-
* so the data will be immediately visible and available to query when this method returns.
332-
*
333-
* @param geometry the GeoJSON [Geometry] to set
334-
*/
335-
@Deprecated("use {@link GeoJsonOptions#withSynchronousUpdate(Boolean)} to enable synchronous updates on construction")
336-
fun setGeoJsonSync(geometry: Geometry?) {
337-
if (detached) {
338-
return
339-
}
340-
checkThread()
341-
nativeSetGeometrySync(geometry)
342-
}
343-
344-
/**
345-
* Updates the GeoJson. The update is performed synchronously,
346-
* so the data will be immediately visible and available to query when this method returns.
347-
*
348-
* @param featureCollection the GeoJSON FeatureCollection
349-
*/
350-
@Deprecated("use {@link GeoJsonOptions#withSynchronousUpdate(Boolean)} to enable synchronous updates on construction")
351-
fun setGeoJsonSync(featureCollection: FeatureCollection?) {
352-
if (detached) {
353-
return
354-
}
355-
checkThread()
356-
if (featureCollection != null && featureCollection.features() != null) {
357-
val features = featureCollection.features()
358-
val featuresCopy: List<Feature> = ArrayList(features)
359-
nativeSetFeatureCollectionSync(FeatureCollection.fromFeatures(featuresCopy))
360-
} else {
361-
nativeSetFeatureCollectionSync(featureCollection)
362-
}
363-
}
364-
365-
/**
366-
* Updates the GeoJson. The update is performed synchronously,
367-
* so the data will be immediately visible and available to query when this method returns.
368-
*
369-
* @param json the raw GeoJson FeatureCollection string
370-
*/
371-
@Deprecated("use {@link GeoJsonOptions#withSynchronousUpdate(Boolean)} to enable synchronous updates on construction")
372-
fun setGeoJsonSync(json: String) {
373-
if (detached) {
374-
return
375-
}
376-
checkThread()
377-
nativeSetGeoJsonStringSync(json)
295+
nativeSetGeoJsonString(json)
378296
}
379297

380298
/**
@@ -568,18 +486,6 @@ class GeoJsonSource : Source {
568486
@Keep
569487
private external fun nativeSetGeometry(geometry: Geometry?)
570488

571-
@Keep
572-
private external fun nativeSetGeoJsonStringSync(geoJson: String)
573-
574-
@Keep
575-
private external fun nativeSetFeatureCollectionSync(geoJson: FeatureCollection?)
576-
577-
@Keep
578-
private external fun nativeSetFeatureSync(feature: Feature?)
579-
580-
@Keep
581-
private external fun nativeSetGeometrySync(geometry: Geometry?)
582-
583489
@Keep
584490
private external fun querySourceFeatures(filter: Array<Any>?): Array<Feature>
585491

0 commit comments

Comments
 (0)