Marker ID might change after a map reload

[zugaldia]

The default OnMapChangedListener reloads the markers after a DID_FINISH_LOADING_MAP change event. This reload can produce a change of IDs in the markers because they're effectively removed and re-added:

if (annotation instanceof Marker) {
    Marker marker = (Marker) annotation;
    mNativeMapView.removeAnnotation(annotation.getId());
    long newId = mNativeMapView.addMarker(marker);
    marker.setId(newId);
}

We shouldn't do this because the hashCode of a marker depends on its id, and this change of id can bring unintended consequences. For example, markers won't work well as keys in a HashMap. If this is necessary, this must be documented, or a different hashCode method unaffected by id changes could be implemented.

Possibly related: #1488 which was fixed in #3049.

[1ec5]

In the iOS SDK, MGLMapView originally mapped annotation model objects to their IDs, but we reversed it in #3261 so that IDs are mapped to the objects (along with some metadata). The primary reason is that mbgl considers the ID to be the unique identifier of an annotation, not the annotation model object (which mbgl never sees).

[tnightingale]

As an example, I ran into this when implementing a MapView.OnInfoWindowClickListener for https://github.com/tnightingale/Mapbox/tree/geojson.

Markers were being added before the DID_FINISH_LOADING_MAP event. The marker ids were then used to index data associated with each marker (in this case GeoJSON Feature 'properties'). However when the InfoWindowClickListener fired none of the marker ids matched. I figured out that I needed to wait until DID_FINISH_LOADING_MAP before creating the markers but I assume that there are still other events that could trigger a reset of all marker ids?

[jiajiahan]

I also ran into this problem recently and spent half a day to find out this issue here.

My use case is like this:
I have an object of data that is associated with each marker. When a marker is clicked, I show some info related to the marker on the bottom of the screen rather than the normal info window. So I kept a mapping between the marker ids and my object list. The list of objects is persisted during screen reloads (rotation, switch, etc). When the view is redrawn, I add new markers for each object during map initialization. So normally the info will be shown the first time when the data were fetched by some user interaction. However, after a screen rotation, the marker click will not find any data associated with that id.

After seeing this issue, I added a workaround to add a OnMapchangedListener in which I added the markers. It took a while to find the root cause and really confusing to debug. I think the marker id should be fixed after creation or this behavior should be well documented in all the addMarker{s} methods.

[tobrun]

Doing some tests around this issue:

When creating the map and adding 3 markers:

Adding marker with id : 0
Adding marker with id : 1
Adding marker with id : 2

After an orientation change:

Restoring marker with id : 0
Restoring marker with id : 1
Restoring marker with id : 2

Note that the we used to be able to add markers before DID_FINISH_LOADING_MAP callback, but this is no longer the case with the 4.0.0 release. There we are triggering the getMapAsync callback after DID_FINISH_LOADING_MAP. The initial issue is no longer a problem, before closing I will take a look at the other use cases mentioned above.

[tobrun]

Retested the use case from @tnightingale and this one is resolved for the same reason as above:

When creating the map and adding 3 markers:

Adding marker with id : 0
Adding marker with id : 1
Adding marker with id : 2

Clicking them in the order as they were added:

Click on marker with id : 0
Click on marker with id : 1
Click on marker with id : 2

[tobrun]

@jiajiahan with the upcoming release for 4.0.0 you will be able to extend MarkerOptions and Marker. For an example see this Activity and the child classes of Marker and MarkerOptions.

[tobrun]

The initial issue is resolved and nothing actionable left to do here. Closing

[zugaldia]

Yay!

[vcaen]

@jiajiahan Indeed, Marker id are still changing whenever I move the map so I can't rely on it to map my data to it.

@tobrun The links for your exemples are dead, can you please update them. Thanks

[tobrun]

@vcaen I believe your issue is related to #4553.
You can work around this issue for now using the SNAPSHOT. Reach out if you have any other questions.

[vcaen]

@tobrun Thanks, that's indeed the same issue. By the way, I found a workaround by using the Marker's LngLat (marker.getPosition()) since there is few chances that two markers have the same position.