Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement queryRenderedFeatures for heatmap layers #10996

Merged
merged 3 commits into from Sep 8, 2021
Merged

Conversation

mourner
Copy link
Member

@mourner mourner commented Sep 7, 2021

Closes #10957. Closes #9112. Implements queryRenderedFeatures support for heatmap layers by reusing the logic from the circle layer. Instead of always returning false, it now returns an event containing all features that influence the heatmap rendering in the queried geometry (which are essentially features within the heatmap-radius of it). Makes the following possible:

map.on('click', 'heatmap', (e) => {
  console.log(`Clicked the heatmap! Features influencing this point: ${e.features.length}`);
});

cc @samanpwbb @MaretIdris @ivovandongen

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • manually test the debug page
  • tagged @mapbox/map-design-team @mapbox/static-apis if this PR includes style spec API or visual changes
  • tagged @mapbox/gl-native if this PR includes shader changes or needs a native port
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Added queryRenderedFeatures support to heatmap layers.</changelog>

@mourner mourner marked this pull request as draft September 7, 2021 17:18
@mourner mourner marked this pull request as ready for review September 7, 2021 18:12
Copy link
Contributor

@rreusser rreusser left a comment

Choose a reason for hiding this comment

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

I gave this a test run and looked it over pretty carefully, and… it looks really good. 👏 Could possibly add a test, but not sure if that's worthwhile.

As for documentation a list of places where we could note this:

  • heatmap layer docs: perhaps an odd place, though people probably end up here if they want to create a heatmap.
  • Create a heatmap layer example: Could add even just an output that shows how many points were clicked so that people know they can query this and sort through the data, if desired.
  • click event docs: it doesn't currently mention these details, but it could be helpful to give a verbose list of what's outputted in the event data or which layers are supported.

Though it's quite possible I've missed a more appropriate place for this documentation.

@mourner
Copy link
Member Author

mourner commented Sep 7, 2021

Could possibly add a test, but not sure if that's worthwhile.

Yep, added query tests at the same time as you commented. 👍

Though it's quite possible I've missed a more appropriate place for this documentation.

Perhaps worth adding a sentence in the description of heatmap-radius, noting that queryRenderedFeatures will return features within this radius from the query point?

@rreusser
Copy link
Contributor

rreusser commented Sep 7, 2021

Perhaps worth adding a sentence in the description of heatmap-radius

Yes, that seems like a very reasonable place. I think someone interested in this subtlety will likely end up reading the style documentation pretty carefully. 👍

@arindam1993
Copy link
Contributor

Looks good to me.

I think there also would be a lot of value in adding the heatmap weight/color to the response, atleast for single point queries.
If we want to get fancy we could do it for box-queries too with some FBO rendering and gl.readPixels(), but probably not worth the complexity.

const size = this.paint.get('heatmap-radius').evaluate(feature, featureState);
return queryIntersectsCircle(
queryGeometry, geometry, transform, pixelPosMatrix, elevationHelper,
true, true, new Point(0, 0), size);
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL we don't have heatmap-translate I thought it was feature available across all layers.

@mourner
Copy link
Member Author

mourner commented Sep 8, 2021

I think there also would be a lot of value in adding the heatmap weight/color to the response, atleast for single point queries.

Possibly, and previously I assumed this would be required for implementing heatmap querying, but all use cases I encountered so far didn't need this, as described in #10957. Then there's this conceptual problem of what the density value would represent — in the context of interactive GL JS maps, it's used purely for visual purposes, so heatmap density or color at a particular point isn't very meaningful. So I suggest leaving this for future if use cases like this do arise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add queryRenderedFeatures support to heatmap layer map.on('mouseenter', ...) doesn't work for heatmap layer
3 participants