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

initial featuresAt implementation #3172

Closed
wants to merge 64 commits into from
Closed

initial featuresAt implementation #3172

wants to merge 64 commits into from

Conversation

incanus
Copy link
Contributor

@incanus incanus commented Dec 3, 2015

This is ready for implementation review, then it can be squashed. The goal here is a minimal featuresAt functionality with configurable radius (owing to use in touch gesture scenarios) that returns basic information about queries on style layers marked interactive.

Right now the values returned for each feature are:

  • Style layer name as string
  • Style source name as string
  • Properties as string keys & values

Couple things that can be added later:

  • Further qualifying features returned by query layer, feature type, or true point-in-polygon intersectionality as in GL JS.
  • More concrete and strongly-typed results, including style reference objects and feature geometries, when Runtime styling API #837 and Support GeoJSON sources #2161 land.

One note @jfirebaugh: I wanted to adjust your PointAnnotationImpl use of the BOOST_GEOMETRY_REGISTER_POINT_2D and BOOST_GEOMETRY_REGISTER_BOX macros to be more in line with my setup in interactive_features_impl.hpp and my adjustment of collision_tile.hpp. It should be straightforward but I just couldn't figure it out for named field types in the Boost R-tree indexing scheme.

Quick demo on iOS:

anigif-1449122597

Once this looks good and lands, I will add the Android bindings as well.

Fixes #352.

- return layer name as string + feature properties dictionary
- abstract r-tree & boost stuff into common header
- more typedefs for clarity
- more logging of parse box & interaction point
- move interactivity parsing ahead of any bucket type branching
doesn't matter if we respond to bucket->hasData() here; features still exist in tile
/** Query the visible map for features at a given point.
*
* @param point A point on screen, for example at a user gesture.
* @param radius A distance in which to query features around the point.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should clarify that the distance is measured in screen points, not a physical unit or coordinate delta.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@incanus
Copy link
Contributor Author

incanus commented Dec 15, 2015

Cleaned up the demo a bit so that it works on all POI layers, regardless of zoom level, to be a little more self-evident.

anigif-1450140277

@@ -5,6 +5,8 @@
#include <mbgl/map/source_info.hpp>

#include <mbgl/util/mat4.hpp>
#include <mbgl/util/interactive_features_impl.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be #include <mbgl/util/interactive_features.hpp> right?

@jfirebaugh
Copy link
Contributor

Tests?

@@ -26,6 +26,15 @@ typedef NS_ENUM(NSUInteger, MGLUserTrackingMode) {
MGLUserTrackingModeFollowWithCourse,
};

/** The style layer name for a queried map feature. */
extern NSString * const MGLFeatureLayerNameKey;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unlike MGLUserTrackingMode, these keys are only used by MGLMapView, so they can go in MGLMapView.h.

@1ec5
Copy link
Contributor

1ec5 commented Dec 15, 2015

Would it be straightforward to include coordinates (either screen or geographic coordinates) in each feature description? The use case would be MapKit-style clickable POIs: you’d want the callout to be anchored on the POI, not the tap point, which may be a small distance away.

@incanus
Copy link
Contributor Author

incanus commented Dec 15, 2015

We are dealing with more than just points here. Now that we support GeoJSON sources, the possibility exists to pass actual features, but the early intent here is the use case of tying to external identifier such as OSM ID. We can always make a more robust API in future, particularly when the style API lands. I'm going for basic functionality that I and others can iterate on going forward, but also meeting the use cases we are hearing about as blockers right now.

@1ec5
Copy link
Contributor

1ec5 commented Dec 15, 2015

👍

@1ec5
Copy link
Contributor

1ec5 commented Mar 25, 2016

GL JS has revamped (and renamed) the featuresAt feature in mapbox/mapbox-gl-js#2224.

@jfirebaugh
Copy link
Contributor

Closing here; we'll want an implementation that more closely follows the new paradigm established in Mapbox GL JS.

@jfirebaugh jfirebaugh closed this Mar 30, 2016
@jfirebaugh jfirebaugh deleted the 352-featuresAt branch April 29, 2016 01:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature GL JS parity For feature parity with Mapbox GL JS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants