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

AnnotationManager::getPointAnnotationsInBounds() should map IDs to screen coordinates #4443

Closed
1ec5 opened this issue Mar 23, 2016 · 5 comments
Labels
performance Speed, stability, CPU usage, memory usage, or power usage
Milestone

Comments

@1ec5
Copy link
Contributor

1ec5 commented Mar 23, 2016

For view-based annnotations (#1784 and #3276) to be performant, especially on Android, AnnotationManager::getPointAnnotationsInBounds() needs to supply the screen coordinates of each annotation whose ID it returns. That way we can call getPointAnnotationsInBounds() on each region change and avoid a potentially expensive series of calls to Map::pixelForLatLng() for each annotation in the returned list of annotations.

/cc @tobrun

@1ec5 1ec5 added the performance Speed, stability, CPU usage, memory usage, or power usage label Mar 23, 2016
@1ec5 1ec5 added this to the ios-v3.3.0 milestone Mar 23, 2016
@1ec5
Copy link
Contributor Author

1ec5 commented Mar 23, 2016

This API would also greatly improve performance in the OS X SDK when any annotation supplies a tooltip or cursor, since we need to update tooltip and cursor rects on each region change, just like we intend to do for view-based annotations.

@jfirebaugh
Copy link
Contributor

Any implementation of this would likely just call Map::pixelForLatLng() for each annotation itself. Is there reason to believe this would be faster than having the SDK call it?

@1ec5
Copy link
Contributor Author

1ec5 commented Jun 2, 2016

This ticket has a big question mark on it because I filed it proacively before there were view-backed annotations to profile. @boundsj @tobrun, are you seeing this as a bottleneck currently? If not, we can close and move on.

@boundsj
Copy link
Contributor

boundsj commented Jun 2, 2016

@1ec5 @jfirebaugh the implementation in #4801 only uses getPointAnnotationsInBounds to hit test visible annotation views at a touch point. If I select annotations over and over again for 30 seconds, getPointAnnotationsInBounds does not even register in the time profiler. For the same user activity, on the other end of the spectrum is SMCalloutView:presentCalloutFromRect:inLayer:ofView: constrainedToLayer:animated at about 30% and most of that is is a transparent image mask operation in a CALayer.

For testing if a view is in the viewport we currently use CGRectContainsRect and avoid getPointAnnotationsInBounds entirely. The expensive parts of this approach are currently:

  • MGLAnnotationView:setCenter: 27%
  • _mbglMap->getCameraOptions(padding): 17.2%
  • _mbglMap->pixelForLatLng(latLng) (via MGLMapView:convertLatLng:toPointToView): 13.3%

@jfirebaugh
Copy link
Contributor

It sounds like the current implementation of getPointAnnotationsInBounds is not a bottleneck, so closing here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

No branches or pull requests

3 participants