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

Feature eventing (native equivalent of queryRenderedFeatures and querySourceFeatures) #352

Closed
hallahan opened this issue Jul 2, 2014 · 40 comments
Assignees
Labels
feature GL JS parity For feature parity with Mapbox GL JS

Comments

@hallahan
Copy link
Contributor

hallahan commented Jul 2, 2014

I've been playing with MapboxGL, and I'm trying to figure out how to handle click events to select features. Is this functionality built in?

Looking at the JavaScript in the several blog posts that use mapbox-gl-js, I am able to got the Point object from a click event. Then, I see a function map.featuresAt, but I am not able to get it to return any features...

Any clues? Regarding the native libraries, would this be in the Cocoa library?

@incanus
Copy link
Contributor

incanus commented Jul 2, 2014

It's not there yet on the native side, but it's coming. I could have sworn we had a ticket for this over here, but I guess not as I couldn't find it. So this is it :-)

@incanus
Copy link
Contributor

incanus commented Jul 2, 2014

This will get a Cocoa API, by the way. So you should be able to turn gestures at a CGPoint into an array or dictionary of vector features, which you could then turn around into both data introspection or style modification (feature highlighting, etc.). Something like how UTFGrid works over here: https://github.com/mapbox/mapbox-ios-sdk/blob/f47ae3d96a27661eb4f888660787bffdf0294492/MapView/Map/RMInteractiveSource.h#L115

@hallahan
Copy link
Contributor Author

hallahan commented Jul 2, 2014

Alright, I'll stand by. Thanks!

@hallahan
Copy link
Contributor Author

hallahan commented Apr 9, 2015

What is the status for a native featuresAt implementation? Is the ability to select a feature on the map going to be a feasible use case for native?

@incanus
Copy link
Contributor

incanus commented Apr 9, 2015

It will be eventually, but the focus for now has just been on point annotations, which exist in a runtime style layer and query based on ~50px square padding for touch gestures. We'll eventually port over featuresAt and allow querying of all of the style layers such as OSM base data.

@taylortrimble
Copy link

I am so pumped for featuresAt. Thanks all!

@hallahan
Copy link
Contributor Author

hallahan commented Apr 9, 2015

Being able to select OSM base data would be an extremely valuable feature. This is something I'd enjoy working on. Could you point me in the right direction?

@incanus
Copy link
Contributor

incanus commented Apr 9, 2015

Yes, I would check out the featuresAt functionality in gl-js. The basic idea is an R-tree that bins the features at parse time so that efficient spatial queries can be made at runtime in response to the "at" and a radius. The structures that exist now aren't indexing parsed features this way.

Point annotations do index based on tiles made at runtime as annotations are added and removed, which is how gestural touch of points on the map that are rendered in GL was possible.

@hallahan
Copy link
Contributor Author

hallahan commented Apr 9, 2015

It looks like you have boost's rtree in there. Are you using this in the same way as rbush in the JS implementation? Are you saying only points are added to a spatial index?

@hallahan
Copy link
Contributor Author

hallahan commented Apr 9, 2015

I'm poking around, and the only actual rtree usage I am seeing is in collision.hpp. It looks like it's being used to make sure labels don't collide?

https://github.com/mapbox/mapbox-gl-native/blob/master/src/mbgl/text/collision.hpp#L32

Would we want to use the same container as is being used here for map geometries? I know PostGIS uses geos. I've used JTS Topology Suite in Java, the lib geos was ported from.

http://stackoverflow.com/questions/9891374/is-boost-geometry-mature-enough

What do you guys think? Clearly this is not a trivial feature.

@incanus
Copy link
Contributor

incanus commented Nov 25, 2015

Beginnings of this in 139b9e7. Right now it's just inserting feature names (actually, name_en or unknown if not present) at tile parse time for style layers starting with poi (as an alternative to parsing interactive: true for now).

I realize we eventually want this to be style-aware (which it could fairly easily be) as well as eliminate the need for interactive: true in the style (mapbox/mapbox-gl-js#1479), but this is a start as I keep hitting needs for this and wanted to get my feet wet with boost::geometry::rtree a bit.

Next up is to plumb through interactive gestures such as we do with annotations right now. For debug purposes, we could even piggyback on / add results to the existing annotation query, at least for log-debugging. This should just be a fairly simple featureTree.query(boost::geometry::index::intersects()) in the style of what we do for annotations.

What we should end up with is tapping the map logging nearby POIs when present (i.e. at high zooms) as a proof of concept.

Parsing output right now:

added poi-parks-scalerank2 - River View Cemetery
added poi-parks-scalerank2 - Waverly Country Club
added poi-parks-scalerank2 - Lewis and Clark College
added poi-parks-scalerank2 - Lewis and Clark College
added poi-parks-scalerank2 - Tryon Creek State Natural Area
added poi-parks-scalerank2 - Sellwood Park
added poi-parks-scalerank2 - Powers Marine Park
added poi-parks-scalerank2 - Beth Israel Cemetery
added poi-parks-scalerank2 - Sellwood Riverfront Park
added poi-parks-scalerank2 - Greenwood Hills Cemetery
feature tree for 14,2609,5864 [poi-parks-scalerank2] has 10 members
added poi-scalerank2 - Waverly Country Club
added poi-scalerank2 - Sellwood Park
added poi-scalerank2 - Milwaukie High School
added poi-scalerank2 - Milwaukie Elementary School
feature tree for 14,2610,5864 [poi-scalerank2] has 4 members

This currently does not noticeably affect rendering performance, even though it's adding to the R-tree from inside the workers ahead of render. It does affect things when you add all layers, which is really the meat of mapbox/mapbox-gl-js#1479 — automatically parsing all features at a much larger scale efficiently.

@incanus
Copy link
Contributor

incanus commented Nov 26, 2015

Getting close here. Current blocker is the fact that we postpone parsing of symbol layers until dependencies are loaded, and thus far I've just been adding to a tile's feature tree during !partialParse. We also need to possibly expose style bucket features to the TileWorker, as this is where and when we build up tile feature trees during tile parse. Right now our native API can only add features to a bucket, then pass the buckets as results back to the tile data, then later the painter asks the buckets to render themselves. We never introspect the features. What I may do is have the buckets build up feature trees internally, exposed via a getter, and have the tile data aggregate their contents post-parse rather than try to look at the features straight out of the geometry tiles in duplicate of their primary parsing.

@incanus
Copy link
Contributor

incanus commented Nov 26, 2015

The downside to having buckets build up their own feature trees is duplication of code in each bucket type.

@incanus
Copy link
Contributor

incanus commented Nov 26, 2015

Also of note here: what does a native featuresAt actually return? Unlike JavaScript, styles and features as JSON (or GeoJSON) is not necessarily useful or efficient here, and there are so many strongly-typed objects involved on the native side that exposing all of those to platform clients doesn't necessarily make sense either.

@tobrun
Copy link
Member

tobrun commented Jul 18, 2016

I have started to work on implementing Android equivalent of #5110 in (will update with PR number once that lands). Before implementing the contents of #5110, I need to refactor the Android annotations API to match iOS more. As pointed out by @tmcw in #3288 and related, there are some fundamental issues with the current Android API. Matching the iOS API will not only help implementing this feature but will also make it easier for implementing parity features in the long run.

@1ec5
Copy link
Contributor

1ec5 commented Jul 18, 2016

Note that the iOS SDK is affected by #5201 just like the Android SDK is affected by #3288. If you do make MultiPoint a separate concept from Polyline and Polygon, it'll be a backwards-incompatible change. One alternative I proposed in #5201 (comment) was to create a new class to represent the GeoJSON concept of a multipoint, but call it something else.

@tobrun
Copy link
Member

tobrun commented Jul 19, 2016

@1ec5
Since we have seen some resistance on the name Multipoint, I would suggest naming this exact the same as the GeoJSON equivalent.

  • Do you have any suggestions for a name for the parent of Polygon, Polyline and Multipoint?

    FYI: this is the current inheritance scheme:

    Annotation > Shape > Marker
    Annotation > Shape > ? > Polygon
    Annotation > Shape > ? > PolyLine
    Annotation > Shape > ? > Multipoint

  • Multipoint is currently empty since all logic is encapsulated in ?, do you have the same issue on iOS or am I missing something here?

@1ec5
Copy link
Contributor

1ec5 commented Jul 19, 2016

The discussion in #5201 is specific to the iOS SDK, which has MGLMultiPoint for compatibility with MapKit. That class actually does have code behind it, privately, which is why it can’t just be a protocol. If you’re OK with a backwards-incompatible change in the Android SDK (which would require a semver-major bump), I suggest making all four classes direct children of Shape, for consistency with GeoJSON.

@incanus
Copy link
Contributor

incanus commented Jul 26, 2016

Breaking out port of querySourceFeatures into #5792 since it's of different priority than wrapping up the Android side of queryRenderedFeatures right now.

tobrun added a commit that referenced this issue Jul 27, 2016
tobrun added a commit that referenced this issue Jul 27, 2016
tobrun added a commit that referenced this issue Jul 27, 2016
…in MapboxMap. Exposed constructors of Marker, Polyline and Polygon.
tobrun added a commit that referenced this issue Jul 27, 2016
tobrun added a commit that referenced this issue Jul 27, 2016
tobrun added a commit that referenced this issue Jul 27, 2016
tobrun added a commit that referenced this issue Jul 27, 2016
tobrun added a commit that referenced this issue Jul 27, 2016
… for Android binding, removed Multipoint inheritance issue
tobrun added a commit that referenced this issue Jul 27, 2016
…quivalent, removed duplicate src file names
@tobrun
Copy link
Member

tobrun commented Aug 4, 2016

@ivovandongen created an android follow up issue in #5869.
Closing as all other items were ticketed out or resolved.

@tobrun tobrun closed this as completed Aug 4, 2016
ivovandongen added a commit that referenced this issue Aug 5, 2016
ivovandongen added a commit that referenced this issue Aug 8, 2016
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

No branches or pull requests

8 participants