featuresAt: replace includeGeometry with omitGeometry

[jfirebaugh]

For an API that promises to return GeoJSON objects, the default should be to return valid GeoJSON objects. Omitting the geometry as an optimization should be the non-default behavior.

[mourner]

The API should promise to answer what feature is under the mouse cursor. It's actually very rare to really need the geometry of a feature — can't really think of a good use case. But the performance hit is MASSIVE. So I'm in favor of keeping things as is, and change the expectations by improving the docs instead.

[jfirebaugh]

It's actually very rare to really need the geometry of a feature — can't really think of a good use case.

I disagree -- I think it's actually pretty common. I just filed this because I was looking at #1527, one of the most popular examples, where the geometry is desired. The #1 most popular example also requires it.

[mourner]

I feel like these examples are a bit artificial — calling featuresAt on mousemove for showing object info (like #1191) will be a much more common use case in real world. With geometry included by default for lines and polygons (points are ok), featuresAt performance is unusable on mousemove. I just want to make sure users can't shoot themselves in the foot too easily. And having consistency with featuresIn, which will degrade much more with the change, is another valid point brought up by @anandthakker.

[anandthakker]

This is probably a horrible suggestion, but I'll make it anyway: what about just renaming it to something like featureDataAt/In, and then having separate methods geoJsonDataAt/In? Hokey and verbose, but clearer?

[jfirebaugh]

I think we'll need to find a solution that's both user-friendly and performant. If the bottleneck is computing the geographic coordinates from VT coordinates, then we could have the worker return raw VT coordinates and make each feature's geometry property an enumerable getter that computes (and caches) the geographic coordinates. On the other hand, if the bottleneck is transferring the data from the worker, then we should investigate a solution using transferable objects.

[mourner]

@jfirebaugh that would be great. Full GeoJSON conversion happens regardless of includeGeometry option — see https://github.com/mapbox/mapbox-gl-js/blob/master/js/data/feature_tree.js#L62. The option just nullifies the geometry in the results. So the bottleneck is sending data back to the main thread — there a lot of data to send when you query OSM roads.

[ansis]

includeGeometry is dropped by #2224 in favour of lazy creation. The vectortile pbf is transferred to the main thread.