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

Replace getPointAnnotationsInBounds() w/ queryPointAnnotations()#5165

Merged
boundsj merged 1 commit into
masterfrom
1ec5-query-annotation-5151
Aug 12, 2016
Merged

Replace getPointAnnotationsInBounds() w/ queryPointAnnotations()#5165
boundsj merged 1 commit into
masterfrom
1ec5-query-annotation-5151

Conversation

@1ec5
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 commented May 27, 2016

queryPointAnnotations() accepts a screen rectangle instead of a geographic bounding box. If you pass in the bounds (in screen coordinates) of a rotated, tilted map view, the query is restricted to the trapezoidal region visible in the map view.

Fixes #5151.

@jfirebaugh to review the C++ side. @tobrun to review the Java side, which I edited blind without a Java IDE. 😬

@1ec5 1ec5 added bug iOS Mapbox Maps SDK for iOS refactor macOS Mapbox Maps SDK for macOS Android Mapbox Maps SDK for Android Core The cross-platform C++ core, aka mbgl annotations Annotations on iOS and macOS or markers on Android labels May 27, 2016
@1ec5 1ec5 added this to the ios-v3.3.0 milestone May 27, 2016
@1ec5 1ec5 self-assigned this May 27, 2016
@1ec5 1ec5 changed the title Replaced getPointAnnotationsInBounds() w/ queryPointAnnotations() Replace getPointAnnotationsInBounds() w/ queryPointAnnotations() May 27, 2016
@1ec5
Copy link
Copy Markdown
Contributor Author

1ec5 commented May 27, 2016

This isn’t going to work: annotation views on iOS and marker views on Android work by inserting a point annotation that has no icon and is thus prevented from rendering at the GL level. But as the name suggests, queryRenderedFeatures() only returns GL-rendered features, so if the application is using only annotation views or marker views, this new method always returns an empty vector.

This approach will only work if we use querySourceFeatures(), which is unimplemented in mbgl but needed for parity with GL JS.

@1ec5
Copy link
Copy Markdown
Contributor Author

1ec5 commented May 27, 2016

It would be possible to hack around the issue by setting each annotation/marker view’s icon to a single transparent pixel. It’s unclear to me whether handling rotation and pitch is important enough to use such a hack.

Comment thread src/mbgl/map/map.cpp Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert(*feature.id <= std::numeric_limits<AnnotationID>::max())

… queryPointAnnotations()

queryPointAnnotations() accepts a screen rectangle instead of a geographic bounding box, so marker hit testing works at the edges of a rotated, tilted map view.

Fixes #5151.
@1ec5
Copy link
Copy Markdown
Contributor Author

1ec5 commented Jul 25, 2016

This will unblock #4838, #5467, #2082, and #5787.

@1ec5
Copy link
Copy Markdown
Contributor Author

1ec5 commented Jul 25, 2016

The issue in #5165 (comment) was resolved by associating every view-backed annotation with a spacer image: #5509.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Android Mapbox Maps SDK for Android annotations Annotations on iOS and macOS or markers on Android bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants