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

Replace getPointAnnotationsInBounds() w/ queryPointAnnotations() #5165

Merged
merged 1 commit into from
Aug 12, 2016

Conversation

1ec5
Copy link
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
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
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.

ids.reserve(features.size());
std::transform(features.begin(), features.end(), std::back_inserter(ids), [](auto& feature) -> AnnotationID {
assert(feature.id);
assert(*feature.id <= UINT32_MAX);
Copy link
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
Contributor Author

1ec5 commented Jul 25, 2016

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

@1ec5
Copy link
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.

3 participants