Mapbox-gl-native: Cannot select annotations that lie on a tile boundary

[jadar]

We have a bug where we're not able to select/tap some of our annotations on the map in our iOS app. After some head scratching, we found out that the annotation just happened to lie on a tile boundary! Stepping through the Mapbox code, it appears that annotationTagsInRect:, (called in annotationTagAtPoint:persistingResults:,) returns a std::vector of size 0 when there is clearly an annotation within the rect, except the annotation lies between a few tiles.

Steps to reproduce

1. Create and add/display a UIView-backed annotation on an MGLMapView
2. Place the annotation between a couple tiles. The lat/lon we can reproduce the issue with are (36.38149043210595, -119.59605216979982).
3. Try to tap on it.

Expected behavior

The annotation is detected and the relevant delegate methods are called.

Actual behavior

No delegate methods are called.

_While the screenshot says "THIS WILL WORK", it actually means it won't work._

Configuration

Mapbox SDK versions: 4.2.0
iOS/macOS versions: iOS 11.4
Device/simulator models: iPhone 8 and iPhone X
Xcode version: Xcode 9.4.1

[ChrisLoer]

Hi @jadar thanks for the report! Is there any chance you could post some example code reproducing the problem? What you're describing is a class of bug we've had before, and we believe we've fixed. There may be some subtle difference in how you're exercising the functionality that still causes the problem to manifest.

[jadar]

@ChrisLoer Here is a bare-bones implementation that reproduces the bug. If you zoom in to where the scale shows ~100ft, it won't show a callout. If you zoom out to where the scale shows ~600 ft, the callout shows.
https://gist.github.com/jadar/7a8f8454b78a4168f5724c5c61de6b6b

[ChrisLoer]

Hi @jadar I'm sorry but I'm still not seeing the problem. Here's what I see when I tap on the annotation created with your code pasted into my example app (I moved the location to null island i.e. [0,0] just to be sure it was on a tile boundary, but I couldn't reproduce with your original coordinates either):

[jadar]

Hmm, I can't seem to reproduce it at [0,0] either. When I use the coordinates I gave, it works up until zoom level 16. 15 and below work correctly.

I don't know if it changes anything, but I'm using this style, and the 4.2.0 version of Mapbox.
mapbox://styles/mapbox/satellite-streets-v9
https://github.com/mapbox/mapbox-gl-native/releases/download/ios-v4.2.0/mapbox-ios-sdk-4.2.0-dynamic.zip

[jadar]

It also seems that with [0.000000000000001, 0.00000000000001], it doesn't work at any zoom level.

[ChrisLoer]

Got it, thanks! I can reproduce now at [0.000000000000001, 0.00000000000001]. I'll take a closer look tomorrow and hopefully get to the bottom of it.

[ChrisLoer]

This is a good bug! 😅

The problem is that when the AnnotationManager decides which tile the annotation at [0.000000000000001, 0.00000000000001] should go in, it correctly chooses the "northeast" tile, based on LatLng coordinates:

https://github.com/mapbox/mapbox-gl-native/blob/8400355e996a40014f774d443a0d5deab41d705e/src/mbgl/annotation/annotation_manager.cpp#L142-L147

However, it then translates into tile coordinates:

https://github.com/mapbox/mapbox-gl-native/blob/8400355e996a40014f774d443a0d5deab41d705e/src/mbgl/annotation/symbol_annotation_impl.cpp#L18-L21

TileCoordinate::fromLatLng(0, latLng) returns [0.5, 0.5] (using a coordinate system that represents the whole world in web mercator projection with positions from 0 to 1). You can see the low bits have already been lost (it should really be something like [.5000000000001, .4999999999999]. TileCoordinate::toGeometryCoordinate then returns [0,8192] -- that is, the point that's just off the bottom edge of the tile (the tile goes from [0,0] to [8191,8191].

I haven't actually traced the problem from here all the way back to the annotationsInRect issue, but I'm fairly confident this is the root issue.

@mourner, do you have any ideas about how we should fix this? The quick and dirty fix would be to just build a little tolerance into getTileData() and allow duplicates along the tile edges, but it seems like it would be more consistent to transform to some sort of integer-based coordinate system at the point we insert into the SymbolAnnotationTree, so that we don't have to worry about precision issues after that?

[mourner]

@ChrisLoer yeah, it might make sense to use a global integer coordinates system based on web mercator. Uint32 only allows for values up to z19 (32 - log2(8192)), so this would be need to be Uint64.

[ChrisLoer]

Fixed in #12570.