Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Commit

Permalink
Fix the race condition when updating LocationCompoent's position. (#703)
Browse files Browse the repository at this point in the history
* Fix the race condition when updating LocationCompoent's position.

When using SymbolLayer and GeoJsonSource backed location puck, there's high frequency
modifications done on the main thread and locationFeature is always re-used (modified)
while being parsed on the worker thread. As the `setGeoJson` method is not thread-safe,
we should do a deep copy of the location feature before passing setting it to GeoJsonSource.

* Fix unit test.
  • Loading branch information
pengdev authored Sep 24, 2021
1 parent 3467eef commit 096a0bd
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ private void refreshSource() {

GeoJsonSource source = style.getSourceAs(LOCATION_SOURCE);
if (source != null) {
locationSource.setGeoJson(locationFeature);
locationSource.setGeoJson(locationFeature.toJson());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,9 @@ public GeoJsonSource(String id, Geometry geometry, GeoJsonOptions options) {
* Updates the GeoJson with a single feature. The update is performed asynchronously,
* so the data won't be immediately visible or available to query when this method returns.
*
* Note: This method is not thread-safe. The Feature is parsed on a worker thread, please make sure
* the Feature is immutable.
*
* @param feature the GeoJSON {@link Feature} to set
*/
public void setGeoJson(Feature feature) {
Expand All @@ -270,6 +273,9 @@ public void setGeoJson(Feature feature) {
* Updates the GeoJson with a single geometry. The update is performed asynchronously,
* so the data won't be immediately visible or available to query when this method returns.
*
* Note: This method is not thread-safe. The Geometry is parsed on a worker thread, please make sure
* the Geometry is immutable.
*
* @param geometry the GeoJSON {@link Geometry} to set
*/
public void setGeoJson(Geometry geometry) {
Expand All @@ -284,6 +290,9 @@ public void setGeoJson(Geometry geometry) {
* Updates the GeoJson. The update is performed asynchronously,
* so the data won't be immediately visible or available to query when this method returns.
*
* Note: This method is not thread-safe. The FeatureCollection is parsed on a worker thread, please make sure
* the FeatureCollection is immutable.
*
* @param featureCollection the GeoJSON FeatureCollection
*/
public void setGeoJson(@Nullable FeatureCollection featureCollection) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,14 +536,15 @@ public void onNewLatLngValue_locationFeatureIsUpdated() {
LayerBitmapProvider bitmapProvider = mock(LayerBitmapProvider.class);
LocationComponentOptions options = mock(LocationComponentOptions.class);
Feature locationFeature = mock(Feature.class);
when(locationFeature.toJson()).thenReturn("expected_geojson_string");
LocationLayerController layer = new LocationLayerController(
mapboxMap, mapboxMap.getStyle(), sourceProvider, buildFeatureProvider(locationFeature, options),
bitmapProvider, options, internalRenderModeChangedListener, internalIndicatorPositionChangedListener, false);

getAnimationListener(ANIMATOR_LAYER_LATLNG, layer.getAnimationListeners()).onNewAnimationValue(new LatLng());

// wanted twice (once for initialization)
verify(locationSource, times(2)).setGeoJson(locationFeature);
verify(locationSource, times(2)).setGeoJson("expected_geojson_string");
verify(internalIndicatorPositionChangedListener).onIndicatorPositionChanged(any(Point.class));
}

Expand Down

0 comments on commit 096a0bd

Please sign in to comment.